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.

Sign up for GitHub

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

Don't create default mongo clients when not needed and fix @MongoClientName qualifier #11568

Merged
merged 2 commits into from
Aug 27, 2020
Merged

Don't create default mongo clients when not needed and fix @MongoClientName qualifier #11568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
036d27f
Don't create default mongo clients when not needed
geoand Aug 24, 2020
17d4840
Properly handle @MongoClientName qualifier
geoand Aug 24, 2020
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
6 changes: 6 additions & 0 deletions ...nt/deployment/src/main/java/io/quarkus/mongodb/deployment/MongoClientBuildTimeConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,10 @@ public class MongoClientBuildTimeConfig {
*/
@ConfigItem(name = "metrics.enabled")
public boolean metricsEnabled;

/**
* If set to true, the default clients will always be created even if there are no injection points that use them
*/
@ConfigItem(name = "force-default-clients")
public boolean forceDefaultClients;
}
60 changes: 45 additions & 15 deletions ...b-client/deployment/src/main/java/io/quarkus/mongodb/deployment/MongoClientProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.arc.deployment.BeanContainerBuildItem;
import io.quarkus.arc.deployment.BeanDefiningAnnotationBuildItem;
import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem;
import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
import io.quarkus.arc.processor.BuildExtension;
import io.quarkus.arc.processor.DotNames;
import io.quarkus.arc.processor.InjectionPointInfo;
import io.quarkus.deployment.Feature;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
Expand All @@ -52,12 +54,9 @@
import io.quarkus.smallrye.health.deployment.spi.HealthBuildItem;

