OpenAPI Workspace Document Fragments

[simon-pearson]

• Add IOpenApiReferenceable ResolveReference(JsonPointer pointer, JsonPointer pointerFromRoot = null); method to IOpenApiElement interface.
• Add OpenApiElementBase class which containing an implementation of the ResolveReference method used by many of the classes which implement IOpenApiElement.
• Update implementation of OpenApiWorkspace.ResolveReference method to support referencing document fragments.
• UTs covering the above.

[ghost]

All CLA requirements met.

[darrelmiller]

In general this PR looks really good. I have avoided creating a base class for OpenAPIElements to date. I tend to try and avoid implementation inheritance where possible. I was wondering if I we could use an extension method on IOpenApiElement, but there that would mean adding ResolveReferencedProperty to IOpenApiElement.
I probably just need to get over it. I would think OpenApiElementBase should be abstract.

[darrelmiller]

Rather than adding ResolveReference to IOpenApiElement but then checking to make sure that any non-referenceable element just throws, could we instead move ResolveReference into IOpenApiReferencable?

[darrelmiller]

Can we add XML document blocks for public methods?

[darrelmiller]

This shouldn't be supported. The components object is not allowed to be the target of a reference. If someone wants a document that contains just components then they can create an external OpenApiDocument and just add the info element as well as a components element.

[darrelmiller]

So, I played around a bit and I was able to move the ResolveReference into the IOpenApiReferenceable interface. This allows us to remove it from IOpenApiElement. This guarantees that external fragments can only be referenceable components.

The spec isn't very clear on this but here http://spec.openapis.org/oas/v3.0.3#reference-object it says:

A simple object to allow referencing other components in the specification, internally and externally.

This is implicitly saying you can only point to to something externally if you can also point to it internally. e.g. a component.

The ability to point into a component that lives within a component is debatable. I'm not sure there is a strong use case for doing that. The risk of allowing it is we create re-usable things that are not explicitly identified as re-usable things. It would certainly make fragments easier if we didn't have to support referencing anything other than the top level element in a fragment document.

I know there are some tools that take a much more liberal approach to referencing. They allow pointing to any valid structure within a document or externally. We limited this ability in OpenAPI 3 by saying that references could only point to components.

[simon-pearson]

Apologies for the slow reply to this, I've been on holiday for the last two weeks. I'll get back to you on the above points over the next few days.

[simon-pearson]

Darrel,

I've re-worked my initial PR, making the following key changes:

1. I moved the ResolveReference method onto the IOpenApiReferencable interface as you suggested. This vastly simplifies the implementation of this method. I do have some questions around this (below).
2. Again I've followed your suggestion and have implemented the IOpenApiReferencable.ResolveReference method as an extension method on IOpenApiReferencable, removing the need to introduce a base class.

Questions:

1. It's possible I've completely missed the point with my implementation of the IOpenApiReferencable.ResolveReference method but as I see it the only OpenAPI objects which are referencable themselves (i.e. implement IOpenApiReferencable) and also have referencable child properties are the Header object (with properties schema and examples), the Parameter object (again schema and examples), and the Response object (properties headers and links). This makes my implementation of the IOpenApiReferencable.ResolveReference method much simpler than it was previously. Does this look right to you?
2. Do you agree with the following: if we're saying that the only OpenAPI objects which are valid document fragments are those which implement IOpenApiReferencable then we should change the constraint on the OpenApiStreamReader.ReadFragment method to restrict the method to only returning elements which implement IOpenApiReferencable:

public T ReadFragment<T>(Stream input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic) where T : IOpenApiReferenceable
{
    ...
}

Do these changes better align with what you had in mind for document fragments?

[darrelmiller]

@simon-pearson I think it is a reasonable assertion to say that ReadFragment can only return things that are referenceable. I imagine there might be a few edge use cases where someone might want to construct an OpenAPIDocument from pre-existing fragments of JSON that are not necessarily referenceable, but it feels like a reasonable trade off to lose that ability for the simplicity it brings us. Also, enabling users to have whole new way of having tooling based re-use seems like bad thing thing to encourage. If you really want to do reuse outside of the boundaries of OpenAPI semantics, then I would suggest using some kind of text pre-processor.

I'm going to try and finish reviewing this PR today.

[darrelmiller]

I just realized that this PR is against my OpenApiWorkspace branch, so I'm going to merge that in and get the OpenAPIWorkspace branch updated so it can merge into 1.3 milestone.