Open-EO / openeo-api

The openEO API specification
http://api.openeo.org
Apache License 2.0
91 stars 11 forks source link

Properties with the name "$id" cause issues with spec validation #499

Closed GeraldIr closed 1 year ago

GeraldIr commented 1 year ago

In the new version of the openEO API spec, two instances of a property called "$id" were introduced which causes errors (TypeError unhashable type 'dict') when validating the spec with the openapi-core library (more specifically the openapi-spec-validator) and by extension potentially with other tools for handling openAPI spec.

The occurences are on lines 1126 and 5853 of the openapi.json respectively.

For my own purposes I can just rename them to "$idn" (for instance) locally, but this might be of interest to other people as well.

This does seem like more of an issue on the validation library side, so I'm not sure if it is worth changing, but I wanted to raise the issue regardless.

GeraldIr commented 1 year ago

I just found that according to the openAPI spec "$id" does not actually fall under the "Supported JSON keywords" list https://swagger.io/docs/specification/data-models/keywords/ (along with "$schema" and some other examples), so this might need to be looked into actually.

m-mohr commented 1 year ago

So the $id and $schema are there to properly describe JSON Schema in response bodies (in an example and as a property, both is valid according to OpenAPI, the validator we use also doesn't complain). I think this is a tooling issue in openapi-core. Does it help to wrap it in '? So '$id'?

GeraldIr commented 1 year ago

Can you point me to the openAPI documentation where it says that $id and $schema are valid keywords? All I can find is that all given schemas have to be openAPI schemas NOT JSON schemas and that some specific keywords such as $id and $schema are specifically not supported as already outlined in my other comment (https://swagger.io/docs/specification/data-models/keywords/).

I might just be missing or mixing up something in the documentation so if you could point me to where exactly it allows those keywords that would be appreciated!

Edit: Also this was specifically added in 1.2.0 and response bodies were already being described in 1.1.0 without these fields no?

m-mohr commented 1 year ago

This is NOT about OpenAPI keywords, please have a closer look at where $schema and $id are used! What we are defining are properties for an object in a schema. So the applicable keyword in JSON schema and OpenAPI is "properties". The properties keyword definition in OpenAPI is what is defined in JSON Schema according to OpenAPI. JSON Schema doesn't restrict the allowed property names at all. Thus this is valid and a tooling issue. I don't have the time right now to point you to the docs, sorry, but with the explanations above you should be able to find the relevant places yourself (although don't expect an explicit mention of $schema and $id, just look for allowed property names for the properties keyword in JSON Schema).

In 1.1.0 this was not specifically described, but allowed through the implicit "additionalProperties: true".

GeraldIr commented 1 year ago

No, I understand that you have implemented the JSON Schema (https://json-schema.org/) manually inside of the openAPI document and therefore you are using $id and $schema as those are valid parts of the json-schema.

What I'm asking is why you aren't simply using the inherent openAPI-schema which is already an extended subset of the json-schema. What functionality is being covered by the json-schema that is not covered by the openAPI-schema. The only difference that could be relevant right now is that "null" is not included in the data types (instead being covered by the nullable property)

I don't think calling it a tooling issue is fair, because the "json_schema" object defined inside of the openEO-specification is still being interpreted as a openAPI-schema ITSELF (even though it represents a different type of schema on a meta-level which could be used in the document) and therefore the use of $id and $schema are not valid by my understanding.

m-mohr commented 1 year ago

Sorry, but you are wrong. We describe a JSON Schema in the response body using OpenAPI. openEO processes use JSON Schema to describe their parameters etc and that's what we model here. It has nothing to do with the OpenAPI definition itself, we don't use JSON Schema in the OpenAPI definition (except for was OpenAPI defines)! It is a tooling issue! If you don't belive me, please consult the OpenAPI community and they will confirm that what we do here is valid with regards to defining a JSON Schema response body.

GeraldIr commented 1 year ago

Sorry about the confusion, you're right, the following sentence of the documentation (and the fact that the $id property breaks some libraries) really threw me off "properties – individual property definitions must follow OpenAPI schema rules and not standard JSON Schema" , but yes the $id part is totally fine then.

Sorry again for wasting your time.