Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new resource to get the overview markdown content of an API #12942

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AqeelMuhammad
Copy link
Contributor

@AqeelMuhammad AqeelMuhammad commented Feb 17, 2025

Purpose

A new resource /apis/{apiId}/markdown-content is created to fetch the overview markdown content of an API.

In the current implementation, to obtain the markdown content, the documentId related to the overview document needs to be known. So, at least three APIs should be called, including the GET /apis/{apiId} API, to fetch the markdown content.

From the new resource, we can directly obtain the markdown content if isMarkdownOverview is true in the GET /apis/{apiId} API.

Summary by CodeRabbit

  • New Features

    • Introduced enhanced API documentation with Markdown overview support, enabling retrieval of markdown-formatted content for API details.
    • New endpoints allow users to directly access API and API product markdown content with proper response handling.
    • API models now indicate the availability of Markdown documentation, improving content discovery.
    • Configuration updates ensure seamless, secure access to these new functionalities.
  • Tests

    • Added unit tests to verify the correct functionality of markdown content retrieval and endpoint responses.

Copy link

coderabbitai bot commented Mar 4, 2025

Walkthrough

This pull request introduces support for retrieving and managing Markdown overview documentation for APIs and API products. A new method is added to the API management interfaces and implemented across multiple classes, along with corresponding changes to the API model classes by adding an isMarkdownOverview field and its accessors. Additionally, new REST endpoints and configuration updates are introduced to expose this functionality, and tests have been added to verify the new behavior. The registry persistence layer is also updated to process documentation search queries with additional prefixes.

Changes

File(s) Change Summary
.../APIManager.java Added method getMarkdownOverviewContent(String apiId, String organization) throws APIManagementException to the APIManager interface.
.../API.java, .../APIProduct.java, .../DevPortalAPI.java Introduced new Boolean isMarkdownOverview field with corresponding getter and setter methods (and updated toString() in DevPortalAPI).
.../APIConsumerImpl.java, .../AbstractAPIManager.java Added implementations for getMarkdownOverviewContent and modified getAPIorAPIProductByUUID to check for markdown overview documentation.
.../APIConsumerImplTest.java, .../AbstractAPIManagerTestCase.java Added new test methods testGetMarkdownOverviewContent to verify the markdown overview content retrieval functionality.
.../devportal-api.yaml (both REST store and common files) Added new endpoint GET /apis/{apiId}/markdown-content and included the isMarkdownOverview property in the API schema.
.../APIMappingUtil.java Updated API-to-DTO mapping methods to set the isMarkdownOverview property in the APIDTO.
.../RegistryPersistenceImpl.java Enhanced searchDocumentation to handle search queries with the prefix "other:" for matching document types.
.../api-manager.xml.j2, .../api-manager.xml Added new AllowedURI entries for markdown-content endpoints for both APIs and API products.

Sequence Diagram(s)

sequenceDiagram
    participant C as Client
    participant A as ApisApiServiceImpl
    participant AC as APIConsumerImpl
    participant AM as AbstractAPIManager
    participant RP as RegistryPersistenceImpl

    C->>A: GET /apis/{apiId}/markdown-content
    A->>AC: getMarkdownOverviewContent(apiId, organization)
    AC->>AM: getMarkdownOverviewContent(apiId, organization)
    AM->>RP: searchDocumentation(apiId, "other:_overview")
    RP-->>AM: Documentation content (or null)
    AM-->>AC: Markdown content (or null) with error handling
    AC-->>A: Response with markdown content
    A-->>C: HTTP response with content/headers
Loading

Suggested reviewers

  • tgtshanika
  • chamilaadhi
  • dushaniw
  • AnuGayan
  • tharindu1st

Poem

Oh, I hopped through lines of code, so neat,
Where markdown overviews now take the seat.
New methods, fields, and endpoints in sight,
Guiding APIs with a fresh delight.
I nibble bytes with joy each day,
Celebrating our changes in a rabbit’s way!
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (6)
components/apimgt/org.wso2.carbon.apimgt.rest.api.common/src/main/resources/devportal-api.yaml (1)

839-892: New Endpoint Implementation: Markdown Content Retrieval

The new endpoint /apis/{apiId}/markdown-content is added to directly retrieve the API overview markdown document content, which aligns with the PR objectives. The endpoint’s summary, description, parameters, and security settings are clearly defined. One minor suggestion is to verify if the 200 response’s content is intended to return inline markdown text under a specific media type (for example, text/plain) rather than an empty object. If a content schema is required, consider specifying it for better documentation and client clarity.

components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/resources/devportal-api.yaml (1)

839-892: New API Overview Markdown Endpoint Added
The new /apis/{apiId}/markdown-content GET endpoint is clearly defined with an informative summary and description. It follows the style of existing endpoints by using standard parameters (such as apiId, requestedTenant, and If-None-Match) and response codes (200, 304, 404, and 406).

However, the response body definition under the 200 response is currently an empty content object. Since the endpoint is expected to return inline markdown content in text/plain format, consider explicitly specifying the media type and its schema. For example, you might add a content section like this:

-      responses:
-        200:
-          description: |
-            OK.
-            Inline content returned.
-          headers:
-            ETag:
-              description: |
-                Entity Tag of the response resource.
-                Used by caches, or in conditional requests.
-              schema:
-                type: string
-            Last-Modified:
-              description: |
-                Date and time the resource has been modified the last time.
-                Used by caches, or in conditional requests.
-              schema:
-                type: string
-          content: { }
+      responses:
+        200:
+          description: |
+            OK.
+            Inline markdown content returned.
+          headers:
+            ETag:
+              description: Entity Tag of the response resource. Used by caches or in conditional requests.
+              schema:
+                type: string
+            Last-Modified:
+              description: Date and time the resource was last modified.
+              schema:
+                type: string
+          content:
+            text/plain:
+              schema:
+                type: string

This explicit declaration enhances clarity for API consumers.

components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/APIConsumerImplTest.java (1)

40-40: Consider using specific imports rather than wildcard imports.

Wildcard imports make it harder to track which specific classes are being used and can lead to namespace conflicts. Consider using specific imports for better code maintainability.

-import org.wso2.carbon.apimgt.api.model.*;
+import org.wso2.carbon.apimgt.api.model.AccessTokenInfo;
+import org.wso2.carbon.apimgt.api.model.API;
+import org.wso2.carbon.apimgt.api.model.APIIdentifier;
+import org.wso2.carbon.apimgt.api.model.APIKey;
+import org.wso2.carbon.apimgt.api.model.Application;
+import org.wso2.carbon.apimgt.api.model.Comment;
+import org.wso2.carbon.apimgt.api.model.DocumentationContent;
+import org.wso2.carbon.apimgt.api.model.OAuthAppRequest;
+import org.wso2.carbon.apimgt.api.model.OAuthApplicationInfo;
+import org.wso2.carbon.apimgt.api.model.Scope;
+import org.wso2.carbon.apimgt.api.model.Subscriber;
+import org.wso2.carbon.apimgt.api.model.SubscribedAPI;
+import org.wso2.carbon.apimgt.api.model.Tier;
+import org.wso2.carbon.apimgt.api.model.APIRating;
+import org.wso2.carbon.apimgt.api.model.AccessTokenRequest;
+import org.wso2.carbon.apimgt.api.model.ApiTypeWrapper;
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/APIProduct.java (1)

721-727: Consider documenting the new feature with JavaDoc comments.

Adding descriptive JavaDoc comments for the new methods would improve API documentation and help developers understand the purpose of the isMarkdownOverview property.

+/**
+ * Checks if this API Product has a Markdown overview document.
+ *
+ * @return true if this API Product has a Markdown overview document, false otherwise
+ */
 public Boolean isMarkdownOverview() {
     return isMarkdownOverview;
 }

+/**
+ * Sets whether this API Product has a Markdown overview document.
+ *
+ * @param isMarkdownOverview boolean indicating if the API Product has a Markdown overview document
+ */
 public void setMarkdownOverview(Boolean isMarkdownOverview) {
     this.isMarkdownOverview = isMarkdownOverview;
 }
components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/AbstractAPIManagerTestCase.java (1)

260-289: Complete test coverage for the new functionality

The test method effectively verifies the getMarkdownOverviewContent method in AbstractAPIManager when no documentation is found. It:

  1. Sets up the necessary mocks including ServiceReferenceHolder and configuration services
  2. Configures the apiPersistenceInstance mock to return an empty document list
  3. Verifies that the result is null when no documentation is found

Consider adding an additional test case that verifies behavior when documentation is found. This would provide complete coverage of the success path:

+ @Test
+ public void testGetMarkdownOverviewContentWhenExists() throws Exception {
+     String apiId = "sample-api-id";
+     String organization = "sample-org";
+     String expectedContent = "# API Overview\nThis is a sample markdown content.";
+ 
+     AbstractAPIManager abstractAPIManager = new AbstractAPIManagerWrapper(apiPersistenceInstance);
+ 
+     PowerMockito.mockStatic(ServiceReferenceHolder.class);
+     ServiceReferenceHolder sh = Mockito.mock(ServiceReferenceHolder.class);
+     APIManagerConfigurationService amConfigService = Mockito.mock(APIManagerConfigurationService.class);
+     APIManagerConfiguration amConfig = Mockito.mock(APIManagerConfiguration.class);
+ 
+     PowerMockito.when(ServiceReferenceHolder.getInstance()).thenReturn(sh);
+     PowerMockito.when(sh.getAPIManagerConfigurationService()).thenReturn(amConfigService);
+     PowerMockito.when(amConfigService.getAPIManagerConfiguration()).thenReturn(amConfig);
+ 
+     // Create mock documentation and search result
+     org.wso2.carbon.apimgt.persistence.dto.Documentation mockDoc = Mockito.mock(org.wso2.carbon.apimgt.persistence.dto.Documentation.class);
+     List<org.wso2.carbon.apimgt.persistence.dto.Documentation> docList = Collections.singletonList(mockDoc);
+     DocumentSearchResult searchResult = Mockito.mock(DocumentSearchResult.class);
+     Mockito.when(searchResult.getDocumentationList()).thenReturn(docList);
+     
+     // Set up mock DocumentContent
+     DocumentContent documentContent = Mockito.mock(DocumentContent.class);
+     Mockito.when(documentContent.getContent()).thenReturn(expectedContent);
+     
+     // Configure apiPersistenceInstance to return the search result and document content
+     Mockito.when(apiPersistenceInstance.searchDocumentation(
+             Mockito.any(Organization.class),
+             Mockito.eq(apiId),
+             Mockito.eq(0),
+             Mockito.eq(0),
+             Mockito.eq("other:_overview"),
+             Mockito.isNull()
+     )).thenReturn(searchResult);
+     Mockito.when(apiPersistenceInstance.getDocumentationContent(
+             Mockito.any(Organization.class),
+             Mockito.eq(apiId),
+             Mockito.anyString()
+     )).thenReturn(documentContent);
+     
+     // Execute the method under test
+     DocumentationContent result = abstractAPIManager.getMarkdownOverviewContent(apiId, organization);
+     
+     // Verify the result
+     Assert.assertNotNull(result);
+     Assert.assertEquals(expectedContent, result.getTextContent());
+ }
components/apimgt/org.wso2.carbon.apimgt.persistence/src/main/java/org/wso2/carbon/apimgt/persistence/RegistryPersistenceImpl.java (1)

2816-2821: Enhanced documentation search capabilities by adding support for 'other:' query prefix.

This addition enhances the documentation search functionality by allowing users to search documentation based on the "other type name" property, making it possible to find specific documentation types like markdown overviews more efficiently.

Consider adding null checking for the searchQuery parameter before applying toLowerCase() and split(":") to prevent potential NullPointerExceptions. The following pattern would be safer:

