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.

Sign up for GitHub

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

feat: allow non-component references #132

Merged
merged 5 commits into from
Oct 20, 2023
Merged

feat: allow non-component references #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
32e527b
Allow any type of reference
VisualBean Oct 11, 2023
f34bdac
add another test
VisualBean Oct 11, 2023
d08ce2f
Merge branch 'vnext' into alex/references
VisualBean Oct 11, 2023
c604cc1
add test case
VisualBean Oct 12, 2023
d18e3a3
fix tests
VisualBean Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions ...API.Readers/AsyncApiYamlDocumentReader.cs → ...API.Readers/AsyncApiJsonDocumentReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Copyright (c) The LEGO Group. All rights reserved.
// Copyright (c) The LEGO Group. All rights reserved.

namespace LEGO.AsyncAPI.Readers
{
using System.Collections.Generic;
using System.Linq;
using System.Text.Json.Nodes;
using System.Threading;
using System.Threading.Tasks;
using LEGO.AsyncAPI.Exceptions;
using LEGO.AsyncAPI.Extensions;
Expand Down Expand Up @@ -76,7 +77,7 @@ public AsyncApiDocument Read(JsonNode input, out AsyncApiDiagnostic diagnostic)
return document;
}

public Task<ReadResult> ReadAsync(JsonNode input)
public async Task<ReadResult> ReadAsync(JsonNode input, CancellationToken cancellationToken = default)
{
var diagnostic = new AsyncApiDiagnostic();
var context = new ParsingContext(diagnostic)
Expand Down Expand Up @@ -106,11 +107,11 @@ public Task<ReadResult> ReadAsync(JsonNode input)
}
}

return Task.FromResult(new ReadResult
return new ReadResult
{
AsyncApiDocument = document,
AsyncApiDiagnostic = diagnostic,
});
};
}

private void ResolveReferences(AsyncApiDiagnostic diagnostic, AsyncApiDocument document)
Expand Down
2 changes: 1 addition & 1 deletion src/LEGO.AsyncAPI.Readers/AsyncApiReaderSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public enum ReferenceResolutionSetting
DoNotResolveReferences,

/// <summary>
/// ResolveAllReferences, effectively inlining them.
/// Resolve internal component references and inline them.
/// </summary>
ResolveReferences,
}
Expand Down
28 changes: 22 additions & 6 deletions src/LEGO.AsyncAPI.Readers/V2/AsyncApiV2VersionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,21 @@ public AsyncApiReference ConvertToAsyncApiReference(
Id = reference,
};
}

