TNG / ArchUnit

A Java architecture test library, to specify and assert architecture rules in plain Java
http://archunit.org
Apache License 2.0
3.23k stars 298 forks source link

Question about onlyBeAccessed() #39

Closed stefluhh closed 6 years ago

stefluhh commented 7 years ago

Hi,

I stumbled upon this amazing framework when I read the article on informatik-aktuell.de. The simple idea of this amazes me, because for our Microservices Domain Driven Design project I see a big potential to make use of it, since I am faced often with exactly these questions like "Why can I not directly access the ValueObject out from the ApplicationService" and so on.

So right now I am experimenting a bit and I am trying to setup this enforcement:

@ArchTest
public static final ArchRule enforceValueObjectsCanOnlyBeAccessFromOtherVOAndEntities =
        classes()
                .that().areAssignableTo(ValueObject.class)
                .should().onlyBeAccessed().byClassesThat().areAssignableTo(ValueObject.class)
                .orShould().onlyBeAccessed().byClassesThat().areAssignableTo(Entity.class);

However I seem to miss something, because running this code results in a lot of violations, where it even (!) says something like this:

Method <...Birthdate.hasAgeRequirement(int)> gets field <...Birthdate.birthdate> in (Birthdate.java:39)

How can this be? I mean obviously Birthdate was identified as an ValueObject (which it is), but why is it not allowed to access fields of itself?

Any help is appreciated.

Stef

codecholeric commented 7 years ago

This is weird, because I've added a simple example, a class ValueObject, a class Birthdate extends ValueObject with field Date birthdate, I've added your method Birthdate#hasAgeRequirement(int) accessing field birthdate, also added a class Entity and MyEntity extends Entity accessing Birthdate, and a class Evil not extending anything and accessing Birthdate. In my example, only Evil is reported as violation, Birthdate's self-access is okay, and so is the access from MyEntity. You are right, that the behavior you're reporting is not the expected one :wink: However, I would need some minimal example reproducing this, to understand, what's going on (the logic with andShould() and orShould() is not trivial, since each ArchCondition can add various 'events' which then are ANDed, ORed or negated). Could you try to create a minimal example showing the expected behavior? You could add it to archunit-example and create a PR, if you want, so I can look at it.

stefluhh commented 7 years ago

Thanks for the support, I'll investigate this again once I have time.

stefluhh commented 6 years ago

Hi @codecholeric

I created a simplest possible example in this Repository: https://github.com/SteluHH/ArchUnitQuestionExample

Can you have a look? I am not sure if I maybe also get the whole thing wrong, however one of the 2 tests does not fail for me, however I think it should. Note that the test above fails (as expected).

image

Interesting fact: When I replace Exception.class with RuntimeException.class it works: image

codecholeric commented 6 years ago

I think I found the problem, and your observation about Exception vs RuntimeException is close :wink: The reason is, that ArchUnit is missing some classes in the import to make the determination. You import all classes in the package of TestAggregate, these are three classes. For all references outside of the import, ArchUnit creates a stub, i.e. ArchUnit sees, that ForbiddenException inherits from RuntimeException, however, RuntimeException is not part of the import. Since the information is still important, ArchUnit will create a simple JavaClass with everything that can be determined, which is pretty much only "it's a class and it's called RuntimeException". However, ArchUnit has no way by default, to know that RuntimeException extends from Exception, thus your Example fails, but works, if you replace Exception by RuntimeException. (I hope I'm making sense...) There are two ways, to get what you want:

To do the second, you could create a file archunit.properties within src/test/resources and set the property

resolveMissingDependenciesFromClassPath=true

In both cases your example behaves as expected when I tested it, as far as I can see. Sorry, I know, this is a little tricky (I've had the same confusion in the past).

stefluhh commented 6 years ago

Thanks @codecholeric for the patient explanation, even though I apparently missed to read the manual :-) Now I got it, thanks!

codecholeric commented 6 years ago

As I said, I know the manual :wink:, but even I was confused at some point. Anyway, glad that your example works and that it was no bug :sweat_smile: I'll close this issue.