diff --git a/src/labthings_fastapi/thing_description/__init__.py b/src/labthings_fastapi/thing_description/__init__.py index f0b6af9d..dc7d1d40 100644 --- a/src/labthings_fastapi/thing_description/__init__.py +++ b/src/labthings_fastapi/thing_description/__init__.py @@ -80,37 +80,6 @@ def look_up_reference(reference: str, d: JSONSchema) -> JSONSchema: ) from ke -def is_an_object(d: JSONSchema) -> bool: - """Determine whether a JSON schema dict is an object. - - :param d: a chunk of JSONSchema describing a datatype. - - :return: ``True`` if the ``type`` is ``object``. - """ - return "type" in d and d["type"] == "object" - - -def convert_object(d: JSONSchema) -> JSONSchema: - """Convert an object from JSONSchema to Thing Description. - - Convert JSONSchema objects to Thing Description datatypes. - - Currently, this deletes the ``additionalProperties`` keyword, which is - not supported by Thing Description. - - :param d: the JSONSchema object. - - :return: a copy of ``d``, with ``additionalProperties`` deleted. - """ - out: JSONSchema = d.copy() - # AdditionalProperties is not supported by Thing Description, and it is ambiguous - # whether this implies it's false or absent. I will, for now, ignore it, so we - # delete the key below. - if "additionalProperties" in out: - del out["additionalProperties"] - return out - - def convert_anyof(d: JSONSchema) -> JSONSchema: """Convert the anyof key to oneof. @@ -133,18 +102,27 @@ def convert_anyof(d: JSONSchema) -> JSONSchema: def convert_prefixitems(d: JSONSchema) -> JSONSchema: - """Convert the prefixitems key to items. + r"""Convert the prefixitems key to items. + + The ``items`` key in an ArraySchema_ may work in two ways: - JSONSchema 2019 (as used by thing description) used - `items` with a list of values in the same way that JSONSchema - now uses `prefixitems`. + * If it is a `DataSchema` instance, then it specifies the schema for + all items in the array. This would apply to ``list[int]`` for example. + * If it is a sequence of `DataSchema` instances, then we are describing + a `tuple` with a defined number of elements, each with a defined type, + for example `tuple[int, str]`\ . - JSONSchema 2020 uses `items` to mean the same as `additionalItems` - in JSONSchema 2019 - but Thing Description doesn't support the - `additionalItems` keyword. This will result in us overwriting - additional items, and we raise a ValueError if that happens. + JSONSchema 2020 uses `items` to mean the type of subsequent items (i.e. + ``list[...]``), so it's only ever a single type. When describing a `tuple` + it will use ``prefixItems`` to give the type of each element. - This behaviour may be relaxed in the future. + JSONSchema 2019 (on which ArraySchema_ is based) has a different definition + again, using ``items`` and ``additionalItems``\ . + + We are converting from JSONSchema 2020 (which is what `pydantic` generates) + to ArraySchema_ so we will convert ``prefixItems`` into ``items``\ . If + both keys are present, we will raise an error, as that's not currently + supported. :param d: the JSONSchema object. @@ -152,6 +130,8 @@ def convert_prefixitems(d: JSONSchema) -> JSONSchema: :raise KeyError: if we would overwrite an existing ``items`` key. + + .. _ArraySchema: https://www.w3.org/TR/wot-thing-description/#arrayschema """ if "prefixItems" not in d: return d @@ -163,39 +143,18 @@ def convert_prefixitems(d: JSONSchema) -> JSONSchema: return out -def convert_additionalproperties(d: JSONSchema) -> JSONSchema: - r"""Move additionalProperties into properties, or remove it. - - JSONSchema uses ``additionalProperties`` to define optional properties - of ``object``\ s. For Thing Descriptions, this should be moved inside - the ``properties`` object. - - :param d: the JSONSchema object. - - :return: a copy of ``d``, with ``additionalProperties`` moved into - ``properties`` or deleted if ``properties`` is not present. - """ - if "additionalProperties" not in d: - return d - out: JSONSchema = d.copy() - if "properties" in out and "additionalProperties" not in out["properties"]: - out["properties"]["additionalProperties"] = out["additionalProperties"] - del out["additionalProperties"] - return out - - def check_recursion(depth: int, limit: int) -> None: """Check the recursion count is less than the limit. :param depth: the current recursion depth. :param limit: the maximum recursion depth. - :raise ValueError: if we exceed the recursion depth. + :raise RecursionError: if we exceed the recursion depth. """ if depth > limit: - raise ValueError( + raise RecursionError( f"Recursion depth of {limit} exceeded - perhaps there is a circular " - "reference?" + "reference in a `pydantic` model?" ) @@ -250,11 +209,8 @@ def jsonschema_to_dataschema( recursion_depth += 1 check_recursion(recursion_depth, recursion_limit) - if is_an_object(d): - d = convert_object(d) d = convert_anyof(d) d = convert_prefixitems(d) - d = convert_additionalproperties(d) # After checking the object isn't a reference, we now recursively check # sub-dictionaries and dereference those if necessary. This could be done with a diff --git a/src/labthings_fastapi/thing_description/_model.py b/src/labthings_fastapi/thing_description/_model.py index f65e6d26..2bf175f0 100644 --- a/src/labthings_fastapi/thing_description/_model.py +++ b/src/labthings_fastapi/thing_description/_model.py @@ -180,7 +180,7 @@ class DataSchema(BaseModel): contentEncoding: Optional[str] = None contentMediaType: Optional[str] = None - model_config = ConfigDict(extra="forbid") + model_config = ConfigDict(extra="allow") class Response(BaseModel): diff --git a/tests/test_td_dataschema_generation.py b/tests/test_td_dataschema_generation.py index 6524dc8d..595ee9f4 100644 --- a/tests/test_td_dataschema_generation.py +++ b/tests/test_td_dataschema_generation.py @@ -1,103 +1,194 @@ from __future__ import annotations import json -from typing import Optional +from typing import Annotated, Any, Literal -from pydantic import BaseModel +import pytest +from pydantic import BaseModel, WithJsonSchema +from pydantic_core import ValidationError -from labthings_fastapi.thing_description import type_to_dataschema +from labthings_fastapi.thing_description import convert_prefixitems, type_to_dataschema from labthings_fastapi.thing_description._model import DataSchema -def ds_json_dict(ds: DataSchema) -> dict: - """Serialise a DataSchema to json and reinflate to a dict - - This removes complicated types so we end up with a simple - representation that's slightly more abstracted from the - implementation details - e.g. enums are reduced to their - simple types. This is appropriate if we're testing against - what we'd expect to see in the API. - """ - return json.loads(ds.model_dump_json()) - - -def test_int(): - ds = type_to_dataschema(int) - j = ds_json_dict(ds) - assert j["type"] == "integer" - +class A(BaseModel): + """A model used to check the schema generates OK.""" -def test_float(): - ds = type_to_dataschema(float) - j = ds_json_dict(ds) - assert j["type"] == "number" + a: int + b: int | None = None + """Note that `b` is optional.""" -def test_str(): - ds = type_to_dataschema(str) - j = ds_json_dict(ds) - assert j["type"] == "string" +# This is what the generated schema should look like for the class above. +DATASCHEMA_FOR_A = { + "description": "A model used to check the schema generates OK.", + "type": "object", + "properties": { + "a": {"title": "A", "type": "integer"}, + "b": { + "title": "B", + "oneOf": [{"type": "integer"}, {"type": "null"}], + }, + }, + "required": ["a"], # Note: "b" is optional, so should not be listed. + "title": "A", +} -def test_none(): - ds = type_to_dataschema(type(None)) - j = ds_json_dict(ds) - assert j["type"] == "null" +class B(BaseModel): + """A model use to check the schema may nest other models.""" + first_child: A + second_child: A -def test_array(): - ds = type_to_dataschema(list[int]) - j = ds_json_dict(ds) - assert j["type"] == "array" - assert j["items"]["type"] == "integer" +# The generated schema for B should look like this: +DATASCHEMA_FOR_B = { + "description": "A model use to check the schema may nest other models.", + "properties": { + # Note: unlike in JSONSchema, sub-schemas get inlined, as there's no + # way to refer to a schema defined elsewhere. + "first_child": DATASCHEMA_FOR_A, + "second_child": DATASCHEMA_FOR_A, + }, + "required": ["first_child", "second_child"], + "title": "B", + "type": "object", +} -# This is an annoying edge case where Thing Description -# and JSONSchema differ. For now, it is simply not -# supported so I won't test for it. -# If anyone tries it, they'll get an error rather than -# incorrect behaviour. -# def test_array_union(): -# ds = type_to_dataschema(list[Union[int, str]]) -# j = ds_json_dict(ds) -# assert j["type"] == "array" -# assert j["items"][0]["type"] == "integer" -# assert j["items"][1]["type"] == "string" +class C(BaseModel): + """A model where extra properties are allowed.""" -def test_boolean(): - ds = type_to_dataschema(bool) - j = ds_json_dict(ds) - assert j["type"] == "boolean" + model_config = {"extra": "allow"} + a: int -def test_object(): - class A(BaseModel): - a: int - b: Optional[int] = None +DATASCHEMA_FOR_C = { # The schema we should generate for the class above. + "title": "C", + "description": "A model where extra properties are allowed.", + "properties": {"a": {"title": "A", "type": "integer"}}, + "required": ["a"], + "type": "object", + "additionalProperties": True, # Extra properties are allowed +} - ds = type_to_dataschema(A) - j = ds_json_dict(ds) - assert j["type"] == "object" - assert "a" in j["required"] - assert "b" not in j["required"] +def ds_json_dict(ds: DataSchema) -> dict: + """Serialise a DataSchema to json and reinflate to a dict -def test_nested_object(): - class A(BaseModel): - a: int - b: Optional[int] = None - - class B(BaseModel): - first_child: A - second_child: A - - # locally-defined models can confuse pydantic, so we pass - # in A explicitly to convert annotations into types. - B.model_rebuild(_types_namespace={"A": A}) - - ds = type_to_dataschema(B) - j = ds_json_dict(ds) - assert j["type"] == "object" - assert j["properties"]["first_child"]["type"] == "object" - assert j["properties"]["first_child"]["properties"]["a"]["type"] == "integer" + This removes complicated types so we end up with a simple + representation that's slightly more abstracted from the + implementation details - e.g. enums are reduced to their + simple types. This is appropriate if we're testing against + what we'd expect to see in the API. + """ + return json.loads( + ds.model_dump_json( + exclude_none=True, + ) + ) + + +@pytest.mark.parametrize( + ("python_type", "schema_dict"), + [ + (Any, {}), + (int, {"type": "integer"}), + (float, {"type": "number"}), + (str, {"type": "string"}), + (bool, {"type": "boolean"}), + (None, {"type": "null"}), + # Literal should turn into a JSON "enum" + (Literal[1, 2], {"type": "integer", "enum": [1, 2]}), + (Literal["a", "b"], {"type": "string", "enum": ["a", "b"]}), + # Unions should use "oneOf" + (int | str, {"oneOf": [{"type": "integer"}, {"type": "string"}]}), + (None | str, {"oneOf": [{"type": "string"}, {"type": "null"}]}), + # lists and dicts/models are more complicated, because they're generic + (list, {"type": "array", "items": {}}), + (list[int], {"type": "array", "items": {"type": "integer"}}), + ( + list[int | str], # A list of union types should use "oneOf" + { + "type": "array", + "items": {"oneOf": [{"type": "integer"}, {"type": "string"}]}, + }, + ), + ( + tuple[int, str], # A tuple also becomes an array, but with defined items. + { + "type": "array", + "items": [{"type": "integer"}, {"type": "string"}], + "maxItems": 2, + "minItems": 2, + }, + ), + # a dictionary should be represented as a JSON object. + (dict, {"type": "object", "additionalProperties": True}), + ( + dict[int, str], # Note that non-string keys are unsupported, and ignored. + {"type": "object", "additionalProperties": {"type": "string"}}, + ), + ( + dict[Literal["a", "b"], int], + { + "type": "object", + "additionalProperties": {"type": "integer"}, + "propertyNames": {"enum": ["a", "b"]}, + }, + ), + # A model also becomes an "object" but it has defined "properties". + (A, DATASCHEMA_FOR_A), # Properties with simple types + (B, DATASCHEMA_FOR_B), # Properties that are nested models + (C, DATASCHEMA_FOR_C), # Additional properties are allowed + ], +) +def test_types(python_type, schema_dict): + """Check the schemas generated for a collection of simple types.""" + ds = type_to_dataschema(python_type) + assert ds_json_dict(ds) == schema_dict + + +def test_recursive_type(): + """Check that a recursive model fails with a `RecursionError`.""" + + class Recursive(BaseModel): + child: "Recursive" + + with pytest.raises(RecursionError): + type_to_dataschema(Recursive) + + +def test_prefixitems_overwrite(): + """Check the error if ``items`` and ``prefixItems`` are both present. + + This is very unlikely to happen for any Python datatype I can think of. + """ + json_schema: dict[str, Any] = { + "type": "array", + "prefixItems": [{"type": "string"}, {"type": "number"}], + "items": "int", + } + with pytest.raises(KeyError): + convert_prefixitems(json_schema) + + +def test_invalid_jsonschema(): + """Check the error if a type generates an invalid JSON Schema.""" + + class MyType: + @classmethod + def model_json_schema(cls): + return {"type": "invented"} + + # Just providing `model_json_schema` is a hangover from pydantic v1 + # that we may decide to remove at some point. We should still test it + # so long as it's in the library. + with pytest.raises(ValidationError, match="type\n Input should be "): + type_to_dataschema(MyType) + + # The annotated type below does the same thing, using newer mechanisms + # in `pydantic` to customise the JSONSchema. + with pytest.raises(ValidationError, match="type\n Input should be "): + type_to_dataschema(Annotated[int, WithJsonSchema({"type": "invalid"})])