No $ref when the required attribute of the schema is used

[agoncal]

You can find the code here : https://github.com/agoncal/agoncal-bug-openapi

When documenting the OpenAPI with a required schema attribute set to either true or false, the $ref is not generated.

Take the following POJO:

@Schema
public class Fruit {

    public String name;
    public String description;

    // constructors
}

And the following REST endpoint with no required attribute in @APIResponse:

@Path("/hello")
public class ExampleResource {

    @GET
    @Produces(MediaType.APPLICATION_JSON)
    @APIResponse(
            content = @Content(mediaType = MediaType.APPLICATION_JSON,
                    schema = @Schema(implementation = Fruit.class)))
    public Response hello() {
        return Response.ok(new Fruit("Banana", "Yellow")).build();
    }
}

You get the following generated contract (notice the $ref)

paths:
  /hello:
    get:
      responses:
        default:
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Fruit'
components:
  schemas:
    Fruit:
      type: object
      properties:
        description:
          type: string
        name:
          type: string

Now, add the required attribute in @Schema (to either true or false):

@Path("/hello")
public class ExampleResource {

    @GET
    @Produces(MediaType.APPLICATION_JSON)
    @APIResponse(
            content = @Content(mediaType = MediaType.APPLICATION_JSON,
                    schema = @Schema(implementation = Fruit.class, required = false)))
    public Response hello() {
        return Response.ok(new Fruit("Banana", "Yellow")).build();
    }
}

The generated contract does not have a $ref anymore (the fruit components is still generated though):

paths:
  /hello:
    get:
      responses:
        default:
          content:
            application/json:
              schema:
                type: object
                properties:
                  description:
                    type: string
                  name:
                    type: string
components:
  schemas:
    Fruit:
      type: object
      properties:
        description:
          type: string
        name:
          type: string

[gastaldi]

According to

smallrye-open-api/core/src/main/java/io/smallrye/openapi/runtime/util/JandexUtil.java

Lines 441 to 450 in e617fc7

	/**
	* Returns true if the given @Schema annotation is a simple class schema. This means that
	* the annotation only has one field defined, and that field is "implementation".
	*
	* @param annotation AnnotationInstance
	* @return Is it a simple class @Schema
	*/
	public static boolean isSimpleClassSchema(AnnotationInstance annotation) {
	return annotation.values().size() == 1 && hasImplementation(annotation);
	}

Which is used in

smallrye-open-api/core/src/main/java/io/smallrye/openapi/runtime/io/schema/SchemaFactory.java

Line 221 in f640f55

