chore: remove ledger swagger merge step#2094
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughStandardizes Swagger/OpenAPI generation and routing: pinned Changes
Review rate limit: 3/5 reviews remaining, refill in 12 minutes and 26 seconds. Comment |
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 85.4% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/http/in |
85.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/mongodb/onboarding |
66.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/mongodb/transaction |
66.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/account |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/accounttype |
66.7% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/asset |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/assetrate |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/balance |
97.5% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/ledger |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/operation |
89.9% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/operationroute |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/organization |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/portfolio |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/segment |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/transaction |
94.9% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/postgres/transactionroute |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/rabbitmq |
91.5% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/adapters/redis/transaction/balance |
100.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/services/command |
87.0% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/services/query |
93.1% |
github.com/LerianStudio/midaz/v3/components/ledger/internal/services |
0.0% |
Generated by Go PR Analysis workflow
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 87.7% ✅ PASS |
| Threshold | 85% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/midaz/v3/components/crm/internal/adapters/http/in |
86.2% |
github.com/LerianStudio/midaz/v3/components/crm/internal/adapters/mongodb/alias |
92.1% |
github.com/LerianStudio/midaz/v3/components/crm/internal/adapters/mongodb/holder |
87.1% |
github.com/LerianStudio/midaz/v3/components/crm/internal/services |
95.2% |
Generated by Go PR Analysis workflow
🔒 Security Scan Results —
|
| Policy | Status |
|---|---|
| Default non-root user | ✅ Passed |
| No fixable critical/high CVEs | ✅ Passed |
| No high-profile vulnerabilities | ✅ Passed |
| No AGPL v3 licenses | ✅ Passed |
Pre-release Version Check
✅ No unstable version pins found.
🔒 Security Scan Results —
|
| Policy | Status |
|---|---|
| Default non-root user | ✅ Passed |
| No fixable critical/high CVEs | ✅ Passed |
| No high-profile vulnerabilities | ✅ Passed |
| No AGPL v3 licenses | ✅ Passed |
Pre-release Version Check
✅ No unstable version pins found.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/crm/internal/adapters/http/in/swagger.go (1)
19-41:⚠️ Potential issue | 🟠 MajorAvoid per-request global Swagger mutation and parse
SWAGGER_SCHEMESas CSV.This handler rewrites shared
api.SwaggerInfofields on every request and treatsSWAGGER_SCHEMESas a single token. Under concurrent traffic, this risks data races; andhttp,httpscurrently becomes one invalid scheme value.🔧 Suggested fix
import ( "os" + "strings" + "sync" libCommons "github.com/LerianStudio/lib-commons/v5/commons" "github.com/gofiber/fiber/v2" + + "github.com/LerianStudio/midaz/v3/components/crm/api" ) + +var swaggerConfigOnce sync.Once // WithSwaggerEnvConfig sets the Swagger configuration for the API documentation from environment variables if they are set. func WithSwaggerEnvConfig() fiber.Handler { return func(c *fiber.Ctx) error { - envVars := map[string]*string{ - "SWAGGER_TITLE": &api.SwaggerInfo.Title, - "SWAGGER_DESCRIPTION": &api.SwaggerInfo.Description, - "SWAGGER_VERSION": &api.SwaggerInfo.Version, - "SWAGGER_HOST": &api.SwaggerInfo.Host, - "SWAGGER_BASE_PATH": &api.SwaggerInfo.BasePath, - "SWAGGER_LEFT_DELIM": &api.SwaggerInfo.LeftDelim, - "SWAGGER_RIGHT_DELIM": &api.SwaggerInfo.RightDelim, - } - - for env, field := range envVars { - if value := os.Getenv(env); !libCommons.IsNilOrEmpty(&value) { - if env == "SWAGGER_HOST" && libCommons.ValidateServerAddress(value) == "" { - continue - } - - *field = value - } - } - - if schemes := os.Getenv("SWAGGER_SCHEMES"); schemes != "" { - api.SwaggerInfo.Schemes = []string{schemes} - } + swaggerConfigOnce.Do(func() { + envVars := map[string]*string{ + "SWAGGER_TITLE": &api.SwaggerInfo.Title, + "SWAGGER_DESCRIPTION": &api.SwaggerInfo.Description, + "SWAGGER_VERSION": &api.SwaggerInfo.Version, + "SWAGGER_HOST": &api.SwaggerInfo.Host, + "SWAGGER_BASE_PATH": &api.SwaggerInfo.BasePath, + "SWAGGER_LEFT_DELIM": &api.SwaggerInfo.LeftDelim, + "SWAGGER_RIGHT_DELIM": &api.SwaggerInfo.RightDelim, + } + + for env, field := range envVars { + if value := os.Getenv(env); !libCommons.IsNilOrEmpty(&value) { + if env == "SWAGGER_HOST" && libCommons.ValidateServerAddress(value) == "" { + continue + } + *field = value + } + } + + if schemes := os.Getenv("SWAGGER_SCHEMES"); schemes != "" { + var parsed []string + for _, s := range strings.Split(schemes, ",") { + s = strings.TrimSpace(s) + if s != "" { + parsed = append(parsed, s) + } + } + if len(parsed) > 0 { + api.SwaggerInfo.Schemes = parsed + } + } + }) return c.Next() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/crm/internal/adapters/http/in/swagger.go` around lines 19 - 41, The code currently mutates the shared api.SwaggerInfo on every request (envVars map loop) and treats SWAGGER_SCHEMES as a single token; instead, stop per-request global mutation by copying api.SwaggerInfo into a local variable or constructing a request-local swaggerInfo object and update that local copy (use the same field pointers in envVars but assign to the local copy), only using libCommons.ValidateServerAddress when validating SWAGGER_HOST; additionally parse SWAGGER_SCHEMES as CSV (split on commas, trim whitespace) into []string and assign that to the local swagger copy; ensure the global api.SwaggerInfo is not modified concurrently (initialize global once at startup if needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/crm/internal/adapters/http/in/routes_test.go`:
- Around line 185-218: The test TestNewRouter_ServesSwaggerUIAssets covers
static files but misses the redirect contract for the swagger root; add an
assertion that a GET to "/swagger" returns a 3xx redirect and that the Location
header equals "/swagger/index.html". Concretely, in
TestNewRouter_ServesSwaggerUIAssets (or a new t.Run within it) create req :=
httptest.NewRequest(http.MethodGet, "/swagger", nil), call resp, err :=
app.Test(req, -1), require.NoError(t, err), close resp.Body, then assert that
resp.StatusCode is in the 3xx range (e.g., http.StatusMovedPermanently or
http.StatusFound) and assert.Equal(t, "/swagger/index.html",
resp.Header.Get("Location")) so the redirect behavior is explicitly covered.
In `@components/ledger/api/openapi.yaml`:
- Line 17: The servers.url entry is currently protocol-relative
("//localhost:3002/") which can cause ambiguous behavior in Swagger UI; update
the servers.url value to include an explicit scheme (e.g., change
"//localhost:3002/" to "http://localhost:3002/") so Try-it-out uses a
deterministic protocol; locate the servers -> url key in the OpenAPI document
and replace the protocol-relative URL with the explicit URL string.
In `@components/ledger/internal/adapters/http/in/routes_test.go`:
- Around line 73-92: Update the router test to include the bare "/swagger" path
and assert it returns a 301 redirect: add a test case (either in the existing
loop or as a separate t.Run) that creates req :=
httptest.NewRequest(fiber.MethodGet, "/swagger", nil), calls resp, err :=
app.Test(req), require.NoError(t, err), defers resp.Body.Close(), and then
asserts assert.Equal(t, fiber.StatusMovedPermanently, resp.StatusCode);
reference the existing test variables (path loop, req/resp, app.Test) when
adding this expectation.
In `@components/ledger/internal/bootstrap/service_test.go`:
- Around line 273-295: Add a new test case that sends an HTTP GET to "/swagger"
(using the same server.app.Test flow and req.Host="localhost") and assert the
response is a redirect to "/swagger/index.html": check the response StatusCode
is a redirect (301 or 302) and that the "Location" header equals
"/swagger/index.html". Add this as a separate t.Run (similar style to the
existing loop) near the other swagger asset checks so regressions of the
"/swagger" redirect contract are caught; reference the existing test harness
variables (server, t.Run, req, res) when implementing.
In `@components/ledger/Makefile`:
- Around line 245-255: Always install and invoke the pinned swag binary instead
of using PATH: remove the initial `SWAG_BIN=$$(command -v swag ...)` check and
instead determine GO_BIN via `go env GOBIN` (fall back to `$(go env
GOPATH)/bin`), run `go install github.com/swaggo/swag/cmd/swag@v1.16.6` to
ensure the pinned version is installed into that GO_BIN, set
`SWAG_BIN="$$GO_BIN/swag"`, and then call `$$SWAG_BIN init -g cmd/app/main.go -o
api --parseDependency --parseInternal --outputTypes go,json,yaml` so `SWAG_BIN`
always points to the deterministic v1.16.6 binary.
In `@scripts/postman-coll-generation/sync-postman.sh`:
- Around line 176-179: The current conditional in the sync-postman.sh script
only aborts when both conversions fail; change the gate so the script fails if
either required conversion fails by updating the conditional that checks
LEDGER_STATUS and CRM_STATUS (replace the logical AND with OR) so that if
LEDGER_STATUS != "SUCCESS" || CRM_STATUS != "SUCCESS" the script echoes the
error, cleans up TEMP_DIR and exits 1; ensure you reference the existing
variables LEDGER_STATUS, CRM_STATUS, TEMP_DIR and the output file name
MIDAZ.postman_collection.json when making the change so the script will not
overwrite the collection when one component failed.
---
Outside diff comments:
In `@components/crm/internal/adapters/http/in/swagger.go`:
- Around line 19-41: The code currently mutates the shared api.SwaggerInfo on
every request (envVars map loop) and treats SWAGGER_SCHEMES as a single token;
instead, stop per-request global mutation by copying api.SwaggerInfo into a
local variable or constructing a request-local swaggerInfo object and update
that local copy (use the same field pointers in envVars but assign to the local
copy), only using libCommons.ValidateServerAddress when validating SWAGGER_HOST;
additionally parse SWAGGER_SCHEMES as CSV (split on commas, trim whitespace)
into []string and assign that to the local swagger copy; ensure the global
api.SwaggerInfo is not modified concurrently (initialize global once at startup
if needed).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 54421453-952c-447b-bb22-61132f5e8094
📒 Files selected for processing (41)
components/crm/Makefilecomponents/crm/api/docs.gocomponents/crm/api/swagger.jsoncomponents/crm/api/swagger.yamlcomponents/crm/cmd/app/main.gocomponents/crm/internal/adapters/http/in/routes.gocomponents/crm/internal/adapters/http/in/routes_test.gocomponents/crm/internal/adapters/http/in/swagger.gocomponents/ledger/.env.examplecomponents/ledger/Makefilecomponents/ledger/api/docs.gocomponents/ledger/api/openapi.yamlcomponents/ledger/api/swagger.jsoncomponents/ledger/api/swagger.yamlcomponents/ledger/cmd/app/main.gocomponents/ledger/internal/adapters/http/in/account.gocomponents/ledger/internal/adapters/http/in/accounttype.gocomponents/ledger/internal/adapters/http/in/asset.gocomponents/ledger/internal/adapters/http/in/assetrate.gocomponents/ledger/internal/adapters/http/in/balance.gocomponents/ledger/internal/adapters/http/in/count_transactions_by_filters.gocomponents/ledger/internal/adapters/http/in/ledger.gocomponents/ledger/internal/adapters/http/in/metadata.gocomponents/ledger/internal/adapters/http/in/operation.gocomponents/ledger/internal/adapters/http/in/operation_route.gocomponents/ledger/internal/adapters/http/in/organization.gocomponents/ledger/internal/adapters/http/in/portfolio.gocomponents/ledger/internal/adapters/http/in/routes.gocomponents/ledger/internal/adapters/http/in/routes_test.gocomponents/ledger/internal/adapters/http/in/segment.gocomponents/ledger/internal/adapters/http/in/transaction.gocomponents/ledger/internal/adapters/http/in/transaction_query_handlers.gocomponents/ledger/internal/adapters/http/in/transaction_route.gocomponents/ledger/internal/adapters/http/in/transaction_state_handlers.gocomponents/ledger/internal/bootstrap/service_test.gocomponents/ledger/internal/bootstrap/unified-server.gocomponents/ledger/scripts/merge-swagger.shpostman/MIDAZ.postman_collection.jsonpostman/MIDAZ.postman_environment.jsonscripts/generate-docs.shscripts/postman-coll-generation/sync-postman.sh
💤 Files with no reviewable changes (2)
- components/crm/cmd/app/main.go
- components/ledger/scripts/merge-swagger.sh
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ledger/api/openapi.yaml`:
- Around line 11145-11151: The OpenAPI component BearerAuth under
securitySchemes is incorrect and unused: update your API generation source
(Swaggo annotations or the generation pipeline that produces
x-original-swagger-version: "2.0") to either (A) emit a proper OpenAPI 3 bearer
scheme (securitySchemes.BearerAuth -> type: http, scheme: bearer) and add a
matching security requirement at the root or operation level referencing
BearerAuth, or (B) remove the unused BearerAuth component entirely from the
generator if no authentication is required; locate changes in the generator
annotations that define BearerAuth and the global/operation-level security
declarations to apply the fix.
In `@scripts/postman-coll-generation/sync-postman.sh`:
- Around line 174-175: Update the stale guard comment above the conditional in
sync-postman.sh to describe the actual behavior: change the text that currently
says “at least one component succeeded” to indicate that the script requires
both components to be SUCCESS (i.e., both conversion and schema steps must be
SUCCESS) so the comment matches the conditional that checks both statuses.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 28e77638-9245-4acc-aa67-a6c4ec5f4e10
📒 Files selected for processing (12)
components/crm/api/docs.gocomponents/crm/api/openapi.yamlcomponents/crm/api/swagger.jsoncomponents/crm/api/swagger.yamlcomponents/crm/cmd/app/main.gocomponents/ledger/api/docs.gocomponents/ledger/api/openapi.yamlcomponents/ledger/api/swagger.jsoncomponents/ledger/api/swagger.yamlcomponents/ledger/cmd/app/main.gopostman/MIDAZ.postman_collection.jsonscripts/postman-coll-generation/sync-postman.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/postman-coll-generation/sync-postman.sh`:
- Around line 95-100: The script currently uses wait $LEDGER_PID $CRM_PID while
set -e is active, which can abort the script if either child exits non-zero;
modify the code around the wait call so you temporarily disable errexit (use set
+e), run wait $LEDGER_PID $CRM_PID, capture/ignore its exit for later
inspection, then re-enable errexit (set -e) before proceeding to read the status
files (TEMP_DIR/ledger.status and TEMP_DIR/crm.status) and the existing cleanup
logic; reference the wait invocation and the LEDGER_PID, CRM_PID and TEMP_DIR
variables when making the change.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 1816cdf3-ef65-492f-bbd8-47988027aa71
📒 Files selected for processing (1)
scripts/postman-coll-generation/sync-postman.sh
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
LGTM. Clean simplification — merge-swagger removal, unified swag init with pinned version, standardized Swagger UI routing with redirect + named instance, and Auth header correctly marked as optional.
Post-push fixes also solid: && → || in sync-postman, set +e around wait, stale comment update, and explicit http:// scheme in servers.url.
One follow-up tracked separately: BearerAuth scheme should use type: http, scheme: bearer (OpenAPI 3 standard) instead of type: apiKey, and needs a security reference to actually surface in Swagger UI. Since the YAML is generated, fix belongs in Swaggo annotations or a post-gen patch in generate-docs.sh.
All CI checks passing. Ship it. 🚀
Summary
swag initand remove the obsoletemerge-swagger.shflow.docs.go,swagger.json, andswagger.yaml.plugin-authis enabled./swaggerredirects and asset route coverage.Validation
go test ./components/ledger/internal/bootstrap ./components/ledger/internal/adapters/http/in ./components/ledger/cmd/appgo test ./components/crm/cmd/app ./components/crm/internal/adapters/http/inbash -n scripts/generate-docs.sh && bash -n scripts/postman-coll-generation/sync-postman.shmake -n generate-docsincomponents/ledgermake -n generate-docsincomponents/crm