Skip to content

Feature/INT-1702 - Airline and accommodation sub-tree model alignment - #676

Open
david-ruiz-cko wants to merge 3 commits into
masterfrom
feature/INT-1702
Open

david-ruiz-cko wants to merge 3 commits into
masterfrom
feature/INT-1702

Conversation

@david-ruiz-cko

Copy link
Copy Markdown
Contributor

This pull request introduces several important improvements and corrections to the payment industry data models, focusing on better alignment with the API specification, increased robustness in JSON (de)serialization, and enhanced documentation. The most significant changes include improved handling of polymorphic array/object fields, migration of several properties to more flexible types, and extensive JavaDoc updates for clarity and maintainability.

Improvements to JSON (de)serialization and data model flexibility:

  • Added a custom deserializer in GsonSerializer to handle fields that may be either a single object or an array (notably for airline passenger data), ensuring consistent internal representation as a list and always serializing as an array. This prevents data loss and aligns with the API's flexible input. [1] [2]
  • Updated Industry and related classes to map airline and accommodation properties as lists (List<AirlineData>, List<AccommodationData>) instead of single objects, matching the API specification and fixing previous serialization issues.

Data type corrections and deprecations:

  • Changed several fields from enum types (e.g., CountryCode) to plain String to accommodate the API's use of both two- and three-letter country codes, increasing compatibility. [1] [2]
  • Deprecated and documented old or duplicate classes and fields (e.g., PaymentContextsAccommodationData, serviceClass in FlightLegDetails, hubModelOriginationCountry in ProcessingSettings) to guide developers toward the preferred usage and maintain backward compatibility. [1] [2] [3]

Documentation and JavaDoc enhancements:

  • Added or improved JavaDoc comments across all affected classes and fields, providing clear descriptions, usage notes, and references to the API specification. This improves maintainability and developer understanding. [1] [2] [3] [4] [5] [6] [7] [8] [9]

New features and classes:

  • Introduced PartnerCustomerRiskData to represent merchant-specific key-value pairs for transaction risk data, supporting new API features and aligning with the latest specification. [1] [2]

Property and field corrections:

  • Updated property names and types for consistency with the specification (e.g., classOfTravelling, departureDate as LocalDate, numberOfNightsAtRoomRate as String), and clarified optionality and expected formats. [1] [2] [3]

These changes collectively improve the SDK's correctness, flexibility, and developer usability when handling payment industry-specific data.

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 25, 2026 09:27
@agent-wall-e

agent-wall-e Bot commented Sep 25, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:433>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 27


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 25, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 433>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

🟠 Advisory review: Concerns worth a look

This PR needs a human approval. Before you give it, these are the things I'd want resolved.

The PR fixes real bugs (wrong cardinality, wrong field names, wrong types) and is well-tested, but the custom write-side adapter in singleOrArrayPassengerFactory creates a round-trip asymmetry: a single passenger serializes as an object, not an array, meaning a value written by this SDK and then read back will deserialize differently depending on the reading end (array vs object), and the truncated diff leaves the core adapter write logic unverifiable.

Concerns

  • The singleOrArrayPassengerFactory deliberately serializes a single Passenger as a JSON object, but the singleOrArrayDeserializer normalizes a bare object into a List — this asymmetry means a round-trip through the SDK itself produces a different wire shape than what was deserialized, which could confuse any caller that stores or forwards the serialized output.
  • The write-side of singleOrArrayPassengerFactory is truncated in the diff (the patch is cut off at 'getDelegateAdapter'), so the actual serialization logic — including how it finds the passenger field by name in the delegate adapter — cannot be verified for correctness or for whether it handles null/empty lists correctly.
  • The comment in singleOrArrayPassengerFactory claims an empty list drops the member entirely (to avoid processing_airline_data_0_passenger_invalid), but this logic is in the truncated section and cannot be confirmed; if missing, an empty list serialized as {} or [] would cause API errors.
  • FlightLegDetails now has both serviceClass (deprecated, serializes as service_class) and classOfTravelling (active, serializes as class_of_travelling) — both fields are present on the class simultaneously, so a caller who mistakenly populates both will silently send a key the API discards alongside the correct one.
  • Industry.java field rename from airlineData/accommodationData to airline/accommodation is a breaking API change for any caller using the builder or getter directly; the diff shows no @deprecated bridge getters/setters, unlike the approach taken for serviceClass and hubModelOriginationCountry.
  • PaymentContextsProcessing.accommodationData changes from List to List — this is a binary-incompatible type change on an existing public field; callers who compiled against the old type will break at runtime without a deprecation bridge.
  • The sandbox verification date in the Javadoc comment reads '2026-09-25', which is a future date and appears to be a typo (likely 2024-09-25 or 2025-09-25); this undermines confidence in the documented API behaviour table.
  • The double Javadoc block on singleOrArrayDeserializer (two consecutive /** ... */ blocks, one for the deserializer and one for the factory) means the first block is unreachable/orphaned — the second block overwrites it and documents the wrong method.

⚠️ The diff was too large to read in full, so this review covers only part of the change.


This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02

@agent-wall-e

agent-wall-e Bot commented Sep 25, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:514>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 27


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 25, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 514>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@sonarqubecloud

Copy link
Copy Markdown

* global {@code LOWER_CASE_WITH_UNDERSCORES} naming policy and the {@code LocalDate} adapter
* still apply; this deserializer never maps property names itself.
*
* @param elementType the list element type
* still apply; this deserializer never maps property names itself.
*
* @param elementType the list element type
* @param <T> the list element type

This branch has not been deployed

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants