Documenting RequestParam DTOs

[mustaphazorgati]

Because we have too many request parameter for some endpoints we decided to wrap them into dedicated DTO like so:

@GetMapping(path = "/url")
@Transactional(readOnly = true, rollbackFor = Exception.class)
public ResponseEntity<?> getListOfSomething(
  FilterParameter filterParameter,
  SortParameter sortParameter,
  PagingParameter pagingParameter) { 
    /*...*/
}

Unfortunately this way the code looks clean, but the request parameters which are contained within each DTO are not documented.

Is this currently not supported or are we doing something wrong?

[jmisur]

Thanks for the submition, let's discuss details on the PR.

[jmisur]

Hi @mustaphazorgati. I just looked into this again and realised what you are trying to achieve is actually a support for ModelAttribute without annotation. If I'm getting this right, from documentation of method arguments:

Any other argument

If a method argument is not matched to any of the earlier values in this table and it is a simple type (as determined by BeanUtils#isSimpleProperty, it is a resolved as a @RequestParam. Otherwise, it is resolved as a @ModelAttribute.

We already have a support for @ModelAttribute annotation via JacksonModelAttributeSnippet. See controller and test. This one could theoretically work without annotation if I look at the implementation logic.

So what I would propose you to do:

• Try using the model attribute configuration
• Try annotating your request DTOs with @ModelAttribute annotation

If none of this will work, or will work in a different than intended way, please share your results with us.

[mustaphazorgati]

Hey @jmisur,
thank you for your response. Indeed The JacksonModelAttributeSnippet was what we were looking for. Unfortunately that Snippet only resolves the ModelAttribute DTO. Since we have multiple ModelAttribute DTOs I extended that snippet to recognize multiple ModelAttributes. That change is backwards compatible and does not break anything. Based on this change we could further customize the Snippets for multiple Types. Furthermore I also expanded the JacksonRequestFieldSnippet in the same way.

[jmisur]

Are you able to check master on your project?

[mustaphazorgati]

I'll grab the newest snapshot version and verify it :)
talk to you shortly

[mustaphazorgati]

Everything works like a charm!
When will we get a new release?

[jmisur]

Not so fast. I regenerated documentation, and now it's outputting also specialised DTOs like Pageable as modelattributes. I'm afraid that it catches all non-annotations and non-simple-props according to docs , e.g. Errors, ServletRequest, HttpMethod, Principal, etc.

[mustaphazorgati]

good point.
Suggestion: We remove the support for non-annotated parameters and enforce the user to explicitly use the @ModelAttribute annotation?
I think that's the lesser pain than to support the catch according to the docs and implement a mechanism to explicitly exclude spring related classes.

If you agree I'll create a new PR shortly.

[jmisur]

Yes I agree. Let's rollback the last PR (to some extent) and just request users to use the annotation. It's the simplest and not really an intrusive approach.

[mustaphazorgati]

Alright. PR coming later today.

[mustaphazorgati]

Hey @jmisur,

I have a little issue. After reverting the isModelAttribute method back to verifying that the ModelAtrribute annotation is present the bahviour does not change. This is because the method isProcessedAsModelAttribute (introduced in #393) always returns true which causes every DTO (without annotation) to be accepted as a type.

I think that was not an issue until now, because the previous behavior stopped after getting the first Type. Now the snippet returns multiple types if multiple annotated parameters exist. Can you help me out here?

HttpMethod will not be documented, but Pageable will be.

[mustaphazorgati]

Hey @jmisur,

did you have a chance to look at this?

[jmisur]

I did but I need to dig a bit deeper into this.

[mustaphazorgati]

If I can help somehow let me know. :)

[jmisur]

I think I got it ^^.

[mustaphazorgati]

Hey @jmisur!
That's great to hear! - Thank you for your effort 😊

I had a look through your PR and I am a little confused.
As far as I can tell you removed the isModelAttribute method and fix some tests.
What am I missing? How does this fix the faulty documentation generation?

[jmisur]

That method was a devil in disguise. It was basically performing a supports function of ModelAttributeMethodProcessor. However it was not supposed to do it.

What we're doing now is just delegating the resolution of supporting the current argument onto the chain of handlerMethodArgumentResolvers, which determine what kind of attribute it is and if we support it. This strictly follows the documentation about processing method arguments and keeps the order intact. Because ModelAttributeMethodProcessor is usually the last one in the chain, it will also evaluate for the annotation and everything else not supported by previous handlers.

[mustaphazorgati]

Thank you for the explanation! :)
I am glad this issue is finally done

[jmisur]

Thank you for the explanation! :)
I am glad this issue is finally done

Me too. Took just a couple of months... 🙈
2.0.10 is out.

[mustaphazorgati]

Ah well. We got there eventually. Thanks for the update!