fix(sdk): correct discovery and response negotiation

Author: nkavian

README.md

@@ -341,10 +341,19 @@ its Actions can advertise enrollment, payment, and trust protocols, but ODP does
 credentials, invoke Actions, submit payments, or implement trust protocols. Applications compose
 the appropriate protocol clients around an Action resolved through ODP.
 
-`parse_service_document` is the strict current-version Service parser. Agent inspection and
+`parse_service_document` validates Service metadata against the supported ODP major version.
+Compatible minor versions such as `1.7` are accepted without rewriting the received version;
+SDK-generated documents use `1.0`. Agent inspection and
 Directory results filter unrecognized enrollment, payment, and trust descriptors while retaining
 strict validation for recognized descriptors.
 
+Individual Offering and Collection GETs default to full representations; list and search operations
+default to terse items. A Catalog receives the requested language in `CatalogRequest.language` and
+declares the language it actually returns on each resource. Static catalogs do not translate content.
+
+Refinement parsing detects duplicate JSON values without guessing the type of a string. Comparing
+decimal or date-time strings by their meaning requires the referenced Filter Definition.
+
 ## Errors and validation
 
 Each role exposes typed errors:

scripts/conformance_adapter.py

@@ -242,6 +242,9 @@ async def evaluate_errors_limits(case: dict[str, Any]) -> bool | None:
 
 
 async def evaluate_case(subject: str, case: dict[str, Any], role: str) -> bool | None:
+    if subject == "protocol-version":
+        document = {"odp_version": case["received"], "id": "item", "name": "Item"}
+        return succeeds(lambda: parse_offering(json.dumps(document))) == case["compatible"]
     if subject == "local-identifier":
         return is_local_resource_identifier(case["value"]) == case["valid"]
     if subject == "identity-comparison":

src/offering_protocol/agent/client.py

@@ -189,10 +189,7 @@ async def list_collections(
         self, representation: Representation = Representation.TERSE, limit: int = 0
     ) -> Page[Collection]:
         body = await self._get_page(Operation.LIST_COLLECTIONS, None, representation, limit)
-        page = parse_collection_page(body)
-        for item in page.items:
-            parse_collection(_encode(item))
-        return page
+        return parse_collection_page(body)
 
     async def get_collection(self, identifier: str) -> Collection:
         body = await self._get_page(Operation.GET_COLLECTION, identifier, Representation.FULL, 0)
@@ -206,10 +203,7 @@ async def search_collections(
         body = await self._post_search(
             Operation.SEARCH_COLLECTIONS, request.to_dict(), representation
         )
-        page = parse_collection_page(body)
-        for item in page.items:
-            parse_collection(_encode(item))
-        return page
+        return parse_collection_page(body)
 
     async def list_offerings(
         self, representation: Representation = Representation.TERSE, limit: int = 0
@@ -498,7 +492,11 @@ async def _supporting_json(
                 conditional["if-none-match"] = cached.etag
             if cached.last_modified:
                 conditional["if-modified-since"] = cached.last_modified
+        visited: set[str] = set()
         for redirects in range(_MAXIMUM_REDIRECTS + 1):
+            if current in visited:
+                raise AgentError("ODP supporting document contains a redirect loop")
+            visited.add(current)
             try:
                 response = await self._supporting_transport.send(
                     HttpRequest("GET", current, {"accept": accept, **conditional})
@@ -511,9 +509,14 @@ async def _supporting_json(
                 location = response.headers.get("location")
                 if location is None:
                     raise AgentError("ODP supporting document redirect omitted Location")
-                current = urljoin(current, location)
-                if not _is_https_url(current):
+                target_url = urljoin(current, location)
+                if not _is_https_url(target_url):
                     raise AgentError("ODP supporting document redirect must use HTTPS")
+                if derive_service_origin(target_url) != derive_service_origin(current):
+                    raise AgentError(
+                        "ODP supporting document redirect must remain on the same origin"
+                    )
+                current = target_url
                 continue
             if response.status == 304:
                 if cached is None:
@@ -612,13 +615,6 @@ def parse_problem_response(data: bytes | str, status: int) -> ProblemDetails:
     return parse_problem_response_strict(_agent_body(data, "problem"), status)
 
 
-def _encode(value: object) -> bytes:
-    if not hasattr(value, "model_dump_json"):
-        raise TypeError("ODP model is not serializable")
-    encoded = value.model_dump_json(by_alias=True, exclude_unset=True)
-    return cast(str, encoded).encode()
-
-
 def _append_query(target: str, values: dict[str, str]) -> str:
     parts = urlsplit(target)
     query = dict(parse_qsl(parts.query, keep_blank_values=True))

src/offering_protocol/core/validation.py

@@ -5,6 +5,7 @@
 import json
 import re
 from dataclasses import dataclass, field
+from decimal import Decimal
 from functools import lru_cache
 from importlib.resources import files
 from typing import Any, TypeVar
@@ -17,6 +18,7 @@
 from referencing import Registry, Resource
 
 from offering_protocol.core.models import (
+    VERSION,
     Collection,
     CollectionSearchRequest,
     FilterDefinition,
@@ -715,7 +717,13 @@ def validate_value(value: object, schema_name: str, document_type: str) -> None:
     validator = _validators().get(schema_name)
     if validator is None:
         raise RuntimeError(f"missing bundled schema {schema_name}")
-    issues = [_schema_issue(error) for error in validator.iter_errors(value)]
+    validation_value = value
+    if isinstance(value, dict):
+        version = value.get("odp_version")
+        if isinstance(version, str) and re.fullmatch(r"1\.(0|[1-9][0-9]*)", version):
+            # The bundled schemas describe 1.0; compatible minor versions use the same rules.
+            validation_value = {**value, "odp_version": VERSION}
+    issues = [_schema_issue(error) for error in validator.iter_errors(validation_value)]
     if issues:
         issues.sort(key=lambda issue: (issue.path, issue.keyword, issue.message))
         raise OdpValidationError(document_type, issues)
@@ -889,9 +897,6 @@ def _is_language_tag(value: str) -> bool:
     return not in_extension or len(subtags[-1]) > 1
 
 
-_DECIMAL = re.compile(r"^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?$")
-
-
 def _compare_decimals(left: str, right: str) -> int:
     """Orders two ODP monetary values, which are decimal strings rather than JSON numbers (OFR-48).
 
@@ -916,25 +921,15 @@ def _split_decimal(value: str) -> tuple[str, str]:
     return whole.lstrip("0"), fraction.rstrip("0")
 
 
-def _bucket_key(value: object) -> str:
-    """Compares two bucket values the way the referenced Filter Definition would.
-
-    A response does not carry its Filter Definitions, so the type behind a JSON string -- `string`,
-    `decimal`, `date` or `date-time` -- is not known here. Every one of those compares two strings
-    exactly except `decimal`, whose equality is numeric, so a string that can only be a decimal is
-    reduced to one spelling per value. JSON numbers compare numerically.
-    """
+def _bucket_key(value: object) -> tuple[str, object]:
+    """Check structural duplicates without guessing the type of a string-valued Filter."""
     if isinstance(value, bool):
-        return f"b{value}"
+        return "boolean", value
     if isinstance(value, int | float):
-        return f"n{float(value)}"
+        return "number", Decimal(str(value))
     if isinstance(value, str):
-        if _DECIMAL.match(value):
-            whole, fraction = _split_decimal(value.removeprefix("-"))
-            sign = "-" if value.startswith("-") and (whole or fraction) else ""
-            return f"d{sign}{whole}.{fraction}"
-        return f"s{value}"
-    return f"o{json.dumps(value, separators=(',', ':'), sort_keys=True)}"
+        return "string", value
+    return "other", json.dumps(value, separators=(",", ":"), sort_keys=True)
 
 
 def _agent_body(data: bytes | str, kind: str) -> str:

src/offering_protocol/directory/client.py

@@ -248,11 +248,11 @@ def _parse_search_page(body: bytes) -> SearchPage:
         raise DirectoryError("invalid Directory response: search page has no items")
     if len(raw["items"]) > 100:
         raise DirectoryError("Directory search page exceeds 100 Services")
-    items: list[dict[str, object]] = []
+    items: list[DirectoryService] = []
     issues: list[ServiceIssue] = []
     for index, entry in enumerate(raw["items"]):
         try:
-            items.append(_read_service(entry))
+            items.append(DirectoryService.model_validate(_read_service(entry)))
         except (ModelValidationError, OdpValidationError, ReferenceError, DirectoryError) as error:
             issues.append(ServiceIssue(index=index, message=str(error)))
     try:
@@ -292,6 +292,7 @@ def _read_service(entry: object) -> dict[str, object]:
         item.pop(member, None)
     if document.protocols is not None:
         item["protocols"] = document.protocols.model_dump(mode="json", exclude_defaults=True)
+    item["operations"] = [operation.model_dump(mode="json") for operation in document.operations]
     return item
 
 

src/offering_protocol/service/service.py

@@ -281,7 +281,7 @@ async def _handle(self, request: Request) -> Response:
             self._document.language,
             list(self._document.localizations),
         )
-        exchange = _Exchange(headers=headers, language=language, method=method)
+        exchange = _Exchange(headers=headers, language=self._document.language, method=method)
         if request.path == "/.well-known/odp":
             _require_method(effective, ("GET",))
             return _json_response(self._document, _MAXIMUM_DOCUMENT_BYTES, exchange)
@@ -295,7 +295,7 @@ async def _handle(self, request: Request) -> Response:
             item.name for item in self._document.operations
         }:
             raise RequestError(404, "NOT_FOUND", "ODP operation is not supported")
-        catalog_request = _catalog_request(request, headers, language)
+        catalog_request = _catalog_request(request, headers, language, operation)
         if (effective, path) == ("GET", "/offerings"):
             offering_page = await self._catalog.list_offerings(catalog_request)
             return _json_response(
@@ -370,7 +370,9 @@ async def _get_path(self, path: str, request: CatalogRequest, exchange: _Exchang
         raise RequestError(404, "NOT_FOUND", "ODP resource not found")
 
 
-def _catalog_request(request: Request, headers: dict[str, str], language: str) -> CatalogRequest:
+def _catalog_request(
+    request: Request, headers: dict[str, str], language: str, operation: Operation | None
+) -> CatalogRequest:
     parameters = parse_qsl(request.query, keep_blank_values=True)
     # SVC-73: a repeated `representation` is rejected rather than resolved. Collapsing repeats into
     # a dict silently honoured whichever copy came last, so `representation=terse&representation=
@@ -381,7 +383,10 @@ def _catalog_request(request: Request, headers: dict[str, str], language: str) -
             raise RequestError(400, "INVALID_REQUEST", f"{name} must not be repeated")
         values[name] = value
     try:
-        representation = Representation(values.get("representation", "terse"))
+        default = (
+            "full" if operation in {Operation.GET_COLLECTION, Operation.GET_OFFERING} else "terse"
+        )
+        representation = Representation(values.get("representation", default))
         limit = int(values.get("limit", "0"))
     except ValueError as error:
         raise RequestError(400, "INVALID_REQUEST", "query parameter is invalid") from error
@@ -434,9 +439,20 @@ def _json_response(value: object, maximum_bytes: int, exchange: _Exchange) -> Re
     body = _encode(value)
     if len(body) > maximum_bytes:
         raise ServiceError("response body is too large")
-    etag = _entity_tag(exchange.language, body)
+    if isinstance(value, Page):
+        language = (
+            ", ".join(
+                dict.fromkeys(
+                    getattr(item, "language", "") or exchange.language for item in value.items
+                )
+            )
+            or exchange.language
+        )
+    else:
+        language = getattr(value, "language", "") or exchange.language
+    etag = _entity_tag(language, body)
     headers = {
-        "content-language": exchange.language,
+        "content-language": language,
         "content-type": MEDIA_TYPE,
         "etag": etag,
         "vary": "Accept, Accept-Language",
@@ -554,12 +570,20 @@ def _require_accept(value: str | None) -> None:
     """
     if value is None:
         return
+    specificity = -1
+    quality = 0.0
     for entry in value.split(","):
         media_type = entry.split(";", 1)[0].strip().lower()
         if media_type not in {"*/*", "application/*", MEDIA_TYPE}:
             continue
-        if _quality_of(entry) > 0:
-            return
+        precision = {"*/*": 0, "application/*": 1, MEDIA_TYPE: 2}[media_type]
+        weight = _quality_of(entry)
+        if precision > specificity:
+            specificity, quality = precision, weight
+        elif precision == specificity:
+            quality = max(quality, weight)
+    if quality > 0:
+        return
     raise RequestError(406, "NOT_ACCEPTABLE", f"Accept must allow {MEDIA_TYPE}")
 
 

tests/test_agent.py

@@ -27,7 +27,6 @@
 )
 from offering_protocol.agent.client import (
     _decode_json_object,
-    _encode,
     _expiration,
     _invoke_parser,
 )
@@ -665,8 +664,6 @@ async def test_agent_remaining_traversal_and_supporting_document_edges() -> None
     assert not (await client.list_collection_offerings("plants", limit=1)).items
     assert not await client.search_all_offerings(OfferingSearchRequest(query="plant"))
 
-    with pytest.raises(TypeError):
-        _encode({"not": "a model"})
     with pytest.raises(AgentError):
         _invoke_parser(lambda _: (_ for _ in ()).throw(ValueError("bad")), b"{}")
 

tests/test_agent_conformance.py

@@ -33,6 +33,7 @@
 )
 from offering_protocol.core import (
     CapabilityLink,
+    CollectionSearchRequest,
     FilterCapabilitySource,
     FilterDefinition,
     FilterOperator,
@@ -534,3 +535,75 @@ def exhausted(*args: object, **kwargs: object) -> object:
 
     with pytest.raises(AgentError, match="nested too deeply"):
         _decode_json_object(b"[[[]]]")
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("search", [False, True])
+async def test_collection_page_inherits_version_without_requiring_it_on_items(search: bool) -> None:
+    document = json.loads(SERVICE_DOCUMENT)
+    operation = "search-collections" if search else "list-collections"
+    document["operations"].append({"name": operation, "authentication": "not-required"})
+    transport = QueueTransport(
+        response(json.dumps(document)),
+        response('{"odp_version":"1.7","items":[{"id":"plants","name":"Plants"}]}'),
+    )
+    client = ServiceClient("https://store.example", transport=transport)
+    page = (
+        await client.search_collections(CollectionSearchRequest())
+        if search
+        else await client.list_collections()
+    )
+    assert page.odp_version == "1.7"
+    assert page.items[0].id == "plants"
+    assert "odp_version" not in page.items[0].model_fields_set
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "location",
+    [
+        "https://other.example/schema.json",
+        "https://schemas.example:444/schema.json",
+        "/schema.json",
+    ],
+)
+async def test_supporting_redirects_refuse_origin_changes_and_loops(location: str) -> None:
+    offering = {
+        "odp_version": "1.0",
+        "id": "item",
+        "name": "Item",
+        "schema": {"url": "https://schemas.example/schema.json"},
+        "attributes": {"colour": "green"},
+    }
+    supporting = QueueTransport(response("", status=302, headers={"location": location}))
+    client = ServiceClient(
+        "https://store.example",
+        transport=QueueTransport(response(SERVICE_DOCUMENT), response(json.dumps(offering))),
+        supporting_transport=supporting,
+    )
+    details = await client.get_offering_details("item")
+    assert len(supporting.requests) == 1
+    assert len(details.issues) == 1
+    assert "redirect" in details.issues[0].message
+    assert details.offering.name == "Item"
+    assert not details.offering.attributes
+
+
+@pytest.mark.asyncio
+async def test_supporting_redirects_allow_explicit_default_port() -> None:
+    supporting = QueueTransport(
+        response("", status=302, headers={"location": "https://schemas.example:443/final.json"}),
+        response("{}", content_type="application/json"),
+    )
+    client = ServiceClient("https://store.example", supporting_transport=supporting)
+    assert (
+        await client._supporting_json(
+            "https://schemas.example/start.json",
+            "schema",
+            "application/json",
+            {"application/json"},
+            100,
+        )
+        == {}
+    )
+    assert len(supporting.requests) == 2