Skip to content

Optional subsections are not aware that they are optional. #785

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

Closed
ddaaac opened this issue Feb 9, 2022 · 6 comments
Closed

Optional subsections are not aware that they are optional. #785

ddaaac opened this issue Feb 9, 2022 · 6 comments

Comments

@ddaaac
Copy link
Contributor

ddaaac commented Feb 9, 2022

Hello, I want to document the following payload.

{
  "list" : [ {
    "necessary" : "first",
    "optional" : {
      "field" : "b"
    }
  }, {
    "necessary" : "second"
  } ]
}

list[].optional field is literally an optional field.
And here is my documentation code.

responseFields(
        subsectionWithPath("list[]").description("list"),
),
responseFields(
        beneathPath("list[]").withSubsectionId("sub1"),
        fieldWithPath("necessary").description("necessary"),
        subsectionWithPath("optional").description("optional").optional(),
),
responseFields(
        beneathPath("list[].optional").withSubsectionId("sub2"),
        fieldWithPath("field").description("field").optional(),
)

And I got an exception.

list[].optional identifies multiple sections of the payload and they do not have a common structure. The following non-optional uncommon paths were found: [list[].optional]

I'm guessing it's because the list[].optional path doesn't have a clue that it is optional
(because the fact that list[].optional should be optional appears in the list[] path).

So, is there any feature like beneathPath("path").optional()?

I'm using spring-restdocs-mockmvc:2.0.6

@wilkinsona
Copy link
Member

I'm not sure that it makes sense for a subsection to be optional. If it were optional and it wasn't present, REST Docs would have no way of knowing that you had correctly documented the subsection's fields.

Is there a reason why you're using beneathPath as much as you are? Given the payload the you've shown, I would rather document it as a single set of fields.

@wilkinsona wilkinsona added the status: waiting-for-feedback Feedback is required before progress can be made label Feb 9, 2022
@ddaaac
Copy link
Contributor Author

ddaaac commented Feb 9, 2022

If it were optional and it wasn't present, REST Docs would have no way of knowing that you had correctly documented the subsection's fields.

{
  "list" : [ {
    "necessary" : "first"
  }, {
    "necessary" : "second"
  } ]
}
responseFields(
        subsectionWithPath("list[]").description("list"),
),
responseFields(
        beneathPath("list[]").withSubsectionId("sub1"),
        fieldWithPath("necessary").description("necessary"),
        fieldWithPath("optional").description("optional").type("String").optional(),
),

In the code above there is an "optional" field, but not in the payload.. But this test passed.
Similarly for subsection, it would be nice to have a feature to pass if it is optional.


Is there a reason why you're using beneathPath as much as you are? Given the payload the you've shown, I would rather document it as a single set of fields.

This is only an example for reproducing. In the actual code, the structure is more complex, so I think that the beneathPath is more suitable.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback Feedback is required before progress can be made labels Feb 9, 2022
@wilkinsona
Copy link
Member

As I said above, I'm not sure how it would work. It's straightforward for a field to be optional as it's a single point in the JSON. For an entire subsection things are more complicated. The subsection needs to be extracted and then documented. When that subsection appears multiple times in the JSON and has a different structure, there's no obvious way for REST Docs to know which occurrence it should extract. That's why you're getting the exception at the moment.

Indicating that a subsection's optional doesn't help to answer the question about which subsection should be extracted and documented. Should it be empty content from the array where optional is absent or should it be the content from the array where optional is present? There's also a potential scenario where optional is present in two entries in an array but with different structure. Which one should be selected in that case?

I am not convinced that it's possible for REST Docs to provide an API that would allow you to provide answers to these questions in a concise and sufficiently flexible manner. The subsection extraction is implemented using a strategy interface, PayloadSubsectionExtractor. As things stand, I think your best option is to implement your own extractor that answers the questions in a way that best meets your specific needs.

@wilkinsona wilkinsona added status: waiting-for-feedback Feedback is required before progress can be made and removed status: feedback-provided Feedback has been provided labels Feb 9, 2022
@ddaaac
Copy link
Contributor Author

ddaaac commented Feb 9, 2022

In fact, above code worked fine until version 2.0.5.
I was trying to migrate to version 2.0.6, and the test didn't pass, so I was looking for the reason.

I finally found that 2b5eab3 commit(absent item causes all present fields to be identified as uncommon) changed the manner it works.

Ok... If this was intended, it seems that I can't use subsection...
Thanks for the kind reply 😀

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback Feedback is required before progress can be made labels Feb 9, 2022
@wilkinsona
Copy link
Member

You were relying on a bug, unfortunately.

A wildcard selecting multiple keys in a map is very similar to an array in that we're then dealing with multiple parts of the payload that need to have the same structure for them to be documented accurately. Differing structures were being missed in some cases and throwing unhelpful errors in others. The changes for #715 mean that differing structures should now always be identified and the error message should be more helpful.

Unfortunately, I don't think the behaviour you were relying on can be reinstated without regressing that fix.

@ddaaac
Copy link
Contributor Author

ddaaac commented Feb 9, 2022

Thank you :)

I will fix my code.

@ddaaac ddaaac closed this as completed Feb 9, 2022
@wilkinsona wilkinsona removed status: waiting-for-triage Untriaged issue status: feedback-provided Feedback has been provided labels Feb 9, 2022
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

3 participants