Incorrect logic of @ModelAttribute support

[m-titov]

See #368
The @RequestHeader shouldn't be treated as request parameter. For example,

@GetMapping("/a")
    void m(@RequestHeader("b") SomeClass someClass,
                @Validated(ValidationSequence.class) SomeRequest someRequest) {
 
    }

[jmisur]

Thanks for reporting, some problems with this troublesome annotation was expected. We'll take a look.

[AnWeber]

sorry for the inconvenience. I will try to reproduce the issue. Are you using the snippet with HandleMethodArgumentresolvers set or without?

[m-titov]

Are you using the snippet with HandleMethodArgumentresolvers set or without?

Without. I have only custom class SomeClass and custom class that implements org.springframework.core.convert.converter.Converter<String, SomeClass>

[AnWeber]

I tested your example in my own test project (jdk11). But I could not reproduce the issue. Please provide more information how to produce the issue. I have attached the generated modelattribute.adoc fil: auto-modelattribute.adoc.txt

[m-titov]

SomeClass and SomeRequest should be custom classes. For example:

public class SomeClass {

    private final String someHeader;

    public SomeClass(String someHeader) {
        this.someHeader = someHeader;
    }

    public String getSomeHeader() {
        return someHeader;
    }
}

@Component
public class SomeClassConverter implements Converter<String, SomeClass> {

    @Override
    public SomeClass convert(String source) {
        return new SomeClass(source);
    }
}

public class SomeRequest {

    /**
     * Defines some text
     */
    private String some;

    public String getSome() {
        return some;
    }
}

@RestController
public class SomeController {

    @InitBinder
    public void initBinder(WebDataBinder binder) {
        binder.initDirectFieldAccess();
        binder.registerCustomEditor(String.class, new StringTrimmerEditor(true));
    }

    /**
     * Some description
     *
     * @param someClass   This is some header
     */
    @GetMapping("/some")
    void some(@RequestHeader("someHeader") SomeClass someClass,
              SomeRequest someRequest) {

    }
}

[AnWeber]

I changed my example to using SomeController. But I can not reproduce the issue. Can you please provide your configuration for creating the MockMvc.

[m-titov]

I'm sorry. I was mistaken I use

var resolvers = context.getBeansOfType(HandlerMethodArgumentResolver.class);
...
//            SnippetRegistry.AUTO_REQUEST_HEADERS,
...
AutoDocumentation.modelAttribute(resolvers.values()),

See:

@SpringBootTest
@AutoConfigureMockMvc
@RunWith(SpringJUnit4ClassRunner.class)
public class SomeControllerIT  {

    @Autowired
    ObjectMapper objectMapper;

    @Autowired
    protected MockMvc docMockMvc;

    @Autowired
    private WebApplicationContext context;

    @Rule
    public JUnitRestDocumentation restDocumentation = new JUnitRestDocumentation(resolveOutputDir());

    @Before
    public void setupMVC() {

        var resolvers = context.getBeansOfType(HandlerMethodArgumentResolver.class);

        List<String> snippetNames = Arrays.asList(
            SnippetRegistry.AUTO_AUTHORIZATION,
            SnippetRegistry.AUTO_PATH_PARAMETERS,
            SnippetRegistry.AUTO_REQUEST_PARAMETERS,
//            SnippetRegistry.AUTO_REQUEST_HEADERS,
            SnippetRegistry.AUTO_REQUEST_FIELDS,
            SnippetRegistry.AUTO_MODELATTRIBUTE,
            SnippetRegistry.AUTO_RESPONSE_FIELDS,
            SnippetRegistry.CURL_REQUEST,
            SnippetRegistry.HTTP_RESPONSE
        );

        docMockMvc = webAppContextSetup(context).
            alwaysDo(JacksonResultHandlers.prepareJackson(objectMapper)).
            alwaysDo(commonDocument())
            .apply(documentationConfiguration(restDocumentation)
                .uris()
                .withScheme("http")
                .withHost("localhost")
                .withPort(8080)
                .and()
                .snippets()
                .withDefaults(
                    CliDocumentation.curlRequest(),
                    HttpDocumentation.httpRequest(),
                    HttpDocumentation.httpResponse(),
                    AutoDocumentation.requestFields(),
                    AutoDocumentation.responseFields(),
                    AutoDocumentation.pathParameters(),
                    AutoDocumentation.requestParameters(),
                    AutoDocumentation.modelAttribute(resolvers.values()),
                    AutoDocumentation.description(),
                    AutoDocumentation.methodAndPath(),
                    AutoDocumentation.requestHeaders(),
                    AutoDocumentation.sectionBuilder()
                        .snippetNames(snippetNames)
                        .skipEmpty(false)
                        .build()
                )
            ).build();
    }

    RestDocumentationResultHandler commonDocument() {
        return document("{class-name}/{method-name}",
            Preprocessors.preprocessResponse(
                ResponseModifyingPreprocessors.replaceBinaryContent(),
                ResponseModifyingPreprocessors.limitJsonArrayLength(objectMapper),
                Preprocessors.prettyPrint())
        );
    }

    String resolveOutputDir() {
        String outputDir = System.getProperties().getProperty(
            "org.springframework.restdocs.outputDir");
        if (outputDir == null) {
            outputDir = "build/generated-snippets";
        }
        return outputDir;
    }

    @Test
    public void shouldDo() throws Exception {
        //given
        var someHeaderValue = "someHeaderValue";
        var someRequestText = "someRequestText";

        //when
        var resultActions = docMockMvc.perform(get(
            "/some",
            someRequestText).header("someClass", someHeaderValue));

        //then
        resultActions.
            andDo(print()).
            andExpect(status().isOk());
    }

}

However with

SnippetRegistry.AUTO_REQUEST_HEADERS,
...
AutoDocumentation.modelAttribute(null),

it doesn't document query params:

And with

var resolvers = context.getBeansOfType(HandlerMethodArgumentResolver.class);
...
            SnippetRegistry.AUTO_REQUEST_HEADERS,
...
AutoDocumentation.modelAttribute(resolvers.values()),

it produces incorrect Query parameters section:

[AnWeber]

Without specifying the HandleMethodArgumentResolvers (AutoDocumentation.modelAttribute(null),) you need to explicitly set @ModelAttribute.

If you set HandleMethodArgumentResolvers all Arguments which are not supported by any HandleMethodArgumentResolver are documented. In your example the Array resolvers is empty. You need to explicitly create a Mock of HandleMethodArgumentResolver which supports @RequestHeader. Using var resolvers = context.getBeansOfType(HandlerMethodArgumentResolver.class); is not working like expected.

[m-titov]

Without specifying the HandleMethodArgumentResolvers (AutoDocumentation.modelAttribute(null),) you need to explicitly set @ModelAttribute.

In such case with

...
AutoDocumentation.modelAttribute(null),
...
.snippetNames(...).skipEmpty(false)
...

it produces the Query parameters section twice:

[m-titov]

In your example the Array resolvers is empty. You need to explicitly create a Mock of HandleMethodArgumentResolver which supports @RequestHeader. Using var resolvers = context.getBeansOfType(HandlerMethodArgumentResolver.class); is not working like expected.

Hm.. How about:

@Autowired
RequestMappingHandlerAdapter requestMappingHandlerAdapter;
...
 var resolvers = requestMappingHandlerAdapter.getArgumentResolvers();
...
AutoDocumentation.modelAttribute(resolvers),
...

? Should it work without @ModelAttribute? In such case it doesn't document Query parameters

[jmisur]

Hi @m-titov, I also couldn't reproduce issues mentioned here. The best would be to create an example (feel free to use samples project in SARD repo) and create a PR where this is clearly failing.

[originalrusyn]

Hi @jmisur
I've reproduced issue with two 'Query Parameters' sections on the same page here https://github.com/ScaCap/spring-auto-restdocs/pull/386/files

[jmisur]

Hi this should be fixed in latest 2.0.9-SNAPSHOT. Can you evaluate?

[originalrusyn]

Yes, it looks good to me now