Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add modules syntax to rules API #1078

Merged
merged 28 commits into from
Aug 25, 2023
Merged

Add modules syntax to rules API #1078

merged 28 commits into from
Aug 25, 2023

Conversation

codecholeric
Copy link
Collaborator

@codecholeric codecholeric commented Mar 3, 2023

This PR will add a convenient fluent rules API to partition JavaClasses into (architecture) modules and assert conditions on this module structure. The basic syntax has the form

modules()
  .definedBy... // use one of the various methods to decide how to partition classes into modules
  .should()... // evaluate some predefined or custom condition

While some concepts are quite similar to the Slices API I've noticed over the years that most people don't see the possibilities to use the Slices API to check modularization properties. Thus, I decided to not make the existing API more powerful, but instead create this new API which offers most features of the Slices API (e.g. create from package identifiers, check for cycles) but also more powerful / concise possibilities to introduce modularization into a code base (e.g. by an annotated package-info) and carry meta-information like allowed dependencies from such an annotated object into the modules rule.

@codecholeric codecholeric requested a review from hankem March 3, 2023 19:14
@codecholeric codecholeric force-pushed the add-modules-syntax branch 2 times, most recently from a1ee308 to eb203f0 Compare March 5, 2023 09:19
Copy link
Member

@hankem hankem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't make it through all commits yet. I so far have just a few suggestions for minor renamings, but I thought I'd submit those already.

@codecholeric codecholeric force-pushed the add-modules-syntax branch 3 times, most recently from e4d4c73 to 2095c3d Compare March 18, 2023 20:13
This first step will allow to more easily modularize a code base by introducing the concept of an ArchUnit module. This can help in cases where other modularization techniques (e.g. Java Modules) are not available or feasible. The concept of a module borrows some common ideas from the Slices API, namely that every module will be uniquely identified by a module identifier and a module is principally a mapping `JavaClass` -> `ArchModule.Identifier`, which will partition the source space into buckets of classes having the same identifier.

While principally the Slices API can already be used to write modularization rules, the mental model is a little off. From watching questions on Stackoverflow and ArchUnit issues I always got the feeling that users don't see the possibility to use the Slices API for these purposes, but more for just cutting the code base in slices and asserting freedom of cycles. While this was the original main use case, it now falls short to be noticed as solution for typical modularization problems.

Signed-off-by: Peter Gafert <[email protected]>
To allow a more flexible, dynamic module structure, we support defining modules by root classes. I.e. we identify module root packages by finding classes matching a predicate and then taking the packages of these classes.

Signed-off-by: Peter Gafert <[email protected]>
To enable more powerful module abstractions it makes sense to support some sort of extended meta-information.
This can for example include the allowed dependencies of a module to implement a self-growing module structure
without central dependency definitions.
To support this we parameterize `ArchModule` by a more powerful `Descriptor` instead of just a name.
This way users can supply any custom `Descriptor` if necessary and use it later within their `ArchCondition`s.

Signed-off-by: Peter Gafert <[email protected]>
In cases where a certain descriptor root class of a module is used to define the module it makes sense to offer a convenient API to directly derive the module descriptor from this root class instead of having to search the root class again within all the classes contained within the module. One example is if the root class is annotated with a certain annotation containing the module meta-information. In this case we need the root class again to derive the module descriptor.

Signed-off-by: Peter Gafert <[email protected]>
This gives a convenient API to add a custom annotation with module meta-data to respective root classes and automatically derive modules from this.
This way, we now also support using an annotated `package-info` to define modules in a very concise way:

```
@MyModule(name = "Order", allowedDependencies = {"Customer", "Shipping"})
package com.acme.order
```

Signed-off-by: Peter Gafert <[email protected]>
To integrate our new `ArchModules` infrastructure in a consistent way, we have to offer it as a rule definition, similar to the `slices` API. For now this API is quite rudimentary, offering little convenience to define typical rules (e.g. allowed dependencies).

Signed-off-by: Peter Gafert <[email protected]>
The naming was a little confusing, i.e. that a set of `SpecificParameterProvider` would be called `singleParameterProviders`, but then there is also a class `SingleParametersProvider`, etc. The new names should reflect the role a little better. I also moved the `SingleParameterProvider` toplevel, so we can adjust it per test level in the next step.

Signed-off-by: Peter Gafert <[email protected]>
... so we can adjust it on a per-test level in the next step.

Signed-off-by: Peter Gafert <[email protected]>
This should be one of the core use cases of the modules API, so we should make it easy to check for illegal module dependencies. It seems like the most convenient API we can reach is one where the user can simply provide a predicate determining which `ModuleDependency` is allowed and we can then check all module dependencies against this predicate and report violations.
As with the PlantUML rule it's safe to assume that users will have the requirement to specify how to handle dependencies (e.g. dependencies to core Java or library classes) and to ignore specific class dependencies when checking the rule.

Signed-off-by: Peter Gafert <[email protected]>
This is a simplification that allows to directly write down which modules may depend on which other modules by name. I picked the module name for this definition and not the identifier, because if the two differ I think the name is nicer to read (and also unique as the identifier).

