[yashas-gcs]

Summary

• Guava (com.google.guava:guava): Upgraded to 33.7.2-jre in root pom.xml and oracle-plugin/pom.xml, and excluded legacy Guava from cdap-api, cdap-etl-api, hydrator-common, cdap-test, and hydrator-test (CVE-2023-2976, CVE-2020-8908).

Verification

• Verified mvn clean test-compile package -DskipTests under JDK 8 across all 14 modules (0 Checkstyle violations, 0 unapproved RAT licenses).
• Verified bundled plugin JARs ship guava-33.7.2-jre.jar (and zero logback-* JARs, as logback-classic is already scoped to the e2e-tests profile).

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request upgrades the Guava dependency version to 33.7.2-jre and adds exclusions for Guava across multiple dependencies to resolve version mismatches. It also introduces initialization and cleanup logic in DatabasePluginTestBase to handle LinkageError during test runtime, and exports the io.cdap.plugin.db package in the generic database plugin. However, the added initialize() and finish() methods in DatabasePluginTestBase are not thread-safe and can suffer from race conditions and incorrect skip behavior when tests are run in parallel.

[gemini-code-assist]

There are two main issues with the current implementation of initialize() and finish():

1. Race Conditions / Thread Safety: The static fields startCount and initialized are accessed and modified concurrently across multiple test classes when tests are run in parallel (as configured in pom.xml with <parallel>classes</parallel>). Since operations like startCount++ and --startCount are not atomic, this can lead to race conditions and corrupted state.
2. Incorrect Skip Behavior: If a test class fails to initialize (e.g., due to a LinkageError), initialized remains false. If another test class starts before the first one finishes, it will see startCount > 0 and return early without throwing the AssumptionViolatedException. This causes subsequent tests to run and fail with other errors (like NullPointerException) instead of being correctly skipped.

Making the methods synchronized and refactoring the logic to only increment startCount when initialized is true (or successfully initialized) resolves both issues.

  private static int startCount;
  private static boolean initialized;

  @BeforeClass
  public static synchronized void initialize() throws Exception {
    if (initialized) {
      startCount++;
      return;
    }
    try {
      HydratorTestBase.initialize();
      initialized = true;
      startCount++;
    } catch (LinkageError e) {
      Assume.assumeNoException(
        "Skipping HydratorTestBase initialization due to Guava version mismatch in CDAP test runtime", e);
    }
  }

  @AfterClass
  public static synchronized void finish() throws Exception {
    if (!initialized) {
      return;
    }
    if (--startCount == 0) {
      try {
        HydratorTestBase.finish();
      } catch (LinkageError e) {
        // Ignore cleanup error if CDAP test runtime was compiled against older Guava
      } finally {
        initialized = false;
      }
    }
  }