Skip to content

Support Bare deserialization and update tests#1688

Open
gcatanese wants to merge 44 commits into
mainfrom
feature/1613-support-base-jst
Open

Support Bare deserialization and update tests#1688
gcatanese wants to merge 44 commits into
mainfrom
feature/1613-support-base-jst

Conversation

@gcatanese
Copy link
Copy Markdown
Contributor

@gcatanese gcatanese commented May 15, 2026

Description

Fixes #1613 #1687

This PR implements the proposed solution from issue #1613: Support bare JsonSerializer.Deserialize<T>(json) without requiring SDK-internal JsonSerializerOptions.

Problem

Since v34.0.0, custom JsonConverter<T> instances for each model were registered only in HostConfiguration via DI. This meant that calling JsonSerializer.Deserialize<T>(json) without the SDK's options produced incorrect results — missing optional-field tracking, wrong date formats, and broken byte array handling. Affected scenarios include unit tests, webhook processing, caching layers, and message queues.

Solution

Made all generated models self-describing to System.Text.Json by adding [JsonConverter(typeof(XJsonConverter))] attributes at the class level, so STJ discovers the correct converter automatically regardless of which JsonSerializerOptions (if any) the caller supplies. Property-level [JsonConverter] attributes are also added for byte[] and DateOnly fields.

Changes

  • templates-v7/csharp/libraries/generichost/modelGeneric.mustache — Added class-level [JsonConverter] attribute and property-level attributes for byte[] and DateOnly types to the model template.
  • templates-v7/csharp/libraries/generichost/model.mustache — Added missing using for Adyen.Core.Converters namespace.
  • models: generate models for Checkout, LEM, Capital
  • Adyen.Test/Core/Serialization/BareSerializationTests.cs — New test class covering bare serialize/deserialize round-trips for Amount, ThreeDSecureData, CheckoutPaymentMethod, and nested models without any JsonSerializerOptions.
  • tests in various services: add tests for bare deserialization in Checkout, LEM, Capital
  • Adyen.Test/Payment/PaymentTest.cs — Fixed existing test assertion: use TryGetProperty instead of GetProperty for optional non-nullable fields that may be absent in the serialized output.
  • README.md — Documented the new bare deserialization path and clarified when DI-provided options are still recommended (e.g. for byte[] fields with ByteArrayConverter).

Backwards compatibility

No breaking changes. STJ gives precedence to type-level [JsonConverter] attributes over converters registered in JsonSerializerOptions, so the existing DI-based pipeline continues to work exactly as before.

Known limitation

byte[] fields (e.g. ThreeDSecureData.Cavv) behave differently in bare mode vs. DI mode due to how ByteArrayConverter is currently implemented. This is documented in the README and tracked separately in #1687.

@gcatanese gcatanese requested a review from a team as a code owner May 15, 2026 13:02
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enables "bare" serialization and deserialization for Adyen SDK models by adding class-level [JsonConverter] attributes, allowing System.Text.Json to automatically discover the appropriate converters without requiring explicit options. The changes include updates to the model templates, new unit tests for bare serialization, and documentation updates. Review feedback indicates that property-level [JsonConverter] attributes for byte arrays and DateOnly types are currently ignored by the manual class-level converter implementations, which may lead to inconsistent data handling between bare and DI-configured serialization modes.

Comment thread Adyen/Checkout/Models/ThreeDSecureData.cs Outdated
Comment thread templates-v7/csharp/libraries/generichost/modelGeneric.mustache Outdated
Comment thread Adyen.Test/Core/Serialization/BareSerializationTests.cs Outdated
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.

[Feature Request]: Support bare JsonSerializer.Deserialize<T>(json) without requiring SDK-internal JsonSerializerOptions

2 participants