BeanPostProcessor architecture rules do not work with external classes

[nosan]

See #45196

[nosan]

1. Is there a way to configure the classpath for a custom Gradle task so that these classes are accessible during its execution? (I have managed this myself by using the context ClassLoader.)
2. Do you have any suggestions on how to write a unit test for this?

This works because com.tngtech.archunit.core.importer.resolvers.ClassResolverFromClasspath tries to resolve any missing classes by using Thread.currentThread().getContextClassLoader()

[wilkinsona]

Thanks, @nosan. I think that the "official" Gradle way of doing this would be to use WorkerExecutor and its support for class loader isolation. That may be slightly safer than changing the thread context class loader which I guess could have other unexpected side-effects.

[nosan]

Thanks, @wilkinsona

I will take a look.

[nosan]

@wilkinsona

I've been trying to use WorkerExecutor and pass rules to the underlying WorkAction via WorkParameters.

* What went wrong:
  Execution failed for task ':spring-boot-project:spring-boot-tools:spring-boot-loader-tools:checkArchitectureMain'.
 A failure occurred while executing org.springframework.boot.build.architecture.ArchitectureWorkAction
 Could not isolate value org.springframework.boot.build.architecture.ArchitectureWorkParameters_Decorated@1a9f8595 of type ArchitectureWorkParameters
 Could not serialize value of type ArchRule.Factory.SimpleArchRule

From my understanding, rules are not serializable. Do you have any suggestions on how to handle this?

[wilkinsona]

Hmm, perhaps using the worker API isn't such a good idea after all. Apologies for the dead end.

I don't recall the details of your original approach, but an alternative to manipulating the thread context class loader would be a custom ClassResolver but that's not entirely straightforward as it only supports String arguments so a FileCollection (for example), couldn't be passed straight to it.

Perhaps it's better to just stick with setting the TCCL. We can reconsider if it causes problems in the future.

[nosan]

@philwebb
Please don't merge it. I would like to revisit a custom ClassResolver approach once more.

[nosan]

I've updated this PR to utilize a custom ClassResolver instead of TCCL.

Additionally, I made updates to the ArchitectureCheckTests to use GradleRunner after noticing that all tests were passing regardless of modifications. You can confirm this by altering any test, and it will have no effect.

I also added a test to verify external BPP.

Builed failed due to:

> Task :spring-boot-project:spring-boot-dependencies:bomrCheck FAILED

Neo4j Java Driver
    - Version 5.28.3 is misaligned. It should be 5.28.4.

[nosan]

I can fix ArchitectureCheckTests in a separate PR if needed.

[nosan]

ArchitectureCheckTests can also be fixed with:

	Project project = ProjectBuilder.builder().withProjectDir(projectDir).build();
		project.getTasks().register("checkArchitecture", ArchitectureCheck.class, (task) -> {
			task.setClasses(project.files("classes"));
			callback.accept(task);
			
		}).get();   //This line triggers a callback but is currently being missed
		

But I still need GradleRunner to verify external BPP.

[nosan]

@wilkinsona

don't recall the details of your original approach, but an alternative to manipulating the thread context class loader would be a custom ClassResolver but that's not entirely straightforward as it only supports String arguments so a FileCollection (for example), couldn't be passed straight to it.

Unfortunately, this approach has a significant limitation: it is not possible to pass a FileCollection directly. It can only be passed as a String value, regardless of the method used, whether through constructor parameters or Extension properties. For some reason, ArchUnit restricts the value type to strings. This means the FileCollection must be represented as a String using a separator (such as ',' or ':'). However, this introduces a potential issue: what happens if the file paths themselves contain the chosen delimiter?

Perhaps it's better to just stick with setting the TCCL. We can reconsider if it causes problems in the future.

Sticking with setting the TCCL seems better

[nosan]

This PR includes two commits:

• The first implements a custom ClassResolver approach.
• The second utilizes the Thread Context ClassLoader (TCCL).

[wilkinsona]

Thanks very much, @nosan. I went with the TCCL-based approach in the end.

[nosan]

Thanks, @wilkinsona

I came across the commit a572283, and I was wondering if this @Role check should be backported to the 3.3.x/3.4.x branches?

[wilkinsona]

I don't think it needs to be as we don't have any BPPs in those branches that use infrastructure beans. We can backport the slight relaxation of the rule in the future if needed.