Fix issue when registering a KM for 'Other' KM type #13554

msm1992 · 2026-01-16T08:10:39Z

This PR fixes below issue.

Try to register a KM of type 'Other', with only 'Token Exchange' option selected.
UI will show an error pop up saying "Error while adding the identity provider"

The error occured because there is no keyManagerConfigurationDTO for 'Other' type. As the fix added a null check.

...imgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

wso2-engineering

AI Agent Log Improvement Checklist

⚠️ Warning: AI-Generated Review Comments

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	N
#### Log Improvement Suggestion No: 2	N

coderabbitai · 2026-01-16T08:11:17Z

Note

Other AI code review bot(s) detected

CodeRabbit 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.

📝 Walkthrough

Walkthrough

Adds null/empty guards and two recursive helper methods in APIAdminImpl to safely apply masking to nested key manager and gateway configuration fields during maskValues processing.

Changes

Cohort / File(s)	Change Summary
Masking Logic Enhancement `components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java`	Added null/empty checks before iterating `keyManagerConnectorConfiguration` and auth configurations. Introduced `applyMaskToNestedFields(List<Object>, Map<String,Object>)` and `applyMaskToNestedGatewayFields(List<Object>, Map<String,String>)`. Updated `maskValues` flow to delegate nested masking to these helpers and skip masking when configurations are absent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main fix: addressing an issue with registering a Key Manager for the 'Other' type, which aligns with the changeset's null check additions.
Description check	✅ Passed	The description is directly related to the changeset, explaining the specific issue (missing keyManagerConfigurationDTO), the symptoms (UI error), and the solution (adding null check).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧹 Recent nitpick comments

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java (1)

1773-1784: Add a defensive type check before casting.

configurations is List<Object>, so a non-ConfigurationDto entry would trigger a ClassCastException. A small guard keeps this safe and aligns with the gateway helper’s pattern.

♻️ Suggested fix

-        for (Object configuration : configurations) {
-            if (((ConfigurationDto)configuration).isMask()) {
-                additionalProperties.replace(((ConfigurationDto) configuration).getName(),
-                        APIConstants.DEFAULT_MODIFIED_ENDPOINT_PASSWORD);
-            }
-            // Recursively process nested values
-            applyMaskToNestedFields(((ConfigurationDto)configuration).getValues(), additionalProperties);
-        }
+        for (Object configuration : configurations) {
+            if (!(configuration instanceof ConfigurationDto)) {
+                continue;
+            }
+            ConfigurationDto configurationDto = (ConfigurationDto) configuration;
+            if (configurationDto.isMask()) {
+                additionalProperties.replace(configurationDto.getName(),
+                        APIConstants.DEFAULT_MODIFIED_ENDPOINT_PASSWORD);
+            }
+            // Recursively process nested values
+            applyMaskToNestedFields(configurationDto.getValues(), additionalProperties);
+        }

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e1bd3d and 737ba31.

📒 Files selected for processing (1)

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

🧰 Additional context used

🧠 Learnings (23)

📓 Common learnings

Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:494-494
Timestamp: 2025-08-08T18:04:07.174Z
Learning: In PR wso2/carbon-apimgt#13201, the removal of the duplicate constant APIConstants.API_TYPE_PROP (impl module) and replacing its usage in TemplateBuilderUtil with APIConstants.API_TYPE is deferred to a separate follow-up PR.

Learnt from: piyumaldk
Repo: wso2/carbon-apimgt PR: 13483
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:1282-1286
Timestamp: 2025-10-13T09:56:46.003Z
Learning: In carbon-apimgt (Java), APIProviderImpl.validateKeyManagers(API, List<String>) must carry over disabled Key Managers from the existing API when updating, per product decision. Validation must still enforce that the updated API has at least one enabled Key Manager selected, accounting for API_LEVEL_ALL_KEY_MANAGERS by expanding to enabled tenant KMs for the final check.

Learnt from: Piumal1999
Repo: wso2/carbon-apimgt PR: 13242
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:1947-1951
Timestamp: 2025-08-11T06:25:44.255Z
Learning: APIProviderImpl.processSecretPolicyParameters (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java): For attributes of type Secret, if the incoming value is empty and no existing value is found, the method throws APIManagementException with ExceptionCodes.MISSING_MANDATORY_POLICY_ATTRIBUTES when the attribute is required. Otherwise, it preserves existing values or encrypts new plaintext values; non-string values are removed. This design intentionally keeps validation-before-processing to allow regex checks on plaintext.

Learnt from: nimsara66
Repo: wso2/carbon-apimgt PR: 13322
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java:1564-1593
Timestamp: 2025-08-26T05:35:40.184Z
Learning: In WSO2 Carbon API Manager's Key Manager configuration system, the `updateDisabled` field in ConfigurationDto is only applicable to root-level elements and is not required for nested fields. The validation should not be applied recursively to nested configurations like those found in authConfigurations.

Learnt from: tharikaGitHub
Repo: wso2/carbon-apimgt PR: 13251
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java:3559-3566
Timestamp: 2025-08-14T05:36:06.216Z
Learning: In APIUtil.getGatewayFeatureCatalog (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java), externalGatewayConnectorConfigurationMap is always non-null because it’s obtained and initialized to a new HashMap. Avoid suggesting null-guards for this map in future reviews.

Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13255
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:5685-5708
Timestamp: 2025-08-13T07:19:37.600Z
Learning: In ApiMgtDAO (WSO2 APIM), getCurrentAPIMetadata(String apiUuid) and getAPIMetadataRevision(String apiUuid, String revisionUuid) return a non-null empty Map when no metadata exists. Therefore, APIProviderImpl.populateApiMetadata can safely assign the DAO result without additional null-guards, and downstream calls relying on api.getMetadata().isEmpty() are safe.

Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13423
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/analytics/SynapseAnalyticsDataProvider.java:719-726
Timestamp: 2025-09-26T04:38:12.018Z
Learning: In SynapseAnalyticsDataProvider, sensitive headers like "Authorization" are removed upstream in AnalyticsMetricsHandler.getUserAgentAndCopyRequestHeadersToContext() before reaching the header masking logic in parseMaskSet(), making the empty set fallback safe for malformed JSON configuration.

Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java:792-800
Timestamp: 2025-08-14T02:19:32.862Z
Learning: In components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/API.java, the `metadata` field is initialized as `new HashMap<>()`, so `API#getMetadata()` is non-null by default. Null-guards before `put()` are unnecessary in typical flows.

Learnt from: Piumal1999
Repo: wso2/carbon-apimgt PR: 13242
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:1947-1951
Timestamp: 2025-08-10T19:10:23.541Z
Learning: In APIProviderImpl policy handling, author prefers validating before processing to keep regex checks on plaintext. When suggesting fixes, prefer a post-processing re-validation (or targeted mandatory check) over reordering, to avoid losing required secret attributes after processing.

📚 Learning: 2025-08-14T05:36:06.216Z

Learnt from: tharikaGitHub
Repo: wso2/carbon-apimgt PR: 13251
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java:3559-3566
Timestamp: 2025-08-14T05:36:06.216Z
Learning: In APIUtil.getGatewayFeatureCatalog (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java), externalGatewayConnectorConfigurationMap is always non-null because it’s obtained and initialized to a new HashMap. Avoid suggesting null-guards for this map in future reviews.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-26T05:35:40.184Z

Learnt from: nimsara66
Repo: wso2/carbon-apimgt PR: 13322
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java:1564-1593
Timestamp: 2025-08-26T05:35:40.184Z
Learning: In WSO2 Carbon API Manager's Key Manager configuration system, the `updateDisabled` field in ConfigurationDto is only applicable to root-level elements and is not required for nested fields. The validation should not be applied recursively to nested configurations like those found in authConfigurations.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-26T04:38:12.018Z

Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13423
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/analytics/SynapseAnalyticsDataProvider.java:719-726
Timestamp: 2025-09-26T04:38:12.018Z
Learning: In the WSO2 API Manager analytics flow, the Authorization header (APIConstants.AUTHORIZATION_HEADER_DEFAULT) is removed upstream in AnalyticsMetricsHandler.getUserAgentAndCopyRequestHeadersToContext() before headers reach the masking logic in SynapseAnalyticsDataProvider.parseMaskSet(), making the empty set fallback safe for malformed JSON configuration.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-10-13T09:56:46.003Z

