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

RestAssured documentation fails with multipart with any content other than String #252

Closed psamsotha closed 8 years ago

psamsotha commented 8 years ago

With RestAssured multipart support, we can go things like

given(spec)
    .multipart("data1", new File("..'"))
    .multipart("data2", new byte[] { 1 })
    .multipart("data3", new FakeInputStream())

The problem with this though is the RestAssuredRequestConverter, when it tries to convert the content. Only a few types are supported

private byte[] convertContent(Object content) {
    if (content instanceof String) {
        return ((String) content).getBytes();
    } else if (content instanceof byte[]) {
        return (byte[]) content;
    } else if (content == null) {
        return new byte[0];
    } else {
        throw new IllegalStateException(
                "Unsupported request content: " + content.getClass().getName());
    }
}

You would think that the byte[] should work, but somewhere in the process, before the spec is build, it converts the byte[] into a ByteArrayInpuStream, so it fails like with all the other types.

IllegalStateException: Unsupported request content: java.io.ByteArrayInputStream

The other types (InputStream and File) don't get converted, so you get the same exception as above, but with the respective type in the message.

As a solution, I was thinking maybe just handle the easiest case, checking for the ByteArrayInputStream, in which you can easily convert the to a byte[] without RestAssured being affected. The problems I see with this is that 1. You don't cover all other types, 2. It is dependent on the RestAssured implementation not changing. For the second point maybe just write a test to fail, if it does ever change, cross that bridge if it ever gets to that point. Not the best idea, but somehing I could live with.

Or maybe there is some other way, through some RestAssured APIs to change this behavior, without needing to touch RestDocs. Maybe some preprocess filtering. I don't know, I am no RestAssured expert.

Maybe @johanhaleby has some ideas on how this problem can best be solved.

Below is a complete runnable test that reproduces the errors.

import java.io.File;
import java.io.FileInputStream;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.logging.Logger;

import javax.ws.rs.Consumes;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;

import org.glassfish.grizzly.http.server.HttpServer;
import org.glassfish.jersey.filter.LoggingFilter;
import org.glassfish.jersey.grizzly2.httpserver.GrizzlyHttpServerFactory;
import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
import org.glassfish.jersey.media.multipart.FormDataParam;
import org.glassfish.jersey.media.multipart.MultiPartFeature;
import org.glassfish.jersey.server.ResourceConfig;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.springframework.restdocs.JUnitRestDocumentation;

import com.jayway.restassured.builder.MultiPartSpecBuilder;
import com.jayway.restassured.builder.RequestSpecBuilder;
import com.jayway.restassured.specification.MultiPartSpecification;
import com.jayway.restassured.specification.RequestSpecification;

import static com.jayway.restassured.RestAssured.*;

import static org.springframework.restdocs.restassured.RestAssuredRestDocumentation.*;

/**
 * Dependencies for test
 * 
    <dependencies>
        <dependency>
            <groupId>org.glassfish.jersey.containers</groupId>
            <artifactId>jersey-container-grizzly2-http</artifactId>
            <version>2.22.1</version>
        </dependency>
         <dependency>
            <groupId>org.glassfish.jersey.media</groupId>
            <artifactId>jersey-media-multipart</artifactId>
            <version>2.22.1</version>
        </dependency>
        <dependency>
            <groupId>junit</groupId>
            <artifactId>junit</artifactId>
            <version>4.12</version>
            <scope>test</scope>
        </dependency>
        <dependency>
            <groupId>org.springframework.restdocs</groupId>
            <artifactId>spring-restdocs-restassured</artifactId>
            <version>1.1.0.RELEASE</version>
            <scope>test</scope>
        </dependency>
        <dependency>
           <groupId>com.jayway.restassured</groupId>
           <artifactId>rest-assured</artifactId>
           <version>2.9.0</version>
           <scope>test</scope>
       </dependency>
    </dependencies>
 * 
 */
public class RestassuredMultipartTest {

    private static final String BASE_URI = "http://localhost:8080/api/";
    private static final String FILE_NAME = "test.txt";
    private static final String TEST_DATA = "RandomData";

    @Rule
    public JUnitRestDocumentation documentation 
            = new JUnitRestDocumentation("target/snippets");

    @Rule
    public TemporaryFolder tempFolder = new TemporaryFolder();

    private HttpServer server;

    private RequestSpecification spec;

    private File dataFile;