-else if (searchQuery.toLowerCase().startsWith("other:")) {
-    String requestedDocOtherTypeName = searchQuery.split(":")[1];
+else if (searchQuery != null && searchQuery.toLowerCase().startsWith("other:")) {
+    String[] parts = searchQuery.split(":");
+    if (parts.length > 1) {
+        String requestedDocOtherTypeName = parts[1];
+        if (doc.getOtherTypeName() != null
+                && doc.getOtherTypeName().equalsIgnoreCase(requestedDocOtherTypeName)) {
+            documentationList.add(doc);
+        }
+    }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cde9a9 and 764f2c2.

⛔ Files ignored due to path filters (3)
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/gen/java/org/wso2/carbon/apimgt/rest/api/store/v1/ApisApi.java is excluded by !**/gen/**
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/gen/java/org/wso2/carbon/apimgt/rest/api/store/v1/ApisApiService.java is excluded by !**/gen/**
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/gen/java/org/wso2/carbon/apimgt/rest/api/store/v1/dto/APIDTO.java is excluded by !**/gen/**
📒 Files selected for processing (15)
  • components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIManager.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/API.java (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/APIProduct.java (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConsumerImpl.java (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/AbstractAPIManager.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/APIConsumerImplTest.java (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/test/java/org/wso2/carbon/apimgt/impl/AbstractAPIManagerTestCase.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.persistence/src/main/java/org/wso2/carbon/apimgt/persistence/RegistryPersistenceImpl.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.persistence/src/main/java/org/wso2/carbon/apimgt/persistence/dto/DevPortalAPI.java (3 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.common/src/main/resources/devportal-api.yaml (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/store/v1/impl/ApisApiServiceImpl.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/store/v1/mappings/APIMappingUtil.java (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/resources/devportal-api.yaml (2 hunks)
  • features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2 (2 hunks)
  • features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/config/api-manager.xml (2 hunks)
🔇 Additional comments (20)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/APIManager.java (1)

233-242: Clean implementation of the new method to support markdown overview content retrieval.

The new getMarkdownOverviewContent method is well-designed and properly documented. It aligns perfectly with the PR objective of enabling direct access to API overview markdown content without requiring multiple API calls. The signature follows the existing patterns in the interface, taking an API ID and organization identifier as parameters and returning the appropriate DocumentationContent type.

This implementation enhances the API management functionality by providing a streamlined way to access overview documentation, which improves developer experience.

components/apimgt/org.wso2.carbon.apimgt.rest.api.common/src/main/resources/devportal-api.yaml (1)

4136-4139: New API Schema Property: isMarkdownOverview

The addition of the isMarkdownOverview boolean field in the API schema is well implemented. It clearly indicates whether an API has an associated markdown overview document, which is central to this PR. The example provided is helpful. Please ensure that any related components (such as model classes or getters/setters) are updated accordingly in the rest of the codebase.

components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/resources/devportal-api.yaml (1)

4136-4138: New API Schema Property: isMarkdownOverview
The addition of the isMarkdownOverview boolean property in the API object schema is appropriate and aligns well with the PR objectives. It clearly signals whether an API has an associated Markdown overview document. Including an example value (true) further aids comprehension.

features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/config/api-manager.xml (2)

457-460: Configuration looks good for the API markdown content endpoint.

The new AllowedURI configuration properly adds authorization exemptions for the markdown content endpoint for APIs, allowing GET and HEAD operations without requiring authentication.


521-524: Configuration looks good for the API Product markdown content endpoint.

The new AllowedURI configuration properly adds authorization exemptions for the markdown content endpoint for API products, allowing GET and HEAD operations without requiring authentication.

components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/API.java (2)

149-149: Field declaration looks good.

The isMarkdownOverview field is appropriately declared as a Boolean object rather than a primitive, allowing for null values which is consistent with similar fields in this class.


1409-1415: Getter and setter methods are properly implemented.

The getter and setter methods for the isMarkdownOverview property follow Java Bean naming conventions and are correctly implemented. The methods are consistent with other similar methods in the class.

components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/APIProduct.java (2)

68-68: Field declaration looks good.

The isMarkdownOverview field is appropriately declared as a Boolean object rather than a primitive, allowing for null values which is consistent with similar fields in this class.


721-727: Getter and setter methods are properly implemented.

The getter and setter methods for the isMarkdownOverview property follow Java Bean naming conventions and are correctly implemented. The methods are consistent with other similar methods in the class.

components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/store/v1/impl/ApisApiServiceImpl.java (1)

1221-1246:

✅ Verification successful

Well-structured implementation for retrieving markdown content

The implementation follows the established patterns in the rest of the class:

  1. Properly validates the organization from the message context
  2. Obtains the logged-in user consumer
  3. Handles the null content case appropriately
  4. Returns the content with proper content-type header
  5. Implements comprehensive error handling with specific messages

Run the following to verify this endpoint meets the requirements described in the PR objectives:


🏁 Script executed:

#!/bin/bash
# Verify integration with related components

# Check if the APIConsumer interface has the getMarkdownOverviewContent method
rg -A 5 "getMarkdownOverviewContent" --type java

Length of output: 6768


Endpoint Implementation Verified – All Criteria Met

  • The method correctly validates the organization, retrieves the logged-in user consumer, and handles null documentation content as expected.
  • The proper content-type header is set and error handling is comprehensive, addressing both missing resources and internal errors.
  • Verification confirms that the APIConsumer interface provides the getMarkdownOverviewContent method, ensuring integration with related components is intact.
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/AbstractAPIManager.java (1)

501-516: Well implemented method to retrieve Markdown overview content.

The new getMarkdownOverviewContent method is well-structured and follows consistent error handling patterns. It efficiently searches for documentation with "other:_overview" prefix and returns the content or null if not found.

components/apimgt/org.wso2.carbon.apimgt.rest.api.store.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/store/v1/mappings/APIMappingUtil.java (2)

104-104: Property mapping implementation looks good.

The mapping of isMarkdownOverview from the API model to the DTO follows consistent patterns with other properties.


309-309: Property mapping for APIProduct is consistent.

The mapping of isMarkdownOverview for APIProduct matches the implementation in the API model, ensuring consistent behavior across both entity types.

components/apimgt/org.wso2.carbon.apimgt.persistence/src/main/java/org/wso2/carbon/apimgt/persistence/dto/DevPortalAPI.java (3)

71-71: Appropriately placed field declaration.

The new isMarkdownOverview field follows the existing pattern for Boolean properties in this class.


361-367: Well-implemented getter and setter methods.

The accessor methods follow Java conventions with the "is" prefix for the boolean getter, which correctly indicates this is a boolean property.


387-387: Properly updated toString method.

The toString() method appropriately includes the new field, maintaining complete object representation for debugging.

features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2 (2)

741-744: Addition of new allowed URI for API markdown content access

This configuration adds a new endpoint to allow unauthenticated access to the markdown content of APIs through the Developer Portal. This aligns with the PR objective to provide direct access to API overview markdown content.


809-812: Addition of new allowed URI for API Product markdown content access

This configuration extends the markdown content retrieval functionality to API Products, allowing unauthenticated access to API Product overview markdown content. This is a logical extension of the feature to ensure consistency between APIs and API Products.

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

3963-3970: Implementation of markdown overview detection looks good and efficient.

The code correctly checks if any document in the API has the _overview type and sets the markdownOverview flag accordingly. This implementation efficiently determines whether an API has markdown overview content without requiring additional API calls.


4425-4432: Well-implemented access control for markdown overview content.

This method provides a secure way to retrieve markdown overview content by first checking API visibility restrictions before delegating to the superclass implementation. The proper authorization checks help ensure that only users with the appropriate permissions can access the markdown content.

Comment on lines +996 to +1011
@Test
public void testGetMarkdownOverviewContent() throws Exception {
String apiId = "api123";
String organization = "wso2";
DocumentationContent mockContent = new DocumentationContent();

APIConsumerImpl apiConsumer = PowerMockito.spy(new APIConsumerImplWrapper());

PowerMockito.doNothing().when(apiConsumer).checkAPIVisibilityRestriction(apiId, organization);
PowerMockito.doReturn(mockContent).when(apiConsumer, "getMarkdownOverviewContent", apiId, organization);

DocumentationContent result = apiConsumer.getMarkdownOverviewContent(apiId, organization);

Assert.assertNotNull(result);
Assert.assertEquals(mockContent, result);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Test implementation has a potential issue.

The test is attempting to test the getMarkdownOverviewContent method while simultaneously mocking its behavior with doReturn(mockContent).when(apiConsumer, "getMarkdownOverviewContent", ...). This creates a circular dependency where the method's implementation is bypassed, potentially leading to false positives.

Consider revising the test to either:

  1. Mock the dependencies of getMarkdownOverviewContent instead of mocking the method itself
  2. Add verification that the method calls necessary dependencies with correct parameters
 @Test
 public void testGetMarkdownOverviewContent() throws Exception {
     String apiId = "api123";
     String organization = "wso2";
     DocumentationContent mockContent = new DocumentationContent();
+    mockContent.setSourceType("MARKDOWN");
+    mockContent.setContent("# API Overview");

     APIConsumerImpl apiConsumer = PowerMockito.spy(new APIConsumerImplWrapper());
+    API api = new API(new APIIdentifier("provider", "name", "1.0.0"));
+    api.setUuid(apiId);
+    api.setMarkdownOverview(true);
+    
+    PowerMockito.doReturn(api).when(apiConsumer).getAPIbyUUID(apiId, organization);
+    PowerMockito.doReturn(mockContent).when(apiConsumer).getAPIDocumentationContent(
+        Mockito.anyString(), Mockito.anyString());

-    PowerMockito.doNothing().when(apiConsumer).checkAPIVisibilityRestriction(apiId, organization);
-    PowerMockito.doReturn(mockContent).when(apiConsumer, "getMarkdownOverviewContent", apiId, organization);

     DocumentationContent result = apiConsumer.getMarkdownOverviewContent(apiId, organization);

     Assert.assertNotNull(result);
-    Assert.assertEquals(mockContent, result);
+    Assert.assertEquals("# API Overview", result.getContent());
+    Assert.assertEquals("MARKDOWN", result.getSourceType());
+    
+    // Verify that visibility check was performed
+    PowerMockito.verifyPrivate(apiConsumer).invoke("checkAPIVisibilityRestriction", apiId, organization);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Test
public void testGetMarkdownOverviewContent() throws Exception {
String apiId = "api123";
String organization = "wso2";
DocumentationContent mockContent = new DocumentationContent();
APIConsumerImpl apiConsumer = PowerMockito.spy(new APIConsumerImplWrapper());
PowerMockito.doNothing().when(apiConsumer).checkAPIVisibilityRestriction(apiId, organization);
PowerMockito.doReturn(mockContent).when(apiConsumer, "getMarkdownOverviewContent", apiId, organization);
DocumentationContent result = apiConsumer.getMarkdownOverviewContent(apiId, organization);
Assert.assertNotNull(result);
Assert.assertEquals(mockContent, result);
}
@Test
public void testGetMarkdownOverviewContent() throws Exception {
String apiId = "api123";
String organization = "wso2";
DocumentationContent mockContent = new DocumentationContent();
mockContent.setSourceType("MARKDOWN");
mockContent.setContent("# API Overview");
APIConsumerImpl apiConsumer = PowerMockito.spy(new APIConsumerImplWrapper());
API api = new API(new APIIdentifier("provider", "name", "1.0.0"));
api.setUuid(apiId);
api.setMarkdownOverview(true);
PowerMockito.doReturn(api).when(apiConsumer).getAPIbyUUID(apiId, organization);
PowerMockito.doReturn(mockContent).when(apiConsumer).getAPIDocumentationContent(
Mockito.anyString(), Mockito.anyString());
DocumentationContent result = apiConsumer.getMarkdownOverviewContent(apiId, organization);
Assert.assertNotNull(result);
Assert.assertEquals("# API Overview", result.getContent());
Assert.assertEquals("MARKDOWN", result.getSourceType());
// Verify that visibility check was performed
PowerMockito.verifyPrivate(apiConsumer).invoke("checkAPIVisibilityRestriction", apiId, organization);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants