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

Quarkus CDI-managed beans not available in Pact state callbacks #22611

Open
JapuDCret opened this issue Jan 4, 2022 · 19 comments
Open

Quarkus CDI-managed beans not available in Pact state callbacks #22611

JapuDCret opened this issue Jan 4, 2022 · 19 comments

Comments

@JapuDCret
Copy link

JapuDCret commented Jan 4, 2022

Problem

We have a Pact REST provider test with state callbacks, where we do not want to use testcontainers to startup the application, but rather a normal @QuarkusTest.
For easy use-cases this works, as mentioned in #9677 and demonstrated in https://github.com/skattela/pact-workshop-jvm-quarkus.
But when you need to use @State callbacks to initialize some state in your application, then you'll find out, that you cannot use CDI-managed beans.

Investigation

I investigated this issue and found out, that with that setup we generate two different test class instances.
I think this is normally also the case, but here Pact uses the instance, where Quarkus does not inject anything.

Here's the first instance creation (which will then be used by Pact)
https://github.com/quarkusio/quarkus/blob/2.6/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java#L687
A little below initTestState(extensionContext, state); is called, and it creates the "proper" instance:
https://github.com/quarkusio/quarkus/blob/2.6/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java#L727

Possible solutions

Maybe we can return the actualTestInstance within interceptTestClassConstructor(). (see first comment)

Environment (please complete the following information):

  • Output of uname -a or ver:
    Windows 10 - Version 20H2 (Build 19042.1415)
  • Output of java -version:
    openjdk version "11.0.10" 2021-01-19
  • GraalVM version (if different from Java):
    n/a
  • Quarkus version or git rev:
    2.5.0-Final
  • Build tool (ie. output of mvnw --version or gradlew --version):
    Apache Maven 3.8.4

Misc

Originally posted by @JapuDCret in #9677 (comment)

@geoand
Copy link
Contributor

geoand commented Jan 4, 2022

This is indeed the case, two instances are created in different ClassLoaders, unfortunately there is no way around that.

Maybe we can return the actualTestInstance within interceptTestClassConstructor().

This is bound to break a lot of things :)

@JapuDCret JapuDCret changed the title Quarkus creates two JUnit test instances Quarkus CDI-managed beans not available in Pact state callbacks Jan 4, 2022
@JapuDCret
Copy link
Author

That's what I thought, would also be too easy :D.
But do you see a way, how the proper instance can be forwarded to Pact?

The @State annotated methods are invoked here: https://github.com/pact-foundation/pact-jvm/blob/master/provider/junit5/src/main/kotlin/au/com/dius/pact/provider/junit5/PactVerificationStateChangeExtension.kt#L38-L39
so via the JUnit Jupiter callback BeforeTestExecutionCallback.


I remember now, that Quarkus provides its own callbacks with the proper ClassLoader, maybe we can "hack" a way to wrap the Pact class with this.
https://quarkus.io/guides/getting-started-testing#enrichment-via-quarkustestcallback

@geoand
Copy link
Contributor

geoand commented Jan 4, 2022

I remember now, that Quarkus provides its own callbacks with the proper ClassLoader, maybe we can "hack" a way to wrap the Pact class with this.
https://quarkus.io/guides/getting-started-testing#enrichment-via-quarkustestcallback

Yeah, we would need something like that

@JapuDCret
Copy link
Author

JapuDCret commented Jan 5, 2022

FYI: We now have a super hacky solution, but at least we can continue work now

First we have an abstract class, which every REST Pact Verificiation test relies on and in that we use two helper classes (PactClassLoaderAttributeHolder and PactRestVerificationStates).

First here's the abstract class, maybe just skim over it - the usage of the helper classes are explained after the code.

public abstract class AbstractPactHttpTest {

    protected static Map<String, Object> instanceMap = loadInstanceMap();

    @Inject
    @Any
    protected InMemoryConnector connector;

    @TestTemplate
    @ExtendWith(PactVerificationInvocationContextProvider.class)
    void pactVerificationTestTemplate(PactVerificationContext context) {
        if (context != null) {
            context.verifyInteraction();
        }
    }

