Drop Java 8 support#900
Conversation
475e3a1 to
911908f
Compare
29c3911 to
c0cae34
Compare
c0cae34 to
28d7125
Compare
|
@coderabbitai review this PR |
|
Review: Drop Java 8 support (
|
| File | Note |
|---|---|
MapperProcessor.java |
return true → return false changes annotation claiming semantics; confirm intentional |
MetadataManagerTest.java |
500 ms → 5 s await timeout; worth a comment explaining why JDK 11+ is slower here |
InsightsClientTest.java line 112 |
"1.8.0" is mock JVM-version data for DSE Insights serialization tests — not a real Java 8 requirement, safe to leave |
All other 1.8 matches |
commons-text 1.8, lz4-java 1.8.1, Cassandra 2.1.8 — all library/Cassandra version numbers, not Java version references |
✅ Actions performed
Review triggered.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates the Java driver from Java 8 to Java 11 as the minimum supported version, modernizes build infrastructure with updated dependencies and tooling, replaces shaded Guava utilities with JDK equivalents, updates Mockito test infrastructure from deprecated runners to lifecycle-based management, and improves code quality across core libraries and tests. ChangesJava 11 Platform & Build Configuration
Guava Utility Removal & JDK Replacement
Core Library Code Quality & Logic Improvements
Test Infrastructure & Mockito Modernization
Documentation API Reference Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
guava-shaded/pom.xml (1)
61-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the obsolete Java 8
maven.main.skipguard inguava-shaded/pom.xml
build-helper-maven-pluginsetsmaven.main.skip=trueon any${java.version}that doesn’t start with1.8(^(?!1.8).+). On JDK 11+ this becomestrue, andmaven-compiler-pluginusesmaven.main.skipasskipMain, so this module’s main sources (e.g.,LexicographicalComparatorHolderSubstitution.java,UnsafeComparatorSubstitution.java) won’t compile—risking an incomplete shaded jar.🛠️ Proposed fix
- <plugin> - <!-- - Do not attempt to recompile the module if we are not running JVM 1.8. - Having additional Java source files and having to shade Guava will break - in CI environment, when we run 'mvn verify' on newer JVM and assume that - all classes have been compiled with JVM 1.8. - --> - <groupId>org.codehaus.mojo</groupId> - <artifactId>build-helper-maven-plugin</artifactId> - <version>1.12</version> - <executions> - <execution> - <id>regex-property</id> - <goals> - <goal>regex-property</goal> - </goals> - <configuration> - <name>maven.main.skip</name> - <value>${java.version}</value> - <regex>^(?!1.8).+</regex> - <replacement>true</replacement> - <failIfNoMatch>false</failIfNoMatch> - </configuration> - </execution> - </executions> - </plugin>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@guava-shaded/pom.xml` around lines 61 - 83, The pom contains an execution in build-helper-maven-plugin that sets the maven.main.skip property via the execution with id "regex-property" using regex "^(?!1.8).+". Remove that execution (or disable it) so the plugin no longer forces maven.main.skip=true for JDKs not starting with "1.8"; this ensures maven-compiler-plugin (skipMain) will compile the module's main sources (e.g., LexicographicalComparatorHolderSubstitution.java, UnsafeComparatorSubstitution.java) and the shaded jar is complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tests@v1.yml:
- Around line 116-117: The checkout step using
actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd currently does not
disable persisted credentials; update the "Checkout source" job step to add
persist-credentials: false to the actions/checkout invocation so the
GITHUB_TOKEN is not written to local git config for subsequent steps, matching
the hardening used elsewhere.
- Line 220: Remove the GH_TOKEN environment variable from the job step that runs
make resolve-cassandra-version; the Makefile target uses git ls-remote against
CASSANDRA_REPO and does not need github.token or the gh API, so ensure the step
invoking make resolve-cassandra-version (and any surrounding env block) does not
set GH_TOKEN and that no preceding commands in that same step call gh or GitHub
API endpoints that would require GH_TOKEN.
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/CodecRegistryConstants.java`:
- Line 35: PRIMITIVE_CODECS visibility was narrowed from public to
package-private, breaking the documented extension hook; restore its original
public visibility by changing the declaration of PRIMITIVE_CODECS in
CodecRegistryConstants back to public and keep/update the Javadoc to reflect it
as an external customization point so consumers can rely on it.
In
`@core/src/test/java/com/datastax/oss/driver/internal/core/ContactPointsTest.java`:
- Around line 141-143: Tests in ContactPointsTest construct DefaultEndPoint
using InetAddress.getLoopbackAddress(), which can be IPv6 (::1) on some JVMs and
break assertions expecting IPv4; update the test to use an explicit IPv4
loopback address (e.g., InetAddress.getByName("127.0.0.1") or
Inet4Address.getByName) when creating the InetSocketAddress for DefaultEndPoint
so the produced endpoint string matches the asserted "127.0.0.1:9042" form.
In `@Makefile`:
- Around line 112-120: The resolve_cassandra_latest function uses GNU-only sort
-V which fails on macOS/BSD; update the pipeline inside resolve_cassandra_latest
to replace sort -V with a POSIX dot-separated numeric sort (e.g. use sort -t.
-k1,1n -k2,2n -k3,3n) so versions are correctly ordered across platforms; ensure
the rest of the pipeline (git ls-remote, awk, sed, grep, tail) remains unchanged
and that the new sort receives the plain version strings output by sed/grep.
---
Outside diff comments:
In `@guava-shaded/pom.xml`:
- Around line 61-83: The pom contains an execution in build-helper-maven-plugin
that sets the maven.main.skip property via the execution with id
"regex-property" using regex "^(?!1.8).+". Remove that execution (or disable it)
so the plugin no longer forces maven.main.skip=true for JDKs not starting with
"1.8"; this ensures maven-compiler-plugin (skipMain) will compile the module's
main sources (e.g., LexicographicalComparatorHolderSubstitution.java,
UnsafeComparatorSubstitution.java) and the shaded jar is complete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 424de611-56c8-400c-b0e4-01a9af5f24bc
📒 Files selected for processing (94)
.github/workflows/docs-pages.yml.github/workflows/docs-pr.yml.github/workflows/release.yml.github/workflows/tests@v1.yml.mvn/jvm.configLICENSE_binaryMakefileREADME-dev.mdREADME.mdcore-shaded/pom.xmlcore/src/main/java/com/datastax/dse/driver/api/core/data/time/DateRangePrecision.javacore/src/main/java/com/datastax/dse/driver/internal/core/graph/GraphSON2SerdeTP.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/PackageUtil.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/PlatformInfoFinder.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/schema/InsightMetadata.javacore/src/main/java/com/datastax/dse/driver/internal/core/metadata/schema/DefaultDseAggregateMetadata.javacore/src/main/java/com/datastax/oss/driver/api/core/CqlIdentifier.javacore/src/main/java/com/datastax/oss/driver/api/core/context/DriverContext.javacore/src/main/java/com/datastax/oss/driver/api/core/data/CqlVector.javacore/src/main/java/com/datastax/oss/driver/api/core/metadata/token/Partitioner.javacore/src/main/java/com/datastax/oss/driver/api/core/metrics/Metrics.javacore/src/main/java/com/datastax/oss/driver/api/core/retry/RetryVerdict.javacore/src/main/java/com/datastax/oss/driver/api/core/type/codec/registry/CodecRegistry.javacore/src/main/java/com/datastax/oss/driver/api/core/type/reflect/GenericType.javacore/src/main/java/com/datastax/oss/driver/internal/core/addresstranslation/Ec2MultiRegionAddressTranslator.javacore/src/main/java/com/datastax/oss/driver/internal/core/addresstranslation/SubnetAddressTranslator.javacore/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/cql/DefaultTraceEvent.javacore/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTabletMap.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/PartitionerFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/ScriptBuilder.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/parsing/RawColumn.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/schema/queries/RuleBasedKeyspaceFilter.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/RandomTokenFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/TokenLong64.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/type/codec/extras/OptionalCodec.javacore/src/main/java/com/datastax/oss/driver/internal/core/type/codec/extras/enums/EnumOrdinalCodec.javacore/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/CodecRegistryConstants.javacore/src/main/java/com/datastax/oss/driver/internal/core/util/collection/QueryPlan.javacore/src/test/java/com/datastax/dse/driver/internal/core/graph/GraphSupportCheckerTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/CompletionStageAssert.javacore/src/test/java/com/datastax/oss/driver/internal/core/ContactPointsTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/addresstranslation/FixedHostNameAddressTranslatorTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/channel/MockChannelFactoryHelper.javacore/src/test/java/com/datastax/oss/driver/internal/core/data/AccessibleByIdTestBase.javacore/src/test/java/com/datastax/oss/driver/internal/core/data/AccessibleByIndexTestBase.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/session/MockChannelPoolFactoryHelper.javacore/src/test/java/com/datastax/oss/driver/internal/core/type/codec/TupleCodecTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/type/codec/UdtCodecTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/type/codec/registry/CachingCodecRegistryTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/type/util/VIntCodingTest.javadistribution/pom.xmlexamples/pom.xmlexamples/src/main/java/com/datastax/oss/driver/examples/paging/ForwardPagingRestUi.javaexamples/src/main/java/com/datastax/oss/driver/examples/paging/RandomPagingRestUi.javafaq/README.mdguava-shaded/pom.xmlguava-shaded/src/assembly/shaded-jar.xmlintegration-tests/pom.xmlintegration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverExecutionProfileReloadIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/metadata/NodeStateIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/session/ListenersIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/ssl/DefaultSslEngineFactoryIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/tracker/RequestLoggerIT.javamanual/core/address_resolution/README.mdmanual/core/async/README.mdmanual/core/custom_codecs/README.mdmanual/core/integration/README.mdmanual/core/metadata/schema/README.mdmanual/core/paging/README.mdmanual/mapper/config/README.mdmanual/mapper/daos/delete/README.mdmanual/mapper/daos/getentity/README.mdmanual/mapper/daos/increment/README.mdmanual/mapper/daos/insert/README.mdmanual/mapper/daos/query/README.mdmanual/mapper/daos/select/README.mdmanual/mapper/daos/update/README.mdmapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/MapperProcessor.javamapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/dao/DaoReturnTypeKind.javamapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/util/Classes.javametrics/micrometer/src/main/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerMetricUpdater.javametrics/micrometer/src/test/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerSessionMetricUpdaterTest.javaosgi-tests/pom.xmlperformance/duration-test.yamlpom.xmltest-infra/pom.xmltest-infra/src/main/java/com/datastax/oss/driver/api/testinfra/ccm/CcmBridge.javatest-infra/src/main/java/com/datastax/oss/driver/internal/gremlin/PatchedGremlinDslProcessor.java
💤 Files with no reviewable changes (2)
- core/src/main/java/com/datastax/dse/driver/internal/core/metadata/schema/DefaultDseAggregateMetadata.java
- core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java
5e66c61 to
352dc19
Compare
Micrometer requires valid expected histogram bounds even when SLA buckets are disabled. Stub the lowest, highest, and precision options in the no-SLA session metric test so it exercises the intended branch instead of failing while constructing the distribution configuration.
28d7125 to
f937deb
Compare
Raise the supported runtime and build floor to Java 11, and align docs, configuration, formatting, and dependency versions with that baseline.
Reject invalid subnet prefix strings so address translation fails clearly instead of accepting bad configuration.
PRIMITIVE_CODECS is an array, so final only prevents reassigning the field; callers that can reach a public field can still mutate its contents. The array is only used by codec registry internals, so keep it package-private instead of exposing mutable static state. This avoids MutablePublicArray-style warnings without changing registry behavior.
5077f7b to
020eef8
Compare
851f6ba to
e8880e1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
metrics/micrometer/src/main/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerMetricUpdater.java (1)
58-58: Micrometer(double)casts aren’t required for API compatibilityMicrometer’s
Counter.increment(...)only providesincrement()andincrement(double), andDistributionSummary.record(...)only providesrecord(double)(nolongoverloads). So the explicit(double)casts atMicrometerMetricUpdater.java(lines 58, 65, 73) are optional—Java will implicitly widenlongtodoublewhen calling these methods.getOrCreateCounterFor(metric).increment((double) amount);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@metrics/micrometer/src/main/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerMetricUpdater.java` at line 58, The explicit (double) casts are unnecessary when calling Micrometer methods; remove the redundant casts so calls use Java's implicit widening (e.g., change getOrCreateCounterFor(metric).increment((double) amount) to getOrCreateCounterFor(metric).increment(amount) and likewise remove casts when calling DistributionSummary.record(...) and Timer.record(...) in MicrometerMetricUpdater; update the three occurrences associated with getOrCreateCounterFor, getOrCreateDistributionSummaryFor (or DistributionSummary.record), and Timer.record to pass the long directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@metrics/micrometer/src/main/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerMetricUpdater.java`:
- Line 58: The explicit (double) casts are unnecessary when calling Micrometer
methods; remove the redundant casts so calls use Java's implicit widening (e.g.,
change getOrCreateCounterFor(metric).increment((double) amount) to
getOrCreateCounterFor(metric).increment(amount) and likewise remove casts when
calling DistributionSummary.record(...) and Timer.record(...) in
MicrometerMetricUpdater; update the three occurrences associated with
getOrCreateCounterFor, getOrCreateDistributionSummaryFor (or
DistributionSummary.record), and Timer.record to pass the long directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f1ee499-2297-4fa6-80ba-a315ca749c0a
📒 Files selected for processing (15)
core/src/main/java/com/datastax/oss/driver/internal/core/type/codec/registry/CodecRegistryConstants.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.javaexamples/pom.xmlexamples/src/main/java/com/datastax/oss/driver/examples/paging/ForwardPagingRestUi.javaexamples/src/main/java/com/datastax/oss/driver/examples/paging/RandomPagingRestUi.javaintegration-tests/pom.xmlintegration-tests/src/test/java/com/datastax/oss/driver/core/config/DriverExecutionProfileReloadIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/metadata/NodeStateIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/session/ListenersIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/tracker/RequestLoggerIT.javamapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/MapperProcessor.javametrics/micrometer/src/main/java/com/datastax/oss/driver/internal/metrics/micrometer/MicrometerMetricUpdater.javaosgi-tests/pom.xmltest-infra/pom.xmltest-infra/src/main/java/com/datastax/oss/driver/internal/gremlin/PatchedGremlinDslProcessor.java
✅ Files skipped from review due to trivial changes (2)
- osgi-tests/pom.xml
- examples/src/main/java/com/datastax/oss/driver/examples/paging/RandomPagingRestUi.java
6420ae9 to
1145fb9
Compare
Update maintained CI paths for the Java 11 baseline, including JDK 17 compiler flags, javadoc skips, authenticated version lookup, examples compilation, and runtime-specific reports.
1145fb9 to
34b0ac4
Compare
Enable required annotation processors, silence Java 11 compiler warnings, and improve CCM cleanup logging so integration failures are easier to diagnose. Update tests for newer Mockito behavior by avoiding shared MockitoJUnitRunner usage in parallel integration tests and by verifying cache interactions explicitly where the mock behavior changed. Fix CI failures exposed by the updated validation matrix: provide Micrometer histogram bounds in the no-SLA test, register config-change listeners before manual reloads, avoid assuming a transient one-connection node state before reconnection wins the race, and validate prepared-statement invalidation through observable cache behavior instead of a test-only removal-listener latch.
Keep PRIMITIVE_CODECS public for source and binary compatibility, deprecate direct array access, and add getPrimitiveCodecs() as the immutable-list accessor. Refs #908
34b0ac4 to
d9ac068
Compare
nikagra
left a comment
There was a problem hiding this comment.
Looks 👍 now.
Snyk security review results still to be addressed (I don't have permissions to check details)
Raise the supported runtime/build floor to Java 11 and remove active Java 8 references from build, CI, docs, and live config.
Follow-up:
CodecRegistryConstants.PRIMITIVE_CODECSin the 4.20.0 cycle under Issues for 4.20.0 #907.Verification:
Summary by CodeRabbit
Breaking Changes
Chores
Features