spring-projects / spring-restdocs

Test-driven documentation for RESTful services
https://spring.io/projects/spring-restdocs
Apache License 2.0
1.16k stars 736 forks source link

POST missing parameters if body is also provided #239

Closed scholarsmate closed 8 years ago

scholarsmate commented 8 years ago

Here's the test case:

  @Test
  public void postHello()
    throws Exception
  {
    mockMvc.perform(post(buildApiPath("tests/hello")).param("name", "World").content("I have lots to share"))
        .andExpect(status().isOk())
        .andExpect(content().string("Hello World. Here is your content: I have lots to share"))
        .andDo(document("post-hello"));
  }

Here's the generated curl snippet:

[source,bash]
----
$ curl 'https://localhost:9070/api/v1/tests/hello' -i -X POST -d 'I have lots to share'
----[source,http]
----
POST /api/v1/tests/hello HTTP/1.1
Host: localhost
Content-Length: 20
Content-Type: application/x-www-form-urlencoded

I have lots to share
----[source,http]
----
HTTP/1.1 200 OK
Content-Type: text/plain
Content-Length: 55

Hello World. Here is your content: I have lots to share
----

Swagger gets it right:

curl -X POST --header 'Content-Type: application/json' --header 'Accept: text/plain' -d 'I have lots to share' 'http://192.168.90.70:9070/api/v1/tests/hello?name=World'

wilkinsona commented 8 years ago

I'm not sure that Swagger has got it right. It looks to me like the Content-Type header is wrong as I have lots to share isn't valid JSON.

Can you describe exactly what sort of request you expect to be produced by the MockMvc calls you have made? I'm particularly interested in what you expect the Content-Type to be and how you expect I have lots to share to be included in the request.

When you mix content and request parameters using curl, it defaults to an application/x-www-form-urlencoded request with the request parameters in the path and I have lots to share form URL encoded in the body.

wilkinsona commented 8 years ago

If you provide the query parameters in the URL, then I think REST Docs does the right thing. For example:

mockMvc.perform(post("/?foo=bar").content("Content")).andExpect(status().isOk())
        .andDo(document("curl-snippet-with-posted-query-string-and-content"));

curl

[source,bash]
----
$ curl 'http://localhost:8080/?foo=bar' -i -X POST -d 'Content'
----

HTTP request

[source,http,options="nowrap"]
----
POST /?foo=bar HTTP/1.1
Host: localhost
Content-Length: 7
Content-Type: application/x-www-form-urlencoded

Content
----

@scholarsmate There's certainly some room for improvement, but I could really do with some more details about what you were testing and exactly what form you expected the HTTP request to take.

scholarsmate commented 8 years ago

Hi Andy, thanks for looking into this and sorry for my not getting back to you sooner (I was away for the past week).

TL;DR To generate POST snippets that take both a content body and URL parameters, do the following:

Okay, now for the deatils...

Here are the test endpoints:

package com.novetta.entity.service.rest.resource.impl;

import static org.springframework.http.MediaType.TEXT_PLAIN_VALUE;
import static org.springframework.web.bind.annotation.RequestMethod.GET;
import static org.springframework.web.bind.annotation.RequestMethod.POST;

import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping("api/v1/tests")
public class TestResourceImpl {

  @SuppressWarnings("static-method")
  @RequestMapping(method = GET, value = "/hello", produces = TEXT_PLAIN_VALUE)
  public ResponseEntity<String> helloGet(
    @RequestParam("name") final String name)
  {
    return ResponseEntity.ok("Hello " + name);
  }

  @SuppressWarnings("static-method")
  @RequestMapping(method = POST, value = "/hello", produces = TEXT_PLAIN_VALUE)
  public ResponseEntity<String> helloPost(
    @RequestParam("name") final String name,
    @RequestBody final String content)
  {
    return ResponseEntity.ok("Hello " + name + ". Here is your content: " + content);
  }

}

The snippet I'm looking for is one that shows both the URL parameter and the content body in the curl request. The "name" parameter is getting sent because the test expectations are being met.

The swagger generated snippet works as expected.

$ curl -i -X POST --header 'Content-Type: application/json' --header 'Accept: text/plain' -d 'I have lots to share' 'http://192.168.90.70:9070/api/v1/tests/hello?name=World'
HTTP/1.1 200 OK
Connection: keep-alive
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Type: text/plain; charset=UTF-8
Content-Length: 55
Date: Tue, 31 May 2016 13:19:40 GMT

Hello World. Here is your content: I have lots to share

Changing the content-type to text/plain works the same way and is now more accurate since the content here isn't actually JSON (in my real endpoints though, all the content being POSTed is JSON).

$ curl -i -X POST --header 'Content-Type: text/plain' --header 'Accept: text/plain' -d 'I have lots to share' 'http://192.168.90.70:9070/api/v1/tests/hello?name=World'
HTTP/1.1 200 OK
Connection: keep-alive
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Type: text/plain; charset=UTF-8
Content-Length: 55
Date: Tue, 31 May 2016 13:23:55 GMT

Hello World. Here is your content: I have lots to share

As you suggested, I tried working around the issue by manually tacking the parameter onto the URL and not use the param() method.

 @Test
  public void postHello2()
    throws Exception
  {
    mockMvc.perform(post(buildApiPath("tests/hello") + "/?name=World").content("I have lots to share"))
        .andExpect(status().isOk())
        .andExpect(content().string("Hello World. Here is your content: I have lots to share"))
        .andDo(document("post-hello2"));
  }

Which yielded the following snippet.

$ cat ./build/generated-restdoc/post-hello2/curl-request.adoc
[source,bash]
----
$ curl 'https://localhost:9070/api/v1/tests/hello/?name=World' -i -X POST -d 'I have lots to share'
----

The test expectations are met, but when running the snippet from the command line, we get:

$ curl 'https://localhost:9070/api/v1/tests/hello/?name=World' -i -X POST -d 'I have lots to share'
HTTP/1.1 200 OK
Connection: keep-alive
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Type: text/plain; charset=UTF-8
Content-Length: 67
Date: Tue, 31 May 2016 13:35:02 GMT

Hello World. Here is your content: I+have+lots+to+share=&name=World

So it used the default content type of application/x-www-form-urlencoded. I then changed the test to explicitly set the content-type:

  @Test
  public void postHello2()
    throws Exception
  {
    mockMvc.perform(post(buildApiPath("tests/hello") + "/?name=World")
        .contentType(MediaType.TEXT_PLAIN)
        .content("I have lots to share"))
        .andExpect(status().isOk())
        .andExpect(content().string("Hello World. Here is your content: I have lots to share"))
        .andDo(document("post-hello2"));
  }

Which yields the snippet:

$ cat ./build/generated-restdoc/post-hello2/curl-request.adoc[source,bash]
----
$ curl 'https://localhost:9070/api/v1/tests/hello/?name=World' -i -X POST -H 'Content-Type: text/plain' -d 'I have lots to share'
----

Which when run from the CLI gives:

$ curl 'https://localhost:9070/api/v1/tests/hello/?name=World' -i -X POST -H 'Content-Type: text/plain' -d 'I have lots to share'
HTTP/1.1 200 OK
Connection: keep-alive
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Type: text/plain; charset=UTF-8
Content-Length: 55
Date: Tue, 31 May 2016 13:59:47 GMT

Hello World. Here is your content: I have lots to share

Giving me what I want! I can now work around the issue.

I tried going back and using the param() method with setting the content type to text/plain, but the generated snippet did not include the parameter in the URL.

wilkinsona commented 8 years ago

Thanks for the detailed analysis. That gives me what I need to make some improvements.

wilkinsona commented 8 years ago

@scholarsmate This should be fixed in the next 1.1.1 snapshot available from https://repo.spring.io/libs-snapshot. If you have a chance to try it out, please let me know how you get on.