    @BeforeEach
    void before(PactVerificationContext context) {
        if (context != null) {
            HttpTestTarget testTarget = new HttpTestTarget("localhost", 8081);
            context.setTarget(testTarget);

            final QuarkusClassLoader quarkusClassLoader = (QuarkusClassLoader) connector.getClass().getClassLoader();

            // store the QuarkusClassLoader, so we can use it later to load classes
            putInstance(QuarkusClassLoader.class, quarkusClassLoader);
            // store CDI-managed beans for use within the helpers
            putInstance(InMemoryConnector.class, connector);
            // initialize helpers with the instanceMap
            putInstance(PactRestVerificationStates.class, new PactRestVerificationStates(instanceMap));
        }
    }

    @SuppressWarnings("unchecked")
    protected static Map<String, Object> loadInstanceMap() {
        try {
            // load PactClassLoaderAttributeHolder from the system classloader, so it can be shared between distinct classloaders.
            final Class<?> attributeHolderClass = ClassLoader.getSystemClassLoader().loadClass(PactClassLoaderAttributeHolder.class.getName());

            final Method getInstanceMapMethod = attributeHolderClass.getMethod("getInstanceMap");

            return (Map<String, Object>) getInstanceMapMethod.invoke(null);
        } catch(Exception e) {
            throw new IllegalStateException("An unexpected error occurred, while loading PactRestVerificationStates: ", e);
        }
    }

    protected static <T> void putInstance(Class<T> clazz, T instance) {
        instanceMap.put(clazz.getName(), instance);
    }

    @SuppressWarnings("unchecked")
    protected static <T> T getInstance(Class<T> clazz) {
        return (T) getInstance(clazz.getName());
    }

    protected static Object getInstance(String className) {
        return instanceMap.get(className);
    }

    @SuppressWarnings("unchecked")
    protected static <T> T invokeMethod(String helperClassName, String methodName) {
        ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader();
        try {
            // load the helper instance within the quarkus classloader context, so we can use the CDI beans.
            Thread.currentThread().setContextClassLoader(getInstance(QuarkusClassLoader.class));
            Object helperInstance = getInstance(helperClassName);

            if (helperInstance == null) {
                throw new IllegalArgumentException("Could not find instance for " + helperClassName);
            }

            final Method helperMethod = helperInstance.getClass().getMethod(methodName);

            return (T) helperMethod.invoke(helperInstance);
        } catch(Exception e) {
            throw new IllegalStateException("An unexpected error occurred, while loading invoking the helper: ", e);
        } finally {
            Thread.currentThread().setContextClassLoader(originalClassLoader);
        }
    }
}

In that we use a class (PactClassLoaderAttributeHolder), which we will initialize with the system ClassLoader, so it'll be accessible from both contexts.
This class just exposes a Map, in which we'll store the CDI-managed beans:

public class PactClassLoaderAttributeHolder {
    private static Map<String, Object> instanceMap;

    public static Map<String, Object> getInstanceMap() {
        if (instanceMap == null) {
            instanceMap = new HashMap<>();
        }
        return instanceMap;
    }
}

These instances will then be used by another class, which will be loaded with the Quarkus ClassLoader:

public class PactRestVerificationStates {
    private final Map<String, Object> instanceMap;

    public PactRestVerificationStates(Map<String, Object> instanceMap) {
        this.instanceMap = instanceMap;
    }

    public void createMySuperAwesomeState() {
        // get the required CDI bean
        InMemoryConnector connector = (InMemoryConnector) instanceMap.get(InMemoryConnector.class.getName());

        // do something with the CDI bean
    }
}

Then finally we have the actual Pact provider test, which unfortunately cannot use direct referenced, or the classes are loaded within the wrong ClassLoader:

@QuarkusTest
@Provider("my-provider")
public class MyRestProviderPactVerification extends AbstractPactHttpTest {

    @State("my super awesome state")
    public void mySuperAwesomeState() {
        invokeMethod("com.example.infrastructure.pact.PactRestVerificationStates", "createMySuperAwesomeState");
    }
}

@JapuDCret
Copy link
Author

I would say, that this can be closed - as it's unlikely that something about the ClassLoader will be changed because of this.

It's still not working without the above mentioned workaround, but at least that workaround is available.

@EnvyIT
Copy link

EnvyIT commented Mar 24, 2022

@JapuDCret thanks for your workaround - Can you please share the import section of your classes too . I am currently wondering which InMemoryConnector you are using in your example.

@JapuDCret
Copy link
Author

@EnvyIT we use io.smallrye.reactive.messaging.providers.connectors.InMemoryConnector, from (see Quarkus Kafka > Testing without a Broker).
But this was just an example for a CDI-Bean, e.g. in our use-case we also provide ourselves with an EntityManager and a TransactionManager.

@EnvyIT
Copy link

EnvyIT commented Mar 24, 2022

Well, this is still a major issue in my opinion. The fact that it is not possible to use DI or Mockito for state callbacks are a huge disadvantage in comparison to other frameworks.

This workaround is indeed super-hacky and not working when you want to use Mockito proxy classes.

@holly-cummins
Copy link
Contributor

@edeandrea has come up with another workaround, which is still pretty hacky, but is slightly less verbose.

In a @BeforeEach method, we actually know what the state is. We also have more access to the CDI context.

So we can do the state processing in BeforeEach:

    @BeforeEach
    void beforeEach(PactVerificationContext context) {
        context.setTarget(new HttpTestTarget("localhost", this.quarkusPort));

        // Have to do this here because the CDI context doesn't seem to be available
        // in the @State method below
        var isMyState = Optional.ofNullable(context.getInteraction().getProviderStates())
            .orElseGet(List::of)
            .stream()
            .filter(state -> MY_STATE.equals(state.getName()))
            .count() > 0;

        if (isMyState) {
            // Do my stuff involving CDI and contexts and other Quarkus-y things
        }
    }

@edeandrea
Copy link
Contributor

You also still need the @State method. It's just a no-op.

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 7, 2022

/cc @geoand

@holly-cummins
Copy link
Contributor

I've semi-duplicated this as quarkiverse/quarkus-pact#2. It may be too hard/irrelevant to fix in main Quarkus, but something that an extension can fix.

I wonder if #27821 is related? Both have to do with undesirable classloaders being used in some parts of JUnit 5 test execution.

@Yrlish
Copy link

Yrlish commented Mar 31, 2023

Any updates on this? I've stumbled upon this too, it is very annoying having to do a weird workaround to be able to access Hibernate Entities through the @State-method. I need to setup a state where some entries in my database needs to exist.

@holly-cummins
Copy link
Contributor

"Update" is a strong word, but I have a plan-for-a-plan. The root cause of many of the issues we see where code in Pact tests loses access to Quarkus-y enhancements (like injected CDI beans) is classloading. We want the test classes to run with the Quarkus classloader, but because of the JUnit lifecycle manipulations, it ends up running in the system classloader. We've been a bit blocked, because there wasn't a way to tell JUnit to use an alternate classloader. However, apparently in JUnit 5.10, that limitation is lifted. JUnit 5.10 isn't released yet, so any fix would be a ways off, but I'm planning to start experiments to confirm we actually can switch classloaders, and see how many of these kinds of issues get fixed.

@Yrlish
Copy link

Yrlish commented Mar 31, 2023

Thanks for the update @holly-cummins! Now I know someone is looking on it and has some sort of plan at least, I understand it can take awhile, but that's alright to me. I'm just happy the issue hasn't been tossed to the side and forgotten in the dust. 🙂

@edeandrea
Copy link
Contributor

No no this is very much in the forefront! @holly-cummins and I are doing a lot of talking about Pact coming up in the next few months (https://devnexus.com/presentations/avoiding-common-pitfalls-with-modern-microservices-testing, https://www.devoxx.co.uk/talk/?id=1970, https://www.devbcn.com/talk/423678).

@holly-cummins is definitely the brains behind the operation, but it is definitely something we are continually enhancing.

@Davydhh
Copy link

Davydhh commented Oct 24, 2024

Hi guys, any updates?

@holly-cummins
Copy link
Contributor

Hi guys, any updates?

Sort of! I'm in the (slow) process of rewriting how Quarkus does its test classloading. I think that work should resolve this issue. I'd missed this when I made the list of issues https://github.com/orgs/quarkusio/projects/30 should/might fix, so I've added it to the tracking for that project.

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

No branches or pull requests

9 participants