    @Before
    public void setUp() throws Exception {
        this.server = GrizzlyHttpServerFactory.createHttpServer(URI.create(BASE_URI),
                new ResourceConfig(TestResource.class)
                .register(MultiPartFeature.class)
                .register(new LoggingFilter(Logger.getAnonymousLogger(), true)));
        this.server.start();

        this.spec = new RequestSpecBuilder()
                .addFilter(documentationConfiguration(this.documentation))
                .setBaseUri(BASE_URI).build();

        this.dataFile = this.tempFolder.newFile(FILE_NAME);
        Files.write(this.dataFile.toPath(), 
                TEST_DATA.getBytes(StandardCharsets.UTF_16), 
                StandardOpenOption.APPEND);
    }

    @After
    public void tearDown() {
        this.server.stop();
    }

    @Path("test")
    public static class TestResource {

        @POST
        @Produces("text/plain")
        @Consumes("multipart/form-data")
        public String post(@FormDataParam("data") String data,
                @FormDataParam("data") FormDataContentDisposition fdcd) {
            return data;
        }
    }

    @Test
    public void multiPartPassWithoutDocumentation() throws Exception {
        given(this.spec)
            .multiPart("data", FILE_NAME, new FileInputStream(this.dataFile))
        .when()
            .post("/test")
        .then()
            .assertThat()
            .statusCode(200);   
    }

    @Test
    public void multiPartPassWithString() {
        given(this.spec)
            .multiPart("data", TEST_DATA)
            .filter(document("multipart-string"))
        .when()
            .post("/test")
        .then()
            .assertThat()
            .statusCode(200);  
    }

    @Test
    public void multiPartInputStreamTest() throws Exception { 
        given(this.spec)
            .multiPart("data", FILE_NAME, new FileInputStream(this.dataFile),
                    "text/plain")
            .filter(document("multipart-inputstream"))
        .when()
            .post("/test")
        .then()
            .assertThat()
            .statusCode(200);
    }

    @Test
    public void multiPartByteArrayTest() throws Exception {
        given(this.spec)
            .multiPart("data", FILE_NAME, Files.readAllBytes(this.dataFile.toPath()), 
                    "text/plain")
            .filter(document("multipart-byte-array"))
        .when()
            .post("/test")
        .then()
            .assertThat()
            .statusCode(200);
    }

    @Test
    public void multiPartFileTest() {
        given(this.spec)
            .multiPart("data", this.dataFile, "text/plain")
            .filter(document("multipart-file"))
        .when()
            .post("/test")
        .then()
            .assertThat()
            .statusCode(200);    
    }

    @Test
    public void multiPartWithSpecAndByteArrayTest() throws Exception {
        final MultiPartSpecification multiPart = new MultiPartSpecBuilder(
                Files.readAllBytes(this.dataFile.toPath()))
                .controlName("data")
                .mimeType("text/plain")
                .build();
        given(this.spec)
            .multiPart(multiPart)
            .filter(document("multipart-spec"))
        .when()
            .post("/test")
        .then()
            .assertThat()
            .statusCode(200); 
    }
}
psamsotha commented 8 years ago

Or maybe, just for multipart, whenever it's any other type, just add some placeholder content like <<binary data>>, so that's what would show up in the snippet. Just throwing an idea out there.

wilkinsona commented 8 years ago

I haven't got a full understanding of how and when REST Assured converts content to a different type, but I can see no reason why REST Docs can't support File at the very least. IIRC, the problem with an InputStream was it could then only be read once. It could be wrapped in a BufferedInputStream but, depending on size, there's no guarantee that would work. I'll double-check that this is indeed the case.

wilkinsona commented 8 years ago

The conversion happens in MultiPartInternal.getContentBody():

 def getContentBody() {
    if (content instanceof NoParameterValue) {
      content = "";
    }

    if (content instanceof File) {
      new FileBody(content, fileName, mimeType ?: OCTET_STREAM, charset)
    } else if (content instanceof InputStream) {
      returnInputStreamBody()
    } else if (content instanceof byte[]) {
      content = new ByteArrayInputStream(content)
      returnInputStreamBody()
    } else if (content instanceof String) {
      returnStringBody(content)
    } else if (content != null) {
      returnStringBody(content.toString())
    } else {
      throw new IllegalArgumentException("Illegal content: $content")
    }
  }

Note that when the content is a byte[], it's changed to a ByteArrayInputStream. All other content types are left as-is. This appears to be the case across REST Assured 2.x