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

Wrong documentation when nullable property is used in a field that maps to object #3518

Closed
elgleidson opened this issue Apr 16, 2020 · 15 comments
Assignees
Milestone

Comments

@elgleidson
Copy link

elgleidson commented Apr 16, 2020

First of all, the following repository has all the code (and description as well) to reproduce this problem: https://github.com/elgleidson/swagger-problem

I have the following JSON:

{
  "nonNullableField": "not null",
  "nullableField": null,
  "nonNullableObjectField": {
    "someField": "some value"
  },
  "nullableObjectField": null,
  "nonNullableList": [
    "not null"
  ],
  "nullableList": null,
  "nonNullableObjectList": [
    {
      "someField": "some value"
    }
  ],
  "nullableObjectList": null
}

Which is mapped to the following Java classes:

@Value
public class MyResponse {

  @Schema(nullable = false, description = "DO NOT map to json object and DO NOT allow null")
  private final String nonNullableField = "not null";

  @Schema(nullable = true, description = "DO NOT map to json object and allows null")
  private final String nullableField = null;

  @Schema(nullable = false, description = "map to json object and DOES NOT allow null")
  private final MyClass nonNullableObjectField = new MyClass(nonNullableField);

  @Schema(nullable = true, description = "map to json object and allows null")
  private final MyClass nullableObjectField = null;

  @ArraySchema(arraySchema = @Schema(nullable = false, description = "list that DOES NOT map to json object and DOES NOT allow null"))
  private final List<String> nonNullableList = List.of(nonNullableField);

  @ArraySchema(arraySchema = @Schema(nullable = true, description = "list that DOES NOT map to json object and allow null"))
  private final List<String> nullableList = null;

  @ArraySchema(arraySchema = @Schema(nullable = false, description = "list that map to json object and DOES NOT allow null"))
  private final List<MyClass> nonNullableObjectList = List.of(nonNullableObjectField);

  @ArraySchema(arraySchema = @Schema(nullable = true, description = "list that map to json object and allow null"))
  private final List<MyClass> nullableObjectList = null;

}

@Value
@Schema(description = "my class description")
public class MyClass {

  @Schema(description = "my class field description")
  private final String someField;

}

When I go to /v3/api-docs (or /swagger-ui.html) the following documentation is generated:

{
  "MainResponse": {
    "type": "object",
    "properties": {
      "nonNullableField": {
        "type": "string",
        "description": "DO NOT map to json object and DO NOT allow null"
      },
      "nullableField": {
        "type": "string",
        "description": "DO NOT map to json object and allows null",
        "nullable": true
      },
      "nonNullableObjectField": {
        "$ref": "#/components/schemas/MyClass"
      },
      "nullableObjectField": {
        "$ref": "#/components/schemas/MyClass"
      },
      "nonNullableList": {
        "type": "array",
        "description": "list that DOES NOT map to json object and DOES NOT allow null",
        "items": {
          "type": "string"
        } 
      },
      "nullableList": {
        "type": "array",
        "description": "list that DOES NOT map to json object and allow null",
        "nullable": true,
        "items": {
          "type": "string"
        }
      },
      "nonNullableObjectList": {
        "type": "array",
        "description": "list that map to json object and DOES NOT allow null",
        "items": {
          "$ref": "#/components/schemas/MyClass"
        }
      },
      "nullableObjectList": {
        "type": "array",
        "description": "list that map to json object and allow null",
        "nullable": true,
        "items": {
          "$ref": "#/components/schemas/MyClass"
        }
      }
    }
  },
  "MyClass": {
    "type": "object",
    "properties": {
      "someField": {
        "type": "string",
        "description": "my class field description"
      }
    },
    "description": "my class description",
    "nullable": true
  }
}

As you can see, for the fields whose types are not mapped to object the documentation is generated as expected. The same doesn't happen for nullableObjectField: the nullable property is put in MyClass definition instead of the field.

The following documentation should be generated instead:

