From 47903b3acd1daef07ec33c8a989b7679636cf8f5 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sun, 13 Dec 2020 22:30:17 +0100 Subject: [PATCH] add component type dependencies to self Instead of manually adding component type reverse dependencies, we can simply reuse all dependencies from self for the reverse lookup. This does not only speed up the reverse lookup by about factor 2 on my machine, it also makes it trivial in the future to add reverse dependencies for any further new types of dependencies from self. Signed-off-by: Peter Gafert --- .../service/ServiceViolatingLayerRules.java | 13 +++- .../integration/ExamplesIntegrationTest.java | 63 +++++++++-------- .../testutils/ExpectedDependency.java | 5 ++ .../archunit/core/domain/JavaClass.java | 3 +- .../archunit/core/domain/JavaClasses.java | 4 +- .../core/domain/ReverseDependencies.java | 67 +++++++------------ .../archunit/core/domain/JavaClassTest.java | 39 ++++++++++- 7 files changed, 116 insertions(+), 78 deletions(-) diff --git a/archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/layers/service/ServiceViolatingLayerRules.java b/archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/layers/service/ServiceViolatingLayerRules.java index 6382e2c781..01ef8a850d 100644 --- a/archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/layers/service/ServiceViolatingLayerRules.java +++ b/archunit-example/example-plain/src/main/java/com/tngtech/archunit/example/layers/service/ServiceViolatingLayerRules.java @@ -9,6 +9,7 @@ import com.tngtech.archunit.example.layers.controller.two.UseCaseTwoController; import com.tngtech.archunit.example.layers.security.Secured; +@SuppressWarnings("unused") @MyService @ComplexServiceAnnotation( controllerAnnotation = @ComplexControllerAnnotation(simpleControllerAnnotation = @SimpleControllerAnnotation), @@ -17,9 +18,6 @@ serviceType = ServiceType.STANDARD ) public class ServiceViolatingLayerRules { - public static final String illegalAccessToController = "illegalAccessToController"; - public static final String doSomething = "doSomething"; - public static final String dependentMethod = "dependentMethod"; void illegalAccessToController() { System.out.println(UseCaseOneTwoController.someString); @@ -34,7 +32,16 @@ public SomeGuiController dependentMethod(UseCaseTwoController otherController) { return null; } + public SomeGuiController[][] dependentOnComponentTypeMethod(UseCaseTwoController[] otherController) { + return null; + } + @Secured public void properlySecured() { } + + public static final String illegalAccessToController = "illegalAccessToController"; + public static final String doSomething = "doSomething"; + public static final String dependentMethod = "dependentMethod"; + public static final String dependentOnComponentTypeMethod = "dependentOnComponentTypeMethod"; } 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 b72dc12c14..907d20b6bb 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 @@ -155,6 +155,7 @@ import static com.tngtech.archunit.example.layers.core.VeryCentralCore.DO_CORE_STUFF_METHOD_NAME; import static com.tngtech.archunit.example.layers.persistence.layerviolation.DaoCallingService.violateLayerRules; import static com.tngtech.archunit.example.layers.service.ServiceViolatingLayerRules.dependentMethod; +import static com.tngtech.archunit.example.layers.service.ServiceViolatingLayerRules.dependentOnComponentTypeMethod; import static com.tngtech.archunit.example.layers.service.ServiceViolatingLayerRules.illegalAccessToController; import static com.tngtech.archunit.testutils.CyclicErrorMatcher.cycle; import static com.tngtech.archunit.testutils.ExpectedAccess.callFromConstructor; @@ -251,7 +252,7 @@ private static void expectAccessToStandardStreams(ExpectedTestFailures expectFai .inLine(14)) .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .getting().field(System.class, "out") - .inLine(25)); + .inLine(23)); } private static void expectThrownGenericExceptions(ExpectedTestFailures expectFailures) { @@ -686,13 +687,13 @@ Stream LayerDependencyRulesTest() { "should access classes that reside in a package '..controller..'") .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .getting().field(UseCaseOneTwoController.class, someString) - .inLine(25)) + .inLine(23)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toConstructor(UseCaseTwoController.class) - .inLine(26)) + .inLine(24)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toMethod(UseCaseTwoController.class, doSomethingTwo) - .inLine(27)) + .inLine(25)) .ofRule("no classes that reside in a package '..persistence..' should " + "access classes that reside in a package '..service..'") @@ -719,27 +720,32 @@ Stream LayerDependencyRulesTest() { + "only access classes that reside in any package ['..service..', '..persistence..', 'java..']") .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .getting().field(UseCaseOneTwoController.class, UseCaseOneTwoController.someString) - .inLine(25)) + .inLine(23)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toConstructor(UseCaseTwoController.class) - .inLine(26)) + .inLine(24)) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toMethod(UseCaseTwoController.class, UseCaseTwoController.doSomethingTwo) - .inLine(27)) + .inLine(25)) .ofRule("no classes that reside in a package '..service..' " + "should depend on classes that reside in a package '..controller..'") .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .getting().field(UseCaseOneTwoController.class, someString) - .inLine(25).asDependency()) + .inLine(23).asDependency()) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toConstructor(UseCaseTwoController.class) - .inLine(26).asDependency()) + .inLine(24).asDependency()) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toMethod(UseCaseTwoController.class, doSomethingTwo) - .inLine(27).asDependency()) + .inLine(25).asDependency()) .by(method(ServiceViolatingLayerRules.class, dependentMethod).withParameter(UseCaseTwoController.class)) .by(method(ServiceViolatingLayerRules.class, dependentMethod).withReturnType(SomeGuiController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).withParameter(UseCaseTwoController[].class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).dependingOnComponentType(UseCaseTwoController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).withReturnType(SomeGuiController[][].class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).dependingOnComponentType(SomeGuiController[].class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).dependingOnComponentType(SomeGuiController.class)) .by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(ComplexControllerAnnotation.class)) .by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SimpleControllerAnnotation.class)) .by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SomeEnum.class)) @@ -783,23 +789,23 @@ Stream LayerDependencyRulesTest() { + "only depend on classes that reside in any package ['..service..', '..persistence..', 'java..', 'javax..']") .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .getting().field(UseCaseOneTwoController.class, someString) - .inLine(25).asDependency()) + .inLine(23).asDependency()) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toConstructor(UseCaseTwoController.class) - .inLine(26).asDependency()) + .inLine(24).asDependency()) .by(callFromMethod(ServiceViolatingLayerRules.class, illegalAccessToController) .toMethod(UseCaseTwoController.class, doSomethingTwo) - .inLine(27).asDependency()) - .by(method(ServiceViolatingLayerRules.class, dependentMethod) - .withParameter(UseCaseTwoController.class)) - .by(method(ServiceViolatingLayerRules.class, dependentMethod) - .withReturnType(SomeGuiController.class)) - .by(field(ServiceHelper.class, "properlySecured") - .withAnnotationType(Secured.class)) - .by(method(ServiceViolatingLayerRules.class, "properlySecured") - .withAnnotationType(Secured.class)) - .by(constructor(ServiceHelper.class) - .withAnnotationType(Secured.class)) + .inLine(25).asDependency()) + .by(method(ServiceViolatingLayerRules.class, dependentMethod).withParameter(UseCaseTwoController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentMethod).withReturnType(SomeGuiController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).withParameter(UseCaseTwoController[].class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).dependingOnComponentType(UseCaseTwoController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).withReturnType(SomeGuiController[][].class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).dependingOnComponentType(SomeGuiController[].class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod).dependingOnComponentType(SomeGuiController.class)) + .by(field(ServiceHelper.class, "properlySecured").withAnnotationType(Secured.class)) + .by(method(ServiceViolatingLayerRules.class, "properlySecured").withAnnotationType(Secured.class)) + .by(constructor(ServiceHelper.class).withAnnotationType(Secured.class)) .by(annotatedClass(ServiceViolatingDaoRules.class).annotatedWith(MyService.class)) .by(annotatedClass(ServiceViolatingLayerRules.class).annotatedWith(MyService.class)) .by(annotatedClass(ServiceImplementation.class).annotatedWith(MyService.class)) @@ -839,17 +845,17 @@ Stream LayeredArchitectureTest() { .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .toConstructor(UseCaseTwoController.class) - .inLine(26) + .inLine(24) .asDependency()) .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .toMethod(UseCaseTwoController.class, "doSomethingTwo") - .inLine(27) + .inLine(25) .asDependency()) .by(callFromMethod(ServiceViolatingLayerRules.class, "illegalAccessToController") .getting().field(UseCaseOneTwoController.class, "someString") - .inLine(25) + .inLine(23) .asDependency()) .by(callFromMethod(OtherJpa.class, "testConnection") @@ -858,8 +864,11 @@ Stream LayeredArchitectureTest() { .asDependency()) .by(method(ServiceViolatingLayerRules.class, dependentMethod).withParameter(UseCaseTwoController.class)) - .by(method(ServiceViolatingLayerRules.class, dependentMethod).withReturnType(SomeGuiController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod) + .dependingOnComponentType(UseCaseTwoController.class)) + .by(method(ServiceViolatingLayerRules.class, dependentOnComponentTypeMethod) + .dependingOnComponentType(SomeGuiController.class)) .by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(ComplexControllerAnnotation.class)) .by(annotatedClass(ServiceViolatingLayerRules.class).withAnnotationParameterType(SimpleControllerAnnotation.class)) diff --git a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedDependency.java b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedDependency.java index 71a7505b85..96d42baa74 100644 --- a/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedDependency.java +++ b/archunit-integration-test/src/test/java/com/tngtech/archunit/testutils/ExpectedDependency.java @@ -139,6 +139,11 @@ public ExpectedDependency withReturnType(Class returnType) { return new ExpectedDependency(owner, returnType, dependencyPattern); } + public ExpectedDependency dependingOnComponentType(Class componentType) { + String dependencyPattern = getDependencyPattern(getOriginName(), "depends on component type", componentType.getName(), 0); + return new ExpectedDependency(owner, componentType, dependencyPattern); + } + public ExpectedDependency withAnnotationType(Class annotationType) { return new ExpectedDependency(owner, annotationType, getDependencyPattern(getOriginName(), "is annotated with", annotationType.getName(), 0)); } diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java index 19f795d5a8..9a6fe851d2 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClass.java @@ -1295,12 +1295,13 @@ void completeAnnotations(final ImportContext context) { } } - void completeFrom(ImportContext context) { + JavaClassDependencies completeFrom(ImportContext context) { completeComponentType(context); for (JavaCodeUnit codeUnit : codeUnits) { codeUnit.completeFrom(context); } javaClassDependencies = new JavaClassDependencies(this); + return javaClassDependencies; } private void completeComponentType(ImportContext context) { diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClasses.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClasses.java index bdcc6d7cd2..b763d8f450 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClasses.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/JavaClasses.java @@ -197,8 +197,8 @@ static JavaClasses of( JavaPackage defaultPackage = JavaPackage.from(allClasses); for (JavaClass clazz : allClasses) { setPackage(clazz, defaultPackage); - clazz.completeFrom(importContext); - reverseDependenciesCreation.registerDependenciesOf(clazz); + JavaClassDependencies classDependencies = clazz.completeFrom(importContext); + reverseDependenciesCreation.registerDependenciesOf(clazz, classDependencies); } reverseDependenciesCreation.finish(allClasses); return new JavaClasses(defaultPackage, selectedClasses); diff --git a/archunit/src/main/java/com/tngtech/archunit/core/domain/ReverseDependencies.java b/archunit/src/main/java/com/tngtech/archunit/core/domain/ReverseDependencies.java index 76a143a3e4..3d93bb5f5c 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/domain/ReverseDependencies.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/domain/ReverseDependencies.java @@ -15,8 +15,12 @@ */ package com.tngtech.archunit.core.domain; +import java.util.ArrayList; +import java.util.List; import java.util.Set; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -39,7 +43,7 @@ final class ReverseDependencies { private final SetMultimap> annotationTypeDependencies; private final SetMultimap> annotationParameterTypeDependencies; private final SetMultimap instanceofCheckDependencies; - private final LoadingCache> directDependenciesToClassCache; + private final Supplier> directDependenciesToClass; private ReverseDependencies(ReverseDependencies.Creation creation) { accessToFieldCache = CacheBuilder.newBuilder().build(new ResolvingAccessLoader<>(creation.fieldAccessDependencies.build())); @@ -54,7 +58,22 @@ private ReverseDependencies(ReverseDependencies.Creation creation) { this.annotationTypeDependencies = creation.annotationTypeDependencies.build(); this.annotationParameterTypeDependencies = creation.annotationParameterTypeDependencies.build(); this.instanceofCheckDependencies = creation.instanceofCheckDependencies.build(); - this.directDependenciesToClassCache = CacheBuilder.newBuilder().build(new DependenciesToClassLoader()); + this.directDependenciesToClass = createDirectDependenciesToClassSupplier(creation.allDependencies); + } + + private static Supplier> createDirectDependenciesToClassSupplier(final List allDependencies) { + return Suppliers.memoize(new Supplier>() { + @Override + public SetMultimap get() { + ImmutableSetMultimap.Builder result = ImmutableSetMultimap.builder(); + for (JavaClassDependencies dependencies : allDependencies) { + for (Dependency dependency : dependencies.getDirectDependenciesFromClass()) { + result.put(dependency.getTargetClass(), dependency); + } + } + return result.build(); + } + }); } Set getAccessesTo(JavaField field) { @@ -106,7 +125,7 @@ Set getInstanceofChecksWithTypeOf(JavaClass clazz) { } Set getDirectDependenciesTo(JavaClass clazz) { - return directDependenciesToClassCache.getUnchecked(clazz); + return directDependenciesToClass.get().get(clazz); } static final ReverseDependencies EMPTY = new ReverseDependencies(new Creation()); @@ -124,14 +143,16 @@ static class Creation { private final ImmutableSetMultimap.Builder> annotationTypeDependencies = ImmutableSetMultimap.builder(); private final ImmutableSetMultimap.Builder> annotationParameterTypeDependencies = ImmutableSetMultimap.builder(); private final ImmutableSetMultimap.Builder instanceofCheckDependencies = ImmutableSetMultimap.builder(); + private final List allDependencies = new ArrayList<>(); - public void registerDependenciesOf(JavaClass clazz) { + public void registerDependenciesOf(JavaClass clazz, JavaClassDependencies classDependencies) { registerAccesses(clazz); registerFields(clazz); registerMethods(clazz); registerConstructors(clazz); registerAnnotations(clazz); registerStaticInitializer(clazz); + allDependencies.add(classDependencies); } private void registerAccesses(JavaClass clazz) { @@ -270,42 +291,4 @@ public Set load(JavaConstructor member) { return result.build(); } } - - private static class DependenciesToClassLoader extends CacheLoader> { - @Override - public Set load(JavaClass clazz) throws Exception { - ImmutableSet.Builder result = ImmutableSet.builder(); - for (JavaAccess access : clazz.getAccessesToSelf()) { - result.addAll(Dependency.tryCreateFromAccess(access)); - } - for (JavaClass subClass : clazz.getSubClasses()) { - result.add(Dependency.fromInheritance(subClass, clazz)); - } - for (JavaField field : clazz.getFieldsWithTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromField(field)); - } - for (JavaMethod method : clazz.getMethodsWithReturnTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromReturnType(method)); - } - for (JavaMethod method : clazz.getMethodsWithParameterTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromParameter(method, clazz)); - } - for (ThrowsDeclaration throwsDeclaration : clazz.getMethodThrowsDeclarationsWithTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromThrowsDeclaration(throwsDeclaration)); - } - for (JavaConstructor constructor : clazz.getConstructorsWithParameterTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromParameter(constructor, clazz)); - } - for (JavaAnnotation annotation : clazz.getAnnotationsWithTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromAnnotation(annotation)); - } - for (JavaAnnotation annotation : clazz.getAnnotationsWithParameterTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromAnnotationMember(annotation, clazz)); - } - for (InstanceofCheck instanceofCheck : clazz.getInstanceofChecksWithTypeOfSelf()) { - result.addAll(Dependency.tryCreateFromInstanceofCheck(instanceofCheck)); - } - return result.build(); - } - } } diff --git a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java index c401bb583c..c23d5e9218 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/domain/JavaClassTest.java @@ -539,8 +539,7 @@ public void direct_dependencies_from_self_by_annotation() { .areAtLeastOne(annotationMemberOfTypeDependency() .from(ClassWithAnnotationDependencies.class) .to(B.class) - .inLineNumber(0)) - ; + .inLineNumber(0)); } @Test @@ -711,13 +710,47 @@ public void direct_dependencies_to_self_by_annotation() { .inLineNumber(0)); } + @Test + public void finds_array_component_types_as_dependencies_to_self() { + JavaClass javaClass = new ClassFileImporter().importClasses(ArrayComponentTypeDependencies.class, ComponentTypeDependency.class) + .get(ComponentTypeDependency.class); + + assertThatDependencies(javaClass.getDirectDependenciesToSelf()) + .contain( + from(ArrayComponentTypeDependencies.class).to(ComponentTypeDependency.class) + .withDescriptionContaining("Field <%s.asField> depends on component type <%s>", + ArrayComponentTypeDependencies.class.getName(), ComponentTypeDependency.class.getName()) + .inLocation(ArrayComponentTypeDependencies.class, 0). + + from(ArrayComponentTypeDependencies.class).to(ComponentTypeDependency.class) + .withDescriptionContaining("Constructor <%s.(%s)> depends on component type <%s>", + ArrayComponentTypeDependencies.class.getName(), ComponentTypeDependency[].class.getName(), ComponentTypeDependency.class.getName()) + .inLocation(ArrayComponentTypeDependencies.class, 0). + + from(ArrayComponentTypeDependencies.class).to(ComponentTypeDependency.class) + .withDescriptionContaining("Method <%s.asMethodParameter(%s)> depends on component type <%s>", + ArrayComponentTypeDependencies.class.getName(), ComponentTypeDependency[].class.getName(), ComponentTypeDependency.class.getName()) + .inLocation(ArrayComponentTypeDependencies.class, 0). + + from(ArrayComponentTypeDependencies.class).to(ComponentTypeDependency.class) + .withDescriptionContaining("Method <%s.asReturnType()> depends on component type <%s>", + ArrayComponentTypeDependencies.class.getName(), ComponentTypeDependency.class.getName()) + .inLocation(ArrayComponentTypeDependencies.class, 0). + + from(ArrayComponentTypeDependencies.class).to(ComponentTypeDependency.class) + .withDescriptionContaining("Method <%s.asCallTarget()> depends on component type <%s>", + ArrayComponentTypeDependencies.class.getName(), ComponentTypeDependency.class.getName()) + .inLocation(ArrayComponentTypeDependencies.class, 18)); + } + @Test public void direct_dependencies_to_self_finds_correct_set_of_origin_types() { JavaClasses classes = importPackagesOf(getClass()); Set origins = getOriginsOfDependenciesTo(classes.get(WithType.class)); - assertThatTypes(origins).matchInAnyOrder(ClassWithAnnotationDependencies.class, ClassWithSelfReferences.class, OnMethodParam.class); + assertThatTypes(origins).matchInAnyOrder( + ClassWithAnnotationDependencies.class, ClassWithSelfReferences.class, WithNestedAnnotations.class, OnMethodParam.class); origins = getOriginsOfDependenciesTo(classes.get(B.class));