var asyncApiReference = new AsyncApiReference();
if (reference.StartsWith("/"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth noting that I believe the spec also supports "relative" paths. So they could conceivably start with . and .. as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is already fully supported, gotta have to check for the case of having a fragment though.

Something like
./myjsonfile.json/fragment

that would simply fall under 'externalresource' and not isFragment: true

Copy link
Contributor

Choose a reason for hiding this comment

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

So, just to clarify, something like a schema stored as a yaml file separately using cloudevents or some other schema would be an externalResource and also a fragment. But a reference to an entire AsyncAPI doc would simply be an externalResource?

Copy link
Collaborator Author

@VisualBean VisualBean Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think fragments are, when a subset is referenced inside another Json file. (Which is not what I stated above - it should have isFragment:true).

So jsonfile.json#/someLocation would be external AND a fragment
Like /whatever is the same, but in the same host document.

But I got dizzy from all of the RFCs involved.
I need to be determine 100% what "fragment" actually refers to.

I'll get back to you on that, once clarified, so we are 100% in line and on point regarding when, which is what.

Copy link
Collaborator Author

@VisualBean VisualBean Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After some investigating, ive come to the understanding that there are 3 types of references

  1. Fragments
    ./external-doc.json#/fragment different file document. (relative path)
    /fragments/myFragment same document.
    https://example.com/document.json#/fragment different external file document.
  2. Internal
    #/components/schemas/Pet (we already support those)
  3. 'Full' document Referenes
    ./external-doc.json We can choose to support this (although it technically does not align with the spec.')

and is a mix of
Json Reference
JSON Pointer
Uri

The main point is paragraph 3 and 4 of the Json Reference RFC

3. Syntax

A JSON Reference is a JSON object, which contains a member named
"$ref", which has a JSON string value. Example:

{ "$ref": "http://example.com/example.json#/foo/bar" }

If a JSON value does not have these characteristics, then it SHOULD
NOT be interpreted as a JSON Reference.

The "$ref" string value contains a URI [RFC3986], which identifies
the location of the JSON value being referenced. It is an error
condition if the string value does not conform to URI syntax rules.
Any members other than "$ref" in a JSON Reference object SHALL be
ignored.

4. Resolution

Resolution of a JSON Reference object SHOULD yield the referenced
JSON value. Implementations MAY choose to replace the reference with
the referenced value.

If the URI contained in the JSON Reference value is a relative URI,
then the base URI resolution MUST be calculated according to
[RFC3986], section 5.2. Resolution is performed relative to the
referring document.

If a URI contains a fragment identifier, then the fragment should be
resolved per the fragment resolution mechansim of the referrant
document. If the representation of the referrant document is JSON,
then the fragment identifier SHOULD be interpreted as a
[JSON-Pointer].

Copy link
Collaborator Author

@VisualBean VisualBean Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ive pushed tests that should verify this behaviour.

So now it should be a matter of figuring out the Uri scheme, fetching the file/fragment accordingly, using the loader and insert values.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, interesting, definitely adds some clarity.

Especially so that "'Full' document References" aren't in this RFC because I've seen them used a lot on AsyncAPI slack to allow sharing of resources between OpenAPI schemas, AsyncAPI schemas and schema registry.

Is it significant that all the examples are to .json files? Or can we allow for .yaml as well?

Similar context in terms of spec intention vs facilitating actual usage: I'm never sure with this stuff because I'm sure there's a similar situation with servers where the AsyncAPI spec doesn't actually specify its contents can be $ref'd but I've seen a many members of the core AsyncAPI team themselves reffing the servers object in their config.

Copy link
Contributor

@dpwdec dpwdec Oct 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Given the IReferenceLoader pattern on my PR we could actually just push this concern to the user. They can decide how they want to read when using external files. We could provide them with implementations for the canonical uses outlined. But if they want to do something extremely weird then they've got the interface to implement around their needs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My thoughts exactly

{
asyncApiReference.IsFragment = true;
}

asyncApiReference.ExternalResource = segments[0];

return asyncApiReference;

}
else if (segments.Length == 2)
{
// Local reference
if (reference.StartsWith("#"))
{
try
Expand All @@ -84,7 +96,7 @@ public AsyncApiReference ConvertToAsyncApiReference(
}

var id = segments[1];

var asyncApiReference = new AsyncApiReference();
if (id.StartsWith("/components/"))
{
var localSegments = segments[1].Split('/');
Expand All @@ -103,12 +115,16 @@ public AsyncApiReference ConvertToAsyncApiReference(

id = localSegments[3];
}

return new AsyncApiReference
else
{
Type = type,
Id = id,
};
asyncApiReference.IsFragment = true;
}

asyncApiReference.ExternalResource = segments[0];
asyncApiReference.Type = type;
asyncApiReference.Id = id;

return asyncApiReference;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/LEGO.AsyncAPI/Models/AsyncApiDocument.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using System.Collections.Generic;
using LEGO.AsyncAPI.Exceptions;
using LEGO.AsyncAPI.Models.Interfaces;
using LEGO.AsyncAPI.Writers;

Check warning on line 9 in src/LEGO.AsyncAPI/Models/AsyncApiDocument.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest) 

Using directives should be ordered alphabetically by the namespaces. (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1210.md)
using LEGO.AsyncAPI.Services;

/// <summary>
Expand Down Expand Up @@ -150,7 +150,7 @@
return this.ResolveReference(reference) as T;
}

public IAsyncApiReferenceable ResolveReference(AsyncApiReference reference)
internal IAsyncApiReferenceable ResolveReference(AsyncApiReference reference)
{
if (reference == null)
{
Expand Down
45 changes: 44 additions & 1 deletion src/LEGO.AsyncAPI/Models/AsyncApiReference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
/// </summary>
public class AsyncApiReference : IAsyncApiSerializable
{
/// <summary>
/// External resource in the reference.
/// It maybe:
/// 1. a absolute/relative file path, for example: ../commons/pet.json
/// 2. a Url, for example: http://localhost/pet.json
/// </summary>
public string ExternalResource { get; set; }

/// <summary>
/// Gets or sets the element type referenced.
/// </summary>
Expand All @@ -27,17 +35,37 @@
public AsyncApiDocument HostDocument { get; set; } = null;

/// <summary>
/// Gets the full reference string for v2.3.
/// Gets a flag indicating whether a file is a valid OpenAPI document or a fragment
/// </summary>
public bool IsFragment { get; set; } = false;

/// <summary>
/// Gets a flag indicating whether this reference is an external reference.
/// </summary>
public bool IsExternal => this.ExternalResource != null;

/// <summary>
/// Gets the full reference string for v2.
/// </summary>
public string Reference
{
get
{
if (this.IsExternal)
{
return this.GetExternalReferenceV2();
}

if (!this.Type.HasValue)
{
throw new ArgumentNullException(nameof(this.Type));
}

//if (this.Type == ReferenceType.SecurityScheme)

Check warning on line 64 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest) 

Single line comment should begin with a space.

Check warning on line 64 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest)

//{

Check warning on line 65 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest) 

Single line comment should begin with a space.

Check warning on line 65 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest)

// return this.Id;
//}

Check warning on line 67 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest) 

Single-line comments should not be followed by blank line

Check warning on line 67 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (windows-latest) 

Single line comment should begin with a space.

Check warning on line 67 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest)

Check warning on line 67 in src/LEGO.AsyncAPI/Models/AsyncApiReference.cs

View workflow job for this annotation

GitHub Actions / build (macos-latest)


return "#/components/" + this.Type.GetDisplayName() + "/" + this.Id;
}
}
Expand Down Expand Up @@ -67,6 +95,21 @@
writer.WriteEndObject();
}

