From f588350abd3d61c4e757602bf72b871af9ae963b Mon Sep 17 00:00:00 2001 From: Amit Ray <51674969+amitray007@users.noreply.github.com> Date: Sun, 6 Sep 2026 05:08:40 +0530 Subject: [PATCH 1/2] fix: stop sending personalization fields removed from listing bodies Etsy removed is_personalizable, personalization_is_required, personalization_char_count_max and personalization_instructions from the createDraftListing and updateListing request bodies (2026-09 spec). Personalization now lives on the dedicated personalization endpoints, which the SDK already implements. The SDK already warned when these fields were set, but still serialized and transmitted them. They are now stored under underscore-prefixed names that todict excludes, so they never reach the wire. The constructor kwargs and the DeprecationWarning are kept so existing callers don't break, and read access is preserved via properties. Remove the kwargs in the next major version. Also route request body drift through the audit suppression pipeline as a value-bearing `body_drift` finding type, matching enum_staleness and param_drift. Body drift previously bypassed specs/audit-ignore.json entirely, so an accepted finding could only be left as permanent noise. Suppression is scoped to named fields, so newly drifted fields on an already-suppressed operation still surface. Audit: 105/105 operations, 100% coverage, 0 active findings, 0 stale ignores. --- CLAUDE.md | 9 +- etsy_python/v3/models/Listing.py | 76 ++++++++++--- scripts/audit_sdk.py | 173 ++++++++++++++++------------- specs/audit-ignore.json | 82 +++++++++++--- specs/baseline.json | 34 ------ tests/test_audit_ignore.py | 183 +++++++++++++++++++++++++++++++ tests/test_listing_models.py | 80 +++++++++++++- 7 files changed, 490 insertions(+), 147 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8b2bc1c..3fe751a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -200,8 +200,9 @@ Reviewed, accepted audit findings (deliberate deprecated aliases, intentionally partial enums, back-compat kwargs, etc.) live in `specs/audit-ignore.json` — never hard-coded in `audit_sdk.py`. Each run re-derives findings and **only suppresses an entry while its finding still occurs**; for the value-bearing types -(`enum_staleness`, `param_drift`), only the listed `values` are hidden, so a newly -added enum value or newly drifted parameter still surfaces. Entries matching nothing +(`enum_staleness`, `param_drift`, `body_drift`), only the listed `values` are hidden, +so a newly added enum value or newly drifted parameter or body field still surfaces. +Entries matching nothing are reported under a **Stale Ignores** section so the list stays honest, and suppressed findings are listed (with reasons) under **Suppressed (Verified)**. To accept a finding, add an entry (`type` + `key`, plus `direction`/`values` for the @@ -209,5 +210,5 @@ value-bearing types); to stop accepting it, delete the entry. A missing file mea "no suppressions". Supported `type` values: `extra_method`, `enum_staleness`, `param_drift`, -`code_issue`. Prefer an explicit `values` list over `"*"` — a wildcard hides -everything on that key, including drift nobody has reviewed. +`body_drift`, `code_issue`. Prefer an explicit `values` list over `"*"` — a wildcard +hides everything on that key, including drift nobody has reviewed. diff --git a/etsy_python/v3/models/Listing.py b/etsy_python/v3/models/Listing.py index cf08f49..812b422 100644 --- a/etsy_python/v3/models/Listing.py +++ b/etsy_python/v3/models/Listing.py @@ -17,7 +17,8 @@ _PERSONALIZATION_DEPRECATION_MSG = ( "is_personalizable, personalization_is_required, " "personalization_char_count_max, and personalization_instructions " - "are deprecated by the Etsy API and scheduled for removal. " + "were removed from createDraftListing and updateListing by the Etsy API " + "and are no longer sent. " "Use the personalization endpoint (update_listing_personalization) " "instead. See " "https://developers.etsy.com/documentation/tutorials/personalization-migration " @@ -31,11 +32,11 @@ def _warn_if_personalization_used( personalization_char_count_max: Optional[int], personalization_instructions: Optional[str], ) -> None: - """Emit DeprecationWarning when a deprecated personalization field is actively set. + """Emit DeprecationWarning when a removed personalization field is actively set. Falsy values (False, 0, empty string, None) match the API's documented - defaults and remain no-ops after the fields are removed, so they do not - trigger the warning. + defaults and were no-ops even before the fields were removed, so they do + not trigger the warning. """ if any([ is_personalizable, @@ -46,11 +47,60 @@ def _warn_if_personalization_used( warnings.warn( _PERSONALIZATION_DEPRECATION_MSG, DeprecationWarning, - stacklevel=3, + # _warn_if_personalization_used <- _store_personalization <- + # model __init__ <- caller + stacklevel=4, ) -class CreateDraftListingRequest(Request): +class _PersonalizationFieldsMixin: + """Read-only access to the removed personalization fields. + + Etsy dropped these four fields from the createDraftListing and + updateListing request bodies (2026-09 spec). The constructor keyword + arguments are kept so existing callers don't break, but the values are + stored under underscore-prefixed names, which ``todict`` excludes from + serialization -- so they are never sent on the wire. The properties below + keep attribute reads working for callers that inspect the model. Remove + the keyword arguments and this mixin in the next major version. + """ + + @property + def is_personalizable(self) -> Optional[bool]: + return self._is_personalizable + + @property + def personalization_is_required(self) -> Optional[bool]: + return self._personalization_is_required + + @property + def personalization_char_count_max(self) -> Optional[int]: + return self._personalization_char_count_max + + @property + def personalization_instructions(self) -> Optional[str]: + return self._personalization_instructions + + def _store_personalization( + self, + is_personalizable: Optional[bool], + personalization_is_required: Optional[bool], + personalization_char_count_max: Optional[int], + personalization_instructions: Optional[str], + ) -> None: + self._is_personalizable = is_personalizable + self._personalization_is_required = personalization_is_required + self._personalization_char_count_max = personalization_char_count_max + self._personalization_instructions = personalization_instructions + _warn_if_personalization_used( + is_personalizable, + personalization_is_required, + personalization_char_count_max, + personalization_instructions, + ) + + +class CreateDraftListingRequest(_PersonalizationFieldsMixin, Request): nullable = [ "shipping_profile_id", "return_policy_id", @@ -137,11 +187,7 @@ def __init__( self.item_height = item_height self.item_weight_unit = item_weight_unit self.item_dimensions_unit = item_dimensions_unit - self.is_personalizable = is_personalizable - self.personalization_is_required = personalization_is_required - self.personalization_char_count_max = personalization_char_count_max - self.personalization_instructions = personalization_instructions - _warn_if_personalization_used( + self._store_personalization( is_personalizable, personalization_is_required, personalization_char_count_max, @@ -161,7 +207,7 @@ def __init__( ) -class UpdateListingRequest(Request): +class UpdateListingRequest(_PersonalizationFieldsMixin, Request): nullable: List[str] = [ "materials", "shipping_profile_id", @@ -232,11 +278,7 @@ def __init__( self.who_made = who_made self.when_made = when_made self.featured_rank = featured_rank - self.is_personalizable = is_personalizable - self.personalization_is_required = personalization_is_required - self.personalization_char_count_max = personalization_char_count_max - self.personalization_instructions = personalization_instructions - _warn_if_personalization_used( + self._store_personalization( is_personalizable, personalization_is_required, personalization_char_count_max, diff --git a/scripts/audit_sdk.py b/scripts/audit_sdk.py index 49b79d3..89ff09c 100644 --- a/scripts/audit_sdk.py +++ b/scripts/audit_sdk.py @@ -520,7 +520,7 @@ def _norm_values(values: Any) -> Set[str]: # Finding types carrying a `values` set + `direction`, where an ignore may # suppress specific values and leave newly appeared ones active. -_VALUED_FINDING_TYPES = frozenset({"enum_staleness", "param_drift"}) +_VALUED_FINDING_TYPES = frozenset({"enum_staleness", "param_drift", "body_drift"}) def compute_enum_findings( @@ -669,6 +669,80 @@ def compute_param_findings( return findings +def compute_body_findings( + spec: dict, implemented: Dict[str, dict], sdk_models: Dict[str, dict] +) -> List[dict]: + """Compare OAS request body fields against SDK model/method fields. + + Yields one finding per (operation, direction) whose field sets differ. + ``direction`` is ``"missing"`` (in the spec body, absent from the SDK) or + ``"extra"`` (in the SDK, absent from the spec body). ``values`` is a set of + field names, so an ignore listing specific names suppresses only those — a + newly drifted body field on an already-suppressed operation still surfaces. + + Multipart-only endpoints are skipped: their ``FileRequest`` subclasses hold + fields in ``data``/``file`` dicts that this signature-level comparison + cannot resolve. + """ + findings: List[dict] = [] + for op_id, mapping in sorted(implemented.items()): + op = mapping["spec"] + sdk = mapping["sdk"] + + spec_body_fields = get_request_body_fields(op, spec) + if not spec_body_fields: + continue + + rb = op.get("requestBody", {}) + content = rb.get("content", {}) if rb else {} + if "multipart/form-data" in content and "application/json" not in content: + continue + + param_annotations = sdk.get("param_annotations", {}) + model_class_name = None + for pname, ptype in param_annotations.items(): + if ptype in sdk_models: + model_class_name = ptype + break + + if model_class_name: + model_info = sdk_models[model_class_name] + # Normalize kwargs the SDK serializes under a different spec name + # (listing_type/profile_type -> type), so they aren't false drift. + sdk_fields = { + TYPE_FIELD_ALIASES.get(f, f) for f in model_info["init_params"] + } + location = ( + f"model `{model_class_name}` in " + f"models/{model_info['file']}:{model_info['line']}" + ) + else: + # Method has body fields but no model object - compare against params + sdk_fields = set(sdk["params"]) - { + p["name"] for p in op["parameters"] + } - PATH_PARAM_NAMES + location = ( + f"`{mapping['sdk_method']}` in " + f"{sdk['file']}:{sdk['line']}, no model class" + ) + + spec_only = spec_body_fields - sdk_fields + sdk_only = sdk_fields - spec_body_fields + + for direction, values in (("missing", spec_only), ("extra", sdk_only)): + if values: + findings.append( + { + "type": "body_drift", + "key": op_id, + "direction": direction, + "values": values, + "location": location, + } + ) + return findings + + def partition_findings( findings: List[dict], ignores: List[dict] ) -> Tuple[List[dict], List[dict], List[dict]]: @@ -827,6 +901,7 @@ def generate_report( ) findings.extend(compute_enum_findings(spec, sdk_enums)) findings.extend(compute_param_findings(implemented, sdk_models)) + findings.extend(compute_body_findings(spec, implemented, sdk_models)) for issue in concat_issues: findings.append( { @@ -841,6 +916,7 @@ def generate_report( active_enum = [f for f in active if f["type"] == "enum_staleness"] active_code = [f for f in active if f["type"] == "code_issue"] active_param = [f for f in active if f["type"] == "param_drift"] + active_body = [f for f in active if f["type"] == "body_drift"] lines.append("## Coverage Summary\n") lines.append(f"- Total OAS operations: {total_ops}") @@ -943,82 +1019,24 @@ def generate_report( # --- Request Body Drift --- lines.append("\n## Request Body Drift\n") lines.append("Mismatches between OAS request body fields and SDK model class fields.\n") - any_body_drift = False - for op_id, mapping in sorted(implemented.items()): - op = mapping["spec"] - sdk = mapping["sdk"] - - spec_body_fields = get_request_body_fields(op, spec) - if not spec_body_fields: - continue - - # Skip multipart/form-data endpoints (FileRequest subclasses have different patterns) - rb = op.get("requestBody", {}) - content = rb.get("content", {}) if rb else {} - if "multipart/form-data" in content and "application/json" not in content: - continue - - param_annotations = sdk.get("param_annotations", {}) - - # Find the model class used by this method - model_class_name = None - for pname, ptype in param_annotations.items(): - if ptype in sdk_models: - model_class_name = ptype - break - - if model_class_name: - model_info = sdk_models[model_class_name] - model_fields = model_info["init_params"] - # Normalize known aliases (_type -> type mapping in todict) - normalized_model_fields = set() - for f in model_fields: - if f in TYPE_FIELD_ALIASES: - normalized_model_fields.add(TYPE_FIELD_ALIASES[f]) - else: - normalized_model_fields.add(f) - - body_spec_only = spec_body_fields - normalized_model_fields - body_sdk_only = normalized_model_fields - spec_body_fields - - if body_spec_only or body_sdk_only: - any_body_drift = True - lines.append( - f"### {op_id} (model `{model_class_name}` in models/{model_info['file']}:{model_info['line']})\n" - ) - if body_spec_only: - lines.append( - f"- In spec but not model: {', '.join(sorted(body_spec_only))}" - ) - if body_sdk_only: - lines.append( - f"- In model but not spec: {', '.join(sorted(body_sdk_only))}" - ) - lines.append("") - else: - # Method has body fields but no model object - compare against method params - sdk_params = set(sdk["params"]) - body_spec_only = spec_body_fields - sdk_params - body_sdk_only = sdk_params - spec_body_fields - { - p["name"] for p in op["parameters"] - } - PATH_PARAM_NAMES - - if body_spec_only or body_sdk_only: - any_body_drift = True - lines.append( - f"### {op_id} (`{mapping['sdk_method']}` in {sdk['file']}:{sdk['line']}, no model class)\n" - ) - if body_spec_only: - lines.append( - f"- In spec but not SDK: {', '.join(sorted(body_spec_only))}" - ) - if body_sdk_only: - lines.append( - f"- In SDK but not spec: {', '.join(sorted(body_sdk_only))}" - ) - lines.append("") - - if not any_body_drift: + # Grouped by operation so both directions render under one heading, using the + # post-suppression findings from partition_findings. + body_by_op: Dict[str, Dict[str, dict]] = {} + for f in active_body: + body_by_op.setdefault(f["key"], {})[f["direction"]] = f + if body_by_op: + for op_id in sorted(body_by_op): + directions = body_by_op[op_id] + any_f = next(iter(directions.values())) + lines.append(f"### {op_id} ({any_f['location']})\n") + if "missing" in directions: + names = ", ".join(sorted(directions["missing"]["values"])) + lines.append(f"- In spec but not model: {names}") + if "extra" in directions: + names = ", ".join(sorted(directions["extra"]["values"])) + lines.append(f"- In model but not spec: {names}") + lines.append("") + else: lines.append("No request body drift detected.\n") # --- Enum Staleness --- @@ -1089,6 +1107,7 @@ def generate_report( "enum_staleness": "Enum Staleness", "code_issue": "Code Issues", "param_drift": "Query/Path Parameter Drift", + "body_drift": "Request Body Drift", } suppressed_by_type: Dict[str, List[dict]] = {} for f in suppressed: diff --git a/specs/audit-ignore.json b/specs/audit-ignore.json index a7979dd..fa1df39 100644 --- a/specs/audit-ignore.json +++ b/specs/audit-ignore.json @@ -1,11 +1,13 @@ { - "_README": "Reviewed, accepted audit findings that should not count as noise. scripts/audit_sdk.py loads this file and, on EVERY run, re-derives findings and only suppresses an entry while its finding still occurs (for enum_staleness and param_drift, only the listed values are suppressed; newly appeared values stay active). Entries that match nothing are reported under 'Stale Ignores' so this list stays honest. Nothing here is hard-coded in the script — to accept a finding, add an entry; to stop accepting it, delete the entry. Match fields: 'type' + 'key' (+ 'direction' for enum_staleness and param_drift). 'values' (enum_staleness/param_drift only): \"*\" = all, or a list of specific values. 'reason'/'added' are documentation.", + "_README": "Reviewed, accepted audit findings that should not count as noise. scripts/audit_sdk.py loads this file and, on EVERY run, re-derives findings and only suppresses an entry while its finding still occurs (for enum_staleness, param_drift and body_drift, only the listed values are suppressed; newly appeared values stay active). Entries that match nothing are reported under 'Stale Ignores' so this list stays honest. Nothing here is hard-coded in the script \u2014 to accept a finding, add an entry; to stop accepting it, delete the entry. Match fields: 'type' + 'key' (+ 'direction' for enum_staleness, param_drift and body_drift). 'values' (enum_staleness/param_drift/body_drift only): \"*\" = all, or a list of specific values. 'reason'/'added' are documentation.", "ignores": [ { "type": "param_drift", "key": "createDraftListing", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -13,7 +15,9 @@ "type": "param_drift", "key": "getListingsByShop", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -21,7 +25,9 @@ "type": "param_drift", "key": "getListing", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -29,7 +35,9 @@ "type": "param_drift", "key": "findAllListingsActive", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -37,7 +45,9 @@ "type": "param_drift", "key": "findAllActiveListingsByShop", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -45,7 +55,9 @@ "type": "param_drift", "key": "updateListing", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -53,7 +65,9 @@ "type": "param_drift", "key": "getListingInventory", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -61,7 +75,9 @@ "type": "param_drift", "key": "updateListingInventory", "direction": "extra", - "values": ["legacy"], + "values": [ + "legacy" + ], "reason": "Etsy removed the 'legacy' query param from the 8 listing endpoints (2026-07-27 spec) with no deprecation period; it remains valid on 14 other operations. The kwarg is retained so existing callers don't break, but it warns via warn_removed_legacy_param and is no longer sent. Remove the kwarg (and this entry) in the next major version.", "added": "2026-07-30" }, @@ -101,7 +117,9 @@ "type": "enum_staleness", "key": "ShopListing.state -> State", "direction": "extra", - "values": ["removed"], + "values": [ + "removed" + ], "reason": "State.REMOVED is kept for backward compatibility and is not in the OAS response schema. Documented inline in enums/Listing.py; may be removed in the next major version.", "added": "2026-06-02" }, @@ -109,7 +127,9 @@ "type": "enum_staleness", "key": "ShopListingWithAssociations.state -> State", "direction": "extra", - "values": ["removed"], + "values": [ + "removed" + ], "reason": "State.REMOVED is kept for backward compatibility and is not in the OAS response schema. Documented inline in enums/Listing.py; may be removed in the next major version.", "added": "2026-06-02" }, @@ -117,7 +137,10 @@ "type": "enum_staleness", "key": "getListing.includes -> Includes", "direction": "extra", - "values": ["shipping", "inventory"], + "values": [ + "shipping", + "inventory" + ], "reason": "Etsy removed 'Shipping' and 'Inventory' from the includes enum on getListing and getListingsByListingIds (still valid on getListingsByShop). The SDK Includes enum keeps both for backward compatibility and because they remain valid on getListingsByShop, which shares the enum; removing them would be a breaking change. Documented inline in enums/Listing.py.", "added": "2026-07-08" }, @@ -125,7 +148,10 @@ "type": "enum_staleness", "key": "getListingsByListingIds.includes -> Includes", "direction": "extra", - "values": ["shipping", "inventory"], + "values": [ + "shipping", + "inventory" + ], "reason": "Etsy removed 'Shipping' and 'Inventory' from the includes enum on getListing and getListingsByListingIds (still valid on getListingsByShop). The SDK Includes enum keeps both for backward compatibility and because they remain valid on getListingsByShop, which shares the enum; removing them would be a breaking change. Documented inline in enums/Listing.py.", "added": "2026-07-08" }, @@ -133,7 +159,9 @@ "type": "enum_staleness", "key": "getListingsByShop.state -> State", "direction": "extra", - "values": ["removed"], + "values": [ + "removed" + ], "reason": "Same State.REMOVED back-compat value as ShopListing.state; surfaced additionally as the state query parameter on getListingsByShop now that parameter-level enums are audited. Documented inline in enums/Listing.py.", "added": "2026-07-08" }, @@ -144,6 +172,32 @@ "values": "*", "reason": "Same by-design holiday_id handling as ShopHolidayPreference.holiday_id; surfaced additionally as the holiday_id parameter on updateHolidayPreferences now that parameter-level enums are audited. The SDK names US_HOLIDAYS (1-11) and CA_HOLIDAYS (12-23) and documents passing integer IDs directly for other regions. See enums/HolidayPreferences.py.", "added": "2026-07-08" + }, + { + "type": "body_drift", + "key": "createDraftListing", + "direction": "extra", + "values": [ + "is_personalizable", + "personalization_is_required", + "personalization_char_count_max", + "personalization_instructions" + ], + "reason": "Etsy removed is_personalizable, personalization_is_required, personalization_char_count_max and personalization_instructions from the createDraftListing and updateListing request bodies (2026-09 spec); personalization now lives on the dedicated personalization endpoints (update_listing_personalization). The constructor kwargs are retained so existing callers don't break, but the values are stored under underscore-prefixed names that todict excludes, so they are never sent on the wire, and setting one emits a DeprecationWarning. Remove the kwargs (and this entry) in the next major version.", + "added": "2026-09-06" + }, + { + "type": "body_drift", + "key": "updateListing", + "direction": "extra", + "values": [ + "is_personalizable", + "personalization_is_required", + "personalization_char_count_max", + "personalization_instructions" + ], + "reason": "Etsy removed is_personalizable, personalization_is_required, personalization_char_count_max and personalization_instructions from the createDraftListing and updateListing request bodies (2026-09 spec); personalization now lives on the dedicated personalization endpoints (update_listing_personalization). The constructor kwargs are retained so existing callers don't break, but the values are stored under underscore-prefixed names that todict excludes, so they are never sent on the wire, and setting one emits a DeprecationWarning. Remove the kwargs (and this entry) in the next major version.", + "added": "2026-09-06" } ] } diff --git a/specs/baseline.json b/specs/baseline.json index 0cd1d2a..9747287 100644 --- a/specs/baseline.json +++ b/specs/baseline.json @@ -490,23 +490,6 @@ "inches" ] }, - "is_personalizable": { - "type": "boolean", - "description": "[DEPRECATED] When true, this listing is personalizable. The default value is false. NOTE: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details." - }, - "personalization_is_required": { - "type": "boolean", - "description": "[DEPRECATED] When true, this listing requires personalization. The default value is false. NOTE: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details." - }, - "personalization_char_count_max": { - "type": "integer", - "description": "[DEPRECATED] This is an integer value representing the maximum length for the personalization message entered by the buyer. Will only change if is_personalizable is 'true'. Note: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details.", - "format": "int64" - }, - "personalization_instructions": { - "type": "string", - "description": "[DEPRECATED] A string representing instructions for the buyer to enter the personalization. Will only change if is_personalizable is 'true'. Note: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details." - }, "production_partner_ids": { "type": "array", "description": "An array of unique IDs of production partner ids.", @@ -4755,23 +4738,6 @@ "format": "int64", "nullable": true }, - "is_personalizable": { - "type": "boolean", - "description": "[DEPRECATED] When true, this listing is personalizable. The default value is false. NOTE: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details." - }, - "personalization_is_required": { - "type": "boolean", - "description": "[DEPRECATED] When true, this listing requires personalization. The default value is false. NOTE: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details." - }, - "personalization_char_count_max": { - "type": "integer", - "description": "[DEPRECATED] This is an integer value representing the maximum length for the personalization message entered by the buyer. Will only change if is_personalizable is 'true'. Note: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details.", - "format": "int64" - }, - "personalization_instructions": { - "type": "string", - "description": "[DEPRECATED] A string representing instructions for the buyer to enter the personalization. Will only change if is_personalizable is 'true'. Note: This field will be removed on Apr. 9th, 2026. See https://developers.etsy.com/documentation/tutorials/personalization-migration for migration details." - }, "state": { "type": "string", "description": "When _updating_ a listing, this value can be either `active` or `inactive`. Note: Setting a `draft` listing to `active` will also publish the listing on etsy.com and requires that the listing have an image set. Setting a `sold_out` listing to active will update the quantity to 1 and renew the listing on etsy.com.", diff --git a/tests/test_audit_ignore.py b/tests/test_audit_ignore.py index 227fd9f..865e448 100644 --- a/tests/test_audit_ignore.py +++ b/tests/test_audit_ignore.py @@ -669,3 +669,186 @@ def test_shipped_legacy_param_ignores_are_value_scoped(self): for ig in entries: assert ig["direction"] == "extra" assert ig["values"] == ["legacy"] + + def test_no_wildcard_body_drift_ignores(self): + # "*" on a body_drift entry would hide unreviewed request-body drift on + # that operation, defeating the self-verifying property. + path = SCRIPTS_DIR.parent / "specs" / "audit-ignore.json" + for ig in audit_sdk.load_ignores(path): + if ig["type"] == "body_drift": + assert ig["values"] != "*", f"{ig['key']} uses a wildcard" + + def test_shipped_personalization_body_ignores_are_value_scoped(self): + # Etsy removed the four personalization fields from the listing bodies. + # The kwargs are kept for source compatibility but never serialized, so + # the drift is suppressed by value — never "*". + path = SCRIPTS_DIR.parent / "specs" / "audit-ignore.json" + entries = [ + ig for ig in audit_sdk.load_ignores(path) if ig["type"] == "body_drift" + ] + assert {ig["key"] for ig in entries} == {"createDraftListing", "updateListing"} + for ig in entries: + assert ig["direction"] == "extra" + assert set(ig["values"]) == { + "is_personalizable", + "personalization_is_required", + "personalization_char_count_max", + "personalization_instructions", + } + + +# --------------------------------------------------------------------------- # +# compute_body_findings — request body drift +# --------------------------------------------------------------------------- # +class TestComputeBodyFindings: + def _spec(self, body_fields, content_type="application/json"): + return { + "components": {}, + "_body_fields": body_fields, + "_content_type": content_type, + } + + def _implemented(self, body_fields, model_params, content_type="application/json"): + return { + "updateListing": { + "spec": { + "parameters": [], + "requestBody": { + "content": { + content_type: { + "schema": { + "properties": {f: {} for f in body_fields} + } + } + } + }, + }, + "sdk": { + "params": ["listing"], + "param_annotations": {"listing": "UpdateListingRequest"}, + "file": "Listing.py", + "line": 20, + }, + "sdk_method": "update_listing", + } + }, { + "UpdateListingRequest": { + "init_params": list(model_params), + "file": "Listing.py", + "line": 164, + } + } + + def test_extra_model_field_detected(self): + implemented, models = self._implemented(["title"], ["title", "is_personalizable"]) + findings = audit_sdk.compute_body_findings({}, implemented, models) + assert len(findings) == 1 + assert findings[0]["type"] == "body_drift" + assert findings[0]["key"] == "updateListing" + assert findings[0]["direction"] == "extra" + assert findings[0]["values"] == {"is_personalizable"} + assert "models/Listing.py:164" in findings[0]["location"] + + def test_missing_model_field_detected(self): + implemented, models = self._implemented(["title", "description"], ["title"]) + findings = audit_sdk.compute_body_findings({}, implemented, models) + assert len(findings) == 1 + assert findings[0]["direction"] == "missing" + assert findings[0]["values"] == {"description"} + + def test_in_sync_yields_no_findings(self): + implemented, models = self._implemented(["title"], ["title"]) + assert audit_sdk.compute_body_findings({}, implemented, models) == [] + + def test_both_directions_yield_separate_findings(self): + implemented, models = self._implemented(["title"], ["legacy_field"]) + findings = audit_sdk.compute_body_findings({}, implemented, models) + assert {f["direction"] for f in findings} == {"missing", "extra"} + + def test_type_alias_normalized(self): + # The `listing_type` kwarg is serialized as the spec's `type` field + # (see TYPE_FIELD_ALIASES), so it must not be reported as drift. + implemented, models = self._implemented(["type"], ["listing_type"]) + assert audit_sdk.compute_body_findings({}, implemented, models) == [] + + def test_multipart_only_endpoint_skipped(self): + # FileRequest subclasses hold fields in data/file dicts that this + # signature-level comparison cannot resolve. + implemented, models = self._implemented( + ["image"], ["something_else"], content_type="multipart/form-data" + ) + assert audit_sdk.compute_body_findings({}, implemented, models) == [] + + def test_operation_without_body_skipped(self): + implemented = { + "getListing": { + "spec": {"parameters": [], "requestBody": None}, + "sdk": { + "params": [], + "param_annotations": {}, + "file": "Listing.py", + "line": 20, + }, + "sdk_method": "get_listing", + } + } + assert audit_sdk.compute_body_findings({}, implemented, {}) == [] + + +# --------------------------------------------------------------------------- # +# partition_findings — body_drift value verification +# --------------------------------------------------------------------------- # +class TestPartitionBodyDrift: + def _drift(self, values, direction="extra"): + return { + "type": "body_drift", + "key": "updateListing", + "direction": direction, + "values": set(values), + "location": "model `UpdateListingRequest` in models/Listing.py:164", + } + + def _ignore(self, values, direction="extra"): + return { + "type": "body_drift", + "key": "updateListing", + "direction": direction, + "values": values, + "reason": "removed from spec; kwarg kept but never serialized", + } + + def test_listed_field_suppressed(self): + active, suppressed, stale = audit_sdk.partition_findings( + [self._drift({"is_personalizable"})], [self._ignore(["is_personalizable"])] + ) + assert active == [] + assert len(suppressed) == 1 + assert stale == [] + + def test_newly_drifted_field_stays_active(self): + # The whole point of value-scoped suppression: a field nobody reviewed + # must still surface on an already-suppressed operation. + active, suppressed, stale = audit_sdk.partition_findings( + [self._drift({"is_personalizable", "brand_new_field"})], + [self._ignore(["is_personalizable"])], + ) + assert len(active) == 1 + assert active[0]["values"] == {"brand_new_field"} + assert len(suppressed) == 1 + assert suppressed[0]["values"] == {"is_personalizable"} + + def test_ignore_matching_nothing_is_stale(self): + active, suppressed, stale = audit_sdk.partition_findings( + [], [self._ignore(["is_personalizable"])] + ) + assert active == [] + assert suppressed == [] + assert len(stale) == 1 + + def test_direction_must_match(self): + active, suppressed, stale = audit_sdk.partition_findings( + [self._drift({"description"}, direction="missing")], + [self._ignore(["description"], direction="extra")], + ) + assert len(active) == 1 + assert len(stale) == 1 diff --git a/tests/test_listing_models.py b/tests/test_listing_models.py index 58dd223..c17b814 100644 --- a/tests/test_listing_models.py +++ b/tests/test_listing_models.py @@ -196,7 +196,7 @@ def test_create_warns_with_personalization_is_required(self): ) deprecation_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] assert len(deprecation_warnings) == 1 - assert "deprecated by the Etsy API" in str(deprecation_warnings[0].message) + assert "no longer sent" in str(deprecation_warnings[0].message) assert "personalization-migration" in str(deprecation_warnings[0].message) def test_update_no_warning_without_personalization(self): @@ -276,6 +276,84 @@ def test_create_warns_once_with_multiple_personalization_fields(self): assert "is_personalizable" in str(deprecation_warnings[0].message) +class TestPersonalizationFieldsNotSerialized: + """Etsy removed the four personalization fields from createDraftListing and + updateListing (2026-09 spec). The kwargs are kept for source compatibility, + but the values must never reach the wire.""" + + PERSONALIZATION_KWARGS = dict( + is_personalizable=True, + personalization_is_required=True, + personalization_char_count_max=256, + personalization_instructions="Enter name", + ) + + def _make_create_kwargs(self): + return dict( + quantity=1, + title="Test", + description="A test", + price=25.00, + who_made=WhoMade.I_DID, + when_made=WhenMade.TWENTY_TWENTIES, + taxonomy_id=30303, + ) + + def test_create_omits_personalization_from_payload(self): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + req = CreateDraftListingRequest( + **self._make_create_kwargs(), **self.PERSONALIZATION_KWARGS + ) + payload = req.get_dict() + assert [key for key in payload if "personaliz" in key] == [] + # Non-removed fields still serialize normally. + assert payload["title"] == "Test" + + def test_update_omits_personalization_from_payload(self): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + req = UpdateListingRequest(title="Updated", **self.PERSONALIZATION_KWARGS) + payload = req.get_dict() + assert [key for key in payload if "personaliz" in key] == [] + assert payload["title"] == "Updated" + + def test_create_keeps_personalization_readable(self): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + req = CreateDraftListingRequest( + **self._make_create_kwargs(), **self.PERSONALIZATION_KWARGS + ) + assert req.is_personalizable is True + assert req.personalization_is_required is True + assert req.personalization_char_count_max == 256 + assert req.personalization_instructions == "Enter name" + + def test_update_keeps_personalization_readable(self): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + req = UpdateListingRequest(**self.PERSONALIZATION_KWARGS) + assert req.is_personalizable is True + assert req.personalization_is_required is True + assert req.personalization_char_count_max == 256 + assert req.personalization_instructions == "Enter name" + + def test_warning_points_at_caller(self): + # The warning is raised two frames below __init__, so stacklevel must + # be deep enough to blame the caller's line, not the SDK internals. + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + UpdateListingRequest(is_personalizable=True) + assert w[0].filename == __file__ + + def test_defaults_to_none_when_not_passed(self): + req = UpdateListingRequest(title="Updated") + assert req.is_personalizable is None + assert req.personalization_is_required is None + assert req.personalization_char_count_max is None + assert req.personalization_instructions is None + + class TestUpdateListingTranslationRequest: def test_stores_fields(self): req = UpdateListingTranslationRequest( From 07ce603b39adf40623ea5708dd3be88c187cbdbd Mon Sep 17 00:00:00 2001 From: Amit Ray <51674969+amitray007@users.noreply.github.com> Date: Sun, 6 Sep 2026 05:47:44 +0530 Subject: [PATCH 2/2] feat: address code review feedback for personalization field removal - compute_body_findings: restore the original subtraction semantics in the no-model-class branch. Path/query param names are now excluded from the "extra" direction only, as before. Subtracting them from the shared field set made a body field that shares a path-param name (taxonomy_id, shipping_profile_id, shop_section_id, return_policy_id) falsely report as "In spec but not SDK". - Personalization fields: add setters alongside the properties. Making them getter-only broke post-construction assignment with AttributeError, which the back-compat goal was meant to avoid. A write now stores the value and warns, exactly like the constructor kwarg, and still never serializes. _warn_if_personalization_used takes the stacklevel so both paths blame the caller's line. Uses a feat: prefix so the merge produces a minor version bump: the branch changes wire behavior for callers that relied on the four personalization fields being transmitted. Etsy already removed them from the request bodies, so they were being rejected regardless, but the change warrants more than a patch bump. --- etsy_python/v3/models/Listing.py | 41 ++++++++++++++++++++++++----- scripts/audit_sdk.py | 13 +++++++--- tests/test_audit_ignore.py | 44 ++++++++++++++++++++++++++++++++ tests/test_listing_models.py | 42 ++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 11 deletions(-) diff --git a/etsy_python/v3/models/Listing.py b/etsy_python/v3/models/Listing.py index 812b422..bd02b38 100644 --- a/etsy_python/v3/models/Listing.py +++ b/etsy_python/v3/models/Listing.py @@ -31,12 +31,18 @@ def _warn_if_personalization_used( personalization_is_required: Optional[bool], personalization_char_count_max: Optional[int], personalization_instructions: Optional[str], + stacklevel: int = 4, ) -> None: """Emit DeprecationWarning when a removed personalization field is actively set. Falsy values (False, 0, empty string, None) match the API's documented defaults and were no-ops even before the fields were removed, so they do not trigger the warning. + + ``stacklevel`` counts the frames back to the caller. It defaults to the + constructor path (this function <- ``_store_personalization`` <- model + ``__init__`` <- caller); the property setters pass 3, being one frame + shallower. """ if any([ is_personalizable, @@ -47,40 +53,61 @@ def _warn_if_personalization_used( warnings.warn( _PERSONALIZATION_DEPRECATION_MSG, DeprecationWarning, - # _warn_if_personalization_used <- _store_personalization <- - # model __init__ <- caller - stacklevel=4, + stacklevel=stacklevel, ) class _PersonalizationFieldsMixin: - """Read-only access to the removed personalization fields. + """Accessors for the personalization fields Etsy removed from listings. Etsy dropped these four fields from the createDraftListing and updateListing request bodies (2026-09 spec). The constructor keyword arguments are kept so existing callers don't break, but the values are stored under underscore-prefixed names, which ``todict`` excludes from - serialization -- so they are never sent on the wire. The properties below - keep attribute reads working for callers that inspect the model. Remove - the keyword arguments and this mixin in the next major version. + serialization -- so they are never sent on the wire. + + The properties below keep both attribute reads and writes working. A write + stores the value and warns, exactly like passing the keyword argument: it + never reaches the API either way. Remove the keyword arguments and this + mixin in the next major version. """ @property def is_personalizable(self) -> Optional[bool]: return self._is_personalizable + @is_personalizable.setter + def is_personalizable(self, value: Optional[bool]) -> None: + self._is_personalizable = value + _warn_if_personalization_used(value, None, None, None, stacklevel=3) + @property def personalization_is_required(self) -> Optional[bool]: return self._personalization_is_required + @personalization_is_required.setter + def personalization_is_required(self, value: Optional[bool]) -> None: + self._personalization_is_required = value + _warn_if_personalization_used(None, value, None, None, stacklevel=3) + @property def personalization_char_count_max(self) -> Optional[int]: return self._personalization_char_count_max + @personalization_char_count_max.setter + def personalization_char_count_max(self, value: Optional[int]) -> None: + self._personalization_char_count_max = value + _warn_if_personalization_used(None, None, value, None, stacklevel=3) + @property def personalization_instructions(self) -> Optional[str]: return self._personalization_instructions + @personalization_instructions.setter + def personalization_instructions(self, value: Optional[str]) -> None: + self._personalization_instructions = value + _warn_if_personalization_used(None, None, None, value, stacklevel=3) + def _store_personalization( self, is_personalizable: Optional[bool], diff --git a/scripts/audit_sdk.py b/scripts/audit_sdk.py index 89ff09c..39ab442 100644 --- a/scripts/audit_sdk.py +++ b/scripts/audit_sdk.py @@ -705,6 +705,12 @@ def compute_body_findings( model_class_name = ptype break + # Names excluded from the "extra" direction only. A path/query param in + # the method signature is not an unexpected body field, but it may + # legitimately also be a body field (e.g. taxonomy_id), so it must stay + # in the set that answers "does the SDK accept this body field?". + non_body_names: Set[str] = set() + if model_class_name: model_info = sdk_models[model_class_name] # Normalize kwargs the SDK serializes under a different spec name @@ -718,16 +724,15 @@ def compute_body_findings( ) else: # Method has body fields but no model object - compare against params - sdk_fields = set(sdk["params"]) - { - p["name"] for p in op["parameters"] - } - PATH_PARAM_NAMES + sdk_fields = set(sdk["params"]) + non_body_names = {p["name"] for p in op["parameters"]} | PATH_PARAM_NAMES location = ( f"`{mapping['sdk_method']}` in " f"{sdk['file']}:{sdk['line']}, no model class" ) spec_only = spec_body_fields - sdk_fields - sdk_only = sdk_fields - spec_body_fields + sdk_only = sdk_fields - spec_body_fields - non_body_names for direction, values in (("missing", spec_only), ("extra", sdk_only)): if values: diff --git a/tests/test_audit_ignore.py b/tests/test_audit_ignore.py index 865e448..3d134ee 100644 --- a/tests/test_audit_ignore.py +++ b/tests/test_audit_ignore.py @@ -779,6 +779,50 @@ def test_multipart_only_endpoint_skipped(self): ) assert audit_sdk.compute_body_findings({}, implemented, models) == [] + def _no_model_implemented(self, body_fields, sdk_params, spec_params=()): + return { + "updateX": { + "spec": { + "parameters": [{"name": n} for n in spec_params], + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": {f: {} for f in body_fields} + } + } + } + }, + }, + "sdk": { + "params": list(sdk_params), + "param_annotations": {}, + "file": "X.py", + "line": 10, + }, + "sdk_method": "update_x", + } + } + + def test_body_field_sharing_a_path_param_name_is_not_false_drift(self): + # taxonomy_id is a body field here AND a known path-param name. It is + # accepted by the method, so it must not be reported as missing. + implemented = self._no_model_implemented( + ["taxonomy_id", "title"], ["taxonomy_id", "title"] + ) + assert audit_sdk.compute_body_findings({}, implemented, {}) == [] + + def test_path_param_not_in_body_is_not_extra_drift(self): + # A path param in the signature is not an unexpected body field. + implemented = self._no_model_implemented(["title"], ["title", "shop_id"]) + assert audit_sdk.compute_body_findings({}, implemented, {}) == [] + + def test_query_param_not_in_body_is_not_extra_drift(self): + implemented = self._no_model_implemented( + ["title"], ["title", "legacy"], spec_params=["legacy"] + ) + assert audit_sdk.compute_body_findings({}, implemented, {}) == [] + def test_operation_without_body_skipped(self): implemented = { "getListing": { diff --git a/tests/test_listing_models.py b/tests/test_listing_models.py index c17b814..8208fff 100644 --- a/tests/test_listing_models.py +++ b/tests/test_listing_models.py @@ -346,6 +346,48 @@ def test_warning_points_at_caller(self): UpdateListingRequest(is_personalizable=True) assert w[0].filename == __file__ + def test_post_construction_assignment_still_works(self): + # The fields became properties; assignment must keep working for + # callers that set them after construction, and must still not + # serialize. + req = UpdateListingRequest(title="Updated") + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + req.is_personalizable = True + req.personalization_is_required = True + req.personalization_char_count_max = 256 + req.personalization_instructions = "Enter name" + deprecation_warnings = [ + x for x in w if issubclass(x.category, DeprecationWarning) + ] + assert len(deprecation_warnings) == 4 + assert req.is_personalizable is True + assert req.personalization_is_required is True + assert req.personalization_char_count_max == 256 + assert req.personalization_instructions == "Enter name" + assert [key for key in req.get_dict() if "personaliz" in key] == [] + + def test_setter_warning_points_at_caller(self): + # The setter path is one frame shallower than the constructor path, so + # it passes its own stacklevel. + req = UpdateListingRequest(title="Updated") + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + req.is_personalizable = True + assert w[0].filename == __file__ + + def test_setter_does_not_warn_on_falsy_value(self): + req = UpdateListingRequest(title="Updated") + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + req.is_personalizable = False + req.personalization_char_count_max = 0 + deprecation_warnings = [ + x for x in w if issubclass(x.category, DeprecationWarning) + ] + assert len(deprecation_warnings) == 0 + assert req.is_personalizable is False + def test_defaults_to_none_when_not_passed(self): req = UpdateListingRequest(title="Updated") assert req.is_personalizable is None