Implement extensible architecture for Federated API Discovery#13550
Open
JaninduRSD wants to merge 39 commits intowso2:masterfrom
Open
Implement extensible architecture for Federated API Discovery#13550JaninduRSD wants to merge 39 commits intowso2:masterfrom
JaninduRSD wants to merge 39 commits intowso2:masterfrom
Conversation
Fix- ket manager error for fedarated gateways Fix(final)- key manager issue for fedarated gateways fix new line remove
JaninduRSD
requested review from
AnuGayan,
Arshardh,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
RakhithaRR,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
npamudika,
pubudu538,
senthuran16,
tgtshanika,
tharikaGitHub and
tharindu1st
as code owners
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSelects API definition based on API type (AsyncAPI vs Swagger/OAS) across discovery, ZIP creation, versioning, import, and endpoint extraction; adds AsyncAPI parsing utilities and introduces generic federated API builder/factory classes to standardize mapping from external gateway sources. Changes
Sequence DiagramsequenceDiagram
participant FDR as FederatedAPIDiscoveryRunner
participant API as API (model)
participant FGU as FederatedGatewayUtil
participant ZIP as ZIP Stream
FDR->>API: api.isAsync()
alt async
FDR->>API: getAsyncApiDefinition()
else not async
FDR->>API: getSwaggerDefinition()
end
FDR->>FDR: validate definition non-null/non-blank
FDR->>FGU: createZipAsInputStream(apiYaml, apiDefinition, deploymentYaml, zipName, isAsync)
FGU->>FGU: write api.yaml & deployment_environments.yaml
alt async
FGU->>FGU: write Definitions/asyncapi.yaml
else not async
FGU->>FGU: write Definitions/swagger.yaml
end
FGU->>ZIP: return ByteArrayInputStream (ZIP)
ZIP-->>FDR: ZIP stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
Comment on lines
160
to
163
| public String getWsUrl() { | ||
| return getUrl("ws", wsHost, wsPort == DEFAULT_HTTP_PORT ? "" : ":" + wsPort, ""); | ||
| int port = (wsPort != null) ? wsPort : DEFAULT_WS_PORT; | ||
| return getUrl("ws", wsHost, port == DEFAULT_WS_PORT ? "" : ":" + port, ""); | ||
| } |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
Suggested change
| public String getWsUrl() { | |
| return getUrl("ws", wsHost, wsPort == DEFAULT_HTTP_PORT ? "" : ":" + wsPort, ""); | |
| int port = (wsPort != null) ? wsPort : DEFAULT_WS_PORT; | |
| return getUrl("ws", wsHost, port == DEFAULT_WS_PORT ? "" : ":" + port, ""); | |
| } | |
| public String getWsUrl() { | |
| int port = (wsPort != null) ? wsPort : DEFAULT_WS_PORT; | |
| log.debug("Generating WebSocket URL with host: {} and port: {}", wsHost, port); | |
| return getUrl("ws", wsHost, port == DEFAULT_WS_PORT ? "" : ":" + port, ""); |
Comment on lines
165
to
168
| public String getWssUrl() { | ||
| return getUrl("wss", wssHost, wssPort == DEFAULT_HTTPS_PORT ? "" : ":" + wssPort, ""); | ||
| int port = (wssPort != null) ? wssPort : DEFAULT_WSS_PORT; | ||
| return getUrl("wss", wssHost, port == DEFAULT_WSS_PORT ? "" : ":" + port, ""); | ||
| } |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
Suggested change
| public String getWssUrl() { | |
| return getUrl("wss", wssHost, wssPort == DEFAULT_HTTPS_PORT ? "" : ":" + wssPort, ""); | |
| int port = (wssPort != null) ? wssPort : DEFAULT_WSS_PORT; | |
| return getUrl("wss", wssHost, port == DEFAULT_WSS_PORT ? "" : ":" + port, ""); | |
| } | |
| public String getWssUrl() { | |
| int port = (wssPort != null) ? wssPort : DEFAULT_WSS_PORT; | |
| log.debug("Generating Secure WebSocket URL with host: {} and port: {}", wssHost, port); | |
| return getUrl("wss", wssHost, port == DEFAULT_WSS_PORT ? "" : ":" + port, ""); |
| } | ||
| } | ||
|
|
||
| InputStream apiZip = FederatedGatewayUtil.createZipAsInputStream( |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 3
Suggested change
| InputStream apiZip = FederatedGatewayUtil.createZipAsInputStream( | |
| InputStream apiZip = FederatedGatewayUtil.createZipAsInputStream( | |
| log.info("Creating import package for API: " + apidto.getName() + " version: " + apidto.getVersion()); |
|
|
||
| // Import API | ||
| // Import API | ||
| ImportedAPIDTO importedApi = importExportAPI.importAPI(apiZip, false, |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 4
Suggested change
| ImportedAPIDTO importedApi = importExportAPI.importAPI(apiZip, false, | |
| ImportedAPIDTO importedApi = importExportAPI.importAPI(apiZip, false, | |
| log.info("Importing API: " + apidto.getName() + " version: " + apidto.getVersion() + " to environment: " + environment); |
Comment on lines
114
to
116
| public static InputStream createZipAsInputStream(String apiYaml, String swaggerYaml, String deploymentYaml, | ||
| String zipName, String apiType) throws IOException { | ||
| if (apiYaml == null || swaggerYaml == null || deploymentYaml == null) { |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 5
Suggested change
| public static InputStream createZipAsInputStream(String apiYaml, String swaggerYaml, String deploymentYaml, | |
| String zipName, String apiType) throws IOException { | |
| if (apiYaml == null || swaggerYaml == null || deploymentYaml == null) { | |
| public static InputStream createZipAsInputStream(String apiYaml, String swaggerYaml, String deploymentYaml, | |
| String zipName, String apiType) throws IOException { | |
| log.info("Creating zip file for API: " + zipName + " with API type: " + apiType); | |
| if (apiYaml == null || swaggerYaml == null || deploymentYaml == null) { |
Comment on lines
125
to
130
| // Add Definitions/swagger.yaml | ||
| addToZip(zos, zipName + "/" + SWAGGER_YAML_FILE_NAME, swaggerYaml); | ||
| String definitionFileName = ("ASYNC".equalsIgnoreCase(apiType) || "AsyncAPI".equalsIgnoreCase(apiType) | ||
| || "WS".equalsIgnoreCase(apiType) || "WEBSUB".equalsIgnoreCase(apiType) || "SSE".equalsIgnoreCase(apiType)) | ||
| ? "Definitions/asyncapi.yaml" | ||
| : SWAGGER_YAML_FILE_NAME; | ||
| addToZip(zos, zipName + "/" + definitionFileName, swaggerYaml); |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 6
Suggested change
| // Add Definitions/swagger.yaml | |
| addToZip(zos, zipName + "/" + SWAGGER_YAML_FILE_NAME, swaggerYaml); | |
| String definitionFileName = ("ASYNC".equalsIgnoreCase(apiType) || "AsyncAPI".equalsIgnoreCase(apiType) | |
| || "WS".equalsIgnoreCase(apiType) || "WEBSUB".equalsIgnoreCase(apiType) || "SSE".equalsIgnoreCase(apiType)) | |
| ? "Definitions/asyncapi.yaml" | |
| : SWAGGER_YAML_FILE_NAME; | |
| addToZip(zos, zipName + "/" + definitionFileName, swaggerYaml); | |
| // Add Definitions/swagger.yaml | |
| String definitionFileName = ("ASYNC".equalsIgnoreCase(apiType) || "AsyncAPI".equalsIgnoreCase(apiType) | |
| || "WS".equalsIgnoreCase(apiType) || "WEBSUB".equalsIgnoreCase(apiType) || "SSE".equalsIgnoreCase(apiType)) | |
| ? "Definitions/asyncapi.yaml" | |
| : SWAGGER_YAML_FILE_NAME; | |
| log.debug("Using definition file: " + definitionFileName + " for API type: " + apiType); | |
| addToZip(zos, zipName + "/" + definitionFileName, swaggerYaml); |
Comment on lines
1340
to
1342
| } | ||
| if (validKeyManagers.isEmpty()) { | ||
| if (validKeyManagers.isEmpty() && !api.isInitiatedFromGateway()) { | ||
| throw new APIManagementException( |
Contributor
There was a problem hiding this comment.
Log Improvement Suggestion No: 7
Suggested change
| } | |
| if (validKeyManagers.isEmpty()) { | |
| if (validKeyManagers.isEmpty() && !api.isInitiatedFromGateway()) { | |
| throw new APIManagementException( | |
| } | |
| if (validKeyManagers.isEmpty() && !api.isInitiatedFromGateway()) { | |
| log.error("API validation failed: No valid key managers found for API: " + api.getId()); | |
| throw new APIManagementException( |
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.
![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
🤖 Fix all issues with AI agents
In
@components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java:
- Around line 295-306: The code logs a warning when swaggerDefinition (from
api.getSwaggerDefinition()) is null/empty but then still calls
FederatedGatewayUtil.createZipAsInputStream(...) which throws when swaggerYaml
is null; after the existing log.warn/log.debug block for apidto
(name/version/type) add a continue to skip processing this API (or alternatively
set swaggerDefinition to a safe placeholder before calling
createZipAsInputStream), ensuring you update the block around swaggerDefinition,
apiJson, environment and the call to FederatedGatewayUtil.createZipAsInputStream
to avoid passing a null swagger.
🧹 Nitpick comments (1)
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/util/FederatedGatewayUtil.java (1)
126-130: Use the existingYAML_ASYNCAPI_DEFINITION_LOCATIONconstant instead of hardcoding the path.Replace the hardcoded
"Definitions/asyncapi.yaml"withYAML_ASYNCAPI_DEFINITION_LOCATIONfromImportExportConstants. This constant already exists and produces the same value, ensuring consistency with howSWAGGER_YAML_FILE_NAMEis used for OpenAPI definitions.Suggested fix
// Add import: import static org.wso2.carbon.apimgt.impl.importexport.ImportExportConstants.YAML_ASYNCAPI_DEFINITION_LOCATION; // Update lines 126-130: String definitionFileName = ("ASYNC".equalsIgnoreCase(apiType) || "AsyncAPI".equalsIgnoreCase(apiType) || "WS".equalsIgnoreCase(apiType) || "WEBSUB".equalsIgnoreCase(apiType) || "SSE".equalsIgnoreCase(apiType)) ? YAML_ASYNCAPI_DEFINITION_LOCATION : SWAGGER_YAML_FILE_NAME;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/VHost.javacomponents/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.javacomponents/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/util/FederatedGatewayUtil.javacomponents/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
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.
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.
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.
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.
Learnt from: HeshanSudarshana
Repo: wso2/carbon-apimgt PR: 13329
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml:12295-12299
Timestamp: 2025-08-27T10:45:21.255Z
Learning: In wso2/carbon-apimgt, components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml is a copied/synced spec; property names must mirror the upstream YAML. Do not propose renaming fields for casing consistency here (e.g., keep TechnicalOwner/TechnicalOwnerEmail as-is).
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: 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.
Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/devportal-api.yaml:6439-6471
Timestamp: 2025-08-04T12:17:47.981Z
Learning: In the WSO2 API Manager codebase, the `PostRequestBody` and `PatchRequestBody` schemas for comments were generalized and reused in PR #13201; they were not newly introduced.
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:249-257
Timestamp: 2025-06-18T10:07:41.656Z
Learning: When reviewing refactored/moved code in WSO2 Carbon API Manager, if pre-existing issues are found that are outside the scope of the current PR (like code moves/refactoring), the team creates separate GitHub issues to track fixes rather than addressing them in the refactoring PR. This helps maintain focused PR scope.
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).
Learnt from: SavinduDimal
Repo: wso2/carbon-apimgt PR: 13426
File: features/apimgt/org.wso2.carbon.apimgt.federated.gateway.feature/pom.xml:73-112
Timestamp: 2025-09-28T14:54:20.703Z
Learning: In WSO2 API Manager ACP, core Netty modules (netty-common, netty-buffer, netty-transport, netty-codec, netty-handler, netty-resolver, netty-resolver-dns) are already packaged in the runtime environment, so federated gateway features only need to include additional specific Netty modules like netty-codec-http, netty-codec-http2, netty-codec-socks, and netty-handler-proxy.
Learnt from: SavinduDimal
Repo: wso2/carbon-apimgt PR: 13426
File: features/apimgt/org.wso2.carbon.apimgt.federated.gateway.feature/pom.xml:73-112
Timestamp: 2025-09-28T14:54:20.703Z
Learning: In WSO2 API Manager ACP, core Netty modules (netty-common, netty-buffer, netty-transport, netty-codec, netty-handler, netty-resolver, netty-resolver-dns) are already packaged in the runtime environment, so federated gateway features only need to include additional specific Netty modules like netty-codec-http, netty-codec-http2, netty-codec-socks, and netty-handler-proxy.
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13182
File: components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/federated/gateway/FederatedAPIDiscoveryRunner.java:82-132
Timestamp: 2025-08-06T06:23:59.604Z
Learning: In WSO2 Carbon API Manager's federated gateway discovery, reflection-based class loading from gatewayConfiguration.getDiscoveryImplementation() is considered secure because gateway connector configurations are created and managed by trusted parties (system administrators), not end users. The discovery implementation classes come from vetted configurations in a controlled enterprise environment.
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13182
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java:0-0
Timestamp: 2025-07-19T14:47:03.856Z
Learning: In WSO2 Carbon API Manager, the validateAndScheduleFederatedGatewayAPIDiscovery method should not throw exceptions or perform a null check on the environment parameter, as errors are only logged and the environment is validated earlier in the call chain to avoid impacting server startup.
Learnt from: msm1992
Repo: wso2/carbon-apimgt PR: 13361
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java:541-577
Timestamp: 2025-09-10T04:49:41.525Z
Learning: In WSO2 API Manager MCPUtils.getGatewayServerURL method, the user msm1992 indicated that null return handling should use gateway localhost URL as fallback, and this improvement will be implemented in a separate PR rather than the current MCP GW improvements PR #13361.
Learnt from: tharindu1st
Repo: wso2/carbon-apimgt PR: 13317
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIGatewayManager.java:0-0
Timestamp: 2025-09-07T08:33:11.065Z
Learning: The empty APIProduct unDeployFromGateway overload method stub that was flagged in APIGatewayManager.java has been confirmed as removed from the codebase, resolving the silent no-op concern.
Learnt from: msm1992
Repo: wso2/carbon-apimgt PR: 13338
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/APIAuthenticationHandler.java:770-781
Timestamp: 2025-08-29T06:14:37.385Z
Learning: In APIAuthenticationHandler for MCP authentication failures, the manual URL construction was a temporary fix. The current implementation uses MCPUtils.getGatewayServerURL() which picks gateway URL from vhost list with priority to Host header value. The user msm1992 indicated that fallback logic may not be needed given their vhost-based architecture approach. However, MCPUtils.getGatewayServerURL() can return null in several scenarios: no API found for contextPath, empty vhosts list, host header mismatch with no fallback vhosts, or no HTTPS URLs in vhosts.
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13274
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/resources/operationPolicy/operation-policy-specification-schema.json:46-48
Timestamp: 2025-08-15T05:48:13.739Z
Learning: In the carbon-apimgt codebase, when adding new gateway vendors to the supportedGateways enum in operation-policy-specification-schema.json, both UI and backend support are automatically handled by the system without requiring explicit code changes in individual components like APIConstants, DTOs, policy handlers, or tests.
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13182
File: components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/util/FederatedGatewayUtil.java:73-84
Timestamp: 2025-08-06T04:06:48.547Z
Learning: In WSO2 Carbon API Manager's FederatedGatewayUtil.deleteAPI method, parameters (apiUUID, organization) are validated upstream in the call chain before reaching this utility method. The method intentionally swallows APIManagementException and only logs errors to maintain flow continuity during batch operations like federated gateway API cleanup/discovery, where individual failures should not halt the entire process.
📚 Learning: 2025-08-29T06:14:37.385Z
Learnt from: msm1992
Repo: wso2/carbon-apimgt PR: 13338
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/APIAuthenticationHandler.java:770-781
Timestamp: 2025-08-29T06:14:37.385Z
Learning: In APIAuthenticationHandler for MCP authentication failures, the manual URL construction was a temporary fix. The current implementation uses MCPUtils.getGatewayServerURL() which picks gateway URL from vhost list with priority to Host header value. The user msm1992 indicated that fallback logic may not be needed given their vhost-based architecture approach. However, MCPUtils.getGatewayServerURL() can return null in several scenarios: no API found for contextPath, empty vhosts list, host header mismatch with no fallback vhosts, or no HTTPS URLs in vhosts.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/VHost.java
📚 Learning: 2025-06-18T09:19:07.080Z
Learnt from: thisaltennakoon
Repo: wso2/carbon-apimgt PR: 13149
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/restapi/publisher/ApisApiServiceImplUtils.java:643-649
Timestamp: 2025-06-18T09:19:07.080Z
Learning: In WSO2 Carbon APIM, the method APIUtil.getHttpClient(int port, String protocol) ignores the port parameter and only uses the protocol and configuration from ServiceReferenceHolder. Despite accepting a port argument, it internally calls CommonAPIUtil.getHttpClient(protocol, configuration) without passing the port value.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/VHost.java
📚 Learning: 2025-09-07T08:33:11.065Z
Learnt from: tharindu1st
Repo: wso2/carbon-apimgt PR: 13317
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIGatewayManager.java:0-0
Timestamp: 2025-09-07T08:33:11.065Z
Learning: The empty APIProduct unDeployFromGateway overload method in APIGatewayManager.java that was flagged during review has been removed from the codebase and is no longer available.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/util/FederatedGatewayUtil.java
📚 Learning: 2025-07-14T14:03:05.170Z
Learnt from: tharindu1st
Repo: wso2/carbon-apimgt PR: 13144
File: components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/AWSBedrockLLMProviderService.java:59-62
Timestamp: 2025-07-14T14:03:05.170Z
Learning: In WSO2 Carbon API Manager, API definition files are copied from their source location (features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/api_definitions/) to the runtime location (repository/resources/api_definitions/) during the build/deployment process. Therefore, the path construction using "repository" + File.separator + "resources" + File.separator + "api_definitions" in LLM provider service implementations is correct for the runtime environment.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/util/FederatedGatewayUtil.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/APIProviderImpl.java
📚 Learning: 2025-08-15T04:32:38.565Z
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13274
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/ApisApiServiceImpl.java:3109-3111
Timestamp: 2025-08-15T04:32:38.565Z
Learning: In the WSO2 Carbon API Manager, when the context validation is temporarily disabled in the validateAPI endpoint (ApisApiServiceImpl.java), the team prefers to add detailed documentation and logging improvements alongside corresponding UI updates rather than in isolation.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.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/APIProviderImpl.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/APIProviderImpl.java
📚 Learning: 2025-09-30T17:50:08.410Z
Learnt from: tharikaGitHub
Repo: wso2/carbon-apimgt PR: 13405
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/LifeCycleUtils.java:181-185
Timestamp: 2025-09-30T17:50:08.410Z
Learning: In the WSO2 Carbon API Management (carbon-apimgt) codebase, discovered APIs (APIs where `isInitiatedFromGateway()` returns true) are expected not to manage business plans (tiers) and backend endpoints. Therefore, validation checks for tiers and endpoints should be bypassed during lifecycle state transitions (e.g., publishing) for such APIs.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.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/APIProviderImpl.javacomponents/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-09-10T08:28:32.725Z
Learnt from: PasanT9
Repo: wso2/carbon-apimgt PR: 13370
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/APIKeyValidator.java:334-344
Timestamp: 2025-09-10T08:28:32.725Z
Learning: In WSO2 carbon-apimgt, the endUserName in APIKeyValidationInfoDTO cannot be null in MCP flows. The token validation infrastructure (DefaultKeyValidationHandler, APIKeyValidationService) ensures that endUserName is always populated from the validated token, and APIUtil.getUserNameWithTenantSuffix() method is null-safe with explicit null checks.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-08-28T07:49:28.738Z
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:1014-1019
Timestamp: 2025-08-28T07:49:28.738Z
Learning: In WSO2 Carbon API Manager's Key Manager configuration system, TokenType.EXCHANGED intentionally bypasses all validation checks (including the new updateDisabled enforcement) to maintain backward compatibility, as explained by nimsara66 in PR #13322.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-06-30T06:50:20.514Z
Learnt from: nimsara66
Repo: wso2/carbon-apimgt PR: 13158
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/AzureContentSafetyGuardrailProviderServiceImpl.java:75-84
Timestamp: 2025-06-30T06:50:20.514Z
Learning: In WSO2 Carbon API Manager, initialization of service components (like GuardrailProviderService implementations) is guaranteed to occur before service methods are called, so additional initialization validation checks in service methods are not needed.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-09-26T07:19:03.063Z
Learnt from: ashera96
Repo: wso2/carbon-apimgt PR: 13421
File: features/apimgt/org.wso2.carbon.apimgt.keymanager.feature/src/main/resources/sql/h2.sql:0-0
Timestamp: 2025-09-26T07:19:03.063Z
Learning: In WSO2 Carbon API Manager schema changes, migration impacts from primary key alterations (such as removing ENDPOINT_NAME from AM_API_ENDPOINTS composite primary key) are often handled via separate efforts/PRs rather than within the initial schema change PR, allowing for focused scope and separate testing of schema definitions versus migration procedures.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-08-07T13:29:57.960Z
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:221-229
Timestamp: 2025-08-07T13:29:57.960Z
Learning: In WSO2 Carbon API Manager, the keyManagers field from APIDTO.getKeyManagers() and MCPServerDTO.getKeyManagers() methods always returns an array of strings, even though the method signature returns Object. The cast to List<String> is safe in this context due to architectural guarantees.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-08-06T04:06:48.547Z
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13182
File: components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/util/FederatedGatewayUtil.java:73-84
Timestamp: 2025-08-06T04:06:48.547Z
Learning: In WSO2 Carbon API Manager's FederatedGatewayUtil.deleteAPI method, parameters (apiUUID, organization) are validated upstream in the call chain before reaching this utility method. The method intentionally swallows APIManagementException and only logs errors to maintain flow continuity during batch operations like federated gateway API cleanup/discovery, where individual failures should not halt the entire process.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.javacomponents/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-07-19T14:47:03.856Z
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13182
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java:0-0
Timestamp: 2025-07-19T14:47:03.856Z
Learning: In WSO2 Carbon API Manager, the validateAndScheduleFederatedGatewayAPIDiscovery method should not throw exceptions or perform a null check on the environment parameter, as errors are only logged and the environment is validated earlier in the call chain to avoid impacting server startup.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.javacomponents/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.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.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-06-03T06:19:48.138Z
Learnt from: Piumal1999
Repo: wso2/carbon-apimgt PR: 13132
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/PublisherCommonUtils.java:1524-1524
Timestamp: 2025-06-03T06:19:48.138Z
Learning: In PublisherCommonUtils.addAPIWithGeneratedSwaggerDefinition method, when ParseException was added to the throws clause, the calling methods in ApisApiServiceImpl.java and ImportUtils.java already had proper exception handling in place, so no additional changes were needed.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-10-05T02:22:12.946Z
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13463
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java:12086-12104
Timestamp: 2025-10-05T02:22:12.946Z
Learning: In wso2/carbon-apimgt, the Swagger definition on API objects is pre-validated before use in the versioning flow. Specifically, in components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java, method updateAPISwaggerWithVersion(API api) can assume that api.getSwaggerDefinition() is valid JSON when non-null (as it is already validated earlier in the createNewAPIVersion flow). Therefore, wrapping Json parsing in a try/catch for JsonParseException isn’t necessary in this context.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-08-06T06:23:59.604Z
Learnt from: AnuGayan
Repo: wso2/carbon-apimgt PR: 13182
File: components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/federated/gateway/FederatedAPIDiscoveryRunner.java:82-132
Timestamp: 2025-08-06T06:23:59.604Z
Learning: In WSO2 Carbon API Manager's federated gateway discovery, reflection-based class loading from gatewayConfiguration.getDiscoveryImplementation() is considered secure because gateway connector configurations are created and managed by trusted parties (system administrators), not end users. The discovery implementation classes come from vetted configurations in a controlled enterprise environment.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-09-07T18:55:58.401Z
Learnt from: tharindu1st
Repo: wso2/carbon-apimgt PR: 13317
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:0-0
Timestamp: 2025-09-07T18:55:58.401Z
Learning: WSO2 carbon-apimgt: In APIProviderImpl.deleteAPIRevisions(String apiUUID, String organization, boolean onDeleteOrRetire), the method (a) forwards onDeleteOrRetire to undeployAPIRevisionDeployment to ensure correct gateway event semantics, and (b) when the API is initiated from gateway, it removes DB deployments and also calls GatewayArtifactsMgtDAO.removePublishedGatewayLabels(apiUUID, revisionUUID, envs) to avoid stale labels. This is the intended contract going forward.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-10-15T08:23:18.789Z
Learnt from: AqeelMuhammad
Repo: wso2/carbon-apimgt PR: 12835
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/ApiProductsApiServiceImpl.java:458-462
Timestamp: 2025-10-15T08:23:18.789Z
Learning: In the `wso2/carbon-apimgt` repository, REST API-level changes (such as those in `components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1`) are covered by integration tests rather than unit tests. Do not flag missing unit test coverage for REST API service implementation classes.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
📚 Learning: 2025-09-07T08:33:11.065Z
Learnt from: tharindu1st
Repo: wso2/carbon-apimgt PR: 13317
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIGatewayManager.java:0-0
Timestamp: 2025-09-07T08:33:11.065Z
Learning: The empty APIProduct unDeployFromGateway overload method stub that was flagged in APIGatewayManager.java has been confirmed as removed from the codebase, resolving the silent no-op concern.
Applied to files:
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
🧬 Code graph analysis (1)
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java (1)
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/util/FederatedGatewayUtil.java (1)
FederatedGatewayUtil(64-233)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL analysis (java)
- GitHub Check: Agent
🔇 Additional comments (4)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (1)
1277-1345: Relaxed key‑manager requirement for gateway‑initiated APIs looks correct; consider enabled‑KM check semanticsThe new condition
if (validKeyManagers.isEmpty() && !api.isInitiatedFromGateway()) { throw new APIManagementException( "API must have at least one valid and enabled key manager configured", ExceptionCodes.KEY_MANAGER_NOT_FOUND); }cleanly scopes the “at least one valid/enabled KM” requirement to publisher‑initiated APIs while allowing gateway‑initiated/discovered APIs (
isInitiatedFromGateway() == true) to exist without local KM configuration, which matches the intended federated‑gateway behavior. Based on learnings, this is the right direction.Two nuances to keep in mind:
- For update flows,
validKeyManagersis pre‑seeded with anyexistingKeyManagersthat are still selected, regardless of whether those KMs are indisabledKeyManagers. This preserves disabled KMs on existing APIs (as required), but it also means the “empty” check can pass even if all selected KMs are currently disabled. If product intent is still “at least one enabled KM” for non‑gateway APIs, you may want a follow‑up check likevalidKeyManagers.stream().anyMatch(km -> !disabledKeyManagers.contains(km))instead of justisEmpty(), or at least confirm this is acceptable.- Correctness now depends on
api.isInitiatedFromGateway()being reliably set for both create and update paths for discovered APIs beforevalidateKeyManagers(...)is invoked. If there are any code paths where that flag might be false on a discovered API, those would unintentionally still hit the strict validation.Functionally this change is sound for the federated/gateway‑initiated case; the enabled‑KM semantics for non‑gateway flows are the only area that might warrant a quick re‑check against product expectations.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/VHost.java (1)
160-168: LGTM! Correctly aligns port omission logic with WebSocket defaults.The fix properly compares against protocol-specific default ports (
DEFAULT_WS_PORT/DEFAULT_WSS_PORT) instead of the previous HTTP/HTTPS defaults. This ensures the port is correctly omitted from the URL only when it matches the actual WebSocket default (9099/8099), not the unrelated HTTP default (80/443).The added null-checks provide defensive handling, though the fields are initialized with defaults.
components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/util/FederatedGatewayUtil.java (1)
109-137: LGTM! Clean backward-compatible overload pattern.The delegation from the 4-parameter overload to the new 5-parameter version maintains backward compatibility while supporting the new
apiType-based file naming for async API definitions.components/apimgt/org.wso2.carbon.apimgt.federated.gateway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java (1)
310-314: The importAPI method signature has been correctly updated and all callers are consistent.The
ImportExportAPI.importAPImethod now includes theorganizationparameter as the final argument, matching both the interface definition and implementation. The call in FederatedAPIDiscoveryRunner correctly passes the organization parameter along with all required parameters in the correct order. The other caller in ApisApiServiceImpl also uses the updated signature consistently.
...eway/src/main/java/org/wso2/carbon/apimgt/federated/gateway/FederatedAPIDiscoveryRunner.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
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/APIMappingUtil.java`:
- Line 1674: Guard the call to model.getTransports() before splitting in
APIMappingUtil: instead of unconditionally calling
dto.setTransport(List.of(model.getTransports().split(","))), check that
model.getTransports() is non-null and not blank, and only then split and set the
list; otherwise set dto.setTransport(Collections.emptyList()) (or skip setting
so existing fallback logic applies) to avoid NPE and the [""] result for empty
transports.
In
`@components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/AsyncApiParserUtil.java`:
- Around line 1004-1011: In the serversNode.isObject() branch of
AsyncApiParserUtil, mirror the array-handling basePath substitution: after
reading server.get("url").asText(), check server.has("variables") and if
variables.has("basePath") use variables.get("basePath").get("default").asText()
(or the same fallback used in the array branch) to replace any "{basePath}"
placeholder in the URL; apply the same replacement logic and return the
substituted URL so object-style servers get the same basePath handling as the
array path.
...c/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java
Outdated
Show resolved
Hide resolved
....parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/AsyncApiParserUtil.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/APISpecParserUtil.java`:
- Around line 81-88: APISpecParserUtil currently handles variable substitution
for array-style servers but not for object-style servers: update the object
branch that iterates serversNode.elements() (where server is the JsonNode) to
perform the same basePath variable substitution used in the array branch—look
for the logic that reads server.get("url") and applies basePath/variables and
replicate/apply it when server.has("url") in the object-style path; ensure you
also read server.get("variables") (and variable named "basePath" if present) and
replace placeholders in the URL consistently with the existing substitution
implementation.
- Around line 51-57: The Javadoc claims APISpecParserUtil extracts endpoints
from JSON/YAML but the implementation uses new
ObjectMapper().readTree(apiDefinition) which only handles JSON; either switch to
a YAML-capable parser or update the docs. Fix by updating the parsing logic in
APISpecParserUtil (the method with the Javadoc) to use an ObjectMapper
constructed with YAMLFactory (as AsyncApiParserUtil does) so it can parse YAML
or JSON input, or alternatively change the Javadoc to state JSON-only support;
ensure you reference APISpecParserUtil and the method that calls new
ObjectMapper().readTree(apiDefinition) when making the change.
🧹 Nitpick comments (1)
components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/APISpecParserUtil.java (1)
63-63: Consider reusing ObjectMapper as a static field.
ObjectMapperis thread-safe and relatively expensive to instantiate. Creating a new instance on every method call impacts performance, especially under high load.♻️ Proposed refactor
Add a static field at class level:
private static final ObjectMapper objectMapper = new ObjectMapper();Then update line 63:
- JsonNode rootNode = new ObjectMapper().readTree(apiDefinition); + JsonNode rootNode = objectMapper.readTree(apiDefinition);
Comment on lines
51
to
57
| /** | ||
| * Extracts the endpoint URL from an AsyncAPI or OpenAPI definition (JSON/YAML). | ||
| * Supports AsyncAPI v2/v3 (servers as Map) and OpenAPI (servers as Array). | ||
| * | ||
| * @param apiDefinition The API definition string (JSON/YAML). | ||
| * @return The extracted URL or null if not found. | ||
| */ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/APISpecParserUtil.java | head -100Repository: wso2/carbon-apimgt
Length of output: 4513
🏁 Script executed:
rg -n "YAMLFactory|YAMLMapper|new ObjectMapper\(" --type=java -C 2Repository: wso2/carbon-apimgt
Length of output: 50375
Use YAMLFactory for actual YAML parsing, or correct the Javadoc.
The method Javadoc states support for "JSON/YAML", but line 63 uses new ObjectMapper().readTree(apiDefinition), which only parses JSON. True YAML parsing requires YAMLFactory (as used elsewhere in the codebase: see AsyncApiParserUtil lines 203 and 243). Either integrate YAMLFactory to support YAML definitions, or update the Javadoc to reflect JSON-only support.
🤖 Prompt for AI Agents
In
`@components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/APISpecParserUtil.java`
around lines 51 - 57, The Javadoc claims APISpecParserUtil extracts endpoints
from JSON/YAML but the implementation uses new
ObjectMapper().readTree(apiDefinition) which only handles JSON; either switch to
a YAML-capable parser or update the docs. Fix by updating the parsing logic in
APISpecParserUtil (the method with the Javadoc) to use an ObjectMapper
constructed with YAMLFactory (as AsyncApiParserUtil does) so it can parse YAML
or JSON input, or alternatively change the Javadoc to state JSON-only support;
ensure you reference APISpecParserUtil and the method that calls new
ObjectMapper().readTree(apiDefinition) when making the change.
Comment on lines
81
to
88
| } else if (serversNode.isObject()) { | ||
| Iterator<JsonNode> elements = serversNode.elements(); | ||
| if (elements.hasNext()) { | ||
| JsonNode server = elements.next(); | ||
| if (server.has("url")) { | ||
| return server.get("url").asText(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent basePath substitution for object-style servers.
When servers is an array (lines 66-80), basePath variable substitution is performed. However, when servers is an object (AsyncAPI v2/v3 style), no variable substitution occurs. AsyncAPI object-style servers can also have variables with basePath, leading to inconsistent URL resolution.
🔧 Proposed fix to add basePath substitution for object servers
} else if (serversNode.isObject()) {
Iterator<JsonNode> elements = serversNode.elements();
if (elements.hasNext()) {
JsonNode server = elements.next();
if (server.has("url")) {
- return server.get("url").asText();
+ String resolvedUrl = server.get("url").asText();
+ if (server.has("variables") && server.get("variables").has("basePath")) {
+ JsonNode basePath = server.get("variables").get("basePath");
+ if (basePath.has("default")) {
+ String stageName = basePath.get("default").asText();
+ resolvedUrl = resolvedUrl
+ .replace("/{basePath}", "/" + stageName)
+ .replace("{basePath}", stageName);
+ }
+ }
+ return resolvedUrl;
}
}
}📝 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.
Suggested change
| } else if (serversNode.isObject()) { | |
| Iterator<JsonNode> elements = serversNode.elements(); | |
| if (elements.hasNext()) { | |
| JsonNode server = elements.next(); | |
| if (server.has("url")) { | |
| return server.get("url").asText(); | |
| } | |
| } | |
| } else if (serversNode.isObject()) { | |
| Iterator<JsonNode> elements = serversNode.elements(); | |
| if (elements.hasNext()) { | |
| JsonNode server = elements.next(); | |
| if (server.has("url")) { | |
| String resolvedUrl = server.get("url").asText(); | |
| if (server.has("variables") && server.get("variables").has("basePath")) { | |
| JsonNode basePath = server.get("variables").get("basePath"); | |
| if (basePath.has("default")) { | |
| String stageName = basePath.get("default").asText(); | |
| resolvedUrl = resolvedUrl | |
| .replace("/{basePath}", "/" + stageName) | |
| .replace("{basePath}", stageName); | |
| } | |
| } | |
| return resolvedUrl; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/APISpecParserUtil.java`
around lines 81 - 88, APISpecParserUtil currently handles variable substitution
for array-style servers but not for object-style servers: update the object
branch that iterates serversNode.elements() (where server is the JsonNode) to
perform the same basePath variable substitution used in the array branch—look
for the logic that reads server.get("url") and applies basePath/variables and
replicate/apply it when server.has("url") in the object-style path; ensure you
also read server.get("variables") (and variable named "basePath" if present) and
replace placeholders in the URL consistently with the existing substitution
implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/AsyncApiParserUtil.java`:
- Around line 981-1004: The getEndpointUrlFromAsyncApiDefinition method only
reads a "url" field and ignores AsyncAPI 3.0 servers that define host/pathname
and server variables, and it doesn't substitute {basePath} variables; update
getEndpointUrlFromAsyncApiDefinition to: parse the first server object (same
iteration style used in setAsyncApiServer), if server.has("url") return it, else
if server.has("host") or server.has("hostname") build the URL by concatenating
host (or hostname) + optional port + pathname (ensuring pathname has leading
'/'); if the resulting string or the original "url" contains placeholders like
{basePath}, look up server.get("variables") for a "basePath" (or corresponding
variable) and substitute its "default" value into the URL; keep error handling
as-is and return null only if no URL can be resolved. Reference:
getEndpointUrlFromAsyncApiDefinition and setAsyncApiServer.
🧹 Nitpick comments (1)
components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/AsyncApiParserUtil.java (1)
991-997: Minor: Inconsistent indentation and stray comment numbering.The code block has inconsistent indentation and a leftover numbered comment (
// 4.) that appears to be from development notes.✨ Proposed cleanup
if (elements.hasNext()) { - // Get the first server defined (e.g., 'production') JsonNode server = elements.next(); - - // 4. Directly extract the URL + // Extract the URL from the first server if (server.has("url")) { return server.get("url").asText(); } - } + }
Comment on lines
981
to
1004
| public static String getEndpointUrlFromAsyncApiDefinition(String apiDefinition) { | ||
| try { | ||
| if (apiDefinition == null || apiDefinition.trim().isEmpty()) { | ||
| return null; | ||
| } | ||
| JsonNode rootNode = new ObjectMapper().readTree(apiDefinition); | ||
| if (rootNode.has("servers")) { | ||
| JsonNode serversNode = rootNode.get("servers"); | ||
| Iterator<JsonNode> elements = serversNode.elements(); | ||
| if (elements.hasNext()) { | ||
| // Get the first server defined (e.g., 'production') | ||
| JsonNode server = elements.next(); | ||
|
|
||
| // 4. Directly extract the URL | ||
| if (server.has("url")) { | ||
| return server.get("url").asText(); | ||
| } | ||
| } | ||
| } | ||
| } catch (IOException e) { | ||
| log.error("Error while parsing definition to extract URL", e); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
AsyncAPI 3.0 server structure not supported.
This method only checks for a "url" field in the server object. However, AsyncAPI 3.0 uses host and pathname fields instead (as evidenced by the setAsyncApiServer method at lines 844-874). The method will return null for AsyncAPI 3.0 definitions even when valid servers are defined.
Additionally, the method does not handle {basePath} variable substitution that may be present in AsyncAPI server URLs.
🔧 Proposed fix to support AsyncAPI 3.0 and basePath substitution
public static String getEndpointUrlFromAsyncApiDefinition(String apiDefinition) {
try {
if (apiDefinition == null || apiDefinition.trim().isEmpty()) {
return null;
}
JsonNode rootNode = new ObjectMapper().readTree(apiDefinition);
if (rootNode.has("servers")) {
JsonNode serversNode = rootNode.get("servers");
Iterator<JsonNode> elements = serversNode.elements();
if (elements.hasNext()) {
- // Get the first server defined (e.g., 'production')
JsonNode server = elements.next();
-
- // 4. Directly extract the URL
- if (server.has("url")) {
- return server.get("url").asText();
+ String resolvedUrl = null;
+
+ // AsyncAPI 2.x uses "url" field
+ if (server.has("url")) {
+ resolvedUrl = server.get("url").asText();
+ }
+ // AsyncAPI 3.0 uses "host" and "pathname" fields
+ else if (server.has("host")) {
+ String host = server.get("host").asText();
+ String pathname = server.has("pathname") ? server.get("pathname").asText() : "/";
+ String protocol = server.has("protocol") ? server.get("protocol").asText() : "";
+ resolvedUrl = protocol.isEmpty() ? host + pathname : protocol + "://" + host + pathname;
}
- }
+
+ // Handle basePath variable substitution
+ if (resolvedUrl != null && server.has("variables") && server.get("variables").has("basePath")) {
+ JsonNode basePath = server.get("variables").get("basePath");
+ if (basePath.has("default")) {
+ String basePathValue = basePath.get("default").asText();
+ resolvedUrl = resolvedUrl
+ .replace("/{basePath}", "/" + basePathValue)
+ .replace("{basePath}", basePathValue);
+ }
+ }
+ return resolvedUrl;
+ }
}
} catch (IOException e) {
log.error("Error while parsing definition to extract URL", e);
}
return null;
}🤖 Prompt for AI Agents
In
`@components/apimgt/org.wso2.carbon.apimgt.spec.parser/src/main/java/org/wso2/carbon/apimgt/spec/parser/definitions/AsyncApiParserUtil.java`
around lines 981 - 1004, The getEndpointUrlFromAsyncApiDefinition method only
reads a "url" field and ignores AsyncAPI 3.0 servers that define host/pathname
and server variables, and it doesn't substitute {basePath} variables; update
getEndpointUrlFromAsyncApiDefinition to: parse the first server object (same
iteration style used in setAsyncApiServer), if server.has("url") return it, else
if server.has("host") or server.has("hostname") build the URL by concatenating
host (or hostname) + optional port + pathname (ensuring pathname has leading
'/'); if the resulting string or the original "url" contains placeholders like
{basePath}, look up server.get("variables") for a "basePath" (or corresponding
variable) and substitute its "default" value into the URL; keep error handling
as-is and return null only if no URL can be resolved. Reference:
getEndpointUrlFromAsyncApiDefinition and setAsyncApiServer.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/FederatedAPIBuilder.java`:
- Around line 93-99: Update the Javadoc on
FederatedAPIBuilder#getContextTemplate to clearly state it returns the context
template (may include placeholders like {version}) rather than the concrete
context/path; change the description line to something like "Returns the API
context template (may include placeholders such as {version})" and keep the
`@param` and `@return` tags consistent with that wording so reviewers can
distinguish this from getContext.
In
`@components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/FederatedBuilderFactory.java`:
- Around line 46-76: The Javadoc and control flow are inconsistent: create a
private helper method (e.g., findBuilder or lookupBuilder) in
FederatedBuilderFactory that iterates the builders collection and returns the
first FederatedAPIBuilder<T> that canHandle(sourceApi) or null if none found;
change getBuilder(T sourceApi) to call that helper and throw the
IllegalStateException only when the helper returns null (and update the `@return`
Javadoc to match), and change isSupported(T sourceApi) to call the same helper
and return (helper(...) != null) instead of relying on exception-based flow.
🧹 Nitpick comments (4)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/FederatedAPIBuilder.java (2)
56-58: Nullenvis handled forgatewayTypebut still passed tomapSpecificDetails.If
envcan legitimately benull, subclasses inmapSpecificDetailsmust all independently guard against it. If it cannot benull, it would be cleaner to fail fast here (or document the contract). Currently the partial null check is inconsistent.
61-61: Use constantAPIConstants.EXTERNAL_GATEWAY_VENDORinstead of hardcoded string.The constant is already defined in
APIConstants.java(line 3557) alongside other gateway vendor constants (WSO2_GATEWAY_ENVIRONMENT,WSO2_APK_GATEWAY,WSO2_SYNAPSE_GATEWAY). Replace the magic string with the constant for consistency and discoverability.components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/FederatedBuilderFactory.java (2)
84-99:registerBuilderdedup loop can be simplified.The manual boolean flag + loop is verbose. A stream-based or early-return approach is more idiomatic and readable.
♻️ Proposed simplification
protected void registerBuilder(FederatedAPIBuilder<T> builder) { - if (builder != null) { - // 1. Check if ANY builder in the list has the same CLASS as the new one - boolean alreadyExists = false; - for (FederatedAPIBuilder<T> existing : builders) { - if (existing.getClass().equals(builder.getClass())) { - alreadyExists = true; - break; - } - } - - if (!alreadyExists) { - builders.add(builder); - } + if (builder == null) { + return; } + boolean alreadyExists = builders.stream() + .anyMatch(existing -> existing.getClass().equals(builder.getClass())); + if (!alreadyExists) { + builders.add(builder); + } }
54-62: No null guard onsourceApiparameter ingetBuilder.If
sourceApiisnull, each builder'scanHandle(null)will be called, which may throw an unexpectedNullPointerExceptionfrom within a builder implementation. A precondition check would provide a clearer error message.🛡️ Proposed null guard
public FederatedAPIBuilder<T> getBuilder(T sourceApi) { + if (sourceApi == null) { + throw new IllegalArgumentException("sourceApi must not be null"); + } for (FederatedAPIBuilder<T> builder : builders) {
Comment on lines
93
to
99
| /** | ||
| * Extracts the context/path of the API from the raw data. | ||
| * | ||
| * @param sourceApi The raw data object. | ||
| * @return The API context template. | ||
| */ | ||
| protected abstract String getContextTemplate(T sourceApi); |
There was a problem hiding this comment.
Javadoc for getContextTemplate is a copy-paste of getContext.
The @return tag says "The API context template" (correct), but the method description still reads "Extracts the context/path of the API" — same as getContext. Clarify that this returns the template (e.g., with {version} placeholder).
📝 Proposed Javadoc fix
/**
- * Extracts the context/path of the API from the raw data.
+ * Extracts the context template of the API from the raw data (e.g., context with version placeholder).
*
* `@param` sourceApi The raw data object.
* `@return` The API context template.
*/🤖 Prompt for AI Agents
In
`@components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/FederatedAPIBuilder.java`
around lines 93 - 99, Update the Javadoc on
FederatedAPIBuilder#getContextTemplate to clearly state it returns the context
template (may include placeholders like {version}) rather than the concrete
context/path; change the description line to something like "Returns the API
context template (may include placeholders such as {version})" and keep the
`@param` and `@return` tags consistent with that wording so reviewers can
distinguish this from getContext.
Comment on lines
46
to
76
| /** | ||
| * Gets the appropriate builder for the given raw API data. | ||
| * Uses the Strategy pattern - iterates through registered builders | ||
| * and returns the first one that can handle the API type. | ||
| * | ||
| * @param sourceApi The raw API data from the gateway | ||
| * @return The builder that can handle this API type, or null if unsupported | ||
| */ | ||
| public FederatedAPIBuilder<T> getBuilder(T sourceApi) { | ||
| for (FederatedAPIBuilder<T> builder : builders) { | ||
| if (builder.canHandle(sourceApi)) { | ||
| return builder; | ||
| } | ||
| } | ||
| throw new IllegalStateException( | ||
| "No registered builder can handle the given API data"); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the given API type is supported by any registered builder. | ||
| * | ||
| * @param sourceApi The raw API data from the gateway | ||
| * @return true if a builder can handle this API type, false otherwise | ||
| */ | ||
| public boolean isSupported(T sourceApi) { | ||
| try { | ||
| return getBuilder(sourceApi) != null; | ||
| } catch (IllegalStateException e) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Javadoc says getBuilder returns null for unsupported types — it actually throws IllegalStateException.
The @return on line 52 says "or null if unsupported", but the method never returns null; it throws on line 60. This misleads callers.
Additionally, isSupported (lines 70-76) uses exception-based control flow to wrap getBuilder, which is an anti-pattern for a normal expected condition (no builder found). Extract the lookup into a private helper that returns null, and have both methods delegate to it.
♻️ Proposed refactor
+ private FederatedAPIBuilder<T> findBuilder(T sourceApi) {
+ for (FederatedAPIBuilder<T> builder : builders) {
+ if (builder.canHandle(sourceApi)) {
+ return builder;
+ }
+ }
+ return null;
+ }
+
public FederatedAPIBuilder<T> getBuilder(T sourceApi) {
- for (FederatedAPIBuilder<T> builder : builders) {
- if (builder.canHandle(sourceApi)) {
- return builder;
- }
+ FederatedAPIBuilder<T> builder = findBuilder(sourceApi);
+ if (builder != null) {
+ return builder;
}
throw new IllegalStateException(
- "No registered builder can handle the given API data");
+ "No registered builder can handle the given API data");
}
+ /**
+ * {`@inheritDoc`}
+ */
public boolean isSupported(T sourceApi) {
- try {
- return getBuilder(sourceApi) != null;
- } catch (IllegalStateException e) {
- return false;
- }
+ return findBuilder(sourceApi) != null;
}📝 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.
Suggested change
| /** | |
| * Gets the appropriate builder for the given raw API data. | |
| * Uses the Strategy pattern - iterates through registered builders | |
| * and returns the first one that can handle the API type. | |
| * | |
| * @param sourceApi The raw API data from the gateway | |
| * @return The builder that can handle this API type, or null if unsupported | |
| */ | |
| public FederatedAPIBuilder<T> getBuilder(T sourceApi) { | |
| for (FederatedAPIBuilder<T> builder : builders) { | |
| if (builder.canHandle(sourceApi)) { | |
| return builder; | |
| } | |
| } | |
| throw new IllegalStateException( | |
| "No registered builder can handle the given API data"); | |
| } | |
| /** | |
| * Checks if the given API type is supported by any registered builder. | |
| * | |
| * @param sourceApi The raw API data from the gateway | |
| * @return true if a builder can handle this API type, false otherwise | |
| */ | |
| public boolean isSupported(T sourceApi) { | |
| try { | |
| return getBuilder(sourceApi) != null; | |
| } catch (IllegalStateException e) { | |
| return false; | |
| } | |
| } | |
| /** | |
| * Gets the appropriate builder for the given raw API data. | |
| * Uses the Strategy pattern - iterates through registered builders | |
| * and returns the first one that can handle the API type. | |
| * | |
| * `@param` sourceApi The raw API data from the gateway | |
| * `@return` The builder that can handle this API type, or null if unsupported | |
| */ | |
| private FederatedAPIBuilder<T> findBuilder(T sourceApi) { | |
| for (FederatedAPIBuilder<T> builder : builders) { | |
| if (builder.canHandle(sourceApi)) { | |
| return builder; | |
| } | |
| } | |
| return null; | |
| } | |
| /** | |
| * Gets the appropriate builder for the given raw API data. | |
| * Uses the Strategy pattern - iterates through registered builders | |
| * and returns the first one that can handle the API type. | |
| * | |
| * `@param` sourceApi The raw API data from the gateway | |
| * `@return` The builder that can handle this API type, or null if unsupported | |
| */ | |
| public FederatedAPIBuilder<T> getBuilder(T sourceApi) { | |
| FederatedAPIBuilder<T> builder = findBuilder(sourceApi); | |
| if (builder != null) { | |
| return builder; | |
| } | |
| throw new IllegalStateException( | |
| "No registered builder can handle the given API data"); | |
| } | |
| /** | |
| * {`@inheritDoc`} | |
| */ | |
| public boolean isSupported(T sourceApi) { | |
| return findBuilder(sourceApi) != null; | |
| } |
🤖 Prompt for AI Agents
In
`@components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/FederatedBuilderFactory.java`
around lines 46 - 76, The Javadoc and control flow are inconsistent: create a
private helper method (e.g., findBuilder or lookupBuilder) in
FederatedBuilderFactory that iterates the builders collection and returns the
first FederatedAPIBuilder<T> that canHandle(sourceApi) or null if none found;
change getBuilder(T sourceApi) to call that helper and throw the
IllegalStateException only when the helper returns null (and update the `@return`
Javadoc to match), and change isSupported(T sourceApi) to call the same helper
and return (helper(...) != null) instead of relying on exception-based flow.
Tharsanan1
reviewed
Feb 12, 2026
...rg.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIDefinitionHandler.java
Outdated
Show resolved
Hide resolved
JaninduRSD
changed the title
… updateAPIDefinitionWithVersion method
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
3 participants
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.
No description provided.