Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughUpgrades CI toolchain to Java 21 and newer GitHub Actions, migrates FindBugs-related configuration toward SpotBugs, adjusts Maven plugin/dependency versions (including Mockito and Jacoco), removes module-level compiler plugin declarations, adds a SpotBugs exclude file, and changes test static-field reset logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #453 +/- ##
=========================================
Coverage 31.78% 31.78%
+ Complexity 1116 1114 -2
=========================================
Files 137 137
Lines 13551 13553 +2
Branches 2599 2599
=========================================
+ Hits 4307 4308 +1
- Misses 8712 8713 +1
Partials 532 532
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java (1)
113-113:⚠️ Potential issue | 🟡 MinorAssertion message says "2 entries" but the expected count is 3.
The message string is inconsistent with the actual assertion value, which will be confusing on test failure.
🔧 Proposed fix
- assertEquals(extensionConfig.size(), 3, "The extensionConfig map should contain 2 entries."); + assertEquals(extensionConfig.size(), 3, "The extensionConfig map should contain 3 entries.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java` at line 113, Update the failing assertion message in SCIMSystemSchemaExtensionBuilderTest so it matches the expected count: replace the incorrect string "The extensionConfig map should contain 2 entries." used in the assertEquals call that compares extensionConfig.size() to 3 with a message that says "The extensionConfig map should contain 3 entries." (or use a dynamic message referencing extensionConfig.size()) so the assertion message and expected value are consistent.modules/charon-core/pom.xml (1)
126-195:⚠️ Potential issue | 🟠 MajorDuplicate
jacoco-maven-plugindeclarations in<plugins>.There are two
jacoco-maven-pluginplugin blocks (lines 126–176 and 177–195). Maven's last-declaration-wins behavior means the first block — which includesprepare-agent-integration,report-integration, andcheckgoals — is effectively dead configuration. If those goals are needed, this is a silent regression; if not, the first block should be removed to avoid confusion.♻️ Suggested fix — keep only one declaration
Remove lines 126–176 (the first declaration) if the integration/check goals are not needed, or merge the two blocks into one if they are.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/charon-core/pom.xml` around lines 126 - 195, The POM contains duplicate jacoco-maven-plugin blocks which causes the first declaration (with executions id default-prepare-agent, default-prepare-agent-integration, default-report, default-report-integration, default-check) to be ignored; either remove the redundant first plugin block or merge its needed executions (prepare-agent-integration, report-integration, check) into the remaining jacoco-maven-plugin definition so all required goals are declared once under a single plugin entry.pom.xml (2)
281-337:⚠️ Potential issue | 🟠 MajorTriplicate
jacoco-maven-pluginin<pluginManagement>.Three identical
jacoco-maven-pluginblocks appear inpluginManagement(lines 281–299, 300–318, 319–337). Only one is needed — the last one wins, making the first two dead configuration. This adds significant noise and maintenance burden.♻️ Proposed fix — keep one declaration
Remove the first two blocks (lines 281–318) and keep only one:
- <plugin> - <groupId>org.jacoco</groupId> - <artifactId>jacoco-maven-plugin</artifactId> - <version>${jacoco.version}</version> - <executions> - <execution> - <goals> - <goal>prepare-agent</goal> - </goals> - </execution> - <execution> - <id>report</id> - <phase>test</phase> - <goals> - <goal>report</goal> - </goals> - </execution> - </executions> - </plugin> - <plugin> - <groupId>org.jacoco</groupId> - <artifactId>jacoco-maven-plugin</artifactId> - <version>${jacoco.version}</version> - <executions> - <execution> - <goals> - <goal>prepare-agent</goal> - </goals> - </execution> - <execution> - <id>report</id> - <phase>test</phase> - <goals> - <goal>report</goal> - </goals> - </execution> - </executions> - </plugin> <plugin> <groupId>org.jacoco</groupId> <artifactId>jacoco-maven-plugin</artifactId>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 281 - 337, The pom contains three identical jacoco-maven-plugin declarations under pluginManagement; remove the duplicate blocks and keep a single jacoco-maven-plugin entry (the one with artifactId jacoco-maven-plugin and the executions prepare-agent + report) so only one pluginManagement declaration for jacoco remains, eliminating the redundant copies.
135-149:⚠️ Potential issue | 🟠 MajorTriplicate
jacoco-maven-plugindependency in<dependencyManagement>.The
jacoco-maven-plugindependency is declared three times (lines 135–139, 140–144, 145–149) with the same GAV. Only one declaration is needed. Additionally,jacoco-maven-pluginis a Maven plugin — declaring it as a<dependency>(rather than only as a<plugin>) is unusual and typically unnecessary unless it's used for programmatic access.♻️ Proposed fix — deduplicate
<dependency> <groupId>org.jacoco</groupId> <artifactId>jacoco-maven-plugin</artifactId> <version>${jacoco.version}</version> </dependency> - <dependency> - <groupId>org.jacoco</groupId> - <artifactId>jacoco-maven-plugin</artifactId> - <version>${jacoco.version}</version> - </dependency> - <dependency> - <groupId>org.jacoco</groupId> - <artifactId>jacoco-maven-plugin</artifactId> - <version>${jacoco.version}</version> - </dependency>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 135 - 149, The pom currently declares the same dependency three times under dependencyManagement for org.jacoco:jacoco-maven-plugin; remove the duplicate entries so there is only a single declaration (or remove the dependency entirely if you intend to manage it only as a Maven plugin). Locate the repeated <dependency> blocks for artifactId jacoco-maven-plugin and either keep one or delete them all and instead declare jacoco-maven-plugin under <build><plugins> as a <plugin> (artifactId jacoco-maven-plugin) if you need plugin behavior; ensure no duplicate entries remain in dependencyManagement..github/workflows/pr-builder.yml (1)
39-55:⚠️ Potential issue | 🟠 MajorDuplicate "Generate coverage report" and "Upload coverage reports" steps.
Lines 41–46 and 47–54 duplicate the coverage report generation and Codecov upload. The first upload (lines 43–46) lacks
filesandflags, so it will scan and upload all reports found, while the second (lines 50–54) targets a specific file with theunitflag. Runningmvn testtwice is also wasteful since tests already ran inmvn clean installon Line 40.Remove the duplicate steps and keep only the targeted upload:
🐛 Proposed fix
- name: Build with Maven run: mvn clean install -U -B - name: Generate coverage report run: mvn test jacoco:report - - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v4 - with: - token: ${{ secrets.CODECOV_TOKEN }} - - name: Generate coverage report - run: mvn test jacoco:report - - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 with: token: ${{ secrets.CODECOV_TOKEN }} - files : target/site/jacoco/jacoco.xml + files: target/site/jacoco/jacoco.xml flags: unit🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-builder.yml around lines 39 - 55, Remove the duplicated Jacoco/report and Codecov steps: delete the first "Generate coverage report" run (mvn test jacoco:report) and the first "Upload coverage reports to Codecov" action that lacks files/flags, leaving a single jacoco report generation and a single codecov/codecov-action@v4 step; keep the upload step that includes files: target/site/jacoco/jacoco.xml and flags: unit so tests from the earlier mvn clean install are not re-run and only the targeted report is uploaded.
🧹 Nitpick comments (2)
.github/workflows/pr-builder.yml (1)
29-29: Inconsistentactions/cacheversion — v3 here vs v4 incoverage-generator.yml.Consider upgrading to
actions/cache@v4for consistency with the other workflow file.♻️ Proposed fix
- uses: actions/cache@v3 + uses: actions/cache@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-builder.yml at line 29, The workflow uses the older cache action version "uses: actions/cache@v3" which is inconsistent with coverage-generator.yml; update that uses entry to "actions/cache@v4" so both workflows match—locate the line containing "uses: actions/cache@v3" in the pr-builder workflow and change the version to `@v4`.pom.xml (1)
461-462: Upgrademaven-surefire-pluginto 3.x for Java 21 compatibility.Version 2.22.0 (2018) predates Java Platform Module System maturity and has known incompatibilities with Java 21, including module patching failures and outdated JUnit Platform integration (junit-platform-launcher 1.3.1, Byte Buddy 1.8.x). Since the project targets Java 21 and this property is used in
modules/charon-core/pom.xml, upgrading to 3.5.4 or later is recommended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 461 - 462, The pom property maven.surefire.plugin.version is set to 2.22.0 which is incompatible with Java 21; update this property to a 3.x release (recommend 3.5.4 or later) so the modules (e.g., modules/charon-core/pom.xml) that reference ${maven.surefire.plugin.version} will pick up the Java‑21-compatible surefire plugin; ensure any pluginManagement or direct plugin declarations that hardcode the older version are similarly updated or rely on the updated property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/coverage-generator.yml:
- Around line 17-21: The workflow step named "Set up Adopt JDK 21" uses
actions/setup-java@v4 with distribution "adopt" which doesn't provide JDK 21;
change the distribution value to "temurin" so the setup-java action can install
JDK 21 (update the distribution string from "adopt" to "temurin" in the step
that uses actions/setup-java@v4).
In @.github/workflows/pr-builder.yml:
- Around line 22-26: The workflow step named "Set up Adopt JDK 21" uses
actions/setup-java@v4 with distribution: "adopt", but JDK 21 isn't available
under that distribution; update the step (the block with name "Set up Adopt JDK
21" and the uses: actions/setup-java@v4 configuration) to set with:
java-version: 21 and distribution: "temurin" (and optionally rename the step to
"Set up Temurin JDK 21" for clarity).
In `@modules/charon-core/spotbugs-exclude.xml`:
- Around line 1-5: The XML root element is incorrect: change the top-level
<spotbugs> element to <FindBugsFilter> so the SpotBugs filter file is parsable
by spotbugs-maven-plugin; keep the existing <Match> and <Package
name="org.wso2.charon3.core.aParser" /> content intact under the new
<FindBugsFilter> root.
In
`@modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java`:
- Around line 173-179: The helper clearStaticMapField in
SCIMSystemSchemaExtensionBuilderTest directly casts field.get(null) to Map and
calls clear(), which can NPE if the static field is uninitialized; update
clearStaticMapField to retrieve the value via Field.get(null), check for null
before casting/clearing, and if you need the map cleared ensure initialization
(e.g., set a new HashMap into the static field using field.set(null, new
HashMap<>()) or simply return when null). Reference: method clearStaticMapField
and class SCIMSystemSchemaExtensionBuilder.
- Around line 181-191: The assertion message in
SCIMSystemSchemaExtensionBuilderTest is wrong: update the message text used in
the failing assertion (the one that reads "The extensionConfig map should
contain 2 entries.") to reflect the actual expected size 3 so it matches the
assert checking size() == 3; locate the assertion in the test (around line 113)
and change the literal message to "The extensionConfig map should contain 3
entries." Also leave the setStaticField helper (which uses sun.misc.Unsafe)
as-is for test-only manipulation of static/final fields or, if you must avoid
internal APIs long-term, add a package-private reset method to
SCIMSystemSchemaExtensionBuilder instead.
In `@pom.xml`:
- Around line 469-470: The pom.xml defines the Maven property jacoco.version
twice (both set to 0.8.12); remove the duplicate declaration so there is only a
single <jacoco.version> property to avoid future divergence — locate both
occurrences of the property name jacoco.version in the properties block and
delete the redundant one (keep the original or the one in the canonical
properties section), ensuring no other property names are changed and references
to jacoco.version remain intact.
- Around line 353-365: The pom currently disables findbugs by binding
findbugs-maven-plugin executions to phase:none but only declares
spotbugs-maven-plugin inside pluginManagement, so SpotBugs never runs; to fix,
add the spotbugs-maven-plugin entry from pluginManagement into the active
<build><plugins> section (mirror the configuration and executions you want to
run) so spotbugs-maven-plugin becomes an active plugin for the project, leaving
findbugs-maven-plugin executions unchanged or removed as desired.
---
Outside diff comments:
In @.github/workflows/pr-builder.yml:
- Around line 39-55: Remove the duplicated Jacoco/report and Codecov steps:
delete the first "Generate coverage report" run (mvn test jacoco:report) and the
first "Upload coverage reports to Codecov" action that lacks files/flags,
leaving a single jacoco report generation and a single codecov/codecov-action@v4
step; keep the upload step that includes files: target/site/jacoco/jacoco.xml
and flags: unit so tests from the earlier mvn clean install are not re-run and
only the targeted report is uploaded.
In `@modules/charon-core/pom.xml`:
- Around line 126-195: The POM contains duplicate jacoco-maven-plugin blocks
which causes the first declaration (with executions id default-prepare-agent,
default-prepare-agent-integration, default-report, default-report-integration,
default-check) to be ignored; either remove the redundant first plugin block or
merge its needed executions (prepare-agent-integration, report-integration,
check) into the remaining jacoco-maven-plugin definition so all required goals
are declared once under a single plugin entry.
In
`@modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java`:
- Line 113: Update the failing assertion message in
SCIMSystemSchemaExtensionBuilderTest so it matches the expected count: replace
the incorrect string "The extensionConfig map should contain 2 entries." used in
the assertEquals call that compares extensionConfig.size() to 3 with a message
that says "The extensionConfig map should contain 3 entries." (or use a dynamic
message referencing extensionConfig.size()) so the assertion message and
expected value are consistent.
In `@pom.xml`:
- Around line 281-337: The pom contains three identical jacoco-maven-plugin
declarations under pluginManagement; remove the duplicate blocks and keep a
single jacoco-maven-plugin entry (the one with artifactId jacoco-maven-plugin
and the executions prepare-agent + report) so only one pluginManagement
declaration for jacoco remains, eliminating the redundant copies.
- Around line 135-149: The pom currently declares the same dependency three
times under dependencyManagement for org.jacoco:jacoco-maven-plugin; remove the
duplicate entries so there is only a single declaration (or remove the
dependency entirely if you intend to manage it only as a Maven plugin). Locate
the repeated <dependency> blocks for artifactId jacoco-maven-plugin and either
keep one or delete them all and instead declare jacoco-maven-plugin under
<build><plugins> as a <plugin> (artifactId jacoco-maven-plugin) if you need
plugin behavior; ensure no duplicate entries remain in dependencyManagement.
---
Nitpick comments:
In @.github/workflows/pr-builder.yml:
- Line 29: The workflow uses the older cache action version "uses:
actions/cache@v3" which is inconsistent with coverage-generator.yml; update that
uses entry to "actions/cache@v4" so both workflows match—locate the line
containing "uses: actions/cache@v3" in the pr-builder workflow and change the
version to `@v4`.
In `@pom.xml`:
- Around line 461-462: The pom property maven.surefire.plugin.version is set to
2.22.0 which is incompatible with Java 21; update this property to a 3.x release
(recommend 3.5.4 or later) so the modules (e.g., modules/charon-core/pom.xml)
that reference ${maven.surefire.plugin.version} will pick up the
Java‑21-compatible surefire plugin; ensure any pluginManagement or direct plugin
declarations that hardcode the older version are similarly updated or rely on
the updated property.
| - name: Set up Adopt JDK 21 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: 11 | ||
| java-version: 21 | ||
| distribution: "adopt" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
actions/setup-java adopt distribution JDK 21 support
💡 Result:
actions/setup-java still accepts distribution: adopt / adopt-hotspot (and adopt-openj9), but AdoptOpenJDK is no longer updated (it moved to Eclipse Adoptium), so you should not rely on it for JDK 21—use temurin for HotSpot JDK 21 (or semeru for OpenJ9) instead. [1], [2]
Example:
- uses: actions/setup-java@v5
with:
distribution: temurin
java-version: '21'
``` [1]
---
🌐 Web query:
`AdoptOpenJDK JDK 21 available latest version`
💡 Result:
AdoptOpenJDK builds are now published under **Eclipse Temurin (Adoptium)**.
As of **February 17, 2026**, the latest available **JDK 21 (LTS)** release is:
- **Eclipse Temurin 21.0.10+7** (announced **February 5, 2026**) [1]
(Also reflected by downstream package metadata such as Homebrew’s `temurin@21` cask showing **21.0.10,7**.) [2]
**Download:** use the Adoptium/Temurin downloads (or their API), not the old AdoptOpenJDK site. [1]
**Sources:** [1] ([adoptium.net](https://adoptium.net/news/2026/02/eclipse-temurin-8u482-11030-17018-21010-2502-available/?utm_source=openai)), [2] ([formulae.brew.sh](https://formulae.brew.sh/cask/temurin%4021?utm_source=openai))
Citations:
- 1: https://adoptium.net/news/2026/02/eclipse-temurin-8u482-11030-17018-21010-2502-available/?utm_source=openai
- 2: https://formulae.brew.sh/cask/temurin%4021?utm_source=openai
---
🌐 Web query:
`Eclipse Temurin JDK 21 actions/setup-java`
💡 Result:
To set up **Eclipse Temurin JDK 21** in GitHub Actions with **`actions/setup-java`**, use `distribution: temurin` and `java-version: 21` (recommended: `@v5`). ([github.com](https://github.com/actions/setup-java?utm_source=openai))
```yaml
steps:
- uses: actions/checkout@v5
- uses: actions/setup-java@v5
with:
distribution: temurin
java-version: '21'
- run: java -versionOptional (dependency caching + always fetch latest matching JDK build): (github.com)
- uses: actions/setup-java@v5
with:
distribution: temurin
java-version: '21'
cache: gradle # or: maven / sbt
check-latest: trueCitations:
- 1: https://github.com/actions/setup-java?utm_source=openai
- 2: https://github.com/actions/setup-java?utm_source=openai
Change adopt distribution to temurin—JDK 21 is not available from AdoptOpenJDK.
AdoptOpenJDK is no longer maintained and has not released builds since JDK 16. JDK 21 is only available from Eclipse Temurin (the Adoptium successor). The workflow will fail trying to locate JDK 21 with the adopt distribution.
🐛 Proposed fix
- - name: Set up Adopt JDK 21
+ - name: Set up Temurin JDK 21
uses: actions/setup-java@v4
with:
java-version: 21
- distribution: "adopt"
+ distribution: "temurin"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Set up Adopt JDK 21 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: 11 | |
| java-version: 21 | |
| distribution: "adopt" | |
| - name: Set up Temurin JDK 21 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: 21 | |
| distribution: "temurin" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/coverage-generator.yml around lines 17 - 21, The workflow
step named "Set up Adopt JDK 21" uses actions/setup-java@v4 with distribution
"adopt" which doesn't provide JDK 21; change the distribution value to "temurin"
so the setup-java action can install JDK 21 (update the distribution string from
"adopt" to "temurin" in the step that uses actions/setup-java@v4).
| - name: Set up Adopt JDK 21 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: "8" | ||
| java-version: 21 | ||
| distribution: "adopt" |
There was a problem hiding this comment.
Same adopt distribution issue — JDK 21 is not available under adopt.
Same as in coverage-generator.yml: use temurin instead of adopt for JDK 21.
🐛 Proposed fix
- - name: Set up Adopt JDK 21
- uses: actions/setup-java@v4
+ - name: Set up Temurin JDK 21
+ uses: actions/setup-java@v4
with:
java-version: 21
- distribution: "adopt"
+ distribution: "temurin"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Set up Adopt JDK 21 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: "8" | |
| java-version: 21 | |
| distribution: "adopt" | |
| - name: Set up Temurin JDK 21 | |
| uses: actions/setup-java@v4 | |
| with: | |
| java-version: 21 | |
| distribution: "temurin" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-builder.yml around lines 22 - 26, The workflow step
named "Set up Adopt JDK 21" uses actions/setup-java@v4 with distribution:
"adopt", but JDK 21 isn't available under that distribution; update the step
(the block with name "Set up Adopt JDK 21" and the uses: actions/setup-java@v4
configuration) to set with: java-version: 21 and distribution: "temurin" (and
optionally rename the step to "Set up Temurin JDK 21" for clarity).
| @SuppressWarnings("unchecked") | ||
| private void clearStaticMapField(String fieldName) throws NoSuchFieldException, IllegalAccessException { | ||
|
|
||
| Field field = SCIMSystemSchemaExtensionBuilder.class.getDeclaredField(fieldName); | ||
| field.setAccessible(true); | ||
| ((Map<?, ?>) field.get(null)).clear(); | ||
| } |
There was a problem hiding this comment.
Potential NullPointerException if the static map field is null.
field.get(null) is cast directly to Map and .clear() is called without a null guard. If the field hasn't been initialized yet (e.g., due to a class-loading order change), this will throw an NPE.
🛡️ Proposed fix
private void clearStaticMapField(String fieldName) throws NoSuchFieldException, IllegalAccessException {
Field field = SCIMSystemSchemaExtensionBuilder.class.getDeclaredField(fieldName);
field.setAccessible(true);
- ((Map<?, ?>) field.get(null)).clear();
+ Map<?, ?> map = (Map<?, ?>) field.get(null);
+ if (map != null) {
+ map.clear();
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java`
around lines 173 - 179, The helper clearStaticMapField in
SCIMSystemSchemaExtensionBuilderTest directly casts field.get(null) to Map and
calls clear(), which can NPE if the static field is uninitialized; update
clearStaticMapField to retrieve the value via Field.get(null), check for null
before casting/clearing, and if you need the map cleared ensure initialization
(e.g., set a new HashMap into the static field using field.set(null, new
HashMap<>()) or simply return when null). Reference: method clearStaticMapField
and class SCIMSystemSchemaExtensionBuilder.
| private void setStaticField(String fieldName, Object newValue) throws NoSuchFieldException, | ||
| IllegalAccessException { | ||
|
|
||
| field.set(null, newValue); | ||
| Field unsafeField = sun.misc.Unsafe.class.getDeclaredField("theUnsafe"); | ||
| unsafeField.setAccessible(true); | ||
| sun.misc.Unsafe unsafe = (sun.misc.Unsafe) unsafeField.get(null); | ||
|
|
||
| Field field = SCIMSystemSchemaExtensionBuilder.class.getDeclaredField(fieldName); | ||
| long offset = unsafe.staticFieldOffset(field); | ||
| unsafe.putObject(SCIMSystemSchemaExtensionBuilder.class, offset, newValue); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full test file to understand the context and check line 113
cd modules/charon-core && find . -name "SCIMSystemSchemaExtensionBuilderTest.java" -type fRepository: wso2/charon
Length of output: 141
🌐 Web query:
Java 21 sun.misc.Unsafe deprecation removal JEP 471
💡 Result:
In Java 21 (JDK 21), JEP 471 is not present—so there is no new “deprecated for removal” change in sun.misc.Unsafe specifically due to JEP 471 in that release.
What JEP 471 actually does (and when):
-
JEP 471 = “Deprecate the Memory-Access Methods in
sun.misc.Unsafefor Removal” and its release target is JDK 23 (status: Delivered). It deprecates (for removal) the memory-access methods insun.misc.Unsafe(on-heap, off-heap, and “bimodal”), but does not remove them yet. It also introduces the launcher option--sun-misc-unsafe-memory-access={allow|warn|debug|deny}(defaultallowin JDK 23). [1], [2] -
The JEP describes a phased plan: (1) deprecate (JDK 23), (2) runtime warnings (planned “in or before JDK 25”), (3) exceptions by default (JDK 26 or later), then eventual removal in releases following JDK 26. [1]
Related follow-up:
- JEP 498 is the follow-up that adds runtime warnings upon first use, and it was targeted to / delivered in JDK 24. [3]
Sources:
[1] https://openjdk.org/jeps/471
[2] https://mail.openjdk.org/pipermail/core-libs-dev/2024-May/123193.html
[3] https://inside.java/2024/11/28/jep498-target-jdk24/
🌐 Web query:
Java VarHandle set static final field test
💡 Result:
To test that a static final field cannot be mutated via VarHandle, assert that the write access mode is unsupported and that set(...) throws UnsupportedOperationException (because VarHandles treat final fields as read-only). [1][2]
import static org.junit.jupiter.api.Assertions.*;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
import org.junit.jupiter.api.Test;
class VarHandleStaticFinalTest {
static class C {
static final int X = 1;
}
`@Test`
void cannotSetStaticFinalViaVarHandle() throws Throwable {
VarHandle vh = MethodHandles.lookup()
.findStaticVarHandle(C.class, "X", int.class); // ok to obtain [2]
assertFalse(vh.isAccessModeSupported(VarHandle.AccessMode.SET)); // final => no write modes [1][2]
assertThrows(UnsupportedOperationException.class, () -> vh.set(2)); // write throws UOE [1]
assertEquals(1, (int) vh.get()); // reads still work [1]
}
}Notes
- This is by design: if a VarHandle references a
finalfield, write/atomic update modes are unsupported and throwUnsupportedOperationException. [1][2]
Sources: VarHandle spec (final fields are read-only for VarHandle write/update modes) [1]; MethodHandles.Lookup.findStaticVarHandle (final fields => write/update modes unsupported) [2].
🏁 Script executed:
# Read the test file to check line 113 and the full context of the methods
cat -n modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java | sed -n '100,200p'Repository: wso2/charon
Length of output: 5543
Fix assertion message on line 113 — expected size is 3, not 2.
Line 113 states "The extensionConfig map should contain 2 entries." but asserts size() == 3. Update the message to match the expected count.
Regarding sun.misc.Unsafe usage: This approach is pragmatic for test code. While sun.misc.Unsafe is targeted for deprecation in JDK 23 (not JDK 21), it remains functional and is a common pattern for manipulating static final fields in tests when reflection alone is insufficient. If Java 21+ compatibility without internal APIs is a strict requirement, refactor the production code to provide a test-friendly reset mechanism (e.g., a package-private reset method on SCIMSystemSchemaExtensionBuilder). VarHandle is not a viable alternative here because it intentionally prevents writes to final fields by design.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@modules/charon-core/src/test/java/org/wso2/charon3/core/config/SCIMSystemSchemaExtensionBuilderTest.java`
around lines 181 - 191, The assertion message in
SCIMSystemSchemaExtensionBuilderTest is wrong: update the message text used in
the failing assertion (the one that reads "The extensionConfig map should
contain 2 entries.") to reflect the actual expected size 3 so it matches the
assert checking size() == 3; locate the assertion in the test (around line 113)
and change the literal message to "The extensionConfig map should contain 3
entries." Also leave the setStaticField helper (which uses sun.misc.Unsafe)
as-is for test-only manipulation of static/final fields or, if you must avoid
internal APIs long-term, add a package-private reset method to
SCIMSystemSchemaExtensionBuilder instead.
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>findbugs-maven-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>analyze-compile</id> | ||
| <phase>none</phase> | ||
| </execution> | ||
| <execution> | ||
| <id>findbugs</id> | ||
| <phase>none</phase> | ||
| </execution> | ||
| </executions> |
There was a problem hiding this comment.
FindBugs disabled correctly, but SpotBugs is not activated in <plugins>.
The findbugs-maven-plugin executions are correctly disabled by binding them to phase: none. However, the replacement spotbugs-maven-plugin is only declared in <pluginManagement> (lines 243–263), not in the active <plugins> section. This means SpotBugs will not run for any module unless that module explicitly adds the plugin. If the intent is to replace FindBugs project-wide, add SpotBugs to <build><plugins> as well.
♻️ Proposed fix — activate SpotBugs in build/plugins
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<executions>
<execution>
<id>analyze-compile</id>
<phase>none</phase>
</execution>
<execution>
<id>findbugs</id>
<phase>none</phase>
</execution>
</executions>
</plugin>
+ <plugin>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-maven-plugin</artifactId>
+ </plugin>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <plugin> | |
| <groupId>org.codehaus.mojo</groupId> | |
| <artifactId>findbugs-maven-plugin</artifactId> | |
| <executions> | |
| <execution> | |
| <id>analyze-compile</id> | |
| <phase>none</phase> | |
| </execution> | |
| <execution> | |
| <id>findbugs</id> | |
| <phase>none</phase> | |
| </execution> | |
| </executions> | |
| <plugin> | |
| <groupId>org.codehaus.mojo</groupId> | |
| <artifactId>findbugs-maven-plugin</artifactId> | |
| <executions> | |
| <execution> | |
| <id>analyze-compile</id> | |
| <phase>none</phase> | |
| </execution> | |
| <execution> | |
| <id>findbugs</id> | |
| <phase>none</phase> | |
| </execution> | |
| </executions> | |
| </plugin> | |
| <plugin> | |
| <groupId>com.github.spotbugs</groupId> | |
| <artifactId>spotbugs-maven-plugin</artifactId> | |
| </plugin> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pom.xml` around lines 353 - 365, The pom currently disables findbugs by
binding findbugs-maven-plugin executions to phase:none but only declares
spotbugs-maven-plugin inside pluginManagement, so SpotBugs never runs; to fix,
add the spotbugs-maven-plugin entry from pluginManagement into the active
<build><plugins> section (mirror the configuration and executions you want to
run) so spotbugs-maven-plugin becomes an active plugin for the project, leaving
findbugs-maven-plugin executions unchanged or removed as desired.
| <jacoco.version>0.8.12</jacoco.version> | ||
| <mockito-core.version>5.3.1</mockito-core.version> |
There was a problem hiding this comment.
Duplicate jacoco.version property — defined on both Line 469 and Line 477.
The jacoco.version property is declared twice. While both currently have the same value (0.8.12), this is error-prone — a future update might change only one, causing subtle version mismatches.
♻️ Proposed fix — remove the duplicate
<jacoco.version>0.8.12</jacoco.version>
<mockito-core.version>5.3.1</mockito-core.version>
<sonar.projectKey>wso2_charon</sonar.projectKey>
<sonar.moduleKey>${artifactId}</sonar.moduleKey>
<sonar.organization>wso2</sonar.organization>
<sonar.host.url>https://sonarcloud.io</sonar.host.url>
-
- <jacoco.version>0.8.12</jacoco.version>
</properties>Also applies to: 477-477
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pom.xml` around lines 469 - 470, The pom.xml defines the Maven property
jacoco.version twice (both set to 0.8.12); remove the duplicate declaration so
there is only a single <jacoco.version> property to avoid future divergence —
locate both occurrences of the property name jacoco.version in the properties
block and delete the redundant one (keep the original or the one in the
canonical properties section), ensuring no other property names are changed and
references to jacoco.version remain intact.
|
PR builder started |
|
PR builder completed |
jenkins-is-staging
left a comment
jenkins-is-staging
left a comment
There was a problem hiding this comment.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/22207618615
2 participants
Purpose
Goals
Approach
User stories
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit