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

Provide an option for a field to be hidden from the documentation while still being validated #747

Open
sherzinger opened this issue Sep 24, 2021 · 1 comment

Comments

@sherzinger
Copy link

Hi,

Let's say we have a field description like this:

fieldWithPath("foo")
          .type(JsonFieldType.STRING)
          .description("some description")

If the returned value is "foo": null, then an exception is being thrown because null is not considered a STRING.
In such cases, we can mark the field as .optional(), signalling that a String may be provided, but can also be null.

The problem I noticed is that .ignored() also gets rid of this exception. IMHO that shouldn't happen, because the returned value (null) still violates the field description. In other words: the value is still optional, and hiding it from the docs does not change that fact.

In short, for a nullable field "foo", which should not be included in the docs, I'd expect this syntax:

fieldWithPath("foo")
          .ignored()
          .optional()
          .type(JsonFieldType.STRING)
          .description("some description")

Of course all of the above is a matter of how we interpret what it means for a description to be ignored.
If the above is expected behavior, maybe it would make sense to split "hiding from docs" and "ignoring type" into separate fields, so we can do this fieldWithPath("foo").hide().optional() (hide, but still validate) ?

@sherzinger sherzinger changed the title .ignore() should not preceed .optional() .ignored() should not preceed .optional() Sep 24, 2021
@wilkinsona
Copy link
Member

Thanks for the suggestion.

To provide a bit of background, ignored() and optional() have been implemented purely from the perspective of documentation generation. If a field is ignored(), that is to say it's not going to appear in the documentation at all, the type of its value, whether or not it's always present, etc, becomes irrelevant from that perspective. Given this, the behaviour that you have observed is things working as intended.

What you have suggested shifts the perspective a bit to also using REST Docs for validation of fields that won't appear in the documentation. I can see the benefit in supporting that, but it would have to be done alongside the existing support for ignored() as you've suggested with hide(). This would allow users who are making use of the current behaviour to continue to use ignored(). Those who want a field to be hidden from the documentation but to still be validated could then use a new hide() or hidden()` method.

I do wonder if adding support for hide() or hidden() would create some confusion as you'd end up with code that's purely for validation intermingled with code that's documenting your API. On the one hand, that's probably quite a concise way of performing that validation. On the other hand, it may be something that's better done separately, using MockMvc's various matchers, for example.

I think we should leave this issue open for a while and see how much interest there is from other users. Thanks again for the suggestion.

@wilkinsona wilkinsona changed the title .ignored() should not preceed .optional() Provide an option for a field to be hidden from the documentation while still being validated Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants