Revert unused util class and add configuration to configure push device registration context validity period#7893
Conversation
…tries" This reverts commit 76f3baa.
📝 WalkthroughWalkthroughRemoved unused cache expiration utility class and associated tests. Added push authenticator device registration validity period configuration (180 seconds) to server feature defaults, applying changes across template and resource files. Changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable the changed files summary in the walkthrough.Disable the |
|
PR builder started |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2 (1)
1503-1505: Consider documenting theValidityPeriodunit inline.A short comment (for example, “in seconds”) would make this safer to operate and reduce misconfiguration.
✏️ Suggested tweak
+ <!-- Validity period for push device registration context (in seconds). --> <PushAuthenticator> <DeviceRegistrationContext> <ValidityPeriod>{{push_authenticator.device_registration_context.validity_period}}</ValidityPeriod> </DeviceRegistrationContext> </PushAuthenticator>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2` around lines 1503 - 1505, The template lacks a unit for DeviceRegistrationContext.ValidityPeriod which can cause misconfiguration; update the identity.xml.j2 PushAuthenticator/DeviceRegistrationContext section to include an inline comment specifying the expected unit (e.g., "in seconds") next to <ValidityPeriod> and, if applicable, add the same unit note to the Jinja variable reference push_authenticator.device_registration_context.validity_period or its configuration documentation so operators know the unit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2`:
- Around line 1503-1505: The template lacks a unit for
DeviceRegistrationContext.ValidityPeriod which can cause misconfiguration;
update the identity.xml.j2 PushAuthenticator/DeviceRegistrationContext section
to include an inline comment specifying the expected unit (e.g., "in seconds")
next to <ValidityPeriod> and, if applicable, add the same unit note to the Jinja
variable reference
push_authenticator.device_registration_context.validity_period or its
configuration documentation so operators know the unit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2b8024ca-da6c-4bdf-a937-dbf0008d93e0
📒 Files selected for processing (6)
components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityCacheUtil.javacomponents/identity-core/org.wso2.carbon.identity.core/src/test/java/org/wso2/carbon/identity/core/util/IdentityCacheUtilTest.javacomponents/identity-core/org.wso2.carbon.identity.core/src/test/resources/testng.xmlfeatures/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xmlfeatures/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.json
💤 Files with no reviewable changes (3)
- components/identity-core/org.wso2.carbon.identity.core/src/test/resources/testng.xml
- components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityCacheUtil.java
- components/identity-core/org.wso2.carbon.identity.core/src/test/java/org/wso2/carbon/identity/core/util/IdentityCacheUtilTest.java
There was a problem hiding this comment.
Pull request overview
This PR reverts the cache-entry expiry utility introduced in #7646 (and its associated unit test wiring) and adds a new configurable setting for the Push Authenticator device registration context validity period, exposing it via both the default JSON config and the generated/static identity.xml configurations.
Changes:
- Added
push_authenticator.device_registration_context.validity_periodto the identity-core server feature default configuration JSON. - Exposed the same setting in both
identity.xmlandidentity.xml.j2under a new<PushAuthenticator>section. - Removed
IdentityCacheUtilandIdentityCacheUtilTest, and de-registered the test fromtestng.xml.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.json | Adds the new Push Authenticator validity period config key with a default value. |
| features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2 | Emits the new config as <PushAuthenticator><DeviceRegistrationContext><ValidityPeriod>.... |
| features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml | Adds the corresponding static default XML configuration block. |
| components/identity-core/org.wso2.carbon.identity.core/src/test/resources/testng.xml | Removes the deleted IdentityCacheUtilTest from the TestNG suite. |
| components/identity-core/org.wso2.carbon.identity.core/src/test/java/org/wso2/carbon/identity/core/util/IdentityCacheUtilTest.java | Deleted as part of reverting the unused util. |
| components/identity-core/org.wso2.carbon.identity.core/src/main/java/org/wso2/carbon/identity/core/util/IdentityCacheUtil.java | Deleted as part of reverting the unused util. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7893 +/- ##
============================================
+ Coverage 52.28% 52.61% +0.33%
+ Complexity 20929 20085 -844
============================================
Files 2142 2140 -2
Lines 131158 124555 -6603
Branches 25865 25389 -476
============================================
- Hits 68572 65538 -3034
+ Misses 54088 50962 -3126
+ Partials 8498 8055 -443
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:
|
|
PR builder completed |
|
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/23311295521
Purpose
Related PRs
Related issue