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

Fluency for notAccessed #254

Closed
abeyecon opened this issue Oct 24, 2019 · 7 comments
Closed

Fluency for notAccessed #254

abeyecon opened this issue Oct 24, 2019 · 7 comments
Labels

Comments

@abeyecon
Copy link

Using the following rule I can ensure that no impl class gets accessed directly (they will instead be injected via spring @component injection against their interface). However it doesn't read as well as it could, and the violation message doesn't read well either. Suggest adding .notBeAccessed()

@archtest
public static final ArchRule implOnlyUsedByInjection = classes()
.that().resideInAPackage(IMPL)
.should().onlyBeAccessed().byAnyPackage();

Violation Message:
Architecture Violation [Priority: MEDIUM] - Rule 'classes that reside in a package 'com.eyecon.xyz.impl..' should only be accessed by any package ['']' was violated (51 times):

@hankem
Copy link
Member

hankem commented Oct 24, 2019

Even though not fluent, you can easily use a custom condition:

    @ArchTest
    public static final ArchRule implOnlyUsedByInjection = classes()
            .that().resideInAPackage(IMPL)
            .should(notBeAccessedAtAll());

    static ArchCondition<JavaClass> notBeAccessedAtAll() {
        return new ArchCondition<JavaClass>("not be accessed at all") {
            @Override
            public void check(JavaClass clazz, ConditionEvents events) {
                Set<JavaAccess<?>> accessesToSelf = clazz.getAccessesToSelf();
                StringBuilder message = new StringBuilder(clazz.getName()).append(" is accessed ").append(accessesToSelf.size()).append(" times");
                if (!accessesToSelf.isEmpty()) {
                    message.append(":");
                }
                for (JavaAccess<?> javaAccess : accessesToSelf) {
                    message.append("\n").append(javaAccess.getDescription());
                }
                events.add(new SimpleConditionEvent(clazz, accessesToSelf.isEmpty(), message.toString()));
            }
        };
    }

Violation Message:

java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] -
 Rule 'classes that reside in a package 'com.eyecon.xyz.impl..' should not be accessed at all'
 was violated (51 times):

@abeyecon
Copy link
Author

Thanks @hankem that is a very tidy example

@codecholeric
Copy link
Collaborator

Just to mention, if it does what you want, and you only don't like the message, you could also just overwrite that:

@ArchTest
public static final ArchRule implOnlyUsedByInjection = classes()
    .that().resideInAPackage(IMPL)
    .should().onlyBeAccessed().byAnyPackage()

    .as("Spring bean implementations should not be directly accessed")
    .because("we want to use Spring DI and only to refer to them through their interface");

That's also a documentation for anyone reading your rule, what was the intention.

@abeyecon
Copy link
Author

        return new ArchCondition<JavaClass>("not be accessed at all") {
            @Override
            public void check(final JavaClass clazz, final ConditionEvents events) {
                final Set<JavaAccess<?>> accessesToSelf = clazz.getAccessesToSelf();
                final Set<JavaAccess<?>> externalAccessesToSelf = accessesToSelf.stream()
                        .filter(access -> access.getOriginOwner() != clazz)
                        .collect(Collectors.toSet());
                final StringBuilder message = new StringBuilder(clazz.getName()).append(" is accessed ")
                        .append(externalAccessesToSelf.size()).append(" times");
                if (!externalAccessesToSelf.isEmpty()) {
                    message.append(":");
                }
                for (final JavaAccess<?> javaAccess : externalAccessesToSelf) {
                    message.append("\n").append(javaAccess.getDescription());
                }
                events.add(new SimpleConditionEvent(clazz, externalAccessesToSelf.isEmpty(), message.toString()));
            }
        };
    }

Modified code to allow class to access itself internally

@abeyecon
Copy link
Author

Thanks for the help. Love ArchUnit. Looking forward to 1.0 so that our large business friends can use it without fighting with the corporate policies.

@codecholeric
Copy link
Collaborator

Yes, we should really release 1.0
There is no real reason for keeping it 0.x, it's stable and has been out there for a long time, it's mainly just me thinking "why should this be 1.0, it's not different from the last release" 🙄
I really feel #171 should be in there, but since I'm gonna merge that into the next release, maybe then it's time 😉

@codecholeric
Copy link
Collaborator

PS: If your question is solved, do you want to close the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants