Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,15 @@ All code lives under `etsy_python/v3/`:
Every resource is a `@dataclass` with `session: EtsyClient`. Methods return `Union[Response, RequestException]`.

- **GET/DELETE**: Parameters passed as `**kwargs`, built into query string via `generate_get_uri()`
- **Required ID-list params** (`listing_ids`, `payment_ids`, `shop_section_ids`, ...): join with `join_ids(name, ids)` from `common/Utils.py`. It raises `ValueError` for an empty list and `TypeError` for a string, locally and before any request, so these methods can raise those in addition to `RequestException`
- **POST/PUT/PATCH**: Payload is a `Request` model, serialized via `.get_dict()` to JSON
- **File uploads**: Payload is a `FileRequest`, sent as multipart form data
- **Enums**: Always use `.value` to extract the API string (e.g., `State.ACTIVE.value` -> `"active"`)

### Utility Functions (`common/Utils.py`)

- `generate_get_uri(uri, **kwargs)` -- builds query string from kwargs dict
- `join_ids(name, ids)` -- validates and comma-joins a required ID list (raises `ValueError` if empty, `TypeError` if a string)
- `todict(obj)` -- recursive serializer handling Enums, nested objects, nullable fields
- `generate_bytes_from_file(file)` -- reads file bytes for uploads

