diff --git a/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java b/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java index 57fd5d9ac1..22198ce123 100644 --- a/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java +++ b/archunit-junit/junit4/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerInternal.java @@ -43,6 +43,7 @@ import static com.tngtech.archunit.junit.internal.ArchRuleDeclaration.elementShouldBeIgnored; import static com.tngtech.archunit.junit.internal.ArchRuleDeclaration.toDeclarations; import static com.tngtech.archunit.junit.internal.ArchTestExecution.getValue; +import static com.tngtech.archunit.junit.internal.ReflectionUtils.findAnnotation; import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static java.util.stream.Collectors.toList; @@ -53,7 +54,7 @@ final class ArchUnitRunnerInternal extends ParentRunner imple ArchUnitRunnerInternal(Class testClass) throws InitializationError { super(testClass); - checkAnnotation(testClass); + checkTestRunnable(testClass); try { ArchUnitSystemPropertyTestFilterJunit4.filter(this); @@ -62,12 +63,8 @@ final class ArchUnitRunnerInternal extends ParentRunner imple } } - private static AnalyzeClasses checkAnnotation(Class testClass) { - AnalyzeClasses analyzeClasses = testClass.getAnnotation(AnalyzeClasses.class); - ArchTestInitializationException.check(analyzeClasses != null, - "Class %s must be annotated with @%s", - testClass.getSimpleName(), AnalyzeClasses.class.getSimpleName()); - return analyzeClasses; + private static void checkTestRunnable(Class testClass) { + findAnnotation(testClass, AnalyzeClasses.class); } @Override @@ -180,7 +177,7 @@ private static class JUnit4ClassAnalysisRequest implements ClassAnalysisRequest private final AnalyzeClasses analyzeClasses; JUnit4ClassAnalysisRequest(Class testClass) { - analyzeClasses = checkAnnotation(testClass); + analyzeClasses = findAnnotation(testClass, AnalyzeClasses.class); } @Override diff --git a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerTest.java b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerTest.java index d1e1d1c13c..4027c7869b 100644 --- a/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerTest.java +++ b/archunit-junit/junit4/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitRunnerTest.java @@ -1,5 +1,6 @@ package com.tngtech.archunit.junit.internal; +import java.lang.annotation.Retention; import java.util.Set; import com.tngtech.archunit.core.domain.JavaClass; @@ -28,6 +29,7 @@ import static com.tngtech.archunit.core.domain.TestUtils.importClasses; import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes; +import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; @@ -49,7 +51,9 @@ public class ArchUnitRunnerTest { @InjectMocks private ArchUnitRunnerInternal runner = newRunner(SomeArchTest.class); @InjectMocks - private ArchUnitRunnerInternal runnerOfMaxTest = newRunner(MaxAnnotatedTest.class); + private ArchUnitRunnerInternal runnerOfMaxAnnotatedTest = newRunner(MaxAnnotatedTest.class); + @InjectMocks + private ArchUnitRunnerInternal runnerOfMetaAnnotatedTest = newRunner(MetaAnnotatedTest.class); @Before public void setUp() { @@ -58,7 +62,7 @@ public void setUp() { @Test public void runner_creates_correct_analysis_request() { - runnerOfMaxTest.run(new RunNotifier()); + runnerOfMaxAnnotatedTest.run(new RunNotifier()); verify(cache).getClassesToAnalyzeFor(eq(MaxAnnotatedTest.class), analysisRequestCaptor.capture()); @@ -92,10 +96,26 @@ public void rejects_missing_analyze_annotation() { ) .isInstanceOf(ArchTestInitializationException.class) .hasMessageContaining(Object.class.getSimpleName()) - .hasMessageContaining("must be annotated") + .hasMessageContaining("is not (meta-)annotated") .hasMessageContaining(AnalyzeClasses.class.getSimpleName()); } + @Test + public void runner_creates_correct_analysis_request_for_meta_annotated_class() { + runnerOfMetaAnnotatedTest.run(new RunNotifier()); + + verify(cache).getClassesToAnalyzeFor(eq(MetaAnnotatedTest.class), analysisRequestCaptor.capture()); + + AnalyzeClasses analyzeClasses = MetaAnnotatedTest.class.getAnnotation(MetaAnnotatedTest.MetaAnalyzeClasses.class) + .annotationType().getAnnotation(AnalyzeClasses.class); + ClassAnalysisRequest analysisRequest = analysisRequestCaptor.getValue(); + assertThat(analysisRequest.getPackageNames()).isEqualTo(analyzeClasses.packages()); + assertThat(analysisRequest.getPackageRoots()).isEqualTo(analyzeClasses.packagesOf()); + assertThat(analysisRequest.getLocationProviders()).isEqualTo(analyzeClasses.locations()); + assertThat(analysisRequest.scanWholeClasspath()).as("scan whole classpath").isTrue(); + assertThat(analysisRequest.getImportOptions()).isEqualTo(analyzeClasses.importOptions()); + } + private ArchUnitRunnerInternal newRunner(Class testClass) { try { return new ArchUnitRunnerInternal(testClass); @@ -160,4 +180,19 @@ public static class MaxAnnotatedTest { public static void someTest(JavaClasses classes) { } } + + @MetaAnnotatedTest.MetaAnalyzeClasses + public static class MetaAnnotatedTest { + @ArchTest + public static void someTest(JavaClasses classes) { + } + + @Retention(RUNTIME) + @AnalyzeClasses( + packages = {"com.foo", "com.bar"}, + wholeClasspath = true + ) + public @interface MetaAnalyzeClasses { + } + } } diff --git a/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java b/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java index 87071f98f6..b8f83d3174 100644 --- a/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java +++ b/archunit-junit/junit5/engine/src/main/java/com/tngtech/archunit/junit/internal/ArchUnitTestDescriptor.java @@ -39,12 +39,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static com.google.common.base.Preconditions.checkArgument; import static com.tngtech.archunit.junit.internal.DisplayNameResolver.determineDisplayName; +import static com.tngtech.archunit.junit.internal.ReflectionUtils.findAnnotation; import static com.tngtech.archunit.junit.internal.ReflectionUtils.getAllFields; import static com.tngtech.archunit.junit.internal.ReflectionUtils.getAllMethods; import static com.tngtech.archunit.junit.internal.ReflectionUtils.getValueOrThrowException; import static com.tngtech.archunit.junit.internal.ReflectionUtils.invokeMethod; +import static com.tngtech.archunit.junit.internal.ReflectionUtils.tryFindAnnotation; import static com.tngtech.archunit.junit.internal.ReflectionUtils.withAnnotation; class ArchUnitTestDescriptor extends AbstractArchUnitTestDescriptor implements CreatesChildren { @@ -71,8 +72,8 @@ static void resolve(TestDescriptor parent, ElementResolver resolver, ClassCache } private static void createTestDescriptor(TestDescriptor parent, ClassCache classCache, Class clazz, ElementResolver childResolver) { - if (clazz.getAnnotation(AnalyzeClasses.class) == null) { - LOG.warn("Class {} is not annotated with @{} and thus cannot run as a top level test. " + if (!tryFindAnnotation(clazz, AnalyzeClasses.class).isPresent()) { + LOG.warn("Class {} is not (meta-)annotated with @{} and thus cannot run as a top level test. " + "This warning can be ignored if {} is only used as part of a rules library included via {}.in({}.class).", clazz.getName(), AnalyzeClasses.class.getSimpleName(), clazz.getSimpleName(), ArchTests.class.getSimpleName(), clazz.getSimpleName()); @@ -291,15 +292,7 @@ private static class JUnit5ClassAnalysisRequest implements ClassAnalysisRequest private final AnalyzeClasses analyzeClasses; JUnit5ClassAnalysisRequest(Class testClass) { - analyzeClasses = checkAnnotation(testClass); - } - - private static AnalyzeClasses checkAnnotation(Class testClass) { - AnalyzeClasses analyzeClasses = testClass.getAnnotation(AnalyzeClasses.class); - checkArgument(analyzeClasses != null, - "Class %s must be annotated with @%s", - testClass.getSimpleName(), AnalyzeClasses.class.getSimpleName()); - return analyzeClasses; + analyzeClasses = findAnnotation(testClass, AnalyzeClasses.class); } @Override diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java index 1fac69668c..449c2f3269 100644 --- a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java +++ b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/ArchUnitTestEngineTest.java @@ -27,6 +27,7 @@ import com.tngtech.archunit.junit.internal.testexamples.FullAnalyzeClassesSpec; import com.tngtech.archunit.junit.internal.testexamples.LibraryWithPrivateTests; import com.tngtech.archunit.junit.internal.testexamples.SimpleRuleLibrary; +import com.tngtech.archunit.junit.internal.testexamples.TestClassWithMetaAnnotationForAnalyzeClasses; import com.tngtech.archunit.junit.internal.testexamples.TestClassWithMetaTag; import com.tngtech.archunit.junit.internal.testexamples.TestClassWithMetaTags; import com.tngtech.archunit.junit.internal.testexamples.TestClassWithTags; @@ -169,6 +170,20 @@ void a_single_test_class() { assertThat(child.getParent().get()).isEqualTo(descriptor); } + @Test + void a_test_class_with_meta_annotated_analyze_classes() { + EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(TestClassWithMetaAnnotationForAnalyzeClasses.class); + + TestDescriptor descriptor = testEngine.discover(discoveryRequest, engineId); + + TestDescriptor child = getOnlyElement(descriptor.getChildren()); + assertThat(child).isInstanceOf(ArchUnitTestDescriptor.class); + assertThat(child.getUniqueId()).isEqualTo(engineId.append(CLASS_SEGMENT_TYPE, TestClassWithMetaAnnotationForAnalyzeClasses.class.getName())); + assertThat(child.getDisplayName()).isEqualTo(TestClassWithMetaAnnotationForAnalyzeClasses.class.getSimpleName()); + assertThat(child.getType()).isEqualTo(CONTAINER); + assertThat(child.getParent()).get().isEqualTo(descriptor); + } + @Test void source_of_a_single_test_class() { EngineDiscoveryTestRequest discoveryRequest = new EngineDiscoveryTestRequest().withClass(SimpleRuleField.class); @@ -505,10 +520,10 @@ void mixed_class_methods_and_fields() { expectedLeafIds.add(simpleRuleFieldTestId(engineId)); expectedLeafIds.add(simpleRuleMethodTestId(engineId)); Stream.concat( - SimpleRules.RULE_FIELD_NAMES.stream().map(fieldName -> - simpleRulesId(engineId).append(FIELD_SEGMENT_TYPE, fieldName)), - SimpleRules.RULE_METHOD_NAMES.stream().map(methodName -> - simpleRulesId(engineId).append(METHOD_SEGMENT_TYPE, methodName))) + SimpleRules.RULE_FIELD_NAMES.stream().map(fieldName -> + simpleRulesId(engineId).append(FIELD_SEGMENT_TYPE, fieldName)), + SimpleRules.RULE_METHOD_NAMES.stream().map(methodName -> + simpleRulesId(engineId).append(METHOD_SEGMENT_TYPE, methodName))) .forEach(expectedLeafIds::add); assertThat(getAllLeafUniqueIds(rootDescriptor)) @@ -1074,6 +1089,21 @@ void cache_is_cleared_afterwards() { verify(classCache, atLeastOnce()).getClassesToAnalyzeFor(any(Class.class), any(ClassAnalysisRequest.class)); verifyNoMoreInteractions(classCache); } + + @Test + void a_class_with_analyze_classes_as_meta_annotation() { + execute(createEngineId(), TestClassWithMetaAnnotationForAnalyzeClasses.class); + + verify(classCache).getClassesToAnalyzeFor(eq(TestClassWithMetaAnnotationForAnalyzeClasses.class), classAnalysisRequestCaptor.capture()); + ClassAnalysisRequest request = classAnalysisRequestCaptor.getValue(); + AnalyzeClasses expected = TestClassWithMetaAnnotationForAnalyzeClasses.class.getAnnotation(TestClassWithMetaAnnotationForAnalyzeClasses.MetaAnalyzeClasses.class) + .annotationType().getAnnotation(AnalyzeClasses.class); + assertThat(request.getPackageNames()).isEqualTo(expected.packages()); + assertThat(request.getPackageRoots()).isEqualTo(expected.packagesOf()); + assertThat(request.getLocationProviders()).isEqualTo(expected.locations()); + assertThat(request.scanWholeClasspath()).as("scan whole classpath").isTrue(); + assertThat(request.getImportOptions()).isEqualTo(expected.importOptions()); + } } @Nested diff --git a/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/TestClassWithMetaAnnotationForAnalyzeClasses.java b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/TestClassWithMetaAnnotationForAnalyzeClasses.java new file mode 100644 index 0000000000..3141f35de0 --- /dev/null +++ b/archunit-junit/junit5/engine/src/test/java/com/tngtech/archunit/junit/internal/testexamples/TestClassWithMetaAnnotationForAnalyzeClasses.java @@ -0,0 +1,21 @@ +package com.tngtech.archunit.junit.internal.testexamples; + +import java.lang.annotation.Retention; + +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.lang.ArchRule; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@TestClassWithMetaAnnotationForAnalyzeClasses.MetaAnalyzeClasses +public class TestClassWithMetaAnnotationForAnalyzeClasses { + + @ArchTest + public static final ArchRule rule_in_class_with_meta_analyze_class_annotation = RuleThatFails.on(UnwantedClass.class); + + @Retention(RUNTIME) + @AnalyzeClasses(wholeClasspath = true) + public @interface MetaAnalyzeClasses { + } +} diff --git a/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ArchTestInitializationException.java b/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ArchTestInitializationException.java index 947dbe56cb..b6765b7bd4 100644 --- a/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ArchTestInitializationException.java +++ b/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ArchTestInitializationException.java @@ -1,7 +1,7 @@ package com.tngtech.archunit.junit.internal; class ArchTestInitializationException extends RuntimeException { - private ArchTestInitializationException(String message, Object... args) { + ArchTestInitializationException(String message, Object... args) { super(String.format(message, args)); } diff --git a/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ReflectionUtils.java b/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ReflectionUtils.java index 5810f575db..bb0117a080 100644 --- a/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ReflectionUtils.java +++ b/archunit-junit/src/main/java/com/tngtech/archunit/junit/internal/ReflectionUtils.java @@ -23,6 +23,8 @@ import java.lang.reflect.Modifier; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; +import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; @@ -38,20 +40,6 @@ class ReflectionUtils { private ReflectionUtils() { } - static Set> getAllSupertypes(Class type) { - if (type == null) { - return Collections.emptySet(); - } - - ImmutableSet.Builder> result = ImmutableSet.>builder() - .add(type) - .addAll(getAllSupertypes(type.getSuperclass())); - for (Class c : type.getInterfaces()) { - result.addAll(getAllSupertypes(c)); - } - return result.build(); - } - static Collection getAllFields(Class owner, Predicate predicate) { return getAll(owner, Class::getDeclaredFields).filter(predicate).collect(toList()); } @@ -68,18 +56,22 @@ private static Stream getAll(Class type, Function, T[]> colle return result.build(); } - static T newInstanceOf(Class type) { - return com.tngtech.archunit.base.ReflectionUtils.newInstanceOf(type); - } + private static Set> getAllSupertypes(Class type) { + if (type == null) { + return Collections.emptySet(); + } - @SuppressWarnings("unchecked") // callers must know, what they do here, we can't make this compile safe anyway - private static T getValue(Field field, Object owner) { - try { - field.setAccessible(true); - return (T) field.get(owner); - } catch (IllegalAccessException e) { - throw new ReflectionException(e); + ImmutableSet.Builder> result = ImmutableSet.>builder() + .add(type) + .addAll(getAllSupertypes(type.getSuperclass())); + for (Class c : type.getInterfaces()) { + result.addAll(getAllSupertypes(c)); } + return result.build(); + } + + static T newInstanceOf(Class type) { + return com.tngtech.archunit.base.ReflectionUtils.newInstanceOf(type); } static T getValueOrThrowException(Field field, Class fieldOwner, Function exceptionConverter) { @@ -94,6 +86,16 @@ static T getValueOrThrowException(Field field, Class fieldOwner, Function } } + @SuppressWarnings("unchecked") // callers must know what they do here, we can't make this compile safe anyway + private static T getValue(Field field, Object owner) { + try { + field.setAccessible(true); + return (T) field.get(owner); + } catch (IllegalAccessException e) { + throw new ReflectionException(e); + } + } + static T invokeMethod(Method method, Class methodOwner, Object... args) { if (Modifier.isStatic(method.getModifiers())) { return invoke(null, method, args); @@ -102,7 +104,7 @@ static T invokeMethod(Method method, Class methodOwner, Object... args) { } } - @SuppressWarnings("unchecked") // callers must know, what they do here, we can't make this compile safe anyway + @SuppressWarnings("unchecked") // callers must know what they do here, we can't make this compile safe anyway private static T invoke(Object owner, Method method, Object... args) { method.setAccessible(true); try { @@ -124,4 +126,41 @@ private static void rethrowUnchecked(Throwable throwable) static Predicate withAnnotation(Class annotationType) { return input -> input.isAnnotationPresent(annotationType); } + + /** + * Same as {@link #tryFindAnnotation(Class, Class)}, but throws an exception if no annotation can be found. + */ + static T findAnnotation(final Class clazz, Class annotationType) { + return tryFindAnnotation(clazz, annotationType).orElseThrow(() -> + new ArchTestInitializationException("Class %s is not (meta-)annotated with @%s", clazz.getName(), annotationType.getSimpleName())); + } + + /** + * Recursively searches for an annotation of type {@link T} on the given {@code clazz}. + * Returns the first matching annotation that is found. + * Any further matching annotation possibly present within the meta-annotation hierarchy will be ignored. + * If no matching annotation can be found {@link Optional#empty()} will be returned. + * + * @param clazz The {@link Class} from which to retrieve the annotation + * @return The first found annotation instance reachable in the meta-annotation hierarchy or {@link Optional#empty()} if none can be found + */ + static Optional tryFindAnnotation(final Class clazz, Class annotationType) { + return tryFindAnnotation(clazz.getAnnotations(), annotationType, new HashSet<>()); + } + + private static Optional tryFindAnnotation(final Annotation[] annotations, Class annotationType, Set visited) { + for (Annotation annotation : annotations) { + if (!visited.add(annotation)) { + continue; + } + + Optional result = annotationType.isInstance(annotation) + ? Optional.of(annotationType.cast(annotation)) + : tryFindAnnotation(annotation.annotationType().getAnnotations(), annotationType, visited); + if (result.isPresent()) { + return result; + } + } + return Optional.empty(); + } } diff --git a/archunit-junit/src/test/java/com/tngtech/archunit/junit/internal/ReflectionUtilsTest.java b/archunit-junit/src/test/java/com/tngtech/archunit/junit/internal/ReflectionUtilsTest.java index 0f8796f611..1872f54ddd 100644 --- a/archunit-junit/src/test/java/com/tngtech/archunit/junit/internal/ReflectionUtilsTest.java +++ b/archunit-junit/src/test/java/com/tngtech/archunit/junit/internal/ReflectionUtilsTest.java @@ -1,19 +1,23 @@ package com.tngtech.archunit.junit.internal; +import java.lang.annotation.Retention; import java.lang.reflect.Field; import java.lang.reflect.Member; import java.lang.reflect.Method; import java.util.Collection; import java.util.function.Predicate; -import org.junit.Test; +import org.junit.jupiter.api.Test; import static com.tngtech.archunit.base.Predicates.alwaysTrue; import static com.tngtech.archunit.testutil.ReflectionTestUtils.field; import static com.tngtech.archunit.testutil.ReflectionTestUtils.method; +import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class ReflectionUtilsTest { + @Test public void getAllFields() { Collection fields = ReflectionUtils.getAllFields(Child.class, named("field")); @@ -38,14 +42,6 @@ public void getAllMethods() { ); } - @Test - public void getAllSupertypes() { - assertThat(ReflectionUtils.getAllSupertypes(Child.class)).containsOnly( - Child.class, ChildInterface.class, UpperMiddle.class, LowerMiddle.class, Parent.class, - SomeInterface.class, OtherInterface.class, Object.class - ); - } - @Test public void getAllMethods_of_interface() { assertThat(ReflectionUtils.getAllMethods(Subinterface.class, alwaysTrue())) @@ -62,6 +58,41 @@ public void getAllFields_of_interface() { field(OtherInterface.class, "OTHER_CONSTANT")); } + @Test + public void findAnnotation_should_find_direct_annotation() { + SomeAnnotation actual = ReflectionUtils.findAnnotation(DirectlyAnnotated.class, SomeAnnotation.class); + + assertThat(actual).isInstanceOf(SomeAnnotation.class); + assertThat(actual.value()).as(SomeAnnotation.class.getSimpleName() + ".value()").isEqualTo("default"); + } + + @Test + public void findAnnotation_should_find_meta_annotation() { + SomeAnnotation actual = ReflectionUtils.findAnnotation(MetaAnnotated.class, SomeAnnotation.class); + + assertThat(actual).isInstanceOf(SomeAnnotation.class); + assertThat(actual.value()).as(SomeAnnotation.class.getSimpleName() + ".value()").isEqualTo("Meta-Annotation"); + } + + @Test + void findAnnotation_should_reject_annotation_missing_from_hierarchy() { + assertThatThrownBy(() -> ReflectionUtils.findAnnotation(Object.class, SomeAnnotation.class)) + .isInstanceOf(ArchTestInitializationException.class) + .hasMessage("Class %s is not (meta-)annotated with @%s", Object.class.getName(), SomeAnnotation.class.getSimpleName()); + } + + @Test + void tryFindAnnotation_should_return_empty_when_annotation_missing_from_hierarchy() { + assertThat(ReflectionUtils.tryFindAnnotation(Object.class, SomeAnnotation.class)).as("Optional.of(SomeAnnotation)").isEmpty(); + } + + @Test + void findAnnotation_should_return_depth_first_result_of_multiple_annotations_in_hierarchy() { + SomeAnnotation actual = ReflectionUtils.findAnnotation(AnnotationMultipleTimesInHierarchy.class, SomeAnnotation.class); + + assertThat(actual.value()).isEqualTo("default"); + } + private Predicate named(String name) { return input -> input.getName().equals(name); } @@ -157,4 +188,27 @@ private interface OtherInterface { private interface Subinterface extends SomeInterface, OtherInterface { } + + @Retention(RUNTIME) + private @interface SomeAnnotation { + String value() default "default"; + } + + @SomeAnnotation + private static class DirectlyAnnotated { + } + + @Retention(RUNTIME) + @SomeAnnotation("Meta-Annotation") + private @interface MetaAnnotatedWithSomeAnnotation { + } + + @MetaAnnotatedWithSomeAnnotation + private static class MetaAnnotated { + } + + @SomeAnnotation + @MetaAnnotatedWithSomeAnnotation + private static class AnnotationMultipleTimesInHierarchy { + } } diff --git a/archunit/src/main/java/com/tngtech/archunit/library/freeze/StoreEmptyException.java b/archunit/src/main/java/com/tngtech/archunit/library/freeze/StoreEmptyException.java new file mode 100644 index 0000000000..bdb4af70a7 --- /dev/null +++ b/archunit/src/main/java/com/tngtech/archunit/library/freeze/StoreEmptyException.java @@ -0,0 +1,23 @@ +/* + * Copyright 2014-2025 TNG Technology Consulting GmbH + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.tngtech.archunit.library.freeze; + +class StoreEmptyException extends RuntimeException { + StoreEmptyException(String message) { + super(message); + } + +} diff --git a/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java b/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java index 05eba8224a..2caf86fd1e 100644 --- a/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java +++ b/archunit/src/main/java/com/tngtech/archunit/library/freeze/TextFileBasedViolationStore.java @@ -73,11 +73,17 @@ public final class TextFileBasedViolationStore implements ViolationStore { private static final String ALLOW_STORE_CREATION_DEFAULT = "false"; private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "default.allowStoreUpdate"; private static final String ALLOW_STORE_UPDATE_DEFAULT = "true"; + private static final String DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.deleteEmptyRuleViolation"; + private static final String DELETE_EMPTY_RULE_VIOLATION_DEFAULT = "false"; + private static final String WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "default.warnEmptyRuleViolation"; + private static final String WARN_EMPTY_RULE_VIOLATION_DEFAULT = "false"; private final RuleViolationFileNameStrategy ruleViolationFileNameStrategy; private boolean storeCreationAllowed; private boolean storeUpdateAllowed; + private boolean deleteEmptyRule; + private boolean warnEmptyRuleViolation; private File storeFolder; private FileSyncedProperties storedRules; @@ -103,6 +109,8 @@ public TextFileBasedViolationStore(RuleViolationFileNameStrategy ruleViolationFi public void initialize(Properties properties) { storeCreationAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, ALLOW_STORE_CREATION_DEFAULT)); storeUpdateAllowed = Boolean.parseBoolean(properties.getProperty(ALLOW_STORE_UPDATE_PROPERTY_NAME, ALLOW_STORE_UPDATE_DEFAULT)); + deleteEmptyRule = Boolean.parseBoolean(properties.getProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, DELETE_EMPTY_RULE_VIOLATION_DEFAULT)); + warnEmptyRuleViolation = Boolean.parseBoolean(properties.getProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, WARN_EMPTY_RULE_VIOLATION_DEFAULT)); String path = properties.getProperty(STORE_PATH_PROPERTY_NAME, STORE_PATH_DEFAULT); storeFolder = new File(path); ensureExistence(storeFolder); @@ -140,15 +148,38 @@ public boolean contains(ArchRule rule) { @Override public void save(ArchRule rule, List violations) { log.trace("Storing evaluated rule '{}' with {} violations: {}", rule.getDescription(), violations.size(), violations); + if (violations.isEmpty() && warnEmptyRuleViolation) { + throw new StoreEmptyException(String.format("Saving empty violations for freezing rule is disabled (enable by configuration %s.%s=true)", + ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME)); + } + if (violations.isEmpty() && deleteEmptyRule && !contains(rule)) { + // do nothing, new rule file should not be created + return; + } if (!storeUpdateAllowed) { throw new StoreUpdateFailedException(String.format( "Updating frozen violations is disabled (enable by configuration %s.%s=true)", ViolationStoreFactory.FREEZE_STORE_PROPERTY_NAME, ALLOW_STORE_UPDATE_PROPERTY_NAME)); } + if (violations.isEmpty() && deleteEmptyRule) { + deleteRuleFile(rule); + return; + } + String ruleFileName = ensureRuleFileName(rule); write(violations, new File(storeFolder, ruleFileName)); } + private void deleteRuleFile(ArchRule rule) { + try { + String ruleFileName = storedRules.getProperty(rule.getDescription()); + Files.delete(storeFolder.toPath().resolve(ruleFileName)); + } catch (IOException e) { + throw new StoreUpdateFailedException(e); + } + storedRules.removeProperty(rule.getDescription()); + } + private void write(List violations, File ruleDetails) { StringBuilder builder = new StringBuilder(); for (String violation : violations) { @@ -255,6 +286,11 @@ void setProperty(String propertyName, String value) { syncFileSystem(); } + void removeProperty(String propertyName) { + loadedProperties.remove(ensureUnixLineBreaks(propertyName)); + syncFileSystem(); + } + private void syncFileSystem() { try (FileOutputStream outputStream = new FileOutputStream(propertiesFile)) { loadedProperties.store(outputStream, ""); diff --git a/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java b/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java index 8c5e4f87d9..5a22364b7f 100644 --- a/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/library/freeze/FreezingArchRuleTest.java @@ -45,6 +45,7 @@ import static com.tngtech.archunit.testutil.Assertions.assertThatRule; import static java.nio.file.Files.delete; import static java.nio.file.Files.exists; +import static java.nio.file.Files.readAllLines; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -53,8 +54,11 @@ public class FreezingArchRuleTest { private static final String STORE_PROPERTY_NAME = "freeze.store"; private static final String STORE_DEFAULT_PATH_PROPERTY_NAME = "freeze.store.default.path"; + private static final String FREEZE_REFREEZE = "freeze.refreeze"; private static final String ALLOW_STORE_CREATION_PROPERTY_NAME = "freeze.store.default.allowStoreCreation"; private static final String ALLOW_STORE_UPDATE_PROPERTY_NAME = "freeze.store.default.allowStoreUpdate"; + private static final String DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.deleteEmptyRuleViolation"; + private static final String WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME = "freeze.store.default.warnEmptyRuleViolation"; private static final String LINE_MATCHER_PROPERTY_NAME = "freeze.lineMatcher"; @Rule @@ -152,13 +156,13 @@ public void allows_to_overwrite_frozen_violations_if_configured() { ArchRule anotherViolation = rule("some description").withViolations("first violation", "second violation").create(); ArchRule frozenWithNewViolation = freeze(anotherViolation).persistIn(violationStore); - ArchConfiguration.get().setProperty("freeze.refreeze", Boolean.TRUE.toString()); + ArchConfiguration.get().setProperty(FREEZE_REFREEZE, Boolean.TRUE.toString()); assertThatRule(frozenWithNewViolation) .checking(importClasses(getClass())) .hasNoViolation(); - ArchConfiguration.get().setProperty("freeze.refreeze", Boolean.FALSE.toString()); + ArchConfiguration.get().setProperty(FREEZE_REFREEZE, Boolean.FALSE.toString()); assertThatRule(frozenWithNewViolation) .checking(importClasses(getClass())) @@ -482,6 +486,72 @@ public void can_prevent_default_ViolationStore_from_updating_existing_rules() th expectStoreUpdateDisabledException(() -> frozenRule.check(importClasses(getClass()))); } + @Test + public void warns_when_default_ViolationStore_is_empty() throws IOException { + useTemporaryDefaultStorePath(); + ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true"); + ArchConfiguration.get().setProperty(FREEZE_REFREEZE, "true"); + FreezingArchRule frozenRule = freeze(rule("some description").withoutViolations().create()); + frozenRule.check(importClasses(getClass())); + + // disallowing empty violation file should throw + ArchConfiguration.get().setProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true"); + assertThatThrownBy(() -> frozenRule.check(importClasses(getClass()))) + .isInstanceOf(StoreEmptyException.class) + .hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)"); + } + + @Test + public void warns_when_default_ViolationStore_gets_empty() throws IOException { + useTemporaryDefaultStorePath(); + ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true"); + RuleCreator someRule = rule("some description"); + freeze(someRule.withViolations("remaining", "will be solved").create()).check(importClasses(getClass())); + + // disallowing empty violation file should throw + ArchConfiguration.get().setProperty(WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true"); + FreezingArchRule frozenRule = freeze(someRule.withoutViolations().create()); + assertThatThrownBy(() -> frozenRule.check(importClasses(getClass()))) + .isInstanceOf(StoreEmptyException.class) + .hasMessageContaining("Saving empty violations for freezing rule is disabled (enable by configuration " + WARN_EMPTY_RULE_VIOLATION_PROPERTY_NAME + "=true)"); + } + + @Test + public void can_skip_default_ViolationStore_creation_for_empty_violations() throws IOException { + File storeFolder = useTemporaryDefaultStorePath(); + ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName()); + ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true"); + ArchConfiguration.get().setProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true"); + + ArchRule rule = rule("some description").withoutViolations().create(); + freeze(rule).check(importClasses(getClass())); + + assertThat(storeFolder.list()).containsOnly("stored.rules"); + String storeContents = String.join("\n", readAllLines(storeFolder.toPath().resolve("stored.rules"))); + assertThat(storeContents).doesNotContain(rule.getDescription()); + } + + @Test + public void can_delete_default_ViolationStore_rule_file_for_empty_violations() throws IOException { + // given + File storeFolder = useTemporaryDefaultStorePath(); + ArchConfiguration.get().setProperty(STORE_PROPERTY_NAME, MyTextFileBasedViolationStore.class.getName()); + ArchConfiguration.get().setProperty(ALLOW_STORE_CREATION_PROPERTY_NAME, "true"); + ArchConfiguration.get().setProperty(DELETE_EMPTY_RULE_VIOLATION_PROPERTY_NAME, "true"); + + freeze(rule("some description").withViolations("violation 1").create()).check(importClasses(getClass())); + assertThat(storeFolder.list()).containsOnly("stored.rules", "some description test"); + + // when + ArchRule rule = rule("some description").withoutViolations().create(); + freeze(rule).check(importClasses(getClass())); + + // then + assertThat(storeFolder.list()).containsOnly("stored.rules"); + String storeContents = String.join("\n", readAllLines(storeFolder.toPath().resolve("stored.rules"))); + assertThat(storeContents).doesNotContain(rule.getDescription()); + } + @Test public void allows_to_adjust_default_store_file_names_via_delegation() throws IOException { // GIVEN diff --git a/docs/userguide/009_JUnit_Support.adoc b/docs/userguide/009_JUnit_Support.adoc index f19c96d2cb..89b7cfd0e0 100644 --- a/docs/userguide/009_JUnit_Support.adoc +++ b/docs/userguide/009_JUnit_Support.adoc @@ -73,7 +73,7 @@ For further information see <>. ==== Controlling the Import Which classes will be imported can be controlled in a declarative way through `@AnalyzeClasses`. -If no packages or locations are provided, the whole classpath will be imported. +If no packages or locations are provided, the package of the annotated test class will be imported. You can specify packages to import as strings: [source,java,options="nowrap"] @@ -116,6 +116,32 @@ production code, and only consider code that is directly supplied and does not c As explained in <>, you can write your own custom implementation of `ImportOption` and then supply the type to `@AnalyzeClasses`. +To import the whole classpath, instead of just the package of the test class, use the option + +[source,java,options="nowrap"] +---- +@AnalyzeClasses(wholeClasspath = true) +---- + +Note that `@AnalyzeClasses` can also be used as a meta-annotation to avoid repeating the same configuration: + +[source,java,options="nowrap"] +---- +@Retention(RetentionPolicy.RUNTIME) +@AnalyzeClasses(packagesOf = MyApplicationRoot.class, importOptions = DoNotIncludeTests.class) +public @interface AnalyzeMainClasses {} +---- + +This annotation can then be used on test classes without repeating the specific configuration of `@AnalyzeClasses`: + +[source,java,options="nowrap"] +---- +@AnalyzeMainClasses +public class ArchitectureTest { + // ... +} +---- + ==== Controlling the Cache By default, all classes will be cached by location. This means that between different