Skip to content

Fix deprecated methods - Reporting plugin - #548

Closed
parth-sharma-10 wants to merge 1 commit into
openMF:developfrom
parth-sharma-10:fix/reporting-plugin-deprecated-methods
Closed

parth-sharma-10 wants to merge 1 commit into
openMF:developfrom
parth-sharma-10:fix/reporting-plugin-deprecated-methods

Conversation

@parth-sharma-10

@parth-sharma-10 parth-sharma-10 commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

After the Java 25 / Spring Boot 4.1 upgrade, ./mvnw clean package reports two deprecation warnings in the reporting plugin:

  • BirtReadOnlyConnectionFactory uses StringUtils.containsIgnoreCase, deprecated in commons-lang3 3.20 in favor of Strings.CI.contains.
  • BirtReportingProcessServiceImpl#setConnectionDetail calls .put() on the raw Map returned by BIRT's IEngineTask#getAppContext() (a third-party API that predates generics), producing unchecked-operation warnings.

Changes

  • Replace StringUtils.containsIgnoreCase with Strings.CI.contains in beginReadOnlyTransaction.
  • Scope @SuppressWarnings("unchecked") to setConnectionDetail, with a comment noting why (raw Map from BIRT's API, not something we control).

Verified with JAVA_HOME pointed at JDK 25: mvnw clean compile -Dmaven.compiler.showDeprecation=true now reports zero warnings from plugin source (the remaining sun.misc.Unsafe warning is lombok's own internal code, unrelated to this plugin's source).

Test plan

  • ./mvnw clean compile on JDK 25 — no deprecation/unchecked warnings from plugin source
  • ./mvnw test-compile on JDK 25 — clean
  • ./mvnw test -Dtest=BirtReadOnlyConnectionFactoryTest,BirtReportingProcessServiceImplTest — 44/44 pass
  • ./mvnw spotless:check — clean

Summary by CodeRabbit

  • Chores
    • Updated internal compatibility handling for case-insensitive database URL detection without changing behavior.
    • Suppressed a compiler warning related to BIRT connection context handling; no user-facing functionality changed.

- BirtReadOnlyConnectionFactory: StringUtils.containsIgnoreCase is
  deprecated in commons-lang3 3.20; use Strings.CI.contains instead.
- BirtReportingProcessServiceImpl: IEngineTask#getAppContext() returns
  a raw Map (BIRT's own API predates generics), causing unchecked
  put() warnings in setConnectionDetail; scope a @SuppressWarnings
  to that method since the raw type comes from a third-party API.
@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 10749adb-c541-4068-a31e-f87867a436fb

📥 Commits

Reviewing files that changed from the base of the PR and between 2712126 and 05d2080.

📒 Files selected for processing (2)
  • src/main/java/org/apache/fineract/infrastructure/report/service/BirtReadOnlyConnectionFactory.java
  • src/main/java/org/apache/fineract/infrastructure/report/service/BirtReportingProcessServiceImpl.java

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The report services replace deprecated case-insensitive string checks with Strings.CI.contains. The BIRT connection setup suppresses an unchecked warning caused by the raw application-context map. Runtime behavior remains unchanged.

Changes

Maintenance updates

Layer / File(s) Summary
API and warning cleanup
src/main/java/org/apache/fineract/infrastructure/report/service/BirtReadOnlyConnectionFactory.java, src/main/java/org/apache/fineract/infrastructure/report/service/BirtReportingProcessServiceImpl.java
MySQL and MariaDB JDBC URL checks use Strings.CI.contains. setConnectionDetail documents and suppresses the unchecked warning from BIRT's raw Map API.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Suggested labels: ⏱️ <10 Min Review

Suggested reviewers: iohacker

Merge Risk: ⚪ Minimal · up to 05d20

The warning cleanup preserves the described behavior, and no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: updating deprecated methods in the reporting plugin. It is concise and relevant to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@parth-sharma-10
parth-sharma-10 deleted the fix/reporting-plugin-deprecated-methods branch September 17, 2026 08:52
@parth-sharma-10

Copy link
Copy Markdown
Contributor Author

Superseded by #549 — same change, rebranched as MX-414-fix-deprecated-methods so the branch, commit and PR title carry the Jira key (MX-414).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant