quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

The LOCAL class has a scope annotation but it must be ignored per the CDI rules

Open gbourant opened this issue 1 year ago • 4 comments

Describe the bug

If you run the HelloResourceTest it throws the following error: [error]: Build step io.quarkus.arc.deployment.ArcProcessor#generateResources threw an exception: jakarta.enterprise.inject.spi.DeploymentException: java.lang.IllegalStateException: The LOCAL class gr.AnotherTest$1Rest has a scope annotation but it must be ignored per the CDI rules.

I tried Include/Exclude Jakarta REST classes and quarkus.arc.exclude-types but it did not work. (not sure how to apply quarkus.arc.exclude-types for local classes.)

@QuarkusTest
class HelloResourceTest {
    @Test
    void testHelloEndpoint() {
    }
}
public class AnotherTest {

    @Test
    public void test(){
        {
            @ApplicationScoped
            class Rest   {}
        }
    }

    @Test
    public void test2(){
        {
            class Rest  io.quarkiverse.renarde.Controller {}
        }
    }
}

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

gbourant avatar Jun 30 '24 11:06 gbourant

/cc @Ladicek (arc), @manovotn (arc), @mkouba (arc)

quarkus-bot[bot] avatar Jun 30 '24 11:06 quarkus-bot[bot]

[error]: Build step io.quarkus.arc.deployment.ArcProcessor#generateResources threw an exception: jakarta.enterprise.inject.spi.DeploymentException: java.lang.IllegalStateException: The LOCAL class gr.AnotherTest$1Rest

This isn't a bug. Inner classes cannot become CDI beans as per CDI specification (here's link to the spec).

If you want Rest class to become a bean, you can always either make it a separate class or you can make the class static as static nested classes are supported.

manovotn avatar Jun 30 '24 14:06 manovotn

In my example there are only local classes (defined in a code block), we cannot use static in local classes, the code does not compile.

I write some ArchUnit tests, i don't want my classes to have behaviour, i just want to check "that class has that annotation".

Local classes can become very handy


@Test
public void testControllerPublicMethodsMustBeAnnotatedWithRestAnnotationsIsWorkingAsExpected() {
{
    class Rest extends Controller {    }
    classes()
    .should(ControllerArchitectureRules.CONTROLLER_PUBLIC_METHODS_MUST_BE_ANNOTATED_WITH_REST_ANNOTATIONS)
    .check(new ClassFileImporter().importClasses(Rest.class));
}

{
    class Rest extends Controller {
        public String hello() {
            return "should fail";
        }
    }

    classes()
    .should(ControllerArchitectureRules.CONTROLLER_PUBLIC_METHODS_MUST_BE_ANNOTATED_WITH_REST_ANNOTATIONS)
    .check(new ClassFileImporter().importClasses(Rest.class));

}
}

gbourant avatar Jun 30 '24 14:06 gbourant

In my example there are only local classes (defined in a code block), we cannot use static in local classes, the code does not compile.

I see, my comment still applies though, those cannot become beans. Quarkus tries to detect all classes with CDI bean defining annotations and reports errors as having such annotation in place while not being able to turn it into a bean is typically a user error.

I am not sure I understand the test case scenario here; why assert annotations that you just declared on a local class?

I tried Include/Exclude Jakarta REST classes and quarkus.arc.exclude-types but it did not work. (not sure how to apply quarkus.arc.exclude-types for local classes.)

Either way, if you really want to keep these around, then excluding them (quarkus.arc.exclude-types) if what I would try as well but I am not sure if we support that for local classes.

As for how to reference an inner class, you can see that directly in the exception - gr.AnotherTest$1Rest is how you reference the first Rest class. If you have multiple of them, the integer value increases. Try inputting that as value of quarkus.arc.exclude-types. Alternatively, you can try to exclude the whole package; some examples with wildcards are in the docs.

manovotn avatar Jun 30 '24 15:06 manovotn

I am not sure I understand the test case scenario here; why assert annotations that you just declared on a local class?

With ArchUnit I write two types of tests. The ones that are NOT shown here test the actual java classes (src/main/java), and the ones that are showing here are actually testing that the ArchUnit tests work as expected.

Yeah, I tried that quarkus.arc.exclude-types=gr.AnotherTest$1Rest even quarkus.arc.exclude-types=gr.* and it does not work.

So a test looks like this:


@Test
public void testControllerPrivateMethodsMustNotBeAnnotatedWithRestAnnotations() {

// 1st code block test
{
            @Path("rest")
            class Rest {
                @GET
                private String hello() {
                    return "world";
                }
            }

            try {
                should.check(new ClassFileImporter().importClasses(Rest.class));
                fail("It should have failed");
            } catch (AssertionError e) {
                var errorMessage = e.getMessage();
                var expected = """
                        Architecture Violation [Priority: MEDIUM] - Rule 'classes should not have private methods with annotations' was violated (1 times):
                        Method gr.admin.architecture.ArchitectureTest$6Rest.hello is annotated with [JavaAnnotation{jakarta.ws.rs.GET}]\
                        """;
                assertEquals(expected, errorMessage);
            }
        }
// 2nd code block test (omitted)
}

gbourant avatar Jun 30 '24 20:06 gbourant

@gbourant I'm sorry for my ignorance but what the point in annotating the local class with a scope annotation?

mkouba avatar Jul 01 '24 07:07 mkouba

@mkouba, an ArchUnit test validates that the source code fulfills some requirements. So when i run that test i see it passes,but how do i know that the ArchUnit test actually validates the thing it is supposed to? For that purpose i write another test as shown here which uses classes that i know that they are in an invalid state so if the ArchUnit throws an exception i know that it works as expected.

The reason I'm choosing to use local classes (locality of behaviour) is to keep the code more organized.

I don't know if that answered your question :)

gbourant avatar Jul 01 '24 07:07 gbourant

So if I undestand it correctly the @ApplicationScoped is an error condition that you want to test with ArchUnit but ignore during regular @QuarkusTests?

quarkus.arc.exclude-types won't work, the class is already ignored during discovery but catched by the WrongAnnotationUsageProcessor which attempts to protect users from similar errors. The only thing you can do is to disable this feature for tests in application.properties:

%test.quarkus.arc.detect-wrong-annotations=false

mkouba avatar Jul 01 '24 08:07 mkouba

So if I undestand it correctly the @ApplicationScoped is an error condition that you want to test with ArchUnit but ignore during regular @QuarkusTests?

Correct.

%test.quarkus.arc.detect-wrong-annotations=false

This one worked, but i do like safety. Is it possible to ignore some classes with some maven magic?

gbourant avatar Jul 01 '24 08:07 gbourant

%test.quarkus.arc.detect-wrong-annotations=false

This one worked, but i do like safety. Is it possible to ignore some classes with some maven magic?

Nope. But you should still get the warnings and failures during regular build and in the dev mode.

mkouba avatar Jul 01 '24 08:07 mkouba

Closing the issue as Martin's comment should provide a solution. If you disagree, feel free to reopen or just continue discussion.

manovotn avatar Jul 02 '24 05:07 manovotn