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

After use, destroy Dependent-scoped instances obtained via CdiLookupS… #1308

Merged
merged 1 commit into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.smallrye.graphql.cdi;

import javax.enterprise.inject.Instance;

import io.smallrye.graphql.spi.ManagedInstance;

public class CDIManagedInstance<T> implements ManagedInstance<T> {

private final Instance<T> instance;
private final T object;
private final boolean isDependentScoped;

CDIManagedInstance(Instance<T> instance, boolean isDependentScoped) {
this.instance = instance;
this.isDependentScoped = isDependentScoped;
this.object = instance.get();
}

@Override
public T get() {
return object;
}

@Override
public void destroyIfNecessary() {
if (isDependentScoped) {
instance.destroy(object);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
package io.smallrye.graphql.cdi;

import java.util.Set;

import javax.enterprise.context.Dependent;
import javax.enterprise.inject.AmbiguousResolutionException;
import javax.enterprise.inject.UnsatisfiedResolutionException;
import javax.enterprise.inject.spi.Bean;
import javax.enterprise.inject.spi.CDI;

import io.smallrye.graphql.spi.LookupService;
import io.smallrye.graphql.spi.ManagedInstance;

/**
* Lookup service that gets the beans via CDI
Expand All @@ -23,8 +30,21 @@ public Class<?> getClass(Class<?> declaringClass) {
}

@Override
public Object getInstance(Class<?> declaringClass) {
return CDI.current().select(declaringClass).get();
public <T> ManagedInstance<T> getInstance(Class<T> declaringClass) {
CDI<Object> cdi = CDI.current();
Bean<?> bean = getExactlyOneObject(cdi.getBeanManager().getBeans(declaringClass));
boolean isDependentScope = bean.getScope().equals(Dependent.class);
return new CDIManagedInstance<>(cdi.select(declaringClass), isDependentScope);
}

private <T> T getExactlyOneObject(Set<T> set) {
if (set.size() > 1) {
throw new AmbiguousResolutionException();
}
if (set.size() == 0) {
throw new UnsatisfiedResolutionException();
}
return set.iterator().next();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ public String getConfigKey() {

private ValidatorFactory getValidatorFactory() {
try {
ValidatorFactory validatorFactory = (ValidatorFactory) lookupService.getInstance(ValidatorFactory.class);
return validatorFactory;
return lookupService.getInstance(ValidatorFactory.class).get();
} catch (Exception t) {
return Validation.buildDefaultValidatorFactory();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
import io.smallrye.graphql.execution.event.InvokeInfo;
import io.smallrye.graphql.spi.ClassloadingService;
import io.smallrye.graphql.spi.LookupService;
import io.smallrye.graphql.spi.ManagedInstance;
import io.smallrye.mutiny.Multi;
import io.smallrye.mutiny.Uni;

/**
* Invoke methods using reflection
Expand Down Expand Up @@ -82,10 +85,22 @@ public <T> T invoke(Object... arguments) throws Exception {
arguments = injectContext(arguments);
}
try {
Object operationInstance = lookupService.getInstance(operationClass);
eventEmitter.fireBeforeMethodInvoke(new InvokeInfo(operationInstance, method, arguments));

return (T) this.method.invoke(operationInstance, arguments);
ManagedInstance<?> operationInstance = lookupService.getInstance(operationClass);
Object operationInstance1 = operationInstance.get();
eventEmitter.fireBeforeMethodInvoke(new InvokeInfo(operationInstance1, method, arguments));
T result = (T) method.invoke(operationInstance1, arguments);
if (result instanceof Uni) {
return (T) ((Uni) result).onTermination().invoke(() -> {
operationInstance.destroyIfNecessary();
});
} else if (result instanceof Multi) {
return (T) ((Multi) result).onTermination().invoke(() -> {
operationInstance.destroyIfNecessary();
});
} else {
operationInstance.destroyIfNecessary();
return result;
}
} catch (InvocationTargetException ex) {
//Invoked method has thrown something, unwrap
Throwable throwable = ex.getCause();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ static LookupService load() {

Class<?> getClass(Class<?> declaringClass);

Object getInstance(Class<?> declaringClass);
/**
* Obtain an instance of the requested class. Don't forget to call `destroyIfNecessary()` on it after use
* to avoid leaks!
*/
<T> ManagedInstance<T> getInstance(Class<T> declaringClass);

boolean isResolvable(Class<?> declaringClass);

Expand All @@ -59,9 +63,9 @@ public Class<?> getClass(Class<?> declaringClass) {
}

@Override
public Object getInstance(Class<?> declaringClass) {
public <T> ManagedInstance<T> getInstance(Class<T> declaringClass) {
try {
return declaringClass.getConstructor().newInstance();
return new DefaultManagedInstance<T>(declaringClass.getConstructor().newInstance());
} catch (NoSuchMethodException | SecurityException | InstantiationException | IllegalAccessException
| IllegalArgumentException | InvocationTargetException ex) {
throw msg.countNotGetInstance(ex);
Expand All @@ -73,4 +77,22 @@ public boolean isResolvable(Class<?> declaringClass) {
return true;
}
}

class DefaultManagedInstance<T> implements ManagedInstance<T> {
private final T instance;

public DefaultManagedInstance(T instance) {
this.instance = instance;
}

@Override
public T get() {
return instance;
}

@Override
public void destroyIfNecessary() {
// nothing
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.smallrye.graphql.spi;

public interface ManagedInstance<T> {

T get();

void destroyIfNecessary();

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package io.smallrye.graphql.tests.issue1307;

import static org.assertj.core.api.Assertions.assertThat;

import java.net.URL;
import java.util.concurrent.atomic.AtomicBoolean;

import javax.annotation.PreDestroy;
import javax.enterprise.context.Dependent;

import org.eclipse.microprofile.graphql.GraphQLApi;
import org.eclipse.microprofile.graphql.Query;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.junit.Arquillian;
import org.jboss.arquillian.test.api.ArquillianResource;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.EmptyAsset;
import org.jboss.shrinkwrap.api.spec.WebArchive;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;

import io.smallrye.graphql.tests.GraphQLAssured;

/**
* Test to verify that a @Dependent-scoped GraphQL API will get each instance destroyed after use,
* to prevent leaks
*/
@RunWith(Arquillian.class)
public class DependentScopeApiTest {

@Deployment
public static WebArchive deployment() {
return ShrinkWrap.create(WebArchive.class, "dependent-scope-test.war")
.addAsWebInfResource(EmptyAsset.INSTANCE, "beans.xml")
.addClasses(DependentScopedApi.class);
}

@ArquillianResource
URL testingURL;

@Test
public void test() {
GraphQLAssured graphQLAssured = new GraphQLAssured(testingURL);
String response = graphQLAssured
.post("query {hello}");

assertThat(response).isEqualTo("{\"data\":{\"hello\":\"hi\"}}");
Assert.assertTrue("Instance of the API class has to be destroyed after each request",
DependentScopedApi.DESTROYED.get());
}

@GraphQLApi
@Dependent
public static class DependentScopedApi {

public static AtomicBoolean DESTROYED = new AtomicBoolean();

@Query
public String hello() {
return "hi";
}

@PreDestroy
public void onDestroy() {
DESTROYED.set(true);
}

}
}