Nextgen Devportal Configurations #12750
Conversation
piyumaldk
requested review from
tharindu1st,
AnuGayan,
chamilaadhi,
Arshardh,
dushaniw and
tgtshanika
as code owners
piyumaldk
force-pushed
the
nextgen-devportal
branch
from
80ff980 to
9722ed4
Compare
|
|
||
| import java.util.List; | ||
|
|
||
| public class ApiMetaDataDTO { |
| */ | ||
| public interface NewDevPortalHandler { | ||
|
|
||
| boolean isNewPortalEnabled(); |
There was a problem hiding this comment.
we don't need this at interface method
There was a problem hiding this comment.
change to DevPortalHandler
piyumaldk
force-pushed
the
nextgen-devportal
branch
from
c08e7a6 to
3c53605
Compare
WalkthroughThis pull request introduces significant changes to the API management system. The update methods for both organization and API theme statuses now return an Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant OTS as OrgThemesApiServiceImpl
participant AA as APIAdmin (Impl)
participant DAO as ApiMgtDAO
participant DP as DevPortalHandlerV2Impl
C->>OTS: updateOrgThemeStatus(id, contentPublishStatusDTO)
OTS->>AA: updateOrgThemeStatus(organization, action)
AA->>DAO: Update theme status in DB
DAO-->>AA: Return InputStream
OTS->>DP: If DEVPORTAL_V2, call publish/unpublish org content
DP-->>OTS: Processing result or error
OTS-->>C: Return InputStream response (or error)
sequenceDiagram
participant C as Client
participant APS as ApisApiServiceImpl
participant AP as APIProvider (Impl)
participant DAO as ApiMgtDAO
participant DP as DevPortalHandlerV2Impl
C->>APS: updateApiThemeStatus(apiId, id, contentPublishStatusDTO)
APS->>AP: updateApiThemeStatus(organization, action, apiId)
AP->>DAO: Perform DB update and generate InputStream response
DAO-->>AP: Return InputStream
APS->>DP: If DEVPORTAL_V2, call publish/unpublish API metadata
DP-->>APS: Processing result or error
APS-->>C: Return response with InputStream (or error)
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: 5
🧹 Nitpick comments (27)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/DevPortalConstants.java (1)
22-23: Consider movingJAVA_IO_TMPDIRto a more generic constants class.While
DEVPORTAL_V2is appropriately placed here, the system property constantJAVA_IO_TMPDIRmight be better suited in a more generic constants class since it's not specific to the Developer Portal functionality.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java (1)
4015-4016: Consider adding index hints for performanceThe queries check for theme usage by counting occurrences. For large tables, this could benefit from proper indexing.
Consider adding index hints to optimize the COUNT queries:
-CHECK_IF_DRAFTED_ORG_THEME_IS_USED = "SELECT COUNT(*) FROM AM_DEVPORTAL_ORG_CONTENT WHERE DRAFTED_ARTIFACT = ? AND ORGANIZATION = ?"; +CHECK_IF_DRAFTED_ORG_THEME_IS_USED = "SELECT /*+ INDEX(AM_DEVPORTAL_ORG_CONTENT IDX_ORG_DRAFTED_ARTIFACT) */ COUNT(*) FROM AM_DEVPORTAL_ORG_CONTENT WHERE DRAFTED_ARTIFACT = ? AND ORGANIZATION = ?"; -CHECK_IF_DRAFTED_API_THEME_IS_USED = "SELECT COUNT(*) FROM AM_DEVPORTAL_API_CONTENT WHERE DRAFTED_ARTIFACT = ? AND ORGANIZATION = ? AND API_UUID = ?"; +CHECK_IF_DRAFTED_API_THEME_IS_USED = "SELECT /*+ INDEX(AM_DEVPORTAL_API_CONTENT IDX_API_DRAFTED_ARTIFACT) */ COUNT(*) FROM AM_DEVPORTAL_API_CONTENT WHERE DRAFTED_ARTIFACT = ? AND ORGANIZATION = ? AND API_UUID = ?";components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dto/devportal/ApiMetaDataDTO.java (3)
2-2: Update the copyright year to the current 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.
24-26: Enhance class documentation with field descriptions.While the class comment provides a basic overview, it would be more helpful to include descriptions of the main fields and their purposes.
/** * This class used for the API Metadata data structure which is expected by Developer Portal V2. + * + * @apiInfo Contains basic API information including name, version, visibility, and ownership details + * @subscriptionPolicies List of subscription policies available for the API + * @endPoints Contains sandbox and production endpoint URLs for the API */
30-30: Consider using a more specific type for subscription policies.Using
List<Map<String, String>>for subscription policies makes the structure less type-safe and harder to maintain. Consider creating a dedicated DTO for subscription policies.public class SubscriptionPolicyDTO { private String policyName; private String description; private String quotaType; private String quota; private String unit; // Add getters and setters }Then update the field:
- private List<Map<String, String>> subscriptionPolicies; + private List<SubscriptionPolicyDTO> subscriptionPolicies;components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java (6)
18031-18059: Consider handling non-PUBLISH/UNPUBLISH actions more explicitly.
While the logic caters toPUBLISHorUNPUBLISH, you might want to explicitly handle or reject other action values. This helps fail fast when encountering unexpected scenarios.
18088-18093: Adjust log level or wording when user lacks a drafted theme.
Usinglog.warnand referencing "User does not have the theme as drafted" might not reflect an actual warning if the user simply doesn't have a drafted theme. You could consider a more user-friendly message or a different log level (e.g., info) to distinguish normal from genuinely problematic scenarios.
18290-18290: Use standardized exception handling for missing artifacts.
Throwing a rawSQLExceptionhere mixes DB-level and application-level concerns. Consider throwing or wrapping it in a domain-specificAPIManagementExceptionwith an appropriate error code.
18429-18493: Validate uniqueness or collision handling in reference ID methods.
When adding a reference ID (refId) or removing it, ensure collisions don’t occur if multiple threads or processes call these methods for the same API simultaneously. Consider adding transaction-level locks or constraints in the schema if concurrency might be an issue.
18535-18564: Consider deduplicating the theme update logic for APIs and Orgs.
The logic inupdateApiThemeStatusstrongly resemblesupdateOrgThemeStatus. Consolidating them into a shared helper could eliminate repetitive code and reduce maintenance overhead.
18594-18598: Improve the user-facing error when the drafted theme is missing.
If the user never actually created a drafted theme, throwing "User does not have the theme as drafted" may be expected rather than an error condition. Consider clarifying the message or the log level.components/apimgt/org.wso2.carbon.apimgt.rest.api.admin.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/admin/v1/impl/OrgThemesApiServiceImpl.java (4)
119-122: ValidateportalTyperetrieval.
Consider adding a null or empty check for the retrievedportalTypein case the config property isn't set, to avoid potential NullPointerExceptions or empty property lookups.if (portalType == null || portalType.isEmpty()) { // handle default fallback or throw an exception }
123-134: Validate content usage for publish/unpublish.
This block correctly integrates withDevPortalHandlerV2Impl. However, if a large theme package is provided, consider adding safeguards (e.g., size validations) to avoid memory pressure during the streaming/copying process.
135-136: Refine exception message.
The error message says "Failed to update API theme status" but the class context is specifically "Organization theme status." Consider renaming to maintain consistency and clarity.- throw new APIManagementException("Failed to update API theme status", e); + throw new APIManagementException("Failed to update organization theme status", e);
138-141: Clarify precondition failure message.
When Next Gen Devportal is not enabled, returning aPRECONDITION_FAILEDis appropriate. Consider elaborating or linking documentation for how to switch to or enable the Next Gen Devportal, so users can self-remediate more easily.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandlerV2Impl.java (5)
72-94: Singleton pattern for DevPortalHandlerV2Impl.
The nestedHolderclass technique is a thread-safe and lazy-loaded approach. However, if the class evolves into requiring constructor parameters, a more flexible approach (e.g., dependency injection) might be preferable.
199-216: Add logging for partial uploads.
Publishing organization content is a multi-step process. Log intermediate steps or partial statuses if the upload or HTTP call fails, especially for large archives.
381-418: Improve error messages for missing image files.
An exception is thrown if theimagesfolder is not found. Consider clarifying the allowed file structure in user-facing documentation or logs, plus verifying whether the presence of images is mandatory for success.
422-424: Centralize SSL configuration.
Using a dedicated builder for SSL in each method is consistent, but centralizing these settings or caching the HTTP client can reduce overhead. If new TLS or cipher requirements arise, we can configure them in one place.
462-481: Combine repeated HTTP request patterns.
apiPostAction,apiPutAction, etc., share a similar pattern. Consider extracting a common utility to avoid duplication and to standardize handling (e.g., request headers, logging). This can enhance maintainability.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/LifeCycleUtils.java (1)
82-106: Publish/Unpublish logic for DevPortal V2.
- Publishing references are added for
APIConstants.PUBLISHandAPIConstants.REPUBLISH. Verify that the correct actions are always passed from callers (no uppercase/lowercase mismatches).- Consider more robust error handling if
refIdorapiis null.- Removing references upon unpublish is correct—ensure partial unpublish failures are logged or retried.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (2)
1061-1075: Verify DevPortal V2 integration error handling.The error handling could be improved by:
- Adding more detailed error messages
- Considering retries for transient failures
- Adding metrics/monitoring for DevPortal operations
- log.error(e.getMessage()); + String errorMsg = String.format("Error updating API metadata in DevPortal V2 for API: %s. Error: %s", + api.getId(), e.getMessage()); + log.error(errorMsg, e); + // TODO: Add metrics tracking + // TODO: Consider implementing retry mechanismAlso applies to: 2679-2693
1061-1075: Consider adding transaction handling for DevPortal operations.The metadata updates should be transactional to maintain consistency between API Manager and DevPortal.
Consider:
- Implementing a transaction manager
- Adding compensating actions for failures
- Maintaining an audit log of DevPortal operations
- Implementing a reconciliation mechanism
Also applies to: 2679-2693
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandler.java (1)
1-100: LGTM! Well-structured interface with comprehensive documentation.The interface provides a clear and well-documented API for managing developer portal interactions. The method signatures are consistent, and the exception handling is appropriate.
Consider adding
@sincetags in the JavaDoc to track when these methods were introduced, which helps with API versioning and backward compatibility.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java (1)
1692-1693: LGTM! Consider adding error handling.The change to return an
InputStreamfromupdateOrgThemeStatusaligns with the Next Gen DevPortal requirements. However, consider adding try-catch blocks to handle potential I/O exceptions that may occur when dealing with the input stream.public InputStream updateOrgThemeStatus(String organization, String action) throws APIManagementException { - return apiMgtDAO.updateOrgThemeStatus(organization, action); + try { + return apiMgtDAO.updateOrgThemeStatus(organization, action); + } catch (Exception e) { + throw new APIManagementException("Error updating organization theme status for org: " + + organization + ", action: " + action, e); + } }components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/ApisApiServiceImpl.java (2)
5042-5062: Consider adding input validation for label operations.While the methods handle basic label operations, they should validate:
- The label list is not null or empty
- Individual label names follow naming conventions
- Maximum number of labels that can be attached
public Response attachLabelsToAPI(String apiId, RequestLabelListDTO requestLabelListDTO, MessageContext messageContext) throws APIManagementException { + // Validate input + if (requestLabelListDTO == null || requestLabelListDTO.getLabels() == null || + requestLabelListDTO.getLabels().isEmpty()) { + throw new APIManagementException("Label list cannot be empty", + ExceptionCodes.INVALID_LABEL_LIST); + } + // Validate each label + for (String label : requestLabelListDTO.getLabels()) { + if (!APIUtil.isValidLabelName(label)) { + throw new APIManagementException("Invalid label name: " + label, + ExceptionCodes.INVALID_LABEL_NAME); + } + } APIProvider apiProvider = RestApiCommonUtil.getLoggedInUserProvider();
5064-5071: Add audit logging for theme operations.The theme management methods should include audit logging to track:
- Theme deletions
- Theme content retrievals
- Theme imports
- Theme status changes
public Response deleteApiTheme(String apiId, String id, MessageContext messageContext) throws APIManagementException { String tenantDomain = RestApiCommonUtil.getLoggedInUserTenantDomain(); APIProvider apiProvider = RestApiCommonUtil.getLoggedInUserProvider(); apiProvider.deleteApiTheme(tenantDomain, id, apiId); + // Add audit log + APIUtil.logAuditMessage("API_THEME", "DELETE", + String.format("Theme %s deleted for API %s", id, apiId)); return Response.status(Response.Status.OK).entity("Theme deleted successfully").build(); }Also applies to: 5074-5093, 5096-5123, 5126-5132
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
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/APIProvider.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIAdminImpl.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java(4 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandler.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandlerV2Impl.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java(11 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/DevPortalConstants.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/DevPortalProcessingConstants.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dto/devportal/ApiMetaDataDTO.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/LifeCycleUtils.java(2 hunks)components/apimgt/org.wso2.carbon.apimgt.rest.api.admin.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/admin/v1/impl/OrgThemesApiServiceImpl.java(2 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(7 hunks)features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/DevPortalProcessingConstants.java
🔇 Additional comments (26)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/DevPortalConstants.java (1)
27-27: LGTM!The
UNPUBLISHconstant follows the established naming pattern and complements the existingPUBLISHconstant, forming a complete set of publishing action constants.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java (2)
4008-4012: LGTM: New SQL queries for artifact managementThe new SQL queries for managing artifact types and drafted/published IDs follow good practices:
- Clear and descriptive query names
- Consistent with existing naming conventions
- Proper use of parameterized queries for security
4021-4023: LGTM: Well-structured reference ID management queriesThe new SQL queries for managing API references are well-designed:
- Clear CRUD operations (Add/Get/Remove)
- Proper use of composite keys (API_UUID + ORGANIZATION)
- Consistent with existing query patterns
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dto/devportal/ApiMetaDataDTO.java (1)
196-218: LGTM!The getter and setter implementations are correct and follow standard Java conventions.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java (5)
18063-18074: Null check for 'artifactContentBytes' before returning.
When the action is neitherPUBLISHnorUNPUBLISHor an exception is raised prematurely,artifactContentBytesremainsnullbefore being converted into aByteArrayInputStream. Consider returning an empty stream or throwing an exception to avoid returning a null-based stream.
18300-18352: Ensure consistent error reporting for artifact removal failures.
The approach to query the artifact type and then conditionally remove it is good. However, if removal fails or if the artifact type is invalid, consider throwing a domain-specific exception to keep the exception flow consistent with the rest of the DAO methods.
18381-18381: Double-check preparation of parameters for the drafted-org-theme query.
The current code setsthemeIdtwice. Verify that the underlying SQL statement indeed requires both parameters for correct filtering, and ensure there’s no oversight in usage of placeholders.
18571-18579: Check handling of 'artifactContentBytes' when the theme artifact is not found.
Similar toupdateOrgThemeStatus, handle the case whereartifactContentBytesmight remainnullif no drafted or published artifact is found for the API.
18807-18812: Re-check indexing when preparing 'themeId' parameters.
Similar to the other drafted checks, ensure that placeholders and parameter assignments match the SQL statement. A mismatch could lead to silent failures or incorrect results.components/apimgt/org.wso2.carbon.apimgt.rest.api.admin.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/admin/v1/impl/OrgThemesApiServiceImpl.java (2)
24-26: Imports look consistent and necessary.
No issues detected with these new import statements, as each import is directly used in the subsequent code.
117-118: Ensure downstream callers are updated for new method signature.
Changing the method signature to accept aContentPublishStatusDTOwith an action parameter seems valid. Verify that no existing caller code is inadvertently passing the old signature.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandlerV2Impl.java (3)
1-17: File header and licensing statements are in order.
No issues noted here. Make sure to keep these adequately updated if future license changes occur.
113-138: Check for non-2xx statuses.
Currently, success is assumed based on status code 201. If the remote service might return other 2xx codes (e.g., 202), consider adjusting the check to allow them. Also consider logging response body details on errors to aid debugging.
251-273: Ensure concurrency safety withorgIdMap.
ConcurrentHashMapis used, but if multiple threads callgetOrgIdsimultaneously for different organizations that don't yet exist inorgIdMap, the function calls the externalorgGetAction. Confirm that no race conditions cause repeated calls for the same tenant before the map is updated.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/utils/LifeCycleUtils.java (2)
11-13: Updated imports appear justified.
Imports fororg.wso2.carbon.apimgt.impl.*andDevPortalConstantsare used throughout the new DevPortal logic. No concerns.
78-81: Guard against null or empty portal type.
When fetching the property viagetFirstProperty(APIConstants.API_STORE_TYPE), ensure a fallback or error handling if it’s not set or empty, to avoid logic flow confusion.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (5)
126-126: LGTM! Import statement added for DevPortal constants.The addition of the DevPortal constants import is appropriate for the upcoming DevPortal V2 integration changes.
1061-1075: New logic added for DevPortal V2 integration.The code adds a new check to handle API metadata updates for DevPortal V2:
- Checks if portal type is DEVPORTAL_V2 from configuration
- For published APIs, updates metadata in DevPortal using DevPortalHandlerV2Impl
- Includes error handling for APIManagementException
The implementation follows good practices by:
- Checking configuration before execution
- Using proper error handling
- Logging errors appropriately
2679-2693: New logic added for DevPortal V2 unpublish handling.The code adds handling for unpublishing API metadata from DevPortal V2:
- Checks if portal type is DEVPORTAL_V2 from configuration
- Unpublishes metadata and removes reference ID
- Includes proper error handling
The implementation follows good practices by:
- Checking configuration before execution
- Using proper error handling
- Logging errors appropriately
8179-8182: Method signature change for updateApiThemeStatus.The return type has been changed from
voidtoInputStreamto match interface changes.The change is consistent with the interface modifications mentioned in the AI summary.
1061-1075:❓ Verification inconclusive
Verify configuration handling for DevPortal V2.
The code should validate the configuration values more thoroughly:
Also applies to: 2679-2693
🏁 Script executed:
#!/bin/bash # Description: Verify DevPortal V2 configuration settings # Check if API_STORE_TYPE property exists and has valid values rg -A 5 "API_STORE_TYPE.*=.*DEVPORTAL_V2" # Check for any hardcoded DevPortal V2 URLs or endpoints rg "devportal.*v2.*url" -iLength of output: 72
Manual verification needed for DevPortal V2 config validation
The regex verification yielded no results, so it’s unclear if the expected configuration value is present. Please manually confirm that:
- The configuration property (API_STORE_TYPE) is actually set to a value corresponding to
DevPortalConstants.DEVPORTAL_V2in your configuration sources.- There are no additional hardcoded URLs/endpoints for DevPortal V2 that might bypass this check (applies also to lines 2679–2693).
Once you’ve verified these settings, consider enhancing the validation logic (or adding appropriate logging/unit tests) to catch misconfigurations earlier.
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIAdmin.java (1)
548-551: LGTM! Return type change enhances theme management functionality.The change from
voidtoInputStreamallows returning the theme content stream directly, which is a more efficient approach than having separate calls for status update and content retrieval.components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIProvider.java (1)
2179-2182: LGTM! Consistent return type change for theme management.The change from
voidtoInputStreamaligns with the similar change in theAPIAdmininterface, maintaining consistency across the API management interfaces.features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2 (1)
592-594: LGTM! New NextGen Portal configuration block added.The changes introduce two new configuration elements for the Next Generation Developer Portal:
PortalType: Defines the type of developer portal to be usedPortalAccessURL: Specifies the access URL for the portalThese additions enhance the configuration capabilities by providing necessary settings for the NextGen Developer Portal integration.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (1)
936-939: LGTM! Well-structured constants for Next Gen Developer Portal configuration.The new constants are appropriately placed in the API_STORE section and follow the established naming conventions. They provide the necessary configuration options for enabling the Next Gen Developer Portal, specifying its type, and configuring its access URL.
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)
51-51: LGTM: Import statements are properly organized.The added imports are correctly placed in their respective groups (impl, dao, internal) and are necessary for the new functionality.
Also applies to: 55-55, 65-65
| public static class ApiInfo { | ||
| private String referenceID; | ||
| private String apiName; | ||
| private String orgName; | ||
| private String provider; | ||
| private String apiCategory; | ||
| private String apiDescription; | ||
| private String visibility; | ||
| private List<String> visibleGroups; | ||
| private Owners owners; | ||
| private String apiVersion; | ||
| private String apiType; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add field validation and consider using enums for constrained values.
- Critical fields like
referenceID,apiName, andapiVersionshould be validated. - Fields like
visibilityandapiTypewould benefit from enum types to ensure valid values.
public enum APIVisibility {
PUBLIC,
PRIVATE,
RESTRICTED
}
public enum APIType {
HTTP,
WEBSOCKET,
GRAPHQL
// Add other supported types
}Then update the fields:
- private String visibility;
+ private APIVisibility visibility;
- private String apiType;
+ private APIType apiType;Also add validation in setters:
public void setReferenceID(String referenceID) {
if (referenceID == null || referenceID.trim().isEmpty()) {
throw new IllegalArgumentException("Reference ID cannot be null or empty");
}
this.referenceID = referenceID;
}| public static class EndPoints { | ||
| private String sandboxURL; | ||
| private String productionURL; | ||
|
|
||
|
|
||
| public String getSandboxURL() { | ||
| return sandboxURL; | ||
| } | ||
|
|
||
| public void setSandboxURL(String sandboxURL) { | ||
| this.sandboxURL = sandboxURL; | ||
| } | ||
|
|
||
| public String getProductionURL() { | ||
| return productionURL; | ||
| } | ||
|
|
||
| public void setProductionURL(String productionURL) { | ||
| this.productionURL = productionURL; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add URL validation and remove extra blank line.
- The URLs should be validated to ensure they are well-formed.
- Remove the extra blank line at line 178.
public void setSandboxURL(String sandboxURL) {
+ if (sandboxURL != null && !sandboxURL.trim().isEmpty()) {
+ try {
+ new java.net.URL(sandboxURL);
+ } catch (java.net.MalformedURLException e) {
+ throw new IllegalArgumentException("Invalid sandbox URL", e);
+ }
+ }
this.sandboxURL = sandboxURL;
}
public void setProductionURL(String productionURL) {
+ if (productionURL != null && !productionURL.trim().isEmpty()) {
+ try {
+ new java.net.URL(productionURL);
+ } catch (java.net.MalformedURLException e) {
+ throw new IllegalArgumentException("Invalid production URL", e);
+ }
+ }
this.productionURL = productionURL;
}Committable suggestion skipped: line range outside the PR's diff.
| @Override | ||
| public void publishAPIContent(String organization, String refId, InputStream content, String apiName) throws APIManagementException { | ||
| String tempPath = | ||
| System.getProperty(DevPortalConstants.JAVA_IO_TMPDIR) + File.separator + "publishing-api-themes"; | ||
| String tempFile = apiName + APIConstants.ZIP_FILE_EXTENSION; | ||
| File apiThemeArchive = new File(tempPath, tempFile); | ||
| try { | ||
| FileUtils.copyInputStreamToFile(content, apiThemeArchive); | ||
| String imageMetadata = imageMetadataGenerator(apiThemeArchive); | ||
| String orgId = getOrgId(organization); | ||
| HttpResponseData responseData = apiContentPutAction(orgId, refId, apiThemeArchive, imageMetadata); | ||
| if (responseData.getStatusCode() == 201) { | ||
| log.info("Successfully published API content for API: " + apiName + " and organization: " + organization); | ||
| } else { | ||
| throw new APIManagementException("Failed to publish API content for API: " + apiName + " and organization: " + organization); | ||
| } | ||
| } catch (IOException e) { | ||
| throw new APIManagementException("Error while processing Input stream: " + e.getMessage(), e); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Manage large theme zip uploads gracefully.
While copying the input stream to a file, large files can cause memory or disk exhaustion. Consider adding size checks or quotas. Also ensure the temporary directory is on a partition with sufficient space.
| private static SSLConnectionSocketFactory generateSSLSF() throws APIManagementException { | ||
| ServerConfiguration serverConfig = ServerConfiguration.getInstance(); | ||
| char[] keyStorePassword = serverConfig.getFirstProperty("Security.KeyStore.Password").toCharArray(); | ||
| char[] keyPassword = serverConfig.getFirstProperty("Security.KeyStore.KeyPassword").toCharArray(); | ||
| char[] trustStorePassword = serverConfig.getFirstProperty("Security.TrustStore.Password").toCharArray(); | ||
| try { | ||
| // Key Store | ||
| KeyStore keyStore; | ||
| String keyStoreType = serverConfig.getFirstProperty("Security.KeyStore.Type"); | ||
| if (keyStoreType != null) { | ||
| keyStore = KeyStore.getInstance(keyStoreType); | ||
| } else { | ||
| keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); | ||
| } | ||
| try (FileInputStream keyStoreStream = new FileInputStream( | ||
| serverConfig.getFirstProperty("Security.KeyStore.Location"))) { | ||
| keyStore.load(keyStoreStream, keyStorePassword); | ||
| } | ||
|
|
||
| // Trust Store | ||
| KeyStore trustStore; | ||
| String trustStoreType = serverConfig.getFirstProperty("Security.TrustStore.Type"); | ||
| if (trustStoreType != null) { | ||
| trustStore = KeyStore.getInstance(trustStoreType); | ||
| } else { | ||
| trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); | ||
| } | ||
| try (FileInputStream trustStoreStream = new FileInputStream( | ||
| serverConfig.getFirstProperty("Security.TrustStore.Location"))) { | ||
| trustStore.load(trustStoreStream, trustStorePassword); | ||
| } | ||
|
|
||
| return new SSLConnectionSocketFactory( | ||
| SSLContexts.custom() | ||
| .loadKeyMaterial( | ||
| keyStore, | ||
| keyPassword, | ||
| (aliases, socket) -> serverConfig.getFirstProperty("Security.KeyStore.KeyAlias")) | ||
| .loadTrustMaterial(trustStore, null).build()); | ||
| } catch (GeneralSecurityException | IOException e) { | ||
| throw new APIManagementException("Error while certification processing: " + e.getMessage(), e); | ||
| } finally { | ||
| // Clear Sensitive Data From Memory | ||
| Arrays.fill(keyStorePassword, ' '); | ||
| Arrays.fill(keyPassword, ' '); | ||
| Arrays.fill(trustStorePassword, ' '); | ||
| } | ||
| } |
There was a problem hiding this comment.
Key/trust store loading validations.
Ensure the file paths exist and the passwords are valid before attempting load operations. If the file is missing or corrupt, fail with more descriptive messages. Clear or mask the store path and password from logs to prevent sensitive data leakage.
| APIManagerConfiguration apiManagerConfiguration = | ||
| ServiceReferenceHolder.getInstance().getAPIManagerConfigurationService().getAPIManagerConfiguration(); | ||
| String portalType = apiManagerConfiguration.getFirstProperty(APIConstants.API_STORE_TYPE); | ||
|
|
||
| if (DevPortalConstants.DEVPORTAL_V2.equals(portalType)) { | ||
| DevPortalHandler devPortalHandler = DevPortalHandlerV2Impl.getInstance(); | ||
| String tenantDomain = RestApiCommonUtil.getLoggedInUserTenantDomain(); | ||
| String action = contentPublishStatusDTO.getAction().value(); | ||
| APIProvider apiProvider = RestApiCommonUtil.getLoggedInUserProvider(); | ||
|
|
||
| try (InputStream content = apiProvider.updateApiThemeStatus(tenantDomain, action, apiId)) { | ||
| String refId = ApiMgtDAO.getInstance().getRefId(apiId, tenantDomain); | ||
| if (DevPortalConstants.PUBLISH.equals(action)) { | ||
| API api = apiProvider.getLightweightAPIByUUID(apiId, tenantDomain); | ||
| devPortalHandler.publishAPIContent(tenantDomain, refId, content, api.getId().getApiName()); | ||
| } else if (DevPortalConstants.UNPUBLISH.equals(action)) { | ||
| devPortalHandler.unpublishAPIContent(tenantDomain, refId); | ||
| } | ||
| } catch (IOException e) { | ||
| throw new APIManagementException("Failed to update API theme status", e); | ||
| } | ||
| return Response.status(Response.Status.OK).entity("Status updated successfully").build(); | ||
| } else { | ||
| return Response.status(Response.Status.PRECONDITION_FAILED).entity("Please enable Next Gen Devportal " + | ||
| "to publish or unpublish").build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for content stream operations.
The updateApiThemeStatus method has several areas that need improvement:
- The try-with-resources block should handle IOException separately from APIManagementException
- The content stream should be validated before processing
- The action value should be validated before processing
try (InputStream content = apiProvider.updateApiThemeStatus(tenantDomain, action, apiId)) {
+ // Validate content stream
+ if (content == null) {
+ throw new APIManagementException("Theme content cannot be null");
+ }
+ // Validate action
+ if (!DevPortalConstants.PUBLISH.equals(action) &&
+ !DevPortalConstants.UNPUBLISH.equals(action)) {
+ throw new APIManagementException("Invalid action: " + action);
+ }
String refId = ApiMgtDAO.getInstance().getRefId(apiId, tenantDomain);
if (DevPortalConstants.PUBLISH.equals(action)) {
API api = apiProvider.getLightweightAPIByUUID(apiId, tenantDomain);
devPortalHandler.publishAPIContent(tenantDomain, refId, content, api.getId().getApiName());
} else if (DevPortalConstants.UNPUBLISH.equals(action)) {
devPortalHandler.unpublishAPIContent(tenantDomain, refId);
}
-} catch (IOException e) {
- throw new APIManagementException("Failed to update API theme status", e);
+} catch (IOException e) {
+ throw new APIManagementException("Error processing theme content", e);
+} catch (APIManagementException e) {
+ throw new APIManagementException("Error updating theme status: " + e.getMessage(), 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.
| APIManagerConfiguration apiManagerConfiguration = | |
| ServiceReferenceHolder.getInstance().getAPIManagerConfigurationService().getAPIManagerConfiguration(); | |
| String portalType = apiManagerConfiguration.getFirstProperty(APIConstants.API_STORE_TYPE); | |
| if (DevPortalConstants.DEVPORTAL_V2.equals(portalType)) { | |
| DevPortalHandler devPortalHandler = DevPortalHandlerV2Impl.getInstance(); | |
| String tenantDomain = RestApiCommonUtil.getLoggedInUserTenantDomain(); | |
| String action = contentPublishStatusDTO.getAction().value(); | |
| APIProvider apiProvider = RestApiCommonUtil.getLoggedInUserProvider(); | |
| try (InputStream content = apiProvider.updateApiThemeStatus(tenantDomain, action, apiId)) { | |
| String refId = ApiMgtDAO.getInstance().getRefId(apiId, tenantDomain); | |
| if (DevPortalConstants.PUBLISH.equals(action)) { | |
| API api = apiProvider.getLightweightAPIByUUID(apiId, tenantDomain); | |
| devPortalHandler.publishAPIContent(tenantDomain, refId, content, api.getId().getApiName()); | |
| } else if (DevPortalConstants.UNPUBLISH.equals(action)) { | |
| devPortalHandler.unpublishAPIContent(tenantDomain, refId); | |
| } | |
| } catch (IOException e) { | |
| throw new APIManagementException("Failed to update API theme status", e); | |
| } | |
| return Response.status(Response.Status.OK).entity("Status updated successfully").build(); | |
| } else { | |
| return Response.status(Response.Status.PRECONDITION_FAILED).entity("Please enable Next Gen Devportal " + | |
| "to publish or unpublish").build(); | |
| } | |
| } | |
| APIManagerConfiguration apiManagerConfiguration = | |
| ServiceReferenceHolder.getInstance().getAPIManagerConfigurationService().getAPIManagerConfiguration(); | |
| String portalType = apiManagerConfiguration.getFirstProperty(APIConstants.API_STORE_TYPE); | |
| if (DevPortalConstants.DEVPORTAL_V2.equals(portalType)) { | |
| DevPortalHandler devPortalHandler = DevPortalHandlerV2Impl.getInstance(); | |
| String tenantDomain = RestApiCommonUtil.getLoggedInUserTenantDomain(); | |
| String action = contentPublishStatusDTO.getAction().value(); | |
| APIProvider apiProvider = RestApiCommonUtil.getLoggedInUserProvider(); | |
| try (InputStream content = apiProvider.updateApiThemeStatus(tenantDomain, action, apiId)) { | |
| // Validate content stream | |
| if (content == null) { | |
| throw new APIManagementException("Theme content cannot be null"); | |
| } | |
| // Validate action | |
| if (!DevPortalConstants.PUBLISH.equals(action) && | |
| !DevPortalConstants.UNPUBLISH.equals(action)) { | |
| throw new APIManagementException("Invalid action: " + action); | |
| } | |
| String refId = ApiMgtDAO.getInstance().getRefId(apiId, tenantDomain); | |
| if (DevPortalConstants.PUBLISH.equals(action)) { | |
| API api = apiProvider.getLightweightAPIByUUID(apiId, tenantDomain); | |
| devPortalHandler.publishAPIContent(tenantDomain, refId, content, api.getId().getApiName()); | |
| } else if (DevPortalConstants.UNPUBLISH.equals(action)) { | |
| devPortalHandler.unpublishAPIContent(tenantDomain, refId); | |
| } | |
| } catch (IOException e) { | |
| throw new APIManagementException("Error processing theme content", e); | |
| } catch (APIManagementException e) { | |
| throw new APIManagementException("Error updating theme status: " + e.getMessage(), e); | |
| } | |
| return Response.status(Response.Status.OK).entity("Status updated successfully").build(); | |
| } else { | |
| return Response.status(Response.Status.PRECONDITION_FAILED).entity("Please enable Next Gen Devportal to publish or unpublish").build(); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandlerV2Impl.java (3)
76-84: Handle potential SSL initialization failure more gracefully.
Currently, if SSL initialization fails, aRuntimeExceptionis thrown, which might crash the entire application on startup. Consider adding retries or fallback mechanisms to allow the system to continue in a degraded (non-SSL) mode or provide more controlled error handling.
242-256: Implement the SOAP definition retrieval.
Currently, there is a TODO comment for SOAP APIs. Providing an appropriate WSDL or relevant definition would ensure completeness and functional parity for SOAP-based APIs.Do you want me to provide a sample SOAP definition handling implementation or open a new issue to track this enhancement?
475-516: Refactor repeated HTTP request creation logic.
Multiple methods (e.g.,apiPostAction,apiPutAction,apiContentPutAction,orgContentPutAction) perform very similar HTTP request tasks. Consider extracting the common code into a reusable utility method to reduce duplication and simplify maintenance.Also applies to: 518-556, 558-598
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandlerV2Impl.java(1 hunks)
🔇 Additional comments (4)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/DevPortalHandlerV2Impl.java (4)
70-70: Great introduction of a new Dev Portal handler class.
This class provides a dedicated implementation for handling Next Gen DevPortal interactions.
88-91: Validate concurrency usage for lazy initialization.
The static “Holder” pattern is generally safe for lazy initialization, but please confirm that it aligns with your system’s concurrency and class-loading needs.
176-189: Manage large theme zip uploads gracefully.
Copying the entire input stream to a file can cause memory/disk exhaustion if the file is large or maliciously sized. Consider adding size checks, stream throttling, or quotas to prevent resource exhaustion.
600-647: Ensure keystore and truststore file validations.
Before loading key/trust stores, confirm the paths exist and the files are valid. Also, verify that the alias is present. Log any issues with caution so as not to reveal sensitive info.
| private static String imageMetadataGenerator(File zipFile) throws APIManagementException, IOException { | ||
| File tempDirectory = null; | ||
| try { | ||
| tempDirectory = CommonUtil.createTempDirectory(null); | ||
| String extractedFolderName = CommonUtil.extractArchive(zipFile, tempDirectory.getAbsolutePath()); | ||
| File imagesFolder = new File(tempDirectory, extractedFolderName + "/images"); | ||
| if (!imagesFolder.exists() || !imagesFolder.isDirectory()) { | ||
| throw new APIManagementException("Images folder not found in ZIP"); | ||
| } | ||
|
|
||
| String apiIconName = null; | ||
| String apiHeroName = null; | ||
| for (File file : Objects.requireNonNull(imagesFolder.listFiles())) { | ||
| String fileName = file.getName().toLowerCase(); | ||
| if (fileName.startsWith("api-icon")) { | ||
| apiIconName = file.getName(); | ||
| } else if (fileName.startsWith("api-hero")) { | ||
| apiHeroName = file.getName(); | ||
| } | ||
| } | ||
|
|
||
| JSONObject jsonResponse = new JSONObject(); | ||
| if (apiIconName != null) { | ||
| jsonResponse.put("api-icon", apiIconName); | ||
| } | ||
| if (apiHeroName != null) { | ||
| jsonResponse.put("api-hero", apiHeroName); | ||
| } | ||
|
|
||
| return jsonResponse.toString(); | ||
| } catch (APIImportExportException e) { | ||
| throw new APIManagementException(e); | ||
| } finally { | ||
| if (tempDirectory != null) { | ||
| FileUtils.deleteDirectory(tempDirectory); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Validate extracted archives to prevent directory traversal attacks.
When extracting files from a zip, ensure that the process does not overwrite arbitrary file system locations outside the intended directory. Use a safe extraction method or validations to mitigate potential security risks.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/internal/APIManagerComponent.java (1)
347-355: 🛠️ Refactor suggestionUnregister tenant listener service in deactivate method.
The tenant listener service registration should be properly cleaned up when the component is deactivated.
Apply this diff to handle service unregistration:
@Deactivate protected void deactivate(ComponentContext componentContext) { if (log.isDebugEnabled()) { log.debug("Deactivating API manager component"); } + try { registration.unregister(); + if (log.isDebugEnabled()) { + log.debug("Successfully unregistered API manager services"); + } + } catch (Exception e) { + log.error("Error unregistering API manager services", e); + } APIManagerFactory.getInstance().clearAll(); }
🧹 Nitpick comments (3)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/TenantListener.java (1)
1-17: Update the copyright year.The copyright year 2025 is incorrect as it's beyond the current year (February 2025).
-/* - * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. +/* + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.org) All Rights Reserved.components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/internal/APIManagerComponent.java (2)
51-51: Replace wildcard import with specific imports.Using wildcard imports reduces code readability and can lead to naming conflicts. Consider explicitly importing the required classes.
-import org.wso2.carbon.apimgt.impl.*; +import org.wso2.carbon.apimgt.impl.TenantListener; +import org.wso2.carbon.apimgt.impl.APIConstants; +import org.wso2.carbon.apimgt.impl.APIManagerAnalyticsConfiguration; +import org.wso2.carbon.apimgt.impl.APIManagerConfiguration; +import org.wso2.carbon.apimgt.impl.APIManagerConfigurationService; +import org.wso2.carbon.apimgt.impl.APIManagerConfigurationServiceImpl; +import org.wso2.carbon.apimgt.impl.APIManagerFactory;
199-200: Improve service registration comment.The current comment is too brief. Add more context about the purpose of the TenantListener service.
- //Registering Tenant Listener + // Register TenantListener service to handle tenant lifecycle events + // This listener logs tenant creation events and can be extended for additional tenant management tasks
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/TenantListener.java(1 hunks)components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/internal/APIManagerComponent.java(3 hunks)
| //Registering Tenant Listener | ||
| bundleContext.registerService(TenantMgtListener.class.getName(), new TenantListener(), null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for tenant listener registration.
The tenant listener service registration should include error handling to ensure proper initialization.
Apply this diff to add error handling:
//Registering Tenant Listener
- bundleContext.registerService(TenantMgtListener.class.getName(), new TenantListener(), null);
+ try {
+ bundleContext.registerService(TenantMgtListener.class.getName(), new TenantListener(), null);
+ if (log.isDebugEnabled()) {
+ log.debug("TenantListener service registered successfully");
+ }
+ } catch (Exception e) {
+ log.error("Error registering TenantListener service", e);
+ throw new RuntimeException("Failed to register TenantListener service", 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.
| //Registering Tenant Listener | |
| bundleContext.registerService(TenantMgtListener.class.getName(), new TenantListener(), null); | |
| //Registering Tenant Listener | |
| try { | |
| bundleContext.registerService(TenantMgtListener.class.getName(), new TenantListener(), null); | |
| if (log.isDebugEnabled()) { | |
| log.debug("TenantListener service registered successfully"); | |
| } | |
| } catch (Exception e) { | |
| log.error("Error registering TenantListener service", e); | |
| throw new RuntimeException("Failed to register TenantListener service", e); | |
| } |
| log.info("Tenant was created"); | ||
| log.info(tenantInfoBean); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve logging implementation.
The current logging implementation has several issues:
- Logs raw object which may not be human-readable
- No debug level checks for performance
- Missing tenant details in the log message
- log.info("Tenant was created");
- log.info(tenantInfoBean);
+ if (log.isDebugEnabled()) {
+ log.debug("Processing tenant creation for: " + tenantInfoBean.getTenantDomain());
+ }
+ log.info("Tenant created successfully - Domain: " + tenantInfoBean.getTenantDomain()
+ + ", Tenant ID: " + tenantInfoBean.getTenantId());📝 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.
| log.info("Tenant was created"); | |
| log.info(tenantInfoBean); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Processing tenant creation for: " + tenantInfoBean.getTenantDomain()); | |
| } | |
| log.info("Tenant created successfully - Domain: " + tenantInfoBean.getTenantDomain() | |
| + ", Tenant ID: " + tenantInfoBean.getTenantId()); |
| @@ -21,17 +21,16 @@ | |||
| import org.apache.commons.io.FileUtils; | |||
| import org.wso2.carbon.apimgt.api.APIManagementException; | |||
| import org.wso2.carbon.apimgt.api.ExceptionCodes; | |||
| import org.wso2.carbon.apimgt.impl.APIAdminImpl; | |||
| import org.wso2.carbon.apimgt.impl.APIConstants; | |||
| import org.wso2.carbon.apimgt.impl.*; | |||
There was a problem hiding this comment.
Remove * packages
Support for Next Gen Devportal
Summary by CodeRabbit
New Features
ApiMetaDataDTOfor structured API metadata management.Enhancements