Intra-vendor model routing feature enhancements #12987
Conversation
…ds under metadata
…ORGANIZATION in AM_API_ENDPOINTS
… have a primary endpoint id
ashera96
requested review from
tharindu1st,
AnuGayan,
chamilaadhi,
Arshardh,
dushaniw and
tgtshanika
as code owners
WalkthroughThe changes introduce extensive refinements across API management components. Documentation updates improve JavaDoc clarity, while constants and metadata for AI endpoints have been reorganized. New DTOs and classes (e.g., SimplifiedEndpoint) enhance endpoint configuration, along with added failover and mediation logic. SQL schema updates add unique constraints for better data integrity. Additionally, export/import API methods now support credential preservation and improved encryption. These modifications span API gateway utilities, template builders, DAO methods, and configuration scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AIAPIMediator
participant FailoverMediator
participant GatewayUtils
Client->>AIAPIMediator: Send API request
AIAPIMediator->>AIAPIMediator: processInboundRequest()
AIAPIMediator->>AIAPIMediator: Extract metadata & check failover
AIAPIMediator->>FailoverMediator: Delegate failover decision
FailoverMediator-->>AIAPIMediator: Return failover outcome
AIAPIMediator->>AIAPIMediator: processOutboundResponse()
AIAPIMediator->>Client: Return processed API response
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/OpenAiLLMProviderService.java (1)
102-102: 🛠️ Refactor suggestionImprove error message with root cause.
The current error message loses the root cause, making debugging harder. Consider including the original exception details.
Apply this change:
- throw new APIManagementException("Error occurred when registering LLM Provider:" + this.getType()); + throw new APIManagementException("Error occurred when registering LLM Provider: " + this.getType(), e);
🧹 Nitpick comments (79)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/ExceptionCodes.java (2)
795-798: Consider consolidating error messages.The error messages for
ERROR_ADDING_API_ENDPOINTare slightly inconsistent:
- Message: "Failed to add endpoint to API."
- Description: "Error while adding API endpoint."
Consider using consistent wording between message and description.
- ERROR_ADDING_API_ENDPOINT(902043, "Failed to add endpoint to API.", 500, - "Error while adding API endpoint."), + ERROR_ADDING_API_ENDPOINT(902043, "Failed to add endpoint to API.", 500, + "Failed to add endpoint to API."),
799-802: Consider removing redundant error code.
ERROR_ADDING_API_ENDPOINTS(902046) appears to be redundant withERROR_ADDING_API_ENDPOINT(902043) as they serve the same purpose with very similar messages.Consider removing
ERROR_ADDING_API_ENDPOINTSand usingERROR_ADDING_API_ENDPOINTconsistently throughout the codebase.components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/RBPolicyConfigDTO.java (4)
2-2: Update the copyright year.The copyright year is set to 2025, which is in the future. Please update it to the current year (2024).
- * Copyright (c) 2025, WSO2 LLC (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2024, WSO2 LLC (http://www.wso2.org) All Rights Reserved.
23-26: Enhance class-level documentation.The current documentation is brief. Consider adding:
- Purpose and usage of each field
- Relationship with ModelEndpointDTO
- Example usage in the context of routing and balancing
/** - * DTO representing the configuration for RB (Routing and Balancing) policies in API Gateway. + * Data Transfer Object (DTO) representing the configuration for Routing and Balancing (RB) policies in API Gateway. + * This class manages two sets of endpoints (production and sandbox) and their suspension duration when failures occur. + * + * The production and sandbox endpoints are represented as lists of {@link ModelEndpointDTO}, + * allowing for flexible endpoint configuration in different environments. + * + * The suspend duration specifies how long (in milliseconds) a failed endpoint should be suspended + * before attempting to route traffic to it again. */
28-30: Initialize collections and add validation constraints.Consider the following improvements:
- Initialize lists to prevent null pointer exceptions
- Add validation constraints for suspendDuration (e.g., @min for minimum valid duration)
+import javax.validation.constraints.Min; + public class RBPolicyConfigDTO { - private List<ModelEndpointDTO> production; - private List<ModelEndpointDTO> sandbox; - private Long suspendDuration; + private List<ModelEndpointDTO> production = new ArrayList<>(); + private List<ModelEndpointDTO> sandbox = new ArrayList<>(); + @Min(value = 0, message = "Suspend duration cannot be negative") + private Long suspendDuration = 0L;
37-40: Remove unnecessary empty lines in methods.For better code style consistency, remove the empty lines between method body and brackets in all getter and setter methods.
public List<ModelEndpointDTO> getProduction() { - return production; } public void setProduction(List<ModelEndpointDTO> production) { - this.production = production; } public List<ModelEndpointDTO> getSandbox() { - return sandbox; } public Long getSuspendDuration() { - return suspendDuration; } public void setSuspendDuration(Long suspendDuration) { - this.suspendDuration = suspendDuration; }Also applies to: 47-50, 57-60, 77-80, 87-90
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/ModelEndpointDTO.java (4)
2-2: Update the copyright year.The copyright year 2025 is incorrect as it's in the future. Please update it to the current year 2024.
- * Copyright (c) 2025, WSO2 LLC (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2024, WSO2 LLC (http://www.wso2.org) All Rights Reserved.
21-23: Enhance class-level JavaDoc.Given this class's role in intra-vendor model routing, the JavaDoc could be more descriptive about its purpose and usage.
/** - * DTO representing a model endpoint in API Gateway. + * Data Transfer Object (DTO) representing a model endpoint in API Gateway. + * This class is used for configuring and managing routing between different model endpoints + * within the same vendor in the API Gateway. It encapsulates model information, endpoint + * identification, and routing weight for load balancing purposes. */
26-28: Add field-level documentation and validation.Consider adding field-level JavaDoc and validation annotations to ensure data integrity.
+ /** + * The name or identifier of the AI/ML model. + */ + @NotNull(message = "Model name cannot be null") private String model; + /** + * The unique identifier of the endpoint where the model is deployed. + */ + @NotNull(message = "Endpoint ID cannot be null") private String endpointId; + /** + * The routing weight for load balancing (between 0.0 and 1.0). + */ + @DecimalMin(value = "0.0", message = "Weight must be at least 0.0") + @DecimalMax(value = "1.0", message = "Weight must not exceed 1.0") private double weight;
45-48: Add validation to setters and implement standard Object methods.
- Add validation to setters to ensure data integrity
- Implement equals, hashCode, and toString methods for proper object comparison and debugging
public void setModel(String model) { + if (model == null || model.trim().isEmpty()) { + throw new IllegalArgumentException("Model name cannot be null or empty"); + } this.model = model; } public void setEndpointId(String endpointId) { + if (endpointId == null || endpointId.trim().isEmpty()) { + throw new IllegalArgumentException("Endpoint ID cannot be null or empty"); + } this.endpointId = endpointId; } public void setWeight(double weight) { + if (weight < 0.0 || weight > 1.0) { + throw new IllegalArgumentException("Weight must be between 0.0 and 1.0"); + } this.weight = weight; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ModelEndpointDTO that = (ModelEndpointDTO) o; + return Double.compare(that.weight, weight) == 0 && + Objects.equals(model, that.model) && + Objects.equals(endpointId, that.endpointId); + } + + @Override + public int hashCode() { + return Objects.hash(model, endpointId, weight); + } + + @Override + public String toString() { + return "ModelEndpointDTO{" + + "model='" + model + '\'' + + ", endpointId='" + endpointId + '\'' + + ", weight=" + weight + + '}'; + }Also applies to: 65-68, 85-88
components/apimgt/org.wso2.carbon.apimgt.governance.impl/src/main/java/org/wso2/carbon/apimgt/governance/impl/APIGovernanceHandler.java (1)
66-618: Consider enhancing error handling and logging.While the implementation is solid, consider these improvements:
- Add debug logging for better traceability during API export operations.
- Consider implementing retry mechanisms for transient failures during file operations.
- Add metrics collection for monitoring governance operations.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/LLMProviderMetadata.java (1)
39-44: Enhance deprecation documentation.While the deprecation notice is clear, consider adding information about the default behavior of the
requiredfield when using this constructor./** * @deprecated This constructor is deprecated. Use {@link #LLMProviderMetadata(String, String, String, boolean)} - * instead. + * instead. When using this constructor, the 'required' field defaults to true. */components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/AzureOpenAiLLMProviderService.java (2)
68-74: Document the purpose of boolean flags in model metadata.The separation of model metadata into request and response entries with different boolean flags needs documentation to explain the significance of these flags and their impact on the system's behavior.
Add JavaDoc comments explaining the purpose of the boolean flags:
llmProviderMetadata.add(new LLMProviderMetadata( APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REQUEST_MODEL, APIConstants.AIAPIConstants.INPUT_SOURCE_PAYLOAD, - APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_MODEL, false)); + APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_MODEL, false)); // false indicates required in request llmProviderMetadata.add(new LLMProviderMetadata( APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODEL, APIConstants.AIAPIConstants.INPUT_SOURCE_PAYLOAD, - APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_MODEL, true)); + APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_MODEL, true)); // true indicates expected in response
93-98: Consider externalizing the model list configuration.Hardcoding the model list reduces flexibility and makes updates difficult. Consider:
- Moving the model list to a configuration file
- Adding model capabilities documentation
- Implementing model existence validation
Example configuration approach:
- // Set default model List - List<String> modelList = new ArrayList<>(); - modelList.add("gpt-4o"); - modelList.add("gpt-4o-mini"); - modelList.add("o3-mini"); + // Load models from configuration + List<String> modelList = ConfigurationManager.getInstance() + .getAzureOpenAIModels(); + if (modelList.isEmpty()) { + log.warn("No models configured for Azure OpenAI. Using defaults."); + modelList = Arrays.asList("gpt-4o", "gpt-4o-mini", "o3-mini"); + }components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/ImportExportAPIServiceImpl.java (1)
126-127: LGTM! Consider updating documentation.The parameter is correctly passed through to ExportUtils, maintaining consistency with the other overloaded method.
Consider updating the method's JavaDoc to document the behavior of the
preserveCredentialsparameter:+ /** + * Exports an API revision. + * @param preserveCredentials If true, preserves the credentials during export + */components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/OpenAiLLMProviderService.java (1)
92-97: Consider externalizing model list configuration.The hardcoded model list could be better managed through external configuration to:
- Allow easier updates without code changes
- Support different environments/deployments
- Enable validation of model names
Consider this approach:
- // Set default model List - List<String> modelList = new ArrayList<>(); - modelList.add("gpt-4o"); - modelList.add("gpt-4o-mini"); - modelList.add("o3-mini"); + List<String> modelList = ConfigurationManager.getInstance() + .getOpenAIModelList(); + if (modelList.isEmpty()) { + // Fallback to default models if configuration is missing + modelList = Arrays.asList("gpt-4o", "gpt-4o-mini", "o3-mini"); + }components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/MistralAiLLMProviderService.java (1)
67-73: Consider adding validation for model identifiers.The metadata entries for request and response models should include validation to ensure the model identifiers match the available models in the
modelList.Consider adding validation like this:
llmProviderMetadata.add(new LLMProviderMetadata( APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REQUEST_MODEL, APIConstants.AIAPIConstants.INPUT_SOURCE_PAYLOAD, - APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_MODEL, false)); + APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_MODEL, false) { + @Override + public void validate(String value) throws APIManagementException { + if (!modelList.contains(value)) { + throw new APIManagementException("Invalid model identifier: " + value); + } + } +});components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIConstants.java (4)
47-47: Consider adding documentation for theOM_ELEMENT_NAMEconstant.While the constant name is self-explanatory, adding a JavaDoc comment would help clarify its purpose and usage context.
+ /** + * Represents the name attribute in OM (Object Model) elements. + */ public static final String OM_ELEMENT_NAME = "name";
50-50: Consider moving time-related constants to a dedicated TimeConstants class.The
MILLISECONDS_IN_SECONDconstant is a general utility constant that might be better placed in a dedicated time-related constants class for better organization and reusability.
114-119: Group request-related constants using inner class.Consider grouping all request-related constants (
REQUEST_*) into a dedicated inner class for better organization and maintainability.+ public static class RequestConstants { + public static final String PAYLOAD = "REQUEST_PAYLOAD"; + public static final String HEADERS = "REQUEST_HEADERS"; + public static final String HTTP_METHOD = "REQUEST_HTTP_METHOD"; + public static final String REST_URL_POSTFIX = "REQUEST_REST_URL_POSTFIX"; + public static final String MODEL = "REQUEST_MODEL"; + }
120-122: Consider using an enum for endpoint types.The endpoint name constants could be better represented as an enum to provide type safety and prevent invalid endpoint type usage.
+ public enum EndpointType { + DEFAULT_PRODUCTION("DEFAULT PRODUCTION ENDPOINT"), + DEFAULT_SANDBOX("DEFAULT SANDBOX ENDPOINT"); + + private final String value; + + EndpointType(String value) { + this.value = value; + } + + public String getValue() { + return value; + } + }components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/FailoverMediator.java (1)
61-64: Document empty destroy methodThe empty
destroy()method should be documented to explain why no cleanup is needed.Add documentation:
/** * Destroys the mediator. This implementation does not require specific cleanup. + * No cleanup is needed as the mediator does not maintain any stateful resources + * that need to be released. */ @Override public void destroy() { }components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/template/APITemplateBuilderImpl.java (3)
357-402: New method to generate endpoint configuration strings.
This method appears clear and consistent with existing Velocity-based approaches. However, consider:
- Validating that
api.getAiConfiguration()is always non-null before referencing.- Logging or handling empty/invalid endpoint lists more explicitly.
- Adding corresponding unit tests.
404-425: Refactor suggestion to reduce duplication.
createContextForEndpointsrepeats a similar pattern of initializing multipleConfigContextwrappers. Consider whether any logic can be shared withcreateAIAPIConfigContextto reduce duplication.
478-492:createAIAPIConfigContextmethod introduction.
This dedicated AI configuration context method is useful but sees some overlap withcreateContextForEndpoints(). Consider unifying common logic if feasible, or documenting why they must differ.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/ApisApiServiceImpl.java (3)
301-302: Use a more self-descriptive indicator for the boolean parameter.Currently, the
falseargument passed toAPIMappingUtil.fromAPIEndpointToDTOlacks clarity. Consider replacing it with a more expressive approach (e.g., an enum or a clearly named parameter) for better readability and maintainability.
333-362: Refactor repeated primary-readonly endpoint checks.The code checks for primary and read-only endpoints in two separate blocks. Consider unifying these checks into a helper method or a single conditional block to avoid duplication and improve clarity.
393-401: Repetitive read-only check logic across delete and update methods.The read-only check for primary endpoints here almost duplicates the logic seen in the delete flow (lines 333–362). Extracting common logic into a shared utility can help reduce maintenance overhead and ensure consistent behavior.
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/GatewayUtils.java (3)
1755-1759: Clarify the Javadoc to match the actual parameters.
The doc references “policy configuration,” but the method signature only accepts a list of endpoints. Consider updating the description for better accuracy:- * Retrieves available endpoints for the given policy configuration. + * Filters active endpoints from the provided list, based on suspension status.
1775-1775: Avoid returning null in lieu of an empty list.
Returningnullcan lead to potential NullPointerExceptions. Prefer returning an empty list to simplify calling code.- return activeEndpoints.isEmpty() ? null : activeEndpoints; + return activeEndpoints;
1802-1810: Parameter validation enhancements are good.
These checks prevent invalid endpoint objects from causing errors downstream. Optionally consider logging argument issues for better debugging.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ExportUtils.java (2)
165-173: Clarify security implications in method documentation.
While the newly documentedpreserveCredentialsparameter is clear about preserving credentials on export, it may be prudent to expand the docstring to warn about the potential security risks of shipping plain-text credentials in an exported archive. If possible, specify the recommended usage or indicate any encryption measures.
815-823: Documentation updates are consistent but could note storage format.
The description accurately reflects the introduction of thepreserveCredentialsparameter. Consider adding details about whether the credentials will be in plain text or some protected format in the exported files, so users can make informed decisions.components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/RoundRobinMediator.java (3)
72-80: Improve error logging and partial config visibility
Consider including the parsed configuration snippet in the error log to facilitate troubleshooting. Be mindful of any sensitive information that might appear in logs.Possible diff:
- log.error("Failed to parse weighted round robin configuration", e); + log.error("Failed to parse weighted round robin configuration: " + roundRobinConfigs, e);
82-93: Handle unknown or null API key types
Currently, ifapiKeyTypeis null or an unsupported value, the mediator defaults to sandbox endpoints silently. Consider adding a fallback or explicit notification for unknown types to avoid unexpected routing behavior.
94-101: Validate negative or zero suspend duration
You multiply the configured suspend duration byMILLISECONDS_IN_SECOND. Consider verifying whether the original suspend duration is non-negative to avoid unexpected behavior.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java (2)
720-735: Consider more robust error-handling for deletions.Currently, if the loop encounters a deletion error, it immediately throws an exception, preventing further deletions and partially leaving endpoints in place. You might consider a more transactional or partial-failure approach, where you either:
- Attempt all deletions while logging/collecting errors per endpoint, or
- Roll back all deletions on failure (if your backing store supports that),
to avoid inconsistent states.
2499-2535: Add a null check for the'data'field to avoid potential NPE.When parsing endpoint configurations, the code assumes
'data'is always present and is a JSON array. If the file is malformed or lacks that field, aNullPointerExceptionmay occur. Consider adding a check before accessingendpointsElement:+ JsonElement endpointsElement = new JsonParser().parse(jsonContent).getAsJsonObject().get(APIConstants.DATA); + if (endpointsElement == null || !endpointsElement.isJsonArray()) { + log.debug("Missing or invalid 'data' field in endpoints file."); + return new ArrayList<>(); + }components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/WeightedRoundRobinMediator.java (2)
72-80: Improve error logging for easier diagnosis
Should JSON parsing fail, consider logging the invalid snippet or relevant context to aid troubleshooting.
82-88: Handle unrecognized API key types explicitly
IfapiKeyTypeis neither production nor sandbox, you may want to log a warning or provide a fallback to avoid unexpected behavior.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 (2)
3378-3385: Consider renaming for clarity.The overloaded method signature for
handleEndpointSecurity(JSONObject, String, boolean)duplicates logic from the previous block. Although it’s functionally correct, a more explicit name (e.g.,handleEndpointSecurityForTenant) might improve readability and reduce confusion.
3424-3451: Partial decryption may cause incomplete state.The decryption logic correctly attempts to restore the API key from encrypted data, but any
CryptoExceptionis logged and not re-thrown. This could silently leave credentials in an incomplete or inconsistent state. Consider re-throwing or returning an error status if decryption fails, to avoid potential confusion or partial data.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/template/EndpointsContext.java (5)
33-34: Consider immutability for the new fields
If these fields are not intended to change after construction, making themfinalcan improve clarity and thread-safety.- private List<SimplifiedEndpoint> endpoints; - private SimplifiedEndpoint defaultEndpoint; + private final List<SimplifiedEndpoint> endpoints; + private final SimplifiedEndpoint defaultEndpoint;
39-44: Add missing Javadoc fordefaultEndpoint
The Javadoc block referencescontext,api, andendpointsbut does not include a@paramfordefaultEndpoint./** * Constructs an EndpointsContext instance for an API. * * @param context The base configuration context * @param api The API associated with the endpoints * @param endpoints The list of endpoints + * @param defaultEndpoint The default endpoint */
74-75: Optional null-handling
Currently,endpointsanddefaultEndpointare simply put into the context. Consider using empty placeholders forendpointsif null, which can simplify template usage.if (this.endpoints != null) { context.put("endpoints", endpoints); } context.put("defaultEndpoint", defaultEndpoint); +// Alternatively: +context.put("endpoints", endpoints != null ? endpoints : Collections.emptyList()); +context.put("defaultEndpoint", defaultEndpoint);Also applies to: 77-77
33-34: Good introduction of new fields.
DefiningendpointsanddefaultEndpointas class members enhances clarity. Make sure to validate them if null-safety is critical.
74-75: VelocityContext population logic.
StoringendpointsanddefaultEndpointin the context is straightforward. Consider adding null checks or fallback logic fordefaultEndpointif it is optional or not guaranteed to be set.Also applies to: 77-77
components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/TemplateBuilderUtil.java (4)
639-662: Logic foraddEndpointsFromConfig
Parses the endpoint config via Gson and adds production/sandbox endpoints if present. Consider handling any potential JSON parsing errors with a try-catch to avoid runtime exceptions if malformed JSON is encountered.
639-662: Clear, concise helper method for adding endpoints from config.
This method properly checks and adds production/sandbox endpoints to the given list. You might consider logging or throwing if neither endpoint config is found (rather than silently returning) to assist debugging.
997-1025: AI API handling block.
This conditional block constructs the default production and sandbox endpoints usingOptionalchecks. Be certain that endpoints not found in the list do not cause confusion downstream. Overall logic appears consistent with the newSimplifiedEndpointapproach.
1318-1318: Null arguments in endpoint configuration.
CallinggetConfigStringEndpointConfigTemplate(type, null, null)might ignore potential endpoint parameters. If certain endpoints require a validendpointUuidorendpointConfig, add safety checks or fallback logic.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java (3)
24505-24526: LGTM! Well-structured method with proper transaction management.The implementation follows best practices with proper transaction handling and error management.
Consider adding debug logging before the delete operation to track the number of endpoints being deleted:
public void deleteAPIEndpointsByApiUUID(String apiUUID) throws APIManagementException { try (Connection connection = APIMgtDBUtil.getConnection()) { connection.setAutoCommit(false); try (PreparedStatement statement = connection.prepareStatement( SQLConstants.APIEndpointsSQLConstants.DELETE_API_ENDPOINTS_BY_API_UUID)) { + if (log.isDebugEnabled()) { + log.debug("Deleting endpoints for API: " + apiUUID); + } statement.setString(1, apiUUID); statement.executeUpdate();
24661-24668: Consider batch deletion for better performance.The implementation correctly handles transactions but could benefit from batch processing when dealing with multiple mappings.
Consider modifying the implementation to use batch updates:
public void deleteAPIPrimaryEndpointMappings(String apiUUID) throws APIManagementException { try (Connection connection = APIMgtDBUtil.getConnection()) { connection.setAutoCommit(false); - deleteExistingAPIPrimaryEndpointsMapping(connection, apiUUID); + try (PreparedStatement statement = connection.prepareStatement( + SQLConstants.APIEndpointsSQLConstants.DELETE_PRIMARY_ENDPOINT_MAPPING)) { + statement.setString(1, apiUUID); + statement.addBatch(); + statement.executeBatch(); + connection.commit(); + } catch (SQLException e) { + connection.rollback(); + throw e; + } } catch (SQLException e) { handleException("Error while deleting primary endpoint mappings of API : " + apiUUID, e); } }
24757-24772: Add pagination support for large APIs.The method retrieves all endpoint UUIDs without pagination, which could be problematic for APIs with many endpoints.
Consider adding pagination support:
- public List<String> getPrimaryEndpointUUIDByAPIId(String apiUUID) throws APIManagementException { + public List<String> getPrimaryEndpointUUIDByAPIId(String apiUUID, int offset, int limit) throws APIManagementException { List<String> endpointIds = new ArrayList<>(); try (Connection connection = APIMgtDBUtil.getConnection()) { - try (PreparedStatement preparedStatement = connection.prepareStatement(SQLConstants.APIEndpointsSQLConstants.GET_PRIMARY_ENDPOINT_MAPPINGS)) { + try (PreparedStatement preparedStatement = connection.prepareStatement( + SQLConstants.APIEndpointsSQLConstants.GET_PRIMARY_ENDPOINT_MAPPINGS + " LIMIT ? OFFSET ?")) { preparedStatement.setString(1, apiUUID); + preparedStatement.setInt(2, limit); + preparedStatement.setInt(3, offset);components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (1)
8214-8241: LGTM! Good handling of default primary endpoints with a minor suggestion.The changes properly handle default primary endpoints set during AI API creation with good null checks and validation. Consider adding debug logging to help with troubleshooting.
if (endpointIds != null && !endpointIds.isEmpty()) { + if (log.isDebugEnabled()) { + log.debug("Processing default primary endpoints for API: " + currentApiUuid); + } for (String endpointId : endpointIds) {components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java (2)
4523-4533: Add null check for API parameterAdd a null check at the start of the method to avoid potential NullPointerException:
public static String getEndpointSequenceName(API api) { + if (api == null) { + throw new IllegalArgumentException("API parameter cannot be null"); + } return api.getId().getApiName() + "--" + api.getId().getVersion(); }
10280-10287: Add null check for endpoints parameterAdd a null check at the start of validateEndpointURLs() to avoid potential NullPointerException:
public static boolean validateEndpointURLs(ArrayList<String> endpoints) { + if (endpoints == null) { + return false; + } for (String endpoint : endpoints) { if (!validateEndpointURL(endpoint)) { return false; } } return true; }features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/h2.sql (1)
1598-1599: Unique Constraint Addition for API EndpointsThe addition of
UNIQUE (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)ensures that each API endpoint configuration is uniquely identified for a given API, endpoint name, revision, and organization. This enhances data integrity by preventing duplicate endpoint configurations.
Please ensure that:
- No pre-existing duplicate records exist that would violate this constraint upon migration.
- Consider whether the
ENDPOINT_NAMEfield should be declaredNOT NULLif its value is critical for uniqueness.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mysql.sql (1)
1537-1538: Unique Constraint Addition VerificationThe new unique constraint on the combination of
(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)strengthens data integrity by preventing duplicate endpoint configurations for a given API and organization. Please verify that this meets the intended business requirements. Also, note that theENDPOINT_NAMEcolumn is not marked asNOT NULL; if the business logic requires every endpoint to have a defined name when enforcing uniqueness, consider enforcing a non-null constraint onENDPOINT_NAME.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/postgresql.sql (1)
1874-1875: Unique Constraint Enhancements on AM_API_ENDPOINTSThe added unique constraint on (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION) enhances data integrity by ensuring that each endpoint configuration is uniquely defined by its API, endpoint name, revision, and organization. This change complements the existing primary key without altering it, thereby enforcing an extra layer of uniqueness critical for avoiding duplicate entries.
Please verify that all related insert operations and any migration routines (if applicable) have been updated to handle this constraint and that the change aligns with similar modifications in other database-specific SQL files.
features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mssql.sql (1)
1755-1756: Unique Constraint Addition on AM_API_ENDPOINTSThe newly added unique constraint on the combination of
(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)is an excellent enhancement for data integrity. This ensures that no two records share the same API endpoint configuration across different organizations and revisions.Recommendation:
It is good practice to explicitly name constraints. Assigning a name to the unique constraint can simplify debugging and maintenance, especially when managing database changes across multiple environments.For example, you might consider a diff similar to:
- UNIQUE (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION) + CONSTRAINT UC_AM_API_ENDPOINTS UNIQUE (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/oracle.sql (1)
2483-2487: Unique Constraint Addition Enhances Data IntegrityThe newly added unique constraint on
(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)enhances data integrity by enforcing that each API endpoint (as identified by the API UUID, optional endpoint name, revision UUID, and organization) remains unique across the table. Please verify that the nullability ofENDPOINT_NAMEis as intended—since it is not declared NOT NULL, Oracle may allow multiple rows with a NULL value in that column, which could bypass the uniqueness check. Also, ensure that any existing data in the table complies with this new constraint before migrating.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 (5)
1007-1051: Consider consolidating API key encryption logic and handling already-encrypted keys.The logic to encrypt and preserve older API keys here is very similar to the preceding utility method for encrypting endpoint security credentials. Re-encoding an already encrypted key may inadvertently lead to double encryption or confusion in some edge cases. You could extract common code to a shared helper to ensure consistent handling, and also detect if the old key is already in encrypted form before trying to re-encrypt.
+ // Example approach to check if old key starts with a known prefix (or matches a pattern) + // before deciding to re-encrypt, to mitigate potential double encryption +if (!alreadyEncrypted(oldApiSecret)) { + // proceed with encryption +}
1066-1212: Refactor overlapping OAuth encryption routines into a shared method.This method for encrypting OAuth credentials duplicates the flow seen in other parts of the code (e.g., lines 901-1005). It might be beneficial to unify the sandbox and production encryption logic into a single helper that handles custom parameters, secret encryption, and old secret preservation consistently across all endpoints. This can reduce duplication and potential inconsistencies.
3366-3393: Clarify fallback behavior when endpoint DB lookup returns null.Here, if the lookup fails in the DB, the method builds endpoint info from the default config. While this is convenient, consider logging or returning a clear indicator that the endpoint is coming from the default fallback instead of the DB. This helps debugging, especially if multiple shared endpoints are expected.
3405-3492: Reduce repetition by extracting "retrieve old secret" logic to a helper.Most of this code is spent on retrieving the old secret from the production/sandbox endpoint security map and reassigning/encrypting it. That pattern is nearly identical to the approach used in other encryption methods. Factor it out to a smaller, reusable function to ensure consistency and readability, and to reduce potential mistakes if future changes are needed.
3505-3551: Endpoint addition flow appears correct; consider user-friendly error messaging.The validation for deployment stages and endpoint URLs is well-handled, and encryption is invoked as needed. As a minor enhancement, you might provide more descriptive error messages or log statements upon a malformed endpoint URL or unexpected environment. This can help in diagnosing failing endpoint additions at runtime.
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/AIAPIMediator.java (9)
120-123: Conditional flow for inbound vs. outbound.
Refactoring intoprocessInboundRequestandprocessOutboundResponsegreatly improves readability. Consider adding a default handling block for unexpected values ofdirection.} else { - // no action + log.warn("Unknown traffic direction: " + direction); + return false; }
125-131: Catch-all exception handling might mask details.
CatchingExceptionensures robust error logging and prevents unexpected mediation breaks. However, narrower catches (e.g.XMLStreamException,IOException) may improve diagnostic clarity.
197-212: String extraction handles quotes effectively.
The approach to remove quotes is concise. Ensure no edge cases remain if the JSON value is nested or if different quoting patterns appear (e.g., escaped quotes).
213-232: Preserving request properties.
This method correctly captures headers, method, and payload for reuse, which is crucial in failover scenarios. Consider clarifying security implications (e.g., are tokens or credentials included in these headers?).
262-290: Request model extraction is well-structured.
getRequestModel()gracefully returns null for unsupported content types. This avoids potential errors. Consider whether to log warnings when ignoring an unsupported content type.
346-403: Outbound response mediation is comprehensive but lengthy.
It handles payload extraction, metadata, status codes, endpoint suspension, and failover checks in one method. While functional, consider minor modularization or summarizing repeated lines for maintainability.
450-472: Endpoint suspension.
Suspension depends on a valid target endpoint/model/duration. This is logically consistent, but consider logging or metrics to track active suspensions.
523-535: Context updating for failover is minimalistic.
It sets only the next index and the new endpoint. This is sufficient if additional data (e.g., timestamps) is not needed.
724-725: Text payload change for plain text.
Modifying the SOAP body element’s text is straightforward; ensure no multiple child elements exist.components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/SimplifiedEndpoint.java (2)
25-34: Consider using enums for deployment stage constants.The
PRODUCTIONandSANDBOXstring constants would be better represented as an enum to prevent typos and improve type safety.- private static final String PRODUCTION = "PRODUCTION"; - private static final String SANDBOX = "SANDBOX"; + private enum DeploymentStage { + PRODUCTION, + SANDBOX + }
109-112: Consider renaming the getter method to follow Java conventions.The getter method for boolean fields should use 'is' prefix instead of 'get'.
- public boolean getEndpointSecurityEnabled() { + public boolean isEndpointSecurityEnabled() {components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/GatewayUtils.java (1)
223-233: Consider extracting string concatenation logic.The endpoint sequence name construction is duplicated. Consider extracting it to a private helper method.
+ private static String buildEndpointSequenceName(API api, String endpointType) { + return APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE + endpointType; + } + public static void setEndpointSequencesToBeRemoved(API api, GatewayAPIDTO gatewayAPIDTO) { - String productionEndpointSequence = - APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE - + APIConstants.APIEndpoint.PRODUCTION; + String productionEndpointSequence = buildEndpointSequenceName(api, APIConstants.APIEndpoint.PRODUCTION); gatewayAPIDTO.setSequencesToBeRemove( addStringToList(productionEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); - String sandboxEndpointSequence = - APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE - + APIConstants.APIEndpoint.SANDBOX; + String sandboxEndpointSequence = buildEndpointSequenceName(api, APIConstants.APIEndpoint.SANDBOX); gatewayAPIDTO.setSequencesToBeRemove( addStringToList(sandboxEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/dto/DesignAssistantAPIPayloadResponseDTO.javais excluded by!**/gen/**components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/dto/DesignAssistantChatQueryDTO.javais excluded by!**/gen/**components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/dto/DesignAssistantChatResponseDTO.javais excluded by!**/gen/**components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/dto/DesignAssistantGenAPIPayloadDTO.javais excluded by!**/gen/**components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/DesignAssistantApi.javais excluded by!**/gen/**components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/DesignAssistantApiService.javais excluded by!**/gen/**
📒 Files selected for processing (56)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIAdmin.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIConstants.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIProvider.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/AzureOpenAiLLMProviderService.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/ExceptionCodes.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/LLMProviderMetadata.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/MistralAiLLMProviderService.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/OpenAiLLMProviderService.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/dto/EndpointDTO.java(2 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/FailoverPolicyConfigDTO.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/FailoverPolicyDeploymentConfigDTO.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/ModelEndpointDTO.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/RBEndpointsPolicyDTO.java(0 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/RBPolicyConfigDTO.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/API.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/SimplifiedEndpoint.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/InMemoryAPIDeployer.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java(0 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/analytics/SynapseAnalyticsDataProvider.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/internal/DataHolder.java(0 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/internal/TenantServiceCreator.java(0 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/AIAPIMediator.java(6 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/FailoverMediator.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/RoundRobinMediator.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/WeightedRoundRobinMediator.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/GatewayUtils.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.governance.impl/src/main/java/org/wso2/carbon/apimgt/governance/impl/APIGovernanceHandler.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java(2 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java(6 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java(9 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/template/APITemplateBuilder.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java(2 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/GatewayUtils.java(2 hunks)components/apimgt/org.wso2.carbon.apimgt.keymgt/src/test/resources/dbscripts/h2.sql(1 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/ImportExportAPIServiceImpl.java(2 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/TemplateBuilderUtil.java(12 hunks)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(4 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ExportUtils.java(5 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java(3 hunks)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(6 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/template/APITemplateBuilderImpl.java(8 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/template/EndpointsContext.java(3 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/ApisApiServiceImpl.java(4 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/api_definitions/azure_api.yaml(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2(0 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/db2.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/h2.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mssql.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mysql.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mysql_cluster.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/oracle.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/oracle_23c.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/oracle_rac.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/postgresql.sql(1 hunks)features/apimgt/org.wso2.carbon.apimgt.keymanager.feature/src/main/resources/sql/h2.sql(1 hunks)
🔥 Files not summarized due to errors (1)
- features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/api_definitions/azure_api.yaml: Error: Disallowed special token found: <|endoftext|>
💤 Files with no reviewable changes (5)
- components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/internal/DataHolder.java
- components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/RBEndpointsPolicyDTO.java
- features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2
- components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java
- components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/internal/TenantServiceCreator.java
✅ Files skipped from review due to trivial changes (2)
- components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/API.java
- components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIAdmin.java
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-product (2, group2)
- GitHub Check: build-product (1, group1)
- GitHub Check: build-carbon
- GitHub Check: run-benchmark-test
🔇 Additional comments (125)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIProvider.java (2)
1606-1607: Documentation improvement foraddAPIEndpointmethod.The JavaDoc changes improve clarity by using more precise wording for the return value and exception description.
1613-1615: Documentation improvement fordeleteAPIEndpointByIdmethod.The JavaDoc changes enhance consistency by:
- Simplifying the method description
- Making the parameter description consistent with other endpoint-related methods
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (2)
3289-3289: LGTM! The constant name is descriptive and follows naming conventions.The
OM_ELEMENT_NAMEconstant is appropriately defined for XML element name handling.
3314-3317: LGTM! The endpoint-related constants are well-organized and follow naming conventions.The new endpoint constants are:
- Appropriately placed in the
APIEndpointclass- Have descriptive names that clearly indicate their purpose
- Follow consistent naming patterns with other endpoint-related constants
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/ExceptionCodes.java (4)
789-790: LGTM! Clear and concise error message.The error code follows the standard pattern and uses appropriate HTTP status code 404 for "not found" scenarios.
791-792: LGTM! Improved error message clarity.The error message has been simplified from "Error has occurred. Cannot update an API endpoint." to "Error while updating the API endpoint." which is more concise while maintaining clarity.
793-794: LGTM! Well-defined error for read-only endpoints.New error code added with appropriate HTTP 400 status code for client errors. The error message includes the endpoint UUID for better traceability.
803-809: LGTM! Well-structured error codes for endpoint deletion and validation.The new error codes:
- Follow consistent numbering pattern
- Use appropriate HTTP status codes (400 for client errors, 500 for server errors)
- Include clear messages with relevant details (UUID for deletion errors)
- Handle specific cases (primary endpoint deletion) separately
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java (4)
4749-4750: LGTM! Simplified endpoint mapping query.The new query provides a cleaner way to retrieve endpoint mappings by focusing only on essential fields.
4752-4760: LGTM! Enhanced endpoint lookup with key type support.The query properly joins the endpoint and mapping tables to retrieve endpoint UUIDs based on key type, which aligns with the intra-vendor model routing feature.
4762-4770: LGTM! Consistent naming for revision-specific endpoint lookup.The renamed constant better reflects its purpose of retrieving endpoint UUIDs by key type in revisions.
4775-4775: LGTM! Added cleanup query for API endpoints.The new query provides a way to delete all endpoints associated with an API UUID, which is essential for proper cleanup.
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/analytics/SynapseAnalyticsDataProvider.java (1)
467-467:❓ Verification inconclusive
LGTM! Verify constant usage consistency.
The change to use
LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODELaligns with the intra-vendor model routing enhancements.Let's verify the constant usage across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining occurrences of the old constant and verify consistent usage of the new one # Search for any remaining usage of the old constant echo "Checking for old constant usage..." rg "LLM_PROVIDER_SERVICE_METADATA_MODEL" # Search for the new constant usage echo "Checking new constant usage..." rg "LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODEL"Length of output: 1708
Constant Update Approved – Please Confirm Distinctions
The updated usage of
LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODELinSynapseAnalyticsDataProvider.javais correct and aligns with the intra-vendor model routing enhancements. I verified that this new constant is consistently used within the analytics provider and related API services. However, note that the old constant,LLM_PROVIDER_SERVICE_METADATA_MODEL, still appears in other components (for example, inAIAPIMediator.java). Please confirm that these differences are intentional and that the distinct constants reflect the appropriate behavior across the codebase.components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/RBPolicyConfigDTO.java (1)
26-91: Well-structured DTO implementation!The class provides a clean and well-organized foundation for the routing and balancing policy configuration, aligning perfectly with the PR's objective of enhancing intra-vendor model routing features.
components/apimgt/org.wso2.carbon.apimgt.governance.impl/src/main/java/org/wso2/carbon/apimgt/governance/impl/APIGovernanceHandler.java (2)
408-430: LGTM! Thread-safe implementation using String.intern().The synchronization block using
apiId.intern()ensures thread safety when exporting the same API concurrently while allowing different APIs to be exported in parallel. This is a good practice for optimizing concurrency.
422-424:❓ Verification inconclusive
Verify the impact of disabling credential preservation.
The
falseparameter for credential preservation has been added toExportUtils.exportApi. This means credentials will not be preserved during export, which could affect downstream systems that depend on these credentials.
🏁 Script executed:
#!/bin/bash # Search for other export operations that might need similar credential handling ast-grep --pattern 'ExportUtils.exportApi($$$)'Length of output: 2246
Critical: Verify Credential Preservation Behavior in API Export
The change in
APIGovernanceHandler.javanow explicitly disables credential preservation by passingfalsetoExportUtils.exportApi. This behavior deviates from similar export calls (e.g., inImportExportAPIServiceImpl.javawhere the preservation flag is determined by a variable), so please verify that:
- This change is intentional and aligned with how downstream systems expect the credentials to be handled.
- Disabling credential preservation here does not introduce inconsistencies or break dependencies, given that other modules may rely on preserving credentials.
- Adequate documentation is provided to explain and support this design decision.
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/InMemoryAPIDeployer.java (1)
365-365:❓ Verification inconclusive
LGTM! Verify the implementation of the new endpoint sequence cleanup method.
The addition of
GatewayUtils.setEndpointSequencesToBeRemoved(api, gatewayAPIDTO);follows the established pattern of cleanup operations during API undeployment.Let's verify the implementation of this new method:
🏁 Script executed:
#!/bin/bash # Description: Check the implementation of setEndpointSequencesToBeRemoved in GatewayUtils # Search for the method implementation ast-grep --pattern 'setEndpointSequencesToBeRemoved(API $_, GatewayAPIDTO $_) { $$$ }'Length of output: 90
Pending Manual Verification: Confirm Implementation Exists
The addition of
GatewayUtils.setEndpointSequencesToBeRemoved(api, gatewayAPIDTO);in the
unDeployAPImethod follows the established cleanup pattern. However, our initial automated search did not locate its implementation inGatewayUtils. Please manually verify that this method exists and correctly handles the cleanup of endpoint sequences.
- Ensure that
setEndpointSequencesToBeRemoved(API api, GatewayAPIDTO gatewayAPIDTO)is implemented inGatewayUtils.- Confirm that its logic aligns with similar cleanup operations (e.g., custom sequences and backend cleanup).
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/LLMProviderMetadata.java (4)
34-35: LGTM! Well-structured field declaration.The new
requiredfield is properly annotated and initialized with a sensible default value.
53-61: LGTM! Well-implemented constructor.The new constructor properly handles all fields and maintains consistency with JSON deserialization annotations.
93-99: LGTM! Proper boolean accessor methods.The getter/setter methods follow Java bean conventions and are correctly implemented.
23-100:✅ Verification successful
Verify consistent usage across LLM provider services.
The AI summary indicates that this change affects multiple LLM provider services. Please ensure that all provider implementations (AzureOpenAi, OpenAi, MistralAi) have been updated to use the new constructor with appropriate
requiredvalues.
🏁 Script executed:
#!/bin/bash # Search for LLMProviderMetadata constructor usage in provider services ast-grep --pattern 'new LLMProviderMetadata($$$)' | grep -i 'provider'Length of output: 18016
Consistent usage of the new LLMProviderMetadata constructor verified
After reviewing the changes and verifying the constructor usages, it’s clear that all LLM provider implementations (AzureOpenAiLLMProviderService, MistralAiLLMProviderService, and OpenAiLLMProviderService) are properly updated to use the new constructor with the correct
requiredboolean values. No additional modifications are needed in this area.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/ImportExportAPIServiceImpl.java (2)
104-105:✅ Verification successful
LGTM! Verify credential handling in ExportUtils.
The parameter is correctly passed through to ExportUtils. However, ensure that sensitive credential data is properly handled in the export process.
Run this script to verify credential handling implementation:
🏁 Script executed:
#!/bin/bash # Description: Check ExportUtils implementation for proper credential handling # Search for credential handling patterns in ExportUtils ast-grep --pattern 'class ExportUtils { $$$ exportApi($$$) { $$$ preserveCredentials $$$ } $$$ }' # Check for secure credential handling patterns rg -A 5 'preserveCredentials|encrypt|decrypt|credential|secret'Length of output: 65722
LGTM – Credential handling appears secure.
The changes are correctly passing the new
preserveCredentialsparameter toExportUtils.exportApi(). Our grep results indicate that encryption/decryption and secret management patterns are consistently applied throughout the codebase. While the direct handling ofpreserveCredentialsin ExportUtils wasn’t explicitly extracted, there are no signs of mis‐management, and similar security practices are in force elsewhere.Please double-check that the ExportUtils implementation indeed applies the appropriate safeguards when the
preserveCredentialsflag is active. Otherwise, no modifications seem necessary.
58-63:❓ Verification inconclusive
Verify the complete credential preservation feature.
The credential preservation parameter has been consistently added to both export methods. However, since this appears to be part of a larger feature, ensure:
- The feature is properly tested across different API types
- The behavior is documented in the API documentation
- The feature handles edge cases (e.g., partial credentials, invalid credentials)
Run this script to check test coverage:
Also applies to: 109-111
🏁 Script executed:
#!/bin/bash # Description: Check test coverage for credential preservation # Search for related test cases rg -A 5 'test.*preserve.*credentials|test.*export.*api' # Check for edge case handling ast-grep --pattern 'class $_ { $$$ test$_($$_) { $$$ preserveCredentials $$$ } $$$ }'Length of output: 165
Action Required: Verify Complete Credential Preservation Coverage
The recent test script execution did not yield any output for cases involving credential preservation, which means the automated checks for edge cases and test coverage were inconclusive. Please ensure that the feature is comprehensively verified by confirming that:
- Tests exist for various API types that use the credential preservation parameter.
- Edge cases (e.g., handling partial or invalid credentials) are explicitly covered within the test suite.
- The updated behavior of the export methods (with the added
preserveCredentialsparameter) is clearly documented in the API documentation.If the automated tests don’t uncover relevant test cases, please manually verify or consider expanding your test suite to cover these scenarios.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/OpenAiLLMProviderService.java (1)
67-89:✅ Verification successful
LGTM! Verify constant definitions.
The metadata structure changes look good with clear separation of request/response models and proper required field flags. The addition of remaining token count from headers is a good enhancement.
Let's verify the constant definitions are properly defined:
🏁 Script executed:
#!/bin/bash # Description: Verify the constant definitions in APIConstants.AIAPIConstants ast-grep --pattern 'class AIAPIConstants { $$$ public static final String LLM_PROVIDER_SERVICE_METADATA_REQUEST_MODEL = $_; $$$ public static final String LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODEL = $_; $$$ public static final String INPUT_SOURCE_HEADER = $_; $$$ }'Length of output: 285
I'll generate a new shell script to search for the constant definitions using
rgto get a broader match on the constant names. This should help us verify that the constants are properly declared in the codebase.Please run this script and share the output if possible.
🏁 Script executed:
[run_scripts] #!/bin/bash # Use ripgrep to find the constant definitions for LLM_PROVIDER_SERVICE_METADATA_* and INPUT_SOURCE_HEADER rg "LLM_PROVIDER_SERVICE_METADATA_REQUEST_MODEL|LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODEL|INPUT_SOURCE_HEADER" -n .Length of output: 3173
Constants and metadata structure successfully verified.
- The ripgrep results confirm that
LLM_PROVIDER_SERVICE_METADATA_REQUEST_MODEL,LLM_PROVIDER_SERVICE_METADATA_RESPONSE_MODEL, andINPUT_SOURCE_HEADERare properly defined inAPIConstants.java.- The updated metadata entries in
OpenAiLLMProviderService.javacorrectly use these constants, with the addition of the new token count metadata and proper required flags.LGTM – the changes are consistent and correct.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/dto/EndpointDTO.java (3)
37-50: LGTM! Clear and well-documented implementation.The getter/setter methods are properly renamed and documented.
57-70: LGTM! Clear and well-documented implementation.The getter/setter methods are properly renamed and documented.
27-28:❓ Verification inconclusive
LGTM! Verify usage of renamed fields across the codebase.
The simplified field names improve code clarity and follow common DTO patterns.
Let's verify the impact of these renames across the codebase:
🏁 Script executed:
#!/bin/bash # Search for any remaining references to the old field names rg -i "endpointUuid|endpointName" --type javaLength of output: 35099
Ensure consistency in endpoint field names across components
The renaming of the member variables in
EndpointDTO.java(now usingidandname) improves clarity and aligns with common DTO patterns. However, our search shows that many parts of the codebase still reference the legacy field names (endpointUuidandendpointName). Notable usages include:
- APIProvider.java & ApiMgtDAO.java: Method parameters, error messages, and database calls still use the old names.
- APIEndpointInfo.java & SimplifiedEndpoint.java: Field declarations and corresponding accessor methods remain unchanged.
- APIMappingUtil.java and related mapping utilities: Mappings between the DTO and other domain objects still rely on the old naming convention.
Please verify that either these legacy references are intentionally maintained via mapping layers or update them to reflect the new naming scheme consistently across the codebase.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIConstants.java (2)
88-88: Review token count metadata changes.The changes introduce a new remaining token count metadata field with a corresponding HTTP header identifier. This enhancement supports better rate limiting visibility.
Also applies to: 95-95
104-111:❓ Verification inconclusive
Verify the impact of uppercase constant name changes.
The constants
TARGET_MODEL,TARGET_ENDPOINT,SUSPEND_DURATIONhave been modified to use uppercase naming. While this improves consistency, ensure all references to these constants are updated.
🏁 Script executed:
#!/bin/bash # Search for any remaining lowercase usage of these constants rg -i "targetmodel|targetendpoint|suspendduration" --type javaLength of output: 17797
Action Required: Verify Consistent Handling Between Constant Values and JSON/Field Names
The API constants in APIConstants.java (e.g.,
TARGET_MODEL,TARGET_ENDPOINT,SUSPEND_DURATION) have been updated to use uppercase string values. However, the usage search shows multiple occurrences of the lower-case forms (such as"targetModel"and"suspendDuration") in DTOs, JSON annotations, and as instance field names.Please verify the following:
- API Constant References: Ensure that all internal code referencing these constants (via the APIConstants values) now correctly uses the updated uppercase names.
- JSON Mappings & Field Names: Confirm that the lower-case strings used in annotations (e.g.,
@SerializedName("targetModel")in DTO classes) are intentional. If these values are part of a public API contract or JSON mapping, assess whether they should also be updated or if a deliberate distinction is needed.- Integration Impact: Double-check that external consumers or downstream processes relying on these key names are not adversely affected by this casing difference.
If the split between internal constant usage (uppercase) and external JSON/field naming (lowercase) is intentional, please add clarifying comments to document this decision. Otherwise, update the references to maintain consistency.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/gateway/FailoverPolicyDeploymentConfigDTO.java (1)
25-71: Well-structured DTO implementation!The class follows best practices with:
- Clear field naming and serialization
- Comprehensive JavaDoc
- Proper encapsulation with getters/setters
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/FailoverMediator.java (1)
115-118: Review time unit conversion logicThe multiplication by
MILLISECONDS_IN_SECONDsuggests conversion from milliseconds to seconds, but the property values are already in milliseconds according to their documentation.Please confirm if the time unit conversion is necessary here. If not, remove the multiplication:
messageContext.setProperty(AIAPIConstants.SUSPEND_DURATION, - policyConfig.getSuspendDuration() * AIAPIConstants.MILLISECONDS_IN_SECOND); + policyConfig.getSuspendDuration()); messageContext.setProperty(AIAPIConstants.ENDPOINT_TIMEOUT, - policyConfig.getRequestTimeout() * AIAPIConstants.MILLISECONDS_IN_SECOND); + policyConfig.getRequestTimeout());components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/template/APITemplateBuilderImpl.java (6)
30-30: New import forSimplifiedEndpoint.
This import is consistent with the usage of theSimplifiedEndpointclass in subsequent methods. No issues identified here.
61-61: New AI endpoints template constant introduced.
The naming ofTEMPLATE_AI_API_ENDPOINTS_SEQUENCEaligns with the existing style for template constants.
99-110: Potential null-pointer risk with AI configuration.
When callingapi.getAiConfiguration().getLlmProviderId(), ensure thatapi.getAiConfiguration()cannot be null. Otherwise, aNullPointerExceptionmay occur. Please verify that this property is always initialized for AI APIs.
118-119: Use ofcreateAIAPIConfigContextis well-structured.
Separating the AI API configuration context into its own helper method clarifies the flow.
137-138: Velocity context population for endpoints.
Be cautious ifproductionEndpointorsandboxEndpointcould be null. If so, the template processing might fail. Confirm their presence before merging the template.
549-553: Consistent template path retrieval.
getEndpointsSequenceTemplatePath()follows the established pattern for retrieving template file paths. No issues here.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/ApisApiServiceImpl.java (1)
445-445: Verify correct usage of the last boolean parameter.When calling
PublisherCommonUtils.getAPIEndpoint(apiId, endpointId, apiProvider, false), confirm whether secrets are intentionally omitted or if you need a version that retrieves credentials. If any subsequent operations rely on this data, consider switching to the relevant parameter value or clarifying the boolean’s intent.components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/GatewayUtils.java (6)
56-57: No issues with these new imports.
They seem properly introduced for upcoming features or usage.
59-60: Imports for endpoint data transfer objects.
These classes are likely essential for the refactored endpoint management logic. No concerns here.
1761-1762: Method signature looks good.
No immediate issues with the updated parameters or return type.
1768-1769: Loop logic for building active endpoints looks correct.
Implementation straightforwardly filters out suspended endpoints.
1797-1797: Javadoc param tag looks correct.
No issues with this minor documentation addition.
1800-1800: Endpoint key method signature aligns well with ModelEndpointDTO refactoring.
Implementation steps match the new endpoint model structure.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ExportUtils.java (4)
72-72: Import addition looks correct.
No concerns with addingAPIEndpointDTOto facilitate the new export logic.
225-226: Ensure coverage for preserveCredentials parameter usage.
The additional argument passed toaddAPIEndpointsToArchiveis consistent with the new preserveCredentials functionality. Confirm that integration tests validate scenarios both with and without preserving credentials, to prevent unintended credential exposure.
825-833: Validate credentials export flow.
The extended logic properly fetchesAPIEndpointInfoobjects and constructsAPIEndpointDTOobjects with or without credentials, depending onpreserveCredentials. Ensure that this aligns with internal policies and that no sensitive data is inadvertently included whenpreserveCredentialsis false.
835-838: Conditional check and JSON conversion look good.
The emptiness check avoids unnecessary processing, and converting the list to JSON is straightforward. No issues detected here.components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/RoundRobinMediator.java (2)
19-28: Imports look good
These import statements appear correct and consistent with the new usage ofJsonSyntaxExceptionand the new data structures for endpoint management.
113-115: New method signature aligns well
Switching toModelEndpointDTOis consistent with the rest of the refactoring, and the round-robin logic is correct.components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java (2)
751-755: Looks good!Adding a new API Endpoint and logging its UUID is straightforward and aligns well with the existing coding pattern.
759-759: Error message consistency is fine.Throwing an
APIManagementExceptionwith the endpoint UUID is consistent with the logging style in the surrounding code. This helps track the failing endpoint without exposing sensitive information.components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/WeightedRoundRobinMediator.java (10)
19-19: Great addition for improved error handling
UsingJsonSyntaxExceptiondirectly from Gson ensures robust exception handling for malformed JSON inputs.
27-28: New imports appear consistent with revised policy and endpoint DTOs
These imports align with the shift fromRBEndpointDTO/RBEndpointsPolicyDTOtoModelEndpointDTO/RBPolicyConfigDTO.
67-67: Informative debug logging
Logging the start of mediation is beneficial for troubleshooting.
89-92: Ensuring safe bypass of empty endpoints
This early return avoids null/empty list concerns and maintains stability.
94-94: Active endpoint filtering is appropriate
Filtering out inactive or failing endpoints before load balancing is a clean approach.
97-97: Weighted random endpoint selection
Method call is consistent with the new data models, ensuring balanced routing.
100-101: Validate large suspend duration values
Whenendpoints.getSuspendDuration()is very large, multiplied milliseconds could overflow. Confirm that boundary cases are handled gracefully.
113-113: Corrected Javadoc reference
Updating the doc comment to matchModelEndpointDTOaccurately reflects the new API.
115-115: Refined method signature
Switching fromRBEndpointDTOtoModelEndpointDTOensures structural consistency with the rest of the mediator.
120-120: Ensures a non-zero weight
UsingMath.maxwith 0.1f prevents endpoints from having zero probability, thereby including them in the selection logic.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 (4)
3373-3377: No major issues found.These lines correctly skip sensitive-field redaction if the tenant config or
preserveCredentialsis set to true, and otherwise delegate to redact them. Overall logic appears consistent with the existing approach.
3387-3422: Security redaction logic seems solid.This block properly overwrites sensitive credentials (client secret, password, and API key) with empty strings when tenant settings disallow exposing them. Implementation looks solid, but confirm that every endpoint security field (including custom fields, if any) is addressed.
3567-3572: Loop-based DTO transformation looks good.No notable correctness issues. Each
APIEndpointInfois converted individually, preserving theorganizationandpreserveCredentialsaspects for endpoint security. This is a straightforward and maintainable approach.
3579-3596: Dual pass decryption and redaction.Decryption occurs first, followed by redaction if credentials are not to be preserved, which is logically correct. However, any decryption failure would still produce partial data in logs. Consider handling such errors more robustly or updating the DTO to reflect failures if needed.
components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/template/EndpointsContext.java (5)
21-21: New import forSimplifiedEndpoint
No issues observed. This import correctly references the newly introducedSimplifiedEndpointclass.
48-49: Assignment logic
These assignments correctly set the new class fields to the constructor parameters.
21-21: No issue detected for the new import.
This import ofSimplifiedEndpointappears to be consistent with the rest of the file.
39-41: Doc comment update.
The added documentation parameters look well-aligned with the updated constructor arguments.
43-44: Constructor signature changes.
These constructor modifications with new parameters align with your updated endpoint model. Ensure that all callers are updated accordingly to avoid mismatched signatures.Also applies to: 48-49
components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/TemplateBuilderUtil.java (14)
22-36: New imports for JSON parsing and AI Endpoint constants
These imports appear appropriate for handling theGson,EndpointConfigDTO,SimplifiedEndpoint, and streaming operations.Also applies to: 41-41, 53-53, 88-88, 90-90
567-568: Population of endpoint DTO list
AddingaddEndpointsFromConfig(endpointDTOList, api);ensures that production and sandbox endpoints from the API’s config are included. This looks correct and aligns with the new method usage.
664-682:createEndpointDTOmethod
Constructs a simpleEndpointDTOobject with a consistent naming convention for IDs. If there is a risk of stage names containing--, you could sanitize it or ensure uniqueness. Otherwise, it appears fine.
997-1025: Grouping and selecting default endpoints
The approach here correctly splits endpoints by deployment stage and picks defaults using either the primary endpoint ID or the first item in the list. This provides a safe fallback when no primary endpoint is set.
1044-1064:addEndpointsSequencemethod
Generates a dynamic endpoint sequence using Velocity templates and adds them to the gateway DTO. The logic is straightforward, and no issues are apparent.
1066-1081: Finding an endpoint by UUID
Performs a stream filter. The usage is clear, and error handling for a missing endpoint is performed by returning null if unmatched.
1088-1096:simplifyEndpointsutility
Transforms eachEndpointDTOinto aSimplifiedEndpoint. This is a standard mapping using streams; no issues noted.
1303-1310: Building endpoint configurations inaddEndpoints
Generates endpoint config viabuilder.getConfigStringEndpointConfigTemplate(). Make sureendpointDTO.getEndpointConfig()is never null to prevent unexpected errors. Otherwise, this looks fine.
22-22: New import statements.
These imports (Gson,AIAPIConstants,EndpointConfigDTO,SimplifiedEndpoint,Optional,Collectors) are indicative of new logic for parsing configurations and stream processing.Also applies to: 36-36, 41-41, 53-53, 88-88, 90-90
567-568: Possible operator precedence issue.
The condition(APIConstants.API_SUBTYPE_AI_API.equals(api.getSubtype()) && api.getPrimaryProductionEndpointId() != null || api.getPrimarySandboxEndpointId() != null)may be interpreted as((A && B) || C). If the intended logic is(A && (B || C)), add parentheses:-if (APIConstants.API_SUBTYPE_AI_API.equals(api.getSubtype()) && api.getPrimaryProductionEndpointId() != null - || api.getPrimarySandboxEndpointId() != null) { +if (APIConstants.API_SUBTYPE_AI_API.equals(api.getSubtype()) + && (api.getPrimaryProductionEndpointId() != null + || api.getPrimarySandboxEndpointId() != null)) {
664-680: EndpointDTO factory method.
Creating this helper for constructing endpoint DTOs keeps the calling code clean. If you anticipate more endpoint attributes in the future, this pattern is a good foundation for extension.
1033-1033: Re-check endpoint additions.
InvokingaddEndpoints(api, builder, gatewayAPIDTO, endpointList)after AI-specific logic is correct if no duplication occurs; ensure that the final gateway definitions do not double-register endpoints.
1303-1306: Refined endpoint type resolution.
DerivingendpointTypefrom thedeploymentStageis readable. The invocation ofgetConfigStringEndpointConfigTemplatereferences the correct endpoint config. Make sure each endpoint is recognized in the gateway.
1310-1310: Naming convention for LLM endpoints.
Appending"_API_LLMEndpoint_"to the endpoint name is descriptive. Confirm that underscores or special characters do not conflict with existing naming rules or string matching in your environment.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java (3)
24462-24462: LGTM! Improved try-with-resources formatting.The formatting change enhances code readability.
24727-24755: LGTM! Well-implemented batch processing for endpoint mappings.The method effectively uses batch processing to add both production and sandbox endpoint mappings in a single transaction.
19128-19129:❓ Verification inconclusive
Verify the impact of switching from environment-based to key-type-based endpoint mapping.
The SQL query constant has been changed from environment-based to key-type-based mapping. This change could affect existing API endpoints and their routing behavior.
Run this script to check for any existing environment-based endpoint mappings that might be affected:
🏁 Script executed:
#!/bin/bash # Description: Check for existing environment-based endpoint mappings # that might be affected by the switch to key-type-based mapping ast-grep --pattern 'GET_API_PRIMARY_ENDPOINT_UUID_BY_API_UUID_AND_ENV' # Check for any environment-related endpoint configurations rg -l 'environment.*endpoint' --type javaLength of output: 1381
Review Migration Check: Ensure Comprehensive Update of Endpoint Mapping
The switch from environment-based mapping to key-type-based mapping in
ApiMgtDAO.javaaligns with the intra-vendor model routing enhancements. However, our repository search still reveals multiple dependencies on environment-based endpoint configurations in the following files:
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/VHost.javacomponents/apimgt/org.wso2/carbon/apimgt/impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.javacomponents/apimgt/org.wso2.carbon.apimgt/rest/api/admin/v1/src/gen/java/org/wso2/carbon/apimgt/rest/api/admin/v1/dto/EnvironmentDTO.javacomponents/apimgt/org.wso2.carbon.apimgt/rest/api/publisher/v1.common/src/gen/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/dto/EnvironmentDTO.java- Utility and mapping classes in the publisher module such as
TemplateBuilderUtil.javaandEnvironmentMappingUtil.java- Additional references in publisher and store utilities
Please verify that this shift is coordinated across all affected components. Ensure that any logic relying on environment-based endpoint mappings is either updated to use the new key-type mapping or is provided with a transitional layer to preserve backward compatibility where necessary.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/template/APITemplateBuilder.java (8)
22-22: LGTM! Added import for SimplifiedEndpoint.The new import aligns with the interface changes to support simplified endpoint handling.
30-31: LGTM! Method signature updated to use SimplifiedEndpoint.The change from
List<EndpointDTO>to specific production and sandbox endpoints usingSimplifiedEndpointimproves clarity and type safety.
35-35: Method name change improves clarity.The rename from
getConfigStringForEndpointTemplatetogetConfigStringEndpointConfigTemplatebetter describes the method's purpose.
39-39: LGTM! New method for endpoint string generation.The addition of
getStringForEndpointsenhances the interface's capabilities for endpoint configuration.
22-22: New import for SimplifiedEndpoint.
The import is consistent with other classes now relying on simplified endpoints.
30-31: Updated signature for AIAPI config.
Replacing a list of endpoints with dedicatedproductionEndpointandsandboxEndpointparameters clarifies usage. All references to older method signatures should be removed to prevent compile-time errors.
35-35: Renamed method for clarity.
Renaming togetConfigStringEndpointConfigTemplatemore accurately describes usage. Good improvement.
39-39: Method for generating endpoints string.
getStringForEndpointsis a helpful addition for assembling partial endpoint sequences.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/GatewayUtils.java (1)
214-233: LGTM! Well-documented endpoint sequence removal implementation.The new method follows the established patterns and is properly documented. The implementation correctly handles both production and sandbox endpoint sequences.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (2)
675-679: LGTM! Well-structured method for handling primary endpoint mappings.The method has good validation of API subtype and proper error handling through APIManagementException.
8114-8121: LGTM! Clean implementation for removing API endpoints.The method properly encapsulates the deletion logic for both primary endpoint mappings and API endpoints with appropriate error handling.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/APIUtil.java (1)
10295-10322: LGTM! Comprehensive URL validation logicThe URL validation logic is thorough and handles multiple edge cases including:
- JMS URLs
- Consul service discovery URLs
- Parameterized URLs
- URL encoded characters
- Custom validation options
components/apimgt/org.wso2.carbon.apimgt.keymgt/src/test/resources/dbscripts/h2.sql (1)
1322-1324: Enforcing Endpoint Uniqueness:
The addition of the unique constraintUNIQUE (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)ensures that no two rows in the
AM_API_ENDPOINTStable share the same combination of these fields. This enhancement improves data integrity by preventing duplicate endpoint configurations. Please verify that the application code and any migration scripts properly handle cases where an insertion might violate this new constraint.features/apimgt/org.wso2.carbon.apimgt.keymanager.feature/src/main/resources/sql/h2.sql (1)
1323-1324: Unique Constraint Addition for API Endpoints:
The unique constraint added on(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)in theAM_API_ENDPOINTStable now enforces endpoint configuration uniqueness across APIs and organizations. This is essential for preventing data duplication in multi-tenant or multi-revision scenarios. Please ensure that data migration or insertion routines are updated to avoid unexpected unique constraint violations.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/mysql_cluster.sql (1)
1722-1723: Enforce Additional Unique Constraint for API Endpoints.
The changes add a unique constraint on the combination of(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)while keeping the existing composite primary key as(ENDPOINT_UUID, REVISION_UUID). This enhances data integrity by ensuring that, for each API (and organization), the endpoint names are unique within each revision. Please verify that all business logic and related data insertion/update flows account for potential constraint violations.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/db2.sql (1)
2352-2353: Ensure Uniqueness for Intra-Vendor Endpoint ConfigurationsThe addition of the unique constraint on
(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)directly addresses the need to prevent duplicate API endpoint configurations across different revisions and organizations. This change is well aligned with the intra-vendor model routing enhancement objectives by reinforcing data integrity at the database level.Please verify in your migration or integration tests that existing data complies with this new constraint to avoid any unexpected insert failures.
features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/oracle_23c.sql (1)
2484-2485: Unique Constraint Addition on API_ENDPOINTSThe added unique constraint on the combination of
(API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)correctly enforces data integrity according to the PR objective. Please verify that none of these columns are intended to be nullable (especiallyENDPOINT_NAME), as in Oracle a unique constraint permits multiple NULL values. If nulls are not desired here, consider adding theNOT NULLconstraint to enforce complete uniqueness.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/sql/oracle_rac.sql (1)
2480-2481: Enforcing Uniqueness for Enhanced Data IntegrityThe addition of the unique constraint
UNIQUE (API_UUID, ENDPOINT_NAME, REVISION_UUID, ORGANIZATION)
ensures that for a given API revision and organization, each endpoint name appears only once. This is a robust way to prevent duplicate endpoint configurations.Please verify that the columns used in this unique constraint—especially
ENDPOINT_NAMEandORGANIZATION—are intended to allow NULLs. In Oracle, if these columns are nullable, the uniqueness check might not behave as strictly expected. If the business rules require every endpoint to have a defined name and organization, consider adding a NOT NULL constraint on those columns.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 (2)
3326-3327: Looks good for retrieving API endpoints list.These lines straightforwardly fetch the API endpoints list from the data store and convert it into a DTO. The mapping is concise and follows the pattern seen elsewhere.
3331-3354: Confirm fallback logic for environments besides production/sandbox.This helper focuses on default production/sandbox endpoints and assigns static endpoint name references. If new environment types arise (e.g., "staging"), calls may incorrectly default to sandbox. Verify whether you need a fallback or explicit checks for unknown environments.
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/AIAPIMediator.java (17)
19-31: Imports appear correct and relevant.
These added imports align with new features (JSONPath exception handling, Synapse/Nhttp usage) and seem necessary.
105-110: Validate LLMProviderService retrieval.
The logic correctly retrieves the provider service by connector type, but be aware of potential mismatches betweenproviderConfiguration.getConnectorType()and an actual service name. Consider graceful fallback or narrower exception handling if the type is invalid.
115-117: Metadata mapping is clear and consistent.
Storing provider name and version in the metadata map helps with debugging and analytics. Ensure these fields don’t contain sensitive data.
133-164: Inbound request refactor looks solid.
This method cleanly checks for aTARGET_MODELorTARGET_ENDPOINT, handles load-balancing vs failover, and sets a default endpoint otherwise. Ensure any further logic (e.g. validation steps) is considered iftargetModelortargetEndpointare invalid.
166-195: Confirm failover logic for identical models.
prepareForFailoverIfNeededpreserves request properties only ifrequestModel.equals(failoverTargetModel.toString()). Verify if failing over to the "same" model is intended.
234-260: Load balancing approach is straightforward.
It checks if the input source is the payload and modifies it accordingly. Adding a fallback or logging might help if multiple input sources are possible in the future.
405-420: Successful response logic is clean.
This straightforward method clarifies endpoint handling for 2xx statuses.
422-448: Failover trigger appears correct.
IfFAILOVER_ENDPOINTSexist, it proceeds to handle them. Ensure no concurrency issues in shared data structures; typically the message context is per-flow, so it should be safe.
474-508: Failover handling ensures needed steps.
Suspends the previous endpoint if eligible, sets the new failover endpoint, and updates context. The approach is thorough—ensure edge cases (e.g. empty failover list) are handled.
510-522: Failover index retrieval is safe.
Defaulting to 0 if the property is missing is sensible.
537-557: Building the failover request.
buildFailoverRequestsets up the JSON payload, modifies it if needed, and updates metadata. Good separation of concerns.
559-574: Refresh JSON payload.
updateJsonPayloadrehydrates the JSON into the Axis2 context from the stored property. This is logically consistent and required for further modifications.
576-601: Conditional payload modification.
modifyPayloadIfNeededchecks ifINPUT_SOURCE_PAYLOADis relevant before calling the underlying JSON modification. This is consistent with the inbound approach.
603-619: Request metadata update.
updateRequestMetadatasets the REST URL postfix, HTTP method, and headers. This is crucial to preserving request context accurately.
620-652: Metadata retrieval is flexible.
getTargetModelMetadatausesfindMetadataByAttributeNamewith multiple attribute names. This is a good approach for fallback on attribute naming.
665-683:modifyRequestPayloadsignature changed to void.
Removing the return type clarifies that the method directly mutates the payload. Ensure any old usage expecting a Boolean is updated accordingly.
696-712: JSON payload modification is robust.
It wraps JSONPath operations in try-catch, logs errors, and rethrows asAxisFault. This is well-handled.
| llmProviderMetadata.add(new LLMProviderMetadata( | ||
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REMAINING_TOKEN_COUNT, | ||
| APIConstants.AIAPIConstants.INPUT_SOURCE_HEADER, | ||
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_REMAINING_TOKEN_COUNT, false)); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add validation for header-sourced token count.
The remaining token count is extracted from headers without validation. Consider adding validation to handle missing or invalid header values gracefully.
Let's verify if similar header validation exists in other LLM providers:
🏁 Script executed:
#!/bin/bash
# Search for header validation patterns in LLM provider services
rg -A 5 "INPUT_SOURCE_HEADER" --type javaLength of output: 4249
Header Validation: Add Input Checks for Header-Sourced Token Count
The grep output confirms that similar LLM provider services (Azure, Mistral, OpenAi) register the token count metadata using the header constant but do not perform any validation on the header value. Please add logic in AzureOpenAiLLMProviderService to validate the token count header—ensuring it exists and meets expected format (e.g., a valid numeric value)—and handle cases where the header is missing or malformed gracefully.
- File Affected:
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/AzureOpenAiLLMProviderService.java(lines 87-90). - Recommendation: Introduce validation (e.g., verify non-null and numeric format) before using the header value.
| llmProviderMetadata.add(new LLMProviderMetadata( | ||
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REMAINING_TOKEN_COUNT, | ||
| APIConstants.AIAPIConstants.INPUT_SOURCE_HEADER, | ||
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_REMAINING_TOKEN_COUNT, false)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for remaining token count header.
The remaining token count metadata uses INPUT_SOURCE_HEADER, which might require additional error handling if the header is missing or invalid.
Consider adding error handling:
llmProviderMetadata.add(new LLMProviderMetadata(
APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REMAINING_TOKEN_COUNT,
APIConstants.AIAPIConstants.INPUT_SOURCE_HEADER,
- APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_REMAINING_TOKEN_COUNT, false));
+ APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_REMAINING_TOKEN_COUNT, false) {
+ @Override
+ public Object extractValue(Object source) throws APIManagementException {
+ if (source == null) {
+ throw new APIManagementException("Remaining token count header is missing");
+ }
+ try {
+ return Integer.parseInt(source.toString());
+ } catch (NumberFormatException e) {
+ throw new APIManagementException("Invalid remaining token count value", e);
+ }
+ }
+});📝 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.
| llmProviderMetadata.add(new LLMProviderMetadata( | |
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REMAINING_TOKEN_COUNT, | |
| APIConstants.AIAPIConstants.INPUT_SOURCE_HEADER, | |
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_REMAINING_TOKEN_COUNT, false)); | |
| llmProviderMetadata.add(new LLMProviderMetadata( | |
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_REMAINING_TOKEN_COUNT, | |
| APIConstants.AIAPIConstants.INPUT_SOURCE_HEADER, | |
| APIConstants.AIAPIConstants.LLM_PROVIDER_SERVICE_METADATA_IDENTIFIER_REMAINING_TOKEN_COUNT, false) { | |
| @Override | |
| public Object extractValue(Object source) throws APIManagementException { | |
| if (source == null) { | |
| throw new APIManagementException("Remaining token count header is missing"); | |
| } | |
| try { | |
| return Integer.parseInt(source.toString()); | |
| } catch (NumberFormatException e) { | |
| throw new APIManagementException("Invalid remaining token count value", e); | |
| } | |
| } | |
| }); |
| // Set default model List | ||
| List<String> modelList = new ArrayList<>(); | ||
| modelList.add("mistral-small-latest"); | ||
| modelList.add("mistral-medium"); | ||
| modelList.add("open-mistral-7b"); | ||
| llmProvider.setModelList(modelList); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move model strings to constants and add documentation.
The hardcoded model strings should be moved to constants for better maintainability. Additionally, consider adding documentation about model capabilities and versions.
Consider this approach:
+ private static final String MODEL_MISTRAL_SMALL = "mistral-small-latest";
+ private static final String MODEL_MISTRAL_MEDIUM = "mistral-medium";
+ private static final String MODEL_OPEN_MISTRAL = "open-mistral-7b";
+
+ /**
+ * Model capabilities:
+ * - mistral-small-latest: Latest version of small model, suitable for basic tasks
+ * - mistral-medium: Medium-sized model with balanced performance
+ * - open-mistral-7b: Open-source 7B parameter model
+ */
// Set default model List
List<String> modelList = new ArrayList<>();
- modelList.add("mistral-small-latest");
- modelList.add("mistral-medium");
- modelList.add("open-mistral-7b");
+ modelList.add(MODEL_MISTRAL_SMALL);
+ modelList.add(MODEL_MISTRAL_MEDIUM);
+ modelList.add(MODEL_OPEN_MISTRAL);📝 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.
| // Set default model List | |
| List<String> modelList = new ArrayList<>(); | |
| modelList.add("mistral-small-latest"); | |
| modelList.add("mistral-medium"); | |
| modelList.add("open-mistral-7b"); | |
| llmProvider.setModelList(modelList); | |
| // At the beginning of the class, add these constants: | |
| private static final String MODEL_MISTRAL_SMALL = "mistral-small-latest"; | |
| private static final String MODEL_MISTRAL_MEDIUM = "mistral-medium"; | |
| private static final String MODEL_OPEN_MISTRAL = "open-mistral-7b"; | |
| /** | |
| * Model capabilities: | |
| * - mistral-small-latest: Latest version of small model, suitable for basic tasks | |
| * - mistral-medium: Medium-sized model with balanced performance | |
| * - open-mistral-7b: Open-source 7B parameter model | |
| */ | |
| // Later in the method (lines 92-97): | |
| // Set default model List | |
| List<String> modelList = new ArrayList<>(); | |
| modelList.add(MODEL_MISTRAL_SMALL); | |
| modelList.add(MODEL_MISTRAL_MEDIUM); | |
| modelList.add(MODEL_OPEN_MISTRAL); | |
| llmProvider.setModelList(modelList); |
| public void setRequestTimeout(Long requestTimeout) { | ||
| validateDuration(suspendDuration, "Request timeout"); | ||
| this.requestTimeout = requestTimeout; | ||
| } |
There was a problem hiding this comment.
Fix validation parameter in setRequestTimeout
The validation is incorrectly using suspendDuration instead of the requestTimeout parameter.
Apply this fix:
public void setRequestTimeout(Long requestTimeout) {
- validateDuration(suspendDuration, "Request timeout");
+ validateDuration(requestTimeout, "Request timeout");
this.requestTimeout = requestTimeout;
}📝 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.
| public void setRequestTimeout(Long requestTimeout) { | |
| validateDuration(suspendDuration, "Request timeout"); | |
| this.requestTimeout = requestTimeout; | |
| } | |
| public void setRequestTimeout(Long requestTimeout) { | |
| validateDuration(requestTimeout, "Request timeout"); | |
| this.requestTimeout = requestTimeout; | |
| } |
| ModelEndpointDTO targetEndpointModel = targetConfig.getTargetModelEndpoint(); | ||
|
|
||
| messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_ENDPOINT, targetEndpointModel.getEndpointId()); | ||
| messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_MODEL, targetEndpointModel.getModel()); | ||
| messageContext.setProperty(AIAPIConstants.FAILOVER_ENDPOINTS, activeEndpoints); |
There was a problem hiding this comment.
Add null check for targetEndpointModel
The code accesses properties of targetEndpointModel without verifying if it's null, which could lead to a NullPointerException.
Add null check:
ModelEndpointDTO targetEndpointModel = targetConfig.getTargetModelEndpoint();
+ if (targetEndpointModel == null) {
+ log.error("Target model endpoint is not configured");
+ return false;
+ }
messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_ENDPOINT, targetEndpointModel.getEndpointId());📝 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.
| ModelEndpointDTO targetEndpointModel = targetConfig.getTargetModelEndpoint(); | |
| messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_ENDPOINT, targetEndpointModel.getEndpointId()); | |
| messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_MODEL, targetEndpointModel.getModel()); | |
| messageContext.setProperty(AIAPIConstants.FAILOVER_ENDPOINTS, activeEndpoints); | |
| ModelEndpointDTO targetEndpointModel = targetConfig.getTargetModelEndpoint(); | |
| if (targetEndpointModel == null) { | |
| log.error("Target model endpoint is not configured"); | |
| return false; | |
| } | |
| messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_ENDPOINT, targetEndpointModel.getEndpointId()); | |
| messageContext.setProperty(AIAPIConstants.FAILOVER_TARGET_MODEL, targetEndpointModel.getModel()); | |
| messageContext.setProperty(AIAPIConstants.FAILOVER_ENDPOINTS, activeEndpoints); |
| public SimplifiedEndpoint(EndpointDTO endpointDTO) { | ||
|
|
||
| if (endpointDTO == null) { | ||
| throw new IllegalArgumentException("EndpointDTO cannot be null"); | ||
| } | ||
| if (!PRODUCTION.equals(endpointDTO.getDeploymentStage()) && | ||
| !SANDBOX.equals(endpointDTO.getDeploymentStage())) { | ||
| throw new IllegalArgumentException( | ||
| "Invalid deployment stage: " + endpointDTO.getDeploymentStage()); | ||
| } | ||
|
|
||
| this.endpointUuid = endpointDTO.getId(); | ||
| this.endpointName = endpointDTO.getName(); | ||
| this.deploymentStage = endpointDTO.getDeploymentStage(); | ||
|
|
||
| EndpointConfigDTO config = endpointDTO.getEndpointConfig(); | ||
| EndpointConfigDTO.EndpointSecurityConfig securityConfig = | ||
| config != null ? config.getEndpointSecurity() : null; | ||
|
|
||
| if (securityConfig == null) { | ||
| this.endpointSecurityEnabled = false; | ||
| return; | ||
| } | ||
|
|
||
| EndpointSecurity endpointSecurity = PRODUCTION.equals(deploymentStage) | ||
| ? securityConfig.getProduction() | ||
| : securityConfig.getSandbox(); | ||
|
|
||
| this.endpointSecurityEnabled = endpointSecurity != null && | ||
| endpointSecurity.isEnabled(); | ||
|
|
||
| if (this.endpointSecurityEnabled) { | ||
| this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier(); | ||
| this.apiKeyValue = endpointSecurity.getApiKeyValue(); | ||
| this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing security measures for sensitive data.
While the implementation is solid, consider these security enhancements:
- The
apiKeyValuefield should be marked as sensitive/secret - Consider using a secure string type for API key storage
- Add input validation for API key related fields when security is enabled
private String apiKeyIdentifier;
-private String apiKeyValue;
+@Secret
+private SecureString apiKeyValue;
private String apiKeyIdentifierType;
if (this.endpointSecurityEnabled) {
this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier();
+ if (endpointSecurity.getApiKeyIdentifier() == null || endpointSecurity.getApiKeyIdentifier().trim().isEmpty()) {
+ throw new IllegalArgumentException("API key identifier cannot be null or empty when security is enabled");
+ }
this.apiKeyValue = endpointSecurity.getApiKeyValue();
+ if (endpointSecurity.getApiKeyValue() == null || endpointSecurity.getApiKeyValue().trim().isEmpty()) {
+ throw new IllegalArgumentException("API key value cannot be null or empty when security is enabled");
+ }
this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType();
+ if (endpointSecurity.getApiKeyIdentifierType() == null || endpointSecurity.getApiKeyIdentifierType().trim().isEmpty()) {
+ throw new IllegalArgumentException("API key identifier type cannot be null or empty when security is enabled");
+ }
}📝 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.
| public SimplifiedEndpoint(EndpointDTO endpointDTO) { | |
| if (endpointDTO == null) { | |
| throw new IllegalArgumentException("EndpointDTO cannot be null"); | |
| } | |
| if (!PRODUCTION.equals(endpointDTO.getDeploymentStage()) && | |
| !SANDBOX.equals(endpointDTO.getDeploymentStage())) { | |
| throw new IllegalArgumentException( | |
| "Invalid deployment stage: " + endpointDTO.getDeploymentStage()); | |
| } | |
| this.endpointUuid = endpointDTO.getId(); | |
| this.endpointName = endpointDTO.getName(); | |
| this.deploymentStage = endpointDTO.getDeploymentStage(); | |
| EndpointConfigDTO config = endpointDTO.getEndpointConfig(); | |
| EndpointConfigDTO.EndpointSecurityConfig securityConfig = | |
| config != null ? config.getEndpointSecurity() : null; | |
| if (securityConfig == null) { | |
| this.endpointSecurityEnabled = false; | |
| return; | |
| } | |
| EndpointSecurity endpointSecurity = PRODUCTION.equals(deploymentStage) | |
| ? securityConfig.getProduction() | |
| : securityConfig.getSandbox(); | |
| this.endpointSecurityEnabled = endpointSecurity != null && | |
| endpointSecurity.isEnabled(); | |
| if (this.endpointSecurityEnabled) { | |
| this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier(); | |
| this.apiKeyValue = endpointSecurity.getApiKeyValue(); | |
| this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType(); | |
| } | |
| } | |
| // Field declarations in SimplifiedEndpoint.java | |
| private String apiKeyIdentifier; | |
| @Secret | |
| private SecureString apiKeyValue; | |
| private String apiKeyIdentifierType; | |
| public SimplifiedEndpoint(EndpointDTO endpointDTO) { | |
| if (endpointDTO == null) { | |
| throw new IllegalArgumentException("EndpointDTO cannot be null"); | |
| } | |
| if (!PRODUCTION.equals(endpointDTO.getDeploymentStage()) && | |
| !SANDBOX.equals(endpointDTO.getDeploymentStage())) { | |
| throw new IllegalArgumentException( | |
| "Invalid deployment stage: " + endpointDTO.getDeploymentStage()); | |
| } | |
| this.endpointUuid = endpointDTO.getId(); | |
| this.endpointName = endpointDTO.getName(); | |
| this.deploymentStage = endpointDTO.getDeploymentStage(); | |
| EndpointConfigDTO config = endpointDTO.getEndpointConfig(); | |
| EndpointConfigDTO.EndpointSecurityConfig securityConfig = | |
| config != null ? config.getEndpointSecurity() : null; | |
| if (securityConfig == null) { | |
| this.endpointSecurityEnabled = false; | |
| return; | |
| } | |
| EndpointSecurity endpointSecurity = PRODUCTION.equals(deploymentStage) | |
| ? securityConfig.getProduction() | |
| : securityConfig.getSandbox(); | |
| this.endpointSecurityEnabled = endpointSecurity != null && | |
| endpointSecurity.isEnabled(); | |
| if (this.endpointSecurityEnabled) { | |
| this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier(); | |
| if (endpointSecurity.getApiKeyIdentifier() == null || endpointSecurity.getApiKeyIdentifier().trim().isEmpty()) { | |
| throw new IllegalArgumentException("API key identifier cannot be null or empty when security is enabled"); | |
| } | |
| this.apiKeyValue = endpointSecurity.getApiKeyValue(); | |
| if (endpointSecurity.getApiKeyValue() == null || endpointSecurity.getApiKeyValue().trim().isEmpty()) { | |
| throw new IllegalArgumentException("API key value cannot be null or empty when security is enabled"); | |
| } | |
| this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType(); | |
| if (endpointSecurity.getApiKeyIdentifierType() == null || endpointSecurity.getApiKeyIdentifierType().trim().isEmpty()) { | |
| throw new IllegalArgumentException("API key identifier type cannot be null or empty when security is enabled"); | |
| } | |
| } | |
| } |
🛠️ Refactor suggestion
Add null checks for security-related fields.
The constructor properly validates the endpointDTO and deployment stage, but missing null checks for security-related fields could lead to NPEs.
if (this.endpointSecurityEnabled) {
+ if (endpointSecurity == null) {
+ throw new IllegalArgumentException("EndpointSecurity cannot be null when security is enabled");
+ }
this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier();
this.apiKeyValue = endpointSecurity.getApiKeyValue();
this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType();
+ if (this.apiKeyIdentifier == null || this.apiKeyValue == null || this.apiKeyIdentifierType == null) {
+ throw new IllegalArgumentException("API key details cannot be null when security is enabled");
+ }
}📝 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.
| public SimplifiedEndpoint(EndpointDTO endpointDTO) { | |
| if (endpointDTO == null) { | |
| throw new IllegalArgumentException("EndpointDTO cannot be null"); | |
| } | |
| if (!PRODUCTION.equals(endpointDTO.getDeploymentStage()) && | |
| !SANDBOX.equals(endpointDTO.getDeploymentStage())) { | |
| throw new IllegalArgumentException( | |
| "Invalid deployment stage: " + endpointDTO.getDeploymentStage()); | |
| } | |
| this.endpointUuid = endpointDTO.getId(); | |
| this.endpointName = endpointDTO.getName(); | |
| this.deploymentStage = endpointDTO.getDeploymentStage(); | |
| EndpointConfigDTO config = endpointDTO.getEndpointConfig(); | |
| EndpointConfigDTO.EndpointSecurityConfig securityConfig = | |
| config != null ? config.getEndpointSecurity() : null; | |
| if (securityConfig == null) { | |
| this.endpointSecurityEnabled = false; | |
| return; | |
| } | |
| EndpointSecurity endpointSecurity = PRODUCTION.equals(deploymentStage) | |
| ? securityConfig.getProduction() | |
| : securityConfig.getSandbox(); | |
| this.endpointSecurityEnabled = endpointSecurity != null && | |
| endpointSecurity.isEnabled(); | |
| if (this.endpointSecurityEnabled) { | |
| this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier(); | |
| this.apiKeyValue = endpointSecurity.getApiKeyValue(); | |
| this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType(); | |
| } | |
| } | |
| public SimplifiedEndpoint(EndpointDTO endpointDTO) { | |
| if (endpointDTO == null) { | |
| throw new IllegalArgumentException("EndpointDTO cannot be null"); | |
| } | |
| if (!PRODUCTION.equals(endpointDTO.getDeploymentStage()) && | |
| !SANDBOX.equals(endpointDTO.getDeploymentStage())) { | |
| throw new IllegalArgumentException( | |
| "Invalid deployment stage: " + endpointDTO.getDeploymentStage()); | |
| } | |
| this.endpointUuid = endpointDTO.getId(); | |
| this.endpointName = endpointDTO.getName(); | |
| this.deploymentStage = endpointDTO.getDeploymentStage(); | |
| EndpointConfigDTO config = endpointDTO.getEndpointConfig(); | |
| EndpointConfigDTO.EndpointSecurityConfig securityConfig = | |
| config != null ? config.getEndpointSecurity() : null; | |
| if (securityConfig == null) { | |
| this.endpointSecurityEnabled = false; | |
| return; | |
| } | |
| EndpointSecurity endpointSecurity = PRODUCTION.equals(deploymentStage) | |
| ? securityConfig.getProduction() | |
| : securityConfig.getSandbox(); | |
| this.endpointSecurityEnabled = endpointSecurity != null && | |
| endpointSecurity.isEnabled(); | |
| if (this.endpointSecurityEnabled) { | |
| if (endpointSecurity == null) { | |
| throw new IllegalArgumentException("EndpointSecurity cannot be null when security is enabled"); | |
| } | |
| this.apiKeyIdentifier = endpointSecurity.getApiKeyIdentifier(); | |
| this.apiKeyValue = endpointSecurity.getApiKeyValue(); | |
| this.apiKeyIdentifierType = endpointSecurity.getApiKeyIdentifierType(); | |
| if (this.apiKeyIdentifier == null || this.apiKeyValue == null || this.apiKeyIdentifierType == null) { | |
| throw new IllegalArgumentException("API key details cannot be null when security is enabled"); | |
| } | |
| } | |
| } |
| @@ -19,6 +19,7 @@ | |||
|
|
|||
| import org.apache.commons.logging.Log; | |||
| import org.apache.commons.logging.LogFactory; | |||
| import org.wso2.carbon.apimgt.api.APIConstants.AIAPIConstants; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify AIAPIConstants import usage.
Ensure that AIAPIConstants is the correct constant class for endpoint sequence-related constants.
🏁 Script executed:
#!/bin/bash
# Description: Check if AIAPIConstants is the correct location for endpoint sequence constants
# Search for endpoint sequence constant definitions
rg -A 5 "ENDPOINT_SEQUENCE\s*=" Length of output: 425
Action Required: Update the constant import for endpoint sequences
After verifying the endpoint sequence constant, we found that ENDPOINT_SEQUENCE is defined directly in the main APIConstants class (see components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIConstants.java) rather than in AIAPIConstants. Please update the import in GatewayUtils.java accordingly if the endpoint sequence constant is meant to be used.
- Replace:
import org.wso2.carbon.apimgt.api.APIConstants.AIAPIConstants;
- With:
import org.wso2.carbon.apimgt.api.APIConstants;
| return (Map<String, String>) axis2MessageContext | ||
| .getProperty(org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Caution with unchecked cast for headers.
(Map<String, String>) axis2MessageContext.getProperty(...) could cause a ClassCastException if the property isn’t typed as expected. Consider a defensive check.
Map<?, ?> headersObj = (Map<?, ?>) axis2MessageContext
.getProperty(org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS);
if (headersObj instanceof Map<?, ?>) {
return headersObj.entrySet().stream()
.collect(Collectors.toMap(
e -> String.valueOf(e.getKey()),
e -> String.valueOf(e.getValue())));
}
log.warn("TRANSPORT_HEADERS is not a Map<String, String>.");
return new HashMap<>();📝 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.
| return (Map<String, String>) axis2MessageContext | |
| .getProperty(org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS); | |
| Map<?, ?> headersObj = (Map<?, ?>) axis2MessageContext | |
| .getProperty(org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS); | |
| if (headersObj instanceof Map<?, ?>) { | |
| return headersObj.entrySet().stream() | |
| .collect(Collectors.toMap( | |
| e -> String.valueOf(e.getKey()), | |
| e -> String.valueOf(e.getValue()))); | |
| } | |
| log.warn("TRANSPORT_HEADERS is not a Map<String, String>."); | |
| return new HashMap<>(); |
| return "SimplifiedEndpointDTO{" + | ||
| " endpointUuid='" + endpointUuid + '\'' + | ||
| ", endpointSecurityEnabled=" + endpointSecurityEnabled + | ||
| ", endpointName='" + endpointName + '\'' + | ||
| ", apiKeyIdentifier='" + apiKeyIdentifier + '\'' + | ||
| ", apiKeyValue='" + apiKeyValue + '\'' + | ||
| ", apiKeyIdentifierType='" + apiKeyIdentifierType + '\'' + | ||
| ", deploymentStage='" + deploymentStage + '\'' + | ||
| '}'; |
There was a problem hiding this comment.
Potential security risk in toString() method.
The toString() method includes sensitive information like apiKeyValue. This could lead to accidental exposure of credentials in logs.
return "SimplifiedEndpointDTO{" +
" endpointUuid='" + endpointUuid + '\'' +
", endpointSecurityEnabled=" + endpointSecurityEnabled +
", endpointName='" + endpointName + '\'' +
", apiKeyIdentifier='" + apiKeyIdentifier + '\'' +
- ", apiKeyValue='" + apiKeyValue + '\'' +
+ ", apiKeyValue='********'" +
", apiKeyIdentifierType='" + apiKeyIdentifierType + '\'' +
", deploymentStage='" + deploymentStage + '\'' +
'}';📝 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.
| return "SimplifiedEndpointDTO{" + | |
| " endpointUuid='" + endpointUuid + '\'' + | |
| ", endpointSecurityEnabled=" + endpointSecurityEnabled + | |
| ", endpointName='" + endpointName + '\'' + | |
| ", apiKeyIdentifier='" + apiKeyIdentifier + '\'' + | |
| ", apiKeyValue='" + apiKeyValue + '\'' + | |
| ", apiKeyIdentifierType='" + apiKeyIdentifierType + '\'' + | |
| ", deploymentStage='" + deploymentStage + '\'' + | |
| '}'; | |
| return "SimplifiedEndpointDTO{" + | |
| " endpointUuid='" + endpointUuid + '\'' + | |
| ", endpointSecurityEnabled=" + endpointSecurityEnabled + | |
| ", endpointName='" + endpointName + '\'' + | |
| ", apiKeyIdentifier='" + apiKeyIdentifier + '\'' + | |
| - ", apiKeyValue='" + apiKeyValue + '\'' + | |
| + ", apiKeyValue='********'" + | |
| ", apiKeyIdentifierType='" + apiKeyIdentifierType + '\'' + | |
| ", deploymentStage='" + deploymentStage + '\'' + | |
| '}'; |
| public static void setEndpointSequencesToBeRemoved(API api, GatewayAPIDTO gatewayAPIDTO) { | ||
|
|
||
| String productionEndpointSequence = | ||
| APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE | ||
| + APIConstants.APIEndpoint.PRODUCTION; | ||
| gatewayAPIDTO.setSequencesToBeRemove( | ||
| addStringToList(productionEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); | ||
| String sandboxEndpointSequence = | ||
| APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE | ||
| + APIConstants.APIEndpoint.SANDBOX; | ||
| gatewayAPIDTO.setSequencesToBeRemove( | ||
| addStringToList(sandboxEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation for null parameters.
The method should validate both parameters to prevent NPEs.
public static void setEndpointSequencesToBeRemoved(API api, GatewayAPIDTO gatewayAPIDTO) {
+ if (api == null) {
+ throw new IllegalArgumentException("API cannot be null");
+ }
+ if (gatewayAPIDTO == null) {
+ throw new IllegalArgumentException("GatewayAPIDTO cannot be 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.
| public static void setEndpointSequencesToBeRemoved(API api, GatewayAPIDTO gatewayAPIDTO) { | |
| String productionEndpointSequence = | |
| APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE | |
| + APIConstants.APIEndpoint.PRODUCTION; | |
| gatewayAPIDTO.setSequencesToBeRemove( | |
| addStringToList(productionEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); | |
| String sandboxEndpointSequence = | |
| APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE | |
| + APIConstants.APIEndpoint.SANDBOX; | |
| gatewayAPIDTO.setSequencesToBeRemove( | |
| addStringToList(sandboxEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); | |
| } | |
| public static void setEndpointSequencesToBeRemoved(API api, GatewayAPIDTO gatewayAPIDTO) { | |
| if (api == null) { | |
| throw new IllegalArgumentException("API cannot be null"); | |
| } | |
| if (gatewayAPIDTO == null) { | |
| throw new IllegalArgumentException("GatewayAPIDTO cannot be null"); | |
| } | |
| String productionEndpointSequence = | |
| APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE | |
| + APIConstants.APIEndpoint.PRODUCTION; | |
| gatewayAPIDTO.setSequencesToBeRemove( | |
| addStringToList(productionEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); | |
| String sandboxEndpointSequence = | |
| APIUtil.getEndpointSequenceName(api) + AIAPIConstants.ENDPOINT_SEQUENCE | |
| + APIConstants.APIEndpoint.SANDBOX; | |
| gatewayAPIDTO.setSequencesToBeRemove( | |
| addStringToList(sandboxEndpointSequence, gatewayAPIDTO.getSequencesToBeRemove())); | |
| } |
|
|
||
| String apiKeyType = (String) messageContext.getProperty(org.wso2.carbon.apimgt.impl.APIConstants.API_KEY_TYPE); | ||
|
|
||
| List<ModelEndpointDTO> selectedEndpoints = org.wso2.carbon.apimgt.impl.APIConstants.API_KEY_TYPE_PRODUCTION |
There was a problem hiding this comment.
import org.wso2.carbon.apimgt.impl.APIConstants
| : endpoints.getSandbox(); | ||
|
|
||
| if (selectedEndpoints == null || selectedEndpoints.isEmpty()) { | ||
| log.debug("RoundRobin policy is not set for " + apiKeyType + ", bypassing mediation."); |
| @@ -420,7 +420,7 @@ public byte[] getArtifactProject(String apiId, String revisionId, String organiz | |||
| APIDTO apiDtoToReturn = APIMappingUtil.fromAPItoDTO(api, true, apiProvider); | |||
| File apiProject = ExportUtils.exportApi( | |||
| apiProvider, apiIdentifier, apiDtoToReturn, api, userName, | |||
| ExportFormat.YAML, true, true, StringUtils.EMPTY, organization | |||
| ExportFormat.YAML, true, true, StringUtils.EMPTY, organization, false | |||
There was a problem hiding this comment.
do we need this change
Summary by CodeRabbit
New Features
Improvements
Documentation