Learnt from: piyumaldk
Repo: wso2/carbon-apimgt PR: 13483
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:1282-1286
Timestamp: 2025-10-13T09:56:46.003Z
Learning: In carbon-apimgt (Java), APIProviderImpl.validateKeyManagers(API, List<String>) must carry over disabled Key Managers from the existing API when updating, per product decision. Validation must still enforce that the updated API has at least one enabled Key Manager selected, accounting for API_LEVEL_ALL_KEY_MANAGERS by expanding to enabled tenant KMs for the final check.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-14T07:22:15.846Z

Learnt from: tharikaGitHub
Repo: wso2/carbon-apimgt PR: 13251
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.common/src/main/resources/admin-api.yaml:6055-6060
Timestamp: 2025-08-14T07:22:15.846Z
Learning: In wso2/carbon-apimgt, the file components/apimgt/org.wso2.carbon.apimgt.rest.api.common/src/main/resources/admin-api.yaml is an automated copy file that gets updated during compilation/build from the source file components/apimgt/org.wso2.carbon.apimgt.rest.api.admin.v1/src/main/resources/admin-api.yaml; changes should be made to the source file only.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-11T06:25:44.255Z

Learnt from: Piumal1999
Repo: wso2/carbon-apimgt PR: 13242
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:1947-1951
Timestamp: 2025-08-11T06:25:44.255Z
Learning: APIProviderImpl.processSecretPolicyParameters (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java): For attributes of type Secret, if the incoming value is empty and no existing value is found, the method throws APIManagementException with ExceptionCodes.MISSING_MANDATORY_POLICY_ATTRIBUTES when the attribute is required. Otherwise, it preserves existing values or encrypts new plaintext values; non-string values are removed. This design intentionally keeps validation-before-processing to allow regex checks on plaintext.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-11T15:07:24.831Z

Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13380
File: components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/federated/gateway/FederatedAPIDiscoveryRunner.java:247-255
Timestamp: 2025-09-11T15:07:24.831Z
Learning: In WSO2 Carbon API Manager's federated gateway discovery, FederatedAPIDiscovery connector implementations are responsible for handling null reference artifacts in the isAPIUpdated() method, rather than adding null checks in the calling code (FederatedAPIDiscoveryRunner).

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-07-30T13:59:48.856Z

Learnt from: BiyonFernando
Repo: wso2/carbon-apimgt PR: 13191
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/scheduler/GatewayValidationScheduler.java:81-87
Timestamp: 2025-07-30T13:59:48.856Z
Learning: In WSO2 Carbon API Manager's GatewayNotificationConfiguration, all nested configuration objects including GatewayCleanupConfiguration are initialized with default values at field declaration level, and the configuration parsing only updates existing values through setters, ensuring that null checks are unnecessary as the objects are guaranteed to be non-null.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-11-14T11:27:25.922Z

Learnt from: tharindu1st
Repo: wso2/carbon-apimgt PR: 13532
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:7412-7447
Timestamp: 2025-11-14T11:27:25.922Z
Learning: WSO2 carbon-apimgt: In APIProviderImpl.resumeDeployedAPIRevisionInternal(String apiId, String organization, String revisionUUID, String revisionId, String environment, boolean skipDeployToGateway), GatewayArtifactsMgtDAO label updates (addAndRemovePublishedGatewayLabels / removePublishedGatewayLabels) must not run when isInitiatedFromGateway=true (i.e., skipDeployToGateway is true). Label mapping updates are performed only for publisher-initiated flows; gateway-initiated flows should not invoke GatewayArtifactsMgtDAO here. This does not change the delete flow behavior, where stale labels are explicitly cleaned up for gateway-initiated APIs.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-10-10T05:10:50.118Z

Learnt from: ashera96
Repo: wso2/carbon-apimgt PR: 13476
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:1319-1331
Timestamp: 2025-10-10T05:10:50.118Z
Learning: WSO2 Carbon APIM: In APIProviderImpl.updateAPIResources (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java), when an API is associated with one or more MCP Servers, URI-template updates are intentionally skipped to prevent functional changes to the underlying API; users must first remove MCP servers to modify resources.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-25T09:28:42.598Z

Learnt from: ashanhr
Repo: wso2/carbon-apimgt PR: 13416
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIManagerConfiguration.java:633-639
Timestamp: 2025-09-25T09:28:42.598Z
Learning: In the APIManagerConfiguration.java file, configuration values from deployment.toml can still result in null XML elements during parsing, which is why null checks are consistently used throughout the file for similar configuration parsing scenarios, including Redis config, APIKeyValidator config, and others.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-06-18T10:07:57.821Z

