Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(DeliveryRequestApiService): filter deliveries requested by partner role and incoterms #435

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Dismissed Show dismissed Hide dismissed
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());
Dismissed Show dismissed Hide dismissed
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
Dismissed Show dismissed Hide dismissed
);
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"
);
Dismissed Show dismissed Hide dismissed
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) {
tom-rm-meyer-ISST marked this conversation as resolved.
Show resolved Hide resolved
return responsibility == DeliveryResponsibilityEnumeration.SUPPLIER ||
responsibility == DeliveryResponsibilityEnumeration.PARTIAL;
} else {
return responsibility == DeliveryResponsibilityEnumeration.CUSTOMER ||
responsibility == DeliveryResponsibilityEnumeration.PARTIAL;
}
};
}
}
Loading
Loading