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: Forbid a method call only from specific methods, not whole classes? #1152

Closed
FlorianKutz opened this issue Aug 9, 2023 · 5 comments

Comments

@FlorianKutz
Copy link

To show what I mean, let's look at this example. I want to prevent that someone creates test objects within a method that is annotated with @Test. However, if the method is called within a method that does not have this annotation, it should be fine.

@QuarkusTest
class MyTest {
    @Inject
    PettingService pettingService;
    @Inject
    DBManager dbManager;

    @BeforeEach
    void prepare() {
        // Ok because in BeforeEach
        dbManager.createAnimal("Dog", "Larry");
    }

    @AfterEach
    void teardown() {
        dbManager.removeAnimal("Dog", "Larry");
    }

    @Test
    void goodTestGiveDogAPet() {
        // Ok. The dog was created in BeforeEach
        pettingService.giveDogAPat("Larry");
        assertTrue(pettingService.didDogGetAGoodPet("Larry"));
    }

    @Test
    void badTestGiveDogAPet() {
        // Not ok. This method must not be called in Test-methods
        dbManager.createAnimal("Dog", "Toska");

        pettingService.giveDogAPat("Toska");
        assertTrue(pettingService.didDogGetAGoodPet("Toska"));
        
        dbManager.removeAnimal("Dog", "Toska");
    }
}

I've seen this option, but it doesn't work here. The class should be able to call this method, just not within specific methods.

noClasses()
            .that()
            .areAnnotatedWith(QuarkusTest.class)
            .should()
            .callMethod(DBManager.class, "createAnimal", String.class, String.class);

So how can I make this work? There seems to be no option to just do this:

noMethods()
                .that()
                .areAnnotatedWith(Test.class)
                .should()
                .callMethod(DBManager.class, "createAnimal", String.class, String.class);
@hankem
Copy link
Member

hankem commented Aug 10, 2023

I believe that you're right; such a .callMethod is still missing in the fluent API (but would certainly be a nice addition 😉).

You can however define it as a custom ArchCondition, which could probably look like this:

imports
import com.tngtech.archunit.core.domain.JavaCodeUnit;
import com.tngtech.archunit.lang.ArchCondition;
import com.tngtech.archunit.lang.ConditionEvents;
import com.tngtech.archunit.lang.SimpleConditionEvent;

import static com.tngtech.archunit.core.domain.Formatters.formatMethod;
import static com.tngtech.archunit.core.domain.JavaCall.Predicates.target;
import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name;
import static com.tngtech.archunit.core.domain.properties.HasOwner.Predicates.With.owner;
import static com.tngtech.archunit.core.domain.properties.HasParameterTypes.Predicates.rawParameterTypes;
import static com.tngtech.archunit.lang.ConditionEvent.createMessage;
import static java.lang.String.format;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList;
static ArchCondition<? super JavaCodeUnit> callMethod(Class<?> methodOwner, String methodName, Class<?>... rawParameterTypes) {
    String methodDescription = format("method <%s>",
            formatMethod(methodOwner.getName(), methodName, stream(rawParameterTypes).map(Class::getName).collect(toList()))
    );
    return new ArchCondition<>("call " + methodDescription) {
        @Override
        public void check(JavaCodeUnit codeUnit, ConditionEvents events) {
            boolean satisfied = codeUnit.getMethodCallsFromSelf().stream().anyMatch(call ->
                    target(
                            owner(name(methodOwner.getName()))
                                    .and(name(methodName))
                                    .and(rawParameterTypes(rawParameterTypes))
                    ).test(call)
            );
            String message = createMessage(codeUnit, (satisfied ? "calls " : "does not call ") + methodDescription);
            events.add(new SimpleConditionEvent(codeUnit, satisfied, message));
        }
    };
}

To use it, you only have to change your code a little:

noMethods()
        .that()
        .areAnnotatedWith(Test.class)
        .should(callMethod(DBManager.class, "createAnimal", String.class, String.class));

@FlorianKutz
Copy link
Author

For some reason this check always triggered. Fortunately I was able to adapt this code to adapt this code:

    private static ArchCondition<? super JavaCodeUnit> callMethod(Class<?> methodOwner, String methodName, Class<?>... rawParameterTypes) {
        String methodDescription = String.format("method <%s>",
                formatMethod(methodOwner.getName(), methodName, Arrays.stream(rawParameterTypes).map(Class::getName).collect(Collectors.toList()))
        );
        return new ArchCondition<>("call " + methodDescription) {
            @Override
            public void check(JavaCodeUnit codeUnit, ConditionEvents events) {
                boolean satisfied = codeUnit.getMethodCallsFromSelf().stream().anyMatch(call -> {
                            List<JavaClass> callParamTypes = call.getOwner().getRawParameterTypes();
                            if (!methodName.equals(call.getName()) ||
                                    !methodOwner.equals(call.getTarget().getOwner().reflect()) ||
                                    rawParameterTypes.length != callParamTypes.size()) {
                                return false;
                            }

                            for(int i = 0; i < rawParameterTypes.length; i++) {
                                if(!rawParameterTypes[i].equals(callParamTypes.get(i).reflect())) {
                                    return false;
                                }
                            }
                            return true;
                        }
                );
                String message = createMessage(codeUnit, (satisfied ? "calls " : "does not call ") + methodDescription);
                events.add(new SimpleConditionEvent(codeUnit, satisfied, message));
            }
        };
    }

@hankem Your response was very helpful regardless as it showed me the right way. This could also be used to imitate the callMethodWhere-method. Thank you very much.

@hankem
Copy link
Member

hankem commented Aug 11, 2023

Hm, I'll double check that later; I was convinced that it worked in my version of your example. 🤔

But anyways, one suggestion for your version: rawParameterTypes[i].equals(callParamTypes.get(i).reflect() can be simplified to callParamTypes.get(i).isEquivalentTo(rawParameterTypes[i]), which is not only more concise, but should also be more performant.

@hankem
Copy link
Member

hankem commented Aug 11, 2023

It seems that the following solution using the static DescribedPredicate.and(DescribedPredicate...) instead of two instance methods .and(DescribedPredicate) used in #1152 (comment) works as expected (but I currently fail to explain the difference 🤔):

imports
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.core.domain.JavaCodeUnit;
import com.tngtech.archunit.lang.ArchCondition;
import com.tngtech.archunit.lang.ConditionEvents;
import com.tngtech.archunit.lang.SimpleConditionEvent;

import static com.tngtech.archunit.core.domain.Formatters.formatMethod;
import static com.tngtech.archunit.core.domain.JavaCall.Predicates.target;
import static com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo;
import static com.tngtech.archunit.core.domain.properties.HasName.Predicates.name;
import static com.tngtech.archunit.core.domain.properties.HasOwner.Predicates.With.owner;
import static com.tngtech.archunit.core.domain.properties.HasParameterTypes.Predicates.rawParameterTypes;
import static com.tngtech.archunit.lang.ConditionEvent.createMessage;
import static java.lang.String.format;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList;
static ArchCondition<? super JavaCodeUnit> callMethod(Class<?> methodOwner, String methodName, Class<?>... rawParameterTypes) {
    String methodDescription = format("method <%s>",
            formatMethod(methodOwner.getName(), methodName, stream(rawParameterTypes).map(Class::getName).collect(toList()))
    );
    return new ArchCondition<>("call " + methodDescription) {
        @Override
        public void check(JavaCodeUnit codeUnit, ConditionEvents events) {
            boolean satisfied = codeUnit.getMethodCallsFromSelf().stream().anyMatch(
                    target(DescribedPredicate.and(
                            owner(equivalentTo(methodOwner)),
                            name(methodName),
                            rawParameterTypes(rawParameterTypes)
                    ))
            );
            String message = createMessage(codeUnit, (satisfied ? "calls " : "does not call ") + methodDescription);
            events.add(new SimpleConditionEvent(codeUnit, satisfied, message));
        }
    };
}

@hankem
Copy link
Member

hankem commented Aug 11, 2023

Oooh... I've just realized that the .and methods in

    owner(name(methodOwner.getName()))
            .and(name(methodName))
            .and(rawParameterTypes(rawParameterTypes))

actually resolve to the static and method 🤯 – so this was nothing but a complicated way to denote

    DescribedPredicates
            .and(rawParameterTypes(rawParameterTypes))

i.e. callMethod(DBManager.class, "createAnimal", String.class, String.class) effectively only checked whether a method with two String parameters was called and therefore also matched dbManager.removeAnimal("Dog", "Toska"). Phew, that was tricky!

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