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

SpringDoc - Adding extra object's wrong property #2959

Closed
rksvtln opened this issue Apr 4, 2025 · 4 comments
Closed

SpringDoc - Adding extra object's wrong property #2959

rksvtln opened this issue Apr 4, 2025 · 4 comments
Labels
question Further information is requested

Comments

@rksvtln
Copy link

rksvtln commented Apr 4, 2025

Describe the bug

  • Similar problem with this issue but there was no follow up from the reporter, hence, I created a new one SpringDoc - Adding extra object's properties #2824
  • In addition to issue described above, it happens on all object with an attribute name starts with 'set'.
  • This is the sample object generated by springdoc: "ting" was added incorrectly
    "SampleResponse":{"type":"object","properties":{"ting":{"$ref":"#/components/schemas/SampleResponse"},"id":{"type":"string","description":"ID"},"description":{"type":"string","description":"Description"},"setting":{"type":"string","description":"Setting"}}}

To Reproduce
Steps to reproduce the behavior:

  • I'm using org.springdoc:springdoc-openapi-starter-webmvc-ui-2.2.0 (tried to upgrade to 2.6.0 as well, but the issue is still there)
  • Sample code that produces the issue:

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
public class SampleController {

  @GetMapping("/sample")
  public ResponseEntity<SampleResponse> getSampleResponse() {
      return ResponseEntity.ok(new SampleResponse().id("12345").description("sample response").setting("sample setting"));
  }
}

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeName;
import io.swagger.v3.oas.annotations.media.Schema;

@JsonTypeName("sampleResponse")
public class SampleResponse {

  private String id;
  private String description;
  private String setting;

  public SampleResponse id(String id) {
      this.id = id;
      return this;
  }

  @Schema(name = "id", description = "ID", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("id")
  public String getId() {
      return id;
  }

  public void setId(String id) {
      this.id = id;
  }

  public SampleResponse description(String description) {
      this.description = description;
      return this;
  }

  @Schema(name = "description", description = "Description", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("description")
  public String getDescription() {
      return description;
  }

  public void setDescription(String description) {
      this.description = description;
  }

  public SampleResponse setting(String setting) {
      this.setting = setting;
      return this;
  }
  @Schema(name = "setting", description = "Setting", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("setting")
  public String getSetting() {
      return setting;
  }

  public void setSetting(String setting) {
      this.setting = setting;
  }
}

The sampleResponse in openapi json:
"SampleResponse": { "type":"object", "properties":{ "ting": { "$ref":"#/components/schemas/SampleResponse"}, "id": {"type":"string","description":"ID"}, "description": {"type":"string","description":"Description"}, "setting":{"type":"string","description":"Setting"} } }

Expected behavior

  • The incorrect extra field should not be shown

Additional context

  • I'm still using SpringBoot 3.3.x, so I cannot upgrade to latest at the moment
  • I'm using openapi-generator to generate the objects (the object above is an example how the object were generated)
@Mattias-Sehlstedt
Copy link

Hi @rksvtln,

First I will format the response so it is easier for a reader to interpret it.

{
  "SampleResponse": {
    "type": "object",
    "properties": {
      "ting": {
        "$ref": "#/components/schemas/SampleResponse"
      },
      "id": {
        "type": "string",
        "description": "ID"
      },
      "description": {
        "type": "string",
        "description": "Description"
      },
      "setting": {
        "type": "string",
        "description": "Setting"
      }
    }
  }
}

The issues stems from how swagger-core introspects methods to generate schema properties. They do not only look at the @Schema annotations, but also certain types of public methods in general.

I could for example add

public boolean isName() {
      return false;
}

to your class, and that would add a new property "name" to the schema. This is due to them removing known prefixes such as "get, set, is" to derive the parameter's name. This will of course have odd consequences when the name is setting, since set will be removed.

You can play around with it yourself by using

ResolvedSchema resolvedSchema = ModelConverters.getInstance(true)
                .resolveAsResolvedSchema(
                        new AnnotatedType(SampleResponse.class).resolveAsRef(false));

A minimal case that would highlight this is if we reduce your class to

public class SampleResponse {
    public void setId(String id) {
    }

    public void setDescription(String description) {
    }

    public SampleResponse setting(String setting) {
        return this;
    }

    public boolean isName() {
        return false;
    }
}

then the generated schema for that class is

{
  "SampleResponse": {
    "type": "object",
    "properties": {
      "ting": {
        "$ref": "#/components/schemas/SampleResponse"
      },
      "id": {
        "type": "string"
      },
      "description": {
        "type": "string"
      },
      "name": {
        "type": "boolean"
      }
    }
  }
}

@rksvtln
Copy link
Author

rksvtln commented Apr 8, 2025

Thanks @Mattias-Sehlstedt for the explanation!
So the fix for this is to avoid creating these kind of public methods (if possible), or maybe using custom ModelConverter, but it means I have to explicitly define which classes have these "set" in their methods intentionally, right?

@Mattias-Sehlstedt
Copy link

I would avoid having methods that are not prefixed with get/set, since the introspection seems to interpret anything that starts with set as being a property definition for the schema (so if we do not follow the get/set prefix, we always have the risk of having a broken schema if one work accidentally starts with set....

From my testing it looks like "thing" is dropped from the properties if you for example rename the method setting to getSetting. I.e., the introspection is "clever" and assumes that all variables that can be set are values that should be (de)serialized later when the model is received/sent. I.e., it should be present in the schema since it is a property that the client might send/receive. If it is only a getter it might be a read-only property that should not be exposed.

If your design requires you to have methods that affect the generated schema in an odd way (for example, I am interpreting your methods as being a "parameter-by-parameter-builder"), then I would suggest investigating if having @Schema(hidden = true) resolves the issue. E.g.

@Schema(hidden = true)
public SampleResponse setting(String setting) {
  return this;
}

for the methods that are just part of the builder pattern.

I would not use a custom ModelConverter, since in my experience that is an extreme overhead to implement, and you also have to maintain it over time. There are instead much easier and maintainable solutions, e.g., the one above: or to maybe delegate the object instantiation/construction to a separate class, so that the API-class only contains information tied to how data is (de)serialized and annotations that guides the OpenAPI-schema representation.

@bnasslahsen
Copy link
Collaborator

Thank you @Mattias-Sehlstedt for your clear explanation.

@bnasslahsen bnasslahsen added the question Further information is requested label Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants