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

New annotation proposal: ignorable objects (@ApiParam#hidden() and @ApiModel#hidden()) #1173

Closed
who opened this issue Jun 17, 2015 · 25 comments
Labels
Milestone

Comments

@who
Copy link

who commented Jun 17, 2015

Springfox has a really convenient way of excluding things from being included by swagger's parsing stack. [ See https://github.com/springfox/springfox/blob/master/springfox-core/src/main/java/springfox/documentation/annotations/ApiIgnore.java ]

This is very useful, and I would like to recommend that something to this effect be included in the Swagger core annotation libraries.

@who
Copy link
Author

who commented Jun 17, 2015

Update:

Because of how swagger-core 1.5 now automatically includes ALL function parameters that it encounters in a resource/controller method, an annotation along the lines of the above is even more beneficial.

@who
Copy link
Author

who commented Jun 17, 2015

Here is a repro:

This method ...

    @POST
    @Path("/strings")
    @ApiOperation(value = "Send a string in the body",
        nickname = "gimmieString",
        authorizations = @Authorization(value = "basicAuth"))
    public String gimmieString(@ApiParam("theString") String theString, @Auth User user) {
        return "you sent: "+ theString;
    }

... yields this parameters spec:

...
"/strings" : {
    ...
    "parameters": [
      {
        "schema": {
          "type": "string"
        },
        "required": false,
        "description": "theString",
        "name": "body",
        "in": "body"
      },
      {
        "schema": {
          "$ref": "#/definitions/User"
        },
        "required": false,
        "name": "body",
        "in": "body"
      }
    ]
...
}
...

Notice how there are two body parameters. This creates an invalid swagger spec. I want swagger to completely ignore the @Auth User user parameter, and I do not want a User definition created.

@who
Copy link
Author

who commented Jun 17, 2015

@webron Let me know if the above is a sufficient repro.

@webron
Copy link
Contributor

webron commented Jun 18, 2015

@who - it is, to an extent, but this is a different issue than the original request. The original request may be able to provide a workaround, but this should be solved in a different way if possible. Can you open a separate ticket on it?

@webron
Copy link
Contributor

webron commented Jun 22, 2015

@who - I've added a feature in #1188 that would help overcome the specific use case you raise here (which is valid, and we should find a better way of handling it in general). Would #1188 cover your needs or can you think of other cases where @ApiIgnore would work? I'd rather not add a new annotation if the functionality can be covered by existing ones, but by all means, if it's needed, we'll consider it.

@who
Copy link
Author

who commented Jun 22, 2015

@webron #1188 would cover the majority of the over-inclusion issues I'm seeing. It'd be a welcome change!

@who
Copy link
Author

who commented Jun 22, 2015

It may also be worthwhile to add @ApiModel#hidden() in addition to the proposed @ApiParam#hidden()

@webron
Copy link
Contributor

webron commented Jun 22, 2015

Not really sure how that would work. Models are scanned if they are referenced. If those are hidden, that would break the reference and the output in general.

@webron
Copy link
Contributor

webron commented Jul 13, 2015

@who - any additional input on the @ApiModel#hidden()?

@who
Copy link
Author

who commented Jul 13, 2015

@webron

If a class is annotated with @ApiIgnore, swagger-core should not use it as a model nor reference it (in another model, parameter, etc). This is how both springfox and swagger-maven-plugin handle ignoring class-level items.

@webron
Copy link
Contributor

webron commented Jul 13, 2015

Yes, but if an operation refers to it, what happens?

@GET
@ApiOperation(...)
public Foo bar() {
}

and

@ApiModel(hidden=true)
public class Foo {
...
}

What's the bar's return type in the Swagger definition?

@who
Copy link
Author

who commented Jul 13, 2015

If you indeed wanted to @ApiIgnore Foo, you probably wouldn't create an @ApiOperation that returns it, as this is a logical conflict, if I understand correctly.

I suppose one of the following would be the way to handle the logical conflict:

  1. Return a dummy ref of type:"object"
  2. Return a ref to Foo, but don't create the model, which intentionally creates a spec that doesn't validate, causing the user to rectify the initial logical conflict.

@webron
Copy link
Contributor

webron commented Jul 13, 2015

Okay, glad I managed to explain the problem I see with the example. Not always easy to communicate ideas. On the other hand, I'm not sure I see the use case you're trying to solve. Can you show me a similar case where hiding a model would affect anything? Models are scanned if they are referred to, not just randomly, so you shouldn't end up having a model that's under definitions and not used. So when would be the case you'd want to hide it and it wouldn't be a logical conflict? Really just trying to understand so the solution would be proper.

@who
Copy link
Author

who commented Jul 13, 2015

Here's one example:

public class Foo {
  private String unignoredString;
  private Bar bar;
}
@ApiModel(hidden=true)
public class Bar {
}

Given the above, in the generated swagger spec, I would expect:

  1. Definition of Bar is not present
  2. Reference in Foo's definition to Bar is not present

@who
Copy link
Author

who commented Jul 13, 2015

Also, @dilipkrish may be able to provide some rationale from the Springfox side for having class-level ignores. I think it's mainly used to ignore controllers, but he may be able to shed some light.

See: https://github.com/springfox/springfox/blob/master/springfox-core/src/main/java/springfox/documentation/annotations/ApiIgnore.java#L28

@webron
Copy link
Contributor

webron commented Jul 13, 2015

Why not use http://docs.swagger.io/swagger-core/v1.5.0/apidocs/io/swagger/annotations/ApiModelProperty.html#hidden() on private Bar bar then? The complexity (and logical conflict risk) of adding it on the model doesn't seem to be worth it compared to the existing alternative.

Would love to get @dilipkrish's input as well of course :)

@who
Copy link
Author

who commented Jul 13, 2015

Yes, you could definitely hide at the property level, but you'd have to add that annotation any time the class is referred to across all models that use it. If you wanted an authoritative way to ensure a class is never leaked out when referred to, class-level is most assuring; it safeguards against including an unwanted model if you miss adding the annotation at the getter/field level.

@webron
Copy link
Contributor

webron commented Jul 13, 2015

That's fair, but considering there's an alternative, and that your suggestion has a risk for the logical conflict, I'm not sure we should introduce this complexity to the library right now.

@who
Copy link
Author

who commented Jul 13, 2015

That is understandable.

I'm really mainly excited for @ApiParam#hidden :). It'll cover 90%+ of the things we need to ignore. The other 10% we can work around until ignore works at the class level too.

@webron
Copy link
Contributor

webron commented Jul 13, 2015

Would you be okay with changing the title of the issue to @ApiModel#ignore()? We can keep the thread for the details, and I'll mark it as a potential future feature.

@webron
Copy link
Contributor

webron commented Jul 13, 2015

BTW, the @ApiParam#hidden() is available in the 1.5.1-SNAPSHOT at sonatype if you'd like to test it out, and of course in the previous comment I meant to write @ApiModel#hidden().

@who who changed the title New annotation proposal: @ApiIgnore New annotation proposal: ignorable objects (@ApiParam#hidden() and @ApiModel#hidden()) Jul 13, 2015
@webron webron added the Feature label Jul 13, 2015
@webron webron added this to the Future milestone Jul 13, 2015
@dilipkrish
Copy link
Contributor

My 2 💵s

The @ApiIgnore annotation we use are for hiding api operations or a collection of operations (Controller)

Personally I prefer to use annotations only as an aid to the api description and not as the source of truth. For e.g. if we need additional descriptive messages or if we need to infer something thats not immediately available via introspection. Having said that, it sometimes makes sense to have redundancy, where we want the annotation to be a tiebreaker or override a value like when the api developer needs to signal that they know what they are doing.

Coming to this specific question, it seems weird to me to have @ApiModel(hidden). Like @webron said, there might be problems with this. For one we often conflate requests and response models, so the model itself ends up being a superset of the properties. Secondly, if you change the model you have to keep the annotation in sync. Given a sufficiently large codebase this can get quite hard to reason about and untenable to find and adjust model visibilities.

@webron
Copy link
Contributor

webron commented Jul 31, 2015

@who - I believe we covered it for now. I don't think we should have the @ApiModel#hidden for now, and perhaps in the future we can come up with a general other way of fine tuned control here.

@fehguy
Copy link
Contributor

fehguy commented Aug 10, 2015

work-arounds described in the thread.

@fehguy fehguy closed this as completed Aug 10, 2015
@ghost
Copy link

ghost commented Nov 3, 2017

for people who still confused after finish reading the thread.

to hidden params in controller function

    @ApiOperation("设备更新")
    @PostMapping("/update")
    @ApiImplicitParams({
        @ApiImplicitParam(name = "dType", dataType = "DeviceType", paramType = "query", value = "设备类型", required=true),
        @ApiImplicitParam(name = "dNo", dataType = "string", paramType = "query", value = "设备号", required=true),
        @ApiImplicitParam(name = "dVer", dataType = "string", paramType = "query", value = "版本", required=true),
        @ApiImplicitParam(name = "simNo", dataType = "string", paramType = "query", value = "sim卡号", required=true),
        @ApiImplicitParam(name = "cardNo", dataType = "string", paramType = "query", value = "安全芯片号", required=true),
        @ApiImplicitParam(name = "dStatus", dataType = "DeviceStatus", paramType = "query", value = "设备状态", required=true),
        @ApiImplicitParam(name = "dDesc", dataType = "string", paramType = "query", value = "设备备注", required=true)
    })
    public ApiResponse updateById(Long id,@ApiIgnore @ApiParam(hidden = true) Device device) {
    		Device beforeUpdate = deviceRepository.findOne(id);
    		device.setCreateDate(beforeUpdate.getCreateDate());
    		device.setUpdateDate(System.currentTimeMillis()/1000);
        Device ret = deviceRepository.saveAndFlush(device);
        return RestResponseGenerator.genSuccess(ret, "更新设备"+id+"成功");
    }

@ApiIgnore @ApiParam(hidden = true) will hidden the parameter.
@ApiImplicitParams to write down your own.
Long id will be kept as a param.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants