From 25001155f38baabda7c3846fb59807c5a17d67a0 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 19 Dec 2025 12:47:17 -0500 Subject: [PATCH] fix: handle all authorization types in get_properties_by_agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves #99 - get_properties_by_agent() now handles all AdCP v2 authorization types: - inline_properties: Properties defined directly on the agent - property_ids: Filter top-level properties by property_id - property_tags: Filter top-level properties by tags - publisher_properties: Cross-domain property selectors Also fixes AuthorizationContext to use "property_id" field per AdCP v2 schema instead of the incorrect "id" field. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/adcp/adagents.py | 62 +++++++-- tests/test_adagents.py | 301 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 328 insertions(+), 35 deletions(-) diff --git a/src/adcp/adagents.py b/src/adcp/adagents.py index a05433f8..d2008414 100644 --- a/src/adcp/adagents.py +++ b/src/adcp/adagents.py @@ -478,12 +478,19 @@ def get_all_tags(adagents_data: dict[str, Any]) -> set[str]: def get_properties_by_agent(adagents_data: dict[str, Any], agent_url: str) -> list[dict[str, Any]]: """Get all properties authorized for a specific agent. + Handles all authorization types per the AdCP specification: + - inline_properties: Properties defined directly in the agent's properties array + - property_ids: Filter top-level properties by property_id + - property_tags: Filter top-level properties by tags + - publisher_properties: References properties from other publisher domains + (returns the selector objects, not resolved properties) + Args: adagents_data: Parsed adagents.json data agent_url: URL of the agent to filter by Returns: - List of properties for the specified agent (empty if agent not found or no properties) + List of properties for the specified agent (empty if agent not found) Raises: AdagentsValidationError: If adagents_data is malformed @@ -495,6 +502,11 @@ def get_properties_by_agent(adagents_data: dict[str, Any], agent_url: str) -> li if not isinstance(authorized_agents, list): raise AdagentsValidationError("adagents.json must have 'authorized_agents' array") + # Get top-level properties for reference-based authorization types + top_level_properties = adagents_data.get("properties", []) + if not isinstance(top_level_properties, list): + top_level_properties = [] + # Normalize the agent URL for comparison normalized_agent_url = normalize_url(agent_url) @@ -510,12 +522,44 @@ def get_properties_by_agent(adagents_data: dict[str, Any], agent_url: str) -> li if normalize_url(agent_url_from_json) != normalized_agent_url: continue - # Found the agent - return their properties - properties = agent.get("properties", []) - if not isinstance(properties, list): - return [] - - return [p for p in properties if isinstance(p, dict)] + # Found the agent - determine authorization type + authorization_type = agent.get("authorization_type", "") + + # Handle inline_properties (properties array directly on agent) + if authorization_type == "inline_properties" or "properties" in agent: + properties = agent.get("properties", []) + if not isinstance(properties, list): + return [] + return [p for p in properties if isinstance(p, dict)] + + # Handle property_ids (filter top-level properties by property_id) + if authorization_type == "property_ids": + authorized_ids = set(agent.get("property_ids", [])) + return [ + p + for p in top_level_properties + if isinstance(p, dict) and p.get("property_id") in authorized_ids + ] + + # Handle property_tags (filter top-level properties by tags) + if authorization_type == "property_tags": + authorized_tags = set(agent.get("property_tags", [])) + return [ + p + for p in top_level_properties + if isinstance(p, dict) and set(p.get("tags", [])) & authorized_tags + ] + + # Handle publisher_properties (cross-domain references) + # Returns the selector objects; caller must resolve against other domains + if authorization_type == "publisher_properties": + publisher_props = agent.get("publisher_properties", []) + if not isinstance(publisher_props, list): + return [] + return [p for p in publisher_props if isinstance(p, dict)] + + # No recognized authorization type - return empty + return [] return [] @@ -544,8 +588,8 @@ def __init__(self, properties: list[dict[str, Any]]): if not isinstance(prop, dict): continue - # Extract property ID - prop_id = prop.get("id") + # Extract property ID (per AdCP v2 schema, the field is "property_id") + prop_id = prop.get("property_id") if prop_id and isinstance(prop_id, str): self.property_ids.append(prop_id) diff --git a/tests/test_adagents.py b/tests/test_adagents.py index 4975a60e..30e4ba51 100644 --- a/tests/test_adagents.py +++ b/tests/test_adagents.py @@ -576,12 +576,14 @@ def test_get_all_tags_no_tags(self): class TestGetPropertiesByAgent: """Test getting properties for a specific agent.""" - def test_get_properties_by_agent(self): - """Should return properties for specified agent.""" + def test_get_properties_by_agent_inline_properties(self): + """Should return inline properties for agent with authorization_type=inline_properties.""" adagents_data = { "authorized_agents": [ { "url": "https://agent1.example.com", + "authorization_type": "inline_properties", + "authorized_for": "Test properties", "properties": [ { "property_type": "website", @@ -595,23 +597,184 @@ def test_get_properties_by_agent(self): }, ], }, + ] + } + + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") + assert len(properties) == 2 + assert properties[0]["name"] == "Site 1" + assert properties[1]["name"] == "App 1" + + def test_get_properties_by_agent_legacy_properties(self): + """Should return properties for agent without explicit authorization_type.""" + adagents_data = { + "authorized_agents": [ { - "url": "https://agent2.example.com", + "url": "https://agent1.example.com", "properties": [ { "property_type": "website", - "name": "Site 2", - "identifiers": [{"type": "domain", "value": "site2.com"}], - } + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "site1.com"}], + }, ], }, ] } + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") + assert len(properties) == 1 + assert properties[0]["name"] == "Site 1" + + def test_get_properties_by_agent_property_ids(self): + """Should filter top-level properties by property_id for authorization_type=property_ids.""" + adagents_data = { + "properties": [ + { + "property_id": "site1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "site1.com"}], + }, + { + "property_id": "site2", + "property_type": "website", + "name": "Site 2", + "identifiers": [{"type": "domain", "value": "site2.com"}], + }, + { + "property_id": "site3", + "property_type": "website", + "name": "Site 3", + "identifiers": [{"type": "domain", "value": "site3.com"}], + }, + ], + "authorized_agents": [ + { + "url": "https://agent1.example.com", + "authorization_type": "property_ids", + "authorized_for": "Selected properties", + "property_ids": ["site1", "site3"], + }, + ], + } + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") assert len(properties) == 2 assert properties[0]["name"] == "Site 1" - assert properties[1]["name"] == "App 1" + assert properties[1]["name"] == "Site 3" + + def test_get_properties_by_agent_property_tags(self): + """Should filter top-level properties by tags for authorization_type=property_tags.""" + adagents_data = { + "properties": [ + { + "property_id": "site1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "site1.com"}], + "tags": ["premium", "news"], + }, + { + "property_id": "site2", + "property_type": "website", + "name": "Site 2", + "identifiers": [{"type": "domain", "value": "site2.com"}], + "tags": ["sports"], + }, + { + "property_id": "site3", + "property_type": "website", + "name": "Site 3", + "identifiers": [{"type": "domain", "value": "site3.com"}], + "tags": ["premium", "entertainment"], + }, + ], + "authorized_agents": [ + { + "url": "https://agent1.example.com", + "authorization_type": "property_tags", + "authorized_for": "Premium properties", + "property_tags": ["premium"], + }, + ], + } + + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") + assert len(properties) == 2 + assert properties[0]["name"] == "Site 1" + assert properties[1]["name"] == "Site 3" + + def test_get_properties_by_agent_property_tags_multiple(self): + """Should match properties with any of the authorized tags.""" + adagents_data = { + "properties": [ + { + "property_id": "site1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "site1.com"}], + "tags": ["news"], + }, + { + "property_id": "site2", + "property_type": "website", + "name": "Site 2", + "identifiers": [{"type": "domain", "value": "site2.com"}], + "tags": ["sports"], + }, + { + "property_id": "site3", + "property_type": "website", + "name": "Site 3", + "identifiers": [{"type": "domain", "value": "site3.com"}], + "tags": ["entertainment"], + }, + ], + "authorized_agents": [ + { + "url": "https://agent1.example.com", + "authorization_type": "property_tags", + "authorized_for": "News and sports", + "property_tags": ["news", "sports"], + }, + ], + } + + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") + assert len(properties) == 2 + assert properties[0]["name"] == "Site 1" + assert properties[1]["name"] == "Site 2" + + def test_get_properties_by_agent_publisher_properties(self): + """Should return publisher_properties selectors for publisher_properties type.""" + adagents_data = { + "authorized_agents": [ + { + "url": "https://agent1.example.com", + "authorization_type": "publisher_properties", + "authorized_for": "Cross-domain properties", + "publisher_properties": [ + { + "publisher_domain": "cnn.com", + "selection_type": "by_tag", + "property_tags": ["ctv"], + }, + { + "publisher_domain": "espn.com", + "selection_type": "all", + }, + ], + }, + ], + } + + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") + assert len(properties) == 2 + assert properties[0]["publisher_domain"] == "cnn.com" + assert properties[0]["selection_type"] == "by_tag" + assert properties[1]["publisher_domain"] == "espn.com" + assert properties[1]["selection_type"] == "all" def test_get_properties_by_agent_protocol_agnostic(self): """Should match agent URL regardless of protocol.""" @@ -619,6 +782,8 @@ def test_get_properties_by_agent_protocol_agnostic(self): "authorized_agents": [ { "url": "https://agent1.example.com", + "authorization_type": "inline_properties", + "authorized_for": "Test", "properties": [ { "property_type": "website", @@ -640,6 +805,8 @@ def test_get_properties_by_agent_not_found(self): "authorized_agents": [ { "url": "https://agent1.example.com", + "authorization_type": "inline_properties", + "authorized_for": "Test", "properties": [ { "property_type": "website", @@ -654,21 +821,37 @@ def test_get_properties_by_agent_not_found(self): properties = get_properties_by_agent(adagents_data, "https://unknown-agent.com") assert len(properties) == 0 + def test_get_properties_by_agent_no_top_level_properties(self): + """Should return empty list when using property_ids/tags but no top-level props.""" + adagents_data = { + "authorized_agents": [ + { + "url": "https://agent1.example.com", + "authorization_type": "property_ids", + "authorized_for": "Test", + "property_ids": ["site1"], + }, + ], + } + + properties = get_properties_by_agent(adagents_data, "https://agent1.example.com") + assert len(properties) == 0 + class TestAuthorizationContext: """Test AuthorizationContext class.""" def test_extract_property_ids(self): - """Should extract property IDs from properties.""" + """Should extract property IDs from properties using property_id field.""" properties = [ { - "id": "prop1", + "property_id": "prop1", "property_type": "website", "name": "Site 1", "identifiers": [{"type": "domain", "value": "site1.com"}], }, { - "id": "prop2", + "property_id": "prop2", "property_type": "mobile_app", "name": "App 1", "identifiers": [{"type": "bundle_id", "value": "com.site1.app"}], @@ -682,13 +865,13 @@ def test_extract_property_tags(self): """Should extract unique tags from properties.""" properties = [ { - "id": "prop1", + "property_id": "prop1", "property_type": "website", "name": "Site 1", "tags": ["premium", "news"], }, { - "id": "prop2", + "property_id": "prop2", "property_type": "website", "name": "Site 2", "tags": ["premium", "sports"], @@ -702,11 +885,11 @@ def test_deduplicate_tags(self): """Should deduplicate tags.""" properties = [ { - "id": "prop1", + "property_id": "prop1", "tags": ["premium", "news"], }, { - "id": "prop2", + "property_id": "prop2", "tags": ["premium", "sports"], }, ] @@ -716,7 +899,7 @@ def test_deduplicate_tags(self): assert ctx.property_tags.count("premium") == 1 def test_handle_missing_fields(self): - """Should handle properties without ID or tags.""" + """Should handle properties without property_id or tags.""" properties = [ { "property_type": "website", @@ -732,7 +915,7 @@ def test_raw_properties_preserved(self): """Should preserve raw properties data.""" properties = [ { - "id": "prop1", + "property_id": "prop1", "property_type": "website", "name": "Site 1", "custom_field": "custom_value", @@ -747,7 +930,7 @@ def test_repr(self): """Should have useful string representation.""" properties = [ { - "id": "prop1", + "property_id": "prop1", "tags": ["premium"], } ] @@ -772,9 +955,11 @@ async def test_single_publisher_authorized(self): "authorized_agents": [ { "url": "https://our-agent.com", + "authorization_type": "inline_properties", + "authorized_for": "Test", "properties": [ { - "id": "prop1", + "property_id": "prop1", "property_type": "website", "name": "Site 1", "identifiers": [{"type": "domain", "value": "nytimes.com"}], @@ -807,9 +992,14 @@ async def test_multiple_publishers(self): "authorized_agents": [ { "url": "https://our-agent.com", + "authorization_type": "inline_properties", + "authorized_for": "Test", "properties": [ { - "id": "nyt_prop1", + "property_id": "nyt_prop1", + "property_type": "website", + "name": "NYT Site", + "identifiers": [{"type": "domain", "value": "nytimes.com"}], "tags": ["news"], } ], @@ -821,9 +1011,14 @@ async def test_multiple_publishers(self): "authorized_agents": [ { "url": "https://our-agent.com", + "authorization_type": "inline_properties", + "authorized_for": "Test", "properties": [ { - "id": "wsj_prop1", + "property_id": "wsj_prop1", + "property_type": "website", + "name": "WSJ Site", + "identifiers": [{"type": "domain", "value": "wsj.com"}], "tags": ["business"], } ], @@ -859,7 +1054,16 @@ async def test_skip_unauthorized_publishers(self): "authorized_agents": [ { "url": "https://our-agent.com", - "properties": [{"id": "prop1"}], + "authorization_type": "inline_properties", + "authorized_for": "Test", + "properties": [ + { + "property_id": "prop1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "nytimes.com"}], + } + ], } ] } @@ -869,7 +1073,16 @@ async def test_skip_unauthorized_publishers(self): "authorized_agents": [ { "url": "https://different-agent.com", - "properties": [{"id": "prop2"}], + "authorization_type": "inline_properties", + "authorized_for": "Test", + "properties": [ + { + "property_id": "prop2", + "property_type": "website", + "name": "Site 2", + "identifiers": [{"type": "domain", "value": "wsj.com"}], + } + ], } ] } @@ -903,7 +1116,16 @@ async def test_skip_missing_adagents_json(self): "authorized_agents": [ { "url": "https://our-agent.com", - "properties": [{"id": "prop1"}], + "authorization_type": "inline_properties", + "authorized_for": "Test", + "properties": [ + { + "property_id": "prop1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "nytimes.com"}], + } + ], } ] } @@ -937,7 +1159,16 @@ async def test_skip_invalid_adagents_json(self): "authorized_agents": [ { "url": "https://our-agent.com", - "properties": [{"id": "prop1"}], + "authorization_type": "inline_properties", + "authorized_for": "Test", + "properties": [ + { + "property_id": "prop1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "nytimes.com"}], + } + ], } ] } @@ -970,7 +1201,16 @@ async def test_empty_result_when_no_authorizations(self): "authorized_agents": [ { "url": "https://different-agent.com", - "properties": [{"id": "prop1"}], + "authorization_type": "inline_properties", + "authorized_for": "Test", + "properties": [ + { + "property_id": "prop1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "example.com"}], + } + ], } ] } @@ -995,7 +1235,16 @@ async def test_uses_provided_http_client(self): "authorized_agents": [ { "url": "https://our-agent.com", - "properties": [{"id": "prop1"}], + "authorization_type": "inline_properties", + "authorized_for": "Test", + "properties": [ + { + "property_id": "prop1", + "property_type": "website", + "name": "Site 1", + "identifiers": [{"type": "domain", "value": "nytimes.com"}], + } + ], } ] }