feat(bpdm): new api endpoint to upload business partner data via csv file #951
Conversation
| ApiResponse(responseCode = "400", description = "On malformed legal entity request", content = [Content()]), | ||
| ] | ||
| ) | ||
| @PostMapping("/input/business-partners/uploadCsv", consumes = ["multipart/form-data"]) |
There was a problem hiding this comment.
REST Conformity: From this release on, I propose to get our REST API definitions in order and more closely follow the REST principles. In this spirit, paths should lead to resources and these resources should be named as proper nouns. So I would suggest the following changes:
- Find a proper noun resource name for the CSV-Upload resource
- give it it's own path independent of "business-partners". Either keep it under "input" path or even put the resource in the root path (both solutions have arguments for and against, I would be OK with both)
There was a problem hiding this comment.
With this comment,
Sure, I will use proper noun resource name for the CSV-upload.
But just wanted to confirm, are you suggesting me to create different api client and controllers as well? I covered it under gate business partner api as ultimately business partner data need to be uploaded.
There was a problem hiding this comment.
It is arguable if it ultimately really just business partner that is uploaded. From a REST perspective you create a new upload process resource. Assuming the resource would be called "partner-upload-process" (just an example) then I would see two possible paths where this resource is available:
input/partner-upload-process
or
partner-upload-process
Regarding controller: I would suggest to create a new controller for that resource. It will make endpoint annotations pretty straight forward and it establishes more clearly that this upload is its own resource.
There was a problem hiding this comment.
I agree it is arguable, what I interpreted from requirement currently is just business partner that is going to uploaded.
May be once feature is ready then we can come to concrete solution, but regarding resource noun and controller I can find feasibility and keep it ready on this branch at least.
| ApiResponse(responseCode = "400", description = "On malformed legal entity request", content = [Content()]), | ||
| ] | ||
| ) | ||
| @PostMapping("/input/business-partners/uploadCsv", consumes = ["multipart/form-data"]) |
There was a problem hiding this comment.
Upload Functionality:
From the discussed requirements (that we should put into an issue somewhere here) it sounds to me that it should be possible to upload also more business partners than just the API limit. This suggests to me, that this endpoint should therefore be an asynchronous operation.
For an asynchronous operation the POST endpoint should immediately return the creation of the upload process job. Also there should be a GET on this resource to get the current status (initial, pending, success, error) as well as (intermediate) results and/or errors.
| @@ -66,4 +69,22 @@ class BusinessPartnerController( | |||
| ): PageDto<BusinessPartnerOutputDto> { | |||
| return businessPartnerService.getBusinessPartnersOutput(paginationRequest.toPageRequest(), externalIds, getCurrentUserBpn()) | |||
| } | |||
|
|
|||
| @PreAuthorize("hasAuthority(${PermissionConfigProperties.WRITE_INPUT_PARTNER})") | |||
There was a problem hiding this comment.
see REST API conformity:
Best each resource has its own permissions for each operation. read for GET and create/write for POST
For this, the applicationConfig needs to be adjusted. Also the default realm files need to be adjusted. (Ideally in both Helm Charts and Tests)
| val csvData: List<Map<String, String>> = CsvFileUtils.parseCsv(file) | ||
| // Validate the CSV data | ||
| for (rowData in csvData) { | ||
| if (rowData["externalId"].isNullOrBlank() || rowData["legalEntity.legalEntityBpn"].isNullOrBlank()) { |
There was a problem hiding this comment.
I feel with such a low-level tool the CSV will be soon too much to handle.
What about using this tool with bean based binding: https://www.baeldung.com/opencsv ?
| val businessPartnerDtos = CsvFileUtils.mapToBusinessPartnerRequests(csvData) | ||
| logger.debug { "Executing processFile() with parameters $businessPartnerDtos" } | ||
| val businessPartnerEntities = businessPartnerDtos.map { dto -> businessPartnerMappings.toBusinessPartnerInput(dto, ownerBpnl) } | ||
| businessPartnerRepository.saveAll(businessPartnerEntities) |
There was a problem hiding this comment.
As already discussed: Better use the service here to make sure that the business partner input is processed as normal.
Description
In this pull request, we have added new api endpoint which will accept the CSV file as input for upsetting business partner input data from BPDM Gate service.
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review: