Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work towards OpenAPI 3.1 support #22

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from
Draft

Work towards OpenAPI 3.1 support #22

wants to merge 27 commits into from

Conversation

kevindew
Copy link
Owner

No description provided.

These are intended to be used as ways to determine whether particular
OpenAPI features will be enabled.
This class is to replace the use of a string as the means for
representing the version of an OpenAPI version. A new class has been
added to make it simpler to compare versions in a more sophisticated way
than strings. The class inherits from Gem::Version which is a part of
Ruby stdlib which has this comparison logic built in.

The spaceship operator method has been replaced with one that will
apply polymorphish to comparisons. E.g:

It replaces the need for:

```
OpenapiVersion.new("3.11") > OpenapiVersion.new("3.2")
```

with:

```
OpenapiVersion.new("3.11") > "3.2"
```

I added this in as it felt more natural to do this than have some
contrived `equal_or_greater_than?` methods. However I do have a concern
that I may have created some Ruby magic
Previously this argument only accepted boolean arguments and didn't
allow you to specify that this could execute code. This code changes
that by allowing lambda functions and symbols (that reference methods)
as arguments.

This has been added so that specifying whether a field is required can
be determined by the openapi_version.
This feature allows fields of a node to be marked as whether they are
allowed to exist or not. This has been added in to support the OpenAPI
3.1 where there are fields that are only valid in OpenAPI 3.1 documents.

A field can take an allowed value of a boolean, a proc or a symbol. It
is expected that, outside of contrived tests, it will never use a plain
boolean as there is no point marking a field as not allowed everywhere.
This field was introduced with OpenAPI 3.1
This is a requirement from OpenAPI v3.1: The OpenAPI document MUST
contain at least one paths field, a components field or a webhooks
field. [1]

[1]: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#openapi-document
This is an attempt to try catalogue the changes that are needed - the
most substantial of which seem to be in the Schema object.
With OpenAPI 3.1 the schema object is quite different (conforming to
2012-12 spec of JSON Schema). I think the easiest way to deal with this
is to create separate classes for working with it.

This starts this process by creating Schema::V3_0 classes which is a
rename of the existing Schema classes.
This has been provided so that the OpenAPI 3.1 Schema can accept a
schema with extensions without the `x-` prefix that OpenAPI nodes
require.
Sorry - this is way too big for a single commit.

This changes the approach for handling references to be one where data
can be merged between a chain of references. Previously the approach was
to only allow a single $ref field (as per OpenAPI 3.0)

The reason for making this change is based on aspects in the
specification notably the reference object [1] that allows title and
summary fields to be overridden through the referencing process.

As far as I could tell schemas also share this property as per the newer
JSON Schema specifications (most applicable one for OpenAPI 3.1 is
2020-12) though I found it quite hard to find a clear source on
behaviour [2].

To support this there are now some new concepts:

- A node context now has multiple source_locations - as data can be
  combined from multiple places
- A node context has a concept, input_locations, which record all of the
  data involved in a node that aren't purely references (i.e. all the
  source locations that contributed to the node data)
- There is now a Referenceable module that can be mixed into NodeFactory
  classes, this allows NodeFactories to act more as the mediator in
  charge of a reference, whereas previously more went through a
  Reference field (this is to reflect the nature of merging)
- Aspects of the ObjectFactory::NodeBuilder class have been separated
  into ObjectFactory::NodeErrors, ObjectFactory::ResolvedInputBuilder
  and a new iteration of it's namesake. This is to reflect an increase
  in complexity in node building due to the node data merging
- As part of the new NodeBuilder class the building of node objects is
  now done by the NodeBuilder class calling public methods on node
  factories. This has led to them having their #build_object methods
  replaced with #build_node methods that need a public interface

[1]: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#referenceObject
[2]: https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-00#section-8.2.3.1
This is unfortunately rather long, but there doesn't seem any clear
single responsibility opportunities that can be embraced so I'm taking
the coward/assertive route of telling the linter: no.
In OpenAPI 3.1 references can have summary and description metadata for
nodes that support those fields. They operate in a cascading pattern
where each reference can overwrite the previous ones.
This adds the identifier field to License - as outlined in the OpenAPI
3.1 Specification [1].

A couple of things I considered doing for this, but ultimately didn't
do, were:

- Determine whether the SPDX license expression format could be
  validated with a regex. It looked like it probably could but not sure
  if we want to be that strict.
- Add a hook in that would allow us to decide which OpenAPI versions
  would use the mutually_exclusive DSL method. I decided this wasn't
  necessary for now since the only benefit seems to be a slightly nicer
  error message.
We already validate this. In 3.0.x to 3.1.x the spec changed from:

> An enumeration of string values to be used if the substitution options are from a limited set. The array SHOULD NOT be empty.

to:

> An enumeration of string values to be used if the substitution options are from a limited set. The array MUST NOT be empty.
This code doesn't validate the type of a Security Scheme so we don't
need to change the code for the new type of mutualTLS.
A change in OpenAPI 3.1 is that the Discriminator object can accept
extensions, when previously this was not allowed.

In order to code this I've had to adjust the DSL so that the
allow_extensions method can accept a block.

We didn't have unit tests for the DSL so I'm only testing this at the
node level. This should be ok as it has full coverage.
@rngtng
Copy link

rngtng commented Jun 29, 2023

@kevindew great to see there's progress in 3.1 support.. what the current status here? happy to support!

- perhaps have context support a merge concept for source location
- Think about dealing with recursive defined as "#"

Dealing with the new JSON Schema approach for OpenAPI 3.1.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about piggyback on existing implementation like https://github.com/voxpupuli/json-schema?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit rusty in memory but there's some tricky challenges as I recall. If I remember right, that gem doesn't support a version of JSON schema as new as what OpenAPI supports and I have recollections that it's more useful for validation than it is for traversal.

@kevindew
Copy link
Owner Author

@rngtng Hey, thanks for the interest and sorry for taking a while to get back to you.

Currently I'm hoping to find some time in the next few months to pick this up again and make progress. It's generally a case of working through the items in https://github.com/kevindew/openapi3_parser/blob/fc98338bcc16fb724c3974eec8103ee76e61fb65/TODO.md

@rngtng
Copy link

rngtng commented Aug 21, 2023

thx @kevindew - let's see if I find time to get a deeper view and maybe support you..

@ekzobrain
Copy link

ekzobrain commented Nov 18, 2023

@kevindew Hi. I suggest to focus on reading only and leave validation to a separate gem: https://github.com/davishmcclurg/json_schemer
It does a great job of OpenApi spec validation including bundled JSONSchema validation (of any spec version), so you will need to implement just reading capabilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants