[issue-87]: Added a new feature to get instance count by namespace. #103
Conversation
WalkthroughA new API interface and its implementation for retrieving the count of instances by namespace were introduced. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ApolloOpenApiClient
participant InstanceOpenApiService
participant HTTP_Server
Client->>ApolloOpenApiClient: getInstanceCountByNamespace(appId, env, cluster, namespace)
ApolloOpenApiClient->>InstanceOpenApiService: getInstanceCountByNamespace(appId, env, cluster, namespace)
InstanceOpenApiService->>HTTP_Server: HTTP GET /envs/{env}/apps/{appId}/clusters/{cluster}/namespaces/{namespace}/instances
HTTP_Server-->>InstanceOpenApiService: Instance count (JSON)
InstanceOpenApiService-->>ApolloOpenApiClient: instance count (int)
ApolloOpenApiClient-->>Client: instance count (int)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiServiceTest.java (1)
47-62: Consider adding more comprehensive tests.While the current test verifies the correct URI construction and HTTP client interaction, consider adding tests for:
- Validating the returned count matches the response value
- Error handling scenarios when the HTTP request fails
- Edge cases with null or empty parameters
- Default value handling for clusterName and namespaceName
@Test public void testGetInstanceCountByNamespaceReturnsCorrectValue() throws Exception { // Setup StringEntity responseEntity = new StringEntity("42"); when(someHttpResponse.getEntity()).thenReturn(responseEntity); // Execute int result = instanceOpenApiService.getInstanceCountByNamespace(someAppId, someEnv, someCluster, someNamespace); // Verify assertEquals(42, result); } @Test public void testGetInstanceCountWithDefaultValues() throws Exception { // Setup StringEntity responseEntity = new StringEntity("1"); when(someHttpResponse.getEntity()).thenReturn(responseEntity); // Execute with null cluster and namespace instanceOpenApiService.getInstanceCountByNamespace(someAppId, someEnv, null, null); // Capture and verify the request ArgumentCaptor<HttpGet> request = ArgumentCaptor.forClass(HttpGet.class); verify(httpClient, times(1)).execute(request.capture()); HttpGet get = request.getValue(); // Should use default values "default" for cluster and "application" for namespace assertTrue(get.getURI().toString().contains("/clusters/default/")); assertTrue(get.getURI().toString().contains("/namespaces/application/")); } @Test(expected = RuntimeException.class) public void testGetInstanceCountWithHttpError() throws Exception { // Setup when(httpClient.execute(any(HttpGet.class))).thenThrow(new IOException("Connection error")); // This should throw a RuntimeException instanceOpenApiService.getInstanceCountByNamespace(someAppId, someEnv, someCluster, someNamespace); }apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiService.java (3)
34-35: Add Javadoc for this methodConsider adding Javadoc to document the purpose of this method, its parameters, return values, and possible exceptions. This would help API consumers understand how to use this method properly.
+/** + * Gets the count of instances for a specified namespace. + * + * @param appId The application ID + * @param env The environment + * @param clusterName The cluster name (defaults to "default" if null or empty) + * @param namespaceName The namespace name (defaults to "application" if null or empty) + * @return The number of instances for the specified namespace + * @throws RuntimeException if the API call fails + */ @Override public int getInstanceCountByNamespace(String appId, String env, String clusterName, String namespaceName) {
46-52: Consider extracting "instances" as a constantFor better maintainability and to avoid hardcoded strings, consider extracting "instances" as a class constant.
+private static final String INSTANCES_RESOURCE = "instances"; @Override public int getInstanceCountByNamespace(String appId, String env, String clusterName, String namespaceName) { // existing code... OpenApiPathBuilder pathBuilder = OpenApiPathBuilder.newBuilder() .envsPathVal(env) .appsPathVal(appId) .clustersPathVal(clusterName) .namespacesPathVal(namespaceName) - .customResource("instances"); + .customResource(INSTANCES_RESOURCE);
53-58: Consider using a more specific exception typeThe method currently throws a generic RuntimeException. Consider creating or using a more specific exception type to help API consumers better handle different error scenarios.
try (CloseableHttpResponse response = get(pathBuilder)) { return gson.fromJson(EntityUtils.toString(response.getEntity()), Integer.class); } catch (Throwable ex) { - throw new RuntimeException(String.format("Get instance count: appId: %s, cluster: %s, namespace: %s in env: %s failed", + throw new ApolloOpenApiException(String.format("Get instance count: appId: %s, cluster: %s, namespace: %s in env: %s failed", appId, clusterName, namespaceName, env), ex); }Note: If
ApolloOpenApiExceptiondoesn't exist, you would need to create this class or use another appropriate exception type from your codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/api/InstanceOpenApiService.java(1 hunks)apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClient.java(4 hunks)apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiService.java(1 hunks)apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiServiceTest.java(1 hunks)apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilderTest.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/api/InstanceOpenApiService.java (1)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiService.java (1)
InstanceOpenApiService(27-60)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClient.java (1)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiService.java (1)
InstanceOpenApiService(27-60)
apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilderTest.java (1)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilder.java (1)
OpenApiPathBuilder(34-164)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiService.java (1)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilder.java (1)
OpenApiPathBuilder(34-164)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (8)
🔇 Additional comments (10)
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/api/InstanceOpenApiService.java (1)
19-26: Clear and concise API interface design.The interface is well-defined with a single method that has a clear purpose and appropriate parameters for retrieving the instance count by namespace. The Javadoc indicates this feature is available since version 2.5.0, which aligns with the PR objective of adding a new feature to get instance count by namespace.
apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/url/OpenApiPathBuilderTest.java (1)
227-240: Appropriate test case for new endpoint path construction.The test case correctly verifies that the URL path for the instances endpoint is accurately constructed using the expected pattern:
"envs/{env}/apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/instances". This test follows the same pattern as other service path tests in the file and ensures the consistency of the API path construction.apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/ApolloOpenApiClient.java (4)
25-25: Appropriate import for the new service.Adding the import for the InstanceOpenApiService is correctly placed with the other service imports, maintaining code organization.
55-55: Consistent field declaration.The private final field for the instanceService follows the same pattern as other service fields in the class, maintaining consistency in the codebase.
71-71: Proper initialization of the new service.The instanceService is initialized in the constructor following the same pattern as other services, using the shared HTTP client, base URL, and GSON instance.
249-255: Well-documented public API method.The getInstanceCountByNamespace method is properly documented with the @SInCE 2.5.0 annotation, and it correctly delegates to the instanceService. This follows the established pattern in the ApolloOpenApiClient class for exposing service functionality.
apollo-openapi/src/test/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiServiceTest.java (1)
28-45: Good test setup and initialization.The test class properly extends AbstractOpenApiServiceTest and has a clear setup method that initializes test data and the service under test. The test variables and service initialization follow the established patterns.
apollo-openapi/src/main/java/com/ctrip/framework/apollo/openapi/client/service/InstanceOpenApiService.java (3)
27-32: Class structure looks goodThe class extension and implementation hierarchy is appropriate. The constructor properly passes parameters to the superclass.
36-41: Default parameter values are handled correctlyThe code properly handles null or empty parameters by setting default values for clusterName and namespaceName. This aligns with Apollo's convention of using ConfigConsts for default values.
43-44: Input validation is implemented correctlyThe code properly validates that required parameters (appId and env) are not empty before proceeding.
|
I have read the CLA Document and I hereby sign the CLA |
2 participants
What's the purpose of this PR
Get instance count by namespaces
Which issue(s) this PR fixes:
For #87
Brief changelog
Added a new feature to get instance count by namespace.
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
New Features
Tests