From 65639a24c186e8caadb3028fab38565022bf31e8 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 7 Jul 2020 11:49:40 +0200 Subject: [PATCH] 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 --- .../springframework/core/env/Profiles.java | 17 +++-- .../core/env/ProfilesParser.java | 34 +++++++-- .../core/env/ProfilesTests.java | 71 +++++++++++++++++-- 3 files changed, 107 insertions(+), 15 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/env/Profiles.java b/spring-core/src/main/java/org/springframework/core/env/Profiles.java index cb4df12b28e6..7f4fba7073cb 100644 --- a/spring-core/src/main/java/org/springframework/core/env/Profiles.java +++ b/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 matches 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 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"}. - *

The following operators are supported in profile expressions: + *

The following operators are supported in profile expressions. *

*

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)"}. + *

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 profile strings. * @param profiles the profile strings to include * @return a new {@link Profiles} instance */ diff --git a/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java b/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java index 1ac87617061e..5d4dd0c86b41 100644 --- a/spring-core/src/main/java/org/springframework/core/env/ProfilesParser.java +++ b/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 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 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 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 "); } + } } diff --git a/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java b/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java index 60f1b8b95267..88cd281a2c00 100644 --- a/spring-core/src/test/java/org/springframework/core/env/ProfilesTests.java +++ b/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. @@ -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 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 supplier) { try { supplier.get(); fail("Not malformed"); @@ -305,7 +366,7 @@ private void assertMalformed(Supplier supplier) { assertTrue(ex.getMessage().contains("Malformed")); } } - + private static Predicate activeProfiles(String... profiles) { return new MockActiveProfiles(profiles); }