Automatically register new scopes for existing system API resources#7692
Open
sadilchamishka wants to merge 6 commits intowso2:masterfrom
Open
Automatically register new scopes for existing system API resources#7692sadilchamishka wants to merge 6 commits intowso2:masterfrom
sadilchamishka wants to merge 6 commits intowso2:masterfrom
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
Contributor
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 | Y | A debug log during the flow is better to have. |
| #### Log Improvement Suggestion No: 2 | N | Same information already logged |
| #### Log Improvement Suggestion No: 3 | N | As exception is thrown, no need to log an error. |
| #### Log Improvement Suggestion No: 4 | N | Same information already logged |
| #### Log Improvement Suggestion No: 5 | N | Not required to log while iterating the loop |
| #### Log Improvement Suggestion No: 6 | Y | That debug log is better to have. |
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new tenant-scoped API to retrieve all system API resources with their scopes and implements it across manager, cache-backed DAO, DAO implementation, SQL constants, and utility helpers to use the aggregated mapping. Changes
Sequence DiagramsequenceDiagram
participant Client
participant APIResourceManager
participant CacheBackedDAO
participant APIResourceDAO
participant Database
Client->>APIResourceManager: getAllSystemAPIResourcesWithScopes(tenantId)
APIResourceManager->>CacheBackedDAO: getAllSystemAPIResourcesWithScopes(tenantId)
CacheBackedDAO->>APIResourceDAO: getAllSystemAPIResourcesWithScopes(tenantId)
APIResourceDAO->>Database: Execute GET_SYSTEM_API_RESOURCES_WITH_SCOPES (tenantId)
Database-->>APIResourceDAO: Rows (IDENTIFIER, SCOPE_NAME)
APIResourceDAO->>APIResourceDAO: Aggregate scopes per IDENTIFIER into Map<String, List<String>>
APIResourceDAO-->>CacheBackedDAO: Return Map<String, List<String>>
CacheBackedDAO-->>APIResourceManager: Return Map
APIResourceManager-->>Client: Return Map<String, List<String>>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
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 |
sadilchamishka
commented
Jan 23, 2026
| Map<String, List<String>> systemAPIsWithScopes = getAllSystemAPIResourcesWithScopes(); | ||
| for (String apiIdentifier : systemAPIsWithScopes.keySet()) { | ||
| APIResource apiResource = tempConfigs.remove(apiIdentifier); | ||
| if (apiResource.getScopes().size() != systemAPIsWithScopes.get(apiIdentifier).size()) { |
Contributor
Author
There was a problem hiding this comment.
If system scops always added, this simple logic is enough. If system scopes can be deleted, the logic should be improved to match the elements of the list.
sadilchamishka
commented
Jan 23, 2026
| public static List<APIResource> getSystemAPIs(String tenantDomain) throws APIResourceMgtException { | ||
|
|
||
| // Get APIs with SYSTEM type. | ||
| int systemAPICount = APIResourceManagerImpl.getInstance().getAPIResources(null, null, 1, |
Contributor
Author
There was a problem hiding this comment.
Not related to this effort. But looks the query send to get the count is not necessary.
Here I have set Integer.MAX_VALUE or we can set arbitrary large integer where system APIs of the WSO2 Identity server may not exceed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/APIResourceManagementDAO.java (1)
1-2: Update copyright year to include 2026.As per coding guidelines, the copyright year should be the current year or a range ending in the current year. Since this file is being modified in 2026, update the copyright.
Suggested fix
-* Copyright (c) 2023, WSO2 LLC. (http://www.wso2.com). +* Copyright (c) 2023-2026, WSO2 LLC. (http://www.wso2.com).components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/APIResourceManager.java (1)
1-3: Update license header year to include 2026.
The header currently shows 2023; guidelines require the current year or a range ending in 2026.💡 Suggested update
- * Copyright (c) 2023, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2023-2026, WSO2 LLC. (http://www.wso2.com).components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/util/APIResourceManagementUtil.java (1)
1-2: Update the license header year to 2026.The header currently ends at 2025, but the guidelines require the current year (2026).
✅ Proposed fix
- * Copyright (c) 2023-2025, WSO2 LLC. (https://www.wso2.com). + * Copyright (c) 2023-2026, WSO2 LLC. (https://www.wso2.com).components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/impl/CacheBackedAPIResourceMgtDAO.java (1)
1-2: Update the license header year to include 2026.The header still ends at 2023; please update it to a range ending in 2026.
As per coding guidelines, ensure Java files use the current-year license header.🧾 Proposed fix
- * Copyright (c) 2023, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2023-2026, WSO2 LLC. (http://www.wso2.com).components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/APIResourceManagerImpl.java (1)
1-2: Update the license header year to include 2026.The header currently ends at 2025; please update it to a range ending in 2026.
As per coding guidelines, ensure Java files use the current-year license header.🧾 Proposed fix
- * Copyright (c) 2023-2025, WSO2 LLC. (https://www.wso2.com). + * Copyright (c) 2023-2026, WSO2 LLC. (https://www.wso2.com).components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/impl/APIResourceManagementDAOImpl.java (1)
1-2: Update the license header year to include 2026.The header currently ends at 2025; please update it to a range ending in 2026.
As per coding guidelines, ensure Java files use the current-year license header.🧾 Proposed fix
- * Copyright (c) 2023-2025, WSO2 LLC. (https://www.wso2.com). + * Copyright (c) 2023-2026, WSO2 LLC. (https://www.wso2.com).
🤖 Fix all issues with AI agents
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/APIResourceManager.java`:
- Around line 211-217: The getAllSystemAPIResourcesWithScopes method and its SQL
constant GET_SYSTEM_API_RESOURCES_WITH_SCOPES lack tenant scoping; update the
API by adding a tenantDomain (or tenantId) parameter to
getAllSystemAPIResourcesWithScopes, modify the SQL query
GET_SYSTEM_API_RESOURCES_WITH_SCOPES to include a TENANT_ID filter, pass the
tenant parameter when executing the query, and update all callers to supply the
caller's tenant (or, if system APIs are intentionally global, add explicit
Javadoc on getAllSystemAPIResourcesWithScopes and ensure authorization is
enforced at call sites instead of changing the query); locate and change the
method declaration in APIResourceManager and corresponding DAO/impl that
executes GET_SYSTEM_API_RESOURCES_WITH_SCOPES.
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/APIResourceManagerImpl.java`:
- Around line 258-262: Add a concise Javadoc comment to the public method
APIResourceManagerImpl.getAllSystemAPIResourcesWithScopes describing what the
method returns (a Map of system API resource identifiers to their list of
scopes), any exceptions thrown (APIResourceMgtException), and the behavior
(delegates to CACHE_BACKED_DAO.getAllSystemAPIResourcesWithScopes()); place the
Javadoc immediately above the method declaration and follow project doc style
(one-line summary, blank line, param/return/throws as applicable).
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/constant/SQLConstants.java`:
- Around line 245-246: Update the file header year to include 2026 and change
the SQL constant GET_SYSTEM_API_RESOURCES_WITH_SCOPES to (1) fully qualify
columns (use API_RESOURCE.IDENTIFIER instead of IDENTIFIER and SCOPE.NAME
remains qualified) and (2) add tenant filters on both tables by adding
conditions like (API_RESOURCE.TENANT_ID = ? OR API_RESOURCE.TENANT_ID IS NULL)
AND (SCOPE.TENANT_ID = ? OR SCOPE.TENANT_ID IS NULL) to the WHERE clause while
preserving the existing API_RESOURCE.TYPE != 'BUSINESS' check; locate and edit
SQLConstants.GET_SYSTEM_API_RESOURCES_WITH_SCOPES, API_RESOURCE, SCOPE,
TENANT_ID, API_RESOURCE.IDENTIFIER, and SCOPE.NAME to apply these changes.
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/impl/APIResourceManagementDAOImpl.java`:
- Around line 770-793: Add a Javadoc block above the public method
getAllSystemAPIResourcesWithScopes() describing its purpose, return value, and
error behavior: state that it returns a Map<String,List<String>> mapping API
identifiers to their scopes, mention there are no parameters, and document that
it throws APIResourceMgtException (or APIResourceMgtServerException) on DB/SQL
failures; include a brief description of the thrown exception conditions and any
relevant usage notes so the method complies with public API doc guidelines.
- Around line 773-783: The SQL in
SQLConstants.GET_SYSTEM_API_RESOURCES_WITH_SCOPES currently filters only TYPE !=
'BUSINESS' which mismatches the isSystemAPI() semantics; update the SQL constant
value so the WHERE clause excludes all non-system types by using WHERE
API_RESOURCE.TYPE NOT IN ('BUSINESS','SYSTEM','MCP','VC') so the DAO query in
APIResourceManagementDAOImpl (used when populating apiScopesMap) returns the
same set as isSystemAPI().
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/impl/CacheBackedAPIResourceMgtDAO.java`:
- Around line 314-318: The public method getAllSystemAPIResourcesWithScopes in
CacheBackedAPIResourceMgtDAO lacks a Javadoc; add a brief Javadoc above the
method that summarizes its purpose (e.g., returns a map of system API resource
names to their scopes), include an `@return` describing the Map<String,
List<String>> returned, and an `@throws` APIResourceMgtException describing when
it is thrown, and follow the existing Javadoc style used in this class for
consistency.
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/util/APIResourceManagementUtil.java`:
- Around line 185-187: Add a Javadoc comment to the public helper method
getAllSystemAPIResourcesWithScopes in class APIResourceManagementUtil describing
its purpose (returns a map of all system API resource identifiers to their
associated scope lists), document the return value (Map<String, List<String>>
mapping resource -> scopes), declare the thrown APIResourceMgtException, and
include tags such as `@return`, `@throws` APIResourceMgtException and an appropriate
`@since` or author tag to match project conventions; keep the description concise
and place it immediately above the method signature.
- Around line 105-110: Guard against null APIResource from tempConfigs.remove
and compare scopes by name: after calling tempConfigs.remove(apiIdentifier) (in
APIResourceManagementUtil where systemAPIsWithScopes is iterated), check if
apiResource is null and skip or handle accordingly to avoid an NPE, then compare
the scope sets rather than sizes by retrieving apiResource.getScopes() and the
List from getAllSystemAPIResourcesWithScopes().get(apiIdentifier), convert both
to sets of scope names (or extract scope identifiers) and detect differences to
decide when to put into duplicateConfigs (using apiResource.getIdentifier()).
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for detecting existing (non-business) API resources and automatically registering newly introduced scopes for them, rather than only registering system APIs once.
Changes:
- Updated system API bootstrap logic to compare configured scopes vs. persisted scopes and trigger updates when new scopes are detected.
- Introduced a new API/DAO method to retrieve system API resource identifiers with their scopes.
- Added a new SQL constant and DAO implementation to fetch identifier-to-scope mappings from the database.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/util/APIResourceManagementUtil.java | Updates startup registration flow to detect scope changes and apply updates. |
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/impl/CacheBackedAPIResourceMgtDAO.java | Exposes new DAO method via cache-backed DAO. |
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/impl/APIResourceManagementDAOImpl.java | Implements DB retrieval of system API identifiers and scopes. |
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/dao/APIResourceManagementDAO.java | Adds DAO contract for fetching system API identifiers and scopes. |
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/constant/SQLConstants.java | Adds SQL constant for system API identifier-to-scope query. |
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/APIResourceManagerImpl.java | Adds manager implementation method delegating to DAO. |
| components/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt/src/main/java/org/wso2/carbon/identity/api/resource/mgt/APIResourceManager.java | Adds manager interface method for identifier-to-scope retrieval. |
Comment on lines
+105
to
+109
| Map<String, List<String>> systemAPIsWithScopes = getAllSystemAPIResourcesWithScopes(); | ||
| for (String apiIdentifier : systemAPIsWithScopes.keySet()) { | ||
| APIResource apiResource = tempConfigs.remove(apiIdentifier); | ||
| if (apiResource.getScopes().size() != systemAPIsWithScopes.get(apiIdentifier).size()) { | ||
| duplicateConfigs.put(apiResource.getIdentifier(), apiResource); |
There was a problem hiding this comment.
The new “auto-register new scopes for existing APIs” behavior introduced here isn’t covered by tests. Extend existing APIResourceManagementUtilTest to verify that when a new scope is added in config, addSystemAPIs() adds it in DB and does not try to re-register existing APIs (including APIs that currently have zero scopes).
|
sadilchamishka
requested review from
SujanSanjula96,
ThaminduR,
piraveena and
shashimalcse
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #7692 +/- ##
============================================
- Coverage 50.80% 50.69% -0.12%
+ Complexity 19450 19309 -141
============================================
Files 2144 2125 -19
Lines 124774 124379 -395
Branches 26499 26334 -165
============================================
- Hits 63395 63049 -346
- Misses 53226 53262 +36
+ Partials 8153 8068 -85
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 started |
|
PR builder completed |
jenkins-is-staging
approved these changes
Jan 23, 2026
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/21286347969
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes in this pull request
A new efficient method is introduced to retrieve the system API resources with the scopes.
That method is used to detect the scope changes at the server startup.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.