{
  "MainResponse": {
    "type": "object",
    "properties": {
      "nonNullableField": {
        "type": "string",
        "description": "DO NOT map to json object and DO NOT allow null"
      },
      "nullableField": {
        "type": "string",
        "description": "DO NOT map to json object and allows null",
        "nullable": true
      },
      "nonNullableObjectField": {
        "$ref": "#/components/schemas/MyClass",
        "description": "map to json object and DOES NOT allow null"
      },
      "nullableObjectField": {
        "$ref": "#/components/schemas/MyClass",
        "description": "map to json object and allows null",
        "nullable": true
      },
      "nonNullableList": {
        "type": "array",
        "description": "list that DOES NOT map to json object and DOES NOT allow null",
        "items": {
          "type": "string"
        } 
      },
      "nullableList": {
        "type": "array",
        "description": "list that DOES NOT map to json object and allow null",
        "nullable": true,
        "items": {
          "type": "string"
        }
      },
      "nonNullableObjectList": {
        "type": "array",
        "description": "list that map to json object and DOES NOT allow null",
        "items": {
          "$ref": "#/components/schemas/MyClass"
        }
      },
      "nullableObjectList": {
        "type": "array",
        "description": "list that map to json object and allow null",
        "nullable": true,
        "items": {
          "$ref": "#/components/schemas/MyClass"
        }
      }      
    }
  },
  "MyClass": {
    "type": "object",
    "properties": {
      "someField": {
        "type": "string",
        "description": "my class field description"
      }
    },
    "description": "my class description"
  }
}
@webron
Copy link
Contributor

webron commented Apr 20, 2020

This is going to be challenging to investigate. Looking at the linked issue, it looks like you're using springdoc, and they sent you here because they claim to be using our processor (which they might). However, officially, we don't support Spring MVC/Boot as they use a different annotatiions to describe operations.

We're very grateful for the sample project, however, in order to isolate that the issue really is in swagger-core and not springdoc, we'd need a sample that's based on JAX-RS and not spring boot to reproduce it. Is there any chance you can produce such sample?

@bnasslahsen
Copy link

Hi @webron,

Here's a sample code (pure java) that helped us confirm the reproduce.
It's using MyResponse class mentioned by @elgleidson.

I wish it could help.

ResolvedSchema resolvedSchema = ModelConverters.getInstance()
		.resolveAsResolvedSchema(new AnnotatedType(MyResponse.class));
if (resolvedSchema.schema != null) {
	Schema schemaN = resolvedSchema.schema;
	Map<String, Schema> schemaMap = resolvedSchema.referencedSchemas;
	StringSchema stringSchema = (StringSchema) schemaMap.get("MyResponse").getProperties().get("nonNullableField");
	if (stringSchema.getNullable() == null) {
		throw new IllegalArgumentException("nonNullableField, should not be null");
	}
}

@frantuma frantuma self-assigned this Apr 21, 2020
@frantuma frantuma added this to the M1 milestone Apr 23, 2020
@frantuma frantuma modified the milestones: M1, M2 May 11, 2020
@frantuma frantuma modified the milestones: M2, M3 Jun 2, 2020
@frantuma frantuma modified the milestones: M3, M4 Jun 22, 2020
@frantuma frantuma modified the milestones: M4, M5 Jul 9, 2020
@frantuma frantuma modified the milestones: M5, M7 Aug 14, 2020
@frantuma frantuma modified the milestones: M7, M9 Sep 30, 2020
@frantuma frantuma modified the milestones: M9, M10 Oct 21, 2020
@frankruegamer
Copy link

Is there any progress on this issue?

@waage
Copy link

waage commented May 31, 2021

Any progress?

@davidmelia
Copy link

any progress?

@maxLBCarbon
Copy link

Hello :)

Any progress on this bug ?

@const
Copy link

const commented Aug 18, 2023

First thing, the expected definition like:

      "nonNullableObjectField": {
        "$ref": "#/components/schemas/MyClass",
        "description": "map to json object and allows null",
        "nullable" : true   
      }

is incorrect. Becase "$ref" replaces definition, and all sibling properties it will be ignored. See https://swagger.io/docs/specification/using-ref/ .

$ref and Sibling Elements

Any sibling elements of a $ref are ignored. This is because $ref works by replacing itself and everything on its level with the definition it is pointing at.

So, event if this is forced, the importing tools will likely ignore it.

I think expected variant is:

      "nullableObjectField": {
        "oneOf" : [{"$ref": "#/components/schemas/MyClass"}],
        "description": "map to json object and DOES NOT allow null",
        "nullable" : true   
      }

However it is not easy to implement becase the ref still wanted even if oneOf specified in swagger.

I have put extension on the container object:

  @Schema(
      description = "My dto",
      name = "MyDto,
      extensions =
          @Extension(
              name = "x-force-null",
              properties =
                  @ExtensionProperty(name = "myNullableProperty", value = "true")))
  class MyDto {
    @Schema(description = "something or null")
    OtherDto myNullableProperty;
  }

Note, extensions are ignored on ref properties as well. Then I wrote OpenApiCustomiser customizer in Spring that walks over schema and does the following:

 @SuppressWarnings("unchecked")
  public void processSchema(Schema<?> schemaModel) {
    if (schemaModel.getExtensions() != null) {
      var map = (Map<String, Object>) schemaModel.getExtensions().get("x-force-null");
      if (map != null) {
        if (schemaModel.getProperties() != null) {
          for (String property : map.keySet()) {
            var type = schemaModel.getProperties().get(property);
            if (type.get$ref() != null) {
              var t = new ComposedSchema();
              t.setNullable(true);
              t.setOneOf(List.of(type));
              schemaModel.getProperties().put(property, t);
            } else {
              type.setNullable(true);
            }
          }
        }
        if (schemaModel.getExtensions().size() == 1) {
          schemaModel.setExtensions(null);
        } else {
          schemaModel.getExtensions().remove("x-force-null");
        }
      }
    }
  }

Basically, it extension presents, it converts all ref properties mentioned in it to oneOf and then remove extension from schema.

This works for me, because with have a single consumer of the result schema that understands this variant. Result could differ for other tools.

I think the correct implementation would be if anything property-specific like description, nullable, extensions are specifed for reference type, then oneOf type should be automatically used with a single candidate, and other things should be put near oneOf. Note that even for nonNullableField the description is lost, and this is an information loss because we now do not know what is local semantics of the field as $ref specifies global semantics and syntax.

@elgleidson
Copy link
Author

elgleidson commented Apr 9, 2024

@const sorry for the late reply.

I didn't test your suggestion, but I did some things in my example to work around the issue: using Jakarta (previously Javax) annotations:

@Value
public class MyResponse {

  @NotBlank
  @Schema(nullable = false, description = "DOES NOT map to json object and DOES NOT allow null")
  private final String nonNullableField;

  @Schema(nullable = true, description = "DOES NOT map to json object and DOES allow null")
  private final String nullableField;

  @Schema(nullable = false, description = "DOES map to json object and DOES NOT allow null")
  @NotNull
  private final MyClass nonNullableObjectField;

  @Schema(nullable = true, description = "DOES map to json object and DOES allow null")
  private final MyClass nullableObjectField;

  @ArraySchema(arraySchema = @Schema(nullable = false, description = "list that DOES NOT map to json object and DOES NOT allow null"))
  @NotEmpty
  private final List<String> nonNullableList;

  @ArraySchema(arraySchema = @Schema(nullable = true, description = "list that DOES NOT map to json object and DOES allow null"))
  private final List<String> nullableList;

  @ArraySchema(arraySchema = @Schema(nullable = false, description = "list that DOES map to json object and DOES NOT allow null"))
  @NotEmpty
  private final List<MyClass> nonNullableObjectList;

  @ArraySchema(arraySchema = @Schema(nullable = true, description = "list that DOES map to json object and DOES allow null"))
  private final List<MyClass> nullableObjectList;

}

Which results in the following specification (just the important bit):

