From d0f0de537c6e72050cdf4f1565df78567bfba1a9 Mon Sep 17 00:00:00 2001 From: Heiko Scherrer Date: Sun, 19 Apr 2015 21:22:22 +0200 Subject: [PATCH 1/2] Ignore parameters when finding a LinkExtractor for a Content-Type Closes gh-57 --- .../restdocs/hypermedia/LinkExtractors.java | 11 ++++----- .../hypermedia/LinkExtractorsTests.java | 23 ++++++++++++++++--- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java b/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java index 4b1551ae..72a8a4a9 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java @@ -24,11 +24,10 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletResponse; -import com.fasterxml.jackson.databind.ObjectMapper; - /** * Static factory methods providing a selection of {@link LinkExtractor link extractors} * for use when documentating a hypermedia-based API. @@ -65,15 +64,15 @@ public abstract class LinkExtractors { /** * Returns the {@code LinkExtractor} for the given {@code contentType} or {@code null} * if there is no extractor for the content type. - * - * @param contentType The content type + * + * @param contentType The content type, may include parameters * @return The extractor for the content type, or {@code null} */ public static LinkExtractor extractorForContentType(String contentType) { - if (MediaType.APPLICATION_JSON_VALUE.equals(contentType)) { + if (MediaType.parseMediaType(contentType).isCompatibleWith(MediaType.APPLICATION_JSON)) { return atomLinks(); } - else if ("application/hal+json".equals(contentType)) { + else if (MediaType.parseMediaType(contentType).isCompatibleWith(new MediaType("application","hal+json"))) { return halLinks(); } return null; diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java index 8af0ba5b..d5a8302e 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java @@ -16,7 +16,9 @@ package org.springframework.restdocs.hypermedia; +import static org.hamcrest.core.IsNull.notNullValue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import java.io.File; import java.io.FileReader; @@ -33,10 +35,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import org.springframework.http.InvalidMediaTypeException; import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.restdocs.hypermedia.Link; -import org.springframework.restdocs.hypermedia.LinkExtractor; -import org.springframework.restdocs.hypermedia.LinkExtractors; import org.springframework.util.FileCopyUtils; /** @@ -62,6 +62,23 @@ public class LinkExtractorsTests { this.linkType = linkType; } + @Test(expected = InvalidMediaTypeException.class) + public void emptyContentType() { + LinkExtractors.extractorForContentType(null); + } + + @Test + public void combinedContentTypeMatches() { + LinkExtractor linkExtractor = LinkExtractors.extractorForContentType("application/json;charset=UTF-8"); + assertThat(linkExtractor, notNullValue()); + } + + @Test + public void notDefinedMediaTypesMatches() { + LinkExtractor linkExtractor = LinkExtractors.extractorForContentType("application/hal+json;charset=UTF-8"); + assertThat(linkExtractor, notNullValue()); + } + @Test public void singleLink() throws IOException { Map> links = this.linkExtractor From 474796d15e5f6a62bf4ab3d4c23557e11333a911 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 21 Apr 2015 11:00:50 +0100 Subject: [PATCH 2/2] Polish changes to Content-Type handling in LinkExtractors - If the request has no Content-Type (null or an empty string), a null extractor is returned rather than an exception being thrown. This is consistent with the old behaviour. - Tests have been reworked a little bit to separate out the new normal unit tests from the existing parameterised tests. Closes gh-56 --- .../restdocs/hypermedia/LinkExtractors.java | 24 ++-- .../LinkExtractorsPayloadTests.java | 124 ++++++++++++++++++ .../hypermedia/LinkExtractorsTests.java | 123 ++++------------- 3 files changed, 162 insertions(+), 109 deletions(-) create mode 100644 spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsPayloadTests.java diff --git a/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java b/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java index 72a8a4a9..806496c5 100644 --- a/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java +++ b/spring-restdocs/src/main/java/org/springframework/restdocs/hypermedia/LinkExtractors.java @@ -24,9 +24,11 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; -import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.util.StringUtils; + +import com.fasterxml.jackson.databind.ObjectMapper; /** * Static factory methods providing a selection of {@link LinkExtractor link extractors} @@ -69,11 +71,14 @@ public abstract class LinkExtractors { * @return The extractor for the content type, or {@code null} */ public static LinkExtractor extractorForContentType(String contentType) { - if (MediaType.parseMediaType(contentType).isCompatibleWith(MediaType.APPLICATION_JSON)) { - return atomLinks(); - } - else if (MediaType.parseMediaType(contentType).isCompatibleWith(new MediaType("application","hal+json"))) { - return halLinks(); + if (StringUtils.hasText(contentType)) { + MediaType mediaType = MediaType.parseMediaType(contentType); + if (mediaType.isCompatibleWith(MediaType.APPLICATION_JSON)) { + return atomLinks(); + } + if (mediaType.isCompatibleWith(HalLinkExtractor.HAL_MEDIA_TYPE)) { + return halLinks(); + } } return null; } @@ -95,7 +100,10 @@ public abstract class LinkExtractors { } @SuppressWarnings("unchecked") - private static class HalLinkExtractor extends JsonContentLinkExtractor { + static class HalLinkExtractor extends JsonContentLinkExtractor { + + private static final MediaType HAL_MEDIA_TYPE = new MediaType("application", + "hal+json"); @Override public Map> extractLinks(Map json) { @@ -140,7 +148,7 @@ public abstract class LinkExtractors { } @SuppressWarnings("unchecked") - private static class AtomLinkExtractor extends JsonContentLinkExtractor { + static class AtomLinkExtractor extends JsonContentLinkExtractor { @Override public Map> extractLinks(Map json) { diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsPayloadTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsPayloadTests.java new file mode 100644 index 00000000..eba107de --- /dev/null +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsPayloadTests.java @@ -0,0 +1,124 @@ +/* + * Copyright 2014-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.restdocs.hypermedia; + +import static org.junit.Assert.assertEquals; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.util.FileCopyUtils; + +/** + * Parameterized tests for {@link LinkExtractors} with various payloads. + * + * @author Andy Wilkinson + */ +@RunWith(Parameterized.class) +public class LinkExtractorsPayloadTests { + + private final LinkExtractor linkExtractor; + + private final String linkType; + + @Parameters + public static Collection data() { + return Arrays.asList(new Object[] { LinkExtractors.halLinks(), "hal" }, + new Object[] { LinkExtractors.atomLinks(), "atom" }); + } + + public LinkExtractorsPayloadTests(LinkExtractor linkExtractor, String linkType) { + this.linkExtractor = linkExtractor; + this.linkType = linkType; + } + + @Test + public void singleLink() throws IOException { + Map> links = this.linkExtractor + .extractLinks(createResponse("single-link")); + assertLinks(Arrays.asList(new Link("alpha", "http://alpha.example.com")), links); + } + + @Test + public void multipleLinksWithDifferentRels() throws IOException { + Map> links = this.linkExtractor + .extractLinks(createResponse("multiple-links-different-rels")); + assertLinks(Arrays.asList(new Link("alpha", "http://alpha.example.com"), + new Link("bravo", "http://bravo.example.com")), links); + } + + @Test + public void multipleLinksWithSameRels() throws IOException { + Map> links = this.linkExtractor + .extractLinks(createResponse("multiple-links-same-rels")); + assertLinks(Arrays.asList(new Link("alpha", "http://alpha.example.com/one"), + new Link("alpha", "http://alpha.example.com/two")), links); + } + + @Test + public void noLinks() throws IOException { + Map> links = this.linkExtractor + .extractLinks(createResponse("no-links")); + assertLinks(Collections. emptyList(), links); + } + + @Test + public void linksInTheWrongFormat() throws IOException { + Map> links = this.linkExtractor + .extractLinks(createResponse("wrong-format")); + assertLinks(Collections. emptyList(), links); + } + + private void assertLinks(List expectedLinks, Map> actualLinks) { + Map> expectedLinksByRel = new HashMap<>(); + for (Link expectedLink : expectedLinks) { + List expectedlinksWithRel = expectedLinksByRel.get(expectedLink + .getRel()); + if (expectedlinksWithRel == null) { + expectedlinksWithRel = new ArrayList<>(); + expectedLinksByRel.put(expectedLink.getRel(), expectedlinksWithRel); + } + expectedlinksWithRel.add(expectedLink); + } + assertEquals(expectedLinksByRel, actualLinks); + } + + private MockHttpServletResponse createResponse(String contentName) throws IOException { + MockHttpServletResponse response = new MockHttpServletResponse(); + FileCopyUtils.copy(new FileReader(getPayloadFile(contentName)), + response.getWriter()); + return response; + } + + private File getPayloadFile(String name) { + return new File("src/test/resources/link-payloads/" + this.linkType + "/" + name + + ".json"); + } +} diff --git a/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java b/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java index d5a8302e..89cba6f0 100644 --- a/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java +++ b/spring-restdocs/src/test/java/org/springframework/restdocs/hypermedia/LinkExtractorsTests.java @@ -16,129 +16,50 @@ package org.springframework.restdocs.hypermedia; -import static org.hamcrest.core.IsNull.notNullValue; -import static org.junit.Assert.assertEquals; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; -import java.io.File; -import java.io.FileReader; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; -import org.springframework.http.InvalidMediaTypeException; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.util.FileCopyUtils; +import org.springframework.restdocs.hypermedia.LinkExtractors.AtomLinkExtractor; +import org.springframework.restdocs.hypermedia.LinkExtractors.HalLinkExtractor; /** * Tests for {@link LinkExtractors}. - * + * * @author Andy Wilkinson */ -@RunWith(Parameterized.class) public class LinkExtractorsTests { - private final LinkExtractor linkExtractor; - - private final String linkType; - - @Parameters - public static Collection data() { - return Arrays.asList(new Object[] { LinkExtractors.halLinks(), "hal" }, - new Object[] { LinkExtractors.atomLinks(), "atom" }); - } - - public LinkExtractorsTests(LinkExtractor linkExtractor, String linkType) { - this.linkExtractor = linkExtractor; - this.linkType = linkType; - } - - @Test(expected = InvalidMediaTypeException.class) - public void emptyContentType() { - LinkExtractors.extractorForContentType(null); + @Test + public void nullContentTypeYieldsNullExtractor() { + assertThat(LinkExtractors.extractorForContentType(null), nullValue()); } @Test - public void combinedContentTypeMatches() { - LinkExtractor linkExtractor = LinkExtractors.extractorForContentType("application/json;charset=UTF-8"); - assertThat(linkExtractor, notNullValue()); + public void emptyContentTypeYieldsNullExtractor() { + assertThat(LinkExtractors.extractorForContentType(""), nullValue()); } @Test - public void notDefinedMediaTypesMatches() { - LinkExtractor linkExtractor = LinkExtractors.extractorForContentType("application/hal+json;charset=UTF-8"); - assertThat(linkExtractor, notNullValue()); + public void applicationJsonContentTypeYieldsAtomExtractor() { + LinkExtractor linkExtractor = LinkExtractors + .extractorForContentType("application/json"); + assertThat(linkExtractor, instanceOf(AtomLinkExtractor.class)); } @Test - public void singleLink() throws IOException { - Map> links = this.linkExtractor - .extractLinks(createResponse("single-link")); - assertLinks(Arrays.asList(new Link("alpha", "http://alpha.example.com")), links); + public void applicationHalJsonContentTypeYieldsHalExtractor() { + LinkExtractor linkExtractor = LinkExtractors + .extractorForContentType("application/hal+json"); + assertThat(linkExtractor, instanceOf(HalLinkExtractor.class)); } @Test - public void multipleLinksWithDifferentRels() throws IOException { - Map> links = this.linkExtractor - .extractLinks(createResponse("multiple-links-different-rels")); - assertLinks(Arrays.asList(new Link("alpha", "http://alpha.example.com"), - new Link("bravo", "http://bravo.example.com")), links); + public void contentTypeWithParameterYieldsExtractor() { + LinkExtractor linkExtractor = LinkExtractors + .extractorForContentType("application/json;foo=bar"); + assertThat(linkExtractor, instanceOf(AtomLinkExtractor.class)); } - @Test - public void multipleLinksWithSameRels() throws IOException { - Map> links = this.linkExtractor - .extractLinks(createResponse("multiple-links-same-rels")); - assertLinks(Arrays.asList(new Link("alpha", "http://alpha.example.com/one"), - new Link("alpha", "http://alpha.example.com/two")), links); - } - - @Test - public void noLinks() throws IOException { - Map> links = this.linkExtractor - .extractLinks(createResponse("no-links")); - assertLinks(Collections. emptyList(), links); - } - - @Test - public void linksInTheWrongFormat() throws IOException { - Map> links = this.linkExtractor - .extractLinks(createResponse("wrong-format")); - assertLinks(Collections. emptyList(), links); - } - - private void assertLinks(List expectedLinks, Map> actualLinks) { - Map> expectedLinksByRel = new HashMap<>(); - for (Link expectedLink : expectedLinks) { - List expectedlinksWithRel = expectedLinksByRel.get(expectedLink - .getRel()); - if (expectedlinksWithRel == null) { - expectedlinksWithRel = new ArrayList<>(); - expectedLinksByRel.put(expectedLink.getRel(), expectedlinksWithRel); - } - expectedlinksWithRel.add(expectedLink); - } - assertEquals(expectedLinksByRel, actualLinks); - } - - private MockHttpServletResponse createResponse(String contentName) throws IOException { - MockHttpServletResponse response = new MockHttpServletResponse(); - FileCopyUtils.copy(new FileReader(getPayloadFile(contentName)), - response.getWriter()); - return response; - } - - private File getPayloadFile(String name) { - return new File("src/test/resources/link-payloads/" + this.linkType + "/" + name - + ".json"); - } }