Learnt from: thisaltennakoon
Repo: wso2/carbon-apimgt PR: 13149
File: components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/GraphQLSchemaDefinition.java:262-276
Timestamp: 2025-06-18T10:07:57.821Z
Learning: In the GraphQLSchemaDefinition class (and potentially other areas of the WSO2 API Manager codebase), template.getAuthType() can return null, which creates NPE risks when calling .equalsIgnoreCase() on the result. This specific issue has been tracked in https://github.com/wso2/api-manager/issues/3971.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-04T06:19:19.370Z

Learnt from: ashera96
Repo: wso2/carbon-apimgt PR: 13345
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:537-538
Timestamp: 2025-09-04T06:19:19.370Z
Learning: In wso2/carbon-apimgt, when adding new constants to support specific functionality in a PR, it's acceptable to limit the scope to only using the constant where directly related to the new feature being implemented. Broader cleanup of existing hard-coded literals can be deferred to separate follow-up PRs to maintain focused scope.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-08T18:04:07.174Z

Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:494-494
Timestamp: 2025-08-08T18:04:07.174Z
Learning: In PR wso2/carbon-apimgt#13201, the removal of the duplicate constant APIConstants.API_TYPE_PROP (impl module) and replacing its usage in TemplateBuilderUtil with APIConstants.API_TYPE is deferred to a separate follow-up PR.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-09T05:00:15.216Z

Learnt from: ashera96
Repo: wso2/carbon-apimgt PR: 13358
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java:0-0
Timestamp: 2025-09-09T05:00:15.216Z
Learning: In WSO2 carbon-apimgt, when implementing focused features like AI API revisioning, it's acceptable to leave existing patterns (like hardcoded strings in SQLConstants) unchanged to maintain limited PR scope, even when refactoring opportunities exist. Broader cleanup can be deferred to separate PRs.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-09T06:18:41.445Z

Learnt from: ashera96
Repo: wso2/carbon-apimgt PR: 13358
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java:0-0
Timestamp: 2025-09-09T06:18:41.445Z
Learning: In WSO2 carbon-apimgt revision-related PRs, when implementing focused features like AI API revisioning, it's acceptable to retain existing hardcoded string patterns in SQL constants (like 'Current API') to maintain limited PR scope, even when refactoring opportunities exist. Broader SQL constant cleanup can be deferred to separate PRs to avoid scope creep.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-10T11:22:38.891Z

Learnt from: ashanhr
Repo: wso2/carbon-apimgt PR: 13372
File: features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/org.wso2.carbon.apimgt.core.default.json:190-190
Timestamp: 2025-09-10T11:22:38.891Z
Learning: In WSO2 carbon-apimgt, the core configuration handling and context setup is done in the carbon-apimgt repository, while the actual Velocity endpoint templates (.vm files) that consume these configurations are maintained in the separate product-apim repository. Template changes should be made in product-apim, not carbon-apimgt.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-17T08:56:08.224Z

Learnt from: ashanhr
Repo: wso2/carbon-apimgt PR: 13372
File: features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2:2029-2031
Timestamp: 2025-09-17T08:56:08.224Z
Learning: Template changes for carbon-apimgt configurations should be made in the product-apim repository via .vm files, as referenced by the failover configuration implementation in https://github.com/wso2/product-apim/pull/13836, which demonstrates the proper separation where carbon-apimgt handles configuration setup and product-apim handles template consumption.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-09-10T08:43:34.290Z

Learnt from: YasasRangika
Repo: wso2/carbon-apimgt PR: 13371
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java:0-0
Timestamp: 2025-09-10T08:43:34.290Z
Learning: In carbon-apimgt, validation/normalization for disallowed "Unlimited" throttling tier is already handled by PR wso2/carbon-apimgt#11710, so APIUtil.validateAndUpdateURITemplates(...) should not duplicate that logic. Future fixes should rely on the centralized validation from `#11710` rather than rewriting tiers here.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-08T06:17:39.942Z

Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIDTOTypeWrapper.java:1-17
Timestamp: 2025-08-08T06:17:39.942Z
Learning: In PR wso2/carbon-apimgt#13201, the existing license header in components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIDTOTypeWrapper.java is correct; the alternative header variant suggested by the bot is older and should not be applied.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-22T10:58:02.268Z

