Skip to content

Commit 5ce9ee4

Browse files
authored
Merge pull request #1546 from adcontextprotocol/bokelley/adcp-client-python-issue-1545
fix(types)!: allow assignment-only creative syncs
2 parents 15c85cf + 81c2a8b commit 5ce9ee4

5 files changed

Lines changed: 166 additions & 4 deletions

File tree

‎examples/v3_reference_seller/src/platform.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1768,7 +1768,7 @@ async def sync_creatives(
17681768
advertiser_id = ctx.account.metadata["advertiser_id"]
17691769
results: list[SyncCreativeResult] = []
17701770
client = self._client(ctx)
1771-
for creative in req.creatives:
1771+
for creative in req.creatives or []:
17721772
# Re-sync of a known creative_id is a no-op upload — the
17731773
# upstream's idempotency table is keyed on body-fingerprint
17741774
# and rejects same-key/different-body with 409. The spec

‎examples/v3_reference_seller/tests/test_smoke_broadening.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,6 +1416,38 @@ async def test_sync_creatives_uploads_each_creative_to_upstream(
14161416
assert "spring-300x250" in body
14171417

14181418

1419+
@pytest.mark.asyncio
1420+
@respx.mock(base_url=_RESPX_BASE_URL)
1421+
async def test_sync_creatives_assigns_existing_creative_without_upload(respx_mock: Any) -> None:
1422+
from adcp.types import SyncCreativesRequest
1423+
1424+
attach = respx_mock.post("/v1/orders/ord_1/lineitems/pkg_1/creative-attach").mock(
1425+
return_value=httpx.Response(200, json={})
1426+
)
1427+
platform = _platform_with_upstream()
1428+
ctx = _build_ctx()
1429+
scope = platform._account_scope(ctx)
1430+
platform._creative_id_map[(scope, "existing")] = "up_cr_1"
1431+
platform._buy_state[(scope, "ord_1")] = {"packages": {"pkg_1": {}}}
1432+
req = SyncCreativesRequest.model_validate(
1433+
{
1434+
"account": {"account_id": "signed-buyer-main"},
1435+
"idempotency_key": "assignment-only-sync-0001",
1436+
"assignments": [{"creative_id": "existing", "package_id": "pkg_1"}],
1437+
}
1438+
)
1439+
1440+
response = await platform.sync_creatives(req, ctx)
1441+
1442+
assert attach.call_count == 1
1443+
assert json.loads(attach.calls.last.request.content) == {"creative_id": "up_cr_1"}
1444+
assert len(respx_mock.calls) == 1
1445+
assert response.creatives == []
1446+
assert platform._buy_state[(scope, "ord_1")]["packages"]["pkg_1"]["creative_assignments"] == [
1447+
{"creative_id": "existing"}
1448+
]
1449+
1450+
14191451
@pytest.mark.asyncio
14201452
@respx.mock(base_url=_RESPX_BASE_URL)
14211453
async def test_get_media_buys_filters_by_advertiser_id(respx_mock: Any) -> None:

‎src/adcp/types/canonical_creative.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,8 @@ class UpdateMediaBuyResponse3(_LegacyUpdateMediaBuyResponse3, CanonicalBoundaryM
12191219
class SyncCreativesRequest(_LegacySyncCreativesRequest, CanonicalBoundaryModel):
12201220
"""Canonical creative sync request; creatives are canonical assets."""
12211221

1222-
creatives: list[CreativeAsset] = Field(min_length=1) # type: ignore[assignment]
1222+
# Requiredness is version-specific: AdCP 3.2 permits assignment-only syncs.
1223+
creatives: list[CreativeAsset] | None = Field(default=None, min_length=1) # type: ignore[assignment]
12231224

12241225

12251226
class ListCreativesRequest(_LegacyListCreativesRequest, CanonicalBoundaryModel):
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
"""AdCP 3.2 can traffic existing creatives without uploading assets (#1545)."""
2+
3+
from __future__ import annotations
4+
5+
import json
6+
from typing import Any
7+
8+
import pytest
9+
from pydantic import ValidationError
10+
11+
from adcp.server.base import ADCPHandler, ToolContext
12+
from adcp.server.mcp_tools import create_tool_caller
13+
from adcp.types import CreativeAsset, SyncCreativesRequest
14+
from adcp.validation import ValidationHookConfig, validate_request
15+
16+
_BASE = {
17+
"adcp_version": "3.2",
18+
"idempotency_key": "assignment-only-sync-0001",
19+
"account": {"account_id": "acct_1"},
20+
}
21+
_ASSIGNMENTS = {"assignments": [{"creative_id": "cr_1", "package_id": "pkg_1"}]}
22+
_MUTATIONS = [
23+
pytest.param(_ASSIGNMENTS, id="assignments"),
24+
*[
25+
pytest.param(
26+
{
27+
"assignment_operations": [
28+
{
29+
"operation": operation,
30+
"creative_id": "cr_1",
31+
"package_id": "pkg_1",
32+
**({"replaces_creative_id": "cr_old"} if operation == "replace" else {}),
33+
}
34+
]
35+
},
36+
id=operation,
37+
)
38+
for operation in ("assign", "unassign", "replace")
39+
],
40+
]
41+
42+
43+
@pytest.mark.parametrize("mutation", _MUTATIONS)
44+
@pytest.mark.parametrize("json_input", [False, True], ids=["python", "json"])
45+
def test_public_model_accepts_assignment_only_sync(
46+
mutation: dict[str, Any], json_input: bool
47+
) -> None:
48+
payload = {**_BASE, **mutation}
49+
request = (
50+
SyncCreativesRequest.model_validate_json(json.dumps(payload))
51+
if json_input
52+
else SyncCreativesRequest.model_validate(payload)
53+
)
54+
55+
assert request.creatives is None
56+
wire = request.model_dump(mode="json", exclude_unset=True)
57+
assert wire == payload
58+
assert validate_request("sync_creatives", wire, version="3.2").valid
59+
60+
61+
def test_empty_creatives_are_rejected_even_with_assignments() -> None:
62+
with pytest.raises(ValidationError) as caught:
63+
SyncCreativesRequest.model_validate({**_BASE, **_ASSIGNMENTS, "creatives": []})
64+
assert caught.value.errors()[0]["loc"] == ("creatives",)
65+
assert caught.value.errors()[0]["type"] == "too_short"
66+
67+
68+
def test_sync_still_requires_assets_or_assignments() -> None:
69+
with pytest.raises(ValidationError) as caught:
70+
SyncCreativesRequest.model_validate(_BASE)
71+
assert caught.value.errors()[0]["type"] == "anyOf"
72+
73+
74+
def test_asset_sync_still_parses_canonical_creatives() -> None:
75+
request = SyncCreativesRequest.model_validate(
76+
{
77+
**_BASE,
78+
"creatives": [
79+
{"creative_id": "cr_1", "name": "Banner", "format_kind": "image", "assets": {}}
80+
],
81+
}
82+
)
83+
assert request.creatives is not None
84+
assert isinstance(request.creatives[0], CreativeAsset)
85+
assert request.creatives[0].format_kind == "image"
86+
87+
88+
def test_public_schema_allows_omitting_creatives() -> None:
89+
assert "creatives" not in SyncCreativesRequest.model_json_schema()["required"]
90+
91+
92+
class _AssignmentHandler(ADCPHandler[Any]):
93+
adcp_capabilities = {"media_buy": {"features": {"canonical_creatives": True}}}
94+
95+
def __init__(self) -> None:
96+
self.received: SyncCreativesRequest | None = None
97+
98+
async def sync_creatives(
99+
self, params: SyncCreativesRequest, context: ToolContext | None = None
100+
) -> dict[str, Any]:
101+
self.received = params
102+
return {"creatives": []}
103+
104+
105+
@pytest.mark.parametrize("mutation", _MUTATIONS)
106+
async def test_assignment_only_sync_reaches_typed_handler(mutation: dict[str, Any]) -> None:
107+
handler = _AssignmentHandler()
108+
caller = create_tool_caller(
109+
handler, "sync_creatives", validation=ValidationHookConfig(requests="strict")
110+
)
111+
112+
result = await caller({**_BASE, **mutation})
113+
114+
assert result["creatives"] == []
115+
assert isinstance(handler.received, SyncCreativesRequest)
116+
assert handler.received.creatives is None
117+
assert handler.received.model_dump(mode="json", exclude_unset=True) == {**_BASE, **mutation}
118+
119+
120+
@pytest.mark.parametrize("version", ["3.0", "3.1"])
121+
def test_older_version_schemas_still_require_creatives(version: str) -> None:
122+
payload = {**_BASE, **_ASSIGNMENTS}
123+
payload.pop("adcp_version")
124+
outcome = validate_request("sync_creatives", payload, version=version)
125+
126+
assert not outcome.valid
127+
assert any(
128+
issue.pointer == "/creatives" and issue.keyword == "required" for issue in outcome.issues
129+
)

‎tests/type_checks/canonical_override_field_types.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ def create_media_buy_response1_packages(response: CreateMediaBuyResponse1) -> li
7979

8080

8181
def sync_creatives_request_creatives(request: SyncCreativesRequest) -> list[str]:
82-
assert_type(request.creatives, list[CreativeAsset])
83-
return [asset.creative_id for asset in request.creatives]
82+
assert_type(request.creatives, list[CreativeAsset] | None)
83+
return [asset.creative_id for asset in request.creatives or []]
8484

8585

8686
def list_creatives_response_creatives(response: ListCreativesResponse) -> list[str]:

0 commit comments

Comments
 (0)