From 8f118378ea18bd753d12f95e03ea51a438ed085c Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sun, 7 Jan 2024 14:53:37 +0100 Subject: [PATCH 1/2] fix `OnionArchitecture` losing `optionalLayers` after `as` The `OnionArchitecture.as(..)` method (and transitively `because(..)`) didn't pass the `optionalLayers` property along when creating a `new OnionArchitecture(..)` on return. Thus, once a test called `as(..)` or `because(..)` on the rule text `optionalLayers` would be reset to `false`. Signed-off-by: Peter Gafert --- .../archunit/library/Architectures.java | 8 ++++--- .../library/OnionArchitectureTest.java | 23 +++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java index d62b98641d..1d6b22139a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java @@ -790,12 +790,14 @@ private OnionArchitecture( Optional> domainServicePredicate, Optional> applicationPredicate, Map> adapterPredicates, + boolean optionalLayers, List ignoredDependencies, Optional overriddenDescription) { this.domainModelPredicate = domainModelPredicate; this.domainServicePredicate = domainServicePredicate; this.applicationPredicate = applicationPredicate; this.adapterPredicates = adapterPredicates; + this.optionalLayers = optionalLayers; this.ignoredDependencies = ignoredDependencies; this.overriddenDescription = overriddenDescription; } @@ -1036,8 +1038,8 @@ public void check(JavaClasses classes) { } @Override - public ArchRule because(String reason) { - return ArchRule.Factory.withBecause(this, reason); + public OnionArchitecture because(String reason) { + return (OnionArchitecture) Factory.withBecause(this, reason); } /** @@ -1052,7 +1054,7 @@ public ArchRule allowEmptyShould(boolean allowEmptyShould) { @Override public OnionArchitecture as(String newDescription) { return new OnionArchitecture(domainModelPredicate, domainServicePredicate, - applicationPredicate, adapterPredicates, ignoredDependencies, + applicationPredicate, adapterPredicates, optionalLayers, ignoredDependencies, Optional.of(newDescription)); } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java b/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java index 16659338a4..8e71be1836 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java @@ -1,7 +1,10 @@ package com.tngtech.archunit.library; import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.function.Function; +import java.util.stream.Stream; import com.google.common.collect.ImmutableSet; import com.tngtech.archunit.core.domain.JavaClasses; @@ -40,6 +43,7 @@ import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach; import static java.beans.Introspector.decapitalize; import static java.lang.System.lineSeparator; +import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; import static org.assertj.core.api.Assertions.assertThat; @@ -178,9 +182,24 @@ public void onion_architecture_rejects_empty_layers_by_default() { assertFailureOnionArchitectureWithEmptyLayers(result); } + @DataProvider + public static List ruleTextModifications() { + return Stream.>of( + Function.identity(), + onionArchitecture -> onionArchitecture + .as("Onion architecture consisting of (optional) changed"), + onionArchitecture -> onionArchitecture + .because("changed"), + onionArchitecture -> onionArchitecture + .as("Onion architecture consisting of (optional) changed") + .because("changed") + ).collect(toList()); + } + @Test - public void onion_architecture_allows_empty_layers_if_all_layers_are_optional() { - OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers().withOptionalLayers(true); + @UseDataProvider("ruleTextModifications") + public void onion_architecture_allows_empty_layers_if_all_layers_are_optional(Function modifyRule) { + OnionArchitecture architecture = modifyRule.apply(anOnionArchitectureWithEmptyLayers().withOptionalLayers(true)); assertThat(architecture.getDescription()).startsWith("Onion architecture consisting of (optional)"); JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); From bc751cebd0419df001db9edf7517bb0a2cb680e2 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Mon, 25 Mar 2024 00:02:58 +0100 Subject: [PATCH 2/2] fix `OnionArchitecture` losing contained check after `as` The `OnionArchitecture.as(..)` method (and transitively `because(..)`) didn't pass the `allClassesAreContainedInArchitectureCheck` property along when creating a `new OnionArchitecture(..)` on return. Thus, once a test called `as(..)` or `because(..)` on the rule text `allClassesAreContainedInArchitectureCheck` would be reset to `Disabled`. Signed-off-by: Peter Gafert --- .../tngtech/archunit/library/Architectures.java | 16 ++++++++++++---- .../archunit/library/OnionArchitectureTest.java | 7 ++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java index 1d6b22139a..6ee276c764 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java @@ -792,7 +792,8 @@ private OnionArchitecture( Map> adapterPredicates, boolean optionalLayers, List ignoredDependencies, - Optional overriddenDescription) { + Optional overriddenDescription, + AllClassesAreContainedInArchitectureCheck allClassesAreContainedInArchitectureCheck) { this.domainModelPredicate = domainModelPredicate; this.domainServicePredicate = domainServicePredicate; this.applicationPredicate = applicationPredicate; @@ -800,6 +801,7 @@ private OnionArchitecture( this.optionalLayers = optionalLayers; this.ignoredDependencies = ignoredDependencies; this.overriddenDescription = overriddenDescription; + this.allClassesAreContainedInArchitectureCheck = allClassesAreContainedInArchitectureCheck; } /** @@ -1053,9 +1055,15 @@ public ArchRule allowEmptyShould(boolean allowEmptyShould) { @Override public OnionArchitecture as(String newDescription) { - return new OnionArchitecture(domainModelPredicate, domainServicePredicate, - applicationPredicate, adapterPredicates, optionalLayers, ignoredDependencies, - Optional.of(newDescription)); + return new OnionArchitecture( + domainModelPredicate, + domainServicePredicate, + applicationPredicate, + adapterPredicates, + optionalLayers, + ignoredDependencies, + Optional.of(newDescription), + allClassesAreContainedInArchitectureCheck); } @Override diff --git a/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java b/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java index 8e71be1836..ceb391c8b7 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java @@ -220,12 +220,13 @@ public void onion_architecture_rejects_empty_layers_if_layers_are_explicitly_not } @Test - public void onion_architecture_ensure_all_classes_are_contained_in_architecture() { + @UseDataProvider("ruleTextModifications") + public void onion_architecture_ensure_all_classes_are_contained_in_architecture(Function modifyRule) { JavaClasses classes = new ClassFileImporter().importClasses(First.class, Second.class); - OnionArchitecture architectureNotCoveringAllClasses = onionArchitecture().withOptionalLayers(true) + OnionArchitecture architectureNotCoveringAllClasses = modifyRule.apply(onionArchitecture().withOptionalLayers(true) .domainModels("..first..") - .ensureAllClassesAreContainedInArchitecture(); + .ensureAllClassesAreContainedInArchitecture()); assertThatRule(architectureNotCoveringAllClasses).checking(classes) .hasOnlyOneViolation("Class <" + Second.class.getName() + "> is not contained in architecture");