From 56db075510a3ed2be169d1096e302867597aa42b Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Thu, 27 Apr 2017 12:00:14 +0200 Subject: [PATCH 1/3] Hardcoded separator '/' was actually correct, since we are dealing with URIs (this was actually one of the motivations, to convert every input to URI as quickly as possible, to enable consistent treatment of locations) --- .../archunit/core/importer/ImportOption.java | 16 ++++++---------- .../core/importer/DontIncludeTestsTest.java | 12 +++++++----- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java index e53ffe53d1..a596827e10 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java @@ -15,7 +15,6 @@ */ package com.tngtech.archunit.core.importer; -import java.io.File; import java.util.Set; import com.google.common.collect.ImmutableSet; @@ -68,18 +67,15 @@ public boolean includes(Location location) { } /** - * NOTE: This excludes all class files residing in some directory ../test/.. or - * ../test-classes/.. (Maven/Gradle standard), so don't use this, if you have a package - * test that you want to import. + * NOTE: This excludes all class files residing in some directory {@value #MAVEN_INFIX} or + * {@value #GRADLE_INFIX} (Maven/Gradle standard). Thus it is just a best guess, how tests can be identified, + * in other environments, it might be necessary, to implement the correct {@link ImportOption} yourself. */ final class DontIncludeTests implements ImportOption { - private static final Set EXCLUDED_INFIXES = ImmutableSet.of( - anyFolder("test"), - anyFolder("test-classes")); + private static final String MAVEN_INFIX = "/target/test-classes/"; + private static final String GRADLE_INFIX = "/build/classes/test/"; - private static String anyFolder(String infix) { - return String.format("%s%s%s", File.separator, infix, File.separator); - } + private static final Set EXCLUDED_INFIXES = ImmutableSet.of(MAVEN_INFIX, GRADLE_INFIX); @Override public boolean includes(Location location) { diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/DontIncludeTestsTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/DontIncludeTestsTest.java index f5237b05c8..726b5d100e 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/DontIncludeTestsTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/DontIncludeTestsTest.java @@ -37,16 +37,18 @@ public void excludes_test_class() { @DataProvider public static Object[][] folders() { return $$( - $("test", false), - $("test-classes", false), - $("classes", true), - $("main", true) + $(new String[]{"build", "classes", "test"}, false), + $(new String[]{"target", "classes", "test"}, true), + $(new String[]{"target", "test-classes"}, false), + $(new String[]{"build", "test-classes"}, true), + $(new String[]{"build", "classes", "main"}, true), + $(new String[]{"target", "classes"}, true) ); } @Test @UseDataProvider("folders") - public void detects_both_Gradle_and_Maven_style(String folderName, boolean expectedInclude) throws IOException { + public void detects_both_Gradle_and_Maven_style(String[] folderName, boolean expectedInclude) throws IOException { File folder = temporaryFolder.newFolder(folderName); File targetFile = new File(folder, getClass().getSimpleName() + ".class"); Files.copy(locationOf(getClass()).asURI().toURL().openStream(), targetFile.toPath()); From 9ade9859c447bcde56b656313b780aeffc3840f8 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Thu, 27 Apr 2017 12:06:40 +0200 Subject: [PATCH 2/3] Added Javadoc to ImportOptions --- .../tngtech/archunit/core/importer/ImportOptions.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOptions.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOptions.java index c536b3a2ba..b4d3e95194 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOptions.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOptions.java @@ -24,6 +24,11 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; +/** + * A collection of {@link ImportOption} to filter class locations. All supplied {@link ImportOption}s will be joined + * with AND, i.e. only {@link Location}s that are accepted by all {@link ImportOption}s + * will be imported. + */ public final class ImportOptions { private final Set options; @@ -36,6 +41,10 @@ private ImportOptions(Set options) { this.options = checkNotNull(options); } + /** + * @param option An {@link ImportOption} to evaluate on {@link Location}s of class files + * @return self to add further {@link ImportOption}s in a fluent way + */ @PublicAPI(usage = ACCESS) public ImportOptions with(ImportOption option) { return new ImportOptions(ImmutableSet.builder().addAll(options).add(option).build()); From b6796ea0ebb86894460e4410dfa9a053c85aee0e Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Thu, 27 Apr 2017 13:00:51 +0200 Subject: [PATCH 3/3] Migrated @AnalyzeClasses(importOption = ...) to an array @AnalyzeClasses(importOptions = {Options1.class, Options2.class}), since the ClassFileImporter supports this API anyway, so it is more consistent in the end, and more useful, e.g. if one wants to combine DontImportJars.class and DontImportTests.class. --- .../archunit/ArchUnitArchitectureTest.java | 2 +- .../archunit/junit/AnalyzeClasses.java | 17 ++++++++++++- .../tngtech/archunit/junit/ClassCache.java | 24 +++++++++++-------- .../junit/ClassCacheConcurrencyTest.java | 4 ++-- .../archunit/junit/ClassCacheTest.java | 7 +++--- .../archunit/maventest/ArchUnitSmokeTest.java | 9 ++----- .../core/importer/ClassFileImporter.java | 6 +++++ .../archunit/core/importer/ImportOption.java | 12 +++------- .../archunit/core/importer/Location.java | 9 +++++++ .../archunit/lang/SimpleConditionEvent.java | 2 +- 10 files changed, 58 insertions(+), 34 deletions(-) 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 bf4f5406d3..d67efd2b32 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 @@ -39,7 +39,7 @@ @RunWith(ArchUnitRunner.class) @AnalyzeClasses( packagesOf = ArchUnitArchitectureTest.class, - importOption = ArchUnitArchitectureTest.ArchUnitProductionCode.class) + importOptions = ArchUnitArchitectureTest.ArchUnitProductionCode.class) public class ArchUnitArchitectureTest { static final String THIRDPARTY_PACKAGE_IDENTIFIER = "..thirdparty.."; diff --git a/archunit-junit/src/main/java/com/tngtech/archunit/junit/AnalyzeClasses.java b/archunit-junit/src/main/java/com/tngtech/archunit/junit/AnalyzeClasses.java index 041cfda0fd..36e250d329 100644 --- a/archunit-junit/src/main/java/com/tngtech/archunit/junit/AnalyzeClasses.java +++ b/archunit-junit/src/main/java/com/tngtech/archunit/junit/AnalyzeClasses.java @@ -18,7 +18,9 @@ import java.lang.annotation.Retention; import java.lang.annotation.Target; +import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.core.importer.ImportOptions; import static java.lang.annotation.ElementType.TYPE; import static java.lang.annotation.RetentionPolicy.RUNTIME; @@ -31,9 +33,22 @@ @Target(TYPE) @Retention(RUNTIME) public @interface AnalyzeClasses { + /** + * @return Packages to look for in all URLs known to the actual {@link java.net.URLClassLoader} + */ String[] packages() default {}; + /** + * @return Classes that specify packages to look for in all URLs known to the actual {@link java.net.URLClassLoader} + */ Class[] packagesOf() default {}; - Class importOption() default ImportOption.Everything.class; + /** + * Allows to filter the class import. The supplied types will be instantiated and used to create the + * {@link ImportOptions} passed to the {@link ClassFileImporter}. Considering caching, compare the notes on + * {@link ImportOption}. + * + * @return The types of {@link ImportOption} to use for the import + */ + Class[] importOptions() default {}; } diff --git a/archunit-junit/src/main/java/com/tngtech/archunit/junit/ClassCache.java b/archunit-junit/src/main/java/com/tngtech/archunit/junit/ClassCache.java index c218f997e1..c75beb408b 100644 --- a/archunit-junit/src/main/java/com/tngtech/archunit/junit/ClassCache.java +++ b/archunit-junit/src/main/java/com/tngtech/archunit/junit/ClassCache.java @@ -26,6 +26,7 @@ import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.core.importer.ImportOptions; import com.tngtech.archunit.core.importer.Location; import com.tngtech.archunit.core.importer.Locations; @@ -55,7 +56,7 @@ private LocationsKey locationsToImport(Class testClass) { .addAll(toPackageStrings(analyzeClasses.packagesOf())) .build(); Set locations = packages.isEmpty() ? Locations.inClassPath() : locationsOf(packages); - return new LocationsKey(analyzeClasses.importOption(), locations); + return new LocationsKey(analyzeClasses.importOptions(), locations); } private Set toPackageStrings(Class[] classes) { @@ -119,31 +120,34 @@ public JavaClasses get() { private synchronized void initialize() { if (javaClasses == null) { - ImportOption importOption = newInstanceOf(locationsKey.importOptionClass); - javaClasses = cacheClassFileImporter.importClasses(importOption, locationsKey.locations); + ImportOptions importOptions = new ImportOptions(); + for (Class optionClass : locationsKey.importOptionClasses) { + importOptions = importOptions.with(newInstanceOf(optionClass)); + } + javaClasses = cacheClassFileImporter.importClasses(importOptions, locationsKey.locations); } } } // Used for testing -> that's also the reason it's declared top level static class CacheClassFileImporter { - JavaClasses importClasses(ImportOption importOption, Collection locations) { - return new ClassFileImporter().withImportOption(importOption).importLocations(locations); + JavaClasses importClasses(ImportOptions importOptions, Collection locations) { + return new ClassFileImporter(importOptions).importLocations(locations); } } private static class LocationsKey { - private final Class importOptionClass; + private final Set> importOptionClasses; private final Set locations; - private LocationsKey(Class importOptionClass, Set locations) { - this.importOptionClass = importOptionClass; + private LocationsKey(Class[] importOptionClasses, Set locations) { + this.importOptionClasses = ImmutableSet.copyOf(importOptionClasses); this.locations = locations; } @Override public int hashCode() { - return Objects.hash(importOptionClass, locations); + return Objects.hash(importOptionClasses, locations); } @Override @@ -155,7 +159,7 @@ public boolean equals(Object obj) { return false; } final LocationsKey other = (LocationsKey) obj; - return Objects.equals(this.importOptionClass, other.importOptionClass) + return Objects.equals(this.importOptionClasses, other.importOptionClasses) && Objects.equals(this.locations, other.locations); } } diff --git a/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheConcurrencyTest.java b/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheConcurrencyTest.java index 89d2a7f5d8..c793e8102c 100644 --- a/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheConcurrencyTest.java +++ b/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheConcurrencyTest.java @@ -8,7 +8,7 @@ import java.util.concurrent.Future; import com.tngtech.archunit.Slow; -import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.core.importer.ImportOptions; import com.tngtech.archunit.junit.ClassCache.CacheClassFileImporter; import org.junit.Rule; import org.junit.Test; @@ -53,7 +53,7 @@ public void concurrent_access() throws Exception { for (Future future : futures) { future.get(1, MINUTES); } - verify(classFileImporter, atMost(TEST_CLASSES.size())).importClasses(any(ImportOption.class), anyCollection()); + verify(classFileImporter, atMost(TEST_CLASSES.size())).importClasses(any(ImportOptions.class), anyCollection()); verifyNoMoreInteractions(classFileImporter); } diff --git a/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheTest.java b/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheTest.java index 48c69bad91..2f229ad2f5 100644 --- a/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheTest.java +++ b/archunit-junit/src/test/java/com/tngtech/archunit/junit/ClassCacheTest.java @@ -3,6 +3,7 @@ import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.core.importer.ImportOptions; import com.tngtech.archunit.core.importer.Location; import com.tngtech.archunit.junit.ClassCache.CacheClassFileImporter; import com.tngtech.archunit.testutil.ArchConfigurationRule; @@ -127,7 +128,7 @@ public void distinguishes_import_option_when_caching() { } private void verifyNumberOfImports(int number) { - verify(cacheClassFileImporter, times(number)).importClasses(any(ImportOption.class), anyCollection()); + verify(cacheClassFileImporter, times(number)).importClasses(any(ImportOptions.class), anyCollection()); verifyNoMoreInteractions(cacheClassFileImporter); } @@ -147,11 +148,11 @@ public static class TestClassWithFilterJustByPackageOfClass { public static class TestClassWithNonExistingPackage { } - @AnalyzeClasses(importOption = TestFilterForJUnitJars.class) + @AnalyzeClasses(importOptions = TestFilterForJUnitJars.class) public static class TestClassFilteringJustJUnitJars { } - @AnalyzeClasses(importOption = AnotherTestFilterForJUnitJars.class) + @AnalyzeClasses(importOptions = AnotherTestFilterForJUnitJars.class) public static class TestClassFilteringJustJUnitJarsWithDifferentFilter { } diff --git a/archunit-maven-test/src/test/java/com/tngtech/archunit/maventest/ArchUnitSmokeTest.java b/archunit-maven-test/src/test/java/com/tngtech/archunit/maventest/ArchUnitSmokeTest.java index 9cd3a44e50..e5a1852b80 100644 --- a/archunit-maven-test/src/test/java/com/tngtech/archunit/maventest/ArchUnitSmokeTest.java +++ b/archunit-maven-test/src/test/java/com/tngtech/archunit/maventest/ArchUnitSmokeTest.java @@ -3,6 +3,7 @@ import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.core.importer.ImportOption.DontIncludeTests; import com.tngtech.archunit.core.importer.Location; import com.tngtech.archunit.junit.AnalyzeClasses; import com.tngtech.archunit.junit.ArchTest; @@ -12,7 +13,7 @@ import static org.junit.Assert.assertEquals; @RunWith(ArchUnitRunner.class) -@AnalyzeClasses(packages = "com.tngtech.archunit.maventest", importOption = ArchUnitSmokeTest.NoTests.class) +@AnalyzeClasses(packages = "com.tngtech.archunit.maventest", importOptions = DontIncludeTests.class) public class ArchUnitSmokeTest { @ArchTest public static void runs_without_exception(JavaClasses classes) { @@ -27,10 +28,4 @@ public static void runs_without_exception(JavaClasses classes) { assertEquals("Number of fields in ClassTwo", classes.get(ClassTwo.class).getFields().size(), 0); assertEquals("Number of methods in ClassTwo", classes.get(ClassTwo.class).getMethods().size(), 1); } - - public static class NoTests implements ImportOption { - public boolean includes(Location location) { - return !location.asURI().toString().contains("test-classes"); - } - } } diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImporter.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImporter.java index ac3ad9ecdd..9734ffe465 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImporter.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/ClassFileImporter.java @@ -37,6 +37,12 @@ import static java.util.Collections.singleton; import static java.util.Collections.singletonList; +/** + * The central API to import {@link JavaClasses}. Supports various types of {@link Location}, e.g. {@link Path}, + * {@link JarFile} or {@link URL}. The {@link Location}s that are scanned, can be filtered by passing any number of + * {@link ImportOption} to {@link #withImportOption(ImportOption)}, which will then be ANDed (compare + * {@link ImportOptions}. + */ public final class ClassFileImporter { private final ImportOptions importOptions; diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java index a596827e10..f3bdfa3913 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/ImportOption.java @@ -59,16 +59,10 @@ public boolean includes(Location location) { } } - final class Everything implements ImportOption { - @Override - public boolean includes(Location location) { - return true; - } - } - /** - * NOTE: This excludes all class files residing in some directory {@value #MAVEN_INFIX} or - * {@value #GRADLE_INFIX} (Maven/Gradle standard). Thus it is just a best guess, how tests can be identified, + * NOTE: This excludes all class files residing in some directory + * ../target/test-classes/.. or ../build/classes/test/.. (Maven/Gradle standard). + * Thus it is just a best guess, how tests can be identified, * in other environments, it might be necessary, to implement the correct {@link ImportOption} yourself. */ final class DontIncludeTests implements ImportOption { diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/Location.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/Location.java index fb1a0d2d52..2b3de520c7 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/Location.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/Location.java @@ -33,6 +33,11 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.tngtech.archunit.PublicAPI.Usage.ACCESS; +/** + * Handles various forms of location, from where classes can be imported, in a consistent way. Any location + * will be treated like an {@link URI}, thus there won't be any platform dependent file separator problems, + * or similar. + */ public abstract class Location { private static final String FILE_SCHEME = "file"; private static final String JAR_SCHEME = "jar"; @@ -50,6 +55,10 @@ public URI asURI() { abstract ClassFileSource asClassFileSource(ImportOptions importOptions); + /** + * @param part A part to check the respective location {@link URI} for + * @return true, if the respective {@link URI} contains the given part + */ @PublicAPI(usage = ACCESS) public boolean contains(String part) { return uri.toString().contains(part); diff --git a/archunit/src/main/java/com/tngtech/archunit/lang/SimpleConditionEvent.java b/archunit/src/main/java/com/tngtech/archunit/lang/SimpleConditionEvent.java index fdc8cee3c3..47b99a5e49 100644 --- a/archunit/src/main/java/com/tngtech/archunit/lang/SimpleConditionEvent.java +++ b/archunit/src/main/java/com/tngtech/archunit/lang/SimpleConditionEvent.java @@ -56,7 +56,7 @@ public void describeTo(CollectsLines messages) { @Override public T getCorrespondingObject() { - return null; + return correspondingObject; } @Override