fix: Make get_versioned_release_schema more robust to bad extensions

open-contracting · Dec 17, 2024 · 628d0cb · 628d0cb
1 parent 8d643fe
commit 628d0cb
Show file tree

Hide file tree

Showing 7 changed files with 208 additions and 41 deletions.
diff --git a/docs/changelog.rst b/docs/changelog.rst
@@ -1,6 +1,11 @@
 Changelog
 =========
 
+0.6.8 (2024-12-17)
+------------------
+
+-  :meth:`~ocdsextensionregistry.versioned_release_schema.get_versioned_release_schema`: Handle extensions that set ``items`` to an array or omit ``$ref`` or ``items`` where these are expected.
+
 0.6.7 (2024-12-15)
 ------------------
 

diff --git a/docs/conf.py b/docs/conf.py
@@ -23,7 +23,7 @@
 author = "Open Contracting Partnership"
 
 # The short X.Y version
-version = "0.6.7"
+version = "0.6.8"
 # The full version, including alpha/beta/rc tags
 release = version
 

diff --git a/ocdsextensionregistry/exceptions.py b/ocdsextensionregistry/exceptions.py
@@ -61,7 +61,11 @@ def __str__(self):
         return f"{self.extension}({self.codelist}): {cls.__module__}.{cls.__name__}: {self.exc}"
 
 
-class VersionedReleaseTypeWarning(OCDSExtensionRegistryWarning):
+class VersionedReleaseWarning(OCDSExtensionRegistryWarning):
+    """Base class for warnings while creating a versioned release."""
+
+
+class VersionedReleaseTypeWarning(VersionedReleaseWarning):
     """Used when a type is unexpected or unrecognized while creating a versioned release."""
 
     def __init__(self, pointer, types, schema):
@@ -71,3 +75,25 @@ def __init__(self, pointer, types, schema):
 
     def __str__(self):
         return f"{self.pointer} has unrecognized type {self.types}"
+
+
+class VersionedReleaseRefWarning(VersionedReleaseWarning):
+    """Used when a subschema has no ``type`` or ``$ref``, while creating a versioned release."""
+
+    def __init__(self, pointer, schema):
+        self.pointer = pointer
+        self.schema = schema
+
+    def __str__(self):
+        return f"{self.pointer} has no type and no $ref"
+
+
+class VersionedReleaseItemsWarning(VersionedReleaseWarning):
+    """Used when an array has no ``items`` or ``items`` is an array, while creating a versioned release."""
+
+    def __init__(self, pointer, schema):
+        self.pointer = pointer
+        self.schema = schema
+
+    def __str__(self):
+        return f"{self.pointer}/items is not set or is an array"
diff --git a/ocdsextensionregistry/versioned_release_schema.py b/ocdsextensionregistry/versioned_release_schema.py
@@ -3,7 +3,11 @@
 
 import jsonref
 