public class MongoClientProcessor {
private static final DotName MONGOCLIENT_ANNOTATION = DotName.createSimple(MongoClientName.class.getName());

@BuildStep
BeanDefiningAnnotationBuildItem registerConnectionBean() {
return new BeanDefiningAnnotationBuildItem(MONGOCLIENT_ANNOTATION);
}
private static final DotName MONGO_CLIENT_ANNOTATION = DotName.createSimple(MongoClientName.class.getName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it didn't work before, please let's move this annotation to the API package i.e. no runtime.. It's user API.

Also we will probably need to update the doc and quickstart if it's mentioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed to do this in a follow up

private static final DotName MONGO_CLIENT = DotName.createSimple(MongoClient.class.getName());
private static final DotName REACTIVE_MONGO_CLIENT = DotName.createSimple(ReactiveMongoClient.class.getName());

@BuildStep
CodecProviderBuildItem collectCodecProviders(CombinedIndexBuildItem indexBuildItem) {
Expand Down Expand Up @@ -94,7 +93,7 @@ public void mongoClientNames(ApplicationArchivesBuildItem applicationArchivesBui
BuildProducer<MongoClientNameBuildItem> mongoClientName) {
Set<String> values = new HashSet<>();
IndexView indexView = applicationArchivesBuildItem.getRootArchive().getIndex();
Collection<AnnotationInstance> mongoClientAnnotations = indexView.getAnnotations(MONGOCLIENT_ANNOTATION);
Collection<AnnotationInstance> mongoClientAnnotations = indexView.getAnnotations(MONGO_CLIENT_ANNOTATION);
for (AnnotationInstance annotation : mongoClientAnnotations) {
values.add(annotation.value().asString());
}
Expand Down Expand Up @@ -139,6 +138,9 @@ void build(
BuildProducer<SyntheticBeanBuildItem> syntheticBeanBuildItemBuildProducer,
BuildProducer<AdditionalBeanBuildItem> additionalBeans) {

// add the @MongoClientName class otherwise it won't registered as a qualifier
additionalBeans.produce(AdditionalBeanBuildItem.builder().addBeanClass(MongoClientName.class).build());

List<ConnectionPoolListener> poolListenerList = connectionPoolListenerProvider.stream()
.map(MongoConnectionPoolListenerBuildItem::getConnectionPoolListener)
.collect(Collectors.toList());
Expand All @@ -163,19 +165,47 @@ void build(
@Record(ExecutionTime.RUNTIME_INIT)
@BuildStep
void generateClientBeans(MongoClientRecorder recorder,
BeanRegistrationPhaseBuildItem registrationPhase,
List<MongoClientNameBuildItem> mongoClientNames,
MongoClientBuildTimeConfig mongoClientBuildTimeConfig,
MongodbConfig mongodbConfig,
List<MongoUnremovableClientsBuildItem> mongoUnremovableClientsBuildItem,
BuildProducer<SyntheticBeanBuildItem> syntheticBeanBuildItemBuildProducer) {

boolean makeUnremovable = !mongoUnremovableClientsBuildItem.isEmpty();

// default blocking client
syntheticBeanBuildItemBuildProducer.produce(createBlockingSyntheticBean(recorder, mongodbConfig, makeUnremovable,
MongoClientBeanUtil.DEFAULT_MONGOCLIENT_NAME, false));
// default reactive client
syntheticBeanBuildItemBuildProducer.produce(createReactiveSyntheticBean(recorder, mongodbConfig, makeUnremovable,
MongoClientBeanUtil.DEFAULT_MONGOCLIENT_NAME, false));
boolean createDefaultBlockingMongoClient = false;
boolean createDefaultReactiveMongoClient = false;
if (makeUnremovable || mongoClientBuildTimeConfig.forceDefaultClients) {
// all clients are expected to exist in this case
createDefaultBlockingMongoClient = true;
createDefaultReactiveMongoClient = true;
} else {
// we only create the default client if they are actually used by injection points
for (InjectionPointInfo injectionPoint : registrationPhase.getContext().get(BuildExtension.Key.INJECTION_POINTS)) {
DotName injectionPointType = injectionPoint.getRequiredType().name();
if (injectionPointType.equals(MONGO_CLIENT) && injectionPoint.hasDefaultedQualifier()) {
createDefaultBlockingMongoClient = true;
} else if (injectionPointType.equals(REACTIVE_MONGO_CLIENT) && injectionPoint.hasDefaultedQualifier()) {
createDefaultReactiveMongoClient = true;
}

if (createDefaultBlockingMongoClient && createDefaultReactiveMongoClient) {
break;
}
}
}

if (createDefaultBlockingMongoClient) {
syntheticBeanBuildItemBuildProducer.produce(createBlockingSyntheticBean(recorder, mongodbConfig,
makeUnremovable || mongoClientBuildTimeConfig.forceDefaultClients,
MongoClientBeanUtil.DEFAULT_MONGOCLIENT_NAME, false));
}
if (createDefaultReactiveMongoClient) {
syntheticBeanBuildItemBuildProducer.produce(createReactiveSyntheticBean(recorder, mongodbConfig,
makeUnremovable || mongoClientBuildTimeConfig.forceDefaultClients,
MongoClientBeanUtil.DEFAULT_MONGOCLIENT_NAME, false));
}

for (MongoClientNameBuildItem mongoClientName : mongoClientNames) {
// named blocking client
Expand Down Expand Up @@ -229,7 +259,7 @@ private SyntheticBeanBuildItem applyCommonBeanConfig(boolean makeUnremovable, St
String namedQualifier = MongoClientBeanUtil.namedQualifier(clientName, false);
configurator.addQualifier().annotation(DotNames.NAMED).addValue("value", namedQualifier).done();
if (addMongoClientQualifier) {
configurator.addQualifier().annotation(MONGOCLIENT_ANNOTATION).addValue("value", clientName).done();
configurator.addQualifier().annotation(MONGO_CLIENT_ANNOTATION).addValue("value", clientName).done();
}
}
return configurator.done();
Expand Down
12 changes: 12 additions & 0 deletions ...ent/deployment/src/test/java/io/quarkus/mongodb/DefaultAndNamedMongoClientConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.junit.jupiter.api.extension.RegisterExtension;

import com.mongodb.client.MongoClient;
import com.mongodb.client.internal.MongoClientImpl;

import io.quarkus.arc.Arc;
import io.quarkus.mongodb.runtime.MongoClientName;
Expand Down Expand Up @@ -44,6 +45,9 @@ void cleanup() {

@Test
public void testNamedDataSourceInjection() {
assertProperConnection(client, 27018);
assertProperConnection(client2, 27019);

assertThat(client.listDatabases().first()).isNotEmpty();
assertThat(client2.listDatabases().first()).isNotEmpty();

Expand All @@ -52,4 +56,12 @@ public void testNamedDataSourceInjection() {
assertThat(Arc.container().instance(MongoClient.class, NamedLiteral.of("cluster2")).get()).isNotNull();
assertThat(Arc.container().instance(MongoClient.class, NamedLiteral.of("cluster3")).get()).isNull();
}

private void assertProperConnection(MongoClient client, int expectedPort) {
assertThat(client).isInstanceOfSatisfying(MongoClientImpl.class, c -> {
assertThat(c.getCluster().getSettings().getHosts()).hasOnlyOneElementSatisfying(sa -> {
assertThat(sa.getPort()).isEqualTo(expectedPort);
});
});
}
}
23 changes: 23 additions & 0 deletions ...ongodb-client/deployment/src/test/java/io/quarkus/mongodb/NamedMongoClientConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,23 @@

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

import java.lang.annotation.Annotation;

import javax.enterprise.inject.Default;
import javax.inject.Inject;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.mongodb.client.MongoClient;

import io.quarkus.arc.Arc;
import io.quarkus.arc.InjectableBean;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.mongodb.runtime.MongoClientName;
import io.quarkus.test.QuarkusUnitTest;

Expand Down Expand Up @@ -44,5 +51,21 @@ void cleanup() {
public void testNamedDataSourceInjection() {
assertThat(client.listDatabases().first()).isNotEmpty();
assertThat(client2.listDatabases().first()).isNotEmpty();

assertNoDefaultClient();
}

private void assertNoDefaultClient() {
boolean hasDefault = false;
for (InstanceHandle<MongoClient> handle : Arc.container().select(MongoClient.class).handles()) {
InjectableBean<MongoClient> bean = handle.getBean();
for (Annotation qualifier : bean.getQualifiers()) {
if (qualifier.annotationType().equals(Default.class)) {
hasDefault = true;
}
}
}
Assertions.assertFalse(hasDefault,
"The default mongo client should not have been present as it is not used in any injection point");
}
}
52 changes: 52 additions & 0 deletions ...lient/deployment/src/test/java/io/quarkus/mongodb/NamedReactiveMongoClientConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,26 @@

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

import java.lang.annotation.Annotation;
import java.lang.reflect.Field;

import javax.enterprise.inject.Default;
import javax.inject.Inject;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.mongodb.reactivestreams.client.MongoClient;
import com.mongodb.reactivestreams.client.internal.MongoClientImpl;

import io.quarkus.arc.Arc;
import io.quarkus.arc.InjectableBean;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.mongodb.impl.ReactiveMongoClientImpl;
import io.quarkus.mongodb.reactive.ReactiveMongoClient;
import io.quarkus.mongodb.runtime.MongoClientName;
import io.quarkus.test.QuarkusUnitTest;
Expand Down Expand Up @@ -41,7 +53,47 @@ void cleanup() {

@Test
public void testNamedDataSourceInjection() {
assertProperConnection(client, 27018);
assertProperConnection(client2, 27019);

assertThat(client.listDatabases().collectItems().first().await().indefinitely()).isNotEmpty();
assertThat(client2.listDatabases().collectItems().first().await().indefinitely()).isNotEmpty();

assertNoDefaultClient();
}

private void assertProperConnection(ReactiveMongoClient client, int expectedPort) {
assertThat(client).isInstanceOfSatisfying(ReactiveMongoClientImpl.class, rc -> {
Field mongoClientField;
try {
mongoClientField = ReactiveMongoClientImpl.class.getDeclaredField("client");
} catch (NoSuchFieldException e) {
throw new RuntimeException(e);
}
mongoClientField.setAccessible(true);
MongoClient c;
try {
c = (MongoClientImpl) mongoClientField.get(rc);
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
assertThat(c.getClusterDescription().getClusterSettings().getHosts()).hasOnlyOneElementSatisfying(sa -> {
assertThat(sa.getPort()).isEqualTo(expectedPort);
});
});
}

private void assertNoDefaultClient() {
boolean hasDefault = false;
for (InstanceHandle<ReactiveMongoClient> handle : Arc.container().select(ReactiveMongoClient.class).handles()) {
InjectableBean<ReactiveMongoClient> bean = handle.getBean();
for (Annotation qualifier : bean.getQualifiers()) {
if (qualifier.annotationType().equals(Default.class)) {
hasDefault = true;
}
}
}
Assertions.assertFalse(hasDefault,
"The default reactive mongo client should not have been present as it is not used in any injection point");
}
}