From 9d2d227fb7f711c1c0880e659fdda384062da9a3 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Thu, 16 Jun 2022 22:13:08 +0700 Subject: [PATCH 1/2] split `ArchitecturesTest` into separate test classes The tests for onion architecture and layered architecture are quite unrelated except for some common test infrastructure. In the end these are two classes housed inside the same umbrella class, but that does not mean we have to test them in one single test class. Having a single test class makes it harder to understand which test method belongs to which class. Signed-off-by: Peter Gafert --- ...Test.java => LayeredArchitectureTest.java} | 353 +++--------------- .../library/OnionArchitectureTest.java | 272 ++++++++++++++ .../library/dependencies/GivenSlicesTest.java | 4 +- 3 files changed, 325 insertions(+), 304 deletions(-) rename archunit/src/test/java/com/tngtech/archunit/library/{ArchitecturesTest.java => LayeredArchitectureTest.java} (54%) create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java diff --git a/archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java b/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java similarity index 54% rename from archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java rename to archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java index fcddccf48b..49663d6536 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/ArchitecturesTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java @@ -8,24 +8,14 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; -import com.tngtech.archunit.base.DescribedPredicate; -import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.EvaluationResult; -import com.tngtech.archunit.library.Architectures.LayeredArchitecture; -import com.tngtech.archunit.library.Architectures.OnionArchitecture; import com.tngtech.archunit.library.testclasses.first.any.pkg.FirstAnyPkgClass; import com.tngtech.archunit.library.testclasses.first.three.any.FirstThreeAnyClass; import com.tngtech.archunit.library.testclasses.mayonlyaccesslayers.forbidden.MayOnlyAccessLayersForbiddenClass; import com.tngtech.archunit.library.testclasses.mayonlyaccesslayers.origin.MayOnlyAccessLayersOriginClass; -import com.tngtech.archunit.library.testclasses.onionarchitecture.adapter.cli.CliAdapterLayerClass; -import com.tngtech.archunit.library.testclasses.onionarchitecture.adapter.persistence.PersistenceAdapterLayerClass; -import com.tngtech.archunit.library.testclasses.onionarchitecture.adapter.rest.RestAdapterLayerClass; -import com.tngtech.archunit.library.testclasses.onionarchitecture.application.ApplicationLayerClass; -import com.tngtech.archunit.library.testclasses.onionarchitecture.domain.model.DomainModelLayerClass; -import com.tngtech.archunit.library.testclasses.onionarchitecture.domain.service.DomainServiceLayerClass; import com.tngtech.archunit.library.testclasses.second.three.any.SecondThreeAnyClass; import com.tngtech.archunit.library.testclasses.some.pkg.SomePkgClass; import com.tngtech.archunit.library.testclasses.some.pkg.sub.SomePkgSubclass; @@ -34,34 +24,24 @@ import com.tngtech.java.junit.dataprovider.DataProviders; import com.tngtech.java.junit.dataprovider.UseDataProvider; import org.junit.Assert; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; -import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo; import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAnyPackage; -import static com.tngtech.archunit.core.domain.JavaClass.Predicates.simpleNameContaining; -import static com.tngtech.archunit.core.domain.JavaClass.Predicates.simpleNameStartingWith; import static com.tngtech.archunit.core.domain.JavaConstructor.CONSTRUCTOR_NAME; import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; import static com.tngtech.archunit.library.Architectures.layeredArchitecture; -import static com.tngtech.archunit.library.Architectures.onionArchitecture; import static com.tngtech.archunit.testutil.Assertions.assertThatRule; 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.regex.Pattern.quote; -import static java.util.stream.Collectors.toSet; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; @RunWith(DataProviderRunner.class) -public class ArchitecturesTest { +public class LayeredArchitectureTest { private static final String NEW_LINE_REPLACE = "###"; - @Rule - public final ExpectedException thrown = ExpectedException.none(); - @DataProvider public static Object[][] layeredArchitectureDefinitions() { return testForEach( @@ -87,9 +67,16 @@ public static Object[][] layeredArchitectureDefinitions() { .whereLayer("Three").mayOnlyBeAccessedByLayers("One", "Two")); } + static String[] absolute(String... pkgSuffix) { + return Arrays.stream(pkgSuffix) + .map(s -> OnionArchitectureTest.class.getPackage().getName() + ".testclasses." + s) + .map(absolute -> absolute.replaceAll("\\.\\.\\.+", "..")) + .toArray(String[]::new); + } + @Test @UseDataProvider("layeredArchitectureDefinitions") - public void layered_architecture_description(LayeredArchitecture architecture) { + public void layered_architecture_description(Architectures.LayeredArchitecture architecture) { assertThat(architecture.getDescription()).isEqualTo( "Layered architecture consisting of" + lineSeparator() + "layer 'One' ('..library.testclasses.some.pkg..')" + lineSeparator() + @@ -102,7 +89,7 @@ public void layered_architecture_description(LayeredArchitecture architecture) { @Test public void layered_architecture_overridden_description() { - LayeredArchitecture architecture = layeredArchitecture() + Architectures.LayeredArchitecture architecture = layeredArchitecture() .layer("One").definedBy("some.pkg..") .whereLayer("One").mayNotBeAccessedByAnyLayer() .as("overridden"); @@ -123,29 +110,31 @@ public void layered_architecture_because_clause() { @Test public void layered_architecture_defining_constraint_on_non_existing_target_layer_is_rejected() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("no layer"); - thrown.expectMessage("NotThere"); - - layeredArchitecture() - .layer("Some").definedBy("any") - .whereLayer("NotThere").mayNotBeAccessedByAnyLayer(); + assertThatThrownBy(() -> + layeredArchitecture() + .layer("Some").definedBy("any") + .whereLayer("NotThere").mayNotBeAccessedByAnyLayer() + ) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("no layer") + .hasMessageContaining("NotThere"); } @Test public void layered_architecture_defining_constraint_on_non_existing_origin_is_rejected() { - thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("no layer"); - thrown.expectMessage("NotThere"); - - layeredArchitecture() - .layer("Some").definedBy("any") - .whereLayer("Some").mayOnlyBeAccessedByLayers("NotThere"); + assertThatThrownBy(() -> + layeredArchitecture() + .layer("Some").definedBy("any") + .whereLayer("Some").mayOnlyBeAccessedByLayers("NotThere") + ) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("no layer") + .hasMessageContaining("NotThere"); } @Test public void layered_architecture_rejects_empty_layers_by_default() { - LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers(); + Architectures.LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers(); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -155,7 +144,7 @@ public void layered_architecture_rejects_empty_layers_by_default() { @Test public void layered_architecture_allows_empty_layers_if_all_layers_are_optional() { - LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(true); + Architectures.LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(true); assertThat(architecture.getDescription()).startsWith("Layered architecture consisting of (optional)"); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -165,7 +154,7 @@ public void layered_architecture_allows_empty_layers_if_all_layers_are_optional( @Test public void layered_architecture_rejects_empty_layers_if_layers_are_explicity_not_optional_by_default() { - LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(false); + Architectures.LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(false); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -173,7 +162,7 @@ public void layered_architecture_rejects_empty_layers_if_layers_are_explicity_no assertFailureLayeredArchitectureWithEmptyLayers(result); } - private LayeredArchitecture aLayeredArchitectureWithEmptyLayers() { + private Architectures.LayeredArchitecture aLayeredArchitectureWithEmptyLayers() { return layeredArchitecture() .layer("Some").definedBy(absolute("should.not.be.found..")) .layer("Other").definedBy(absolute("also.not.found")) @@ -189,7 +178,7 @@ private void assertFailureLayeredArchitectureWithEmptyLayers(EvaluationResult re @Test public void layered_architecture_allows_individual_empty_optionalLayer() { - LayeredArchitecture architecture = layeredArchitecture() + Architectures.LayeredArchitecture architecture = layeredArchitecture() .optionalLayer("can be absent").definedBy(absolute("should.not.be.found..")); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -201,7 +190,7 @@ public void layered_architecture_allows_individual_empty_optionalLayer() { @Test @UseDataProvider("layeredArchitectureDefinitions") - public void layered_architecture_gathers_all_layer_violations(LayeredArchitecture architecture) { + public void layered_architecture_gathers_all_layer_violations(Architectures.LayeredArchitecture architecture) { JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); EvaluationResult result = architecture.evaluate(classes); @@ -218,7 +207,7 @@ public void layered_architecture_gathers_all_layer_violations(LayeredArchitectur @DataProvider public static Object[][] toIgnore() { - LayeredArchitecture layeredArchitecture = layeredArchitecture() + Architectures.LayeredArchitecture layeredArchitecture = layeredArchitecture() .layer("One").definedBy(absolute("some.pkg..")) .whereLayer("One").mayNotBeAccessedByAnyLayer(); @@ -256,7 +245,7 @@ public void layered_architecture_combines_multiple_ignores() { FirstAnyPkgClass.class, SomePkgSubclass.class, SecondThreeAnyClass.class, SomePkgClass.class); - LayeredArchitecture layeredArchitecture = layeredArchitecture() + Architectures.LayeredArchitecture layeredArchitecture = layeredArchitecture() .layer("One").definedBy(absolute("some.pkg..")) .whereLayer("One").mayNotBeAccessedByAnyLayer() .ignoreDependency(FirstAnyPkgClass.class, SomePkgSubclass.class); @@ -294,7 +283,7 @@ public static Object[][] layeredArchitectureMayOnlyAccessLayersDefinitions() { @Test @UseDataProvider("layeredArchitectureMayOnlyAccessLayersDefinitions") - public void layered_architecture_may_only_access_layers_description(LayeredArchitecture architecture) { + public void layered_architecture_may_only_access_layers_description(Architectures.LayeredArchitecture architecture) { assertThat(architecture.getDescription()).isEqualTo( "Layered architecture consisting of" + lineSeparator() + "layer 'Allowed' ('..library.testclasses.mayonlyaccesslayers.allowed..')" + lineSeparator() + @@ -306,7 +295,7 @@ public void layered_architecture_may_only_access_layers_description(LayeredArchi @Test @UseDataProvider("layeredArchitectureMayOnlyAccessLayersDefinitions") - public void layered_architecture_gathers_may_only_access_layers_violations(LayeredArchitecture architecture) { + public void layered_architecture_gathers_may_only_access_layers_violations(Architectures.LayeredArchitecture architecture) { JavaClasses classes = new ClassFileImporter().importPackages(absolute("mayonlyaccesslayers")); EvaluationResult result = architecture.evaluate(classes); @@ -325,7 +314,7 @@ public void layered_architecture_gathers_may_only_access_layers_violations(Layer @Test @UseDataProvider("layeredArchitectureMayOnlyAccessLayersDefinitions") - public void layered_architecture_can_ignore_may_only_access_layers_violations(LayeredArchitecture architecture) { + public void layered_architecture_can_ignore_may_only_access_layers_violations(Architectures.LayeredArchitecture architecture) { JavaClasses classes = new ClassFileImporter().importPackages(absolute("mayonlyaccesslayers")); architecture = architecture.ignoreDependency(MayOnlyAccessLayersOriginClass.class, MayOnlyAccessLayersForbiddenClass.class) @@ -336,178 +325,28 @@ public void layered_architecture_can_ignore_may_only_access_layers_violations(La assertThat(architecture.evaluate(classes).hasViolation()).as("result has violation").isFalse(); } - @Test - public void onion_architecture_description() { - OnionArchitecture architecture = onionArchitecture() - .domainModels("onionarchitecture.domain.model..") - .domainServices("onionarchitecture.domain.service..") - .applicationServices("onionarchitecture.application..") - .adapter("cli", "onionarchitecture.adapter.cli..") - .adapter("persistence", "onionarchitecture.adapter.persistence..") - .adapter("rest", "onionarchitecture.adapter.rest.command..", "onionarchitecture.adapter.rest.query.."); - - assertThat(architecture.getDescription()).isEqualTo( - "Onion architecture consisting of" + lineSeparator() + - "domain models ('onionarchitecture.domain.model..')" + lineSeparator() + - "domain services ('onionarchitecture.domain.service..')" + lineSeparator() + - "application services ('onionarchitecture.application..')" + lineSeparator() + - "adapter 'cli' ('onionarchitecture.adapter.cli..')" + lineSeparator() + - "adapter 'persistence' ('onionarchitecture.adapter.persistence..')" + lineSeparator() + - "adapter 'rest' ('onionarchitecture.adapter.rest.command..', 'onionarchitecture.adapter.rest.query..')" - ); - } - - @Test - public void onion_architecture_description_with_missing_layers() { - OnionArchitecture architecture = onionArchitecture(); - - assertThat(architecture.getDescription()).isEqualTo("Onion architecture consisting of"); - } - - @Test - public void onion_architecture_overridden_description() { - OnionArchitecture architecture = onionArchitecture() - .domainModels("onionarchitecture.domain.model..") - .domainServices("onionarchitecture.domain.service..") - .applicationServices("onionarchitecture.application..") - .adapter("cli", "onionarchitecture.adapter.cli..") - .adapter("persistence", "onionarchitecture.adapter.persistence..") - .adapter("rest", "onionarchitecture.adapter.rest.command..", "onionarchitecture.adapter.rest.query..") - .as("overridden"); - - assertThat(architecture.getDescription()).isEqualTo("overridden"); - } - - @Test - public void onion_architecture_because_clause() { - ArchRule architecture = onionArchitecture() - .domainModels("onionarchitecture.domain.model..") - .domainServices("onionarchitecture.domain.service..") - .applicationServices("onionarchitecture.application..") - .adapter("cli", "onionarchitecture.adapter.cli..") - .adapter("persistence", "onionarchitecture.adapter.persistence..") - .adapter("rest", "onionarchitecture.adapter.rest.command..", "onionarchitecture.adapter.rest.query..") - .as("overridden") - .because("some reason"); - - assertThat(architecture.getDescription()).isEqualTo("overridden, because some reason"); - } - - @Test - public void onion_architecture_gathers_all_violations() { - OnionArchitecture architecture = getTestOnionArchitecture(); - JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); - - EvaluationResult result = architecture.evaluate(classes); - - assertPatternMatches(result.getFailureReport().getDetails(), getExpectedOnionViolations().toPatterns()); - } - - @Test - public void onion_architecture_is_not_violated_by_ignored_dependencies() { - OnionArchitecture onionIgnoringOriginApplicationLayerClass = getTestOnionArchitecture() - .ignoreDependency(ApplicationLayerClass.class, CliAdapterLayerClass.class) - .ignoreDependency(ApplicationLayerClass.class.getName(), PersistenceAdapterLayerClass.class.getName()) - .ignoreDependency(simpleNameStartingWith("ApplicationLayerCl"), simpleNameContaining("estAdapterLayerCl")); - JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); - - EvaluationResult result = onionIgnoringOriginApplicationLayerClass.evaluate(classes); - - ExpectedOnionViolations expectedViolations = getExpectedOnionViolations().withoutViolationsWithOrigin(ApplicationLayerClass.class); - assertPatternMatches(result.getFailureReport().getDetails(), expectedViolations.toPatterns()); - } - - @Test - public void onion_architecture_with_overwritten_description_retains_ignored_dependencies() { - ArchRule onionIgnoringOriginApplicationLayerClass = getTestOnionArchitecture() - .ignoreDependency(equivalentTo(ApplicationLayerClass.class), DescribedPredicate.alwaysTrue()) - .because("some reason causing description to be overwritten"); - - JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); - - EvaluationResult result = onionIgnoringOriginApplicationLayerClass.evaluate(classes); - - ExpectedOnionViolations expectedViolations = getExpectedOnionViolations().withoutViolationsWithOrigin(ApplicationLayerClass.class); - assertPatternMatches(result.getFailureReport().getDetails(), expectedViolations.toPatterns()); - } - - @Test - public void onion_architecture_rejects_empty_layers_by_default() { - OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers(); - - JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); - - EvaluationResult result = architecture.evaluate(classes); - assertFailureOnionArchitectureWithEmptyLayers(result); - } - - @Test - public void onion_architecture_allows_empty_layers_if_all_layers_are_optional() { - OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers().withOptionalLayers(true); - assertThat(architecture.getDescription()).startsWith("Onion architecture consisting of (optional)"); - - JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); - - EvaluationResult result = architecture.evaluate(classes); - assertThat(result.hasViolation()).as("result of evaluating empty layers has violation").isFalse(); - assertThat(result.getFailureReport().isEmpty()).as("failure report").isTrue(); - } - - @Test - public void onion_architecture_rejects_empty_layers_if_layers_are_explicitly_not_optional_by_default() { - OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers().withOptionalLayers(false); - - JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); - - EvaluationResult result = architecture.evaluate(classes); - assertFailureOnionArchitectureWithEmptyLayers(result); - } - - private OnionArchitecture getTestOnionArchitecture() { - return onionArchitecture() - .domainModels(absolute("onionarchitecture.domain.model")) - .domainServices(absolute("onionarchitecture.domain.service")) - .applicationServices(absolute("onionarchitecture.application")) - .adapter("cli", absolute("onionarchitecture.adapter.cli")) - .adapter("persistence", absolute("onionarchitecture.adapter.persistence")) - .adapter("rest", absolute("onionarchitecture.adapter.rest")); + private String singleLine(EvaluationResult result) { + return Joiner.on(NEW_LINE_REPLACE).join(result.getFailureReport().getDetails()).replace("\n", NEW_LINE_REPLACE); } - private ExpectedOnionViolations getExpectedOnionViolations() { - ExpectedOnionViolations expectedViolations = new ExpectedOnionViolations(); - expectedViolations.from(DomainModelLayerClass.class) - .to(DomainServiceLayerClass.class, ApplicationLayerClass.class, CliAdapterLayerClass.class, - PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); - expectedViolations.from(DomainServiceLayerClass.class) - .to(ApplicationLayerClass.class, CliAdapterLayerClass.class, PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); - expectedViolations.from(ApplicationLayerClass.class) - .to(CliAdapterLayerClass.class, PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); - expectedViolations.from(CliAdapterLayerClass.class).to(PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); - expectedViolations.from(PersistenceAdapterLayerClass.class).to(CliAdapterLayerClass.class, RestAdapterLayerClass.class); - expectedViolations.from(RestAdapterLayerClass.class).to(CliAdapterLayerClass.class, PersistenceAdapterLayerClass.class); - return expectedViolations; + static String expectedAccessViolationPattern(Class from, String fromMethod, Class to, String toMethod) { + return String.format(".*%s.%s().*%s.%s().*", quote(from.getName()), fromMethod, quote(to.getName()), toMethod); } - private OnionArchitecture anOnionArchitectureWithEmptyLayers() { - return onionArchitecture() - .domainModels(absolute("onionarchitecture.domain.model.does.not.exist")) - .domainServices(absolute("onionarchitecture.domain.service.not.there")) - .applicationServices(absolute("onionarchitecture.application.http410")); + static String expectedFieldTypePattern(Class owner, String fieldName, Class fieldType) { + return String.format("Field .*%s\\.%s.* has type .*<%s>.*", owner.getSimpleName(), fieldName, fieldType.getName()); } - private void assertFailureOnionArchitectureWithEmptyLayers(EvaluationResult result) { - assertThat(result.hasViolation()).as("result of evaluating empty layers has violation").isTrue(); - assertPatternMatches(result.getFailureReport().getDetails(), ImmutableSet.of( - expectedEmptyLayerPattern("adapter"), expectedEmptyLayerPattern("application service"), - expectedEmptyLayerPattern("domain model"), expectedEmptyLayerPattern("domain service") - )); + @SuppressWarnings("SameParameterValue") + private static String expectedInheritancePattern(Class child, Class parent) { + return String.format("Class .*%s.* extends class .*.%s.*", child.getSimpleName(), parent.getSimpleName()); } - private String singleLine(EvaluationResult result) { - return Joiner.on(NEW_LINE_REPLACE).join(result.getFailureReport().getDetails()).replace("\n", NEW_LINE_REPLACE); + static String expectedEmptyLayerPattern(String layerName) { + return String.format("Layer '%s' is empty", layerName); } - private void assertPatternMatches(List input, Set expectedRegexes) { + static void assertPatternMatches(List input, Set expectedRegexes) { Set toMatch = new HashSet<>(expectedRegexes); for (String line : input) { if (!matchIteratorAndRemove(toMatch, line)) { @@ -517,7 +356,7 @@ private void assertPatternMatches(List input, Set expectedRegexe assertThat(toMatch).as("Unmatched Patterns").isEmpty(); } - private boolean matchIteratorAndRemove(Set toMatch, String line) { + static boolean matchIteratorAndRemove(Set toMatch, String line) { for (Iterator toMatchIterator = toMatch.iterator(); toMatchIterator.hasNext(); ) { if (line.matches(toMatchIterator.next())) { toMatchIterator.remove(); @@ -527,30 +366,6 @@ private boolean matchIteratorAndRemove(Set toMatch, String line) { return false; } - private static String expectedAccessViolationPattern(Class from, String fromMethod, Class to, String toMethod) { - return String.format(".*%s.%s().*%s.%s().*", quote(from.getName()), fromMethod, quote(to.getName()), toMethod); - } - - private static String expectedEmptyLayerPattern(String layerName) { - return String.format("Layer '%s' is empty", layerName); - } - - private static String expectedFieldTypePattern(Class owner, String fieldName, Class fieldType) { - return String.format("Field .*%s\\.%s.* has type .*<%s>.*", owner.getSimpleName(), fieldName, fieldType.getName()); - } - - @SuppressWarnings("SameParameterValue") - private static String expectedInheritancePattern(Class child, Class parent) { - return String.format("Class .*%s.* extends class .*.%s.*", child.getSimpleName(), parent.getSimpleName()); - } - - private static String[] absolute(String... pkgSuffix) { - return Arrays.stream(pkgSuffix) - .map(s -> ArchitecturesTest.class.getPackage().getName() + ".testclasses." + s) - .map(absolute -> absolute.replaceAll("\\.\\.\\.+", "..")) - .toArray(String[]::new); - } - private static class RuleWithIgnore { private final ArchRule rule; private final String description; @@ -565,70 +380,4 @@ public String toString() { return description; } } - - private static class ExpectedOnionViolations { - private final Set expected; - - private ExpectedOnionViolations() { - this(new HashSet()); - } - - private ExpectedOnionViolations(Set expected) { - this.expected = expected; - } - - From from(Class from) { - return new From(from); - } - - private ExpectedOnionViolations add(ExpectedOnionViolation expectedOnionViolation) { - expected.add(expectedOnionViolation); - return this; - } - - public ExpectedOnionViolations withoutViolationsWithOrigin(Class clazz) { - return new ExpectedOnionViolations(expected.stream() - .filter(expectedViolation -> !expectedViolation.from.equals(clazz)) - .collect(toSet())); - } - - Set toPatterns() { - ImmutableSet.Builder result = ImmutableSet.builder(); - for (ExpectedOnionViolation expectedOnionViolation : expected) { - result.addAll(expectedOnionViolation.toPatterns()); - } - return result.build(); - } - - class From { - private final Class from; - - private From(Class from) { - this.from = from; - } - - ExpectedOnionViolations to(Class... to) { - return ExpectedOnionViolations.this.add(new ExpectedOnionViolation(from, to)); - } - } - } - - private static class ExpectedOnionViolation { - private final Class from; - private final Set> tos; - - private ExpectedOnionViolation(Class from, Class[] tos) { - this.from = from; - this.tos = ImmutableSet.copyOf(tos); - } - - Set toPatterns() { - ImmutableSet.Builder result = ImmutableSet.builder(); - for (Class to : tos) { - result.add(expectedAccessViolationPattern(from, "call", to, "callMe")) - .add(expectedFieldTypePattern(from, decapitalize(to.getSimpleName()), to)); - } - return result.build(); - } - } } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java b/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java new file mode 100644 index 0000000000..ef5bcea795 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/OnionArchitectureTest.java @@ -0,0 +1,272 @@ +package com.tngtech.archunit.library; + +import java.util.HashSet; +import java.util.Set; + +import com.google.common.collect.ImmutableSet; +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.EvaluationResult; +import com.tngtech.archunit.library.Architectures.OnionArchitecture; +import com.tngtech.archunit.library.testclasses.onionarchitecture.adapter.cli.CliAdapterLayerClass; +import com.tngtech.archunit.library.testclasses.onionarchitecture.adapter.persistence.PersistenceAdapterLayerClass; +import com.tngtech.archunit.library.testclasses.onionarchitecture.adapter.rest.RestAdapterLayerClass; +import com.tngtech.archunit.library.testclasses.onionarchitecture.application.ApplicationLayerClass; +import com.tngtech.archunit.library.testclasses.onionarchitecture.domain.model.DomainModelLayerClass; +import com.tngtech.archunit.library.testclasses.onionarchitecture.domain.service.DomainServiceLayerClass; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static com.tngtech.archunit.base.DescribedPredicate.alwaysTrue; +import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo; +import static com.tngtech.archunit.core.domain.JavaClass.Predicates.simpleNameContaining; +import static com.tngtech.archunit.core.domain.JavaClass.Predicates.simpleNameStartingWith; +import static com.tngtech.archunit.library.Architectures.onionArchitecture; +import static com.tngtech.archunit.library.LayeredArchitectureTest.absolute; +import static com.tngtech.archunit.library.LayeredArchitectureTest.assertPatternMatches; +import static com.tngtech.archunit.library.LayeredArchitectureTest.expectedAccessViolationPattern; +import static com.tngtech.archunit.library.LayeredArchitectureTest.expectedEmptyLayerPattern; +import static com.tngtech.archunit.library.LayeredArchitectureTest.expectedFieldTypePattern; +import static java.beans.Introspector.decapitalize; +import static java.lang.System.lineSeparator; +import static java.util.stream.Collectors.toSet; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(DataProviderRunner.class) +public class OnionArchitectureTest { + + @Test + public void onion_architecture_description() { + OnionArchitecture architecture = onionArchitecture() + .domainModels("onionarchitecture.domain.model..") + .domainServices("onionarchitecture.domain.service..") + .applicationServices("onionarchitecture.application..") + .adapter("cli", "onionarchitecture.adapter.cli..") + .adapter("persistence", "onionarchitecture.adapter.persistence..") + .adapter("rest", "onionarchitecture.adapter.rest.command..", "onionarchitecture.adapter.rest.query.."); + + assertThat(architecture.getDescription()).isEqualTo( + "Onion architecture consisting of" + lineSeparator() + + "domain models ('onionarchitecture.domain.model..')" + lineSeparator() + + "domain services ('onionarchitecture.domain.service..')" + lineSeparator() + + "application services ('onionarchitecture.application..')" + lineSeparator() + + "adapter 'cli' ('onionarchitecture.adapter.cli..')" + lineSeparator() + + "adapter 'persistence' ('onionarchitecture.adapter.persistence..')" + lineSeparator() + + "adapter 'rest' ('onionarchitecture.adapter.rest.command..', 'onionarchitecture.adapter.rest.query..')" + ); + } + + @Test + public void onion_architecture_description_with_missing_layers() { + OnionArchitecture architecture = onionArchitecture(); + + assertThat(architecture.getDescription()).isEqualTo("Onion architecture consisting of"); + } + + @Test + public void onion_architecture_overridden_description() { + OnionArchitecture architecture = onionArchitecture() + .domainModels("onionarchitecture.domain.model..") + .domainServices("onionarchitecture.domain.service..") + .applicationServices("onionarchitecture.application..") + .adapter("cli", "onionarchitecture.adapter.cli..") + .adapter("persistence", "onionarchitecture.adapter.persistence..") + .adapter("rest", "onionarchitecture.adapter.rest.command..", "onionarchitecture.adapter.rest.query..") + .as("overridden"); + + assertThat(architecture.getDescription()).isEqualTo("overridden"); + } + + @Test + public void onion_architecture_because_clause() { + ArchRule architecture = onionArchitecture() + .domainModels("onionarchitecture.domain.model..") + .domainServices("onionarchitecture.domain.service..") + .applicationServices("onionarchitecture.application..") + .adapter("cli", "onionarchitecture.adapter.cli..") + .adapter("persistence", "onionarchitecture.adapter.persistence..") + .adapter("rest", "onionarchitecture.adapter.rest.command..", "onionarchitecture.adapter.rest.query..") + .as("overridden") + .because("some reason"); + + assertThat(architecture.getDescription()).isEqualTo("overridden, because some reason"); + } + + @Test + public void onion_architecture_gathers_all_violations() { + OnionArchitecture architecture = getTestOnionArchitecture(); + JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); + + EvaluationResult result = architecture.evaluate(classes); + + assertPatternMatches(result.getFailureReport().getDetails(), getExpectedOnionViolations().toPatterns()); + } + + @Test + public void onion_architecture_is_not_violated_by_ignored_dependencies() { + OnionArchitecture onionIgnoringOriginApplicationLayerClass = getTestOnionArchitecture() + .ignoreDependency(ApplicationLayerClass.class, CliAdapterLayerClass.class) + .ignoreDependency(ApplicationLayerClass.class.getName(), PersistenceAdapterLayerClass.class.getName()) + .ignoreDependency(simpleNameStartingWith("ApplicationLayerCl"), simpleNameContaining("estAdapterLayerCl")); + JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); + + EvaluationResult result = onionIgnoringOriginApplicationLayerClass.evaluate(classes); + + ExpectedOnionViolations expectedViolations = getExpectedOnionViolations().withoutViolationsWithOrigin(ApplicationLayerClass.class); + assertPatternMatches(result.getFailureReport().getDetails(), expectedViolations.toPatterns()); + } + + @Test + public void onion_architecture_with_overwritten_description_retains_ignored_dependencies() { + ArchRule onionIgnoringOriginApplicationLayerClass = getTestOnionArchitecture() + .ignoreDependency(equivalentTo(ApplicationLayerClass.class), alwaysTrue()) + .because("some reason causing description to be overwritten"); + + JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); + + EvaluationResult result = onionIgnoringOriginApplicationLayerClass.evaluate(classes); + + ExpectedOnionViolations expectedViolations = getExpectedOnionViolations().withoutViolationsWithOrigin(ApplicationLayerClass.class); + assertPatternMatches(result.getFailureReport().getDetails(), expectedViolations.toPatterns()); + } + + @Test + public void onion_architecture_rejects_empty_layers_by_default() { + OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers(); + + JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); + + EvaluationResult result = architecture.evaluate(classes); + assertFailureOnionArchitectureWithEmptyLayers(result); + } + + @Test + public void onion_architecture_allows_empty_layers_if_all_layers_are_optional() { + OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers().withOptionalLayers(true); + assertThat(architecture.getDescription()).startsWith("Onion architecture consisting of (optional)"); + + JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); + + EvaluationResult result = architecture.evaluate(classes); + assertThat(result.hasViolation()).as("result of evaluating empty layers has violation").isFalse(); + assertThat(result.getFailureReport().isEmpty()).as("failure report").isTrue(); + } + + @Test + public void onion_architecture_rejects_empty_layers_if_layers_are_explicitly_not_optional_by_default() { + OnionArchitecture architecture = anOnionArchitectureWithEmptyLayers().withOptionalLayers(false); + + JavaClasses classes = new ClassFileImporter().importPackages(absolute("onionarchitecture")); + + EvaluationResult result = architecture.evaluate(classes); + assertFailureOnionArchitectureWithEmptyLayers(result); + } + + private OnionArchitecture getTestOnionArchitecture() { + return onionArchitecture() + .domainModels(absolute("onionarchitecture.domain.model")) + .domainServices(absolute("onionarchitecture.domain.service")) + .applicationServices(absolute("onionarchitecture.application")) + .adapter("cli", absolute("onionarchitecture.adapter.cli")) + .adapter("persistence", absolute("onionarchitecture.adapter.persistence")) + .adapter("rest", absolute("onionarchitecture.adapter.rest")); + } + + private ExpectedOnionViolations getExpectedOnionViolations() { + ExpectedOnionViolations expectedViolations = new ExpectedOnionViolations(); + expectedViolations.from(DomainModelLayerClass.class) + .to(DomainServiceLayerClass.class, ApplicationLayerClass.class, CliAdapterLayerClass.class, + PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); + expectedViolations.from(DomainServiceLayerClass.class) + .to(ApplicationLayerClass.class, CliAdapterLayerClass.class, PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); + expectedViolations.from(ApplicationLayerClass.class) + .to(CliAdapterLayerClass.class, PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); + expectedViolations.from(CliAdapterLayerClass.class).to(PersistenceAdapterLayerClass.class, RestAdapterLayerClass.class); + expectedViolations.from(PersistenceAdapterLayerClass.class).to(CliAdapterLayerClass.class, RestAdapterLayerClass.class); + expectedViolations.from(RestAdapterLayerClass.class).to(CliAdapterLayerClass.class, PersistenceAdapterLayerClass.class); + return expectedViolations; + } + + private OnionArchitecture anOnionArchitectureWithEmptyLayers() { + return onionArchitecture() + .domainModels(absolute("onionarchitecture.domain.model.does.not.exist")) + .domainServices(absolute("onionarchitecture.domain.service.not.there")) + .applicationServices(absolute("onionarchitecture.application.http410")); + } + + static void assertFailureOnionArchitectureWithEmptyLayers(EvaluationResult result) { + assertThat(result.hasViolation()).as("result of evaluating empty layers has violation").isTrue(); + assertPatternMatches(result.getFailureReport().getDetails(), ImmutableSet.of( + expectedEmptyLayerPattern("adapter"), expectedEmptyLayerPattern("application service"), + expectedEmptyLayerPattern("domain model"), expectedEmptyLayerPattern("domain service") + )); + } + + private static class ExpectedOnionViolations { + private final Set expected; + + private ExpectedOnionViolations() { + this(new HashSet<>()); + } + + private ExpectedOnionViolations(Set expected) { + this.expected = expected; + } + + From from(Class from) { + return new From(from); + } + + private ExpectedOnionViolations add(ExpectedOnionViolation expectedOnionViolation) { + expected.add(expectedOnionViolation); + return this; + } + + public ExpectedOnionViolations withoutViolationsWithOrigin(Class clazz) { + return new ExpectedOnionViolations(expected.stream() + .filter(expectedViolation -> !expectedViolation.from.equals(clazz)) + .collect(toSet())); + } + + Set toPatterns() { + ImmutableSet.Builder result = ImmutableSet.builder(); + for (ExpectedOnionViolation expectedOnionViolation : expected) { + result.addAll(expectedOnionViolation.toPatterns()); + } + return result.build(); + } + + class From { + private final Class from; + + private From(Class from) { + this.from = from; + } + + ExpectedOnionViolations to(Class... to) { + return ExpectedOnionViolations.this.add(new ExpectedOnionViolation(from, to)); + } + } + } + + private static class ExpectedOnionViolation { + private final Class from; + private final Set> tos; + + private ExpectedOnionViolation(Class from, Class[] tos) { + this.from = from; + this.tos = ImmutableSet.copyOf(tos); + } + + Set toPatterns() { + ImmutableSet.Builder result = ImmutableSet.builder(); + for (Class to : tos) { + result.add(expectedAccessViolationPattern(from, "call", to, "callMe")) + .add(expectedFieldTypePattern(from, decapitalize(to.getSimpleName()), to)); + } + return result.build(); + } + } +} diff --git a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java index 9e036a0396..497d98c96a 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/dependencies/GivenSlicesTest.java @@ -10,7 +10,7 @@ import com.tngtech.archunit.lang.ConditionEvents; import com.tngtech.archunit.lang.EvaluationResult; import com.tngtech.archunit.lang.syntax.elements.GivenConjunction; -import com.tngtech.archunit.library.ArchitecturesTest; +import com.tngtech.archunit.library.LayeredArchitectureTest; import com.tngtech.archunit.library.dependencies.syntax.GivenSlices; import com.tngtech.archunit.testutil.ArchConfigurationRule; import org.junit.Rule; @@ -21,7 +21,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class GivenSlicesTest { - static final String TEST_CLASSES_PACKAGE = ArchitecturesTest.class.getPackage().getName() + ".testclasses"; + static final String TEST_CLASSES_PACKAGE = LayeredArchitectureTest.class.getPackage().getName() + ".testclasses"; @Rule public final ArchConfigurationRule archConfigurationRule = new ArchConfigurationRule(); From 7529901bdfea3556102007165be702d2f4967d2a Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Thu, 16 Jun 2022 22:27:11 +0700 Subject: [PATCH 2/2] add mandatory dependency settings to `layeredArchitecture()` In particular, since adding the possibility to define forward rules within the layered architecture (e.g. `whereLayer(..).mayOnlyAccessLayers(..)`) there has been quite some confusions about how to deal with false positives, like dependencies to `java.lang.Object`. While it is generally easy to exclude those via `.ignoreDependency(..)` this does not seem to be very intuitive for users as there have been multiple GitHub issues about how to deal with those dependencies. We now "force" users to make a (well-documented) decision how to deal with dependencies right when defining the layered architecture. The three predefined choices should cover the vast majority of user use cases (similar to `PlantUmlArchCondition`). For special cases where these options do not suffice it is always possible to simply pick `consideringAllDependencies()` and do fine grained `ignoreDependency(..)` calls later on. Signed-off-by: Peter Gafert --- .../junit4/LayeredArchitectureTest.java | 4 +- .../junit5/LayeredArchitectureTest.java | 4 +- .../exampletest/LayeredArchitectureTest.java | 4 +- .../archunit/ArchUnitArchitectureTest.java | 2 +- .../integration/ExamplesIntegrationTest.java | 2 +- .../archunit/library/Architectures.java | 169 ++++++++++++++++-- .../syntax/elements/GivenMembersTest.java | 10 +- .../library/LayeredArchitectureTest.java | 165 +++++++++++++---- ...ettingsOutsideOfLayersAccessingLayers.java | 7 + .../dependencysettings/allowed/Allowed.java | 7 + ...ySettingsForbiddenByMayOnlyBeAccessed.java | 4 + ...dencySettingsForbiddenByMayOnlyAccess.java | 9 + .../origin/DependencySettingsOriginClass.java | 11 ++ ...sOutsideOfLayersBeingAccessedByLayers.java | 4 + .../tngtech/archunit/testutil/TestUtils.java | 11 ++ docs/_pages/use-cases.md | 1 + docs/userguide/004_What_to_Check.adoc | 1 + 17 files changed, 351 insertions(+), 64 deletions(-) create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/DependencySettingsOutsideOfLayersAccessingLayers.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/allowed/Allowed.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_backwards/DependencySettingsForbiddenByMayOnlyBeAccessed.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_forwards/DependencySettingsForbiddenByMayOnlyAccess.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/origin/DependencySettingsOriginClass.java create mode 100644 archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings_outside/DependencySettingsOutsideOfLayersBeingAccessedByLayers.java diff --git a/archunit-example/example-junit4/src/test/java/com/tngtech/archunit/exampletest/junit4/LayeredArchitectureTest.java b/archunit-example/example-junit4/src/test/java/com/tngtech/archunit/exampletest/junit4/LayeredArchitectureTest.java index 2ad94527e3..5556d685f9 100644 --- a/archunit-example/example-junit4/src/test/java/com/tngtech/archunit/exampletest/junit4/LayeredArchitectureTest.java +++ b/archunit-example/example-junit4/src/test/java/com/tngtech/archunit/exampletest/junit4/LayeredArchitectureTest.java @@ -16,7 +16,7 @@ @AnalyzeClasses(packages = "com.tngtech.archunit.example.layers") public class LayeredArchitectureTest { @ArchTest - public static final ArchRule layer_dependencies_are_respected = layeredArchitecture() + public static final ArchRule layer_dependencies_are_respected = layeredArchitecture().consideringAllDependencies() .layer("Controllers").definedBy("com.tngtech.archunit.example.layers.controller..") .layer("Services").definedBy("com.tngtech.archunit.example.layers.service..") @@ -27,7 +27,7 @@ public class LayeredArchitectureTest { .whereLayer("Persistence").mayOnlyBeAccessedByLayers("Services"); @ArchTest - public static final ArchRule layer_dependencies_are_respected_with_exception = layeredArchitecture() + public static final ArchRule layer_dependencies_are_respected_with_exception = layeredArchitecture().consideringAllDependencies() .layer("Controllers").definedBy("com.tngtech.archunit.example.layers.controller..") .layer("Services").definedBy("com.tngtech.archunit.example.layers.service..") diff --git a/archunit-example/example-junit5/src/test/java/com/tngtech/archunit/exampletest/junit5/LayeredArchitectureTest.java b/archunit-example/example-junit5/src/test/java/com/tngtech/archunit/exampletest/junit5/LayeredArchitectureTest.java index 8d3d774b02..5cca03c794 100644 --- a/archunit-example/example-junit5/src/test/java/com/tngtech/archunit/exampletest/junit5/LayeredArchitectureTest.java +++ b/archunit-example/example-junit5/src/test/java/com/tngtech/archunit/exampletest/junit5/LayeredArchitectureTest.java @@ -13,7 +13,7 @@ @AnalyzeClasses(packages = "com.tngtech.archunit.example.layers") public class LayeredArchitectureTest { @ArchTest - static final ArchRule layer_dependencies_are_respected = layeredArchitecture() + static final ArchRule layer_dependencies_are_respected = layeredArchitecture().consideringAllDependencies() .layer("Controllers").definedBy("com.tngtech.archunit.example.layers.controller..") .layer("Services").definedBy("com.tngtech.archunit.example.layers.service..") @@ -24,7 +24,7 @@ public class LayeredArchitectureTest { .whereLayer("Persistence").mayOnlyBeAccessedByLayers("Services"); @ArchTest - static final ArchRule layer_dependencies_are_respected_with_exception = layeredArchitecture() + static final ArchRule layer_dependencies_are_respected_with_exception = layeredArchitecture().consideringAllDependencies() .layer("Controllers").definedBy("com.tngtech.archunit.example.layers.controller..") .layer("Services").definedBy("com.tngtech.archunit.example.layers.service..") diff --git a/archunit-example/example-plain/src/test/java/com/tngtech/archunit/exampletest/LayeredArchitectureTest.java b/archunit-example/example-plain/src/test/java/com/tngtech/archunit/exampletest/LayeredArchitectureTest.java index ff9879a9d4..a77db3407e 100644 --- a/archunit-example/example-plain/src/test/java/com/tngtech/archunit/exampletest/LayeredArchitectureTest.java +++ b/archunit-example/example-plain/src/test/java/com/tngtech/archunit/exampletest/LayeredArchitectureTest.java @@ -15,7 +15,7 @@ public class LayeredArchitectureTest { @Test public void layer_dependencies_are_respected() { - layeredArchitecture() + layeredArchitecture().consideringAllDependencies() .layer("Controllers").definedBy("com.tngtech.archunit.example.layers.controller..") .layer("Services").definedBy("com.tngtech.archunit.example.layers.service..") @@ -30,7 +30,7 @@ public void layer_dependencies_are_respected() { @Test public void layer_dependencies_are_respected_with_exception() { - layeredArchitecture() + layeredArchitecture().consideringAllDependencies() .layer("Controllers").definedBy("com.tngtech.archunit.example.layers.controller..") .layer("Services").definedBy("com.tngtech.archunit.example.layers.service..") diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/ArchUnitArchitectureTest.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/ArchUnitArchitectureTest.java index a1579b53b4..4b1234795f 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/ArchUnitArchitectureTest.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/ArchUnitArchitectureTest.java @@ -43,7 +43,7 @@ public class ArchUnitArchitectureTest { static final String THIRDPARTY_PACKAGE_IDENTIFIER = "..thirdparty.."; @ArchTest - public static final ArchRule layers_are_respected = layeredArchitecture() + public static final ArchRule layers_are_respected = layeredArchitecture().consideringAllDependencies() .layer("Root").definedBy("com.tngtech.archunit") .layer("Base").definedBy("com.tngtech.archunit.base..") .layer("Core").definedBy("com.tngtech.archunit.core..") diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java index 690eae7a7c..d405a7ea6c 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/integration/ExamplesIntegrationTest.java @@ -935,7 +935,7 @@ Stream LayeredArchitectureTest() { (memberName, expectedTestFailures) -> expectedTestFailures .ofRule(memberName, - "Layered architecture consisting of" + lineSeparator() + + "Layered architecture considering all dependencies, consisting of" + lineSeparator() + "layer 'Controllers' ('com.tngtech.archunit.example.layers.controller..')" + lineSeparator() + "layer 'Services' ('com.tngtech.archunit.example.layers.service..')" + lineSeparator() + "layer 'Persistence' ('com.tngtech.archunit.example.layers.persistence..')" + lineSeparator() + 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 6db2741060..a582a13d3a 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/Architectures.java @@ -25,6 +25,8 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.BiFunction; +import java.util.stream.Stream; import com.google.common.base.Joiner; import com.tngtech.archunit.PublicAPI; @@ -39,6 +41,7 @@ import com.tngtech.archunit.lang.EvaluationResult; import com.tngtech.archunit.lang.Priority; import com.tngtech.archunit.lang.syntax.PredicateAggregator; +import com.tngtech.archunit.library.Architectures.LayeredArchitecture.DependencySettings; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @@ -47,11 +50,15 @@ import static com.google.common.collect.Lists.newArrayList; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; import static com.tngtech.archunit.base.DescribedPredicate.alwaysFalse; +import static com.tngtech.archunit.base.DescribedPredicate.not; +import static com.tngtech.archunit.core.domain.Dependency.Functions.GET_ORIGIN_CLASS; +import static com.tngtech.archunit.core.domain.Dependency.Functions.GET_TARGET_CLASS; import static com.tngtech.archunit.core.domain.Dependency.Predicates.dependency; import static com.tngtech.archunit.core.domain.Dependency.Predicates.dependencyOrigin; import static com.tngtech.archunit.core.domain.Dependency.Predicates.dependencyTarget; import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo; import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAnyPackage; +import static com.tngtech.archunit.core.domain.JavaClass.Predicates.resideOutsideOfPackages; import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; import static com.tngtech.archunit.lang.SimpleConditionEvent.violated; import static com.tngtech.archunit.lang.conditions.ArchConditions.onlyHaveDependenciesWhere; @@ -100,32 +107,36 @@ private Architectures() { * @return An {@link ArchRule} enforcing the specified layered architecture */ @PublicAPI(usage = ACCESS) - public static LayeredArchitecture layeredArchitecture() { - return new LayeredArchitecture(); + public static DependencySettings layeredArchitecture() { + return new DependencySettings(); } public static final class LayeredArchitecture implements ArchRule { private final LayerDefinitions layerDefinitions; private final Set dependencySpecifications; + private final DependencySettings dependencySettings; private final PredicateAggregator irrelevantDependenciesPredicate; private final Optional overriddenDescription; private final boolean optionalLayers; - private LayeredArchitecture() { + private LayeredArchitecture(DependencySettings dependencySettings) { this(new LayerDefinitions(), - new LinkedHashSet(), + new LinkedHashSet<>(), + dependencySettings, new PredicateAggregator().thatORs(), - Optional.empty(), + Optional.empty(), false); } private LayeredArchitecture(LayerDefinitions layerDefinitions, Set dependencySpecifications, + DependencySettings dependencySettings, PredicateAggregator irrelevantDependenciesPredicate, Optional overriddenDescription, boolean optionalLayers) { this.layerDefinitions = layerDefinitions; this.dependencySpecifications = dependencySpecifications; + this.dependencySettings = dependencySettings; this.irrelevantDependenciesPredicate = irrelevantDependenciesPredicate; this.overriddenDescription = overriddenDescription; this.optionalLayers = optionalLayers; @@ -140,7 +151,14 @@ private LayeredArchitecture(LayerDefinitions layerDefinitions, */ @PublicAPI(usage = ACCESS) public LayeredArchitecture withOptionalLayers(boolean optionalLayers) { - return new LayeredArchitecture(layerDefinitions, dependencySpecifications, irrelevantDependenciesPredicate, overriddenDescription, optionalLayers); + return new LayeredArchitecture( + layerDefinitions, + dependencySpecifications, + dependencySettings, + irrelevantDependenciesPredicate, + overriddenDescription, + optionalLayers + ); } private LayeredArchitecture addLayerDefinition(LayerDefinition definition) { @@ -183,7 +201,8 @@ public String getDescription() { return overriddenDescription.get(); } - List lines = newArrayList("Layered architecture consisting of" + (optionalLayers ? " (optional)" : "")); + String prefix = "Layered architecture " + dependencySettings.description; + List lines = newArrayList(prefix + ", consisting of" + (optionalLayers ? " (optional)" : "")); for (LayerDefinition definition : layerDefinitions) { lines.add(definition.toString()); } @@ -253,10 +272,11 @@ private DescribedPredicate targetMatchesIfDependencyIsRelevant(Strin return ifDependencyIsRelevant(targetPackageMatches); } - private DescribedPredicate ifDependencyIsRelevant(DescribedPredicate originPackageMatches) { + private DescribedPredicate ifDependencyIsRelevant(DescribedPredicate predicate) { + DescribedPredicate configuredPredicate = dependencySettings.ignoreExcludedDependencies.apply(layerDefinitions, predicate); return irrelevantDependenciesPredicate.isPresent() ? - originPackageMatches.or(irrelevantDependenciesPredicate.get()) : - originPackageMatches; + configuredPredicate.or(irrelevantDependenciesPredicate.get()) : + configuredPredicate; } @Override @@ -284,8 +304,13 @@ public ArchRule allowEmptyShould(boolean allowEmptyShould) { @PublicAPI(usage = ACCESS) public LayeredArchitecture as(String newDescription) { return new LayeredArchitecture( - layerDefinitions, dependencySpecifications, - irrelevantDependenciesPredicate, Optional.of(newDescription), optionalLayers); + layerDefinitions, + dependencySpecifications, + dependencySettings, + irrelevantDependenciesPredicate, + Optional.of(newDescription), + optionalLayers + ); } /** @@ -316,8 +341,13 @@ public LayeredArchitecture ignoreDependency(String originFullyQualifiedClassName public LayeredArchitecture ignoreDependency( DescribedPredicate origin, DescribedPredicate target) { return new LayeredArchitecture( - layerDefinitions, dependencySpecifications, - irrelevantDependenciesPredicate.add(dependency(origin, target)), overriddenDescription, optionalLayers); + layerDefinitions, + dependencySpecifications, + dependencySettings, + irrelevantDependenciesPredicate.add(dependency(origin, target)), + overriddenDescription, + optionalLayers + ); } /** @@ -379,6 +409,10 @@ DescribedPredicate containsPredicateFor(String layerName) { return containsPredicateFor(singleton(layerName)); } + DescribedPredicate containsPredicateForAll() { + return containsPredicateFor(layerDefinitions.keySet()); + } + DescribedPredicate containsPredicateFor(final Collection layerNames) { DescribedPredicate result = alwaysFalse(); for (LayerDefinition definition : get(layerNames)) { @@ -515,6 +549,111 @@ public String toString() { return String.format("where layer '%s' %s", layerName, descriptionSuffix); } } + + /** + * Defines which dependencies the layered architecture will consider when checking for violations. Which dependencies + * are considered relevant depends on the context and the way to define the layered architecture (i.e. are the rules + * defined on incoming or outgoing dependencies).
+ * Each setting has advantages and disadvantages. Considering less dependencies makes the rules + * more convenient to write and reduces the number of false positives. On the other hand, it will also increase + * the likelihood to overlook some unexpected corner cases and thus allow some unexpected violations to creep in unnoticed. + */ + @PublicAPI(usage = ACCESS) + public static final class DependencySettings { + final String description; + final BiFunction, DescribedPredicate> ignoreExcludedDependencies; + + private DependencySettings() { + this(null, null); + } + + private DependencySettings(String description, BiFunction, DescribedPredicate> ignoreExcludedDependencies) { + this.description = description; + this.ignoreExcludedDependencies = ignoreExcludedDependencies; + } + + /** + * Defines {@link DependencySettings dependency settings} that consider all dependencies when checking for violations. + * With these settings even dependencies to {@link Object} can lead to violations of the layered architecture. + * However, if the rules are only defined on incoming dependencies (e.g. via + * {@link LayerDependencySpecification#mayOnlyBeAccessedByLayers(String...) mayOnlyBeAccessedByLayers(..)}) + * taking all dependencies into account usually works fine and provides a good level of security to detect corner cases + * (e.g. dependencies like {@code KnownLayer -> SomewhereCompletelyOutsideOfTheLayers -> IllegalTargetForKnownLayer}). + * + * @return {@link DependencySettings dependency settings} to be used when checking for violations of the layered architecture + */ + @PublicAPI(usage = ACCESS) + public LayeredArchitecture consideringAllDependencies() { + return new LayeredArchitecture(setToConsideringAllDependencies()); + } + + /** + * Defines {@link DependencySettings dependency settings} that consider only dependencies from/to certain packages, e.g. the app root. + * All dependencies that either have an origin or a target outside these packages will be ignored. + * When set to the root package(s) of the application under test this offers a good balance between eliminating false positives + * (like dependencies to {@link Object}) and preventing unexpected corner cases that conceal some existing violations + * (e.g. dependencies like {@code KnownLayer -> SomewhereCompletelyOutsideOfTheLayers -> IllegalTargetForKnownLayer}). + * + * @param packageIdentifier {@link PackageMatcher package identifier} defining which origins and targets of dependencies are relevant + * @param furtherPackageIdentifiers Additional {@link PackageMatcher package identifiers} defining relevant packages + * @return {@link DependencySettings dependency settings} to be used when checking for violations of the layered architecture + */ + @PublicAPI(usage = ACCESS) + public LayeredArchitecture consideringOnlyDependenciesInAnyPackage(String packageIdentifier, final String... furtherPackageIdentifiers) { + String[] packageIdentifiers = Stream.concat(Stream.of(packageIdentifier), Stream.of(furtherPackageIdentifiers)).toArray(String[]::new); + return new LayeredArchitecture(setToConsideringOnlyDependenciesInAnyPackage(packageIdentifiers)); + } + + /** + * Defines {@link DependencySettings dependency settings} that consider only dependencies between the layers. + * All dependencies that either have an origin or a target outside any defined layer will be ignored. + * This provides a high level of convenience to eliminate false positives (e.g. dependencies on {@link Object}), + * but also introduces some danger to overlook corner cases that might conceal some unwanted dependencies. + * Take for example a layered architecture + * 

+             * Controller(..controller..) -> Service(..service..) -> Persistence(..persistence..)
+ * then these {@link DependencySettings dependency settings} would e.g. not detect an unwanted dependency + * 

+             * myapp.service.MyService -> myapp.utils.SomeShadyUtils -> myapp.controller.MyController
+ * because {@code myapp.utils} is not part of any layer of the layered architecture. + * For a better balance to also detect such cases refer to {@link #consideringOnlyDependenciesInAnyPackage(String, String...)}. + * + * @return {@link DependencySettings dependency settings} to be used when checking for violations of the layered architecture + */ + @PublicAPI(usage = ACCESS) + public LayeredArchitecture consideringOnlyDependenciesInLayers() { + return new LayeredArchitecture(setToConsideringOnlyDependenciesInLayers()); + } + + private DependencySettings setToConsideringAllDependencies() { + return new DependencySettings( + "considering all dependencies", + (__, predicate) -> predicate + ); + } + + private DependencySettings setToConsideringOnlyDependenciesInAnyPackage(String[] packageIdentifiers) { + DescribedPredicate outsideOfRelevantPackage = resideOutsideOfPackages(packageIdentifiers); + return new DependencySettings( + String.format("considering only dependencies in any package ['%s']", Joiner.on("', '").join(packageIdentifiers)), + (__, predicate) -> predicate.or(originOrTargetIs(outsideOfRelevantPackage)) + ); + } + + private DependencySettings setToConsideringOnlyDependenciesInLayers() { + return new DependencySettings( + "considering only dependencies in layers", + (layerDefinitions, predicate) -> { + DescribedPredicate notInLayers = not(layerDefinitions.containsPredicateForAll()); + return predicate.or(originOrTargetIs(notInLayers)); + } + ); + } + + private DescribedPredicate originOrTargetIs(DescribedPredicate predicate) { + return GET_ORIGIN_CLASS.is(predicate).or(GET_TARGET_CLASS.is(predicate)); + } + } } @PublicAPI(usage = ACCESS) @@ -605,7 +744,7 @@ public OnionArchitecture ignoreDependency(DescribedPredicate } private LayeredArchitecture layeredArchitectureDelegate() { - LayeredArchitecture layeredArchitectureDelegate = layeredArchitecture() + LayeredArchitecture layeredArchitectureDelegate = layeredArchitecture().consideringAllDependencies() .layer(DOMAIN_MODEL_LAYER).definedBy(domainModelPackageIdentifiers) .layer(DOMAIN_SERVICE_LAYER).definedBy(domainServicePackageIdentifiers) .layer(APPLICATION_SERVICE_LAYER).definedBy(applicationPackageIdentifiers) diff --git a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java index 5d37b9d24d..3951f8c52e 100644 --- a/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/lang/syntax/elements/GivenMembersTest.java @@ -49,6 +49,7 @@ import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noFields; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noMembers; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noMethods; +import static com.tngtech.archunit.testutil.TestUtils.union; import static com.tngtech.java.junit.dataprovider.DataProviders.$; import static com.tngtech.java.junit.dataprovider.DataProviders.$$; import static java.util.Collections.emptySet; @@ -447,15 +448,6 @@ public void check(JavaMember item, ConditionEvents events) { }; } - @SafeVarargs - private static Set union(Set... sets) { - ImmutableSet.Builder result = ImmutableSet.builder(); - for (Set set : sets) { - result = result.addAll(set); - } - return result.build(); - } - static DescribedPredicate areNoFieldsWithType(final Class type) { return new DescribedPredicate("are no fields with type " + type.getSimpleName()) { @Override diff --git a/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java b/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java index 49663d6536..ca0d61f88d 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/LayeredArchitectureTest.java @@ -12,6 +12,12 @@ import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.lang.ArchRule; import com.tngtech.archunit.lang.EvaluationResult; +import com.tngtech.archunit.library.Architectures.LayeredArchitecture; +import com.tngtech.archunit.library.testclasses.dependencysettings.DependencySettingsOutsideOfLayersAccessingLayers; +import com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_backwards.DependencySettingsForbiddenByMayOnlyBeAccessed; +import com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_forwards.DependencySettingsForbiddenByMayOnlyAccess; +import com.tngtech.archunit.library.testclasses.dependencysettings.origin.DependencySettingsOriginClass; +import com.tngtech.archunit.library.testclasses.dependencysettings_outside.DependencySettingsOutsideOfLayersBeingAccessedByLayers; import com.tngtech.archunit.library.testclasses.first.any.pkg.FirstAnyPkgClass; import com.tngtech.archunit.library.testclasses.first.three.any.FirstThreeAnyClass; import com.tngtech.archunit.library.testclasses.mayonlyaccesslayers.forbidden.MayOnlyAccessLayersForbiddenClass; @@ -32,6 +38,7 @@ import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name; import static com.tngtech.archunit.library.Architectures.layeredArchitecture; import static com.tngtech.archunit.testutil.Assertions.assertThatRule; +import static com.tngtech.archunit.testutil.TestUtils.union; import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach; import static java.lang.System.lineSeparator; import static java.util.regex.Pattern.quote; @@ -46,6 +53,7 @@ public class LayeredArchitectureTest { public static Object[][] layeredArchitectureDefinitions() { return testForEach( layeredArchitecture() + .consideringAllDependencies() .layer("One").definedBy("..library.testclasses.some.pkg..") .layer("Two").definedBy("..library.testclasses.first.any.pkg..", "..library.testclasses.second.any.pkg..") .optionalLayer("Three").definedBy("..library.testclasses..three..") @@ -53,6 +61,7 @@ public static Object[][] layeredArchitectureDefinitions() { .whereLayer("Two").mayOnlyBeAccessedByLayers("One") .whereLayer("Three").mayOnlyBeAccessedByLayers("One", "Two"), layeredArchitecture() + .consideringAllDependencies() .layer("One").definedBy( resideInAnyPackage("..library.testclasses.some.pkg..") .as("'..library.testclasses.some.pkg..'")) @@ -67,18 +76,11 @@ public static Object[][] layeredArchitectureDefinitions() { .whereLayer("Three").mayOnlyBeAccessedByLayers("One", "Two")); } - static String[] absolute(String... pkgSuffix) { - return Arrays.stream(pkgSuffix) - .map(s -> OnionArchitectureTest.class.getPackage().getName() + ".testclasses." + s) - .map(absolute -> absolute.replaceAll("\\.\\.\\.+", "..")) - .toArray(String[]::new); - } - @Test @UseDataProvider("layeredArchitectureDefinitions") - public void layered_architecture_description(Architectures.LayeredArchitecture architecture) { + public void layered_architecture_description(LayeredArchitecture architecture) { assertThat(architecture.getDescription()).isEqualTo( - "Layered architecture consisting of" + lineSeparator() + + "Layered architecture considering all dependencies, consisting of" + lineSeparator() + "layer 'One' ('..library.testclasses.some.pkg..')" + lineSeparator() + "layer 'Two' ('..library.testclasses.first.any.pkg..', '..library.testclasses.second.any.pkg..')" + lineSeparator() + "optional layer 'Three' ('..library.testclasses..three..')" + lineSeparator() + @@ -89,7 +91,8 @@ public void layered_architecture_description(Architectures.LayeredArchitecture a @Test public void layered_architecture_overridden_description() { - Architectures.LayeredArchitecture architecture = layeredArchitecture() + LayeredArchitecture architecture = layeredArchitecture() + .consideringAllDependencies() .layer("One").definedBy("some.pkg..") .whereLayer("One").mayNotBeAccessedByAnyLayer() .as("overridden"); @@ -100,6 +103,7 @@ public void layered_architecture_overridden_description() { @Test public void layered_architecture_because_clause() { ArchRule architecture = layeredArchitecture() + .consideringAllDependencies() .layer("One").definedBy("some.pkg..") .whereLayer("One").mayNotBeAccessedByAnyLayer() .as("overridden") @@ -110,10 +114,10 @@ public void layered_architecture_because_clause() { @Test public void layered_architecture_defining_constraint_on_non_existing_target_layer_is_rejected() { - assertThatThrownBy(() -> - layeredArchitecture() - .layer("Some").definedBy("any") - .whereLayer("NotThere").mayNotBeAccessedByAnyLayer() + assertThatThrownBy(() -> layeredArchitecture() + .consideringAllDependencies() + .layer("Some").definedBy("any") + .whereLayer("NotThere").mayNotBeAccessedByAnyLayer() ) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("no layer") @@ -122,10 +126,10 @@ public void layered_architecture_defining_constraint_on_non_existing_target_laye @Test public void layered_architecture_defining_constraint_on_non_existing_origin_is_rejected() { - assertThatThrownBy(() -> - layeredArchitecture() - .layer("Some").definedBy("any") - .whereLayer("Some").mayOnlyBeAccessedByLayers("NotThere") + assertThatThrownBy(() -> layeredArchitecture() + .consideringAllDependencies() + .layer("Some").definedBy("any") + .whereLayer("Some").mayOnlyBeAccessedByLayers("NotThere") ) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("no layer") @@ -134,7 +138,7 @@ public void layered_architecture_defining_constraint_on_non_existing_origin_is_r @Test public void layered_architecture_rejects_empty_layers_by_default() { - Architectures.LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers(); + LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers(); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -144,8 +148,8 @@ public void layered_architecture_rejects_empty_layers_by_default() { @Test public void layered_architecture_allows_empty_layers_if_all_layers_are_optional() { - Architectures.LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(true); - assertThat(architecture.getDescription()).startsWith("Layered architecture consisting of (optional)"); + LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(true); + assertThat(architecture.getDescription()).startsWith("Layered architecture considering all dependencies, consisting of (optional)"); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -153,8 +157,8 @@ public void layered_architecture_allows_empty_layers_if_all_layers_are_optional( } @Test - public void layered_architecture_rejects_empty_layers_if_layers_are_explicity_not_optional_by_default() { - Architectures.LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(false); + public void layered_architecture_rejects_empty_layers_if_layers_are_explicitly_not_optional_by_default() { + LayeredArchitecture architecture = aLayeredArchitectureWithEmptyLayers().withOptionalLayers(false); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -162,8 +166,9 @@ public void layered_architecture_rejects_empty_layers_if_layers_are_explicity_no assertFailureLayeredArchitectureWithEmptyLayers(result); } - private Architectures.LayeredArchitecture aLayeredArchitectureWithEmptyLayers() { + private LayeredArchitecture aLayeredArchitectureWithEmptyLayers() { return layeredArchitecture() + .consideringAllDependencies() .layer("Some").definedBy(absolute("should.not.be.found..")) .layer("Other").definedBy(absolute("also.not.found")) .layer("Okay").definedBy("..testclasses..") @@ -178,7 +183,8 @@ private void assertFailureLayeredArchitectureWithEmptyLayers(EvaluationResult re @Test public void layered_architecture_allows_individual_empty_optionalLayer() { - Architectures.LayeredArchitecture architecture = layeredArchitecture() + LayeredArchitecture architecture = layeredArchitecture() + .consideringAllDependencies() .optionalLayer("can be absent").definedBy(absolute("should.not.be.found..")); JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); @@ -190,7 +196,7 @@ public void layered_architecture_allows_individual_empty_optionalLayer() { @Test @UseDataProvider("layeredArchitectureDefinitions") - public void layered_architecture_gathers_all_layer_violations(Architectures.LayeredArchitecture architecture) { + public void layered_architecture_gathers_all_layer_violations(LayeredArchitecture architecture) { JavaClasses classes = new ClassFileImporter().importPackages(absolute("")); EvaluationResult result = architecture.evaluate(classes); @@ -207,7 +213,8 @@ public void layered_architecture_gathers_all_layer_violations(Architectures.Laye @DataProvider public static Object[][] toIgnore() { - Architectures.LayeredArchitecture layeredArchitecture = layeredArchitecture() + LayeredArchitecture layeredArchitecture = layeredArchitecture() + .consideringAllDependencies() .layer("One").definedBy(absolute("some.pkg..")) .whereLayer("One").mayNotBeAccessedByAnyLayer(); @@ -245,7 +252,8 @@ public void layered_architecture_combines_multiple_ignores() { FirstAnyPkgClass.class, SomePkgSubclass.class, SecondThreeAnyClass.class, SomePkgClass.class); - Architectures.LayeredArchitecture layeredArchitecture = layeredArchitecture() + LayeredArchitecture layeredArchitecture = layeredArchitecture() + .consideringAllDependencies() .layer("One").definedBy(absolute("some.pkg..")) .whereLayer("One").mayNotBeAccessedByAnyLayer() .ignoreDependency(FirstAnyPkgClass.class, SomePkgSubclass.class); @@ -262,12 +270,14 @@ public void layered_architecture_combines_multiple_ignores() { public static Object[][] layeredArchitectureMayOnlyAccessLayersDefinitions() { return testForEach( layeredArchitecture() + .consideringAllDependencies() .layer("Allowed").definedBy("..library.testclasses.mayonlyaccesslayers.allowed..") .layer("Forbidden").definedBy("..library.testclasses.mayonlyaccesslayers.forbidden..") .layer("Origin").definedBy("..library.testclasses.mayonlyaccesslayers.origin..") .whereLayer("Origin").mayOnlyAccessLayers("Allowed") .whereLayer("Forbidden").mayNotAccessAnyLayer(), layeredArchitecture() + .consideringAllDependencies() .layer("Allowed").definedBy( resideInAnyPackage("..library.testclasses.mayonlyaccesslayers.allowed..") .as("'..library.testclasses.mayonlyaccesslayers.allowed..'")) @@ -283,9 +293,9 @@ public static Object[][] layeredArchitectureMayOnlyAccessLayersDefinitions() { @Test @UseDataProvider("layeredArchitectureMayOnlyAccessLayersDefinitions") - public void layered_architecture_may_only_access_layers_description(Architectures.LayeredArchitecture architecture) { + public void layered_architecture_may_only_access_layers_description(LayeredArchitecture architecture) { assertThat(architecture.getDescription()).isEqualTo( - "Layered architecture consisting of" + lineSeparator() + + "Layered architecture considering all dependencies, consisting of" + lineSeparator() + "layer 'Allowed' ('..library.testclasses.mayonlyaccesslayers.allowed..')" + lineSeparator() + "layer 'Forbidden' ('..library.testclasses.mayonlyaccesslayers.forbidden..')" + lineSeparator() + "layer 'Origin' ('..library.testclasses.mayonlyaccesslayers.origin..')" + lineSeparator() + @@ -295,7 +305,7 @@ public void layered_architecture_may_only_access_layers_description(Architecture @Test @UseDataProvider("layeredArchitectureMayOnlyAccessLayersDefinitions") - public void layered_architecture_gathers_may_only_access_layers_violations(Architectures.LayeredArchitecture architecture) { + public void layered_architecture_gathers_may_only_access_layers_violations(LayeredArchitecture architecture) { JavaClasses classes = new ClassFileImporter().importPackages(absolute("mayonlyaccesslayers")); EvaluationResult result = architecture.evaluate(classes); @@ -314,7 +324,7 @@ public void layered_architecture_gathers_may_only_access_layers_violations(Archi @Test @UseDataProvider("layeredArchitectureMayOnlyAccessLayersDefinitions") - public void layered_architecture_can_ignore_may_only_access_layers_violations(Architectures.LayeredArchitecture architecture) { + public void layered_architecture_can_ignore_may_only_access_layers_violations(LayeredArchitecture architecture) { JavaClasses classes = new ClassFileImporter().importPackages(absolute("mayonlyaccesslayers")); architecture = architecture.ignoreDependency(MayOnlyAccessLayersOriginClass.class, MayOnlyAccessLayersForbiddenClass.class) @@ -325,6 +335,97 @@ public void layered_architecture_can_ignore_may_only_access_layers_violations(Ar assertThat(architecture.evaluate(classes).hasViolation()).as("result has violation").isFalse(); } + @Test + public void layered_architecture_supports_dependency_setting_considering_all_dependencies() { + LayeredArchitecture layeredArchitecture = defineLayeredArchitectureForDependencySettings( + layeredArchitecture().consideringAllDependencies()); + + EvaluationResult result = layeredArchitecture.evaluate(new ClassFileImporter().importPackages(absolute("dependencysettings"))); + + assertThat(layeredArchitecture.getDescription()).startsWith("Layered architecture considering all dependencies, consisting of"); + assertPatternMatches(result.getFailureReport().getDetails(), + union( + dependencySettingsViolationsInLayers(), + dependencySettingsViolationsOutsideOfLayers(), + dependencySettingsViolationsByJavaLang() + )); + } + + @Test + public void layered_architecture_supports_dependency_setting_considering_only_dependencies_in_any_package() { + LayeredArchitecture layeredArchitecture = defineLayeredArchitectureForDependencySettings( + layeredArchitecture().consideringOnlyDependenciesInAnyPackage("..dependencysettings..", "..dependencysettings_outside..")); + + EvaluationResult result = layeredArchitecture.evaluate(new ClassFileImporter().importPackages(absolute("dependencysettings"))); + + assertThat(layeredArchitecture.getDescription()).startsWith( + "Layered architecture considering only dependencies in any package ['..dependencysettings..', '..dependencysettings_outside..'], consisting of"); + assertPatternMatches(result.getFailureReport().getDetails(), + union( + dependencySettingsViolationsInLayers(), + dependencySettingsViolationsOutsideOfLayers() + )); + } + + @Test + public void layered_architecture_supports_dependency_setting_considering_only_dependencies_in_layers() { + LayeredArchitecture layeredArchitecture = defineLayeredArchitectureForDependencySettings( + layeredArchitecture().consideringOnlyDependenciesInLayers()); + + EvaluationResult result = layeredArchitecture.evaluate(new ClassFileImporter().importPackages(absolute("dependencysettings"))); + + assertThat(layeredArchitecture.getDescription()).startsWith( + "Layered architecture considering only dependencies in layers, consisting of"); + assertPatternMatches(result.getFailureReport().getDetails(), dependencySettingsViolationsInLayers()); + } + + private LayeredArchitecture defineLayeredArchitectureForDependencySettings(LayeredArchitecture layeredArchitecture) { + return layeredArchitecture + .layer("Origin").definedBy("..library.testclasses.dependencysettings.origin..") + .layer("Allowed").definedBy("..library.testclasses.dependencysettings.allowed..") + .layer("ForbiddenByMayOnlyAccess").definedBy("..library.testclasses.dependencysettings.forbidden_forwards..") + .layer("ForbiddenByMayOnlyBeAccessed").definedBy("..library.testclasses.dependencysettings.forbidden_backwards..") + .whereLayer("Origin").mayOnlyAccessLayers("Allowed") + .whereLayer("Origin").mayNotBeAccessedByAnyLayer() + .whereLayer("ForbiddenByMayOnlyBeAccessed").mayNotBeAccessedByAnyLayer() + .whereLayer("ForbiddenByMayOnlyBeAccessed").mayNotAccessAnyLayer(); + } + + private Set dependencySettingsViolationsByJavaLang() { + return ImmutableSet.of( + expectedInheritancePattern(DependencySettingsOriginClass.class, Object.class), + expectedAccessViolationPattern(DependencySettingsOriginClass.class, CONSTRUCTOR_NAME, Object.class, CONSTRUCTOR_NAME), + expectedInheritancePattern(DependencySettingsForbiddenByMayOnlyBeAccessed.class, Object.class), + expectedAccessViolationPattern(DependencySettingsForbiddenByMayOnlyBeAccessed.class, CONSTRUCTOR_NAME, Object.class, CONSTRUCTOR_NAME) + ); + } + + private Set dependencySettingsViolationsOutsideOfLayers() { + return ImmutableSet.of( + expectedFieldTypePattern( + DependencySettingsOutsideOfLayersAccessingLayers.class, "origin", DependencySettingsOriginClass.class), + expectedFieldTypePattern( + DependencySettingsOriginClass.class, "beingAccessedByLayers", DependencySettingsOutsideOfLayersBeingAccessedByLayers.class) + ); + } + + private Set dependencySettingsViolationsInLayers() { + return ImmutableSet.of( + expectedFieldTypePattern( + DependencySettingsOriginClass.class, "forbiddenByMayOnlyAccess", DependencySettingsForbiddenByMayOnlyAccess.class), + expectedFieldTypePattern( + DependencySettingsForbiddenByMayOnlyAccess.class, "forbiddenByMayOnlyBeAccessed", DependencySettingsForbiddenByMayOnlyBeAccessed.class), + expectedFieldTypePattern( + DependencySettingsForbiddenByMayOnlyAccess.class, "origin", DependencySettingsOriginClass.class)); + } + + static String[] absolute(String... pkgSuffix) { + return Arrays.stream(pkgSuffix) + .map(s -> OnionArchitectureTest.class.getPackage().getName() + ".testclasses." + s) + .map(absolute -> absolute.replaceAll("\\.\\.\\.+", "..")) + .toArray(String[]::new); + } + private String singleLine(EvaluationResult result) { return Joiner.on(NEW_LINE_REPLACE).join(result.getFailureReport().getDetails()).replace("\n", NEW_LINE_REPLACE); } diff --git a/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/DependencySettingsOutsideOfLayersAccessingLayers.java b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/DependencySettingsOutsideOfLayersAccessingLayers.java new file mode 100644 index 0000000000..06effbc036 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/DependencySettingsOutsideOfLayersAccessingLayers.java @@ -0,0 +1,7 @@ +package com.tngtech.archunit.library.testclasses.dependencysettings; + +import com.tngtech.archunit.library.testclasses.dependencysettings.origin.DependencySettingsOriginClass; + +public class DependencySettingsOutsideOfLayersAccessingLayers { + DependencySettingsOriginClass origin; +} diff --git a/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/allowed/Allowed.java b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/allowed/Allowed.java new file mode 100644 index 0000000000..f6a4d7ab8c --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/allowed/Allowed.java @@ -0,0 +1,7 @@ +package com.tngtech.archunit.library.testclasses.dependencysettings.allowed; + +import com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_forwards.DependencySettingsForbiddenByMayOnlyAccess; + +public class Allowed { + DependencySettingsForbiddenByMayOnlyAccess notForbiddenFromHere; +} diff --git a/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_backwards/DependencySettingsForbiddenByMayOnlyBeAccessed.java b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_backwards/DependencySettingsForbiddenByMayOnlyBeAccessed.java new file mode 100644 index 0000000000..9ddb97a54a --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_backwards/DependencySettingsForbiddenByMayOnlyBeAccessed.java @@ -0,0 +1,4 @@ +package com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_backwards; + +public class DependencySettingsForbiddenByMayOnlyBeAccessed { +} diff --git a/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_forwards/DependencySettingsForbiddenByMayOnlyAccess.java b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_forwards/DependencySettingsForbiddenByMayOnlyAccess.java new file mode 100644 index 0000000000..88463adfff --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/forbidden_forwards/DependencySettingsForbiddenByMayOnlyAccess.java @@ -0,0 +1,9 @@ +package com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_forwards; + +import com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_backwards.DependencySettingsForbiddenByMayOnlyBeAccessed; +import com.tngtech.archunit.library.testclasses.dependencysettings.origin.DependencySettingsOriginClass; + +public class DependencySettingsForbiddenByMayOnlyAccess { + DependencySettingsOriginClass origin; + DependencySettingsForbiddenByMayOnlyBeAccessed forbiddenByMayOnlyBeAccessed; +} diff --git a/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/origin/DependencySettingsOriginClass.java b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/origin/DependencySettingsOriginClass.java new file mode 100644 index 0000000000..2d8490292b --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings/origin/DependencySettingsOriginClass.java @@ -0,0 +1,11 @@ +package com.tngtech.archunit.library.testclasses.dependencysettings.origin; + +import com.tngtech.archunit.library.testclasses.dependencysettings.allowed.Allowed; +import com.tngtech.archunit.library.testclasses.dependencysettings.forbidden_forwards.DependencySettingsForbiddenByMayOnlyAccess; +import com.tngtech.archunit.library.testclasses.dependencysettings_outside.DependencySettingsOutsideOfLayersBeingAccessedByLayers; + +public class DependencySettingsOriginClass { + Allowed allowed; + DependencySettingsForbiddenByMayOnlyAccess forbiddenByMayOnlyAccess; + DependencySettingsOutsideOfLayersBeingAccessedByLayers beingAccessedByLayers; +} diff --git a/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings_outside/DependencySettingsOutsideOfLayersBeingAccessedByLayers.java b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings_outside/DependencySettingsOutsideOfLayersBeingAccessedByLayers.java new file mode 100644 index 0000000000..14d77cb280 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/library/testclasses/dependencysettings_outside/DependencySettingsOutsideOfLayersBeingAccessedByLayers.java @@ -0,0 +1,4 @@ +package com.tngtech.archunit.library.testclasses.dependencysettings_outside; + +public class DependencySettingsOutsideOfLayersBeingAccessedByLayers { +} diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java b/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java index a7b1fd150f..9627701ccb 100644 --- a/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/TestUtils.java @@ -8,7 +8,9 @@ import java.util.Arrays; import java.util.Properties; import java.util.Random; +import java.util.Set; +import com.google.common.collect.ImmutableSet; import com.tngtech.archunit.core.domain.properties.HasName; import org.assertj.core.util.Files; @@ -87,4 +89,13 @@ public static URI relativeResourceUri(Class relativeToClass, String resourceN throw new RuntimeException(e); } } + + @SafeVarargs + public static Set union(Set... sets) { + ImmutableSet.Builder result = ImmutableSet.builder(); + for (Set set : sets) { + result = result.addAll(set); + } + return result.build(); + } } diff --git a/docs/_pages/use-cases.md b/docs/_pages/use-cases.md index 68f827ad63..d27ef4fe26 100644 --- a/docs/_pages/use-cases.md +++ b/docs/_pages/use-cases.md @@ -74,6 +74,7 @@ classes().that().areAssignableTo(EntityManager.class) ``` layeredArchitecture() + .consideringAllDependencies() .layer("Controller").definedBy("..controller..") .layer("Service").definedBy("..service..") .layer("Persistence").definedBy("..persistence..") diff --git a/docs/userguide/004_What_to_Check.adoc b/docs/userguide/004_What_to_Check.adoc index 980993c925..473b02fab6 100644 --- a/docs/userguide/004_What_to_Check.adoc +++ b/docs/userguide/004_What_to_Check.adoc @@ -258,6 +258,7 @@ note right on link #crimson: Access goes against layers [source,java] ---- layeredArchitecture() + .consideringAllDependencies() .layer("Controller").definedBy("..controller..") .layer("Service").definedBy("..service..") .layer("Persistence").definedBy("..persistence..")