Signed-off-by: Peter Gafert <[email protected]>
We will add ArchUnit's cycle detection algorithm to the public API, since it is a) also useful to users (one use case was e.g. to detect cycles within static initializers) and b) we will reuse it from the new modules API. We don't make it public in this step, because we want to move this out into a separate package while making clear which changes are related to moving the files and which changes are related to refactoring the existing classes to allow a clearer API.
The cycle detection algorithm so far was designed very closely along the requirements of `SliceCycleArchCondition`. When we open this as public API we redesign it and for example don't use the concept of `ATTACHMENT`, but simply allow users to pass their own implementation of `Edge`.

Signed-off-by: Peter Gafert <[email protected]>
Now that the new cycle detection interface has been designed we can relocate all cycle detection related classes to a new package `cycle_detection` and expose the new public API to users.

Signed-off-by: Peter Gafert <[email protected]>
This class adds no additional behavior to a simple `Set<SliceDependency>` and only encapsulates the mapping. By that it seems more confusing than useful, since we can just write the mapping inline, making it more clear that we're indeed only interested in a simple `Set<SliceDependency>` and nothing more.

Signed-off-by: Peter Gafert <[email protected]>
We know that a cycle always has edges, so finding the edge with the lexicographically minimum description will never yield absent. But we should make this explicit and ignore the warning.

Signed-off-by: Peter Gafert <[email protected]>
Just naming this `DescribedPredicate` `predicate` makes it really hard to understand what it's used for down the line.

Signed-off-by: Peter Gafert <[email protected]>
This rule doesn't use much that is `Slice`-specific. We can make this easily more generic by replacing `Slice` with a generic `COMPONENT` if we parameterize
* how to retrieve the classes contained within a `COMPONENT`
* how to retrieve the description of a `COMPONENT`
* how to retrieve the outgoing class dependencies of a `COMPONENT`
We now make this new generic `CycleArchCondition` public API so users can use it for their own components, and we can reuse it for the new modules API.

Signed-off-by: Peter Gafert <[email protected]>
This is another common requirement when testing for modularization, that no matter the visibility of individual classes the API of each module is a dedicated subset of all (compile) accessible classes. The reason is that the Java visibility modifiers are typically not powerful enough to model such a module API, unless all classes are thrown together in a single package (where we can use package-visibility), which is not feasible for more complex modules.

Signed-off-by: Peter Gafert <[email protected]>
... as a more convenient version of `onlyDependOnEachOtherThroughClassesThat(predicate)` that allows to specify the allowed dependency targets in a fluent way. Unfortunately, this means we either have to open up `ClassesThatInternal` to be used from this other place, or we have to duplicate it. Since it would also implement the same interface `ClassesThat` and thus would have to change in two places every time we adjust that interface, I decided to make it `@Internal` and public. It is not pretty but likely still better than duplication, and since users don't get any guarantees on `@Internal` classes it doesn't impair maintainability.

Signed-off-by: Peter Gafert <[email protected]>
While `orShould()` will likely not make any sense for module rules (in particular because we don't support `noModules()`), `andShould()` might be some nice convenience to e.g. combine checking for allowed dependencies and allowed API in one rule. We add `andShould()` and the generic counterpart `andShould(condition)` for now and see if any user ever really misses `orShould()`.
Note that we can always only return `ArchRule` from the generic `{and}Should(condition)`, because `ignoreDependency` of `ModulesRule` doesn't make any sense for rules with custom conditions. I.e. there is no way to generally ignore dependencies in this case, because users can freely handle all dependencies within their custom condition. Principally, we could split it into `ModulesRule` and `ModulesRuleCapableOfIgnoringDependencies`, but for now I consider this too much complexity for an uncertain benefit. Because in the end, if users use custom conditions anyway, they might as well `and` or `or` them together directly, like `should(conditionOne.and(conditionTwo))`, so `andShould(..)` doesn't really bring much value here.

Signed-off-by: Peter Gafert <[email protected]>
We can support this as a standard case to make it really simple to define an annotation to declare a module including dependencies.

Signed-off-by: Peter Gafert <[email protected]>
This makes the use case to expose custom packages for each module via the module descriptor annotation very simple. I.e. users can simply declare a property name of a `String[]` property on the descriptor annotation, and we will treat all the values in this array as package identifiers and only allow access to those classes within the module that match one of the package identifiers.

Signed-off-by: Peter Gafert <[email protected]>
This gives us the chance to easier break it if necessary, in case we find some really bad oversight somewhere.

Signed-off-by: Peter Gafert <[email protected]>
This gives us the chance to easier break it if necessary, in case we find some really bad oversight somewhere.

Signed-off-by: Peter Gafert <[email protected]>
@codecholeric codecholeric merged commit 0e90343 into main Aug 25, 2023
21 checks passed
@codecholeric codecholeric deleted the add-modules-syntax branch August 25, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants