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

Fix MongoClientCache for factories with same/equal configurations #112

Merged
merged 1 commit into from Apr 3, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -58,13 +58,12 @@ public boolean equals(final Object o) {
return false;
}
final DefaultMongoClientFactory that = (DefaultMongoClientFactory) o;
return config.equals(that.config)
&& Objects.equals(mongoDriverInformation, that.mongoDriverInformation);
return config.equals(that.config);
}

@Override
public int hashCode() {
return Objects.hash(config, mongoDriverInformation);
return Objects.hash(config);
}

private static MongoDriverInformation generateMongoDriverInformation(final String configType) {
Expand Down
Expand Up @@ -29,6 +29,7 @@
import com.mongodb.spark.sql.connector.assertions.Assertions;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
Expand All @@ -46,7 +47,7 @@
*/
@ThreadSafe
final class MongoClientCache {
private final HashMap<MongoClientFactory, CachedMongoClient> cache = new HashMap<>();
private final Map<MongoClientFactory, CachedMongoClient> cache = new HashMap<>();
private final long keepAliveNanos;
private final long initialCleanUpDelayMS;
private final long cleanUpDelayMS;
Expand Down Expand Up @@ -92,7 +93,7 @@ synchronized MongoClient acquire(final MongoClientFactory mongoClientFactory) {
return cache
.computeIfAbsent(
mongoClientFactory,
(factory) -> new CachedMongoClient(this, factory.create(), keepAliveNanos))
factory -> new CachedMongoClient(this, factory.create(), keepAliveNanos))
.acquire();
}

Expand Down
@@ -0,0 +1,53 @@
package com.mongodb.spark.sql.connector.connection;

import com.mongodb.client.MongoClient;
import com.mongodb.client.internal.MongoClientImpl;
import com.mongodb.spark.sql.connector.config.MongoConfig;
import org.junit.jupiter.api.Test;

import java.util.HashMap;
import java.util.Map;

import static com.mongodb.spark.sql.connector.config.MongoConfig.CONNECTION_STRING_CONFIG;
import static com.mongodb.spark.sql.connector.config.MongoConfig.DATABASE_NAME_CONFIG;
import static com.mongodb.spark.sql.connector.config.MongoConfig.PREFIX;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotSame;

class DefaultMongoClientFactoryTest {

private static final Map<String, String> CONFIG_MAP = new HashMap<>();

static {
CONFIG_MAP.put(PREFIX + CONNECTION_STRING_CONFIG, "mongodb://localhost:27017");
CONFIG_MAP.put(PREFIX + DATABASE_NAME_CONFIG, "db");
}

@Test
void factoriesWithSameConfigCreateClientsWithEqualSettings() {
MongoConfig config = MongoConfig.createConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config);

MongoClient client1 = factory1.create();
MongoClient client2 = factory2.create();

assertInstanceOf(MongoClientImpl.class, client1);
assertInstanceOf(MongoClientImpl.class, client2);
assertEquals(
((MongoClientImpl) client1).getSettings(), ((MongoClientImpl) client2).getSettings());
}

@Test
void factoriesWithSameConfigCreateNotSameClients() {
MongoConfig config = MongoConfig.createConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config);

MongoClient client1 = factory1.create();
MongoClient client2 = factory2.create();

assertNotSame(client1, client2);
}
}
Expand Up @@ -18,27 +18,39 @@
package com.mongodb.spark.sql.connector.connection;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.mongodb.client.MongoClient;
import com.mongodb.spark.sql.connector.config.MongoConfig;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
public class MongoClientCacheTest {
import java.util.HashMap;
import java.util.Map;

@ExtendWith(MockitoExtension.class)
class MongoClientCacheTest {
private static final Map<String, String> CONFIG_MAP = new HashMap<>();
@Mock
private MongoClientFactory mongoClientFactory;

@Mock
private MongoClient mongoClient;

static {
CONFIG_MAP.put(
MongoConfig.PREFIX + MongoConfig.CONNECTION_STRING_CONFIG, "mongodb://localhost:27017");
CONFIG_MAP.put(MongoConfig.PREFIX + MongoConfig.DATABASE_NAME_CONFIG, "db");
}

@Test
void testNormalUsecase() {
void testNormalUseCase() {
MongoClientCache mongoClientCache = new MongoClientCache(0, 0, 100);
when(mongoClientFactory.create()).thenReturn(mongoClient);

Expand All @@ -56,6 +68,75 @@ void testNormalUsecase() {
verify(mongoClient, times(1)).close();
}

@Test
void factoriesWithSameConfigCreateSameClientsThroughCache() {
MongoConfig config = MongoConfig.createConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config);
MongoClientCache mongoClientCache = new MongoClientCache(0, 0, 100);

MongoClient client1 = mongoClientCache.acquire(factory1);
MongoClient client2 = mongoClientCache.acquire(factory2);

assertSame(client1, client2);
}

@Test
void factoriesWithEqualConfigCreateNotSameClientsThroughCache() {
MongoConfig config1 = MongoConfig.createConfig(CONFIG_MAP);
MongoConfig config2 = MongoConfig.createConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config1);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config2);
MongoClientCache mongoClientCache = new MongoClientCache(0, 0, 100);

MongoClient client1 = mongoClientCache.acquire(factory1);
MongoClient client2 = mongoClientCache.acquire(factory2);

assertNotSame(client1, client2);
}

@Test
void factoriesWithEqualReadConfigsCreateSameClientsThroughCache() {
MongoConfig config1 = MongoConfig.readConfig(CONFIG_MAP);
MongoConfig config2 = MongoConfig.readConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config1);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config2);
MongoClientCache mongoClientCache = new MongoClientCache(0, 0, 100);

MongoClient client1 = mongoClientCache.acquire(factory1);
MongoClient client2 = mongoClientCache.acquire(factory2);

assertSame(client1, client2);
}

@Test
void factoriesWithEqualWriteConfigsCreateNotSameClientsThroughCache() {
MongoConfig config1 = MongoConfig.writeConfig(CONFIG_MAP);
MongoConfig config2 = MongoConfig.writeConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config1);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config2);
MongoClientCache mongoClientCache = new MongoClientCache(0, 0, 100);

MongoClient client1 = mongoClientCache.acquire(factory1);
MongoClient client2 = mongoClientCache.acquire(factory2);

assertSame(client1, client2);
}

@Test
void factoriesWithEqualReadWriteConfigsCreateNotSameClientsThroughCache() {
MongoConfig config1 = MongoConfig.readConfig(CONFIG_MAP);
MongoConfig config2 = MongoConfig.writeConfig(CONFIG_MAP);
DefaultMongoClientFactory factory1 = new DefaultMongoClientFactory(config1);
DefaultMongoClientFactory factory2 = new DefaultMongoClientFactory(config2);
MongoClientCache mongoClientCache = new MongoClientCache(0, 0, 100);

MongoClient client1 = mongoClientCache.acquire(factory1);
MongoClient client2 = mongoClientCache.acquire(factory2);

assertNotSame(client1, client2);
}

@Test
void testKeepAliveReuseOfClient() {
MongoClientCache mongoClientCache = new MongoClientCache(500, 0, 200);
Expand Down