{
  "MyResponse": {
    "required": [
      "nonNullableField",
      "nonNullableList",
      "nonNullableObjectField",
      "nonNullableObjectList"
    ],
    "type": "object",
    "properties": {
      ...
    }
  }
}

Visually, I get this (red * in the required fields):
image

@elgleidson
Copy link
Author

elgleidson commented Apr 9, 2024

In regards to the $ref definition, I see it as a contradiction between its definition:

When you document an API, it is common to have some features which you use across several of API resources. In that case, you can create a snippet for such elements in order to use them multiple times when you need it. With OpenAPI 3.0, you can reference a definition hosted on any location. It can be the same server, or another one – for example, GitHub, SwaggerHub, and so on. To reference a definition, use the $ref keyword:

And its implementation:

Any sibling elements of a $ref are ignored. This is because $ref works by replacing itself and everything on its level with the definition it is pointing at. Consider this example:

components:
  schemas:
    Date:
      type: string
      format: date
    DateWithExample:
      $ref: '#/components/schemas/Date'
      description: Date schema extended with a `default` value... Or not?
      default: 2000-01-01

In the second schema, the description and default properties are ignored, so this schema ends up exactly the same as the referenced Date schema.

Their example makes things even weird: a Date could be used as "date of birth", not nullable, no defaults, in one place, but being used as a "end date", nullable, in other place.

How nobody thought that a component that's suppose to be reused in many places could:

  • be nullable in one place, but not nullable in other?
  • having a description in one place, but a different description in other?
  • having default value in one place, but a different default in other?

I hope they can review this design/implementation decision in a 3.2.x API definition.

Anyway, I appreciated the time you spent replying to my issue.

@Burtsev-Alexey
Copy link

Burtsev-Alexey commented Jul 19, 2024

Same Issue here. We have this class setup:

@Schema(title = "Payment")
public record PaymentData(
@Schema(title = "Amount paid from wallet") MoneyAmount paidWallet,
@Schema(title = "Amount paid from card") MoneyAmount paidCard) {
}
In swagger-ui we see as description for paidWallet and paidCard nor "Amount paid from wallet" nor "Amount paid from card", its some random? text from other usages of MoneyAmount in project....

@frantuma frantuma assigned micryc and unassigned frantuma Aug 27, 2024
@micryc
Copy link
Contributor

micryc commented Sep 2, 2024

@elgleidson @Burtsev-Alexey Hi, sorry for pinging but, out of curiosity, have you tried to generate 3.1 openapi specification ? Does the problem still appear ?

@micryc
Copy link
Contributor

micryc commented Sep 4, 2024

I am closing this ticket for now, feel free to reopen if there will be still any issues even with 3.1 generated spec

@micryc micryc closed this as completed Sep 4, 2024
@Burtsev-Alexey
Copy link

Burtsev-Alexey commented Sep 10, 2024

I'm not sure how to check 3.1 version. We are using Swagger from Spring Boot 3 app and it have this dependencies
image

@micryc
Copy link
Contributor

micryc commented Sep 11, 2024

@Burtsev-Alexey
To generate openapi spec 3.1 version, you have to add a custom springdoc property, in your spring-boot configuration file.
Screenshot 2024-09-11 at 08 12 29

@Burtsev-Alexey
Copy link

Burtsev-Alexey commented Sep 11, 2024

@micryc Thank you, i have switched to 3.1 and now schema is generated properly I see:
"paidWallet": {
"$ref": "#/components/schemas/MoneyAmount",
"title": "Amount paid from wallet"
},
"paidCard": {
"$ref": "#/components/schemas/MoneyAmount",
"title": "Amount paid from card"
}
It was incorrect before 3_1:
"paidWallet": {
"$ref": "#/components/schemas/MoneyAmount"
},
"paidCard": {
"$ref": "#/components/schemas/MoneyAmount"
}

The minor problem is titles are not displayed with 3_1 in swagger-ui, but this is other problem I will probably figure this out some day...

So I think this issue is resolved.

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