Add support for loading schema with oneOf, anyOf, allOf, not $refs

Question

Add support for loading schema with oneOf, anyOf, allOf, not $refs

Closed this issue 4 years ago · 23 comments

Hi fellas,

I get this:

 File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/testing.py", line 37, in validate_response
    verbose_error_message = format_response_tester_error(e, hint=e.response_hint)
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/utils.py", line 47, in format_response_tester_error
    example_item = settings.loader_class.create_dict_from_schema(exception.schema)
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/loaders.py", line 335, in create_dict_from_schema
    return self._iterate_schema_dict(schema)
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/loaders.py", line 294, in _iterate_schema_dict
    elif read_type(value) == 'object':
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/openapi.py", line 53, in read_type
    raise OpenAPISchemaError(
django_swagger_tester.exceptions.OpenAPISchemaError: Schema item has an invalid `type` attribute. The type should be a single string.

I then see this printed:
Schema item: {'allOf': [{'type': 'object', 'description': 'Read-only Product Serializer', 'properties': {'slug': {'type': 'string', 'readOnly': True, 'pattern': '^[-a-zA-Z0-9_]+$'}, 'loop': {'type': 'string', 'readOnly': True}, 'sites': {'type': 'array', 'items': {'type': 'object', 'description': 'read-only Site serializer', 'properties': {'id': {'type': 'integer', 'readOnly': True}, 'slug': {'type': 'string', 'readOnly': True, 'pattern': '^[-a-zA-Z0-9_]+$'}, 'domain': {'type': 'string', 'readOnly': True}}, 'required': ['domain', 'id', 'slug']}, 'readOnly': True}, 'acr': {'allOf': [{'enum': ['3', '4'], 'type': 'string'}], 'readOnly': True}}, 'required': ['acr', 'loop', 'sites', 'slug']}], 'readOnly': True}

which I prettified:

{
  allOf: [
    {
      type: "object",
      description: "Read-only Product Serializer",
      properties: {
        slug: { type: "string", readOnly: True, pattern: "^[-a-zA-Z0-9_]+$" },
        loop: { type: "string", readOnly: True },
        sites: {
          type: "array",
          items: {
            type: "object",
            description: "read-only Site serializer",
            properties: {
              id: { type: "integer", readOnly: True },
              slug: {
                type: "string",
                readOnly: True,
                pattern: "^[-a-zA-Z0-9_]+$",
              },
              domain: { type: "string", readOnly: True },
            },
            required: ["domain", "id", "slug"],
          },
          readOnly: True,
        },
        acr: { allOf: [{ enum: ["3", "4"], type: "string" }], readOnly: True },
      },
      required: ["acr", "loop", "sites", "slug"],
    },
  ],
  readOnly: True,
};

I dont see any issue with the type key here.

Answer 1 · 2020-12-13T19:17:45.000Z

Here is another one:

 File "/opt/project/consumer_portal/base_test_cases.py", line 62, in assertResponse
    validate_response(
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/testing.py", line 28, in validate_response
    SchemaTester(
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/schema_tester.py", line 46, in __init__
    self.test_dict(schema=schema, data=data, reference='init')
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/schema_tester.py", line 154, in test_dict
    if read_type(schema_value) == 'object':
  File "/usr/local/lib/python3.8/site-packages/django_swagger_tester/openapi.py", line 53, in read_type
    raise OpenAPISchemaError(
django_swagger_tester.exceptions.OpenAPISchemaError: Schema item has an invalid `type` attribute. The type should be a single string.

Schema item: {'allOf': [{'enum': ['activated', 'deactivated', 'pending'], 'type': 'string'}], 'nullable': True}

Answer 2 · 2020-12-14T08:18:11.000Z

I suspect it's because of your brackets ([, ]).

As far as I can tell you're seeing the error because this function is unable to read the type of the objects it's receiving, and I believe this logic is structured this way because arrays aren't permitted in OpenAPI specs. I could be wrong here, but this was done intentionally based on that assumption.

I believe your documentation will be correct if you drop the brackets.

Here's a simple example of what I mean:

{
  'title': 'object_type_title',
  'type': 'object',
  'properties': {
    'string': {
      'description': 'This is a string type',
      'type': 'string',
      'example': 'string'
    },
    'integer': {
      'description': 'This is an integer type',
      'type': 'integer',
      'example': 1
    },
    'number': {
      'description': 'This is a number type',
      'type': 'number',
      'example': 1.1
    },
    'bool': {
      'description': 'This is a boolean type',
      'type': 'boolean',
      'example': True
    }
  }
}

Please let me know if that helps you 👍

Answer 3 · 2020-12-14T08:22:32.000Z

well, my specs are auto generated by drf-spectacular and are valid according to swagger-cli. I think the issue is with your handling of allOf, oneOf etc. These are most defintely based on array notation:

https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

Answer 4 · 2020-12-14T08:27:54.000Z

I'm pretty confident drf-yasg does it the way shown in the example, so that difference is probably why this happened. I'll look at this after work today - perhaps it can be resolved as easily as by adding a pass for the first item of arrays and a guard somewhere upstream to ensure it's not empty.

In the meantime PRs are of course always welcome 🥇

Answer 5 · 2020-12-14T10:29:11.000Z

my specs are auto generated by drf-spectacular and are valid according to swagger-cli

Just wanted to follow up with a quick question before looking to solve this. Here's the docs for the OpenAPI 3 array type. I can't see anything indicating that brackets are valid for documenting arrays.

Are you sure this isn't a drf-spectacular issue? It would be really great if you could link some docs showing otherwise.

Answer 6 · 2020-12-14T12:35:13.000Z

this is YAML, not OpenAPI specific. In YAML you can use either bracket notation or the structured list with - notation.

See this for example:
https://stackoverflow.com/questions/23657086/yaml-multi-line-arrays/33136212

Answer 7 · 2020-12-14T12:39:11.000Z

Also @sondrelg I use swagger-cli whenever i have doubts to validate specs - the drf spectacular specs are passing it: swagger-cli validate <file>.

You can see various examples in this repo: https://github.com/OAI/OpenAPI-Specification/tree/master/examples/v3.0

Answer 8 · 2020-12-14T12:44:14.000Z

Ok that's great. I guess I can just add a test for validating a response with this type of example and it should be easy enough to fix.

Answer 9 · 2020-12-14T18:49:54.000Z

@Goldziher, could you check if #119 fixes the issue for you?

Answer 10 · 2020-12-14T19:09:52.000Z

@Goldziher, could you check if #119 fixes the issue for you?

I can, hold on.

Answer 11 · 2020-12-14T19:17:19.000Z

Seems I cannot - i tried installing this particular commit using pipenv and im getting an error:
pipenv install -e git+https://github.com/snok/django-swagger-tester.git@0b87b139c77b6aff4bc36f0a82d005d2b0810c11#egg=django_swagger_tester

Doesnt work because:
ModuleNotFoundError: No module named 'setuptools'

Answer 12 · 2020-12-15T13:55:18.000Z

I've never used pipenv, but can you not just use pip install git+https://github.com/snok/django-swagger-tester/tree/sondrelg/fix-116?

My project mostly use Poetry which seems very similar, but you can always just use pip to install things in your venv directly. Does that work for you?

Answer 13 · 2020-12-15T15:12:16.000Z

It doesnt no. I will temporarily clone this into my project to test.

Answer 14 · 2020-12-15T15:54:06.000Z

@sondrelg your fix does not work im afraid. Its also not where the error is occuring. The error is occurring here:

def read_type(item: dict) -> str:
    ...
    if (
        not item is None
        or not isinstance(item, dict)
        or "type" not in item
        or not item["type"]
        or not isinstance(item["type"], str)
    ):
        raise OpenAPISchemaError(
            f"Schema item has an invalid `type` attribute. The type should be a single string.\n\nSchema item: {item}"
        )
    ....

I slightly rewrote this to be more readable. Anyhow, the problem as far as I can see is that the library does not handle "allOf", "anyOf" openApi objects at all.

The error is this:

django_swagger_tester.exceptions.OpenAPISchemaError: Schema item has an invalid `type` attribute. The type should be a single string.

Schema item: {'allOf': [{'type': 'object', 'description': 'Read-only Product Serializer', 'properties': {'slug': {'type': 'string', 'readOnly': True, 'pattern': '^[-a-zA-Z0-9_]+$'}, 'loop': {'type': 'string', 'readOnly': True}, 'sites': {'type': 'array', 'items': {'type': 'object', 'description': 'read-only Site serializer', 'properties': {'id': {'type': 'integer', 'readOnly': True}, 'slug': {'type': 'string', 'readOnly': True, 'pattern': '^[-a-zA-Z0-9_]+$'}, 'domain': {'type': 'string', 'readOnly': True}}, 'required': ['domain', 'id', 'slug']}, 'readOnly': True}, 'acr': {'allOf': [{'enum': ['3', '4'], 'type': 'string'}], 'readOnly': True}}, 'required': ['acr', 'loop', 'sites', 'slug']}], 'readOnly': True}

Why is the problem raised? because the if clause basically checks these conditions:

       not isinstance(item, dict)
        or "type" not in item.keys()
        or not isinstance(item["type"], str)

The schema is a dict, but it doesnt have a type entry but rather a list of dictionaries that each has a "type" key.

allOf: [
    { type: object, properties: {}}, { type: object, properties: {}}, { type: object, properties: {}},
]

The allOf value of these is a merged object that has the properties of all objects. So the type will be object in this case, but when you validate the schema you also need to check the allOf values.

oneOf is also a list, but the schema should match one and only one of the schemas and so forth. Here are the docs: https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

Answer 15 · 2020-12-15T18:04:44.000Z

Nice, this makes sense. To resolve this, would you be willing to create a test with example data to replicate your issue?

Something like this test, just substituting GOOD_DATA for failing data.

Answer 16 · 2020-12-15T18:40:27.000Z

I'm afraid I do not have the spare time at present and have several pending projects of my own. If I had the time I would resolve the issue myself.

Since this is a core aspect of openapi - I would also suggest you guys get a hold of several complex schemas that are valid (using swagger-cli to ensure they are), and then make sure you are able to actually parse the entire thing. That is, do not check the response validation first, make sure you are in fact able to parse the specs without raising an "SwaggerDocumentationError" or encountering a builtin exception when doing this.

Answer 17 · 2020-12-20T12:46:59.000Z

After re-reading this on a non-work day, I see now that oneOf, anyOf, etc. are $ref mechanics. I got a bit stuck on the type error when reading this last week I think.

Adding support for this would of course be welcomed, but like you I am also pressed for time, and I'm no longer using this package as actively in my own work as I used to, so I don't think I'll be able to sit down and do this myself anytime soon.

I've added the help wanted tag for this issue, and hopefully this can be picked up by someone else at some point.

Answer 18 · 2020-12-29T18:13:20.000Z

Thanks for your time @sondrelg. I will implement this I think when I have time. Two questions-

any issue with me using https://github.com/jfinkhaeuser/prance to implement parsing mechanics?
will you add me to the organization and give me some privileges (like creating a branch?).

Answer 19 · 2020-12-29T20:48:13.000Z

Hi @Goldziher!

any issue with me using https://github.com/jfinkhaeuser/prance to implement parsing mechanics?

No I don't have any issue with that. I think a OAS parser downstream library seems like a reasonable requirement for a OAS validation package.

will you add me to the organization and give me some privileges (like creating a branch?).

Absolutely 👏 That's well earned, and I'll take all the help I can get! I've given you write access to the repo; let me know if you encounter any problems or need anything else 👍

Answer 20 · 2021-01-11T16:15:11.000Z

I have the same problem mentioned here. I'm also generating my schema with drf-spectacular. Is there any way I can help?

Answer 21 · 2021-01-15T17:57:28.000Z

Hi @idesoto,

I will add a PR this weekend (hopefully). If you have schemas you can share to use as a basis for testing it would be helpful to me :).

Answer 22 · 2021-01-20T09:39:41.000Z

Hi @Goldziher, I can test the changes with my schema. Is it the PR that was merged 13 hours ago?

Answer 23 · 2021-01-23T19:24:13.000Z

hi @idesoto - yes, you should be able to test the library against allOf. handling of anyOf and oneOf is still in development.