private string GetExternalReferenceV2()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand what intention is with this method. If, its external why does it start pointing an id in components on the document?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a case, where the reference is a full asyncapidocument, and we are pointing at a 'real' reference there.
So it sets up the Reference to match what the internal references look like, except it has the 'externalResource' set as the document path. made a test case for it

{
if (this.Id != null)
{
if (this.IsFragment)
{
return this.ExternalResource + "#" + this.Id;
}

return this.ExternalResource + "#/components/" + this.Type.GetDisplayName() + "/" + this.Id;
}

return this.ExternalResource;
}

public void Write(IAsyncApiWriter writer)
{
this.SerializeV2(writer);
Expand Down
15 changes: 12 additions & 3 deletions src/LEGO.AsyncAPI/Services/AsyncApiReferenceResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public override void Visit(AsyncApiSchema schema)
this.ResolveMap(schema.Properties);
}

private void ResolveObject<T>(T entity, Action<T> assign) where T : class, IAsyncApiReferenceable
private void ResolveObject<T>(T entity, Action<T> assign) where T : class, IAsyncApiReferenceable, new()
{
if (entity == null)
{
Expand Down Expand Up @@ -184,7 +184,7 @@ private void ResolveObject<T>(T entity, Action<T> assign) where T : class, IAsyn
}
}

private void ResolveMap<T>(IDictionary<string, T> map) where T : class, IAsyncApiReferenceable
private void ResolveMap<T>(IDictionary<string, T> map) where T : class, IAsyncApiReferenceable, new()
{
if (map == null)
{
Expand All @@ -201,8 +201,17 @@ private void ResolveMap<T>(IDictionary<string, T> map) where T : class, IAsyncAp
}
}

private T ResolveReference<T>(AsyncApiReference reference) where T : class, IAsyncApiReferenceable
private T ResolveReference<T>(AsyncApiReference reference) where T : class, IAsyncApiReferenceable, new()
{
if (reference.IsExternal)
{
return new ()
Copy link
Contributor

Choose a reason for hiding this comment

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

So my understanding based on the comments on #129 is that this is where we would inject the code to read an external file or remote resource using the proposed ReferenceLoader?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So basically take the reference loader in the ctor, and use that if isExternal. Yeah

{
UnresolvedReference = true,
Reference = reference,
};
}

try
{
return this.currentDocument.ResolveReference(reference) as T;
Expand Down
Loading
Loading