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

How to exclude libraries and external classes from violations #841

Closed
husam-e opened this issue Apr 3, 2022 · 3 comments
Closed

How to exclude libraries and external classes from violations #841

husam-e opened this issue Apr 3, 2022 · 3 comments

Comments

@husam-e
Copy link

husam-e commented Apr 3, 2022

When leveraging the layeredArchitectures rule set, I get violations due to Kotlin and JDK classes.

Using JUnit5, in a Kotlin only codebase, as follows:

Dependency: com.tngtech.archunit:archunit-junit5:0.23.1

@AnalyzeClasses(
    packages = ["com.myapp.svc.."],
    importOptions = [ImportOption.DoNotIncludeTests::class]
)
class ArchitectureConstraintTest {

  /** Regular attempt based on docs and online examples */
  @ArchTest
  fun `Coarse layers respect dependency boundaries`(importedClasses: JavaClasses) {
    Architectures.layeredArchitecture()
            .layer("Api").definedBy("..api..")
            .layer("Repository").definedBy("..repository..")
            .layer("Service").definedBy("..service..")
            .whereLayer("Api").mayNotAccessAnyLayer()
            .whereLayer("Service").mayOnlyAccessLayers("Api", "Repository")
            .check(importedClasses)
  }

  /** Explicit attempt to ignore libraries */
  @ArchTest
  fun `Coarse layers respect dependency boundaries 2`(importedClasses: JavaClasses) {
    Architectures.layeredArchitecture()
            .layer("Libraries").definedBy(JavaClass.Predicates.resideOutsideOfPackages("com.myapp.svc..")) // attempt to isolate libs, no dice
            .layer("Api").definedBy("..api..")
            .layer("Repository").definedBy("..repository..")
            .layer("Service").definedBy("..service..")
            .whereLayer("Api").mayOnlyAccessLayers("Libraries")
            .whereLayer("Service").mayOnlyAccessLayers("Api", "Repository", "Libraries")
            .check(importedClasses)
  }

  ...
}

Here are the types of errors I get:

for test 1:

Class <com.myapp.svc.common.core.api.CountryCode$Companion> extends class <java.lang.Object> in (CountryCode.kt:0)
Class <com.myapp.svc.common.core.api.CountryCode$Companion> is annotated with <kotlin.Metadata> in (CountryCode.kt:0)
Class <com.myapp.svc.api.User> extends class <java.lang.Object> in (UserModels.kt:0)
Class <com.myapp.svc.api.User> is annotated with <kotlin.Metadata> in (UserModels.kt:0)
... and so on

for test 2:

... Layer 'Libraries' is empty

Excluding JARs or Archives results in the layers being empty so that didn't solve it either. I'm sure all other non-example usages of this lib leverage libraries so wondering how ppl get around this? Is this a Kotlin only issue somehow?

Thanks in advance :)

@codecholeric
Copy link
Collaborator

By specifying @AnalyzeClasses(packages = ["com.myapp.svc.."]) you limit the classes to be analyzed to this package. So the input for your layered architecture will only be classes in the package com.myapp.svc. That's also why the layer defined by resideOutsideOfPackages("com.myapp.svc..") is empty, because there are no classes imported that reside outside of this package.

The problem you're facing is that whereLayer("Api").mayNotAccessAnyLayer() specifies that there may be no dependencies whatsoever from classes inside of the Api layer to classes outside of the Api layer. And the dependency on java.lang.Object is such a dependency (I see that the API is a little confusing there, because you could say that that class is "not contained in any layer" 🤔 Originally this was to avoid programming errors, to avoid missing some classes that are actually relevant, but maybe it's more confusing than helpful)

Anyway, to solve your problem at hand there are 2 ways:

  1. simply use sth. like the following on the layeredArchitecture to ignore all irrelevant dependencies (compare here)
.ignoreDependency(alwaysTrue(), resideInAnyPackage("java.."))
  1. write the layeredArchitecture in an inverse way, e.g.
 .whereLayer("Api").mayOnlyBeAccessedByLayers("Service")

We should maybe think about changing the behavior of mayOnlyAccessLayers though 🤔 I'm just not sure if it would invite to actually miss some app classes that just fly around outside of the defined layered structure and are therefore overlooked 🤔

@husam-e
Copy link
Author

husam-e commented Apr 5, 2022

Hey @codecholeric, thanks for the prompt solution! Your first suggestion worked well for me, using a more generic arg:

    @ArchTest
    fun `Coarse layers respect dependency boundaries 1`(importedClasses: JavaClasses) {
        Architectures.layeredArchitecture()
            .layer("Api").definedBy("..api..")
            .layer("Repository").definedBy("..repository..")
            .layer("Service").definedBy("..service..")
            .whereLayer("Api").mayNotAccessAnyLayer()
            .whereLayer("Service").mayOnlyAccessLayers("Api", "Repository")
            .ignoreDependency(
                DescribedPredicate.alwaysTrue(),
                JavaClass.Predicates.resideOutsideOfPackages("com.myapp.svc..")
            )
            .check(importedClasses)
    }

To your other points, the "scope" of the checks could prob be made clearer, but I can see why it is the way it is. However for layeredArchitecture, it may be good to at least document this scenario, since in practice, every codebase will depend on external classes unlike in the examples. So that can be highlighted explicitly, either with a first class feature like .ignoreClassesNotAnalyzed() or something, or with the suggestion above (the former could become a shorthand for the latter :) ).

I may want some test cases that focus on my own code (as highlighted in this issue), and others that check external dependencies, for which existing behavior will prob work well.

@codecholeric
Copy link
Collaborator

Yes, this is similar to the PlantUml configurations, where you can configure if you want to consider all dependencies, only dependencies in the diagram or all dependencies within certain packages 👍 Probably the API could also be extended here to allow something similar 🤔

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

No branches or pull requests

2 participants