Fix APIM Service Catalog Issues and Add Test Cases#23
Merged
MohamedSabthar merged 3 commits intoballerina-platform:mainfrom Feb 20, 2026
Merged
Conversation
VellummyilumVinoth
requested review from
SasinduDilshara and
manuranga
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBumps project and distribution versions, adds stdlib uuid and data.jsondata, updates many stdlib/OpenAPI versions, moves test mock/token servers into Dockerized projects, restructures tests with per-sample lifecycle hooks, and updates numerous dependency manifests and test assertions. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Test Runner
participant Gradle as Gradle
participant Docker as Docker Engine
participant Token as Token Server (container)
participant Mock as Mock Server (container)
participant FS as Host FS
Runner->>Gradle: start tests
Gradle->>Docker: build Token & Mock images
Docker->>Token: create & run container (BeforeSuite)
Token-->>Runner: readiness probe OK
loop per-sample
Runner->>Docker: run Mock container (BeforeGroups: port,index,unauth)
Docker->>Mock: start container
Mock-->>Runner: readiness probe OK
Runner->>Token: request /oauth2/token
Token-->>Runner: return access_token
Runner->>Mock: POST /services (artifact)
Mock->>FS: write artifacts_<index>.json
Mock-->>Runner: respond (200/401/500)
Runner->>Docker: stop Mock (AfterGroups)
end
Runner->>Docker: stop Token container (AfterSuite)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina/Ballerina.toml`:
- Around line 4-5: The version value "version" currently set to 1.1.3-rc1 does
not match the native jar filename in "path" which includes -SNAPSHOT; update
either the version key or the jar filename so they match: either change version
= "1.1.3-rc1" to version = "1.1.3-rc1-SNAPSHOT" or change the path filename
wso2.apim.catalog-native-1.1.3-rc1-SNAPSHOT.jar to
wso2.apim.catalog-native-1.1.3-rc1.jar so the "version" and the jar name in
"path" are consistent.
- Around line 13-14: The Ballerina.toml currently sets version = "1.1.3-rc1" but
the native JAR path uses a mismatched hybrid version
"wso2.apim.catalog-native-1.1.3-rc1-SNAPSHOT.jar"; fix by making the version and
path consistent: either update the path value to point to
"wso2.apim.catalog-native-1.1.3-SNAPSHOT.jar" to match gradle.properties'
version (1.1.3-SNAPSHOT), or change the version key to "1.1.3-rc1" (and then
keep the path as-is) so both the version and path align. Ensure the 'version'
key and the 'path' string in Ballerina.toml match exactly.
In `@ballerina/CompilerPlugin.toml`:
- Around line 5-9: The CompilerPlugin.toml currently hardcodes
lib/ballerina-to-openapi-2.4.0.jar which mismatches the declared openapiVersion
in gradle.properties (openapiVersion=2.0.3); regenerate the TOML via the
updateTomlFiles Gradle task (run gradle build) so the dependency path in
CompilerPlugin.toml (the [[dependency]] with path
"lib/ballerina-to-openapi-*.jar") is updated to use the correct version (2.0.3)
or make the taml generation use the gradle property directly so future builds
stay in sync.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ballerina-tests/Dependencies.toml (1)
287-294:⚠️ Potential issue | 🟡 MinorUpdate
observedependency version inballerina-tests/Dependencies.tomlto 1.7.1 for consistency.The test manifest pins
observeto 1.7.0, while all sample projects use 1.7.1. This version mismatch can lead to inconsistent dependency resolution between test execution and sample scenarios. Regenerate the file to align with sample project versions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/Dependencies.toml` around lines 287 - 294, The observe package entry currently pins version "1.7.0"; update the version value to "1.7.1" for the [[package]] with org "ballerina" and name "observe" in Dependencies.toml and then regenerate the manifest so the test dependency aligns with sample projects (ensure the dependencies array including {org = "ballerina", name = "jballerina.java"} remains unchanged).
🧹 Nitpick comments (1)
gradle.properties (1)
44-45: Consider using released Observe versions for reproducible builds.These timestamped Observe versions look like nightly artifacts; please verify this aligns with release policy and availability. If reproducibility is a goal, consider pinning to a stable release tag instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle.properties` around lines 44 - 45, The gradle properties currently pin timestamped/nightly artifacts in observeVersion and observeInternalVersion which harms reproducible builds; replace those values with stable released Observe version tags (e.g., a released semver like 1.7.1 or the project's official release identifier) or document/choose a release channel policy, updating observeVersion and observeInternalVersion accordingly, and verify those release coordinates exist in your artifact repository.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-tests/tests/test_services.bal`:
- Around line 63-65: The test data in function testdataGen includes an entry
[12] but the resources for that case are missing; create a new directory named
sample_project_12 under the tests/configs location and add the required
Config.toml (and any other config files the test expects) so
getConfigFilePath(12) can find sample_project_12/Config.toml at runtime; ensure
the configuration contents match what assert_12.json expects so the test case
runs successfully.
In `@test-resources/sample_project_6/Dependencies.toml`:
- Around line 307-310: Dependencies.toml lists package org="ballerinax",
name="wso2.apim.catalog" with version "1.1.3" but Ballerina.toml declares
"1.1.0" — pick the intended version and make both files consistent by updating
the version string in the other file to match (e.g., change Ballerina.toml to
1.1.3 or Dependencies.toml to 1.1.0); ensure you update the version value for
the package identifier wso2.apim.catalog and regenerate or update any dependency
lock/metadata if your build tool requires it.
- Around line 6-8: The distribution version in sample_project_6/Ballerina.toml
needs to be updated to match Dependencies.toml; open
sample_project_6/Ballerina.toml and change the distribution field so it reads
distribution = "2201.13.1" (matching the [ballerina].distribution-version in
Dependencies.toml) to avoid resolution mismatches.
---
Outside diff comments:
In `@ballerina-tests/Dependencies.toml`:
- Around line 287-294: The observe package entry currently pins version "1.7.0";
update the version value to "1.7.1" for the [[package]] with org "ballerina" and
name "observe" in Dependencies.toml and then regenerate the manifest so the test
dependency aligns with sample projects (ensure the dependencies array including
{org = "ballerina", name = "jballerina.java"} remains unchanged).
---
Nitpick comments:
In `@gradle.properties`:
- Around line 44-45: The gradle properties currently pin timestamped/nightly
artifacts in observeVersion and observeInternalVersion which harms reproducible
builds; replace those values with stable released Observe version tags (e.g., a
released semver like 1.7.1 or the project's official release identifier) or
document/choose a release channel policy, updating observeVersion and
observeInternalVersion accordingly, and verify those release coordinates exist
in your artifact repository.
5b5c8da to
5909ebc
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test-resources/sample_project_3/Dependencies.toml (1)
1-322:⚠️ Potential issue | 🔴 CriticalUpdate distribution version to a released Ballerina version.
The file specifies
distribution-version = "2201.13.1"(line 8), but the latest available Ballerina distribution as of February 2026 is v2201.12.11 (released January 7, 2026). The version in this file does not exist yet. Update it to a valid, released version such as2201.12.11or verify the intended version from your Ballerina tooling output.The remaining changes (package version bumps, new
uuidpackage,wso2.apim.catalogversion update) appear structurally correct for this auto-generated dependencies file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-resources/sample_project_3/Dependencies.toml` around lines 1 - 322, The distribution-version field currently set as distribution-version = "2201.13.1" is invalid; update that value to a released Ballerina distribution (e.g., distribution-version = "2201.12.11") by replacing the string in the Dependencies.toml entry named distribution-version, then re-run your Ballerina tooling (or verify the intended version) to confirm the correct released version is used.
🧹 Nitpick comments (6)
ballerina-tests/token-server/Dockerfile (1)
30-30: Avoid hardcoding sensitive values in Dockerfiles.The keystore password
ballerinais hardcoded in the Dockerfile. While this is test infrastructure, prefer passing secrets via build args or runtime environment variables to avoid committing credentials.♻️ Proposed improvement
ENV PORT=9444 ENV KEYSTORE_PATH=/resources/ballerinaKeystore.p12 -ENV KEYSTORE_PASSWORD=ballerina +# KEYSTORE_PASSWORD should be provided at runtime: docker run -e KEYSTORE_PASSWORD=... +ENV KEYSTORE_PASSWORD=""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/token-server/Dockerfile` at line 30, The Dockerfile currently hardcodes the keystore password via ENV KEYSTORE_PASSWORD=ballerina; replace this by removing the fixed literal and sourcing the secret from a build ARG or runtime environment variable instead (e.g., declare ARG KEYSTORE_PASSWORD and use ENV KEYSTORE_PASSWORD=$KEYSTORE_PASSWORD or require callers to pass KEYSTORE_PASSWORD at container start), and ensure any CI/docker-compose docs or build scripts are updated to supply the secret rather than committing it in the Dockerfile.ballerina-tests/mock-server/Dockerfile (1)
23-35: Consider running as a non-root user for defense in depth.The container runs as root by default. While this is test infrastructure, adding a non-root user improves security posture.
♻️ Proposed improvement
# Stage 2: Minimal runtime image FROM eclipse-temurin:21-jre WORKDIR /app +RUN addgroup --system appgroup && adduser --system --ingroup appgroup appuser COPY --from=builder /app/target/bin/apim_catalog_mock_server.jar /app/mock-server.jar ENV PORT=8080 ENV ARTIFACT_INDEX=0 ENV UNAUTHORIZED=false ENV OUTPUT_DIR=/artifacts -EXPOSE ${PORT} +EXPOSE 8080 +USER appuser ENTRYPOINT ["java", "-jar", "/app/mock-server.jar"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/mock-server/Dockerfile` around lines 23 - 35, The Dockerfile's final stage runs the image as root (ENTRYPOINT ["java", "-jar", "/app/mock-server.jar"]) which is risky—create a dedicated non-root user and switch to it: add commands in the Stage 2 block to create a user/group (e.g., uid 1000), set HOME, chown necessary paths like /app and $OUTPUT_DIR, and then add a USER instruction before the ENTRYPOINT so the Java process runs unprivileged; ensure ENV values (PORT, OUTPUT_DIR) remain accessible to that user.gradle.properties (1)
44-45: Timestamped snapshot versions are isolated to observe packages—consider pinning to stable releases if available.The
observeVersionandobserveInternalVersionuse timestamped snapshot identifiers (1.7.1-20260213-094900-5124eb4and1.7.1-20260213-102300-6dc6d7a). While these are limited to these two packages, timestamped snapshots can cause reproducibility issues if versions become unavailable long-term or are intended for stable releases. If stable versions exist, consider pinning to those instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle.properties` around lines 44 - 45, The observeVersion and observeInternalVersion properties use timestamped snapshot values which can break reproducibility; update the gradle properties by replacing observeVersion and observeInternalVersion with their corresponding stable release identifiers (or a documented, fixed artifact version) if stable releases exist, otherwise add a clear comment and fallback mechanism so consumers use a persistent, non-timestamped coordinate; locate the properties named observeVersion and observeInternalVersion and change their values to the canonical stable version strings (or annotate them) so builds remain reproducible.ballerina-tests/mock-server/main.bal (1)
63-101: Consider refactoring to reduce duplication between authorized and unauthorized modes.Both service definitions share identical
healthandGET servicesendpoints. OnlyPOST servicesdiffers in return type. While functional, this duplication increases maintenance burden.♻️ Alternative: use a single service with conditional return
One approach is to make
POST servicesreturn a union type and conditionally return based on theunauthorizedflag:resource function post services(http:Request req) returns http:Unauthorized|http:InternalServerError|error { ServiceSchema schema = check traverseRequest(req); artifacts.push(schema); check io:fileWriteJson(artifactFile, artifacts.toJson()); if unauthorized { return <http:Unauthorized>{body: {message: "Unauthorized"}}; } return <http:InternalServerError>{body: {message: "Return 500 Status code after completing the task"}}; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/mock-server/main.bal` around lines 63 - 101, Duplicate service objects for authorized/unauthorized modes should be collapsed into a single service object (svc): keep the existing resource function get health and resource function get services as-is, then change resource function post services to return a union type that includes http:Unauthorized, http:InternalServerError and error and perform a conditional return based on the existing unauthorized flag; inside post call traverseRequest, push to artifacts and write artifactFile as before, then if unauthorized return the Unauthorized variant otherwise return the InternalServerError variant. Reference: svc, resource function post services, traverseRequest, artifacts, artifactFile, and the unauthorized flag.ballerina-tests/tests/test_utils.bal (2)
117-118: Same optimization opportunity for token server initialization.Consider removing the fixed 15-second sleep and relying on the health check retry loop, similar to the mock server function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/tests/test_utils.bal` around lines 117 - 118, Remove the fixed runtime:sleep(DOCKER_START_TIMEOUT) after creating the Docker container (the io:println line referencing containerName) and replace it with the same health-check retry loop used for the mock server initialization: poll the token server's health endpoint until it returns healthy or a timeout/retry limit is reached, logging attempts and failing deterministically on timeout; specifically, eliminate the runtime:sleep call and implement the retry logic (HTTP health check + backoff) used by the mock server function so readiness is detected dynamically rather than waiting a fixed 15s.
70-71: Consider starting health checks immediately instead of fixed sleep.The 15-second fixed sleep on line 71 delays test execution unnecessarily. The health check loop (lines 74-85) already handles readiness polling with retries.
♻️ Proposed optimization
io:println(string `Docker container '${containerName}' created, waiting for readiness...`); - runtime:sleep(DOCKER_START_TIMEOUT); // Health-check the mock server via GET /health int counter = 0; while counter < DOCKER_RETRY_COUNT { + runtime:sleep(DOCKER_RETRY_DELAY); http:Client|error httpClient = new (string `http://localhost:${port}`, {timeout: 3});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/tests/test_utils.bal` around lines 70 - 71, Remove the fixed wait by deleting the runtime:sleep(DOCKER_START_TIMEOUT) call that follows the io:println for containerName and instead start the existing readiness/health-check polling immediately; locate the print of "Docker container '${containerName}' created..." and the subsequent runtime:sleep call and remove that sleep so the health check loop (the readiness polling logic following that print) runs without the unnecessary 15s delay.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-tests/mock-server/Dockerfile`:
- Line 33: The Dockerfile uses EXPOSE ${PORT}, but Dockerfile instructions do
not expand environment variables at build time; replace the EXPOSE ${PORT} line
with a literal port number (the actual port used by the mock server) so EXPOSE
lists the numeric port instead of attempting to use the PORT variable; locate
the EXPOSE ${PORT} entry in the Dockerfile and change it to EXPOSE <port-number>
consistent with how the server is configured to listen.
In `@ballerina-tests/token-server/Dockerfile`:
- Line 32: The Dockerfile currently uses "EXPOSE ${PORT}" which won't expand at
build-time; change the EXPOSE line to the literal port number used by the
service (replace EXPOSE ${PORT} with EXPOSE <port-number>) so Docker exposes the
correct port; locate the EXPOSE directive in the Dockerfile and use the actual
numeric PORT value defined for the token-server instead of the ${PORT} variable.
In `@native/src/main/java/io/ballerina/wso2/apim/catalog/utils/Constants.java`:
- Line 50: The LOCALHOST constant was changed to "localhost", causing invalid
URLs when HttpServiceConfig (used in ServiceCatalog.java) builds
definitionUrl/SERVICE_URL/DEFINITION_URL via host + COLON + port + basePath;
restore the protocol by changing the LOCALHOST constant back to
"http://localhost" so concatenation in the HttpServiceConfig constructor
produces valid URLs (e.g., "http://localhost:8080/api"); after changing
LOCALHOST, run a quick check of HttpServiceConfig and ServiceCatalog to ensure
no double-protocols occur.
In `@test-resources/sample_project_8/Dependencies.toml`:
- Around line 6-9: Update the distribution value in the Ballerina.toml for
sample_project_8 so it matches Dependencies.toml: change the distribution key in
Ballerina.toml from "2201.10.0" to "2201.13.1" (ensure the Ballerina.toml
'distribution' field equals the Dependencies.toml 'distribution-version' value
to avoid toolchain conflicts).
---
Outside diff comments:
In `@test-resources/sample_project_3/Dependencies.toml`:
- Around line 1-322: The distribution-version field currently set as
distribution-version = "2201.13.1" is invalid; update that value to a released
Ballerina distribution (e.g., distribution-version = "2201.12.11") by replacing
the string in the Dependencies.toml entry named distribution-version, then
re-run your Ballerina tooling (or verify the intended version) to confirm the
correct released version is used.
---
Duplicate comments:
In `@ballerina-tests/tests/test_services.bal`:
- Around line 282-297: The test testSample12 calls getConfigFilePath(12) and
expects sample_project_12 config files and artifacts_12.json to exist; ensure
the config directory and files are present by adding or verifying
ballerina-tests/tests/configs/sample_project_12/Config.toml (and any other files
referenced by getConfigFilePath/getProjPath) and that artifacts_12.json (and
assert_12.json if used) in the test assets match the expected contents
referenced by readAndValidateArtifacts; update or add the sample_project_12
directory and the artifacts_12.json file so getProjName, getProjPath,
getConfigFilePath, and readAndValidateArtifacts have the corresponding resources
at runtime.
In `@ballerina/Ballerina.toml`:
- Around line 13-14: The version field and the JAR path are inconsistent: the
version key (version = "1.2.0") does not match the artifact path which
references "1.2.0-SNAPSHOT"; update the version value to "1.2.0-SNAPSHOT" or
change the path to point to a non-SNAPSHOT JAR so both the version and path (the
version property and path string in Ballerina.toml) match exactly.
In `@test-resources/sample_project_6/Dependencies.toml`:
- Around line 6-8: The distribution-version in Dependencies.toml
("distribution-version" = "2201.13.1") does not match the distribution declared
in Ballerina.toml (reported "2201.10.0"); update one so both files are identical
— either change the distribution-version key in Dependencies.toml to "2201.10.0"
to match Ballerina.toml or update Ballerina.toml to "2201.13.1" — ensure
dependencies-toml-version remains correct and run the project build to verify
resolution.
- Around line 307-310: Dependencies.toml declares package org="ballerinax"
name="wso2.apim.catalog" with version "1.1.3" which conflicts with the version
in Ballerina.toml ("1.1.0"); make them consistent by updating the version string
so both files match—either change Dependencies.toml's version to "1.1.0" or
update Ballerina.toml to "1.1.3" (whichever is the intended release), ensuring
the package identifier (org/name) remains the same and then re-run the
build/tests to verify no further mismatches.
---
Nitpick comments:
In `@ballerina-tests/mock-server/Dockerfile`:
- Around line 23-35: The Dockerfile's final stage runs the image as root
(ENTRYPOINT ["java", "-jar", "/app/mock-server.jar"]) which is risky—create a
dedicated non-root user and switch to it: add commands in the Stage 2 block to
create a user/group (e.g., uid 1000), set HOME, chown necessary paths like /app
and $OUTPUT_DIR, and then add a USER instruction before the ENTRYPOINT so the
Java process runs unprivileged; ensure ENV values (PORT, OUTPUT_DIR) remain
accessible to that user.
In `@ballerina-tests/mock-server/main.bal`:
- Around line 63-101: Duplicate service objects for authorized/unauthorized
modes should be collapsed into a single service object (svc): keep the existing
resource function get health and resource function get services as-is, then
change resource function post services to return a union type that includes
http:Unauthorized, http:InternalServerError and error and perform a conditional
return based on the existing unauthorized flag; inside post call
traverseRequest, push to artifacts and write artifactFile as before, then if
unauthorized return the Unauthorized variant otherwise return the
InternalServerError variant. Reference: svc, resource function post services,
traverseRequest, artifacts, artifactFile, and the unauthorized flag.
In `@ballerina-tests/tests/test_utils.bal`:
- Around line 117-118: Remove the fixed runtime:sleep(DOCKER_START_TIMEOUT)
after creating the Docker container (the io:println line referencing
containerName) and replace it with the same health-check retry loop used for the
mock server initialization: poll the token server's health endpoint until it
returns healthy or a timeout/retry limit is reached, logging attempts and
failing deterministically on timeout; specifically, eliminate the runtime:sleep
call and implement the retry logic (HTTP health check + backoff) used by the
mock server function so readiness is detected dynamically rather than waiting a
fixed 15s.
- Around line 70-71: Remove the fixed wait by deleting the
runtime:sleep(DOCKER_START_TIMEOUT) call that follows the io:println for
containerName and instead start the existing readiness/health-check polling
immediately; locate the print of "Docker container '${containerName}'
created..." and the subsequent runtime:sleep call and remove that sleep so the
health check loop (the readiness polling logic following that print) runs
without the unnecessary 15s delay.
In `@ballerina-tests/token-server/Dockerfile`:
- Line 30: The Dockerfile currently hardcodes the keystore password via ENV
KEYSTORE_PASSWORD=ballerina; replace this by removing the fixed literal and
sourcing the secret from a build ARG or runtime environment variable instead
(e.g., declare ARG KEYSTORE_PASSWORD and use ENV
KEYSTORE_PASSWORD=$KEYSTORE_PASSWORD or require callers to pass
KEYSTORE_PASSWORD at container start), and ensure any CI/docker-compose docs or
build scripts are updated to supply the secret rather than committing it in the
Dockerfile.
In `@gradle.properties`:
- Around line 44-45: The observeVersion and observeInternalVersion properties
use timestamped snapshot values which can break reproducibility; update the
gradle properties by replacing observeVersion and observeInternalVersion with
their corresponding stable release identifiers (or a documented, fixed artifact
version) if stable releases exist, otherwise add a clear comment and fallback
mechanism so consumers use a persistent, non-timestamped coordinate; locate the
properties named observeVersion and observeInternalVersion and change their
values to the canonical stable version strings (or annotate them) so builds
remain reproducible.
native/src/main/java/io/ballerina/wso2/apim/catalog/utils/Constants.java
Outdated
Show resolved
Hide resolved
VellummyilumVinoth
force-pushed
the
fix-apim-catalog-service
branch
2 times, most recently
from
50dbf0f to
4c1659c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
native/src/main/java/io/ballerina/wso2/apim/catalog/ServiceCatalog.java (1)
255-263:⚠️ Potential issue | 🔴 CriticalFix URL protocol scheme in
HttpServiceConfigconstructor.The
HttpServiceConfigconstructor buildsdefinitionUrlvia concatenation (host + COLON + port + basePath), butgetHostname()returns just"localhost"from theLOCALHOSTconstant (Constants.java:51), without any protocol scheme. This produces malformed URLs likelocalhost:8080/basePathinstead of proper HTTP URLs likehttp://localhost:8080/basePath, breaking service catalog functionality.Add the protocol scheme in the constructor or ensure
getHostname()returns a properly formatted URL with the scheme.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@native/src/main/java/io/ballerina/wso2/apim/catalog/ServiceCatalog.java` around lines 255 - 263, HttpServiceConfig's constructor builds definitionUrl by concatenating host + COLON + port + basePath but host (from getHostname()/LOCALHOST) lacks the protocol, producing malformed URLs; update the HttpServiceConfig(String host, String port, String basePath) constructor to ensure the scheme is present (e.g., prepend "http://" when host does not already start with "http://" or "https://") before forming definitionUrl, or alternatively ensure getHostname()/LOCALHOST returns a scheme-qualified host; modify only the constructor logic in HttpServiceConfig to normalize the host and then set this.definitionUrl = normalizedHost + COLON + port + basePath.test-resources/sample_project_1/Ballerina.toml (1)
1-5:⚠️ Potential issue | 🟠 MajorAlign distribution version with Dependencies.toml.
Line 5 uses
distribution = "2201.10.0"whiletest-resources/sample_project_1/Dependencies.tomllistsdistribution-version = "2201.13.1". This mismatch can lead to build/test toolchain conflicts.🔧 Proposed fix
-distribution = "2201.10.0" +distribution = "2201.13.1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-resources/sample_project_1/Ballerina.toml` around lines 1 - 5, The Ballerina.toml distribution value is inconsistent with Dependencies.toml; update the distribution field in Ballerina.toml (the line with distribution = "...") to match the distribution-version value in Dependencies.toml ("2201.13.1") so both files use the same distribution string and avoid toolchain conflicts.test-resources/sample_project_9/Ballerina.toml (1)
5-10:⚠️ Potential issue | 🟡 MinorDistribution version mismatch with Dependencies.toml.
Ballerina.tomldeclaresdistribution = "2201.10.0"(Line 5) but the correspondingDependencies.tomlwas generated withdistribution-version = "2201.13.1". Consider updating todistribution = "2201.13.1"to ensure consistency and prevent potential resolution mismatches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-resources/sample_project_9/Ballerina.toml` around lines 5 - 10, Ballerina.toml's distribution value is out of sync with Dependencies.toml; update the distribution property in Ballerina.toml from "2201.10.0" to match Dependencies.toml's distribution-version "2201.13.1" so both files use the same runtime version (look for the distribution key in Ballerina.toml and the distribution-version key in Dependencies.toml to verify consistency).test-resources/sample_project_10/Ballerina.toml (1)
5-10:⚠️ Potential issue | 🟡 MinorDistribution version mismatch.
distribution = "2201.10.0"does not match theDependencies.tomldistribution version of2201.13.1. Update for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-resources/sample_project_10/Ballerina.toml` around lines 5 - 10, Update the distribution value in the Ballerina.toml by changing the distribution entry currently set to "2201.10.0" to match the Dependencies.toml distribution version "2201.13.1" so the top-level distribution = value is consistent with the project's Dependencies.toml.test-resources/sample_project_5/Ballerina.toml (1)
5-10:⚠️ Potential issue | 🟡 MinorDistribution version mismatch - same pattern as other sample projects.
Consider updating
distribution = "2201.10.0"to"2201.13.1"to match the generatedDependencies.toml. This mismatch appears across multiple sample projects in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-resources/sample_project_5/Ballerina.toml` around lines 5 - 10, The Ballerina.toml has a distribution version mismatch: update the distribution key value from "2201.10.0" to "2201.13.1" so it matches the generated Dependencies.toml; search other sample projects for the same distribution = "2201.10.0" entry and replace them with "2201.13.1" to keep all sample project manifests consistent.test-resources/sample_project_11/Ballerina.toml (1)
5-10:⚠️ Potential issue | 🟡 MinorDistribution version mismatch.
distribution = "2201.10.0"does not match theDependencies.tomldistribution version of2201.13.1. Update for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test-resources/sample_project_11/Ballerina.toml` around lines 5 - 10, The distribution version in Ballerina.toml is inconsistent with Dependencies.toml; update the distribution value (the distribution = "2201.10.0" entry) to match Dependencies.toml (use "2201.13.1") so both files reference the same distribution version.
🧹 Nitpick comments (4)
ballerina-tests/tests/test_utils.bal (1)
141-150: Container cleanup relies on--rmflag fromdocker run.The
cleanDockerContainerfunction only callsdocker stop, which works because containers are started with--rm(auto-remove on stop). Consider adding a comment to document this dependency, or explicitly calldocker rm -fas a fallback.📝 Add clarifying comment
// Stop and remove a Docker container. +// Note: Containers are started with --rm flag, so stop will also remove them. function cleanDockerContainer(string containerName) returns error? {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/tests/test_utils.bal` around lines 141 - 150, The cleanDockerContainer function currently only runs docker stop (in function cleanDockerContainer) and relies on tests starting containers with --rm; update the function to either add a clarifying comment documenting this dependency or make it robust by attempting docker rm -f as a fallback (e.g., if docker stop succeeds but container still exists, run docker rm -f <containerName>), ensuring you keep the existing exit-code checks (result.waitForExit and test:assertExactEquals) and reference the same containerName parameter.ballerina-tests/mock-server/main.bal (1)
63-103: Consider extracting common service logic to reduce duplication.The
authorizedandunauthorizedservice branches share identical/healthand/servicesGET handlers. Only the POST response type differs. Consider extracting shared logic or using a factory pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/mock-server/main.bal` around lines 63 - 103, The two service object branches duplicate resource function get health and get services; extract the shared handlers into a single reusable implementation and only vary the POST behavior via a small factory or by injecting a POST handler. Concretely, move the body of resource function get health and get services into shared functions (e.g., sharedGetHealth(), sharedGetServices()) or a base service object, keep traverseRequest, artifacts and artifactFile usage as-is, and implement a factory function (e.g., createService(postHandler)) that returns a service object where resource function post services calls the injected postHandler (one handler returning http:Unauthorized and another returning http:InternalServerError) so the only difference between authorized/unauthorized instances is the POST response type.ballerina-tests/build.gradle (1)
102-123: Consider using versioned Docker image tags for reproducibility.The Docker images use
:latesttags which can lead to inconsistent test results if images are rebuilt at different times. Consider using a version tag derived from the project version:- commandLine 'docker', 'build', '-t', 'ballerinax/apim-catalog-mock-server:latest', '.' + commandLine 'docker', 'build', '-t', "ballerinax/apim-catalog-mock-server:${project.version}", '.'Additionally, the tasks assume Docker is available. If tests run in environments without Docker, they will fail with an unclear error message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/build.gradle` around lines 102 - 123, The Docker build tasks (buildMockServerDockerImage, buildTokenServerDockerImage, referenced by buildDockerImages) currently hard-code the :latest tag and assume Docker is present; change the tag to a stable, reproducible version (e.g., use project.version or a derived variable like "${project.version}" instead of 'latest') in both commandLine invocations, and add a pre-check or graceful fallback that verifies Docker availability (or skips the task with a clear message) before calling exec so CI runs without Docker fail fast and error messages are informative.ballerina-tests/tests/test_service_impl.bal (1)
17-20: Consider removing this file or converting to documentation.This file now contains only explanatory comments with no executable code. Consider either:
- Removing the file entirely and adding the explanation to a README or the actual test files
- Keeping it if there's a convention for such placeholder files in this project
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina-tests/tests/test_service_impl.bal` around lines 17 - 20, This file contains only explanatory comments and should be removed or converted into documentation: either delete test_service_impl.bal and move the comments into the project README or into test_services.bal/test_utils.bal as a short explanatory section, or keep test_service_impl.bal only if your project convention requires placeholder files—if you retain it, add a one-line header stating it is an intentional placeholder and reference the real lifecycle functions (initializeMockServerContainer/cleanDockerContainer) to make its purpose explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina-tests/mock-server/main.bal`:
- Around line 111-116: The function traverseRequest currently indexes
bodyParts[0] and bodyParts[1] without checking length which can panic; update
traverseRequest to validate the multipart bodyParts array has at least two
elements before accessing them and return a descriptive error if not (e.g.,
"missing multipart parts" or similar). Also ensure the existing checks on
bodyParts[0].getJson() and bodyParts[1].getText() remain (or add additional
error handling) so failures are propagated as errors rather than panics;
reference the bodyParts variable and the traverseRequest, ServiceItem and
ServiceSchema return path when implementing this validation.
In `@ballerina-tests/tests/test_services.bal`:
- Around line 47-53: The assertions in the sample tests invert success/failure:
runOSCommand (used in testSample0 and the analogous testSample1..testSample8
functions) returns () on success and error on failure, but the tests currently
assert result is error then proceed to readAndValidateArtifacts; update each
test (e.g., testSample0) to assert success instead by replacing
test:assertTrue(result is error) with an assertion that result is not an error
(for example test:assertTrue(!(result is error)) or test:assertNotEquals(result,
error)) so the tests correctly expect successful execution before calling
readAndValidateArtifacts.
In `@ballerina/service.bal`:
- Around line 48-57: The function publishArtifacts currently overwrites
lastError and loses earlier failures; change it to collect all failures by
replacing error? lastError with a collection (e.g., an array or map) of failure
records and, inside the foreach over artifacts, when
publishOrUpdateService(artifact) returns an error record the code should append
an entry containing the artifact identity (e.g., artifact.id or other
distinguishing fields) and the error to that collection; after the loop, if the
collection is non-empty return a single composite error that includes details
for every failed artifact, otherwise return nil/success. Use the existing
function names publishArtifacts and publishOrUpdateService to locate and update
the logic and remove the single lastError variable.
In `@test-resources/sample_project_12/Ballerina.toml`:
- Around line 5-8: The Ballerina.toml's distribution value ("distribution") is
out of sync with Dependencies.toml's "distribution-version"; update the
distribution key in Ballerina.toml to match the distribution-version used in
Dependencies.toml (i.e., set distribution to "2201.13.1") so both manifest files
use the same Ballerina distribution version.
---
Outside diff comments:
In `@native/src/main/java/io/ballerina/wso2/apim/catalog/ServiceCatalog.java`:
- Around line 255-263: HttpServiceConfig's constructor builds definitionUrl by
concatenating host + COLON + port + basePath but host (from
getHostname()/LOCALHOST) lacks the protocol, producing malformed URLs; update
the HttpServiceConfig(String host, String port, String basePath) constructor to
ensure the scheme is present (e.g., prepend "http://" when host does not already
start with "http://" or "https://") before forming definitionUrl, or
alternatively ensure getHostname()/LOCALHOST returns a scheme-qualified host;
modify only the constructor logic in HttpServiceConfig to normalize the host and
then set this.definitionUrl = normalizedHost + COLON + port + basePath.
In `@test-resources/sample_project_1/Ballerina.toml`:
- Around line 1-5: The Ballerina.toml distribution value is inconsistent with
Dependencies.toml; update the distribution field in Ballerina.toml (the line
with distribution = "...") to match the distribution-version value in
Dependencies.toml ("2201.13.1") so both files use the same distribution string
and avoid toolchain conflicts.
In `@test-resources/sample_project_10/Ballerina.toml`:
- Around line 5-10: Update the distribution value in the Ballerina.toml by
changing the distribution entry currently set to "2201.10.0" to match the
Dependencies.toml distribution version "2201.13.1" so the top-level distribution
= value is consistent with the project's Dependencies.toml.
In `@test-resources/sample_project_11/Ballerina.toml`:
- Around line 5-10: The distribution version in Ballerina.toml is inconsistent
with Dependencies.toml; update the distribution value (the distribution =
"2201.10.0" entry) to match Dependencies.toml (use "2201.13.1") so both files
reference the same distribution version.
In `@test-resources/sample_project_5/Ballerina.toml`:
- Around line 5-10: The Ballerina.toml has a distribution version mismatch:
update the distribution key value from "2201.10.0" to "2201.13.1" so it matches
the generated Dependencies.toml; search other sample projects for the same
distribution = "2201.10.0" entry and replace them with "2201.13.1" to keep all
sample project manifests consistent.
In `@test-resources/sample_project_9/Ballerina.toml`:
- Around line 5-10: Ballerina.toml's distribution value is out of sync with
Dependencies.toml; update the distribution property in Ballerina.toml from
"2201.10.0" to match Dependencies.toml's distribution-version "2201.13.1" so
both files use the same runtime version (look for the distribution key in
Ballerina.toml and the distribution-version key in Dependencies.toml to verify
consistency).
---
Duplicate comments:
In `@ballerina-tests/Dependencies.toml`:
- Around line 47-365: The lockfile (Dependencies.toml) has stdlib versions that
don't match the pinned values in gradle.properties (e.g., package names crypto,
http, log, mime, task, jwt, oauth2, data.jsondata, time, url, os); update each
package's "version" field in Dependencies.toml to the corresponding pin from
gradle.properties and then regenerate the lockfile (or re-run the dependency
lock/generation step) so the auto-generated file stays aligned with the intended
stdlib pins.
In `@native/src/main/java/io/ballerina/wso2/apim/catalog/utils/Constants.java`:
- Line 51: The LOCALHOST constant was changed to "localhost" which yields
malformed URLs when ServiceCatalog.HttpServiceConfig constructs hosts via host +
COLON + port + basePath; revert or restore the scheme by changing
Constants.LOCALHOST back to include the protocol (e.g., "http://localhost") so
concatenation in HttpServiceConfig produces valid URLs, and ensure any other
code that uses Constants.LOCALHOST still expects a scheme-aware host string.
In `@test-resources/sample_project_0/Ballerina.toml`:
- Around line 7-11: The dependency entry for wso2.apim.catalog in Ballerina.toml
references version "1.2.0" with repository="local", but there is no verification
that the artifact is published before tests; update the CI/build/test scripts to
ensure the local repository artifact is published (or change repository to a
remote one) prior to running tests—search for references to "wso2.apim.catalog",
repository="local", and any bal push/pack steps and add a publish step (e.g.,
bal push/pack to local repo) or a pre-test check that fails fast if the artifact
is missing so tests don't silently fail.
In `@test-resources/sample_project_3/Ballerina.toml`:
- Around line 7-11: The Ballerina.toml dependency for org "ballerinax" name
"wso2.apim.catalog" is set to version "1.2.0" with repository="local", so ensure
the artifact is published to the local repo before tests; either add a publish
step (e.g., run bal pack/bal push or your CI job that publishes the artifact)
and wire it into your test pipeline, or change repository to a remote repo that
already contains v1.2.0. Search for the symbols "wso2.apim.catalog", version
"1.2.0", and repository="local" (and commands like bal push/bal pack) in
Ballerina.toml and CI/build scripts and update the pipeline to publish the
artifact prior to running tests.
In `@test-resources/sample_project_4/Ballerina.toml`:
- Around line 7-11: The dependency block declaring [[dependency]] with org
"ballerinax", name "wso2.apim.catalog", version "1.2.0" and repository="local"
requires that the artifact be published to the local repository before tests
run; locate references to this dependency and any CI/build scripts (search for
"wso2.apim.catalog", repository="local", "bal push", "bal pack", or "local
repository") and ensure the pipeline includes a step to publish/version 1.2.0
into the local repo (e.g., run bal pack/bal push or whatever project-specific
publish task) or change the dependency to a remote repository; update the
relevant build/test scripts so tests don’t run until the local artifact is
available.
In `@test-resources/sample_project_6/Ballerina.toml`:
- Around line 7-11: The Ballerina.toml dependency block specifies
org="ballerinax", name="wso2.apim.catalog", version="1.2.0" with
repository="local", but tests assume that artifact is already published to the
local repo; update the CI/build scripts to publish that artifact before tests
(e.g., run the bal pack/push or local publish step for wso2.apim.catalog 1.2.0)
or change repository to a remote repo that already contains version 1.2.0;
search build/test scripts for references like "wso2.apim.catalog",
repository="local", "bal push", or "bal pack" and add the publish step prior to
test execution so the local repository contains the required artifact.
In `@test-resources/sample_project_6/Dependencies.toml`:
- Around line 6-8: Update the distribution value in
sample_project_6/Ballerina.toml to match the Dependencies.toml
distribution-version ("2201.13.1"); specifically edit the distribution field in
Ballerina.toml so it equals "2201.13.1" to resolve the mismatch between the
distribution key in Ballerina.toml and the distribution-version key in
Dependencies.toml.
In `@test-resources/sample_project_7/Ballerina.toml`:
- Around line 7-11: The Ballerina.toml dependency for wso2.apim.catalog was
changed to version "1.2.0" with repository="local", so ensure the corresponding
artifact is published to the local repository before running tests; search for
local repo publish or packaging steps and any references to "wso2.apim.catalog",
repository="local", "bal push", or "bal pack" in build and test scripts (look
for files like YAML, Gradle, shell scripts and Ballerina.toml) and add or update
publish steps if missing; you can run the provided ripgrep command to find and
verify necessary publish steps and catalog references.
In `@test-resources/sample_project_8/Ballerina.toml`:
- Around line 1-5: The distribution value in Ballerina.toml (the distribution =
"2201.10.0" field) is out of sync with Dependencies.toml's distribution-version
("2201.13.1"); update the distribution field in Ballerina.toml to exactly match
the distribution-version value in Dependencies.toml (2201.13.1) so both files
use the same toolchain version.
---
Nitpick comments:
In `@ballerina-tests/build.gradle`:
- Around line 102-123: The Docker build tasks (buildMockServerDockerImage,
buildTokenServerDockerImage, referenced by buildDockerImages) currently
hard-code the :latest tag and assume Docker is present; change the tag to a
stable, reproducible version (e.g., use project.version or a derived variable
like "${project.version}" instead of 'latest') in both commandLine invocations,
and add a pre-check or graceful fallback that verifies Docker availability (or
skips the task with a clear message) before calling exec so CI runs without
Docker fail fast and error messages are informative.
In `@ballerina-tests/mock-server/main.bal`:
- Around line 63-103: The two service object branches duplicate resource
function get health and get services; extract the shared handlers into a single
reusable implementation and only vary the POST behavior via a small factory or
by injecting a POST handler. Concretely, move the body of resource function get
health and get services into shared functions (e.g., sharedGetHealth(),
sharedGetServices()) or a base service object, keep traverseRequest, artifacts
and artifactFile usage as-is, and implement a factory function (e.g.,
createService(postHandler)) that returns a service object where resource
function post services calls the injected postHandler (one handler returning
http:Unauthorized and another returning http:InternalServerError) so the only
difference between authorized/unauthorized instances is the POST response type.
In `@ballerina-tests/tests/test_service_impl.bal`:
- Around line 17-20: This file contains only explanatory comments and should be
removed or converted into documentation: either delete test_service_impl.bal and
move the comments into the project README or into
test_services.bal/test_utils.bal as a short explanatory section, or keep
test_service_impl.bal only if your project convention requires placeholder
files—if you retain it, add a one-line header stating it is an intentional
placeholder and reference the real lifecycle functions
(initializeMockServerContainer/cleanDockerContainer) to make its purpose
explicit.
In `@ballerina-tests/tests/test_utils.bal`:
- Around line 141-150: The cleanDockerContainer function currently only runs
docker stop (in function cleanDockerContainer) and relies on tests starting
containers with --rm; update the function to either add a clarifying comment
documenting this dependency or make it robust by attempting docker rm -f as a
fallback (e.g., if docker stop succeeds but container still exists, run docker
rm -f <containerName>), ensuring you keep the existing exit-code checks
(result.waitForExit and test:assertExactEquals) and reference the same
containerName parameter.
29bdd06 to
1291323
Compare
VellummyilumVinoth
force-pushed
the
fix-apim-catalog-service
branch
from
1291323 to
6a49bd7
Compare
VellummyilumVinoth
changed the title
VellummyilumVinoth
changed the title
VellummyilumVinoth
changed the title
VellummyilumVinoth
force-pushed
the
fix-apim-catalog-service
branch
4 times, most recently
from
b0e8a45 to
c7cb0c6
Compare
native/src/main/java/io/ballerina/wso2/apim/catalog/utils/Constants.java
Outdated
Show resolved
Hide resolved
native/src/main/java/io/ballerina/wso2/apim/catalog/ServiceCatalog.java
Outdated
Show resolved
Hide resolved
VellummyilumVinoth
force-pushed
the
fix-apim-catalog-service
branch
from
b024811 to
a9a5a18
Compare
VellummyilumVinoth
force-pushed
the
fix-apim-catalog-service
branch
from
a9a5a18 to
447b4ec
Compare
VellummyilumVinoth
force-pushed
the
fix-apim-catalog-service
branch
from
447b4ec to
6c0dd78
Compare
MohamedSabthar
approved these changes
Feb 20, 2026
This was referenced Feb 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Examples
Checklist
Summary
This PR updates the WSO2 APIM Catalog service to version 1.2.0, moves the project to Ballerina distribution 2201.13.1, and modernizes test infrastructure and dependency management. It also applies targeted fixes to catalog URL handling and artifact filtering.
Key changes
Version and dependency updates
Service logic and behavior changes
Test infrastructure modernization
Expanded and reorganized tests
Build and repository configuration
Overall, the changes align the module with the newer Ballerina platform, improve test reliability through containerization, expand test coverage and scenarios, and fix catalog URL and artifact-filtering behavior.