Make Profiles created via Profiles.of() comparable

Prior to this commit, a Profiles instance created via Profiles.of() was
not considered equivalent to another Profiles instance created via
Profiles.of() with the exact same expressions. This makes it difficult
to mock invocations of Environment#acceptsProfiles(Profiles) -- for
example, when using a mocking library such as Mockito.

This commit makes Profiles instances created via Profiles.of()
"comparable" by implementing equals() and hashCode() in ParsedProfiles.

Note, however, that equivalence is only guaranteed if the exact same
profile expression strings are supplied to Profiles.of(). In other
words, Profiles.of("A & B", "C | D") is equivalent to
Profiles.of("A & B", "C | D") and Profiles.of("C | D", "A & B"), but
Profiles.of("X & Y") is not equivalent to Profiles.of("X&Y") or
Profiles.of("Y & X").

Closes gh-25340

Author: sbrannen

spring-core/src/main/java/org/springframework/core/env/Profiles.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -26,6 +26,7 @@
  * {@link #of(String...) of(...)} factory method.
  *
  * @author Phillip Webb
+ * @author Sam Brannen
  * @since 5.1
  */
 @FunctionalInterface
@@ -34,7 +35,7 @@ public interface Profiles {
 	/**
 	 * Test if this {@code Profiles} instance <em>matches</em> against the given
 	 * active profiles predicate.
-	 * @param activeProfiles predicate that tests whether a given profile is
+	 * @param activeProfiles a predicate that tests whether a given profile is
 	 * currently active
 	 */
 	boolean matches(Predicate<String> activeProfiles);
@@ -49,16 +50,20 @@ public interface Profiles {
 	 * {@code "production"}) or a profile expression. A profile expression allows
 	 * for more complicated profile logic to be expressed, for example
 	 * {@code "production & cloud"}.
-	 * <p>The following operators are supported in profile expressions:
+	 * <p>The following operators are supported in profile expressions.
 	 * <ul>
-	 * <li>{@code !} - A logical <em>not</em> of the profile</li>
-	 * <li>{@code &} - A logical <em>and</em> of the profiles</li>
-	 * <li>{@code |} - A logical <em>or</em> of the profiles</li>
+	 * <li>{@code !} - A logical <em>NOT</em> of the profile or profile expression</li>
+	 * <li>{@code &} - A logical <em>AND</em> of the profiles or profile expressions</li>
+	 * <li>{@code |} - A logical <em>OR</em> of the profiles or profile expressions</li>
 	 * </ul>
 	 * <p>Please note that the {@code &} and {@code |} operators may not be mixed
 	 * without using parentheses. For example {@code "a & b | c"} is not a valid
 	 * expression; it must be expressed as {@code "(a & b) | c"} or
 	 * {@code "a & (b | c)"}.
+	 * <p>As of Spring Framework 5.1.17, two {@code Profiles} instances returned
+	 * by this method are considered equivalent to each other (in terms of
+	 * {@code equals()} and {@code hashCode()} semantics) if they are created
+	 * with identical <em>profile strings</em>.
 	 * @param profiles the <em>profile strings</em> to include
 	 * @return a new {@link Profiles} instance
 	 */

spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -18,7 +18,10 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.function.Predicate;
 
@@ -30,6 +33,7 @@
  * Internal parser used by {@link Profiles#of}.
  *
  * @author Phillip Webb
+ * @author Sam Brannen
  * @since 5.1
  */
 final class ProfilesParser {
@@ -56,6 +60,7 @@ private static Profiles parseExpression(String expression) {
 	private static Profiles parseTokens(String expression, StringTokenizer tokens) {
 		return parseTokens(expression, tokens, Context.NONE);
 	}
+
 	private static Profiles parseTokens(String expression, StringTokenizer tokens, Context context) {
 		List<Profiles> elements = new ArrayList<>();
 		Operator operator = null;
@@ -145,12 +150,12 @@ private enum Context {NONE, INVERT, BRACKET}
 
 	private static class ParsedProfiles implements Profiles {
 
-		private final String[] expressions;
+		private final Set<String> expressions = new LinkedHashSet<>();
 
 		private final Profiles[] parsed;
 
 		ParsedProfiles(String[] expressions, Profiles[] parsed) {
-			this.expressions = expressions;
+			Collections.addAll(this.expressions, expressions);
 			this.parsed = parsed;
 		}
 
@@ -164,10 +169,31 @@ public boolean matches(Predicate<String> activeProfiles) {
 			return false;
 		}
 
+		@Override
+		public int hashCode() {
+			return this.expressions.hashCode();
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			if (this == obj) {
+				return true;
+			}
+			if (obj == null) {
+				return false;
+			}
+			if (getClass() != obj.getClass()) {
+				return false;
+			}
+			ParsedProfiles that = (ParsedProfiles) obj;
+			return this.expressions.equals(that.expressions);
+		}
+
 		@Override
 		public String toString() {
-			return StringUtils.arrayToDelimitedString(this.expressions, " or ");
+			return StringUtils.collectionToDelimitedString(this.expressions, " or ");
 		}
+
 	}
 
 }

spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -288,10 +288,72 @@ void malformedExpressions() {
 
 	@Test
 	void sensibleToString() {
-		assertThat(Profiles.of("spring & framework", "java | kotlin").toString()).isEqualTo("spring & framework or java | kotlin");
+		assertThat(Profiles.of("spring")).hasToString("spring");
+		assertThat(Profiles.of("(spring & framework) | (spring & java)")).hasToString("(spring & framework) | (spring & java)");
+		assertThat(Profiles.of("(spring&framework)|(spring&java)")).hasToString("(spring&framework)|(spring&java)");
+		assertThat(Profiles.of("spring & framework", "java | kotlin")).hasToString("spring & framework or java | kotlin");
+		assertThat(Profiles.of("java | kotlin", "spring & framework")).hasToString("java | kotlin or spring & framework");
 	}
 
-	private void assertMalformed(Supplier<Profiles> supplier) {
+	@Test
+	void sensibleEquals() {
+		assertEqual("(spring & framework) | (spring & java)");
+		assertEqual("(spring&framework)|(spring&java)");
+		assertEqual("spring & framework", "java | kotlin");
+
+		// Ensure order of individual expressions does not affect equals().
+		String expression1 = "A | B";
+		String expression2 = "C & (D | E)";
+		Profiles profiles1 = Profiles.of(expression1, expression2);
+		Profiles profiles2 = Profiles.of(expression2, expression1);
+		assertThat(profiles1).isEqualTo(profiles2);
+		assertThat(profiles2).isEqualTo(profiles1);
+	}
+
+	private void assertEqual(String... expressions) {
+		Profiles profiles1 = Profiles.of(expressions);
+		Profiles profiles2 = Profiles.of(expressions);
+		assertThat(profiles1).isEqualTo(profiles2);
+		assertThat(profiles2).isEqualTo(profiles1);
+	}
+
+	@Test
+	void sensibleHashCode() {
+		assertHashCode("(spring & framework) | (spring & java)");
+		assertHashCode("(spring&framework)|(spring&java)");
+		assertHashCode("spring & framework", "java | kotlin");
+
+		// Ensure order of individual expressions does not affect hashCode().
+		String expression1 = "A | B";
+		String expression2 = "C & (D | E)";
+		Profiles profiles1 = Profiles.of(expression1, expression2);
+		Profiles profiles2 = Profiles.of(expression2, expression1);
+		assertThat(profiles1).hasSameHashCodeAs(profiles2);
+	}
+
+	private void assertHashCode(String... expressions) {
+		Profiles profiles1 = Profiles.of(expressions);
+		Profiles profiles2 = Profiles.of(expressions);
+		assertThat(profiles1).hasSameHashCodeAs(profiles2);
+	}
+
+	@Test
+	void equalsAndHashCodeAreNotBasedOnLogicalStructureOfNodesWithinExpressionTree() {
+		Profiles profiles1 = Profiles.of("A | B");
+		Profiles profiles2 = Profiles.of("B | A");
+
+		assertThat(profiles1.matches(activeProfiles("A"))).isTrue();
+		assertThat(profiles1.matches(activeProfiles("B"))).isTrue();
+		assertThat(profiles2.matches(activeProfiles("A"))).isTrue();
+		assertThat(profiles2.matches(activeProfiles("B"))).isTrue();
+
+		assertThat(profiles1).isNotEqualTo(profiles2);
+		assertThat(profiles2).isNotEqualTo(profiles1);
+		assertThat(profiles1.hashCode()).isNotEqualTo(profiles2.hashCode());
+	}
+
+
+	private static void assertMalformed(Supplier<Profiles> supplier) {
 		assertThatIllegalArgumentException().isThrownBy(
 				supplier::get)
 			.withMessageContaining("Malformed");