From 3b30c7832ea77a574287336f711382f915a3917d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sat, 23 May 2015 18:50:32 +0200 Subject: [PATCH] #346 - Polishing. Turned hop into a real value object by making it immutable. Added unit tests for Hop. use verbose concept names instead of abbreviations (e.g. parameters instead of params). Additional assertions and the documentation of such in JavaDoc. @since tag in new Hop class. Original pull request: #350 --- .../springframework/hateoas/client/Hop.java | 71 +++++++---- .../hateoas/client/Traverson.java | 17 +-- .../hateoas/client/HopUnitTests.java | 110 ++++++++++++++++++ .../hateoas/client/TraversonTests.java | 48 ++++---- 4 files changed, 189 insertions(+), 57 deletions(-) create mode 100644 src/test/java/org/springframework/hateoas/client/HopUnitTests.java diff --git a/src/main/java/org/springframework/hateoas/client/Hop.java b/src/main/java/org/springframework/hateoas/client/Hop.java index c587ac66..a28fd8da 100644 --- a/src/main/java/org/springframework/hateoas/client/Hop.java +++ b/src/main/java/org/springframework/hateoas/client/Hop.java @@ -15,17 +15,28 @@ */ package org.springframework.hateoas.client; +import lombok.AccessLevel; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.Value; +import lombok.experimental.Wither; + +import java.util.Collections; import java.util.HashMap; import java.util.Map; -import lombok.Value; +import org.springframework.util.Assert; /** - * Container for cuztomizations to a single traverson "hop" + * Container for customizations to a single traverson "hop" * * @author Greg Turnquist + * @author Oliver Gierke + * @since 0.18 */ -@Value(staticConstructor="rel") +@Value +@RequiredArgsConstructor(access = AccessLevel.PRIVATE) +@Getter(AccessLevel.PACKAGE) public class Hop { /** @@ -36,42 +47,54 @@ public class Hop { /** * Collection of URI Template parameters. */ - private final Map params = new HashMap(); + private final @Wither Map parameters; + + /** + * Creates a new {@link Hop} for the given relation name. + * + * @param rel must not be {@literal null} or empty. + * @return + */ + public static Hop rel(String rel) { + + Assert.hasText(rel, "Relation must not be null or empty!"); + + return new Hop(rel, Collections. emptyMap()); + } /** * Add one parameter to the map of parameters. * - * @param name - * @param value - * @return fluent DSL (Lombok @Wither didn't seem to work) + * @param name must not be {@literal null} or empty. + * @param value can be {@literal null}. + * @return */ - public Hop withParam(String name, String value) { - this.params.put(name, value); - return this; - } + public Hop withParameter(String name, Object value) { - /** - * Add a collection of parameters (does NOT clear out existing ones). - * - * @param newParams - * @return fluent DSL (Lombok @Wither didn't seem to work) - */ - public Hop withParams(Map newParams) { - this.params.putAll(newParams); - return this; + Assert.hasText(name, "Name must not be null or empty!"); + + HashMap parameters = new HashMap(this.parameters); + parameters.put(name, value); + + return new Hop(rel, parameters); } /** * Create a new {@link Map} starting with the supplied template parameters. Then add the ones for this hop. This * allows a local hop to override global parameters. * - * @param globalParameters - * @return a merged map of URI Template parameters + * @param globalParameters must not be {@literal null}. + * @return a merged map of URI Template parameters, will never be {@literal null}. */ - public Map getMergedParameteres(Map globalParameters) { + Map getMergedParameters(Map globalParameters) { + + Assert.notNull(globalParameters, "Global parameters must not be null!"); + Map mergedParameters = new HashMap(); + mergedParameters.putAll(globalParameters); - mergedParameters.putAll(this.params); + mergedParameters.putAll(this.parameters); + return mergedParameters; } } diff --git a/src/main/java/org/springframework/hateoas/client/Traverson.java b/src/main/java/org/springframework/hateoas/client/Traverson.java index 02939a34..4a61b9e0 100644 --- a/src/main/java/org/springframework/hateoas/client/Traverson.java +++ b/src/main/java/org/springframework/hateoas/client/Traverson.java @@ -185,8 +185,8 @@ public class Traverson { */ public Traverson setLinkDiscoverers(List discoverer) { - this.discoverers = discoverers == null ? DEFAULT_LINK_DISCOVERERS : new LinkDiscoverers( - OrderAwarePluginRegistry.create(discoverer)); + this.discoverers = discoverers == null ? DEFAULT_LINK_DISCOVERERS + : new LinkDiscoverers(OrderAwarePluginRegistry.create(discoverer)); return this; } @@ -258,13 +258,16 @@ public class Traverson { * Follows the given rels one by one, which means a request per rel to discover the next resource with the rel in * line. * - * @param hop must not be {@literal null} + * @param hop must not be {@literal null}. * @return + * @see Hop#rel(String) */ public TraversalBuilder follow(Hop hop) { Assert.notNull(hop, "Hop must not be null!"); + this.rels.add(hop); + return this; } @@ -399,17 +402,17 @@ public class Traverson { Link link = rel.findInResponse(responseBody, contentType); if (link == null) { - throw new IllegalStateException(String.format("Expected to find link with rel '%s' in response %s!", rel, - responseBody)); + throw new IllegalStateException( + String.format("Expected to find link with rel '%s' in response %s!", rel, responseBody)); } /** * Don't expand if the parameters are empty */ - if (thisHop.getParams().isEmpty()) { + if (thisHop.getParameters().isEmpty()) { return getAndFindLinkWithRel(link.getHref(), rels); } else { - return getAndFindLinkWithRel(link.expand(thisHop.getMergedParameteres(templateParameters)).getHref(), rels); + return getAndFindLinkWithRel(link.expand(thisHop.getMergedParameters(templateParameters)).getHref(), rels); } } } diff --git a/src/test/java/org/springframework/hateoas/client/HopUnitTests.java b/src/test/java/org/springframework/hateoas/client/HopUnitTests.java new file mode 100644 index 00000000..8385da55 --- /dev/null +++ b/src/test/java/org/springframework/hateoas/client/HopUnitTests.java @@ -0,0 +1,110 @@ +/* + * Copyright 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.hateoas.client; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.util.Collections; +import java.util.Map; + +import org.junit.Test; + +/** + * Unit tests for {@link Hop}. + * + * @author Oliver Gierke + * @soundtrack Dave Matthews Band - The Stone (Before These Crowded Streets) + */ +public class HopUnitTests { + + /** + * @see #346 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsNullRelationName() { + Hop.rel(null); + } + + /** + * @see #346 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsEmptyRelationName() { + Hop.rel(""); + } + + /** + * @see #346 + */ + @Test + public void hasNoParametersByDefault() { + assertThat(Hop.rel("rel").getParameters().entrySet(), is(empty())); + } + + /** + * @see #346 + */ + @Test + public void addsParameterForSingluarWither() { + + Hop hop = Hop.rel("rel").withParameter("key", "value"); + + assertThat(hop.getParameters(), hasEntry("key", (Object) "value")); + assertThat(hop.getParameters().entrySet(), hasSize(1)); + } + + /** + * @see #346 + */ + @Test + public void replacesParametersForWither() { + + Hop hop = Hop.rel("rel").withParameter("key", "value") + .withParameters(Collections. singletonMap("foo", "bar")); + + assertThat(hop.getParameters().entrySet(), hasSize(1)); + assertThat(hop.getParameters(), hasEntry("foo", (Object) "bar")); + } + + /** + * @see #346 + */ + @Test + public void mergesGlobalParameters() { + + Hop hop = Hop.rel("rel").withParameter("key", "value"); + + Map result = hop.getMergedParameters(Collections. singletonMap("foo", "bar")); + + assertThat(result.entrySet(), hasSize(2)); + assertThat(result, allOf(hasEntry("key", (Object) "value"), hasEntry("foo", (Object) "bar"))); + } + + /** + * @see #346 + */ + @Test + public void localParameterOverridesGlobalOnMerging() { + + Hop hop = Hop.rel("rel").withParameter("key", "value"); + + Map result = hop.getMergedParameters(Collections.singletonMap("key", (Object) "global")); + + assertThat(result.entrySet(), hasSize(1)); + assertThat(result, hasEntry("key", (Object) "value")); + } +} diff --git a/src/test/java/org/springframework/hateoas/client/TraversonTests.java b/src/test/java/org/springframework/hateoas/client/TraversonTests.java index 19bfee46..7119d918 100644 --- a/src/test/java/org/springframework/hateoas/client/TraversonTests.java +++ b/src/test/java/org/springframework/hateoas/client/TraversonTests.java @@ -31,7 +31,6 @@ import java.util.Map; import org.junit.After; import org.junit.Before; import org.junit.Test; - import org.springframework.core.ParameterizedTypeReference; import org.springframework.hateoas.Link; import org.springframework.hateoas.MediaTypes; @@ -258,8 +257,8 @@ public class TraversonTests { @Test public void returnsDefaultMessageConverters() { - List> converters = Traverson.getDefaultMessageConverters(Collections - . emptyList()); + List> converters = Traverson + .getDefaultMessageConverters(Collections. emptyList()); assertThat(converters, hasSize(1)); assertThat(converters.get(0), is(instanceOf(StringHttpMessageConverter.class))); @@ -272,9 +271,7 @@ public class TraversonTests { public void chainMultipleFollowOperations() { ParameterizedTypeReference> typeReference = new ParameterizedTypeReference>() {}; - Resource result = traverson.follow("movies") - .follow("movie") - .follow("actor").toObject(typeReference); + Resource result = traverson.follow("movies").follow("movie").follow("actor").toObject(typeReference); assertThat(result.getContent().name, is("Keanu Reaves")); } @@ -288,17 +285,17 @@ public class TraversonTests { this.traverson = new Traverson(URI.create(server.rootResource() + "/springagram"), MediaTypes.HAL_JSON); // tag::hop-with-param[] - ParameterizedTypeReference> resourceParameterizedTypeReference = - new ParameterizedTypeReference>() {}; + ParameterizedTypeReference> resourceParameterizedTypeReference = new ParameterizedTypeReference>() {}; - Resource itemResource = traverson - .follow(rel("items").withParam("projection", "noImages")) - .follow("$._embedded.items[0]._links.self.href") - .toObject(resourceParameterizedTypeReference); + Resource itemResource = traverson.// + follow(rel("items").withParameter("projection", "noImages")).// + follow("$._embedded.items[0]._links.self.href").// + toObject(resourceParameterizedTypeReference); // end::hop-with-param[] assertThat(itemResource.hasLink("self"), is(true)); - assertThat(itemResource.getLink("self").expand().getHref(), equalTo(server.rootResource() + "/springagram/items/1")); + assertThat(itemResource.getLink("self").expand().getHref(), + equalTo(server.rootResource() + "/springagram/items/1")); final Item item = itemResource.getContent(); assertThat(item.image, equalTo(server.rootResource() + "/springagram/file/cat")); @@ -314,20 +311,20 @@ public class TraversonTests { this.traverson = new Traverson(URI.create(server.rootResource() + "/springagram"), MediaTypes.HAL_JSON); // tag::hop-put[] - ParameterizedTypeReference> resourceParameterizedTypeReference = - new ParameterizedTypeReference>() {}; + ParameterizedTypeReference> resourceParameterizedTypeReference = new ParameterizedTypeReference>() {}; Map params = new HashMap(); params.put("projection", "noImages"); - Resource itemResource = traverson - .follow(rel("items").withParams(params)) - .follow("$._embedded.items[0]._links.self.href") - .toObject(resourceParameterizedTypeReference); + Resource itemResource = traverson.// + follow(rel("items").withParameters(params)).// + follow("$._embedded.items[0]._links.self.href").// + toObject(resourceParameterizedTypeReference); // end::hop-put[] assertThat(itemResource.hasLink("self"), is(true)); - assertThat(itemResource.getLink("self").expand().getHref(), equalTo(server.rootResource() + "/springagram/items/1")); + assertThat(itemResource.getLink("self").expand().getHref(), + equalTo(server.rootResource() + "/springagram/items/1")); final Item item = itemResource.getContent(); assertThat(item.image, equalTo(server.rootResource() + "/springagram/file/cat")); @@ -345,15 +342,14 @@ public class TraversonTests { Map params = new HashMap(); params.put("projection", "thisShouldGetOverwrittenByLocalHop"); - ParameterizedTypeReference> resourceParameterizedTypeReference = - new ParameterizedTypeReference>() {}; - Resource itemResource = traverson.follow(rel("items").withParam("projection", "noImages")) + ParameterizedTypeReference> resourceParameterizedTypeReference = new ParameterizedTypeReference>() {}; + Resource itemResource = traverson.follow(rel("items").withParameter("projection", "noImages")) .follow("$._embedded.items[0]._links.self.href") // retrieve first Item in the collection - .withTemplateParameters(params) - .toObject(resourceParameterizedTypeReference); + .withTemplateParameters(params).toObject(resourceParameterizedTypeReference); assertThat(itemResource.hasLink("self"), is(true)); - assertThat(itemResource.getLink("self").expand().getHref(), equalTo(server.rootResource() + "/springagram/items/1")); + assertThat(itemResource.getLink("self").expand().getHref(), + equalTo(server.rootResource() + "/springagram/items/1")); final Item item = itemResource.getContent(); assertThat(item.image, equalTo(server.rootResource() + "/springagram/file/cat"));