Add support for extended values#7774
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
...a.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 |
|
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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a Maven dependency and bumped Carbon Kernel version; introduced a new constant Changes
🚥 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 |
There was a problem hiding this comment.
Pull request overview
Adds support for an extendedValued claim property by surfacing it through the claim-metadata-to-user-core mapping, and wires the module dependencies needed for user API types.
Changes:
- Introduce a new claim property key (
extendedValued) inClaimConstants. - Map the
extendedValuedclaim property into the user-coreClaiminClaimMetadataUtils#convertLocalClaimToClaimMapping. - Add an explicit Maven dependency on
org.wso2.carbon.user.api.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java | Reads extendedValued from local claim properties and sets it on the user-core Claim. |
| components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.java | Adds a constant for the new extendedValued property key. |
| components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/pom.xml | Adds explicit dependency on org.wso2.carbon.user.api. |
...a.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java
Outdated
Show resolved
Hide resolved
| if (claimProperties.containsKey(ClaimConstants.EXTENDED_VALUE_PROPERTY)) { | ||
| claim.setExtendedValued(Boolean.parseBoolean(claimProperties.get(ClaimConstants.EXTENDED_VALUE_PROPERTY))); | ||
| } |
There was a problem hiding this comment.
This new behavior (mapping the extendedValued claim property into the user-core Claim) isn’t covered by the existing ClaimMetadataUtilsTest#testConvertLocalClaimToClaimMapping assertions. Please add/extend a test case that sets ClaimConstants.EXTENDED_VALUE_PROPERTY in claimProperties and verifies claimMapping.getClaim().isExtendedValued() matches.
| public static final String SUB_ATTRIBUTES_PROPERTY = "subAttributes"; | ||
| public static final String SUB_ATTRIBUTE_PREFIX = "subAttribute."; | ||
| public static final String CANONICAL_VALUE_PREFIX = "canonicalValue."; | ||
| public static final String EXTENDED_VALUE_PROPERTY = "extendedValued"; |
There was a problem hiding this comment.
The Java license header in this file still ends at 2025, but this change is being made in 2026. Please update the header year range to end in 2026 (and align to the repo’s standard WSO2 Apache 2.0 header text).
...adata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.java
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate copyright year to 2026.
The copyright header reads
2016-2025but the current year is 2026.📄 Proposed fix
- * Copyright (c) 2016-2025, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2016-2026, WSO2 LLC. (http://www.wso2.com).As per coding guidelines, the license header must have a copyright year as the current year or a range ending in the current year.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java` at line 2, Update the copyright header in ClaimMetadataUtils.java from "2016-2025" to end in the current year "2016-2026"; open the top-of-file license comment in ClaimMetadataUtils.java and replace the year range so the header ends with 2026 to comply with the project's licensing header convention.components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate copyright year to 2026.
The copyright header reads
2016-2025; it should end with the current year, 2026.📄 Proposed fix
- * Copyright (c) 2016-2025, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2016-2026, WSO2 LLC. (http://www.wso2.com).As per coding guidelines, the license header must have a copyright year as the current year or a range ending in the current year.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.java` at line 2, Update the license header year range in the file containing the ClaimConstants class so the ending year is 2026 (e.g., change "2016-2025" to "2016-2026"); locate the header at the top of the ClaimConstants.java file (class ClaimConstants) and modify the copyright line accordingly to comply with the coding guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java`:
- Around line 292-294: The code calls a non-existent method
claim.setExtendedValued(...) (with symbols claimProperties and
ClaimConstants.EXTENDED_VALUE_PROPERTY) which will fail; find the correct API on
org.wso2.carbon.user.core.claim.Claim (open that class) and either replace the
call with the correct setter name (e.g., setExtendedValue or the actual method
provided) or, if Claim has no such field, persist the flag into the claim's
properties map (e.g.,
claim.getProperties().put(ClaimConstants.EXTENDED_VALUE_PROPERTY,
String.valueOf(...))). Update the code in ClaimMetadataUtils to use the resolved
approach so no call to setExtendedValued remains.
---
Outside diff comments:
In
`@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.java`:
- Line 2: Update the license header year range in the file containing the
ClaimConstants class so the ending year is 2026 (e.g., change "2016-2025" to
"2016-2026"); locate the header at the top of the ClaimConstants.java file
(class ClaimConstants) and modify the copyright line accordingly to comply with
the coding guidelines.
In
`@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java`:
- Line 2: Update the copyright header in ClaimMetadataUtils.java from
"2016-2025" to end in the current year "2016-2026"; open the top-of-file license
comment in ClaimMetadataUtils.java and replace the year range so the header ends
with 2026 to comply with the project's licensing header convention.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/pom.xmlcomponents/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.javacomponents/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java
...a.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7774 +/- ##
============================================
+ Coverage 51.03% 51.28% +0.25%
- Complexity 19650 20681 +1031
============================================
Files 2155 2167 +12
Lines 125745 134272 +8527
Branches 25665 27890 +2225
============================================
+ Hits 64176 68866 +4690
- Misses 53403 56754 +3351
- Partials 8166 8652 +486
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:
|
bhagyasakalanka
force-pushed
the
support-large-attributes
branch
from
91070b5 to
826b055
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java (1)
2-2:⚠️ Potential issue | 🟡 MinorUpdate the Java license header year range to include 2026.
Line 2 still ends at
2025; this file is modified in 2026.As per coding guidelines,
**/*.java: Ensure that all Java files contain the appropriate license header at the top with copyright year as the current year or a range ending in the current year (e.g., 2018-2026).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java` at line 2, Update the Java file header in ClaimMetadataUtils.java to extend the copyright year range to include 2026 (e.g., change "2016-2025" to "2016-2026") so the license header is current; locate the header at the top of the file above the class declaration for ClaimMetadataUtils and modify only the year range text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.java`:
- Line 2: Update the Java file header in ClaimMetadataUtils.java to extend the
copyright year range to include 2026 (e.g., change "2016-2025" to "2016-2026")
so the license header is current; locate the header at the top of the file above
the class declaration for ClaimMetadataUtils and modify only the year range
text.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.javacomponents/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtils.javapom.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/main/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimConstants.java
|
PR builder started |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/test/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtilsTest.java`:
- Around line 434-445: The test initializes claimPropertiesMap4 but mistakenly
calls put(...) on claimPropertiesMap3, corrupting other fixtures and leaving
localClaim6 with empty properties; update the map population lines so that all
property put calls use claimPropertiesMap4 (the map passed into the LocalClaim
constructor for localClaim6) to ensure localClaim6 gets its own properties and
localClaim5 remains unchanged.
- Around line 550-553: The assertion for ClaimConstants.EXTENDED_VALUED_PROPERTY
is never reached because claimProperties refers to localClaim2 which doesn't
contain that key; update the test so it asserts against the correct properties
source: either switch claimProperties to the map from the claim that actually
contains EXTENDED_VALUED_PROPERTY (e.g., localClaim1's properties) or add
EXTENDED_VALUED_PROPERTY into localClaim2 before the check, and then verify that
claimMapping.getClaim().isExtendedValued() equals
Boolean.parseBoolean(claimProperties.get(ClaimConstants.EXTENDED_VALUED_PROPERTY)).
- Line 18: Remove the unused stray import com.beust.ah.A from
ClaimMetadataUtilsTest and fix the test-data bug where claimPropertiesMap4 is
created but values are mistakenly added to claimPropertiesMap3; change those
assignments to add properties to claimPropertiesMap4 so that localClaim6 (which
is initialized with claimPropertiesMap4) gets the intended entries (verify
references to claimPropertiesMap4, claimPropertiesMap3, and localClaim6 in the
test and ensure no other unused imports remain).
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/claim-mgt/org.wso2.carbon.identity.claim.metadata.mgt/src/test/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtilsTest.java
...t/src/test/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtilsTest.java
Outdated
Show resolved
Hide resolved
...t/src/test/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtilsTest.java
Show resolved
Hide resolved
...t/src/test/java/org/wso2/carbon/identity/claim/metadata/mgt/util/ClaimMetadataUtilsTest.java
Outdated
Show resolved
Hide resolved
bhagyasakalanka
force-pushed
the
support-large-attributes
branch
2 times, most recently
from
12cb2c2 to
1aa2d95
Compare
bhagyasakalanka
force-pushed
the
support-large-attributes
branch
from
1aa2d95 to
88b24bb
Compare
|
1 similar comment
|
|
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/22612889295
Proposed changes in this pull request
note $subject