RicoSuter / NSwag

The Swagger/OpenAPI toolchain for .NET, ASP.NET Core and TypeScript.
http://NSwag.org
MIT License
6.69k stars 1.24k forks source link

Wrong $ref relative path generated for properties when Schema is injected manually #1127

Open potocnika opened 6 years ago

potocnika commented 6 years ago

I've created a demo project to illustrate the problem: https://github.com/potocnika/swagger-tester

.NET Core 2.0 / Kestrel / VS2017

What we're attempting to do:

  1. Have a /schema endpoint overload which serves the schema for given endpoint - this is working fine with NJsonSchema
  2. Using NSwag, generate a swagger with Swagger3 UI that flattens parametrised endpoints - problem

In the demo project, this is signified by the following endpoints:

When generating swagger documentation, we're using hooks to flatten {type} into concrete types:

All concrete types for {type} inherit from a base abstract class (Animal)

Everything is working fine with one complication - the UI shows errors for $ref properties and looking into the generated swagger.json (/swagger/swagger.json) shows that the $ref properties are rendered as for example "#/paths//animals/dog/Get/responses/200/schema/definitions/Dog" instead of "#/definitions/Dog"

Testing this on the equivalent /animals/dog/schema - this is generated correctly using exact same json contract and settings. Yet the paths are relative to the whole document when that same schema is manually injected into the swagger specification within hooks. I've tested the problem by using NSwag Studio and replacing all $ref properties so that the paths are relative to the schema and not to the swagger document. When this is done, the swagger document validates correctly.

Is this a problem with swagger specification? Should $ref paths relative to the swagger document be resolved or should they only be relative to individual schema?

I looked through the NSwag to see if rendering of the references can be modified to no avail. When debugging, the .Reference of the Json node exists and is set correctly (to expected Type). The problem seem to be when rendering the node, the value is somehow change to point to document root.

potocnika commented 6 years ago

Hi @RSuter , any update on this? It's still an issue.

elmore-inf commented 6 years ago

I think it might work if whatever is creating the JSON Pointer ref escaped the slashes in the path key, and also lower cased the http verb (Get)

#/paths//animals/dog/Get/responses/200/schema/definitions/Dog becomes #/paths/~1animals~1dog/get/responses/200/schema/definitions/Dog

Escaping slashes as per https://tools.ietf.org/html/rfc6901

This clears the errors (cant comment on whether the pointer should be to another section of the JSON).

RicoSuter commented 6 years ago

All this “low-level” schema/path handling stuff is handled in NJS: https://github.com/RSuter/NJsonSchema/blob/master/src/NJsonSchema/JsonPathUtilities.cs

RicoSuter commented 6 years ago

So we need to fix this there, e.g. by first scanning the definitions instead of paths

RicoSuter commented 6 years ago

... ah i think you just need to add the injected schema also to the root definitions and remove it from the other definitions. This way the path is correctly resolved...

You could also use the JsonSchemaVisitor to remove all inner schemas from the definitions and insert it into the definitions of the swagger document:

https://github.com/RSuter/NJsonSchema/blob/master/src/NJsonSchema/Visitors/JsonSchemaVisitorBase.cs

elmore-inf commented 6 years ago

Thanks @RSuter - thats a lot of help!

I think the problem occurs when switching out the response schema eg

     // previously the abstract Animal type - want to replace with the concrete Dog type
     response.Schema = JsonSchema4.FromTypeAsync(typeof(Dog), settings).Result;

this would cause the generation of an inner definitions section, and the JSON Pointer generated to refer to it is malformed as detailed above.

As you mentioned, a work around could be to populate the root definitions and get the $ref to point to that instead. That would sidestep the parameter escaping problem.

Is there a better way to replace the return type? Or could you point me to an example of using the JsonSchemaVisitor?

RicoSuter commented 6 years ago

In scenarios like that its important to not use JsonSchema4.FromTypeAsync but the more controllable generator and resolver class so that definitions are added to the correct location and types not generated multiple times..

RicoSuter commented 6 years ago

... this clearly needs more documentation.

RicoSuter commented 6 years ago

Where do you assign response.Schema in your code? In an operation processor?

elmore-inf commented 6 years ago

That makes sense - I was looking for an earlier hook in because modifying the schema after its generated didnt feel right.

That schema was being reassigned in a Document Processor - hence the schema wrestling.

Im a little confused about the generator/validator naming conventions - are you suggesting jumping right up to implementing a AspNetCoreToSwaggerGenerator type generator or using SwaggerJsonSchemaGenerator and SwaggerSchemaResolver (or something else) from within an Operation Processor?

Appreciate the pointers here - once I'm done perhaps I could contribute a write-up to the docs?

RicoSuter commented 6 years ago

In the document processor, you get the current context and there you get the current generator and resolver...