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

Creates component registries in the OpenApiWorkspace for $ref resolution #1609

Merged
merged 37 commits into from
Apr 5, 2024

Conversation

irvinesunday
Copy link
Contributor

@irvinesunday irvinesunday commented Apr 2, 2024

Fixes #805

This PR:

  • Updates the OpenApiWorkspace.cs class by adding components registries that store JsonSchemas (or fragments), IOpenApiReferenceable objects and stream artifacts. These are indexed with unique key identifiers (URIs) that are a component of the unique document id that the components exist or as provided (in the case of stream artifacts or JsonSchema fragments). These unique key identifiers are also used to fetch the components from the components registry in the OpenApiWorkspace. The format of the URI identifier will be of the format:
    ex: http://openapi.net/{documentId}/components/schema/{name}
  • OpenApiWorkspaces are instantiated in the constructors of OpenApiDocuments. The document's BaseUri property is also instantiated in the constructor and is composed of the base path http://openapi.net/ and the unique document Id, which is a GUID.
  • All reference resolution (local and external) now happens in the workspace of the entry level OpenApiDocument. External documents have their ExternalResource identifier registered together with the unique document id of their host document in a _documentsIdRegister. This is a look-up table that external ref resolution uses to look up the unique document ids of external documents, given an ExternalResource identifier in an OpenApiReference object. This document id extracted from the lookup table will be used alongside the base path and the reference object id to construct the unique URI key that will be used to fetch the resolved component from the workspace component registry.

@irvinesunday irvinesunday marked this pull request as draft April 2, 2024 14:08
public OpenApiDocument()
{
Workspace = new OpenApiWorkspace();
BaseUri = new(OpenApiConstants.BaseRegistryUri + Guid.NewGuid().ToString());

Check notice

Code scanning / CodeQL

Redundant ToString() call Note

Redundant call to 'ToString' on a String object.
Comment on lines +84 to +91
else if (component is Stream stream)
{
if (!_artifactsRegistry.ContainsKey(uri))
{
_artifactsRegistry[uri] = stream;
return true;
}
}

Check notice

Code scanning / CodeQL

Nested 'if' statements can be combined Note

These 'if' statements can be combined.
return resolver.ResolveJsonSchemaReference(reference);
}
return null;
return (T)(object)artifact;

Check warning

Code scanning / CodeQL

Useless upcast Warning

There is no need to upcast from
Stream
to
Object
- the conversion can be done implicitly.
@irvinesunday irvinesunday changed the title Uses the document workspace to resolve local and external references Creates component registries in the OpenApiWorkspace for $ref resolution Apr 3, 2024
@@ -62,7 +39,7 @@ public OpenApiWorkspace(Uri baseUrl)
/// </summary>
public OpenApiWorkspace()
{
BaseUrl = new("file://" + Environment.CurrentDirectory + $"{Path.DirectorySeparatorChar}" );
BaseUrl = new Uri(OpenApiConstants.BaseRegistryUri);
Copy link
Member

Choose a reason for hiding this comment

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

I'm fighting with the feeling that "it can't be this simple", but if every document gets it's own GUID in the baseURI then we really don't need to use a file path. I think this is really good.

Copy link
Member

@darrelmiller darrelmiller left a comment

Choose a reason for hiding this comment

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

I can't say I have been able to mentally compile every line of this, but I really like the way this is looking. We need to get this merged with Maggie's changes and then do some extensive testing on it.

@irvinesunday irvinesunday marked this pull request as ready for review April 3, 2024 08:11
@irvinesunday irvinesunday self-assigned this Apr 3, 2024
@@ -68,7 +68,7 @@
{
Summary = example?.Summary ?? Summary;
Description = example?.Description ?? Description;
Value = JsonNodeCloneHelper.Clone(example?.Value);
Value = example?.Value ?? JsonNodeCloneHelper.Clone(example?.Value);

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
@@ -114,8 +114,8 @@
Style = header?.Style ?? Style;
Explode = header?.Explode ?? Explode;
AllowReserved = header?.AllowReserved ?? AllowReserved;
_schema = JsonNodeCloneHelper.CloneJsonSchema(header?.Schema);
Example = JsonNodeCloneHelper.Clone(header?.Example);
Schema = header?.Schema != null ? JsonNodeCloneHelper.CloneJsonSchema(header?.Schema) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
_schema = JsonNodeCloneHelper.CloneJsonSchema(header?.Schema);
Example = JsonNodeCloneHelper.Clone(header?.Example);
Schema = header?.Schema != null ? JsonNodeCloneHelper.CloneJsonSchema(header?.Schema) : null;
Example = header?.Example != null ? JsonNodeCloneHelper.Clone(header?.Example) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
@@ -167,9 +167,9 @@
Style = parameter?.Style ?? Style;
Explode = parameter?.Explode ?? Explode;
AllowReserved = parameter?.AllowReserved ?? AllowReserved;
_schema = JsonNodeCloneHelper.CloneJsonSchema(parameter?.Schema);
Schema = parameter?.Schema != null ? JsonNodeCloneHelper.CloneJsonSchema(parameter?.Schema) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
Examples = parameter?.Examples != null ? new Dictionary<string, OpenApiExample>(parameter.Examples) : null;
Example = JsonNodeCloneHelper.Clone(parameter?.Example);
Example = parameter?.Example != null ? JsonNodeCloneHelper.Clone(parameter?.Example) : null;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
@@ -68,7 +68,7 @@
{
Summary = example?.Summary ?? Summary;
Description = example?.Description ?? Description;
Value = JsonNodeCloneHelper.Clone(example?.Value);
Value = example?.Value ?? JsonNodeCloneHelper.Clone(example?.Value);

Check failure

Code scanning / CodeQL

Useless ?? expression Error

Both operands of this null-coalescing expression access the same variable or property.
@irvinesunday irvinesunday merged commit 77cd25a into release/2.0.0 Apr 5, 2024
5 of 8 checks passed
@irvinesunday irvinesunday deleted the is/component-registry branch April 5, 2024 08:09
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.

2 participants