Learnt from: msm1992
Repo: wso2/carbon-apimgt PR: 13310
File: features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2:1849-1851
Timestamp: 2025-08-22T10:58:02.268Z
Learning: In wso2/carbon-apimgt PR `#13310`, MCP failure handler naming is finalized: APIConstants.MCP.MCP_FAILURE_HANDLER = "_mcp_failure_handler_" and the template lists <Sequence>_mcp_failure_handler_.xml</Sequence>. Avoid re-flagging naming drift for this handler in future reviews.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

📚 Learning: 2025-08-27T11:55:56.357Z

Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13333
File: components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/ExceptionCodes.java:831-833
Timestamp: 2025-08-27T11:55:56.357Z
Learning: In wso2/carbon-apimgt, PasanT9 prefers keeping ExceptionCodes error messages generic for reusability rather than making them highly specific with placeholders, even when specific validation context is available in the calling code.

Applied to files:

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

🔇 Additional comments (2)

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java (2)

1745-1768: LGTM: null/empty guards prevent masking failures.

These guards avoid NPEs for Token Exchange scenarios without connector configs.

1796-1809: LGTM: recursive masking with instanceof guard is solid.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

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 and usage tips.}

Copilot

Pull request overview

This PR fixes a NullPointerException that occurs when registering a Key Manager (KM) of type 'Other' with only 'Token Exchange' option selected. The issue arose because the getKeyManagerConnectorConfiguration() method returns null for 'Other' type KMs, causing the maskValues() method to fail when attempting to access connection and authentication configurations.

Changes:

Added a null check in the maskValues(KeyManagerConfigurationDTO) method to guard against null KeyManagerConnectorConfiguration objects
Updated comment explaining when the configuration can be null (Token Exchange use case)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...imgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java

Fix issue when registering a KM for 'Other' KM type

8e1bd3d

msm1992 requested review from PasanT9 and Copilot January 16, 2026 08:10

msm1992 requested review from AnuGayan, Arshardh, HeshanSudarshana, HiranyaKavishani, Krishanx92, RakhithaRR, ashera96, chamilaadhi, dushaniw, hisanhunais, npamudika, pubudu538, senthuran16, tgtshanika, tharikaGitHub and tharindu1st as code owners January 16, 2026 08:10

wso2-engineering bot reviewed Jan 16, 2026

View reviewed changes

...imgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java Show resolved Hide resolved

wso2-engineering bot reviewed Jan 16, 2026

View reviewed changes

...imgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java Show resolved Hide resolved

wso2-engineering bot suggested changes Jan 16, 2026

View reviewed changes

Copilot started reviewing on behalf of msm1992 January 16, 2026 08:11 View session

coderabbitai bot previously approved these changes Jan 16, 2026

View reviewed changes

PasanT9 previously approved these changes Jan 16, 2026

View reviewed changes

Copilot AI reviewed Jan 16, 2026

View reviewed changes

wso2-engineering bot approved these changes Jan 16, 2026

View reviewed changes

Address review comments

737ba31

msm1992 dismissed PasanT9’s stale review via 737ba31 January 16, 2026 08:37

msm1992 dismissed coderabbitai[bot]’s stale review via 737ba31 January 16, 2026 08:37

coderabbitai bot approved these changes Jan 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix issue when registering a KM for 'Other' KM type #13554

Fix issue when registering a KM for 'Other' KM type #13554

Uh oh!

msm1992 commented Jan 16, 2026

Uh oh!

Uh oh!

Uh oh!

wso2-engineering bot left a comment •

edited by msm1992

Loading

Uh oh!

coderabbitai bot commented Jan 16, 2026 •

edited

Loading

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix issue when registering a KM for 'Other' KM type #13554

Are you sure you want to change the base?

Fix issue when registering a KM for 'Other' KM type #13554

Uh oh!

Conversation

msm1992 commented Jan 16, 2026

Uh oh!

Uh oh!

Uh oh!

wso2-engineering bot left a comment • edited by msm1992 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

AI Agent Log Improvement Checklist

Uh oh!

coderabbitai bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wso2-engineering bot left a comment •

edited by msm1992

Loading

coderabbitai bot commented Jan 16, 2026 •

edited

Loading