[Platform-API] Store the Gateway Version #1283
[Platform-API] Store the Gateway Version #1283VirajSalaka wants to merge 4 commits intowso2:mainfrom
Conversation
WalkthroughAdds gateway versioning: new Changes
Sequence DiagramsequenceDiagram
actor Client
participant Handler as CreateGateway<br/>Handler
participant Service as GatewayService
participant Repo as GatewayRepository
participant DB as Database
Client->>Handler: POST /gateways (body includes version)
Handler->>Service: RegisterGateway(..., version)
Service->>Service: validateGatewayInput(version)
alt invalid version
Service-->>Handler: validation error
Handler-->>Client: 400 Bad Request
else valid version
Service->>Repo: Create(gateway with version)
Repo->>DB: INSERT INTO gateways (version, ...)
DB-->>Repo: OK
Repo-->>Service: gateway model (with version)
Service->>Handler: GatewayResponse (includes version)
Handler-->>Client: 201 Created (response)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
60bd500 to
f485a00
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
platform-api/src/internal/database/schema.sqlite.sql (1)
81-81: Constrainversionto non-empty at the database layer.
NOT NULLstill allows empty strings. Adding a CHECK avoids persisting invalid empty versions through non-service paths.💡 Suggested schema hardening
- version TEXT NOT NULL default '0.9.0', + version TEXT NOT NULL DEFAULT '0.9.0' CHECK (length(trim(version)) > 0),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-api/src/internal/database/schema.sqlite.sql` at line 81, The schema allows an empty string in the version column despite NOT NULL; update the column definition for the version field to enforce non-empty values at the DB level by adding a CHECK constraint (e.g., CHECK(trim(version) <> '') or CHECK(length(version) > 0) ) so the version TEXT NOT NULL default '0.9.0' definition prevents empty strings; update any migration or create-table statement that defines the version column to include this CHECK constraint.platform-api/src/internal/service/gateway.go (1)
61-64: Apply create-time defaulting in the service before validation.Line 63 validates
versionbefore Line 90 normalizes it. This makes the create flow depend on caller-side default injection. Moving defaulting into the service keeps business behavior consistent across call paths.🔧 Suggested service hardening
func (s *GatewayService) RegisterGateway(orgID, name, displayName, description, vhost string, isCritical bool, functionalityType string, properties map[string]interface{}, version string) (*api.GatewayResponse, error) { + gatewayVersion := strings.TrimSpace(version) + if gatewayVersion == "" { + gatewayVersion = "0.9.0" + } + // 1. Validate inputs - if err := s.validateGatewayInput(orgID, name, displayName, vhost, functionalityType, version); err != nil { + if err := s.validateGatewayInput(orgID, name, displayName, vhost, functionalityType, gatewayVersion); err != nil { return nil, err } @@ - gatewayVersion := strings.TrimSpace(version) - // 5. Create Gateway modelAlso applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-api/src/internal/service/gateway.go` around lines 61 - 64, The service validates the incoming version before applying create-time defaulting/normalization, so move the defaulting logic into the service: in the gateway creation method (the function with signature returning (*api.GatewayResponse, error)), apply the version default/normalization (the existing normalization logic currently located after validation at line ~90) to the version parameter before calling s.validateGatewayInput(orgID, name, displayName, vhost, functionalityType, version); then pass the normalized version into validateGatewayInput and the rest of the flow (ensure you update any local variable that holds version accordingly and keep the existing normalize/default behavior intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platform-api/src/internal/database/schema.postgres.sql`:
- Line 81: Add a migration that alters the existing gateways table to include
the new non-nullable version column with the same default used in the CREATE
TABLE (i.e., add a migration that runs ALTER TABLE gateways ADD COLUMN version
TEXT NOT NULL DEFAULT '0.9.0';), ensure the migration is idempotent or checks
for the column before adding, and wire it into the project's migration sequence
so existing deployments receive the column prior to any code that reads/writes
gateways.version.
In `@platform-api/src/internal/handler/gateway.go`:
- Line 240: The current handler treats all errors from
h.gatewayService.UpdateGateway as 500; change the error handling in the gateway
update handler (the call to h.gatewayService.UpdateGateway(gatewayId, orgId,
req.Description, req.DisplayName, req.IsCritical, req.Properties, req.Version))
to detect validation/client errors returned by the service and respond with HTTP
400 (Bad Request) instead of 500, while other errors remain 500. Also ensure the
error is logged at Error level via the request handler's slog logger (e.g.,
h.logger.Error or equivalent) including the error value and context (gatewayId,
orgId, version) for both 4xx and 5xx cases.
In `@platform-api/src/internal/service/gateway.go`:
- Around line 203-208: The current update silently ignores an explicitly
provided whitespace-only version; instead validate and reject it: in the update
handler where you check version (the block that trims and sets gateway.Version),
after trimming (strings.TrimSpace(*version)) if the trimmed result is empty
return a validation error indicating "version cannot be blank" (do not set
gateway.Version), otherwise set gateway.Version = trimmedVersion; ensure the
handler returns the appropriate HTTP 4xx validation response so callers see the
failure.
In `@platform-api/src/resources/openapi.yaml`:
- Around line 3532-3535: The version property currently allows any string
(including whitespace); update the OpenAPI schemas for CreateGatewayRequest and
UpdateGatewayRequest to enforce stricter validation on the version field by
adding a minLength: 1 and a pattern that matches the server's expected version
format (e.g., a semantic version regex such as one that requires
major.minor.patch like ^\d+\.\d+\.\d+(-[A-Za-z0-9.+-]+)?$). Apply these
constraints to the "version" property in both CreateGatewayRequest and
UpdateGatewayRequest so clients cannot submit empty/whitespace-only or
non-semver values.
---
Nitpick comments:
In `@platform-api/src/internal/database/schema.sqlite.sql`:
- Line 81: The schema allows an empty string in the version column despite NOT
NULL; update the column definition for the version field to enforce non-empty
values at the DB level by adding a CHECK constraint (e.g., CHECK(trim(version)
<> '') or CHECK(length(version) > 0) ) so the version TEXT NOT NULL default
'0.9.0' definition prevents empty strings; update any migration or create-table
statement that defines the version column to include this CHECK constraint.
In `@platform-api/src/internal/service/gateway.go`:
- Around line 61-64: The service validates the incoming version before applying
create-time defaulting/normalization, so move the defaulting logic into the
service: in the gateway creation method (the function with signature returning
(*api.GatewayResponse, error)), apply the version default/normalization (the
existing normalization logic currently located after validation at line ~90) to
the version parameter before calling s.validateGatewayInput(orgID, name,
displayName, vhost, functionalityType, version); then pass the normalized
version into validateGatewayInput and the rest of the flow (ensure you update
any local variable that holds version accordingly and keep the existing
normalize/default behavior intact).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
platform-api/src/api/generated.goplatform-api/src/internal/database/schema.postgres.sqlplatform-api/src/internal/database/schema.sqlite.sqlplatform-api/src/internal/handler/gateway.goplatform-api/src/internal/model/gateway.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/api_deployments_test.goplatform-api/src/internal/repository/gateway.goplatform-api/src/internal/service/api.goplatform-api/src/internal/service/gateway.goplatform-api/src/internal/service/gateway_properties_test.goplatform-api/src/resources/openapi.yaml
| is_critical BOOLEAN DEFAULT FALSE, | ||
| gateway_functionality_type VARCHAR(20) DEFAULT 'regular' NOT NULL, | ||
| is_active BOOLEAN DEFAULT FALSE, | ||
| version TEXT NOT NULL default '0.9.0', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) List SQL files under database area.
echo "=== SQL files in database area ==="
fd -t f '\.sql$' platform-api/src/internal/database 2>/dev/null || echo "Directory or files not found"
# 2) Find DDL touching gateways.version (creation + alteration).
echo -e "\n=== DDL statements touching gateways.version ==="
rg -n --type-add 'sql:*.sql' --type sql \
'CREATE TABLE IF NOT EXISTS gateways|ALTER TABLE\s+gateways|ADD COLUMN\s+version' \
platform-api/src/internal/database 2>/dev/null || echo "No matches found"
# 3) Surface possible migration directories across repo.
echo -e "\n=== SQL files with migration/migrate/schema patterns ==="
fd -t f '\.sql$' 2>/dev/null | rg -n 'migration|migrate|schema' || echo "No migration patterns found"
# 4) Additional search: look for any migration-related structures
echo -e "\n=== Migration directories structure ==="
fd -t d 'migration|migrate' 2>/dev/null || echo "No migration directories found"Repository: wso2/api-platform
Length of output: 895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check init-platform-api-db.sql
echo "=== init-platform-api-db.sql content ==="
cat -n platform-api/src/internal/database/init-platform-api-db.sql
echo -e "\n=== schema.postgres.sql - gateways table section (lines 70-85) ==="
sed -n '70,85p' platform-api/src/internal/database/schema.postgres.sql | cat -n
# Search for any ALTER TABLE in all SQL files
echo -e "\n=== Check for ALTER TABLE across all SQL files ==="
rg -n 'ALTER TABLE' platform-api/src/internal/database/ || echo "No ALTER TABLE found"
# Check if there's any conditional table creation/upgrade logic
echo -e "\n=== Search for upgrade/migration logic ==="
rg -n -i 'upgrade|migration|version.*column|alter.*gateways' platform-api/src/internal/database/ || echo "No upgrade logic found"Repository: wso2/api-platform
Length of output: 2013
Add an ALTER TABLE migration for the version column in gateways table.
The schema file adds version TEXT NOT NULL default '0.9.0' to the gateways table definition using CREATE TABLE IF NOT EXISTS. This only affects fresh database installations; existing deployments retain their gateways table without this column. No migration script exists in the repository to add this column to pre-existing tables.
When the application attempts to INSERT, UPDATE, or SELECT the version column, existing deployments will encounter runtime SQL errors. An explicit ALTER TABLE gateways ADD COLUMN version TEXT NOT NULL DEFAULT '0.9.0'; migration is required for seamless upgrades.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platform-api/src/internal/database/schema.postgres.sql` at line 81, Add a
migration that alters the existing gateways table to include the new
non-nullable version column with the same default used in the CREATE TABLE
(i.e., add a migration that runs ALTER TABLE gateways ADD COLUMN version TEXT
NOT NULL DEFAULT '0.9.0';), ensure the migration is idempotent or checks for the
column before adding, and wire it into the project's migration sequence so
existing deployments receive the column prior to any code that reads/writes
gateways.version.
There was a problem hiding this comment.
Will handle when migrating the actual database explicitly.
There was a problem hiding this comment.
@VirajSalaka, understood—I've noted that you'll handle the migration explicitly during database deployment.
For reference, if you need idempotent migration logic in the future, PostgreSQL's ALTER TABLE ... SET NOT NULL "silently does nothing if the column already has a not-null constraint", and you can check information_schema.columns before adding columns to make migrations safe to run multiple times. But for this PR, manual handling during deployment works as well.
🧠 Learnings used
Learnt from: Arshardh
Repo: wso2/api-platform PR: 841
File: platform-api/src/internal/database/schema.postgres.sql:345-423
Timestamp: 2026-02-01T19:10:45.800Z
Learning: In SQL schema files under the platform-api module, avoid relying on database-level foreign key cascades to handle orphaned LLM policies. Ensure llm_policies references only organization_uuid and do not configure ON DELETE CASCADE for related tables (e.g., llm_providers, llm_proxies). Orphan handling should be implemented at the application layer for this domain to preserve schema flexibility.
Learnt from: RakhithaRR
Repo: wso2/api-platform PR: 989
File: platform-api/src/internal/database/schema.postgres.sql:40-53
Timestamp: 2026-02-09T05:23:35.668Z
Learning: In platform-api/src/internal/database/schema.postgres.sql, ensure the artifacts table enforces a global unique constraint on (name, version, organization_uuid) across all artifact kinds (e.g., RestApi, LLMProvider, LLMProxy). This prevents different artifact kinds within the same organization from sharing the same name+version. If this constraint is not present, add a unique constraint like UNIQUE(name, version, organization_uuid) or a composite unique index, and ensure existing data respects it before applying to maintain data integrity.
| } | ||
|
|
||
| response, err := h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description, req.DisplayName, req.IsCritical, req.Properties) | ||
| response, err := h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description, req.DisplayName, req.IsCritical, req.Properties, req.Version) |
There was a problem hiding this comment.
Map update validation errors to 400 instead of 500.
UpdateGateway now accepts version; validation failures from the service should return Bad Request, not Internal Server Error.
✅ Suggested fix
response, err := h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description, req.DisplayName, req.IsCritical, req.Properties, req.Version)
if err != nil {
if errors.Is(err, constants.ErrGatewayNotFound) {
h.slogger.Error("Gateway not found during update", "error", err)
c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found",
"Gateway not found"))
return
}
+ errMsg := err.Error()
+ if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") ||
+ strings.Contains(errMsg, "must") || strings.Contains(errMsg, "cannot") {
+ h.slogger.Error("Invalid gateway update request", "error", err)
+ c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", errMsg))
+ return
+ }
h.slogger.Error("Failed to update gateway", "error", err)
c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error",
"Failed to update gateway"))
return
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platform-api/src/internal/handler/gateway.go` at line 240, The current
handler treats all errors from h.gatewayService.UpdateGateway as 500; change the
error handling in the gateway update handler (the call to
h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description,
req.DisplayName, req.IsCritical, req.Properties, req.Version)) to detect
validation/client errors returned by the service and respond with HTTP 400 (Bad
Request) instead of 500, while other errors remain 500. Also ensure the error is
logged at Error level via the request handler's slog logger (e.g.,
h.logger.Error or equivalent) including the error value and context (gatewayId,
orgId, version) for both 4xx and 5xx cases.
| if version != nil { | ||
| trimmedVersion := strings.TrimSpace(*version) | ||
| if trimmedVersion != "" { | ||
| gateway.Version = trimmedVersion | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject blank version updates instead of silently no-oping.
If clients send version explicitly but as whitespace, the API returns success without applying that field. Returning a validation error is clearer and avoids misleading successful updates.
✅ Suggested validation behavior
if version != nil {
trimmedVersion := strings.TrimSpace(*version)
- if trimmedVersion != "" {
- gateway.Version = trimmedVersion
- }
+ if trimmedVersion == "" {
+ return nil, errors.New("version cannot be empty")
+ }
+ gateway.Version = trimmedVersion
}📝 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.
| if version != nil { | |
| trimmedVersion := strings.TrimSpace(*version) | |
| if trimmedVersion != "" { | |
| gateway.Version = trimmedVersion | |
| } | |
| } | |
| if version != nil { | |
| trimmedVersion := strings.TrimSpace(*version) | |
| if trimmedVersion == "" { | |
| return nil, errors.New("version cannot be empty") | |
| } | |
| gateway.Version = trimmedVersion | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platform-api/src/internal/service/gateway.go` around lines 203 - 208, The
current update silently ignores an explicitly provided whitespace-only version;
instead validate and reject it: in the update handler where you check version
(the block that trims and sets gateway.Version), after trimming
(strings.TrimSpace(*version)) if the trimmed result is empty return a validation
error indicating "version cannot be blank" (do not set gateway.Version),
otherwise set gateway.Version = trimmedVersion; ensure the handler returns the
appropriate HTTP 4xx validation response so callers see the failure.
| version: | ||
| type: string | ||
| description: Gateway runtime version | ||
| example: "0.9.0" |
There was a problem hiding this comment.
Constrain version in request schemas to match server-side expectations.
Line 3532 and Line 3626 currently allow any string, including whitespace-only values. Tightening this in OpenAPI reduces client/server contract drift and prevents avoidable 400s.
📌 Suggested OpenAPI adjustment
version:
type: string
+ minLength: 1
+ pattern: '.*\S.*'
description: Gateway runtime versionApply the same constraints in both CreateGatewayRequest and UpdateGatewayRequest.
Also applies to: 3626-3629
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platform-api/src/resources/openapi.yaml` around lines 3532 - 3535, The
version property currently allows any string (including whitespace); update the
OpenAPI schemas for CreateGatewayRequest and UpdateGatewayRequest to enforce
stricter validation on the version field by adding a minLength: 1 and a pattern
that matches the server's expected version format (e.g., a semantic version
regex such as one that requires major.minor.patch like
^\d+\.\d+\.\d+(-[A-Za-z0-9.+-]+)?$). Apply these constraints to the "version"
property in both CreateGatewayRequest and UpdateGatewayRequest so clients cannot
submit empty/whitespace-only or non-semver values.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platform-api/src/internal/service/gateway_test.go (1)
182-192: Add a whitespace-only version test case.You validate empty version, but not
" ". Adding this case will prevent trim-related regressions in input handling.Proposed test case addition
{ name: "empty version", orgID: "123e4567-e89b-12d3-a456-426614174000", gatewayName: "prod-gateway-01", displayName: "Production Gateway 01", vhost: "api.example.com", functionalityType: constants.GatewayFunctionalityTypeRegular, version: "", wantErr: true, errContains: "version is required", }, + { + name: "whitespace-only version", + orgID: "123e4567-e89b-12d3-a456-426614174000", + gatewayName: "prod-gateway-01", + displayName: "Production Gateway 01", + vhost: "api.example.com", + functionalityType: constants.GatewayFunctionalityTypeRegular, + version: " ", + wantErr: true, + errContains: "version is required", + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-api/src/internal/service/gateway_test.go` around lines 182 - 192, Add a test case that covers a whitespace-only version to prevent trim-related regressions: in the table-driven tests (the struct with fields like name, orgID, gatewayName, displayName, vhost, functionalityType, version, wantErr, errContains) add a new case (e.g., name: "whitespace-only version") where version is set to " " (spaces) and wantErr is true with errContains "version is required" so the same validation that rejects empty string also rejects whitespace-only input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platform-api/src/internal/service/gateway_test.go`:
- Around line 182-192: Add a test case that covers a whitespace-only version to
prevent trim-related regressions: in the table-driven tests (the struct with
fields like name, orgID, gatewayName, displayName, vhost, functionalityType,
version, wantErr, errContains) add a new case (e.g., name: "whitespace-only
version") where version is set to " " (spaces) and wantErr is true with
errContains "version is required" so the same validation that rejects empty
string also rejects whitespace-only input.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
platform-api/src/internal/handler/gateway.go (1)
248-254:⚠️ Potential issue | 🟡 MinorInconsistent validation-error keywords between
CreateGatewayandUpdateGateway.
CreateGateway(line 101-102) also matches"cannot"in its string-based validation check, but this block omits it. A service-layer error containing only"cannot"would return 400 on create but 500 on update.Proposed fix
- if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "must") { + if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "must") || strings.Contains(errMsg, "cannot") {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-api/src/internal/handler/gateway.go` around lines 248 - 254, The UpdateGateway validation check is missing the same "cannot" keyword used in CreateGateway, causing inconsistent 400 vs 500 responses; update the error-string inspection in the UpdateGateway handler (the block that inspects err.Error() and calls h.slogger.Error / c.JSON) to include "cannot" alongside "required", "invalid", and "must" so validation errors containing "cannot" return HTTP 400 just like in CreateGateway.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@platform-api/src/internal/handler/gateway.go`:
- Around line 248-254: The UpdateGateway validation check is missing the same
"cannot" keyword used in CreateGateway, causing inconsistent 400 vs 500
responses; update the error-string inspection in the UpdateGateway handler (the
block that inspects err.Error() and calls h.slogger.Error / c.JSON) to include
"cannot" alongside "required", "invalid", and "must" so validation errors
containing "cannot" return HTTP 400 just like in CreateGateway.
| } | ||
| // Similar to how it is handled in CreateGateway, check for validation errors and return 400 Bad Request | ||
| errMsg := err.Error() | ||
| if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "must") { |
There was a problem hiding this comment.
String match is fragile, what if we check error is
var ErrValidation = errors.New("validation error")
// In validation logic:
return fmt.Errorf("%w: name is required", ErrValidation)
// In handler:
if errors.Is(err, ErrValidation) {
c.JSON(http.StatusBadRequest, ...)
return
}
This pull request introduces support for a
versionfield in the gateway model, API, and database schema. The changes ensure that gateway runtime version information is stored, retrieved, and updated throughout the platform, including API requests, responses, database operations, and test cases.Gateway Version Support
versionfield to the gateway database schema in both Postgres and SQLite, making it a required column for all gateway records. [1] [2]CreateGatewayRequest,UpdateGatewayRequest,GatewayResponse,RESTAPIGatewayResponse) to include theversionfield, allowing clients to specify and receive gateway version information. [1] [2] [3] [4]Gateway,APIGatewayWithDetails) and repository methods to handle theversionfield, including all relevant SQL queries and struct mappings. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]RegisterGateway,UpdateGateway) and handler logic to support creating and updating gateways with a specified version, including validation and defaulting logic. [1] [2] [3] [4] [5] [6]0.9.0) for newly created gateways when not specified, and updated test cases to include the version field. [1] [2]API and Handler Enhancements
Goals
Approach
User stories
Documentation
Automation tests
Security checks
Samples
Related PRs
Test environment
Summary by CodeRabbit
New Features
Bug Fixes