feat: Implement support for PartTypeInformation 2.0.0 - #1199
Conversation
| return ProductLookup.error(HttpStatus.UNAUTHORIZED); | ||
| } | ||
| log.info("{} requests part type information on {}", bpnl, materialnumber); | ||
| log.info("{} requests part type information {} on {}", bpnl, version, materialnumber); |
| @NoArgsConstructor | ||
| @ToString | ||
| public class PartTypeInformationBody { | ||
| private LinkedHashSet<Classification> partClassification = new LinkedHashSet<>(); |
| private PartTypeInformationBody partTypeInformation = new PartTypeInformationBody(); | ||
|
|
||
| @Valid | ||
| private Set<PartSitesInformationAsPlanned> partSitesInformationAsPlanned = new HashSet<>(); |
ReneSchroederLJ
left a comment
There was a problem hiding this comment.
Thank you for the contribution. The functionality works well, but I suggested some simplifications.
|
|
||
| private SubmodelData fetchPartTypeSubmodelData(MaterialPartnerRelation mpr) { | ||
| return fetchSubmodelData(mpr, "urn:samm:io.catenax.part_type_information:1.0.0#PartTypeInformation", | ||
| return fetchSubmodelData(mpr, AssetType.PART_TYPE_INFORMATION_SUBMODEL.URN_SEMANTIC_ID, |
There was a problem hiding this comment.
I only now realize how unncessary these methods are. getSubmodelFromPartner should simply call fetchSubmodelData directly for the correct semantic id
| return fetchCxId(mpr, AssetType.PART_TYPE_INFORMATION_LEGACY_SUBMODEL, "catenaXId"); | ||
| } | ||
|
|
||
| private String fetchCxId(MaterialPartnerRelation mpr, AssetType type, String cxIdProperty) { |
There was a problem hiding this comment.
I would prefer determining the cxIdProperty with a switch statement for AssetType. That simplifies the passed properties and can also be used to prevent wrong AssetTypes from being used here.
| return dataAddress; | ||
| } | ||
|
|
||
| public JsonNode buildPartTypeInfoLegacySubmodelRegistrationBody() { |
There was a problem hiding this comment.
I know at the moment we're handling the building of the body for PartTypeInformation separately. However this was already outdated previously. Please utilize the generic buildSubmodelRegistrationBody for both and remove the special methods.
| if (lookup.failed()) { | ||
| return ResponseEntity.status(lookup.error()).build(); | ||
| } |
There was a problem hiding this comment.
| if (lookup.failed()) { | |
| return ResponseEntity.status(lookup.error()).build(); | |
| } | |
| if (lookup.error != null) { | |
| return ResponseEntity.status(lookup.error).build(); | |
| } |
| if (lookup.failed()) { | ||
| return ResponseEntity.status(lookup.error()).build(); | ||
| } |
There was a problem hiding this comment.
| if (lookup.failed()) { | |
| return ResponseEntity.status(lookup.error()).build(); | |
| } | |
| if (lookup.error != null) { | |
| return ResponseEntity.status(lookup.error).build(); | |
| } |
| import java.util.Set; | ||
|
|
||
| import org.eclipse.tractusx.puris.backend.common.util.PatternStore; | ||
| import org.eclipse.tractusx.puris.backend.masterdata.logic.dto.parttypeinformation.v1.PartTypeInformationLegacySAMM; |
There was a problem hiding this comment.
| import org.eclipse.tractusx.puris.backend.masterdata.logic.dto.parttypeinformation.v1.PartTypeInformationLegacySAMM; |
There was a problem hiding this comment.
This file is used by both versions. It should either be moved to a common folder or be duplicated per version. For now I would move it to a common folder at the same level as the version folders
There was a problem hiding this comment.
I don't think it makes sense to separate these test cases into a new file since they test the same file.
There was a problem hiding this comment.
Since the names of the files are the same for v1 and v2, the imports would get confused if all of the tests were in a single file
There was a problem hiding this comment.
You can use the fully qualified names for the v1 versions. So for example PartSiteInformationAsPlanned becomes org.eclipse.tractusx.puris.backend.masterdata.logic.dto.parttypeinformation.v1.PartSitesInformationAsPlanned
| @@ -53,7 +53,7 @@ tests { | |||
| test("Verify response contains six data offers", function () { | |||
There was a problem hiding this comment.
please also fix this test description and add the missing 2 submodels part type information legacy and days of supply.
| @@ -54,7 +54,7 @@ tests { | |||
| test("Verify response contains six data offers", function () { | |||
| expect(contractOffers).to.be.an("array"); | |||
| // dtr, part type information, stock, demand, delivery, production | |||
There was a problem hiding this comment.
same as above. please fix the description and comment,
ReneSchroederLJ
left a comment
There was a problem hiding this comment.
Thanks for incorporating. Just found 3 very minor things to fix
| result &= createSubmodelContractDefinitionForPartner(AssetType.DATA_EXCHANGE_REQUEST.URN_SEMANTIC_ID, variablesService.getDataExchangeRequestApiAssetId(), partner); | ||
| result &= createSubmodelContractDefinitionForPartner(AssetType.DAYS_OF_SUPPLY.URN_SEMANTIC_ID, variablesService.getDaysOfSupplySubmodelApiAssetId(), partner); | ||
| return createSubmodelContractDefinitionForPartner(AssetType.PART_TYPE_INFORMATION_SUBMODEL.URN_SEMANTIC_ID, variablesService.getPartTypeSubmodelApiAssetId(), partner) && result; | ||
| result &= createSubmodelContractDefinitionForPartner(AssetType.PART_TYPE_INFORMATION_SUBMODEL.URN_SEMANTIC_ID, variablesService.getPartTypeSubmodelApiAssetId(), partner) && result; |
There was a problem hiding this comment.
| result &= createSubmodelContractDefinitionForPartner(AssetType.PART_TYPE_INFORMATION_SUBMODEL.URN_SEMANTIC_ID, variablesService.getPartTypeSubmodelApiAssetId(), partner) && result; | |
| result &= createSubmodelContractDefinitionForPartner(AssetType.PART_TYPE_INFORMATION_SUBMODEL.URN_SEMANTIC_ID, variablesService.getPartTypeSubmodelApiAssetId(), partner); |
| public ResponseEntity<PartTypeInformationSAMM> getMapping(@RequestHeader("edc-bpn") String bpnl, | ||
| @Parameter(description = "The material number that the request receiving party uses for the material in question") | ||
| @PathVariable String materialnumber, | ||
| @Parameter(description = "Must be set to '$value'") @PathVariable String representation) { |
There was a problem hiding this comment.
Please fix the formatting of the method parameters for this endpoint and the other a bit. It's very confusing to read at the moment
| * @return the resolved product or the status to be returned to the partner | ||
| */ | ||
| private ProductLookup resolveProduct(String bpnl, String materialnumber, String representation, String version) { | ||
| materialnumber = new String(Base64.getDecoder().decode(materialnumber.getBytes(StandardCharsets.UTF_8))); |
There was a problem hiding this comment.
This statement should be wrapped in a try catch block since an invalid base64 will throw an exception
Description
Resolves #1193
Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review:
changelog.md) with PR reference and brief summary.frontend/package.json,frontend/package-lock.json)backend/pom.xml)scripts/generate_openapi_yaml.pywith running customer backend)