-from ocdsextensionregistry.exceptions import VersionedReleaseTypeWarning
+from ocdsextensionregistry.exceptions import (
+    VersionedReleaseItemsWarning,
+    VersionedReleaseRefWarning,
+    VersionedReleaseTypeWarning,
+)
 
 _VERSIONED_TEMPLATE = {
   "type": "array",
@@ -132,18 +136,19 @@ def _get_unversioned_pointers(schema, fields, pointer=""):
         # If an array is whole list merge, its items are unversioned.
         if "array" in types and schema.get("wholeListMerge"):
             return
-        if "array" in types and "items" in schema:
-            item_types = _cast_as_list(schema["items"].get("type", []))
-            # If an array mixes objects and non-objects, it is whole list merge.
-            if any(item_type != "object" for item_type in item_types):
-                return
-            # If it is an array of objects, any `id` fields are unversioned.
-            if "id" in schema["items"]["properties"]:
-                if hasattr(schema["items"], "__reference__"):
-                    reference = schema["items"].__reference__["$ref"][1:]
-                else:
-                    reference = pointer
-                fields.add(f"{reference}/properties/id")
+        if "array" in types and (items := schema.get("items")):
+            if isinstance(items, dict):
+                item_types = _cast_as_list(items.get("type", []))
+                # If an array mixes objects and non-objects, it is whole list merge.
+                if any(item_type != "object" for item_type in item_types):
+                    return
+                # If it is an array of objects, any `id` fields are unversioned.
+                if "id" in items["properties"]:
+                    reference = items.__reference__["$ref"][1:] if hasattr(items, "__reference__") else pointer
+                    fields.add(f"{reference}/properties/id")
+            # This should only occur in tests.
+            else:
+                warnings.warn(VersionedReleaseItemsWarning(pointer, schema), stacklevel=2)
 
         for key, value in schema.items():
             _get_unversioned_pointers(value, fields, pointer=f"{pointer}/{key}")
@@ -193,10 +198,14 @@ def _add_versioned_field(schema, unversioned_pointers, pointer, key, value):
     if not types:
         # Ignore the `amendment` field, which had no `id` field in OCDS 1.0.
         if "deprecated" not in value:
-            versioned_pointer = f"{value['$ref'][1:]}/properties/id"
-            # If the `id` field is on an object not in an array, it needs to be versioned (e.g. buyer/properties/id).
-            if versioned_pointer in unversioned_pointers:
-                value["$ref"] = value["$ref"] + "VersionedId"
+            if "$ref" in value:
+                versioned_pointer = f"{value['$ref'][1:]}/properties/id"
+                # If the `id` field is on an object not in an array, it needs to be versioned (like on `buyer`).
+                if versioned_pointer in unversioned_pointers:
+                    value["$ref"] = value["$ref"] + "VersionedId"
+            # This should only occur in tests.
+            else:
+                warnings.warn(VersionedReleaseRefWarning(pointer, value), stacklevel=2)
         return
 
     # Reference a common versioned definition if possible, to limit the size of the schema.
@@ -213,26 +222,30 @@ def _add_versioned_field(schema, unversioned_pointers, pointer, key, value):
         new_value = deepcopy(value)
 
         if types == ["array"]:
-            item_types = _cast_as_list(value["items"].get("type", []))
-
-            # See https://standard.open-contracting.org/latest/en/schema/merging/#whole-list-merge
-            if value.get("wholeListMerge"):
-                # Update `$ref` to the unversioned definition.
-                if "$ref" in value["items"]:
-                    new_value["items"]["$ref"] = value["items"]["$ref"] + "Unversioned"
-                # Otherwise, similarly, don't iterate over item properties.
-            # See https://standard.open-contracting.org/latest/en/schema/merging/#lists
-            elif "$ref" in value["items"]:
-                # Leave `$ref` to the versioned definition.
-                return
-            # Exceptional case for deprecated `Amendment.changes`.
-            elif item_types == ["object"] and pointer == "/definitions/Amendment/properties/changes":
-                return
-            # Warn in case new combinations are added to the release schema.
-            elif item_types != ["string"]:
-                # Note: Versioning the properties of un-$ref'erenced objects in arrays isn't implemented. However,
-                # this combination hasn't occurred, with the exception of `Amendment/changes`.
-                warnings.warn(VersionedReleaseTypeWarning(f"{pointer}/items", item_types, value), stacklevel=2)
+            if (items := value.get("items")) and isinstance(items, dict):
+                item_types = _cast_as_list(items.get("type", []))
+
+                # See https://standard.open-contracting.org/latest/en/schema/merging/#whole-list-merge
+                if value.get("wholeListMerge"):
+                    # Update `$ref` to the unversioned definition.
+                    if "$ref" in items:
+                        new_value["items"]["$ref"] = items["$ref"] + "Unversioned"
+                    # Otherwise, similarly, don't iterate over item properties.
+                # See https://standard.open-contracting.org/latest/en/schema/merging/#lists
+                elif "$ref" in items:
+                    # Leave `$ref` to the versioned definition.
+                    return
+                # Exceptional case for deprecated `Amendment.changes`.
+                elif item_types == ["object"] and pointer == "/definitions/Amendment/properties/changes":
+                    return
+                # Warn in case new combinations are added to the release schema.
+                elif item_types != ["string"]:
+                    # Note: Versioning the properties of un-$ref'erenced objects in arrays isn't implemented. However,
+                    # this combination hasn't occurred, with the exception of `Amendment/changes`.
+                    warnings.warn(VersionedReleaseTypeWarning(f"{pointer}/items", item_types, value), stacklevel=2)
+            # This should only occur in tests.
+            else:
+                warnings.warn(VersionedReleaseItemsWarning(pointer, value), stacklevel=2)
 
         versioned = deepcopy(_VERSIONED_TEMPLATE)
         versioned["items"]["properties"]["value"] = new_value

diff --git a/pyproject.toml b/pyproject.toml
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ocdsextensionregistry"
-version = "0.6.7"
+version = "0.6.8"
 authors = [{name = "Open Contracting Partnership", email = "[email protected]"}]
 description = "Eases access to information from the extension registry of the Open Contracting Data Standard"
 readme = "README.rst"

diff --git a/tests/fixtures/schema-items-array.json b/tests/fixtures/schema-items-array.json
@@ -0,0 +1,106 @@
+{
+  "properties": {
+    "json_schema_example_fields": {
+      "type": "object",
+      "properties": {
+        "minimum": {
+          "minimum": 2
+        },
+        "minimum2": {
+          "exclusiveMinimum": true,
+          "minimum": 2
+        },
+        "maximum": {
+          "maximum": 2
+        },
+        "maximum2": {
+          "exclusiveMaximum": true,
+          "maximum": 2
+        },
+        "minItems": {
+          "type": "array",
+          "minItems": 2
+        },
+        "maxItems": {
+          "type": "array",
+          "maxItems": 2
+        },
+        "minLength": {
+          "type": "string",
+          "minLength": 2
+        },
+        "maxLength": {
+          "type": "string",
+          "maxLength": 2
+        },
+        "maxProperties": {
+          "type": "object",
+          "maxProperties": 2
+        },
+        "multipleOf": {
+          "type": "number",
+          "multipleOf": 3
+        },
+        "not": {
+          "not": {
+            "type": "string"
+          }
+        },
+        "anyOf": {
+          "anyOf": [
+            {"type": "array"},
+            {"type": "object"}
+          ]
+        },
+        "allOf": {
+          "anyOf": [
+            {"type": "array"},
+            {"type": "object"}
+          ]
+        },
+        "oneOf": {
+          "oneOf": [
+            {"type": "array"},
+            {"type": "object"}
+          ]
+        },
+        "oneOf2": {
+          "oneOf": [
+            {"type": "number"},
+            {"type": "integer"}
+          ]
+        },
+        "additionalItems": {
+          "type": "array",
+          "items": [{
+            "type": "string"
+          }],
+          "additionalItems": false
+        },
+        "additionalProperties": {
+          "type": "object",
+          "additionalProperties": false
+        },
+        "additionalProperties2": {
+          "type": "object",
+          "patternProperties": {
+            "okay": {
+              "type": "string"
+            }
+          },
+          "additionalProperties": false
+        },
+        "dependencies": {
+          "type": "object",
+          "dependencies": {
+            "b": ["a"]
+          }
+        },
+        "format": {
+          "type": "string",
+          "format": "email"
+        }
+      }
+    }
+  }
+}
diff --git a/tests/test_versioned_release_schema.py b/tests/test_versioned_release_schema.py
@@ -1,10 +1,27 @@
 import json
 
+import json_merge_patch
+import pytest
+
 from ocdsextensionregistry import get_versioned_release_schema
+from ocdsextensionregistry.exceptions import VersionedReleaseWarning
 from tests import read
 
 
 def test_get_versioned_release_schema():
+    schema = json.loads(read('release-schema.json'))
+
+    actual = get_versioned_release_schema(schema, '1__1__5')
+
+    assert actual == json.loads(read('versioned-release-validation-schema.json'))
+
+
+def test_items_array():
     schema = get_versioned_release_schema(json.loads(read('release-schema.json')), '1__1__5')
 
-    assert schema == json.loads(read('versioned-release-validation-schema.json'))
+    json_merge_patch.merge(schema, json.loads(read('schema-items-array.json')))
+
+    with pytest.warns(VersionedReleaseWarning) as records:
+        get_versioned_release_schema(schema, '1__1__5')
+
+    assert len(records) == 62