Expand Down Expand Up @@ -210,5 +212,6 @@ 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`,
`body_drift`, `code_issue`. Prefer an explicit `values` list over `"*"` — a wildcard
`body_drift`, `code_issue`. `param_drift` directions are `missing`, `extra`, and `optional`
(required in the spec but defaulted to `None` in the SDK). Prefer an explicit `values` list over `"*"` — a wildcard
hides everything on that key, including drift nobody has reviewed.
17 changes: 16 additions & 1 deletion etsy_python/v3/common/Utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import warnings
from enum import Enum
from typing import Any, Dict, List, Optional
from typing import Any, Dict, Iterable, List, Optional


def warn_removed_legacy_param(operation_id: str) -> None:
Expand All @@ -25,6 +25,21 @@ def warn_removed_legacy_param(operation_id: str) -> None:
)


def join_ids(name: str, ids: Iterable[Any]) -> str:
"""Join a required list of IDs into Etsy's comma-separated query value.

Raises ``TypeError`` for a string (iterating it would split the ID into
characters) and ``ValueError`` when no IDs are given, so a request Etsy
would reject is caught before it is sent.
"""
if isinstance(ids, (str, bytes)):
raise TypeError(f"{name} must be a list of IDs, not a string")
joined = ",".join(map(str, ids)) if ids is not None else ""
if not joined:
raise ValueError(f"{name} is required and must contain at least one ID")
return joined


def generate_get_uri(uri: str, params: Optional[Dict[str, Any]] = None) -> str:
if not params:
return uri
Expand Down
19 changes: 11 additions & 8 deletions etsy_python/v3/resources/Listing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from dataclasses import dataclass
from typing import Optional, List, Dict, Any, Union

from etsy_python.v3.common.Utils import warn_removed_legacy_param
from etsy_python.v3.common.Utils import join_ids, warn_removed_legacy_param
from etsy_python.v3.enums.Listing import Includes, State, SortOn, SortOrder
from etsy_python.v3.exceptions.RequestException import RequestException
from etsy_python.v3.models.Listing import (
Expand Down Expand Up @@ -150,7 +150,7 @@ def get_listings_by_listing_ids(
) -> Union[Response, RequestException]:
endpoint = "/listings/batch"
query_params: Dict[str, Any] = {
"listing_ids": ",".join(list(map(str, listing_ids))),
"listing_ids": join_ids("listing_ids", listing_ids),
"includes": ",".join(list(map(lambda inc: inc.value, includes)))
if includes is not None
else None,
Expand Down Expand Up @@ -179,7 +179,7 @@ def get_listings_inventory_by_listing_ids(
) -> Union[Response, RequestException]:
endpoint = "/listings/batch/inventory"
query_params: Dict[str, Any] = {
"listing_ids": ",".join(list(map(str, listing_ids))),
"listing_ids": join_ids("listing_ids", listing_ids),
}
return self.session.make_request(endpoint, query_params=query_params)

Expand All @@ -188,7 +188,7 @@ def get_listings_shipping_by_listing_ids(
) -> Union[Response, RequestException]:
endpoint = "/listings/batch/shipping"
query_params: Dict[str, Any] = {
"listing_ids": ",".join(list(map(str, listing_ids))),
"listing_ids": join_ids("listing_ids", listing_ids),
}
return self.session.make_request(endpoint, query_params=query_params)

Expand Down Expand Up @@ -260,18 +260,21 @@ def get_listings_by_shop_return_policy(
def get_listings_by_shop_section_id(
self,
shop_id: int,
shop_section_ids: Optional[List[int]] = None,
shop_section_ids: List[int],
limit: int = 25,
offset: int = 0,
sort_on: SortOn = SortOn.CREATED,
sort_order: SortOrder = SortOrder.DESC,
legacy: Optional[bool] = None,
) -> Union[Response, RequestException]:
"""Get listings in the given shop sections.

Raises ``ValueError`` if ``shop_section_ids`` is empty and
``TypeError`` if it is a string, before any request is sent.
"""
endpoint = f"/shops/{shop_id}/shop-sections/listings"
query_params: Dict[str, Any] = {
"shop_section_ids": ",".join(list(map(str, shop_section_ids)))
if shop_section_ids is not None
else None,
"shop_section_ids": join_ids("shop_section_ids", shop_section_ids),
"limit": limit,
"offset": offset,
"sort_on": sort_on.value,
Expand Down
5 changes: 3 additions & 2 deletions etsy_python/v3/resources/Payment.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from dataclasses import dataclass
from typing import Union, Dict, Any, List

from etsy_python.v3.common.Utils import join_ids
from etsy_python.v3.exceptions.RequestException import RequestException
from etsy_python.v3.resources.Session import EtsyClient
from etsy_python.v3.resources.Response import Response
Expand All @@ -16,7 +17,7 @@ def get_payment_account_ledger_entry_payments(
) -> Union[Response, RequestException]:
endpoint = f"/shops/{shop_id}/payment-account/ledger-entries/payments"
query_params: Dict[str, Any] = {
"ledger_entry_ids": ",".join(list(map(str, ledger_entry_ids)))
"ledger_entry_ids": join_ids("ledger_entry_ids", ledger_entry_ids)
}
return self.session.make_request(endpoint, query_params=query_params)

Expand All @@ -42,5 +43,5 @@ def get_payments(
self, shop_id: int, payment_ids: List[int]
) -> Union[Response, RequestException]:
endpoint = f"/shops/{shop_id}/payments"
query_params: Dict[str, Any] = {"payment_ids": ",".join(list(map(str, payment_ids)))}
query_params: Dict[str, Any] = {"payment_ids": join_ids("payment_ids", payment_ids)}
return self.session.make_request(endpoint, query_params=query_params)
29 changes: 26 additions & 3 deletions scripts/audit_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,16 @@ def scan_resource_methods(resources_dir: Path) -> Dict[str, Dict[str, dict]]:
if arg.annotation and isinstance(arg.annotation, ast.Name):
param_annotations[arg.arg] = arg.annotation.id

# Params whose default is the literal None (defaults
# align with the tail of the positional args).
args = item.args.args
defaults = item.args.defaults
none_defaults = {
arg.arg
for arg, default in zip(args[len(args) - len(defaults):], defaults)
if isinstance(default, ast.Constant) and default.value is None
}

# Detect NotImplementedError stubs
is_not_implemented = False
if len(item.body) == 1:
Expand All @@ -211,6 +221,7 @@ def scan_resource_methods(resources_dir: Path) -> Dict[str, Dict[str, dict]]:
file_methods[item.name] = {
"params": params,
"param_annotations": param_annotations,
"none_defaults": none_defaults,
"class": node.name,
"file": py_file.name,
"line": item.lineno,
Expand Down Expand Up @@ -629,7 +640,9 @@ def compute_param_findings(

Yields one finding per (operation, direction) whose parameter sets differ.
``direction`` is ``"missing"`` (in the spec, absent from the SDK signature)
or ``"extra"`` (in the SDK signature, absent from the spec). ``values`` is a
or ``"extra"`` (in the SDK signature, absent from the spec) or
``"optional"`` (required in the spec, but the SDK defaults it to ``None``,
so omitting it silently sends a request Etsy rejects). ``values`` is a
set of parameter names, so an ignore listing specific names suppresses only
those — a newly drifted parameter on an already-suppressed operation still
surfaces. Model-object and path params are excluded from the comparison.
Expand All @@ -653,8 +666,15 @@ def compute_param_findings(
non_model_sdk_params = sdk_params - model_param_names
spec_only = spec_params - non_model_sdk_params
sdk_only = non_model_sdk_params - spec_params - PATH_PARAM_NAMES

for direction, values in (("missing", spec_only), ("extra", sdk_only)):
required_but_optional = {
p["name"] for p in op["parameters"] if p.get("required")
} & set(sdk.get("none_defaults", ()))

for direction, values in (
("missing", spec_only),
("extra", sdk_only),
("optional", required_but_optional),
):
if values:
findings.append(
{
Expand Down Expand Up @@ -1017,6 +1037,9 @@ def generate_report(
if "extra" in directions:
names = ", ".join(sorted(directions["extra"]["values"]))
lines.append(f"- In SDK but not spec: {names}")
if "optional" in directions:
names = ", ".join(sorted(directions["optional"]["values"]))
lines.append(f"- Required in spec but defaults to None in SDK: {names}")
lines.append("")
else:
lines.append("No query/path parameter drift detected.\n")
Expand Down
40 changes: 38 additions & 2 deletions tests/test_audit_ignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,20 @@ def test_ambiguous_suffix_matches_are_not_guessed(self):
# compute_param_findings — query/path parameter drift
# --------------------------------------------------------------------------- #
class TestComputeParamFindings:
def _implemented(self, spec_params, sdk_params, annotations=None):
def _implemented(
self, spec_params, sdk_params, annotations=None, required=(), none_defaults=()
):
return {
"getX": {
"spec": {"parameters": [{"name": n} for n in spec_params]},
"spec": {
"parameters": [
{"name": n, "required": n in required} for n in spec_params
]
},
"sdk": {
"params": list(sdk_params),
"param_annotations": annotations or {},
"none_defaults": set(none_defaults),
"file": "X.py",
"line": 10,
},
Expand Down Expand Up @@ -396,6 +403,35 @@ def test_path_params_excluded(self):
)
assert findings == []

def test_required_param_defaulting_to_none_detected(self):
findings = audit_sdk.compute_param_findings(
self._implemented(
["ids", "limit"],
["ids", "limit"],
required={"ids"},
none_defaults={"ids", "limit"},
),
{},
)
assert len(findings) == 1
assert findings[0]["direction"] == "optional"
assert findings[0]["values"] == {"ids"}

def test_required_param_without_default_not_flagged(self):
findings = audit_sdk.compute_param_findings(
self._implemented(["ids"], ["ids"], required={"ids"}), {}
)
assert findings == []

def test_scan_records_none_defaults(self, tmp_path):
(tmp_path / "X.py").write_text(
"class R:\n"
" def get_x(self, a: int, b=None, c: int = 0, d=None):\n"
" pass\n"
)
info = audit_sdk.scan_resource_methods(tmp_path)["X"]["get_x"]
assert info["none_defaults"] == {"b", "d"}


# --------------------------------------------------------------------------- #
# partition_findings — param_drift value verification
Expand Down
48 changes: 41 additions & 7 deletions tests/test_listing_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,16 +583,50 @@ def test_with_section_ids(self, mock_session):
qp = mock_session.make_request.call_args[1]["query_params"]
assert qp["shop_section_ids"] == "111,222"

def test_without_section_ids(self, mock_session):
mock_session.make_request.return_value = Response(
200, make_shop_listing_collection()
)
def test_section_ids_required(self, mock_session):
"""Etsy marks shop_section_ids as required, so omitting it is a TypeError."""
resource = ListingResource(session=mock_session)

resource.get_listings_by_shop_section_id(MOCK_SHOP_ID)
with pytest.raises(TypeError):
resource.get_listings_by_shop_section_id(MOCK_SHOP_ID)

qp = mock_session.make_request.call_args[1]["query_params"]
assert qp["shop_section_ids"] is None
mock_session.make_request.assert_not_called()

@pytest.mark.parametrize("section_ids", [None, []])
def test_section_ids_none_or_empty_raises(self, mock_session, section_ids):
"""None or [] would send an invalid request, so reject it before calling Etsy."""
resource = ListingResource(session=mock_session)

with pytest.raises(ValueError, match="shop_section_ids"):
resource.get_listings_by_shop_section_id(MOCK_SHOP_ID, section_ids)

mock_session.make_request.assert_not_called()

def test_string_section_ids_raises(self, mock_session):
"""A string would be split into characters and sent as bogus IDs."""
resource = ListingResource(session=mock_session)

with pytest.raises(TypeError, match="shop_section_ids"):
resource.get_listings_by_shop_section_id(MOCK_SHOP_ID, "111,222")

mock_session.make_request.assert_not_called()


@pytest.mark.parametrize(
"method",
[
"get_listings_by_listing_ids",
"get_listings_inventory_by_listing_ids",
"get_listings_shipping_by_listing_ids",
],
)
def test_batch_listing_methods_reject_empty_ids(mock_session, method):
resource = ListingResource(session=mock_session)

with pytest.raises(ValueError, match="listing_ids"):
getattr(resource, method)([])

mock_session.make_request.assert_not_called()


class TestErrorPropagation:
Expand Down
18 changes: 18 additions & 0 deletions tests/test_payment_resource.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import pytest

from etsy_python.v3.resources.Payment import PaymentResource
from etsy_python.v3.resources.Response import Response

Expand Down Expand Up @@ -32,6 +34,14 @@ def test_payment_ids_joined(self, mock_session):
qp = mock_session.make_request.call_args[1]["query_params"]
assert qp["payment_ids"] == f"{MOCK_PAYMENT_ID},20202"

def test_empty_payment_ids_raises(self, mock_session):
resource = PaymentResource(session=mock_session)

with pytest.raises(ValueError, match="payment_ids"):
resource.get_payments(MOCK_SHOP_ID, [])

mock_session.make_request.assert_not_called()


class TestGetShopPaymentAccountLedgerEntryPayments:
def test_ledger_entry_ids_joined(self, mock_session):
Expand All @@ -50,3 +60,11 @@ def test_ledger_entry_ids_joined(self, mock_session):
== f"/shops/{MOCK_SHOP_ID}/payment-account/ledger-entries/payments"
)
assert call_args[1]["query_params"]["ledger_entry_ids"] == f"{MOCK_LEDGER_ENTRY_ID},30303"

def test_empty_ledger_entry_ids_raises(self, mock_session):
resource = PaymentResource(session=mock_session)

with pytest.raises(ValueError, match="ledger_entry_ids"):
resource.get_payment_account_ledger_entry_payments(MOCK_SHOP_ID, [])

mock_session.make_request.assert_not_called()
20 changes: 20 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,32 @@
from enum import Enum

import pytest

from etsy_python.v3.common.Utils import (
generate_bytes_from_file,
generate_get_uri,
join_ids,
todict,
)


class TestJoinIds:
@pytest.mark.parametrize("ids", [[1, 2, 3], (1, 2, 3), (i for i in (1, 2, 3))])
def test_joins_any_iterable(self, ids):
assert join_ids("ids", ids) == "1,2,3"

@pytest.mark.parametrize("ids", [None, [], (), iter([])])
def test_empty_raises_value_error(self, ids):
with pytest.raises(ValueError, match="ids is required"):
join_ids("ids", ids)

@pytest.mark.parametrize("ids", ["111,222", b"111"])
def test_string_raises_type_error(self, ids):
# Iterating a string would send "1,1,1,,,2,2,2".
with pytest.raises(TypeError, match="not a string"):
join_ids("ids", ids)


class TestGenerateGetUri:
def test_no_params(self):
assert generate_get_uri("/shops/123") == "/shops/123"
Expand Down
Loading