Preconfigured rules scan all classes on classpath, not only the ones from the project

[vincent-fuchs]

Summary

When the plugin runs, we see that even the standard java classes are scanned

Type of Issue

It is a :

• bug
• request
• question regarding the documentation

Motivation

Execution would be faster if we were scanning only the classes from the project. Furthermore, it may happen that one standard java class has a violation for some rules.

Current Behavior

When I run the build I have this :

[INFO] Analysing class 'java/io/IOException'
[INFO] Analysing class 'java/time/format/TextStyle'
[INFO] Analysing class 'java/util/Locale'
[INFO] Analysing class 'java/time/ZoneOffset'
[INFO] Analysing class 'java/time/zone/ZoneRules'
[INFO] Analysing class 'java/io/ObjectInputStream'
[INFO] Analysing class 'java/io/InvalidObjectException'

(..my classes are also scanned)

Expected Behavior

Only "my" classes should be scanned.

Steps to Reproduce (for bugs)

I guess this happens in all projects..

Your Environment

• Version used: 2.0.0

[codecholeric]

Could it be, that this is not a bug, but simply configuration of ArchUnit to resolve missing dependencies from the classpath? (i.e. if the classes you are importing have dependencies that you are not importing?)
Compare TNG/ArchUnit#186

[vincent-fuchs]

@codecholeric , yes, it looks like this could be the "issue". But I have a problem : when running ArchUnit through the Maven plugin, the property file in the project using it is not picked up .

that's why I created TNG/ArchUnit#197 : I tried to make several tests in one of my other project, using latest version of ArchUnit and building locally :

	[INFO] Detected Java version 1.8.0_171
	[INFO] trying to find /archunit.properties...
	[INFO] no ArchUnit properties file found, therefore going with default behavior
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\application-test.yml
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\archunit.properties
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\bulkUpdate_directPush_Command.json
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\bulkUpdate_pullRequest_Command.json
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\bulkUpdate_pullRequest_noBranch_Command.json
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\com
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\logback-spring.xml
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\pullRequestEvent.json
	[INFO] C:\Vincent\projects\ci-droid\ci-droid-core\target\test-classes\pushEvent.json

As you can see, the file is there in 2nd position (I placed it at the root of src/test/resources) , but it's picked up by getClass().getResourceAsStream(propertiesResourceName)

Any idea of how to fix this, either in ArchUnit or in the maven plugin ?

[croesch]

I face a similar issue when trying to ignore violations. Did you manage to do that? I guess not, because if I place the archunit_ignore_patterns.txt under src/test/resources it doesn't get found.

com.tngtech.archunit.lang.ArchRule.Assertions.readPatternsFrom(String) uses Assertions.class.getResource('/' + fileNameInClassPath); to read that file. Similar code applies to the configuration in com.tngtech.archunit.ArchConfiguration.readProperties(String).

In com.societegenerale.commons.plugin.ArchUnitMojo.configureContextClassLoader() the context class loader gets set. If I use Thread.currentThread().getContextClassLoader().getResource("archunit_ignore_patterns.txt") in my custom rule, I can access the text file finally.

So regarding to the behavior and the Maven guide I guess it would be necessary to enhance TNG/ArchUnit to inject the violation ignoring patterns programmatically.

Then one could enhance the plugin's configuration to inject values to ArchUnit instead of reading the properties file. ArchConfiguration already allows doing this, if ArchRule.Assertions would accept this, too, the Maven plugin could be configured via POM. Which would have the benefit of allowing simple configuration with the disadvantage that you have to configure ArchUnit differently via plugin than without plugin.

Maybe the best would be to allow injecting the class loader directly? Or at least read the configuration files from the ArchUnitMojo, merge it with the plugin's configuration so that you could configure ArchUnit both: via POM and via txt/properties files.

What do you think @codecholeric and @vincent-fuchs ?

[codecholeric]

In my opinion the problem might be, that ArchUnit should use the ContextClassLoader to load configuration and ignore file. So far the ContextClassLoader was only relevant for me for the import process, but I think it should be consistent, such that every user customized resource should be loaded by the ContextClassLoader. What do you think?
Nevertheless an option to configure the ignore file programmatically might be a reasonable addition.

[croesch]

My opinion: Sounds good - if the configuration and the ignore file (or its entries) could be set programmatically then the Arch Unit Maven Plugin could make use of it and fill it with its own configuration entries.

The only thing for users that would be important to document is the difference in configuring Arch Unit (via files) and the Arch Unit Maven Plugin (via plugin configuration in the POM) - but I think that's consistent to the context. I don't think that Maven Plugin users would expect configuration files to work - only users migrating from using Arch Unit (alone) to Arch Unit Maven Plugin would be affected.

[vincent-fuchs]

OK for me if we can set the configuration file programmatically, so that we define it with the plugin (archunit-maven-plugin.properties ?).
But I think everything after should be managed in ArchUnit "core", ie we probably don't want to have the config file entries available in the pom.xml : sounds like duplication of effort, and it will make the plugin config more verbose, forcing us to document every field, which would already be documented somewhere in https://www.archunit.org/userguide/html/000_Index.html#_advanced_configuration

So we need to wait for a change in ArchUnit to be able to try again, right @codecholeric ? I guess it's not as easy as simply replacing getClass().getResourceAsStream(propertiesResourceName) by ClassLoaders.getCurrentClassLoader(getClass()).getResourceAsStream(propertiesResourceName), is it ?

[codecholeric]

@vincent-fuchs Actually I think it is as easy. At least for now it looks to me like it should solve your problems and not have any negative consequences for the default case.
I would go with that for now and see if anybody has any issues with that. I've just uploaded a SNAPSHOT version, do you want to try if it solves the problem?

<repositories>
    <repository>
        <id>sonatype-snapshots</id>
        <url>https://oss.sonatype.org/content/repositories/snapshots</url>
    </repository>
</repositories>

<dependencies>
    <dependency>
        <groupId>com.tngtech.archunit</groupId>
        <artifactId>archunit</artifactId>
        <version>0.11.0-SNAPSHOT</version>
    </dependency>
</dependencies>

[vincent-fuchs]

quick update : file seems to be found - I see the log statement saying :

[INFO] Reading ArchUnit properties from file..

I'll give a more exhaustive status tomorrow

[vincent-fuchs]

I confirm it's working !

• if I put an incorrect classResolver in archunit.properties, it fails - meaning we try to load it
• other classes than mine are not analyzed when the file contains resolveMissingDependenciesFromClassPath=false

Now I'll wait for the next ArchUnit release to make a release of the plugin. Thanks !!

[vincent-fuchs]

fixed by @codecholeric in TNG/ArchUnit@81c31f6 . archunit.properties file will be found by the plugin once we upgrade to 0.11.0 or greater

[codecholeric]

So is it fixed? 😉

[vincent-fuchs]

So is it fixed? 😉

yes it is, with an archunit.properties file in the maven module ! thanks a lot for your help.

Now it's more difficult to realize though, because the "Analyzing class" log statement is at debug level, so it won't get displayed by default. If I had started using ArchUnit with 0.11.0, I would probably never have realized the "issue" ;-)

[vincent-fuchs]

v 2.2.0 has ArchUnit 0.11.0 , so closing this issue.

[codecholeric]

Can't have it all 😉