if (JandexUtil.isSimpleClassSchema(annotation)) {

This is an expected behavior. But is that correct given that the annotation provided is using the default values?

[gastaldi]

/cc @MikeEdgar

[MikeEdgar]

@agoncal, @gastaldi - thank you for opening and initiating the research on this. I agree that this is not ideal behavior and should be improved.

As you've noted, specifying any attribute other than implementation causes the scanner to consider the @Schema to be customized at the point of reference and no longer eligible to be used as a simple $ref. I'll take a look at this and see if there is a simple solution.

[MikeEdgar]

This situation can be handled simply, but the case where a non-default value is given would still result in this behavior. I think that is generally good because implementation + additional properties effectively means a merging of the two, with the direct reference no longer usable.

What are your thoughts on this?

[gastaldi]

What if you (blindly) merge the two schemas without worrying about the number of properties defined? Would that produce an invalid output?

[MikeEdgar]

The situation where there are two (or more) @Schemas that use the same implementation is the issue. If one uses only implementation and the other uses implementation plus some other properties, it would be incorrect to merge everything together and have both schemas use a $ref to that result.

[agoncal]

And what about having an implementation attribute in @Content ? Most of the time that's what I want to do. And in this case, if implementation is not enough, if I really need to customise stuff, then I know I can use @Schema instead (on a side note, I think @Schema does too many things, from annotating POJOs, attributes... and also content). implementation for simple cases, @Schema for more complex ones. WDYT ?

[MikeEdgar]

@agoncal - those are questions about the specification. That being said, in version 2.0 there will be two new annotations that may help - @RequestBodySchema and @APIResponseSchema.

The more I think about this issue, the more I lean away from attempting to treat property values that are equal to the annotation defaults the same as unspecified annotation properties. We have usually considered values that are specified to be the user/developer declaring that the property should be serialized in the resulting OpenAPI result. This is a challenging situation because using required in @Schema at this location doesn't result in any sort of output so it appears that the final result should have just been a reference (and I am sure there are others like that).

If we treat specified values that are equal to the defaults as if they were unspecified, we may immediately run into trouble where the app developer was expecting values in the output that no longer appear.

@phillip-kruger , @EricWittmann - what are your thoughts on this topic?

[phillip-kruger]

I'll be honest, I have no idea... I don't understand the issue at all... Just looking at the original issue logged, it seems like adding required should not change the schema much, but I don't have all the context and background you have...

[MikeEdgar]

The idea is this - if I have two @Schemas that reference the same POJO class without any additional customization, the resulting output should use a $ref to point to the schema under #/components/schemas. When the @Schema is customized for one of them, e.g. setting nullable to true/false, the ability to reference the general schema is "broken" since we cannot apply that customization generally.

The question with this issue is, if a user provides a customization to @Schema and the value is the same as the annotation's default and it does not result in a change to the output, should the scanner keep track of that and still allow the $ref to be used?

[phillip-kruger]

Ok , so is the required on the POJO ? I though it's on the parameter ? If on the parameter, the ref to the schema object is the same, it's just mandatory in this parameter.... Or am I missing the point ?

[agoncal]

To be honest, I set required to true because I thought the schema was mandatory. What I found really strange was when I set it to the default (false) and I had the same behaviour. So, if I use the default (which is useless, but just to make the point), I would expect the default behaviour, so having a $ref.

[MikeEdgar]

Suppose I have two @Schema annotation that reference the same class and specify nothing else. Whether the @Schema is embedded in another annotation or on a field/parameter generally doesn't matter.

@Schema(implementation = MyBean.class)
Object bean1;
@Schema(implementation = MyBean.class)
Object bean2;

The scanner "knows" that it can create an entry in #/components/schemas and use a $ref for both of them. If one of them is different (below), the scanner presently will combine the schema of MyBean with any other properties given in the annotation. This now makes the schema of bean1 different than the schema of MyBean and prevents usage of the $ref that would still be used for bean2.

@Schema(implementation = MyBean.class, nullable = false) // Explicitly giving the default value
Object bean1;
@Schema(implementation = MyBean.class)
Object bean2;

Regarding the question of how default values should be handled, the scanner will only output annotation properties that have been specified in the code. Even if a property is the same as the default value, its presence is taken as a request by the application to display that value in the output, default or not. At the moment, the scanner actually doesn't know anything about the annotations' default values. If we change that, then we will break many applications that are currently specifying values equal to the defaults via annotations where those properties would no longer appear in the resulting output.

I hope I'm making sense - happy to keep discussing if not :-D

[jmini]

$ref is really limited with OpenAPI 3.0.x. You are not allowed to place any other attribute next to $ref.

paths:
  /hello:
    get:
      responses:
        default:
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Fruit'
                required: true

is not valid.

See OAI/OpenAPI-Specification#556.

---

This will be improved with OpenAPI 3.1.x where usage of $ref is more flexible.

[MikeEdgar]

@agoncal , @gastaldi , @jmini - I think a good balance here would be to allow the $ref to be generated when the only attributes of @Schema used are those that do not change the contents of the schema (regardless of their value). Specifically, name and required. The name is only used as a key in the components map, and required adds the property name to the containing schema's list of required properties.

Thoughts?

[agoncal]

@MikeEdgar yes, I think it's a good balance... but documentation has to be clear. Because when I am in front of my IDE, hit CTRL+SPACE and see all the possible attributes of an annotation, it's not totally clear that using some of those attributes will totally change the behaviour.

So +1 because it's the best/easiest path