skoppu22 commented on code in PR #189:
URL:
https://github.com/apache/cassandra-analytics/pull/189#discussion_r3065347677
##########
cassandra-analytics-cdc/src/test/java/org/apache/cassandra/cdc/test/TestCdcBridgeProvider.java:
##########
@@ -35,39 +36,72 @@
import org.apache.cassandra.cdc.api.CdcOptions;
import org.apache.cassandra.cdc.msg.jdk.JdkMessageConverter;
-public class CdcBridgeProvider
+/**
+ * Test utility for managing CDC bridge instances per version. Call {@link
#setup()} explicitly
+ * before using any getter methods (e.g. from a {@code @BeforeAll} hook). Call
{@link #tearDown()}
+ * to release all cached bridges, temp directories, and classloaders.
+ */
+public class TestCdcBridgeProvider
{
private static final ConcurrentMap<CassandraVersion, CdcOptions> OPTIONS =
new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, Path> COMMIT_LOG_DIRS
= new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, CassandraBridge>
BRIDGES = new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, CdcBridge>
CDC_BRIDGES = new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, JdkMessageConverter>
MESSAGE_CONVERTERS = new ConcurrentHashMap<>();
+ private static final AtomicBoolean initialized = new AtomicBoolean(false);
- static
+ private TestCdcBridgeProvider()
{
- setup();
+ throw new IllegalStateException(getClass() + " is static utility class
and shall not be instantiated");
}
- private CdcBridgeProvider()
+ /**
+ * Initializes bridge instances for all test versions. Idempotent via
{@link AtomicBoolean};
+ * only the first call performs initialization. Must be called from {@code
@BeforeAll} in test classes.
+ */
+ public static void setup()
{
- throw new IllegalStateException(getClass() + " is static utility class
and shall not be instantiated");
+ if (initialized.compareAndSet(false, true))
+ {
+ TestVersionSupplier.testVersions().forEach(v -> {
+ try
+ {
+ setupVersion(v);
+ }
+ catch (IOException e)
+ {
+ throw new IllegalStateException(e);
+ }
+ });
+ }
}
- private static void setup()
+ /**
+ * Releases all cached bridges, temp commit log directories, and resets
the bridge factory.
+ * Call from {@code @AfterAll} in test base classes when explicit cleanup
is needed.
+ */
+ public static void tearDown()
{
- TestVersionSupplier.testVersions().forEach(v -> {
+ OPTIONS.clear();
+ CDC_BRIDGES.clear();
+ BRIDGES.clear();
+ MESSAGE_CONVERTERS.clear();
+ COMMIT_LOG_DIRS.forEach((version, dir) -> {
try
{
- setup(v);
+ Files.deleteIfExists(dir);
}
- catch (IOException e)
+ catch (IOException ignored)
{
- throw new IllegalStateException(e);
+ // best-effort cleanup
}
});
+ COMMIT_LOG_DIRS.clear();
+ CdcBridgeFactory.close();
Review Comment:
Don't we need to clear TypeCaches here?
##########
cassandra-analytics-cdc/src/test/java/org/apache/cassandra/cdc/test/TestCdcBridgeProvider.java:
##########
@@ -35,39 +36,72 @@
import org.apache.cassandra.cdc.api.CdcOptions;
import org.apache.cassandra.cdc.msg.jdk.JdkMessageConverter;
-public class CdcBridgeProvider
+/**
+ * Test utility for managing CDC bridge instances per version. Call {@link
#setup()} explicitly
+ * before using any getter methods (e.g. from a {@code @BeforeAll} hook). Call
{@link #tearDown()}
+ * to release all cached bridges, temp directories, and classloaders.
+ */
+public class TestCdcBridgeProvider
{
private static final ConcurrentMap<CassandraVersion, CdcOptions> OPTIONS =
new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, Path> COMMIT_LOG_DIRS
= new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, CassandraBridge>
BRIDGES = new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, CdcBridge>
CDC_BRIDGES = new ConcurrentHashMap<>();
private static final ConcurrentMap<CassandraVersion, JdkMessageConverter>
MESSAGE_CONVERTERS = new ConcurrentHashMap<>();
+ private static final AtomicBoolean initialized = new AtomicBoolean(false);
- static
+ private TestCdcBridgeProvider()
{
- setup();
+ throw new IllegalStateException(getClass() + " is static utility class
and shall not be instantiated");
}
- private CdcBridgeProvider()
+ /**
+ * Initializes bridge instances for all test versions. Idempotent via
{@link AtomicBoolean};
+ * only the first call performs initialization. Must be called from {@code
@BeforeAll} in test classes.
+ */
+ public static void setup()
{
- throw new IllegalStateException(getClass() + " is static utility class
and shall not be instantiated");
+ if (initialized.compareAndSet(false, true))
+ {
+ TestVersionSupplier.testVersions().forEach(v -> {
+ try
+ {
+ setupVersion(v);
+ }
+ catch (IOException e)
+ {
+ throw new IllegalStateException(e);
+ }
+ });
+ }
}
- private static void setup()
+ /**
+ * Releases all cached bridges, temp commit log directories, and resets
the bridge factory.
+ * Call from {@code @AfterAll} in test base classes when explicit cleanup
is needed.
+ */
+ public static void tearDown()
{
- TestVersionSupplier.testVersions().forEach(v -> {
+ OPTIONS.clear();
+ CDC_BRIDGES.clear();
+ BRIDGES.clear();
+ MESSAGE_CONVERTERS.clear();
+ COMMIT_LOG_DIRS.forEach((version, dir) -> {
try
{
- setup(v);
+ Files.deleteIfExists(dir);
}
- catch (IOException e)
+ catch (IOException ignored)
{
- throw new IllegalStateException(e);
+ // best-effort cleanup
}
});
+ COMMIT_LOG_DIRS.clear();
+ CdcBridgeFactory.close();
Review Comment:
Don't we need to clear TypeCache here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
