Add serializer to response wrapper

[matez0]

Note:
From python 3.8, the _unwrap method could be replaced with using functools.singledispatchmethod.

Implements #144

[matez0]

Updated the author's and committer's email address.

[matez0]

While the README states that "Requires Python > 3.6", the build was run for Python 3.6 as well.
Since the walrus operator only introduced in Python 3.8, I still should not have used it.
The commit avoiding to use walrus operator could be squashed (fixed up) with the commit adding the serializer.
Should I do it?

[matez0]

Rename decorator and argument name for more readability.

[matez0]

Rebased onto the top of main.

[matez0]

Rebased on the top of main.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[matez0]

Rebased on the top of main.

[david-ruiz-cko]

Thanks for this @matez0 — clean implementation, thorough tests (the circular-reference handling with JSON Pointer–style $ref is a nice touch). Two suggestions before merge, both about the attribute-walking strategy in _unwrap_object:

1. dir(data) invokes property getters and walks inherited attributes

https://github.com/checkout/checkout-sdk-python/blob/2573f14/checkout_sdk/checkout_response.py#L57-L65

return {
key: cls._unwrap(getattr(data, key), paths_by_id, path + [key])
for key in dir(data)
if not key.startswith('__')
and not cls._is_function(getattr(data, key))
}

A few concerns with dir():

Property getters fire on serialization. If a consumer subclasses ResponseWrapper (or wraps an object) with a @Property that hits a DB, raises, or has side effects, .dict() will trigger it — and getattr runs twice here (once in the filter, once in the value), so the side effect happens twice.
Inherited / class-level attributes are included. dir() returns everything visible on the class, not just the instance.

Performance. Two getattr calls per attribute.
Could we use vars(data) (i.e. data.dict) instead? It walks instance attributes only, never invokes descriptors, and only requires a single getattr/dict lookup per key.

Heads-up that this is a behavior change: test_serialize_object declares ObjAttr attributes as class variables, which vars(instance) won't see. If we want to keep that test passing as-is, we'd need to either (a) move them to init, or (b) merge type(data).dict in for non-ResponseWrapper instances.

2. Single-underscore attributes leak through the filter

The filter is not key.startswith('__'), so conventional Python privates (_internal_state, _cached_token, etc.) are serialized. The SDK doesn't currently use _-prefixed sensitive fields, but it's a sharp edge for future code or downstream subclasses.

Two options:

• (a) tighten the filter — single underscores are conventionally private
  if not key.startswith('_')

Or if there's a reason to include them:

• (b) keep the current behavior, document it
  def dict(self):
  """
  Serializes the instance to a dictionary recursively.
  ...
  Note: attributes prefixed with a single underscore are included in the output.
  If you store sensitive data on _-prefixed attributes, filter the result before logging or persisting.
  """

Aside

Worth a one-liner in the docstring that responses may contain payment-sensitive fields (PAN, CVV tokens, OAuth tokens) and that callers should treat .dict() output the same as the underlying response — i.e. don't log/persist without redaction. Not a code issue, just an ergonomics nudge so .dict() doesn't make accidental logging too easy.

Tests are great either way — really appreciate the circular-reference coverage.

[armando-rodriguez-cko]

Hi @matez0, following up here since it's been a while. Are you still interested in finishing this up? Happy to help if you'd like a hand applying David's feedback (switching to vars(data), tightening the underscore filter, and the docstring note on sensitive fields). Let us know either way.

[matez0]

Hi @armando-rodriguez-cko,
Sure, I can apply these suggestion soon.

[agent-wall-e]

🟡 Risk Classification: MINOR

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

Classification reasons

• no_low_class_matched
• prod_source_modified

Operational gates

• ✅ jira_ticket
• ✅ independent_review

Files analysed: 2

---

_{wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…}

[agent-wall-e]

🔬 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
no_low_class_matched	informational	§2.2 (fall-through)	None of the deterministic Low classes (§2.2.3, §2.2.4, §2.2.7, docs-only) applied; classifier fell through to LLM evaluation.
prod_source_modified	informational	§2.1 M7 (informational)	At least one file is non-doc, non-test, non-IaC — i.e. application source code was modified.

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]

🟠 Advisory review: Concerns worth a look

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

Adds a dict() serializer to ResponseWrapper that recursively unwraps nested objects, mappings, and iterables, with circular-reference detection. The core logic has a significant correctness bug in the circular-reference guard and a footgun in the executable-filter heuristic.

Concerns

• In _handle_circular_ref, when a previously seen id is encountered, the code checks whether the current path starts with any known ref path (i.e., is a descendant), treating that as a true cycle and returning a $ref. But if it is NOT a descendant, the current path is appended to paths_by_id[id(data)] and the method continues — this means the same object visited via two independent non-cyclic paths will accumulate entries but will not be detected as cyclic on the second visit, which is correct by accident; however a shared object that appears as an ancestor on one branch and an independent sibling on another could have its path list grow unboundedly and future visits will do O(n) prefix comparisons against all previously seen paths, which is a performance/correctness risk for large graphs.
• The _is_executable check (hasattr(data, '__get__')) is used to filter out callables, but __get__ is present on any descriptor, including property, classmethod, staticmethod, and even plain functions stored as class attributes — but it is also present on many non-callable objects that implement the descriptor protocol (e.g. custom __get__ on value objects). Conversely, a callable class instance that does not define __get__ (i.e. a plain __call__ with no descriptor) would NOT be filtered. This heuristic can silently drop legitimate data attributes or include unwanted callables.
• In _unwrap_object, dir(data) is used to enumerate attributes. dir() includes inherited attributes from all base classes (including object), so the serialized output of any ResponseWrapper instance will include the inherited methods/properties that pass the _is_executable_attr filter — e.g. __dict__, __doc__, __module__, __weakref__ are double-underscore and filtered, but single-underscore attributes like _wrap, _is_collection, _unwrap, dict, etc. will appear unless they are caught by _is_executable_attr. Since dict is a method, _is_executable_attr calls _is_executable(getattr(type(data), 'dict', None)) which returns True (bound method has __get__), so it is filtered — but this relies on the heuristic being correct for every inherited attribute, making correctness fragile.
• The docstring states 'attributes prefixed with a single underscore are included in the output', and indeed _http_metadata, _data, _wrap, _is_collection, _unwrap_object, _unwrap, _unwrap_mapping, _unwrap_iterable, _is_executable_attr, _is_executable, _handle_circular_ref all appear via dir(). The static/class methods should be filtered by _is_executable_attr, but _http_metadata and _data (the backing stores set in __init__) will be serialized as top-level keys alongside http_metadata and data, producing duplicate/unexpected keys in the output.
• The paths_by_id dict uses Python object id() values as keys. Since CPython can reuse ids of garbage-collected objects within a single call, a short-lived intermediate object whose id is reused by a later unrelated object could produce a false positive circular-reference detection. This is a known footgun when using id() for identity tracking across a traversal that may drop references.

⚠️ 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}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[matez0]

@david-ruiz-cko

1. I kept the dir() because it includes more data (even from the base classes). However, I only call getattr once when it does not trigger code execution, e.g. like a property.

2. AFAIR, in a project I worked before, an important single underscore attribute was used from the response wrapper object and the data was stored in an encrypted database collection, so I do not want to loose the possibility to extract this data as well. I added the Note and who wants to filter any sensitive data, can easily do this.