Skip to content

Commit

Permalink
Merge pull request #435 from FraunhoferISST/fix/432-delivery
Browse files Browse the repository at this point in the history
fix(DeliveryRequestApiService): filter deliveries requested by partner role and incoterms
  • Loading branch information
tom-rm-meyer-ISST authored Jun 5, 2024
2 parents 9595354 + 7b448f3 commit c2d02c5
Show file tree
Hide file tree
Showing 10 changed files with 731 additions and 204 deletions.
82 changes: 82 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,88 @@ The **need for configuration updates** is **marked bold**.

### Added

* Added backend implementation to CRUD Demand and Capacity
Notifications ([#415](https://github.com/eclipse-tractusx/puris/pull/415))
* Added ERP response interface ([#418](https://github.com/eclipse-tractusx/puris/pull/418))

### Changed

* Fixed check when answering delivery information request from a customer which prevented always to
answer ([#435](https://github.com/eclipse-tractusx/puris/pull/435))

### Removed

N.A.

### Known Knowns

#### Security

The Backend is currently secured via API Key while the Frontend already uses an API-KEY.
See [Admin Guide](./docs/admin/Admin_Guide.md) for further information.

#### Upgradeability

As currently no active user was known migrations of data are not yet supported. The chart technically is upgradeable.

#### Data Sovereignty

For productive use the following enhancements are encouraged

* User FrontEnd available: Role Company Admin is able to query catalogue and see negotiations and transfers
But company rules / policies need to be configured upfront in backend (via postman) to enable automatic contract
negotiations, responsibility lies with Company Admin role
--> add section in the User Manual describing this and the (legal) importance and responsibility behind defining these
rules
* Currently only one standard policy per reg. connector / customer instance is supported (more precisely one for DTR,
one for all submodels), negotiation happens automatically based on this
--> enhance option to select partner and define specific policies (to be planned in context of BPDM Integration)
--> UI for specific configuration by dedicated role (e.g. Comp Admin) and more flexible policy configuration (without
code changes) is needed
* As a non-Admin user I do not have ability to view policies in detail --> transparency for users when interacting with
and requesting / consuming data via dashboard / views on underlying usage policies to be enhanced
* ContractReference Constraint or configuration of policies specific to one partner only not implemented -->
clarification of potential reference to "PURIS standard contract" and enabling of ContractReference for 24.08.
* unclear meaning of different stati in negotations --> add view of successfull contract agreeements wrt which data have
been closed
* current logging only done on info level --> enhance logging of policies (currently only available at debug level)
* in case of non-matching policies (tested in various scenarios) no negotiation takes place -->
**enhance visualization or specific Error message to user**
* no validation of the Schema "profile": "cx-policy:profile2405" (required to ensure interop with other PURIS apps)

#### Styleguide

Overall

* Brief description at the top of each page describing content would be nice for better user experience.

Dashboard

* Similar for Create Delivery (here SOME entries are reset but warnings stay) (**block**)

Stocks

* user needs better guidance to click on a stock to update it (else error prone to enter one
slightly different attribute and Add instead of update)
* Refresh -- update request has been sent successfully. -> more information regarding data transfer needed for user

Catalog

* No action possible -> unclear to user when and how user will consume an offer

Negotiations

* Add filters for transparency (bpnl, state)

## [v2.0.1](https://github.com/eclipse-tractusx/puris/releases/tag/2.0.1)

The following Changelog lists the changes. Please refer to the [documentation](docs/README.md) for configuration needs
and understanding the concept changes.

The **need for configuration updates** is **marked bold**.

### Added

* Added Footer

### Changed
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Beside the dependencies provided in the Helm Chart, the following dependencies h

| Application | App Version | Chart Version |
|-------------------------------------------------------------------------------------------------------------------|-------------|---------------|
| [Tractus-X Connector](https://github.com/eclipse-tractusx/tractusx-edc/tree/main/charts/tractusx-connector) | 0.7.2 | 0.7.2 |
| [Tractus-X Connector](https://github.com/eclipse-tractusx/tractusx-edc/tree/main/charts/tractusx-connector) | 0.7.1 | 0.7.1 |
| [Digital Twin Registry](https://github.com/eclipse-tractusx/sldt-digital-twin-registry/tree/main/charts/registry) | 0.4.3 | 0.4.11 |

## Known Knows
Expand Down
2 changes: 1 addition & 1 deletion backend/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
</parent>
<groupId>org.eclipse.tractusx.puris</groupId>
<artifactId>puris-backend</artifactId>
<version>2.0.1</version>
<version>2.0.2</version>
<name>puris-backend</name>
<description>PURIS Backend</description>
<properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public ResponseEntity<DeliveryInformation> getDeliveryMapping(
log.warn("Rejecting request at Delivery Information Submodel request 2.0.0 endpoint");
return ResponseEntity.badRequest().build();
}

if (!"$value".equals(representation)) {
log.warn("Rejecting request at Delivery Information Submodel request 2.0.0 endpoint, missing '$value' in request");
if (!PatternStore.NON_EMPTY_NON_VERTICAL_WHITESPACE_PATTERN.matcher(representation).matches()) {
Expand All @@ -75,8 +75,11 @@ public ResponseEntity<DeliveryInformation> getDeliveryMapping(
log.warn("Received " + representation + " from " + bpnl);
return ResponseEntity.status(501).build();
}

log.info("Received request for " + materialNumberCx + " from " + bpnl);
var samm = deliveryRequestApiSrvice.handleDeliverySubmodelRequest(bpnl, materialNumberCx);
if (samm == null) {
log.error("SAMM for delivery is null, return 500.");
return ResponseEntity.status(500).build();
}
return ResponseEntity.ok(samm);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import jakarta.validation.constraints.Pattern;
import lombok.*;
import lombok.experimental.SuperBuilder;

import org.eclipse.tractusx.puris.backend.common.domain.model.measurement.ItemUnitEnumeration;
import org.eclipse.tractusx.puris.backend.common.util.PatternStore;
import org.eclipse.tractusx.puris.backend.masterdata.domain.model.Material;
Expand Down Expand Up @@ -108,23 +107,23 @@ public boolean equals(Object o) {

final Delivery that = (Delivery) o;
return this.getMaterial().getOwnMaterialNumber().equals(that.getMaterial().getOwnMaterialNumber()) &&
this.getPartner().getUuid().equals(that.getPartner().getUuid()) &&
this.getTrackingNumber().equals(that.getTrackingNumber()) &&
this.getIncoterm().equals(that.getIncoterm()) &&
this.getDestinationBpns().equals(that.getDestinationBpns()) &&
this.getDestinationBpna().equals(that.getDestinationBpna()) &&
this.getOriginBpns().equals(that.getOriginBpns()) &&
this.getOriginBpna().equals(that.getOriginBpna()) &&
this.getDateOfDeparture().equals(that.getDateOfDeparture()) &&
this.getDateOfArrival().equals(that.getDateOfArrival()) &&
this.getDepartureType().equals(that.getDepartureType()) &&
this.getArrivalType().equals(that.getArrivalType()) &&
this.getIncoterm().equals(that.getIncoterm()) &&
(
Objects.equals(this.getCustomerOrderNumber(), that.getCustomerOrderNumber()) &&
this.getPartner().getUuid().equals(that.getPartner().getUuid()) &&
Objects.equals(this.getTrackingNumber(), that.getTrackingNumber()) &&
this.getIncoterm().equals(that.getIncoterm()) &&
this.getDestinationBpns().equals(that.getDestinationBpns()) &&
this.getDestinationBpna().equals(that.getDestinationBpna()) &&
this.getOriginBpns().equals(that.getOriginBpns()) &&
this.getOriginBpna().equals(that.getOriginBpna()) &&
this.getDateOfDeparture().equals(that.getDateOfDeparture()) &&
this.getDateOfArrival().equals(that.getDateOfArrival()) &&
this.getDepartureType().equals(that.getDepartureType()) &&
this.getArrivalType().equals(that.getArrivalType()) &&
this.getIncoterm().equals(that.getIncoterm()) &&
(
Objects.equals(this.getCustomerOrderNumber(), that.getCustomerOrderNumber()) &&
Objects.equals(this.getCustomerOrderPositionNumber(), that.getCustomerOrderPositionNumber()) &&
Objects.equals(this.getSupplierOrderNumber(), that.getSupplierOrderNumber())
);
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
import lombok.extern.slf4j.Slf4j;
import org.eclipse.tractusx.puris.backend.common.edc.domain.model.SubmodelType;
import org.eclipse.tractusx.puris.backend.common.edc.logic.service.EdcAdapterService;
import org.eclipse.tractusx.puris.backend.delivery.domain.model.DeliveryResponsibilityEnumeration;
import org.eclipse.tractusx.puris.backend.delivery.domain.model.OwnDelivery;
import org.eclipse.tractusx.puris.backend.delivery.logic.adapter.DeliveryInformationSammMapper;
import org.eclipse.tractusx.puris.backend.delivery.logic.dto.deliverysamm.DeliveryInformation;
import org.eclipse.tractusx.puris.backend.masterdata.domain.model.Material;
import org.eclipse.tractusx.puris.backend.masterdata.domain.model.MaterialPartnerRelation;
import org.eclipse.tractusx.puris.backend.masterdata.domain.model.Partner;
import org.eclipse.tractusx.puris.backend.masterdata.logic.service.MaterialPartnerRelationService;
import org.eclipse.tractusx.puris.backend.masterdata.logic.service.MaterialService;
Expand All @@ -35,7 +38,9 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import java.util.List;
import java.util.Optional;
import java.util.function.Predicate;

@Service
@Slf4j
Expand Down Expand Up @@ -68,29 +73,77 @@ public DeliveryInformation handleDeliverySubmodelRequest(String bpnl, String mat
}

Material material = materialService.findByMaterialNumberCx(materialNumberCx);
MaterialPartnerRelation mpr = mprService.findByPartnerAndPartnerCXNumber(partner, materialNumberCx);

if (material == null) {
// Could not identify partner cx number. I.e. we do not have that partner's
// CX id in one of our MaterialPartnerRelation entities. Try to fix this by
// looking for MPR's, where that partner is a supplier and where we don't have
// a partnerCXId yet. Of course this can only work if there was previously an MPR
// created, but for some unforeseen reason, the initial PartTypeRetrieval didn't succeed.
log.warn("Could not find " + materialNumberCx + " from partner " + partner.getBpnl());
mprService.triggerPartTypeRetrievalTask(partner);
material = materialService.findByMaterialNumberCx(materialNumberCx);
// Sidenote: This still means that the ShellDescriptor has not been created and someone tries to access our
// api without using the href from DTR
if (mpr == null) {
log.warn("Could not find " + materialNumberCx + " from partner " + partner.getBpnl());
log.warn("ATTENTION: PARTNER WITH BPNL {} ACCESSED THE SERVICE FOR A MATERIAL WITHOUT SHELL DESCRIPTOR " +
"IN DTR. THIS MIGHT BE A SECURITY ISSUE!", partner.getBpnl());
mprService.triggerPartTypeRetrievalTask(partner);
// check if cx id is now given
mpr = mprService.findByPartnerAndPartnerCXNumber(partner, materialNumberCx);
if (mpr == null) {
log.error("No material partner relation for BPNL '{}' and material global asset id '{}'." +
"Abort answering delivery request.",
partner.getBpnl(),
materialNumberCx
);
return null;
}
}
material = mpr.getMaterial();
}

if (material == null) {
log.error("Unknown Material " + materialNumberCx);
return null;
}

var mpr = mprService.find(material,partner);
if (mpr == null || !mpr.isPartnerSuppliesMaterial()) {
// if the material number cx has been defined by us, we're returning information as an supplier
// that means our partner is acting as customer
// search mpr again by partner and material to have one mpr at hand independent of partner role
boolean partnerIsCustomer = material.getMaterialNumberCx().equals(materialNumberCx);
mpr = mprService.find(material, partner);
if (mpr == null ||
(partnerIsCustomer && !mpr.isPartnerBuysMaterial()) ||
(!partnerIsCustomer && !mpr.isPartnerSuppliesMaterial())
) {
// only send an answer if partner is registered as supplier
log.warn(
"Partner acts as role '{}' but tried to access data for material number cx '{}' for the " +
"opposite role '{}'. Returning no data at all.",
partnerIsCustomer ? "Customer" : "Supplier",
materialNumberCx,
partnerIsCustomer ? "Supplier" : "Customer"
);
return null;
}

var currentDeliveries = ownDeliveryService.findAllByFilters(Optional.of(material.getOwnMaterialNumber()), Optional.empty(), Optional.of(partner.getBpnl()));
List<OwnDelivery> currentDeliveries = ownDeliveryService.findAllByFilters(
Optional.of(material.getOwnMaterialNumber()),
Optional.empty(),
Optional.of(partner.getBpnl())
);

Predicate<OwnDelivery> parnterRoleDirectionPredicate = partnerRoleDirectionPredicate(partnerIsCustomer, mpr);
currentDeliveries = currentDeliveries.stream().filter(
parnterRoleDirectionPredicate
).toList();
log.debug(
"Found '{}' deliveries for material number cx '{}' for partner with bpnl '{}' asking in role '{}'.",
currentDeliveries.size(),
materialNumberCx,
bpnl,
partnerIsCustomer ? "Customer" : "Supplier"
);
return sammMapper.ownDeliveryToSamm(currentDeliveries, partner, material);
}

Expand Down Expand Up @@ -126,4 +179,38 @@ public void doReportedDeliveryRequest(Partner partner, Material material) {
log.error("Error in Reported Deliveries Request for " + material.getOwnMaterialNumber() + " and partner " + partner.getBpnl(), e);
}
}

/**
* filters OwnDelivery entities to be returned to a partner based on his role
* <p>
* Based on accuracy the following rules apply:
* <li>use role derived from cx id and incoterm responsibility</li>
* <li>use mpr role, if only one is present and no incoterms are given</li>
*
* @param partnerIsCustomer to use as role in combination with incoterms (derived from material number cx)
* @param mpr to check the supplies / orders relation in case of missing incoterms
* @return Predicate<OwnDelivery> returning true if incoterms and role match or no incoterm but only one role on mpr is given, else false
*/
public static Predicate<OwnDelivery> partnerRoleDirectionPredicate(boolean partnerIsCustomer, MaterialPartnerRelation mpr) {
return delivery -> {
// return all, if no incoterm is set but partner only acts in one role
if (delivery.getIncoterm() == null) {
// unlikely that we have a mpr without any role set but then we also don't return something
if (mpr.isPartnerBuysMaterial() && mpr.isPartnerSuppliesMaterial()) {
return false;
} else return mpr.isPartnerBuysMaterial() || mpr.isPartnerSuppliesMaterial();
}

// if we have incoterms set, filter more sophisticatedly based on responsbility of incoterms and role
// derived from material number cx
DeliveryResponsibilityEnumeration responsibility = delivery.getIncoterm().getResponsibility();
if (partnerIsCustomer) {
return responsibility == DeliveryResponsibilityEnumeration.SUPPLIER ||
responsibility == DeliveryResponsibilityEnumeration.PARTIAL;
} else {
return responsibility == DeliveryResponsibilityEnumeration.CUSTOMER ||
responsibility == DeliveryResponsibilityEnumeration.PARTIAL;
}
};
}
}
Loading

0 comments on commit c2d02c5

Please sign in to comment.