Missing dependency in a LayeredArchitecture

[mrtnlhmnn]

A few days ago I stumbled over an ArchUnit test for LayeredArchitecture (checking for unwanted dependencies in an Onion-like architecture).

This is a Spring Boot application. I want to check for unwanted direct dependencies to JPA entities from external interfaces - from both a REST interface and also a Web frontend. While this architecture dependency check works perfectly fine for the REST interface, it does not work for the Web frontend.

The main difference between the REST and the Web controller is that

• the REST Controller returns the persistence data and exposes it as the result (in the form of a JSON structure)
• while the Web Controller puts the data found to a generic internal model only (to be used in Thymeleaf then).

I set up a demo Github project https://github.com/mrtnlhmnn/archunit-dependency-problem. See its Readme.md:

Why is the 'OnionDependenciesTest' not reporting the unwanted dependency from 'WebControllerIncorrectlyUsingPersistenceData' - not before the explicit access (in line 36) is commented in? Note that it is reporting the unwanted dependency from 'RestControllerIncorrectlyUsingPersistenceData' correctly.

Any help very much appreciated!

[hankem]

Thanks for providing the demo project, that's very helpful! 💚

ArchUnit currently does not analyze local variables (#768), therefore WebControllerIncorrectlyUsingPersistenceData#dataDetails does not cause a violation unless you actually access EntityData:

public String dataDetails(@PathVariable("id") String id, Model model) {
        Optional<EntityData> data = service.findById(id);

        if (data.isEmpty()) {
            throw new ResponseStatusException(HttpStatus.NOT_FOUND, "data " + id + " not available");
        }
        else {
            // *** comment in the following line to see that than the dependency violation is indeed found and shown
            // int counter = data.get().counter;

            model.addAttribute("dataDetails", data.get());

            return "dataDetails"; // direct to Thymeleaf template
        }
    }

(In a way, the current code without the explicit access to counter is no actual dependency yet; it would still work if service.findById returned an Optional<Object>, right?)

In contrast, RestControllerIncorrectlyUsingPersistenceData#findById declares EntityData as a return type, which is considered a dependency (and therefore causes a violation):

    public EntityData findById(@PathVariable String id) {
        Optional<EntityData> entity = service.findById(id);

        if (entity.isEmpty()) {
            throw new IllegalStateException("nope");
        }
        else {
            return entity.get(); // transformed to JSON automatically
        }
    }

[mrtnlhmnn]

Thanks very much. Well, that's what I guessed: Local variable usage is not considered as a dependency. I am a bit confused by the fact that commenting in the line "int counter = data.get().counter;" does help. Still this is only a local variable usage but the test does then report the incorrect dependency. Anyway.

Thanks for the hint to #768 , I had missed this one.

You might want to tag this issue here as a duplicate of 768, I guess?!

[hankem]

ArchUnit does analyze each code unit's method calls, so a dependency is recorded once you actually use the EntityData API.

[mrtnlhmnn]

Ah, I see. Thanks.

[codecholeric]

For what it's worth, I think you only can catch these sorts of things reliably if you write something like a test for the surface of the public API of your service layer 🤔 I.e. prevent that the service may even return a persistence type. Because, even if ArchUnit would detect the local variable type, you could simply replace it by

Optional<?> entity = service.findById(id);

since no properties of the payload are directly used. But then again, the service could also offer a Object findById(long) {...} and again persistence data could be returned untyped. But in the end, the aspect ArchUnit cares about is how easily you can modify this code and how tangled it is. And while you technically still would serialize persistence data in this case, you could change this completely transparent for the other layers, since the types are not directly used.

[codecholeric]

I would close this issue, since I don't think this particular problem can really be fully solved with ArchUnit (at least the part that is concerned what really happens at runtime, i.e. at runtime no persistence types are passed).
The only follow up I could imagine is to support local variables, which would have found something in this case, but still wouldn't be bulletproof. In any case, there are already other issues that more directly address the desire for local variables, like #768
Any objections?

[mrtnlhmnn]

Absolutely no objections. If #768 could be adressed, this would be great and much appreciated. Thanks and keep up the good work!