integration tests for Async API parsers #13933
integration tests for Async API parsers #13933Hamool-Nizar wants to merge 4 commits intowso2:masterfrom
Conversation
Hamool-Nizar
requested review from
AnuGayan,
Arshardh,
chamilaadhi and
tharindu1st
as code owners
WalkthroughBumps Apicurio data-models version in multiple LICENSE files; adds Async API test suites and test cases plus supporting deployment.toml and AsyncAPI spec files; removes one unused import and registers new tests in TestNG. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPIValidationTestSuite.java (2)
1-2: Update the copyright year.The copyright year is 2022, but this file is being added in 2025. Consider updating to reflect the current year.
/* - * Copyright (c) 2022, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2025, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
47-52: Redundant re-instantiation ofserverConfigurationManager.The
serverConfigurationManageris re-instantiated inrestoreConfigurationusing the samesuperTenantKeyManagerContextthat was set inloadConfiguration. This re-instantiation is unnecessary if the context hasn't changed and may cause issues ifsuperTenantKeyManagerContextis null (e.g., ifloadConfigurationfailed).@AfterTest(alwaysRun = true) public void restoreConfiguration() throws Exception { - - serverConfigurationManager = new ServerConfigurationManager(superTenantKeyManagerContext); + if (serverConfigurationManager == null) { + serverConfigurationManager = new ServerConfigurationManager(superTenantKeyManagerContext); + } serverConfigurationManager.restoreToLastConfiguration(); }all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (4)
1-2: Update the copyright year.The copyright year is 2022, but this file is being added in 2025.
/* - * Copyright (c) 2022, WSO2 Inc. (http://www.wso2.org) All Rights Reserved. + * Copyright (c) 2025, WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
318-327: Consider using a separate field for the V3 application ID.The
appIdfield is overwritten intestAsyncApiV3ApplicationSubscription, losing the reference to the V2 application created in the previous test. While cleanup viasuper.cleanUp()may handle this, if you need to track both applications for verification or explicit cleanup, consider using separate fields (e.g.,appIdV2andappIdV3).
176-178: Inconsistent assertion argument order.TestNG's
Assert.assertEqualsexpects(actual, expected), but line 178 passes(expected, actual). While this doesn't affect test correctness, it will produce confusing failure messages. Line 182 uses the correct order.// create Async API ApiResponse<APIDTO> response = restAPIPublisher .importAsyncAPIDefinition(file, additionalPropertiesObj.toString()); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); + Assert.assertEquals(response.getStatusCode(), HttpStatus.SC_CREATED);The same issue exists at line 236:
- Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); + Assert.assertEquals(response.getStatusCode(), HttpStatus.SC_CREATED);
185-195: Clarify test intent for revision failure assertion.The test expects
createAPIRevisionAndDeployUsingRestto throw an exception (and fail if it succeeds), which is counterintuitive without context. Adding a comment explaining why revisions are expected to fail for advertised/third-party APIs would improve maintainability.String revisionUUID = null; - // Create Revision and Deploy to Gateway + // Revision creation is expected to fail for advertised/third-party APIs try { revisionUUID = createAPIRevisionAndDeployUsingRest(apiId, restAPIPublisher); } catch (ApiException e) { Assert.assertTrue(e.getMessage().contains("Error while adding new API Revision for API : ") - || e.getMessage().contains("Creating API Revisions is not supported")); + || e.getMessage().contains("Creating API Revisions is not supported"), + "Unexpected error message: " + e.getMessage()); } if (revisionUUID != null) { - Assert.fail(); + Assert.fail("Revision creation should not succeed for advertised APIs"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
LICENSE.txt(1 hunks)all-in-one-apim/LICENSE.txt(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestCase.java(0 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestSuite.java(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPIValidationTestSuite.java(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/async/deployment.toml(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/legacyAsync/deployment.toml(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/async/asyncapiv2.yaml(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/async/asyncapiv3.yaml(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml(5 hunks)api-control-plane/LICENSE.txt(1 hunks)gateway/LICENSE.txt(1 hunks)traffic-manager/LICENSE.txt(1 hunks)
💤 Files with no reviewable changes (1)
- all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestCase.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-14T10:30:58.375Z
Learnt from: YasasRangika
Repo: wso2/product-apim PR: 13926
File: api-control-plane/modules/distribution/product/src/main/resources/conf/key-mappings.json:34-41
Timestamp: 2025-11-14T10:30:58.375Z
Learning: In WSO2 product-apim configuration files (key-mappings.json and default.json), some key mappings for user-provided implementations (like classes implementing specific interfaces) intentionally have no default values in default.json. Example: `pass.through.transport.replay.transaction.writer.class` requires users to implement the ReplayDataWriter interface and provide their own class when enabling the replay transaction feature.
Applied to files:
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/legacyAsync/deployment.tomlall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/async/deployment.toml
🧬 Code graph analysis (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (5)
all-in-one-apim/modules/integration/tests-common/clients/apim-governance/src/gen/java/org/wso2/am/integration/clients/governance/ApiException.java (1)
ApiException(20-91)all-in-one-apim/modules/integration/tests-common/clients/apim-governance/src/gen/java/org/wso2/am/integration/clients/governance/ApiResponse.java (1)
ApiResponse(24-59)all-in-one-apim/modules/integration/tests-common/clients/store/src/gen/java/org/wso2/am/integration/clients/store/api/v1/dto/ApplicationDTO.java (1)
ApplicationDTO(37-573)all-in-one-apim/modules/integration/tests-common/integration-test-utils/src/main/java/org/wso2/am/integration/test/utils/base/APIMIntegrationBaseTest.java (1)
APIMIntegrationBaseTest(102-1225)all-in-one-apim/modules/integration/tests-common/integration-test-utils/src/main/java/org/wso2/am/integration/test/utils/base/APIMIntegrationConstants.java (2)
APIMIntegrationConstants(23-197)API_TIER(118-131)
🪛 Gitleaks (8.30.0)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/legacyAsync/deployment.toml
[high] 264-264: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/async/deployment.toml
[high] 264-264: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (16)
gateway/LICENSE.txt (1)
450-450: LGTM — version updated to 2.2.2 and license remains apache2.api-control-plane/LICENSE.txt (1)
450-450: LGTM — Apicurio entry correctly bumped to 2.2.2 with apache2 license.traffic-manager/LICENSE.txt (1)
377-377: Apicurio data-models bumped to 2.2.2; entry is consistent and correct.All occurrences across the repository show only version 2.2.2 with apache2 license — no stale 1.1.5 entries remain. Format aligns with the table throughout.
Apicurio Data Models 2.2.2 is Apache License 2.0 licensed. Ensure the distribution's NOTICE file (if included upstream) is accommodated in your compliance documentation per Apache-2.0 redistribution requirements.
all-in-one-apim/LICENSE.txt (1)
490-490: Apicurio jar entry update looks consistentThe updated
io.apicurio.apicurio-data-models_2.2.2.jarentry preserves the existing type/license pattern; no further changes needed here.LICENSE.txt (1)
445-445: Root license manifest correctly reflects Apicurio version bumpThe Apicurio data-models jar entry is updated to
2.2.2with unchanged license metadata, which is aligned with the rest of the file.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/async/asyncapiv2.yaml (1)
1-20: AsyncAPI 2.0 test spec is minimal but sufficientThe v2 AsyncAPI document is structurally sound for parser/validation tests (single channel, server, and oauth2 scheme); I don’t see issues for its intended use.
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/async/asyncapiv3.yaml (1)
1-28: AsyncAPI 3.0 test spec aligns with the new document structureThe v3 AsyncAPI resource cleanly exercises the new
servers/channels/operationslayout with an oauth2 scheme, which should be a good target for the updated parser and validation tests.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/testng.xml (5)
78-110: Lifecycle test comment tweaks are safeOnly comment formatting changes around the disabled lifecycle tests (
APIEndpointCertificateTestCase,AudienceValidationTestCase); no impact on executed suites.
359-372: Async streaming test suite wiring looks correctAdding
org.wso2.am.integration.tests.streamingapis.async.AsyncAPITestSuitetoapim-streaming-api-testscleanly hooks the new async configuration suite into the existing streaming API group without affecting the other classes.
392-400: Application‑sharing test block comment adjustments are non-functionalOnly indentation/comment marker changes around the commented
APISecurityTestCaseandAPISecurityMutualSSLCertificateChainValidationTestCase; behavior ofApplicationSharing*tests is unchanged.
471-476: Solace broker test block remains fully disabledThe
apim-solace-broker-tests<test>section is still entirely commented out; the reformatting doesn’t alter which tests run.
497-502: Tenant validation suite correctly extended with async testsRegistering
AsyncAPIValidationTestSuiteandAsyncAPITestWithValidationCasein theapim-tenant-validationtest ensures the new async API validation flows run alongside existing tenant-domain checks, with no extra parameters required.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/legacyAsync/deployment.toml (1)
38-40: Legacy async parser flag is correctly scoped to publisherSetting
[apim.publisher] use_legacy_async_parser = truein this dedicatedlegacyAsyncdeployment keeps the legacy parser behavior confined to these tests and won’t affect other configs.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestSuite.java (1)
31-52: AsyncAPITestSuite’s config lifecycle is wired correctlyThe suite’s
@BeforeTest/@AfterTestmethods cleanly apply and then restore thelegacyAsync/deployment.tomlviaServerConfigurationManager, using the super‑tenant KMAutomationContext. The resource path construction matches the new config file layout, so this should integrate smoothly with the streaming async tests.all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/async/deployment.toml (1)
263-267: Verify theapi_tokenis not a real credential.Static analysis flagged this as a potential API key exposure. While this appears to be a test configuration pointing to a local mock endpoint (
localhost:9943), ensure this token is not a real credential that could be misused. Consider using a clearly fake/placeholder token or documenting that this is a test-only value.If this is intended to be a test-only placeholder, consider making it more obviously fake:
[security_audit] -api_token="b57973cf-b74c-4ade-921d-ece83251eceb" +api_token="test-only-fake-token-12345" collection_id="f73b8171-4f71-499b-891a-d34aa71f2d45" base_url="https://localhost:9943/am-auditApi-sample/api/auditapi" global=trueall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (1)
264-268: Theasyncapi.yamlfile exists atall-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/resources/async/asyncapi.yaml. The test code correctly references this resource, and no changes are required.
...t/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java
Outdated
Show resolved
Hide resolved
...t/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java
Show resolved
Hide resolved
...ackend/src/test/resources/artifacts/AM/configFiles/streamingAPIs/legacyAsync/deployment.toml
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (1)
329-336: Resource leak issue (previously flagged).The
BufferedWriteris not wrapped in try-with-resources. Ifout.write()throws an exception,out.close()will not be called, resulting in a resource leak.This issue was previously identified in past reviews. The suggested fix using try-with-resources would ensure the writer is always closed:
private File getTempFileWithContent(String asyncApi) throws Exception { File temp = File.createTempFile("async", ".yaml"); temp.deleteOnExit(); - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(asyncApi); - out.close(); + try (BufferedWriter out = new BufferedWriter(new FileWriter(temp))) { + out.write(asyncApi); + } return temp; }
🧹 Nitpick comments (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (2)
115-117: Add descriptive messages to Assert.fail() calls.Several test methods call
Assert.fail()without explanatory messages when an unexpected success occurs. Adding descriptive failure messages would make test failures easier to debug.Example for line 116:
if (response != null) { - Assert.fail(); + Assert.fail("Expected ApiException when creating ASYNC API without advertise flag, but API was created successfully"); }Apply similar descriptive messages at lines 144, 194, and 303.
Also applies to: 143-145, 193-195, 302-304
162-173: Consider extracting duplicate advertiseInfo and policies setup.The construction of
advertiseInfoJSON andpolicieslist is duplicated across three test methods. Extracting these into helper methods would reduce duplication and improve maintainability.Example refactor:
private JSONObject createAdvertiseInfo(String apiOwner) { JSONObject advertiseInfo = new JSONObject(); advertiseInfo.put("advertised", true); advertiseInfo.put("apiExternalProductionEndpoint", "https://test.com"); advertiseInfo.put("apiExternalSandboxEndpoint", "https://test.com"); advertiseInfo.put("originalDevPortalUrl", "https://test.com"); advertiseInfo.put("vendor", "WSO2"); advertiseInfo.put("apiOwner", apiOwner); return advertiseInfo; } private ArrayList<String> createAsyncPolicies() { ArrayList<String> policies = new ArrayList<>(); policies.add(APIMIntegrationConstants.API_TIER.ASYNC_WH_UNLIMITED); return policies; }Also applies to: 220-231, 278-289
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (6)
all-in-one-apim/modules/integration/tests-common/clients/apim-governance/src/gen/java/org/wso2/am/integration/clients/governance/ApiException.java (1)
ApiException(20-91)all-in-one-apim/modules/integration/tests-common/clients/apim-governance/src/gen/java/org/wso2/am/integration/clients/governance/ApiResponse.java (1)
ApiResponse(24-59)all-in-one-apim/modules/integration/tests-common/clients/publisher/src/gen/java/org/wso2/am/integration/clients/publisher/api/v1/dto/APIDTO.java (1)
APIDTO(51-2302)all-in-one-apim/modules/integration/tests-common/clients/store/src/gen/java/org/wso2/am/integration/clients/store/api/v1/dto/ApplicationDTO.java (1)
ApplicationDTO(37-573)all-in-one-apim/modules/integration/tests-common/integration-test-utils/src/main/java/org/wso2/am/integration/test/utils/base/APIMIntegrationBaseTest.java (1)
APIMIntegrationBaseTest(102-1225)all-in-one-apim/modules/integration/tests-common/integration-test-utils/src/main/java/org/wso2/am/integration/test/utils/base/APIMIntegrationConstants.java (2)
APIMIntegrationConstants(23-197)API_TIER(118-131)
🔇 Additional comments (2)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (2)
50-91: Well-structured test class setup.The test class is properly organized with clear constants, multi-tenancy support via data provider, and standard initialization. The separation of constants for V2 and V3 APIs makes the tests easy to follow.
92-327: Comprehensive test coverage for AsyncAPI parser validation.The test suite provides excellent coverage of AsyncAPI V2 and V3 functionality:
- Validates the advertise-only restriction for ASYNC type APIs
- Tests successful creation and publication flows for both versions
- Includes negative test case for invalid AsyncAPI definitions
- Verifies subscription workflows
- Supports multi-tenancy testing via data provider
The test structure is well-organized with clear method names, proper dependencies, and appropriate assertions for both positive and negative scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (1)
329-336: Use try-with-resources for the temp file writer (revisit prior suggestion)
getTempFileWithContentmanually closes theBufferedWriter. If an exception is thrown while writing, the writer won’t be closed, which is avoidable even in tests.You can make this safer and more idiomatic with try-with-resources:
private File getTempFileWithContent(String asyncApi) throws Exception { File temp = File.createTempFile("async", ".yaml"); temp.deleteOnExit(); - BufferedWriter out = new BufferedWriter(new FileWriter(temp)); - out.write(asyncApi); - out.close(); + try (BufferedWriter out = new BufferedWriter(new FileWriter(temp))) { + out.write(asyncApi); + } return temp; }This keeps behavior the same while ensuring the writer is closed on all paths.
🧹 Nitpick comments (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (1)
148-204: Tweak failure message for API visibility assertionsThe visibility assertions currently read:
assertTrue(APIMTestCaseUtils.isAPIAvailable(apiIdentifier, apiPublisherAllAPIs), "Published API is visible in API Publisher.");This message describes the success state but only appears when the assertion fails, which can be confusing when reading test output.
Consider flipping the wording so the failure message clearly describes the problem:
- assertTrue(APIMTestCaseUtils.isAPIAvailable(apiIdentifier, apiPublisherAllAPIs), - "Published API is visible in API Publisher."); + assertTrue(APIMTestCaseUtils.isAPIAvailable(apiIdentifier, apiPublisherAllAPIs), + "Published API is not visible in API Publisher.");Apply similarly to the V3 publish test.
Also applies to: 206-262
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestSuite.java(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java(1 hunks)all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPIValidationTestSuite.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestSuite.java
- all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPIValidationTestSuite.java
🔇 Additional comments (1)
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java (1)
73-90: Test harness and multi-user coverage look solidUsing
@FactorywithuserModeDataProviderplus@BeforeClass/@AfterClassto exercise both SUPER_TENANT_ADMIN and TENANT_ADMIN paths is a clean pattern and fits the existing test framework well. No changes needed here.Also applies to: 338-341
| @Test(description = "Create Async API V2 as a normal API") | ||
| public void testCreateAsyncApiWithoutAdvertiseOnly() throws Exception { | ||
| asyncAPIDefinition = IOUtils.toString( | ||
| getClass().getClassLoader().getResourceAsStream("async" + File.separator + "asyncapiv2.yaml"), | ||
| StandardCharsets.UTF_8); | ||
|
|
||
| File file = getTempFileWithContent(asyncAPIDefinition); | ||
|
|
||
| JSONObject additionalPropertiesObj = new JSONObject(); | ||
| additionalPropertiesObj.put("name", ASYNC_API_V2_NAME); | ||
| additionalPropertiesObj.put("context", ASYNC_API_V2_CONTEXT); | ||
| additionalPropertiesObj.put("version", ASYNC_API_V2_VERSION); | ||
| additionalPropertiesObj.put("type", ASYNC_TYPE); | ||
|
|
||
| // create Async API | ||
| ApiResponse<APIDTO> response = null; | ||
| try { | ||
| response = restAPIPublisher | ||
| .importAsyncAPIDefinition(file, additionalPropertiesObj.toString()); | ||
| } catch (ApiException e) { | ||
| Assert.assertTrue(e.getResponseBody() | ||
| .contains("ASYNC type APIs only can be created as third party APIs")); | ||
| } | ||
| if (response != null) { | ||
| Assert.fail(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "AsyncAPITestWithValidationCase.java" | head -5Repository: wso2/product-apim
Length of output: 230
🏁 Script executed:
wc -l ./all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.javaRepository: wso2/product-apim
Length of output: 234
🏁 Script executed:
sed -n '92,150p' ./all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.javaRepository: wso2/product-apim
Length of output: 2639
🏁 Script executed:
rg -n "getResponseBody()" ./all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java -B 2 -A 2Repository: wso2/product-apim
Length of output: 1049
🏁 Script executed:
find . -type f -name "*.java" | xargs rg -l "class ApiException" | grep -v ".class" | head -3Repository: wso2/product-apim
Length of output: 502
🏁 Script executed:
cat -n ./all-in-one-apim/modules/integration/tests-common/clients/publisher/src/gen/java/org/wso2/am/integration/clients/publisher/api/ApiException.java | head -100Repository: wso2/product-apim
Length of output: 7528
Add null-safety checks to getResponseBody() calls in both Async API creation tests
Both testCreateAsyncApiWithoutAdvertiseOnly() (line 112) and testCreateAsyncApiV3WithoutAdvertiseOnly() (line 140) directly dereference e.getResponseBody() without null-checking. If the backend returns a null body, this will throw a NullPointerException instead of failing with a clear assertion.
Apply the null-safe pattern already used elsewhere in this file (line 298):
- } catch (ApiException e) {
- Assert.assertTrue(e.getResponseBody()
- .contains("ASYNC type APIs only can be created as third party APIs"));
- }
+ } catch (ApiException e) {
+ String respBody = e.getResponseBody();
+ Assert.assertNotNull(respBody,
+ "Expected error response body for Async API creation failure");
+ Assert.assertTrue(respBody
+ .contains("ASYNC type APIs only can be created as third party APIs"));
+ }Apply the same pattern in both test methods.
📝 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.
| @Test(description = "Create Async API V2 as a normal API") | |
| public void testCreateAsyncApiWithoutAdvertiseOnly() throws Exception { | |
| asyncAPIDefinition = IOUtils.toString( | |
| getClass().getClassLoader().getResourceAsStream("async" + File.separator + "asyncapiv2.yaml"), | |
| StandardCharsets.UTF_8); | |
| File file = getTempFileWithContent(asyncAPIDefinition); | |
| JSONObject additionalPropertiesObj = new JSONObject(); | |
| additionalPropertiesObj.put("name", ASYNC_API_V2_NAME); | |
| additionalPropertiesObj.put("context", ASYNC_API_V2_CONTEXT); | |
| additionalPropertiesObj.put("version", ASYNC_API_V2_VERSION); | |
| additionalPropertiesObj.put("type", ASYNC_TYPE); | |
| // create Async API | |
| ApiResponse<APIDTO> response = null; | |
| try { | |
| response = restAPIPublisher | |
| .importAsyncAPIDefinition(file, additionalPropertiesObj.toString()); | |
| } catch (ApiException e) { | |
| Assert.assertTrue(e.getResponseBody() | |
| .contains("ASYNC type APIs only can be created as third party APIs")); | |
| } | |
| if (response != null) { | |
| Assert.fail(); | |
| } | |
| } | |
| @Test(description = "Create Async API V2 as a normal API") | |
| public void testCreateAsyncApiWithoutAdvertiseOnly() throws Exception { | |
| asyncAPIDefinition = IOUtils.toString( | |
| getClass().getClassLoader().getResourceAsStream("async" + File.separator + "asyncapiv2.yaml"), | |
| StandardCharsets.UTF_8); | |
| File file = getTempFileWithContent(asyncAPIDefinition); | |
| JSONObject additionalPropertiesObj = new JSONObject(); | |
| additionalPropertiesObj.put("name", ASYNC_API_V2_NAME); | |
| additionalPropertiesObj.put("context", ASYNC_API_V2_CONTEXT); | |
| additionalPropertiesObj.put("version", ASYNC_API_V2_VERSION); | |
| additionalPropertiesObj.put("type", ASYNC_TYPE); | |
| // create Async API | |
| ApiResponse<APIDTO> response = null; | |
| try { | |
| response = restAPIPublisher | |
| .importAsyncAPIDefinition(file, additionalPropertiesObj.toString()); | |
| } catch (ApiException e) { | |
| String respBody = e.getResponseBody(); | |
| Assert.assertNotNull(respBody, | |
| "Expected error response body for Async API creation failure"); | |
| Assert.assertTrue(respBody | |
| .contains("ASYNC type APIs only can be created as third party APIs")); | |
| } | |
| if (response != null) { | |
| Assert.fail(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
all-in-one-apim/modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/am/integration/tests/streamingapis/async/AsyncAPITestWithValidationCase.java
around lines 92–118 (and similarly update the other test around line 140), the
catch block directly calls e.getResponseBody() which can be null; change this to
the null-safe pattern used at line 298 by first assigning a non-null string
(e.g. use a ternary or Objects.toString(e.getResponseBody(), "")) to a local
variable and then assert that that variable contains the expected message,
keeping the existing response != null failure check intact.
1 participant
Add new integration tests for the new/modified Async Api parsers.
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.