Use compiler endpoint artifacts for HTTP endpoint export#2640
Open
chiranSachintha wants to merge 1 commit into
Open
Use compiler endpoint artifacts for HTTP endpoint export#2640chiranSachintha wants to merge 1 commit into
chiranSachintha wants to merge 1 commit into
Conversation
chiranSachintha
requested review from
TharmiganK,
daneshk,
lnash94 and
shafreenAnfar
as code owners
📝 WalkthroughWalkthroughEndpoint YAML generation in the compiler plugin is refactored from writing per-service endpoint YAML files to registering endpoint details as a consolidated artifact (endpoints.yaml) via a new context API. The EndpointWrapper class is removed, Jackson dependencies are dropped, and module/build configs and tests are updated accordingly. ChangesEndpoint artifact consolidation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ServiceArtifactsExtractor
participant EndpointYamlGenerator
participant CompilerPluginContext
ServiceArtifactsExtractor->>EndpointYamlGenerator: addEndpointArtifact()
EndpointYamlGenerator->>EndpointYamlGenerator: getEndpoint()
EndpointYamlGenerator->>EndpointYamlGenerator: getEndpointName()
EndpointYamlGenerator->>CompilerPluginContext: addEndpointArtifact(port, basePath, type, schemaPath)
CompilerPluginContext-->>ServiceArtifactsExtractor: consolidated endpoints.yaml entry recorded
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
compiler-plugin-tests/src/test/java/io/ballerina/stdlib/http/compiler/ServiceArtifactsExtractionTest.java (2)
532-553: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMatch the assertion to
endpoints.yaml.safeFileName(p).contains("_endpoint")never matches the consolidated artifact name, so this test can pass even ifendpoints.yamlis generated. Assert directly onartifactDir.resolve("endpoints.yaml")instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compiler-plugin-tests/src/test/java/io/ballerina/stdlib/http/compiler/ServiceArtifactsExtractionTest.java` around lines 532 - 553, The test is checking the wrong artifact name, so it may miss a generated consolidated endpoints file. Update testEndpointYamlSkippedWhenNoServersDefined in ServiceArtifactsExtractionTest to assert directly on artifactDir.resolve("endpoints.yaml") instead of filtering Files.walk with safeFileName(p).contains("_endpoint"). This keeps the check aligned with the actual artifact produced when no servers are defined.
191-202: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale filename makes this test vacuous.
service_endpoint.yamlis no longer emitted; the consolidated artifact isendpoints.yaml, so this assertion always passes and no longer checks the no-server case. Useendpoints.yamlhere, or assert that no*_endpointfiles exist.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compiler-plugin-tests/src/test/java/io/ballerina/stdlib/http/compiler/ServiceArtifactsExtractionTest.java` around lines 191 - 202, The test is asserting against a stale artifact name, so it no longer verifies anything meaningful. Update testConfigurablePortWithRequiredValue in ServiceArtifactsExtractionTest to use the current consolidated artifact name endpoints.yaml, or change the assertion to verify that no *_endpoint files are generated. Keep the check centered on the artifactDir path and the existing executeBallerinaCommand flow.
🧹 Nitpick comments (2)
compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/ServiceArtifactsExtractor.java (1)
169-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
diagnosticsparameter is now unused inexportEndpointYaml. After replacingwriteEndpointYaml()(which added anExceptionDiagnosticonIOException) withaddEndpointArtifact(), this method no longer reads or writesdiagnostics. ConfirmaddEndpointArtifact()/context.addEndpointArtifact()cannot fail in a way that previously surfaced as a diagnostic; if so, drop the now-dead parameter, otherwise reinstate error handling so registration failures aren't silently swallowed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/ServiceArtifactsExtractor.java` around lines 169 - 185, The diagnostics parameter in exportEndpointYaml is now unused after switching from writeEndpointYaml to addEndpointArtifact, so either remove that dead parameter from exportEndpointYaml and its callers if addEndpointArtifact/context.addEndpointArtifact cannot fail, or restore error handling if artifact registration can still throw so failures continue to surface as diagnostics. Check the control flow around exportEndpointYaml, EndpointYamlGenerator.addEndpointArtifact, and any context.addEndpointArtifact usage to keep the method signature and behavior consistent.compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.java (1)
49-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMisplaced Javadoc. This block reads "Adds endpoint details of HTTP service to the build context," which describes
addEndpointArtifact(), but it sits directly above the constructor. Move it ontoaddEndpointArtifact()(or replace it with a constructor-appropriate description).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.java` around lines 49 - 51, The Javadoc for EndpointYamlGenerator is attached to the wrong member: it describes addEndpointArtifact() but currently sits above the constructor. Move that comment to addEndpointArtifact() (or rewrite it to describe the constructor) so the documentation matches the symbol it annotates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.java`:
- Around line 102-103: The endpoint name extraction in getEndpointName() is
removing every occurrence of OPENAPI_SUFFIX instead of only the trailing one,
which can corrupt names containing that substring earlier in the schema name.
Update getEndpointName() in EndpointYamlGenerator to trim only the suffix at the
end of schemaFileName after splitting off the extension, and keep the rest of
the name unchanged.
---
Outside diff comments:
In
`@compiler-plugin-tests/src/test/java/io/ballerina/stdlib/http/compiler/ServiceArtifactsExtractionTest.java`:
- Around line 532-553: The test is checking the wrong artifact name, so it may
miss a generated consolidated endpoints file. Update
testEndpointYamlSkippedWhenNoServersDefined in ServiceArtifactsExtractionTest to
assert directly on artifactDir.resolve("endpoints.yaml") instead of filtering
Files.walk with safeFileName(p).contains("_endpoint"). This keeps the check
aligned with the actual artifact produced when no servers are defined.
- Around line 191-202: The test is asserting against a stale artifact name, so
it no longer verifies anything meaningful. Update
testConfigurablePortWithRequiredValue in ServiceArtifactsExtractionTest to use
the current consolidated artifact name endpoints.yaml, or change the assertion
to verify that no *_endpoint files are generated. Keep the check centered on the
artifactDir path and the existing executeBallerinaCommand flow.
---
Nitpick comments:
In
`@compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.java`:
- Around line 49-51: The Javadoc for EndpointYamlGenerator is attached to the
wrong member: it describes addEndpointArtifact() but currently sits above the
constructor. Move that comment to addEndpointArtifact() (or rewrite it to
describe the constructor) so the documentation matches the symbol it annotates.
In
`@compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/ServiceArtifactsExtractor.java`:
- Around line 169-185: The diagnostics parameter in exportEndpointYaml is now
unused after switching from writeEndpointYaml to addEndpointArtifact, so either
remove that dead parameter from exportEndpointYaml and its callers if
addEndpointArtifact/context.addEndpointArtifact cannot fail, or restore error
handling if artifact registration can still throw so failures continue to
surface as diagnostics. Check the control flow around exportEndpointYaml,
EndpointYamlGenerator.addEndpointArtifact, and any context.addEndpointArtifact
usage to keep the method signature and behavior consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56007526-60c4-41c3-912b-d8556c1036bf
📒 Files selected for processing (6)
compiler-plugin-tests/src/test/java/io/ballerina/stdlib/http/compiler/ServiceArtifactsExtractionTest.javacompiler-plugin/build.gradlecompiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointWrapper.javacompiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.javacompiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/ServiceArtifactsExtractor.javacompiler-plugin/src/main/java/module-info.java
💤 Files with no reviewable changes (3)
- compiler-plugin/build.gradle
- compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointWrapper.java
- compiler-plugin/src/main/java/module-info.java
Comment on lines
+102
to
+103
| private String getEndpointName() { | ||
| return schemaFileName.split("\\.")[0].replace(OPENAPI_SUFFIX, ""); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.java'
echo '--- outline ---'
ast-grep outline "$file" --view expanded || true
echo
echo '--- relevant lines ---'
cat -n "$file" | sed -n '1,220p'
echo
echo '--- OPENAPI_SUFFIX references ---'
rg -n "OPENAPI_SUFFIX|getEndpointName|schemaFileName" compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator -SRepository: ballerina-platform/module-ballerina-http
Length of output: 8067
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/FileNameGeneratorUtil.java'
echo '--- outline ---'
ast-grep outline "$file" --view expanded || true
echo
echo '--- relevant lines ---'
cat -n "$file" | sed -n '1,220p'
echo
echo '--- related constants/usages ---'
rg -n "OPENAPI_SUFFIX|YAML_EXTENSION|getFileName|FileNameGeneratorUtil" compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator -SRepository: ballerina-platform/module-ballerina-http
Length of output: 10413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- getNormalizedFileName definition ---'
rg -n "String getNormalizedFileName|getNormalizedFileName\\(" -S compiler-plugin/src/main/java
echo
echo '--- endpoint artifact consumers ---'
rg -n "EndpointArtifact\\(|getEndpointName\\(|addEndpointArtifact\\(" -S compiler-plugin/src/main/javaRepository: ballerina-platform/module-ballerina-http
Length of output: 1370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- OpenAPISpecGenerator relevant section ---'
file='compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/OpenAPISpecGenerator.java'
ast-grep outline "$file" --view expanded || true
echo
cat -n "$file" | sed -n '130,210p'
echo
echo '--- search for replace(OPENAPI_SUFFIX, "") and suffix stripping patterns ---'
rg -n 'replace\(OPENAPI_SUFFIX, ""\)|endsWith\(OPENAPI_SUFFIX\)|substring\(0, .*OPENAPI_SUFFIX\.length\(\)\)' compiler-plugin/src/main/java -SRepository: ballerina-platform/module-ballerina-http
Length of output: 6412
Strip only the trailing _openapi suffix
getEndpointName() uses replace(OPENAPI_SUFFIX, ""), so any _openapi earlier in the schema name is removed too. A service/path name containing that substring will produce the wrong endpoint name; trim only the trailing suffix instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@compiler-plugin/src/main/java/io/ballerina/stdlib/http/compiler/endpointyaml/generator/EndpointYamlGenerator.java`
around lines 102 - 103, The endpoint name extraction in getEndpointName() is
removing every occurrence of OPENAPI_SUFFIX instead of only the trailing one,
which can corrupt names containing that substring earlier in the schema name.
Update getEndpointName() in EndpointYamlGenerator to trim only the suffix at the
end of schemaFileName after splitting off the extension, and keep the rest of
the name unchanged.
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
1 participant
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.
$title.
Examples
FIxed: wso2/product-integrator#1427
Checklist
Updated the HTTP compiler plugin to export endpoint artifacts through the compiler context instead of writing individual endpoint YAML files.
EndpointYamlGenerator.ServiceArtifactsExtractorto use the new artifact flow.EndpointWrapperhelper.target/artifact/endpoints.yamloutput and support the new API.