#678 - Polishing.

* Moved compilation of Regexp patterns out of methods in order to not repeat it over and over again. * Applied soft assertions where multiple assertions where used where one failure didn't guarantee the failure of all other assertions. * Minor formatting.
2018-12-07 16:10:18 +01:00
parent 6f27ff0379
commit 4ce73154df
2 changed files with 71 additions and 42 deletions
--- a/src/main/java/org/springframework/hateoas/Link.java
+++ b/src/main/java/org/springframework/hateoas/Link.java
@@ -42,6 +42,7 @@ import com.fasterxml.jackson.annotation.JsonInclude;
 * 
 * @author Oliver Gierke
 * @author Greg Turnquist
+ * @author Jens Schauder
 */
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonIgnoreProperties(value = "templated", ignoreUnknown = true)
@@ -53,6 +54,10 @@ public class Link implements Serializable {
 	private static final long serialVersionUID = -9037755944661782121L;
 	private static final String URI_PATTERN = "(https?|ftp|file)://[-a-zA-Z0-9+&@#/%?=~_|!:,.;]*[-a-zA-Z0-9+&@#/%=~_|]";

+	private static final Pattern URI_AND_ATTRIBUTES_PATTERN = Pattern.compile("<(.*)>;(.*)");
+	private static final Pattern KEY_AND_VALUE_PATTERN = Pattern
+			.compile("(\\w+)=\"(\\p{Lower}[\\p{Lower}\\p{Digit}\\.\\-\\s]*|" + URI_PATTERN + ")\"");
+
 	public static final String ATOM_NAMESPACE = "http://www.w3.org/2005/Atom";

 	public static final String REL_SELF = "self";
@@ -308,7 +313,7 @@ public class Link implements Serializable {

 	/**
 	 * Factory method to easily create {@link Link} instances from RFC-5988 compatible {@link String} representations of a
-	 * link. Will return {@literal null} if an empty or {@literal null} {@link String} is given.
+	 * link. Will return {@literal null} if input {@link String} is either empty or {@literal null}.
 	 * 
 	 * @param element an RFC-5899 compatible representation of a link.
 	 * @throws IllegalArgumentException if a non-empty {@link String} was given that does not adhere to RFC-5899.
@@ -321,8 +326,7 @@ public class Link implements Serializable {
 			return null;
 		}

-		Pattern uriAndAttributes = Pattern.compile("<(.*)>;(.*)");
-		Matcher matcher = uriAndAttributes.matcher(element);
+		Matcher matcher = URI_AND_ATTRIBUTES_PATTERN.matcher(element);

 		if (matcher.find()) {

@@ -378,9 +382,7 @@ public class Link implements Serializable {
 		}

 		Map<String, String> attributes = new HashMap<String, String>();
-		Pattern keyAndValue = Pattern
-				.compile("(\\w+)=\"(\\p{Lower}[\\p{Lower}\\p{Digit}\\.\\-\\s]*|" + URI_PATTERN + ")\"");
-		Matcher matcher = keyAndValue.matcher(source);
+		Matcher matcher = KEY_AND_VALUE_PATTERN.matcher(source);

 		while (matcher.find()) {
 			attributes.put(matcher.group(1), matcher.group(2));
--- a/src/test/java/org/springframework/hateoas/LinkUnitTest.java
+++ b/src/test/java/org/springframework/hateoas/LinkUnitTest.java
@@ -16,6 +16,7 @@
 package org.springframework.hateoas;

 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;

 import java.io.IOException;
 import java.io.ObjectOutputStream;
@@ -32,20 +33,27 @@ import org.springframework.http.MediaType;
 * 
 * @author Oliver Gierke
 * @author Greg Turnquist
+ * @author Jens Schauder
 */
 public class LinkUnitTest {

 	@Test
 	public void linkWithHrefOnlyBecomesSelfLink() {
+
 		Link link = new Link("foo");
 		assertThat(link.getRel()).isEqualTo(Link.REL_SELF);
 	}

 	@Test
 	public void createsLinkFromRelAndHref() {
+
 		Link link = new Link("foo", Link.REL_SELF);
-		assertThat(link.getHref()).isEqualTo("foo");
-		assertThat(link.getRel()).isEqualTo(Link.REL_SELF);
+
+		assertSoftly(softly -> {
+
+			softly.assertThat(link.getHref()).isEqualTo("foo");
+			softly.assertThat(link.getRel()).isEqualTo(Link.REL_SELF);
+		});
 	}

 	@Test(expected = IllegalArgumentException.class)
@@ -103,8 +111,11 @@ public class LinkUnitTest {
 	@Test
 	public void returnsNullForNullOrEmptyLink() {

-		assertThat(Link.valueOf(null)).isNull();
-		assertThat(Link.valueOf("")).isNull();
+		assertSoftly(softly -> {
+
+			softly.assertThat(Link.valueOf(null)).isNull();
+			softly.assertThat(Link.valueOf("")).isNull();
+		});
 	}

 	/**
@@ -115,23 +126,26 @@ public class LinkUnitTest {
 	@Test
 	public void parsesRFC5988HeaderIntoLink() {

-		assertThat(Link.valueOf("</something>;rel=\"foo\"")).isEqualTo(new Link("/something", "foo"));
-		assertThat(Link.valueOf("</something>;rel=\"foo\";title=\"Some title\"")).isEqualTo(new Link("/something", "foo"));
-		assertThat(Link.valueOf("</customer/1>;" //
-				+ "rel=\"self\";" //
-				+ "hreflang=\"en\";" //
-				+ "media=\"pdf\";" //
-				+ "title=\"pdf customer copy\";" //
-				+ "type=\"portable document\";" //
-				+ "deprecation=\"http://example.com/customers/deprecated\";" //
-				+ "profile=\"my-profile\"")) //
-						.isEqualTo(new Link("/customer/1") //
-								.withHreflang("en") //
-								.withMedia("pdf") //
-								.withTitle("pdf customer copy") //
-								.withType("portable document") //
-								.withDeprecation("http://example.com/customers/deprecated")
-								.withProfile("my-profile"));
+		assertSoftly(softly -> {
+
+			softly.assertThat(Link.valueOf("</something>;rel=\"foo\"")).isEqualTo(new Link("/something", "foo"));
+			softly.assertThat(Link.valueOf("</something>;rel=\"foo\";title=\"Some title\""))
+					.isEqualTo(new Link("/something", "foo"));
+			softly.assertThat(Link.valueOf("</customer/1>;" //
+					+ "rel=\"self\";" //
+					+ "hreflang=\"en\";" //
+					+ "media=\"pdf\";" //
+					+ "title=\"pdf customer copy\";" //
+					+ "type=\"portable document\";" //
+					+ "deprecation=\"http://example.com/customers/deprecated\";" //
+					+ "profile=\"my-profile\"")) //
+					.isEqualTo(new Link("/customer/1") //
+							.withHreflang("en") //
+							.withMedia("pdf") //
+							.withTitle("pdf customer copy") //
+							.withType("portable document") //
+							.withDeprecation("http://example.com/customers/deprecated").withProfile("my-profile"));
+		});
 	}

 	/**
@@ -141,8 +155,11 @@ public class LinkUnitTest {
 	public void ignoresUnrecognizedAttributes() {
 		Link link = Link.valueOf("</something>;rel=\"foo\";unknown=\"should fail\"");

-		assertThat(link.getHref()).isEqualTo("/something");
-		assertThat(link.getRel()).isEqualTo("foo");
+		assertSoftly(softly -> {
+
+			softly.assertThat(link.getHref()).isEqualTo("/something");
+			softly.assertThat(link.getRel()).isEqualTo("foo");
+		});
 	}

 	@Test(expected = IllegalArgumentException.class)
@@ -168,10 +185,13 @@ public class LinkUnitTest {

 		Link link = new Link("/foo{?page}");

-		assertThat(link.isTemplated()).isTrue();
-		assertThat(link.getVariableNames()).hasSize(1);
-		assertThat(link.getVariableNames()).contains("page");
-		assertThat(link.expand("2")).isEqualTo(new Link("/foo?page=2"));
+		assertSoftly(softly -> {
+
+			softly.assertThat(link.isTemplated()).isTrue();
+			softly.assertThat(link.getVariableNames()).hasSize(1);
+			softly.assertThat(link.getVariableNames()).contains("page");
+			softly.assertThat(link.expand("2")).isEqualTo(new Link("/foo?page=2"));
+		});
 	}

 	/**
@@ -182,8 +202,11 @@ public class LinkUnitTest {

 		Link link = new Link("/foo");

-		assertThat(link.isTemplated()).isFalse();
-		assertThat(link.getVariableNames()).hasSize(0);
+		assertSoftly(softly -> {
+
+			softly.assertThat(link.isTemplated()).isFalse();
+			softly.assertThat(link.getVariableNames()).hasSize(0);
+		});
 	}

 	/**
@@ -214,6 +237,7 @@ public class LinkUnitTest {
 	 */
 	@Test
 	public void parsesLinkRelationWithDotAndMinus() {
+
 		assertThat(Link.valueOf("<http://localhost>; rel=\"rel-with-minus-and-.\"").getRel())
 				.isEqualTo("rel-with-minus-and-.");
 	}
@@ -238,15 +262,18 @@ public class LinkUnitTest {
 		Link linkWithAffordance = originalLink.andAffordance(new TestAffordance());
 		Link linkWithTwoAffordances = linkWithAffordance.andAffordance(new TestAffordance());

-		assertThat(originalLink.getAffordances()).hasSize(0);
-		assertThat(linkWithAffordance.getAffordances()).hasSize(1);
-		assertThat(linkWithTwoAffordances.getAffordances()).hasSize(2);
+		assertSoftly(softly -> {

-		assertThat(originalLink.hashCode()).isNotEqualTo(linkWithAffordance.hashCode());
-		assertThat(originalLink).isNotEqualTo(linkWithAffordance);
+			softly.assertThat(originalLink.getAffordances()).hasSize(0);
+			softly.assertThat(linkWithAffordance.getAffordances()).hasSize(1);
+			softly.assertThat(linkWithTwoAffordances.getAffordances()).hasSize(2);

-		assertThat(linkWithAffordance.hashCode()).isNotEqualTo(linkWithTwoAffordances.hashCode());
-		assertThat(linkWithAffordance).isNotEqualTo(linkWithTwoAffordances);
+			softly.assertThat(originalLink.hashCode()).isNotEqualTo(linkWithAffordance.hashCode());
+			softly.assertThat(originalLink).isNotEqualTo(linkWithAffordance);
+
+			softly.assertThat(linkWithAffordance.hashCode()).isNotEqualTo(linkWithTwoAffordances.hashCode());
+			softly.assertThat(linkWithAffordance).isNotEqualTo(linkWithTwoAffordances);
+		});
 	}

 	/**