Skip to content

Commit

Permalink
Make Profiles created via Profiles.of() comparable
Browse files Browse the repository at this point in the history
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 spring-projectsgh-25340
  • Loading branch information
sbrannen committed Jul 7, 2020
1 parent f4ae18f commit 14d5390
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 15 deletions.
@@ -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.
Expand All @@ -26,6 +26,7 @@
* {@link #of(String...) of(...)} factory method.
*
* @author Phillip Webb
* @author Sam Brannen
* @since 5.1
*/
@FunctionalInterface
Expand All @@ -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);
Expand All @@ -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
*/
Expand Down
@@ -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.
Expand All @@ -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;

Expand All @@ -30,6 +33,7 @@
* Internal parser used by {@link Profiles#of}.
*
* @author Phillip Webb
* @author Sam Brannen
* @since 5.1
*/
final class ProfilesParser {
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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 ");
}

}

}
@@ -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.
Expand Down Expand Up @@ -292,11 +292,72 @@ public void malformedExpressions() {

@Test
public void sensibleToString() {
assertEquals("spring & framework or java | kotlin",
Profiles.of("spring & framework", "java | kotlin").toString());
assertEquals("spring", Profiles.of("spring").toString());
assertEquals("(spring & framework) | (spring & java)", Profiles.of("(spring & framework) | (spring & java)").toString());
assertEquals("(spring&framework)|(spring&java)", Profiles.of("(spring&framework)|(spring&java)").toString());
assertEquals("spring & framework or java | kotlin", Profiles.of("spring & framework", "java | kotlin").toString());
assertEquals("java | kotlin or spring & framework", Profiles.of("java | kotlin", "spring & framework").toString());
}

private void assertMalformed(Supplier<Profiles> supplier) {
@Test
public 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);
assertEquals(profiles1, profiles2);
assertEquals(profiles2, profiles1);
}

private void assertEqual(String... expressions) {
Profiles profiles1 = Profiles.of(expressions);
Profiles profiles2 = Profiles.of(expressions);
assertEquals(profiles1, profiles2);
assertEquals(profiles2, profiles1);
}

@Test
public 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);
assertEquals(profiles1.hashCode(), profiles2.hashCode());
}

private void assertHashCode(String... expressions) {
Profiles profiles1 = Profiles.of(expressions);
Profiles profiles2 = Profiles.of(expressions);
assertEquals(profiles1.hashCode(), profiles2.hashCode());
}

@Test
public void equalsAndHashCodeAreNotBasedOnLogicalStructureOfNodesWithinExpressionTree() {
Profiles profiles1 = Profiles.of("A | B");
Profiles profiles2 = Profiles.of("B | A");

assertTrue(profiles1.matches(activeProfiles("A")));
assertTrue(profiles1.matches(activeProfiles("B")));
assertTrue(profiles2.matches(activeProfiles("A")));
assertTrue(profiles2.matches(activeProfiles("B")));

assertNotEquals(profiles1, profiles2);
assertNotEquals(profiles2, profiles1);
assertNotEquals(profiles1.hashCode(), profiles2.hashCode());
}


private static void assertMalformed(Supplier<Profiles> supplier) {
try {
supplier.get();
fail("Not malformed");
Expand All @@ -305,7 +366,7 @@ private void assertMalformed(Supplier<Profiles> supplier) {
assertTrue(ex.getMessage().contains("Malformed"));
}
}

private static Predicate<String> activeProfiles(String... profiles) {
return new MockActiveProfiles(profiles);
}
Expand Down

0 comments on commit 14d5390

Please sign in to comment.