Skip to content

Replace deprecated third-party and JDK API calls with modern equivalents#18363

Merged
mayankshriv merged 1 commit into
apache:masterfrom
mayankshriv:deprecation-cleanup-quick-wins
Apr 30, 2026
Merged

Replace deprecated third-party and JDK API calls with modern equivalents#18363
mayankshriv merged 1 commit into
apache:masterfrom
mayankshriv:deprecation-cleanup-quick-wins

Conversation

@mayankshriv
Copy link
Copy Markdown
Contributor

@mayankshriv mayankshriv commented Apr 28, 2026

Summary

First round of deprecation cleanup, targeting mechanical, low-risk replacements of deprecated third-party and JDK APIs. Follow-up PRs will address the remaining warnings that require more involved changes (internal Pinot API migrations, larger refactors, etc.).

Each deprecated API and its replacement was identified by:

  1. Compiling with -Xlint:deprecation to find all deprecation warnings
  2. Reading the @deprecated Javadoc on each method in the library source to find the documented replacement
  3. Verifying the replacement compiles and preserves existing behavior

Replacements

Deprecated API Replacement Source
ObjectNode.put(String, JsonNode) .set(String, JsonNode) Jackson 2.21 Javadoc: "Since 2.4 use either set or replace"
JsonNode.fields() .properties() Jackson 2.21 Javadoc: "As of 2.19, replaced by properties()"
Class.newInstance() getDeclaredConstructor().newInstance() JDK 9+: deprecated due to checked exception propagation issues
FileUtils.readFileToString(File) readFileToString(File, Charset) commons-io Javadoc: "Use readFileToString(File, Charset) instead"
FileUtils.write(File, CharSequence) write(File, CharSequence, Charset) commons-io Javadoc: "Use write(File, CharSequence, Charset) instead"
IOUtils.toString(InputStream) toString(InputStream, Charset) commons-io Javadoc: "Use toString(InputStream, Charset) instead"
IOUtils.write(String, OutputStream) write(String, OutputStream, Charset) commons-io Javadoc: "Use write(String, OutputStream, Charset) instead"
Files.createTempDir() (Guava) java.nio.file.Files.createTempDirectory() Guava Javadoc: deprecated in favor of JDK equivalent
StringUtils.remove(String, String) Strings.CS.remove(String, String) commons-lang3 Javadoc: "Use Strings.CS.remove"
StringUtils.replace(String, String, String) Strings.CS.replace(String, String, String) commons-lang3 Javadoc: "Use Strings.CS.replace"
StringUtils.compare(String, String) Strings.CS.compare(String, String) commons-lang3 Javadoc: "Use Strings.CS.compare"
RandomStringUtils.random*() RandomStringUtils.secure().next*() commons-lang3 Javadoc: "Use next*() from secure(), secureStrong(), or insecure()". Used secure() since the deprecated statics delegate to secure().

Note: StringUtils.remove(String, char) is NOT deprecated — only the (String, String) overload is. Those calls were left unchanged.

Test plan

  • spotless:apply — all affected modules formatted
  • checkstyle:check — zero violations across all affected modules
  • license:check — all headers valid
  • test-compile — compiles cleanly with zero new deprecation warnings for the replaced APIs
  • CI full test suite

🤖 Generated with Claude Code

@mayankshriv mayankshriv marked this pull request as draft April 28, 2026 23:39
@mayankshriv mayankshriv changed the title Replace ~200 deprecated API calls with modern equivalents Replace deprecated third-party and JDK API calls with modern equivalents Apr 29, 2026
@mayankshriv mayankshriv force-pushed the deprecation-cleanup-quick-wins branch from a2ce815 to 31482a7 Compare April 29, 2026 04:14
@mayankshriv mayankshriv marked this pull request as ready for review April 29, 2026 04:18
@mayankshriv mayankshriv force-pushed the deprecation-cleanup-quick-wins branch 2 times, most recently from 49c895a to 8e5cff4 Compare April 29, 2026 05:37
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 56.60377% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.44%. Comparing base (6d85277) to head (a9e4c3b).

Files with missing lines Patch % Lines
...ache/pinot/broker/broker/AccessControlFactory.java 0.00% 2 Missing ⚠️
.../apache/pinot/common/auth/BasicAuthTokenUtils.java 0.00% 2 Missing ⚠️
...commender/rules/impl/RealtimeProvisioningRule.java 50.00% 2 Missing ⚠️
...roller/recommender/rules/impl/SegmentSizeRule.java 50.00% 2 Missing ⚠️
...not/minion/event/EventObserverFactoryRegistry.java 0.00% 2 Missing ⚠️
...t/minion/executor/TaskExecutorFactoryRegistry.java 0.00% 2 Missing ⚠️
.../columnminmaxvalue/ColumnMinMaxValueGenerator.java 50.00% 2 Missing ⚠️
...pinot/broker/api/resources/PinotClientRequest.java 0.00% 1 Missing ⚠️
.../apache/pinot/client/admin/SegmentAdminClient.java 0.00% 1 Missing ⚠️
.../pinot/common/function/scalar/StringFunctions.java 0.00% 1 Missing ⚠️
... and 6 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18363      +/-   ##
============================================
- Coverage     63.45%   63.44%   -0.02%     
  Complexity     1683     1683              
============================================
  Files          3253     3253              
  Lines        198884   198897      +13     
  Branches      30798    30798              
============================================
- Hits         126196   126181      -15     
- Misses        62620    62644      +24     
- Partials      10068    10072       +4     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 63.44% <56.60%> (-0.02%) ⬇️
temurin 63.44% <56.60%> (-0.02%) ⬇️
unittests 63.43% <56.60%> (-0.02%) ⬇️
unittests1 55.36% <47.61%> (-0.02%) ⬇️
unittests2 35.02% <45.28%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0
Copy link
Copy Markdown
Contributor

overall lgtm.
Apart from this one-time cleanup, can you add those deprecation into checkstyle.xml to avoid re-introduce those APIs ?
Or maybe we should throw compilation error for deprecated APIs?

@mayankshriv
Copy link
Copy Markdown
Contributor Author

overall lgtm. Apart from this one-time cleanup, can you add those deprecation into checkstyle.xml to avoid re-introduce those APIs ? Or maybe we should throw compilation error for deprecated APIs?

Thanks @xiangfu0 . There doesn't seem to be a checkstyle hook to prevent that, but I had added a lint step in the precommit skill in this PR: #18361

@mayankshriv
Copy link
Copy Markdown
Contributor Author

Added 4 checkstyle rules to config/checkstyle.xml to prevent reintroduction of the deprecated APIs cleaned up here:

  1. IllegalImport — bans com.google.common.io.Files (use java.nio.file.Files instead)
  2. RegexpSinglelineRandomStringUtils.random*()RandomStringUtils.secure().next*()
  3. RegexpSinglelineStringUtils.compare()Strings.CS.compare()
  4. RegexpSinglelineStringUtils.replace()Strings.CS.replace()

These 4 have zero false positives — the deprecated and non-deprecated APIs have distinct names, so the regex matches are precise.

The remaining deprecated APIs we cleaned up (ObjectNode.put, Class.newInstance, JsonNode.fields, StringUtils.remove, FileUtils/IOUtils charset overloads) share method names with non-deprecated overloads, so regex-based rules would produce too many false positives. Those are better caught by compiling with -Xlint:deprecation.

@mayankshriv mayankshriv force-pushed the deprecation-cleanup-quick-wins branch 2 times, most recently from 1372a46 to ddfbe98 Compare April 29, 2026 23:15
First round of deprecation cleanup, targeting mechanical, low-risk
replacements of deprecated third-party and JDK APIs. Each replacement
was identified by compiling with -Xlint:deprecation, reading the
@deprecated Javadoc in the library source, and verifying the
replacement compiles and preserves existing behavior.

Replacements:
- ObjectNode.put(String, JsonNode) → .set(String, JsonNode)
- JsonNode.fields() → .properties()
- Class.newInstance() → getDeclaredConstructor().newInstance()
- FileUtils.readFileToString/write — add Charset parameter
- IOUtils.toString/write — add Charset parameter
- Guava Files.createTempDir() → JDK Files.createTempDirectory()
- StringUtils.remove/replace/compare → Strings.CS equivalents
- RandomStringUtils.random*() → secure().next*()

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mayankshriv mayankshriv force-pushed the deprecation-cleanup-quick-wins branch from ddfbe98 to a9e4c3b Compare April 29, 2026 23:17
@xiangfu0 xiangfu0 added the cleanup Code cleanup or removal of dead code label Apr 30, 2026
@mayankshriv mayankshriv merged commit 88b0d29 into apache:master Apr 30, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Code cleanup or removal of dead code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants