Skip to content

Sd 2783 remove uc11#456

Open
pedrocarvalhodcsa wants to merge 2 commits intodevfrom
SD-2783_remove-UC11
Open

Sd 2783 remove uc11#456
pedrocarvalhodcsa wants to merge 2 commits intodevfrom
SD-2783_remove-UC11

Conversation

@pedrocarvalhodcsa
Copy link
Collaborator

@pedrocarvalhodcsa pedrocarvalhodcsa commented Jan 26, 2026

PR Type

Enhancement


Description

  • Support multiple transport document statuses in UC11 scenarios

  • Add mustBeOneOf validation method for flexible status checking

  • Refactor transport document status handling to accept lists

  • Remove UC12, UC13, UC14 chaining from amendment scenario paths


Diagram Walkthrough

flowchart LR
  A["UC11 Carrier Void TD"] -->|"TD_ISSUED or TD_VOIDED"| B["Shipper Get TD"]
  C["mustBeOneOf validation"] -->|"Accept multiple values"| D["TD Status Check"]
  E["Remove UC12-14 chains"] -->|"Simplify scenarios"| F["Amendment Scenarios"]
Loading

File Walkthrough

Relevant files
Enhancement
9 files
BookingAndEblScenarioListBuilder.java
Support multiple TD statuses in UC11 scenarios                     
+5/-3     
JsonAttribute.java
Add mustBeOneOf validation for multiple values                     
+29/-0   
EblScenarioListBuilder.java
Remove UC12-14 chains and support multiple TD statuses     
+8/-23   
EblAction.java
Refactor TD notification checks to accept status lists     
+3/-13   
Shipper_GetTransportDocumentAction.java
Support multiple expected transport document statuses       
+14/-1   
UC11_Carrier_voidTDAndIssueAmendedTransportDocumentAction.java
Accept both TD_ISSUED and TD_VOIDED statuses in UC11         
+3/-1     
UC6_Carrier_PublishDraftTransportDocumentAction.java
Wrap TD_DRAFT status in list for consistency                         
+3/-1     
CarrierTdNotificationPayloadRequestConformanceCheck.java
Support multiple transport document statuses in checks     
+8/-3     
EblChecks.java
Refactor TD status validation to support multiple statuses
+34/-28 
Bug fix
1 files
UC7_Shipper_ApproveDraftTransportDocumentAction.java
Add missing notification exchange UUID parameter                 
+1/-0     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #SD-2783
🟢 In every UC11 scenario, validate that the transport document (TD) status is either VOIDED
or ISSUED.
In every scenario that includes EBL UC11, stop the scenario at UC11 (do not continue with
later UC steps).
In every UC11 scenario, remove the TDR validation requirement (do not assert "TDR must be
equal to xyz").
Remove duplicate scenarios up to (and including) EBL UC11 across the affected scenario
suites (BKG+TD, SI+TD, TD only).
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logging: The new/changed transport-document status handling and checks do not show any explicit
audit logging for the critical “GET TD” action, so it is unclear whether required audit
trail context is captured elsewhere.

Referred Code
public class Shipper_GetTransportDocumentAction extends EblAction {

  private final List<TransportDocumentStatus> expectedTdStatus;
  private final JsonSchemaValidator responseSchemaValidator;

  public Shipper_GetTransportDocumentAction(
      String carrierPartyName,
      String shipperPartyName,
      EblAction previousAction,
      TransportDocumentStatus expectedTdStatus,
      JsonSchemaValidator responseSchemaValidator) {
    super(shipperPartyName, carrierPartyName, previousAction, "GET TD", 200, true);
    this.expectedTdStatus = List.of(expectedTdStatus);
    this.responseSchemaValidator = responseSchemaValidator;
  }

  public Shipper_GetTransportDocumentAction(
          String carrierPartyName,
          String shipperPartyName,
          EblAction previousAction,
          List<TransportDocumentStatus> expectedTdStatus,


 ... (clipped 5 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Empty set handling: The new mustBeOneOf validator does not explicitly guard against an empty expectedValue
set, which could lead to always-failing validations without a targeted error explaining
the configuration problem.

Referred Code
public static JsonRebasableContentCheck mustBeOneOf(
    JsonPointer jsonPointer, Set<String> expectedValue) {
  return mustBeOneOf(jsonPointer, true, expectedValue);
}

public static JsonRebasableContentCheck mustBeOneOf(
    JsonPointer jsonPointer, boolean isRelevant, Set<String> expectedValue) {
  Objects.requireNonNull(
      expectedValue,
      "expectedValue cannot be null; Note: Use `() -> getDspSupplier().get().foo()` (or similar) when testing a value against a dynamic scenario property");
  return JsonRebasableCheckImpl.of(
      "%s: Must be one of '%s'".formatted(jsonCheckName(jsonPointer), expectedValue),
      isRelevant,
      (body, contextPath) -> {
        var node = body.at(jsonPointer);
        var actualValue = node.asText(null);
        if (!expectedValue.contains(actualValue)) {
          return ConformanceCheckResult.simple(
              Set.of(
                  "The value of '%s' was '%s' which is not equal to any of '%s'"
                      .formatted(


 ... (clipped 7 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate at least one status

Add a check in shipperGetTransportDocument to ensure the expectedTdStatus
varargs array is not empty, throwing an IllegalArgumentException if it is.

booking-and-ebl/src/main/java/org/dcsa/conformance/standards/bookingandebl/BookingAndEblScenarioListBuilder.java [349-350]

 private static BookingAndEblScenarioListBuilder shipperGetTransportDocument(
     TransportDocumentStatus... expectedTdStatus) {
+  if (expectedTdStatus == null || expectedTdStatus.length == 0) {
+    throw new IllegalArgumentException(
+        "At least one expected transport document status must be provided");
+  }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential ArrayIndexOutOfBoundsException and proposes adding a guard clause, which improves the method's robustness and provides a clearer error message.

Medium
General
Clarify expected values list

Improve the error message in mustBeOneOf to clearly list expected values by
using String.join instead of expectedValue.toString().

core/src/main/java/org/dcsa/conformance/core/check/JsonAttribute.java [418-426]

 if (!expectedValue.contains(actualValue)) {
   return ConformanceCheckResult.simple(
       Set.of(
-          "The value of '%s' was '%s' which is not equal to any of '%s'"
+          "The value of '%s' was '%s' which is not one of [%s]"
               .formatted(
                   renderJsonPointer(jsonPointer, contextPath),
                   renderValue(node),
-                  renderValue(expectedValue.toString()))));
+                  String.join(\", \", expectedValue))));
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion improves the clarity of an error message, which enhances debuggability, but it is a minor improvement with low impact.

Low
  • More

() -> EblChecks.getTdPayloadChecks(transportDocumentStatus, dspSupplier)))
() -> {
List<JsonContentCheck> checks = new ArrayList<>();
getTdrCheck().ifPresent(checks::add);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to add this tdr check here?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants