Skip to content

Feat/data exchange forward - #1198

Open
OlgaIvkovic wants to merge 4 commits into
eclipse-tractusx:mainfrom
achtzig20:feat/data-exchange-forward
Open

Feat/data exchange forward#1198
OlgaIvkovic wants to merge 4 commits into
eclipse-tractusx:mainfrom
achtzig20:feat/data-exchange-forward

Conversation

@OlgaIvkovic

Copy link
Copy Markdown
Contributor

Description

  • Adjust BE model for data exchange forwarding
  • Implement data exchange forwading service in the backend
  • Implement tests for data exchange approval and forwarding

Updates #1192

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

  • DEPENDENCIES are up-to-date. Dash license tool. Committers can open IP issues for restricted libs.
  • Copyright and license header are present on all affected files (TRG 7.02)
  • Documentation Notice are present on all affected files (TRG 7.07)
  • If helm chart has been changed, the chart version has been bumped to either next major, minor or patch level (compared to released chart).
  • Changelog updated (changelog.md) with PR reference and brief summary.
  • Frontend version bumped, if needed (frontend/package.json, frontend/package-lock.json)
  • Backend version bumped, if needed (backend/pom.xml)
  • Open API specification updated, if controllers have been changed (use python script scripts/generate_openapi_yaml.py with running customer backend)

@ReneSchroederLJ ReneSchroederLJ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution. The code itself is fine for the most part. Small improvements and one possible security issue.

return;
}

List<OwnDataExchangeRequest> siblings = ownDataExchangeRequestService.findByRelatedDataExchangeRequest(origin);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semantically these should be children of the original ReportedDataExchangeRequest right?

log.error("Failed to finalize approval {}", originApproval.getApprovalId());
return;
}
Partner customer = origin.getNotification().getPartner();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Partner customer = origin.getNotification().getPartner();
Partner partner = origin.getNotification().getPartner();

.desiredStartDateTime(target.start())
.desiredEndDateTime(target.end())
.requestedTypes(new ArrayList<>(origin.getRequestedTypes()))
.text(origin.getText())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For security reasons we can not use the same text for the forwarded requests. I would suggest a generic text for now like: "Forwarded data exchange request from a downstream partner"

Comment on lines +170 to +175
if (forward) {
for (OwnDataExchangeRequest fwd : dataExchangeForwardService.createForwardedRequests(reportedRequest, targets)) {
Partner target = fwd.getNotification().getPartner();
executorService.submit(() -> dataExchangeRequestApiService.sendDataExchangeRequest(fwd, target));
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (forward) {
for (OwnDataExchangeRequest fwd : dataExchangeForwardService.createForwardedRequests(reportedRequest, targets)) {
Partner target = fwd.getNotification().getPartner();
executorService.submit(() -> dataExchangeRequestApiService.sendDataExchangeRequest(fwd, target));
}
}
for (OwnDataExchangeRequest fwd : dataExchangeForwardService.createForwardedRequests(reportedRequest, targets)) {
Partner target = fwd.getNotification().getPartner();
executorService.submit(() -> dataExchangeRequestApiService.sendDataExchangeRequest(fwd, target));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants