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

Descriptions are omitted for schema references #393

Closed
martincostello opened this issue Mar 23, 2019 · 10 comments
Closed

Descriptions are omitted for schema references #393

martincostello opened this issue Mar 23, 2019 · 10 comments

Comments

@martincostello
Copy link

If a property of a definition references another defined type via a reference, the description for the property is not written to the schema.

This means that specificity in a document for a description for a type of a property is not serialized to the schema.

This is because if the Reference property is non-null, it is the only property serialized.

https://github.com/Microsoft/OpenAPI.NET/blob/c08eb18391e42555c00297de5530503f3be318a7/src/Microsoft.OpenApi/Models/OpenApiSchema.cs#L254-L258

https://github.com/Microsoft/OpenAPI.NET/blob/c08eb18391e42555c00297de5530503f3be318a7/src/Microsoft.OpenApi/Models/OpenApiSchema.cs#L427-L431

When the schema is serialized, allowable metadata such as the description should still be serialized when a reference is present.

Expected

"definitions": {
  "Foo": {
    "description": "A foo",
    "type": "object",
    "properties": {
      "bar": {
        "$ref": "#/definitions/Bar",
        "description": "A custom Bar"
      }
    }
  },
  "Bar": {
    "description": "A bar",
    "type": "object",
    "properties": {
      "name": {
        "type": "string"
      }
    }
  }
}

Actual

"definitions": {
  "Foo": {
    "description": "A foo",
    "type": "object",
    "properties": {
      "bar": {
        "$ref": "#/definitions/Bar"
      }
    }
  },
  "Bar": {
    "description": "A bar",
    "type": "object",
    "properties": {
      "name": {
        "type": "string"
      }
    }
  }
}
@darrelmiller
Copy link
Member

@martincostello Unfortunately that is by design. The JSON Reference specification does not allow any keys to be by the side of it. The OpenAPI TSC is currently working on a new mechanism that may allow something equivalent in the future but currently it is not allowed. I know AutoREST supports it, but it is not valid OpenAPI.

@martincostello
Copy link
Author

That’s a shame.

I did try out a V2 schema validator before opening this issue, which appeared to have no issues with the description being present, as well as trying to look up the rules in the spec, but I couldn’t seem to find anything that stated it was forbidden.

@darrelmiller
Copy link
Member

@martincostello It isn't the easiest thing to find. If you see here it says:

This object cannot be extended with additional properties and any properties added SHALL be ignored.

@mikebeaton
Copy link

mikebeaton commented Jun 28, 2019

I've come across this as well. It's frustrating (specific details follow), though I do realise from the above that this is not something controlled by this project, and thank you @darrelmiller for providing the relevant info. I am not sure where else to post an additional comment that might be heard, so I hope it is okay to post here.

A string, int or bool property can have a description of what it is for, but it seems wrong that a custom Address type property, say, should not also have a description to say whether it is a home or work address.

To give a real-world example, I am using Swashbuckle.AspNetCore and it is correctly capturing the schema references where my C# API objects contain child objects, and then also capturing in the schema objects descriptions of what those reference objects are for, from my XML comments.

But then OpenAPI.NET is (correctly!) not serializing the description even though it is present in the schema, and even if I hack the code from this project (it only needs a couple of lines changed) to output descriptions into the JSON where present along with references, then of course the Swagger UI is (correctly!) ignoring them for display in the UI.

It doesn't prima facie sound hard to change the spec to allow only an optional Description field in addition to the $ref field? (So partly I was just wondering why an entire new mechanism is considered necessary for this.)

@mikebeaton
Copy link

mikebeaton commented Jun 28, 2019

See also:

There's a lot to get in to here, but it looks (2nd and 3rd refs above) as if the JSON Schema spec now allows other keywords next to $ref (at least in draft?) and it looks as if the OpenAPI spec delegates its decision on this to the JSON Schema spec?

@zxli
Copy link

zxli commented Aug 16, 2019

I feel I'm facing similar issue. I create OpenApiResponse with both description and reference set. And description is not in the serialized content. The problem is AutoRest is expecting description and will fail to generate client if description is not in the input. With this issue present, it looks like I cannot make the generated Swagger definition work in AutoRest by default.

Here is the implementation in OpenApiResponse. It clearly indicates Reference and description (likely other properties) are mutually exclusive.

    /// <summary>
    /// Serialize <see cref="OpenApiResponse"/> to Open Api v2.0.
    /// </summary>
    public void SerializeAsV2(IOpenApiWriter writer)
    {
        if (writer == null)
        {
            throw Error.ArgumentNull(nameof(writer));
        }

        if (Reference != null)
        {
            Reference.SerializeAsV2(writer);
            return;
        }

        SerializeAsV2WithoutReference(writer);
    }

@darrelmiller
Copy link
Member

darrelmiller commented Aug 18, 2019

@mikebeaton Newer versions of JSON Schema have introduced a major distinction between the notion of annotations and constraints. By doing this they have enabled merging of annotations but remove the problem of trying to figure out how to merge constraints. Descriptions are considered annotations and not constraints.

We are currently trying to figure out if we can safely adopt JSON draft-8 in OpenAPI 3.1. Maybe there will be good news at some point in the near future.

@zxli It is unfortunate that AutoREST decided to allow this. I understand why they did and I have talked with the team about this issue. Hopefully we can move past this in the near future.

@RicoSuter
Copy link

RicoSuter commented Sep 12, 2019

From OAI/OpenAPI-Specification#556 (comment)

I'd expect that this (OpenAPI 3)

"startLocation": {
    "description": "The location of the trip start.",
    "nullable": true,
    "oneOf": [
        {
            "$ref": "#/components/schemas/Location"
        }
    ]
},

or this (Swagger 2)

"startLocation": {
    "description": "The location of the trip start.",
    "allOf": [
        {
            "$ref": "#/components/schemas/Location"
        }
    ]
},

should be allowed, but eg. swagger-codegen does not allow this. Why is one of these samples not the current way to go? Or something similar to avoid $ref with additional properties?

@fearthecowboy
Copy link
Member

It is unfortunate that AutoREST decided to allow this. I understand why they did and I have talked with the team about this issue. Hopefully we can move past this in the near future.

Hey man, it was like that when I got here 🗡

@darrelmiller
Copy link
Member

Fear not @fearthecowboy , this will be allowed in 3.1 !

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

No branches or pull requests

6 participants