From 82fd14d819810a9c5e8a9f5f4f833ba80240eed2 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 11:48:03 +0300 Subject: [PATCH 01/21] Suppress an `ErrorProne` warning. --- base/src/main/java/io/spine/base/EnvironmentType.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/base/src/main/java/io/spine/base/EnvironmentType.java b/base/src/main/java/io/spine/base/EnvironmentType.java index 561b34a230..53f14e1824 100644 --- a/base/src/main/java/io/spine/base/EnvironmentType.java +++ b/base/src/main/java/io/spine/base/EnvironmentType.java @@ -43,11 +43,14 @@ public abstract class EnvironmentType { protected abstract boolean enabled(); /** - * @inheritDoc - * - *

By default, environments types are compared based on their classes. + * @inheritDoc

By default, environments types are compared based on their classes. + * @implNote This class deliberately breaks the substitution principle for the {@code + * equals} method. Extenders are not encouraged to have {@code EnvironmentType} + * hierarchies. If they decide to have them anyway, they are free to + * override {@code equals} and {@code hashCode} accordingly. */ @Override + @SuppressWarnings("EqualsGetClass" /* see @implNote */) public boolean equals(Object obj) { return this.getClass() .equals(obj.getClass()); From bb9f27675e1f0cffd995977a98497c969ff5c945 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 12:09:28 +0300 Subject: [PATCH 02/21] Resolve Error Prone. --- .../test/java/io/spine/base/EnvironmentTest.java | 13 +++++++++---- .../java/io/spine/base/environment/Staging.java | 5 +++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/base/src/test/java/io/spine/base/EnvironmentTest.java b/base/src/test/java/io/spine/base/EnvironmentTest.java index 9e6422eb88..642e9c4c65 100644 --- a/base/src/test/java/io/spine/base/EnvironmentTest.java +++ b/base/src/test/java/io/spine/base/EnvironmentTest.java @@ -20,6 +20,7 @@ package io.spine.base; +import com.google.errorprone.annotations.Immutable; import io.spine.testing.UtilityClassTest; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; @@ -32,7 +33,6 @@ import static com.google.common.truth.Truth.assertThat; import static io.spine.base.Tests.ENV_KEY_TESTS; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; @DisplayName("Environment utility class should") @SuppressWarnings("AccessOfSystemProperties") @@ -217,6 +217,8 @@ private static void register(EnvironmentType... types) { } } + @Immutable + @SuppressWarnings("ImmutableEnumChecker" /* This testing implementation is mutable for simplicity. */) static final class Local extends EnvironmentType { private Local() { @@ -229,7 +231,7 @@ public boolean enabled() { } public static Local type() { - return Singleton.INSTANCE.local; + return Local.Singleton.INSTANCE.local; } private enum Singleton { @@ -245,6 +247,8 @@ private enum Singleton { } } + @Immutable + @SuppressWarnings("ImmutableEnumChecker" /* This testing implementation is mutable for simplicity. */) static final class Staging extends EnvironmentType { static final String STAGING_ENV_TYPE_KEY = "io.spine.base.EnvironmentTest.is_staging"; @@ -253,7 +257,7 @@ private Staging() { } public static Staging type() { - return Singleton.INSTANCE.staging; + return Staging.Singleton.INSTANCE.staging; } @Override @@ -275,6 +279,7 @@ private enum Singleton { } } + @Immutable @SuppressWarnings("unused" /* The only variant is used. */) static final class Travis extends EnvironmentType { @@ -287,7 +292,7 @@ public boolean enabled() { } public static Travis type() { - return Singleton.INSTANCE.travis; + return Travis.Singleton.INSTANCE.travis; } private enum Singleton { diff --git a/base/src/test/java/io/spine/base/environment/Staging.java b/base/src/test/java/io/spine/base/environment/Staging.java index f436b35cdb..ac65b95efd 100644 --- a/base/src/test/java/io/spine/base/environment/Staging.java +++ b/base/src/test/java/io/spine/base/environment/Staging.java @@ -28,6 +28,10 @@ * *

This implementations relies on a static {@code boolean} flag for detection. */ +@SuppressWarnings("ImmutableEnumChecker" + /* + * This testing implementation is mutable for simplicity. + */) public final class Staging extends EnvironmentType { @Override @@ -59,6 +63,7 @@ public enum Singleton { @SuppressWarnings("NonSerializableFieldInSerializableClass") private final Staging staging = new Staging(); + private boolean enabled; } } From 552a8f7b27fb8f83912fadc67d261e6fc0f811e5 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 12:16:47 +0300 Subject: [PATCH 03/21] Bump the version. --- version.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.gradle.kts b/version.gradle.kts index dfac331290..823a4d2e0c 100644 --- a/version.gradle.kts +++ b/version.gradle.kts @@ -25,7 +25,7 @@ * as we want to manage the versions in a single source. */ -val SPINE_VERSION = "1.5.14" +val SPINE_VERSION = "1.5.15" project.extra.apply { this["spineVersion"] = SPINE_VERSION From ae20d7fe502749c0751e3aa3fa85df1ff4297b78 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 13:40:14 +0300 Subject: [PATCH 04/21] Avoid unnecessary caching. --- .../main/java/io/spine/base/Environment.java | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/base/src/main/java/io/spine/base/Environment.java b/base/src/main/java/io/spine/base/Environment.java index f1a1ffd077..9c974c3a03 100644 --- a/base/src/main/java/io/spine/base/Environment.java +++ b/base/src/main/java/io/spine/base/Environment.java @@ -38,10 +38,10 @@ * environment type}. Two environment types exist out of the box: * *

* *

The framework users may define their custom settings depending on the current environment @@ -181,10 +181,9 @@ public Environment createCopy() { * * @return the current environment type. */ - @SuppressWarnings("ConstantConditions"/* no NPE is ensured by the `ensureTypeIsSet` call. */) public boolean is(EnvironmentType type) { - ensureTypeIsSet(); - return currentEnvType.equals(type); + EnvironmentType currentType = cachedOrCalculated(); + return currentType.equals(type); } /** @@ -197,23 +196,8 @@ public boolean is(EnvironmentType type) { * @return the current environment type */ public EnvironmentType type() { - ensureTypeIsSet(); - return currentEnvType; - } - - private void ensureTypeIsSet() { - if (currentEnvType == null) { - determineCurrentType(); - } - } - - private void determineCurrentType() { - for (EnvironmentType type : knownEnvTypes) { - if (type.enabled()) { - this.currentEnvType = type; - return; - } - } + EnvironmentType result = cachedOrCalculated(); + return result; } /** @@ -298,4 +282,22 @@ public void reset() { this.knownEnvTypes = BASE_TYPES; Tests.clearTestingEnvVariable(); } + + private EnvironmentType cachedOrCalculated() { + EnvironmentType result = currentEnvType != null + ? currentEnvType + : currentType(); + return result; + } + + private EnvironmentType currentType() { + for (EnvironmentType type : knownEnvTypes) { + if (type.enabled()) { + return type; + } + } + + // `Production` is the default fallback. + return Production.type(); + } } From d2ab440d3b1f23addb675e06c8607f8072c0d3d3 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 14:05:10 +0300 Subject: [PATCH 05/21] Avoid unnecessary singletons. --- .../main/java/io/spine/base/Environment.java | 10 +++---- .../main/java/io/spine/base/Production.java | 29 +++++-------------- base/src/main/java/io/spine/base/Tests.java | 29 +++++-------------- .../java/io/spine/base/EnvironmentTest.java | 28 +++++++++--------- .../base/environment/EnvironmentTest.java | 3 +- 5 files changed, 36 insertions(+), 63 deletions(-) diff --git a/base/src/main/java/io/spine/base/Environment.java b/base/src/main/java/io/spine/base/Environment.java index 9c974c3a03..7c70a169f1 100644 --- a/base/src/main/java/io/spine/base/Environment.java +++ b/base/src/main/java/io/spine/base/Environment.java @@ -115,7 +115,7 @@ public final class Environment { private static final ImmutableList BASE_TYPES = - ImmutableList.of(Tests.type(), Production.type()); + ImmutableList.of(new Tests(), new Production()); private static final Environment INSTANCE = new Environment(); @@ -208,7 +208,7 @@ public EnvironmentType type() { */ @Deprecated public boolean isTests() { - return is(Tests.type()); + return is(new Tests()); } /** @@ -255,7 +255,7 @@ public void setTo(EnvironmentType type) { @Deprecated @VisibleForTesting public void setToTests() { - this.currentEnvType = Tests.type(); + this.currentEnvType = new Tests(); Tests.enable(); } @@ -269,7 +269,7 @@ public void setToTests() { @Deprecated @VisibleForTesting public void setToProduction() { - this.currentEnvType = Production.type(); + this.currentEnvType = new Production(); Tests.clearTestingEnvVariable(); } @@ -298,6 +298,6 @@ private EnvironmentType currentType() { } // `Production` is the default fallback. - return Production.type(); + return new Production(); } } diff --git a/base/src/main/java/io/spine/base/Production.java b/base/src/main/java/io/spine/base/Production.java index 9f3ea652e9..a541317887 100644 --- a/base/src/main/java/io/spine/base/Production.java +++ b/base/src/main/java/io/spine/base/Production.java @@ -30,30 +30,17 @@ @Immutable public final class Production extends EnvironmentType { - @Override - protected boolean enabled() { - return !Tests.type() - .enabled(); - } - /** - * Returns the singleton instance. + * Creates a new instance. + * + *

All {@code Production} instances are immutable and equivalent. */ - public static Production type() { - return Singleton.INSTANCE.production; + public Production() { } - private enum Singleton { - - INSTANCE; - - @SuppressWarnings({ - "NonSerializableFieldInSerializableClass", - "PMD.SingularField" /* this field cannot be local */}) - private final Production production; - - Singleton() { - this.production = new Production(); - } + @Override + protected boolean enabled() { + boolean tests = new Tests().enabled(); + return !tests; } } diff --git a/base/src/main/java/io/spine/base/Tests.java b/base/src/main/java/io/spine/base/Tests.java index 2d55191c64..73705a5fdf 100644 --- a/base/src/main/java/io/spine/base/Tests.java +++ b/base/src/main/java/io/spine/base/Tests.java @@ -53,6 +53,14 @@ public final class Tests extends EnvironmentType { private static final Pattern TEST_PROP_PATTERN = Pattern.compile("\"' "); + /** + * Creates a new instance. + * + *

All {@code Tests} instances are immutable and equivalent. + */ + public Tests() { + } + /** * Verifies if the code currently runs under a unit testing framework. * @@ -99,25 +107,4 @@ static void clearTestingEnvVariable() { static void enable() { System.setProperty(ENV_KEY_TESTS, String.valueOf(true)); } - - /** - * Returns the singleton instance of this class. - */ - public static Tests type() { - return Singleton.INSTANCE.tests; - } - - private enum Singleton { - - INSTANCE; - - @SuppressWarnings({ - "NonSerializableFieldInSerializableClass", - "PMD.SingularField" /* this field cannot be local */}) - private final Tests tests; - - Singleton() { - this.tests = new Tests(); - } - } } diff --git a/base/src/test/java/io/spine/base/EnvironmentTest.java b/base/src/test/java/io/spine/base/EnvironmentTest.java index 642e9c4c65..c21ff1ea6b 100644 --- a/base/src/test/java/io/spine/base/EnvironmentTest.java +++ b/base/src/test/java/io/spine/base/EnvironmentTest.java @@ -83,11 +83,11 @@ void cleanUp() { @Test @DisplayName("tell that we are under tests if env. variable set to true") void environmentVarTrue() { - Tests tests = Tests.type(); + Tests tests = new Tests(); Environment.instance() .setTo(tests); - assertThat(environment.is(Tests.type())).isTrue(); + assertThat(environment.is(tests)).isTrue(); } @Test @@ -95,14 +95,14 @@ void environmentVarTrue() { void environmentVar1() { System.setProperty(ENV_KEY_TESTS, "1"); - assertThat(environment.is(Tests.type())).isTrue(); + assertThat(environment.is(new Tests())).isTrue(); } @Test @DisplayName("tell that we are under tests if run under known framework") void underTestFramework() { // As we run this from under JUnit... - assertThat(environment.is(Tests.type())).isTrue(); + assertThat(environment.is(new Tests())).isTrue(); } @Test @@ -110,7 +110,7 @@ void underTestFramework() { void environmentVarUnknownValue() { System.setProperty(ENV_KEY_TESTS, "neitherTrueNor1"); - assertThat(environment.is(Production.type())).isTrue(); + assertThat(environment.is(new Production())).isTrue(); } @Test @@ -127,7 +127,7 @@ void underTestFrameworkDeprecated() { void explicitlySetTrue() { environment.setToTests(); - assertThat(environment.is(Tests.type())).isTrue(); + assertThat(environment.is(new Tests())).isTrue(); } @Test @@ -143,17 +143,17 @@ void inProductionUsingDeprecatedMethod() { @Test @DisplayName("turn tests mode on") void turnTestsOn() { - environment.setTo(Tests.type()); + environment.setTo(new Tests()); - assertThat(environment.is(Tests.type())).isTrue(); + assertThat(environment.is(new Tests())).isTrue(); } @Test @DisplayName("turn production mode on") void turnProductionOn() { - environment.setTo(Production.type()); + environment.setTo(new Production()); - assertThat(environment.is(Production.type())).isTrue(); + assertThat(environment.is(new Production())).isTrue(); } @Test @@ -162,7 +162,7 @@ void turnProductionOn() { void turnProductionOnUsingDeprecatedMethod() { environment.setToProduction(); - assertThat(environment.is(Production.type())).isTrue(); + assertThat(environment.is(new Production())).isTrue(); } @Test @@ -192,14 +192,14 @@ void fallBack() { Environment.instance() .register(Travis.type()); assertThat(environment.is(Travis.type())).isFalse(); - assertThat(environment.is(Tests.type())).isTrue(); + assertThat(environment.is(new Tests())).isTrue(); } } @Test @DisplayName("detect the current environment correctly using the `type` method") void determineUsingType() { - assertThat(environment.type()).isSameInstanceAs(Tests.type()); + assertThat(environment.type()).isEqualTo(new Tests()); } @Test @@ -207,7 +207,7 @@ void determineUsingType() { void determineUsingTypeInPresenceOfCustom() { register(Local.type()); - assertThat(environment.type()).isSameInstanceAs(Local.type()); + assertThat(environment.type()).isEqualTo(Local.type()); } private static void register(EnvironmentType... types) { diff --git a/base/src/test/java/io/spine/base/environment/EnvironmentTest.java b/base/src/test/java/io/spine/base/environment/EnvironmentTest.java index 299e6d4ad9..fea7c0fc8e 100644 --- a/base/src/test/java/io/spine/base/environment/EnvironmentTest.java +++ b/base/src/test/java/io/spine/base/environment/EnvironmentTest.java @@ -55,7 +55,6 @@ void fallbackToCustomType() { Staging.disable(); - assertThat(Environment.instance().is(Tests.type())).isTrue(); + assertThat(Environment.instance().is(new Tests())).isTrue(); } - } From 0b52da3a0a224fb19bcaf24d988fa0dde475e31d Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 14:22:16 +0300 Subject: [PATCH 06/21] Fix PMD. --- base/src/main/java/io/spine/base/Production.java | 1 + base/src/main/java/io/spine/base/Tests.java | 1 + 2 files changed, 2 insertions(+) diff --git a/base/src/main/java/io/spine/base/Production.java b/base/src/main/java/io/spine/base/Production.java index a541317887..ca8a352cdc 100644 --- a/base/src/main/java/io/spine/base/Production.java +++ b/base/src/main/java/io/spine/base/Production.java @@ -36,6 +36,7 @@ public final class Production extends EnvironmentType { *

All {@code Production} instances are immutable and equivalent. */ public Production() { + super(); } @Override diff --git a/base/src/main/java/io/spine/base/Tests.java b/base/src/main/java/io/spine/base/Tests.java index 73705a5fdf..d4a34988cb 100644 --- a/base/src/main/java/io/spine/base/Tests.java +++ b/base/src/main/java/io/spine/base/Tests.java @@ -59,6 +59,7 @@ public final class Tests extends EnvironmentType { *

All {@code Tests} instances are immutable and equivalent. */ public Tests() { + super(); } /** From 8c38a706c1c94082b3610eb99f1c75de37a4c5a3 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 14:27:03 +0300 Subject: [PATCH 07/21] Fix a typo. --- .../spine/tools/compiler/gen/column/EntityWithColumnsSpec.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/model-compiler/src/main/java/io/spine/tools/compiler/gen/column/EntityWithColumnsSpec.java b/tools/model-compiler/src/main/java/io/spine/tools/compiler/gen/column/EntityWithColumnsSpec.java index 22716a68f2..4b47eb62c5 100644 --- a/tools/model-compiler/src/main/java/io/spine/tools/compiler/gen/column/EntityWithColumnsSpec.java +++ b/tools/model-compiler/src/main/java/io/spine/tools/compiler/gen/column/EntityWithColumnsSpec.java @@ -145,7 +145,7 @@ private static JavaPoetName className(FieldDeclaration declaration) { */ private GeneratedJavadoc classJavadoc() { return GeneratedJavadoc.twoParagraph( - CodeBlock.of("Entity сolumns of proto type {@code $L}.", messageType.javaClassName()), + CodeBlock.of("Entity columns of proto type {@code $L}.", messageType.javaClassName()), CodeBlock.of("Implement this type to manually override the entity column values.") ); } From 9e135b39c1a7868465342bf837c6c5ae63bf4ee7 Mon Sep 17 00:00:00 2001 From: "serhii.lekariev" Date: Tue, 9 Jun 2020 14:28:40 +0300 Subject: [PATCH 08/21] Prefer `is(Class)` in favor of `is(EnvironmentType)`/ --- .../main/java/io/spine/base/Environment.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/base/src/main/java/io/spine/base/Environment.java b/base/src/main/java/io/spine/base/Environment.java index 7c70a169f1..f07b1bb56f 100644 --- a/base/src/main/java/io/spine/base/Environment.java +++ b/base/src/main/java/io/spine/base/Environment.java @@ -33,9 +33,9 @@ * *

Environment Type Detection

* - *

Current implementation allows to {@linkplain #is(EnvironmentType) check} whether a given - * environment is currently the active one and {@linkplain #type() get an instance of the current - * environment type}. Two environment types exist out of the box: + *

Current implementation allows to {@linkplain #is(Class) check} whether the current environment + * is of the specified class and {@linkplain #type() get an instance of the current environment + * type}. Two environment types exist out of the box: * *