Support user provided identifier for APIs #408
Conversation
WalkthroughAPI identification moved from UUIDs to user-facing handles across model, DB schema, repository, service, handler, utils, and OpenAPI; services continue to use internal UUIDs for relational operations while handlers and DTOs surface handles. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
platform-api/src/internal/handler/api.go (1)
374-431: Fix misleading log field name (apiUUID) now that param is a handle
The error log usesapiUUID=%sbutapiIdis a handle now.- log.Printf("[ERROR] Failed to deploy API revision: apiUUID=%s revisionID=%s error=%v", + log.Printf("[ERROR] Failed to deploy API revision: apiHandle=%s revisionID=%s error=%v", apiId, revisionID, err)platform-api/src/resources/openapi.yaml (2)
1645-1660: OpenAPI: add handle constraints (pattern + length) and quote examples
Right nowidis described as a handle, but clients don’t get the validation rules (and examples are unquoted).API: ... properties: id: type: string description: Unique handle/identifier for the API. Can be provided during creation or auto-generated. - example: my-api-handle + pattern: '^[a-z0-9]+(-[a-z0-9]+)*$' + minLength: 3 + maxLength: 63 + example: "my-api-handle"
3630-3639: OpenAPI: update remaining “UUID” wording/error examples forapiId
The parameter now correctly says “handle”, but multiple endpoint descriptions and examples still mention UUID (e.g., “API ID must be a valid UUID”). Those should be updated to “handle” to avoid breaking clients and SDKs.platform-api/src/internal/database/schema.sql (1)
40-68: Align DB handle constraint with validation and reconsider context uniqueness scopeThe
handlecolumn allows VARCHAR(255) but the validation code enforces a maximum of 63 characters (seeplatform-api/src/internal/utils/handle.go). Align the DB schema to match: useVARCHAR(63).Additionally,
UNIQUE(context, organization_uuid)prevents having multiple API versions with the same context within an organization. However, the indexidx_apis_name_context_versionand the documented API identity pattern suggest versioning under a shared context is intended (similar to/petstore/v1,/petstore/v2). This constraint may be too restrictive. Consider whether the uniqueness should be scoped asUNIQUE(context, version, organization_uuid)orUNIQUE(name, context, version, organization_uuid)instead.Since this schema appears to support fresh installations only (no migration files or ALTER TABLE logic found), ensure any constraint changes are in place before the first production deployment.
platform-api/src/internal/repository/api.go (1)
307-336:GetAPIsByGatewayIDdoes not populate thehandlefield.Unlike other retrieval methods (
GetAPIByUUID,GetAPIsByProjectID,GetAPIsByOrganizationID), this method's SELECT query omits thehandlecolumn (line 310). APIs returned by this method will have an emptyHandlefield.query := ` - SELECT a.uuid, a.name, a.display_name, a.type, a.created_at, a.updated_at + SELECT a.uuid, a.handle, a.name, a.display_name, a.type, a.created_at, a.updated_at FROM apis a INNER JOIN api_deployments ad ON a.uuid = ad.api_uuid WHERE ad.gateway_uuid = ? AND a.organization_uuid = ? ORDER BY a.created_at DESC `And update the Scan accordingly:
err := rows.Scan( - &api.ID, &api.Name, &api.DisplayName, &api.Type, &api.CreatedAt, &api.UpdatedAt, + &api.ID, &api.Handle, &api.Name, &api.DisplayName, &api.Type, &api.CreatedAt, &api.UpdatedAt, )
🧹 Nitpick comments (8)
platform-api/src/internal/model/api.go (1)
24-52: Avoid accidentally exposing internal UUIDs and ensureHandleis always set
model.APInow has bothjson:"id"(UUID) andjson:"handle". If this type is ever marshaled directly, clients may see the UUID even though the API contract treats the identifier as the handle. Also, DBhandleisNOT NULL, so empty string will fail inserts unless always populated.type API struct { - ID string `json:"id" db:"uuid"` + // Internal UUID (should not be exposed via REST responses) + ID string `json:"-" db:"uuid"` Handle string `json:"handle" db:"handle"` Name string `json:"name" db:"name"` ... }If you still need
idin some internal JSON flows, consider a dedicated internal DTO instead of reusingmodel.API.platform-api/src/internal/repository/interfaces.go (1)
46-73: Make handle-based repository method signatures consistent (naming + arg order)
Purely ergonomics, but having consistent(orgID, handle)ordering andorgIDnaming reduces call-site mistakes across service/handler layers.platform-api/src/internal/handler/api.go (1)
133-162: Good switch to handle-based lookup; rename variables/messages to “handle” for clarity
Now thatapiIdpath param is a handle, consider updating user-facing error text and internal variable naming to avoid UUID confusion.platform-api/src/internal/constants/error.go (1)
22-34: Align API contract with handle validation errors (3–63 + regex)
These new errors are useful, but they should be reflected in the OpenAPI schema (pattern + min/max) so clients can pre-validate.platform-api/src/internal/utils/handle.go (1)
156-159: Suffix generation: consider using a hyphen-free source (and/or longer suffix)
Currentuuid.New().String()[:4]is probably fine (hex), but it’s a bit non-obvious and has a small space (65,536). Consider using a hyphen-free UUID string (or a 6–8 char suffix) to reduce collision risk.platform-api/src/internal/repository/api.go (1)
409-478: Consider extracting shared update logic to reduce duplication.
UpdateAPIByHandleduplicates ~60 lines of configuration handling logic fromUpdateAPI. This increases maintenance burden and risk of divergence.Extract the configuration update logic into a helper method:
func (r *APIRepo) updateAPIConfigurations(tx *sql.Tx, api *model.API) error { if err := r.deleteAPIConfigurations(tx, api.ID); err != nil { return err } if api.MTLS != nil { if err := r.insertMTLSConfig(tx, api.ID, api.MTLS); err != nil { return err } } // ... rest of configuration inserts for _, operation := range api.Operations { if err := r.insertOperation(tx, api.ID, api.OrganizationID, &operation); err != nil { return err } } return nil }platform-api/src/internal/service/api.go (2)
266-277: Consider logging errors inHandleExistsCheckfor observability.When the database check fails (line 272), the error is silently swallowed. While returning
trueis a safe fallback, logging the error would help with debugging handle generation failures.func (s *APIService) HandleExistsCheck(orgId string) func(string) bool { return func(handle string) bool { exists, err := s.apiRepo.CheckAPIExistsByHandleInOrganization(handle, orgId) if err != nil { - // On error, assume it exists to be safe (will trigger retry) + // On error, assume it exists to be safe (will trigger retry) + log.Printf("[WARN] Failed to check handle existence for '%s': %v", handle, err) return true } return exists } }
398-419: Minor: Redundant API lookup inDeleteAPIByHandle.The service method fetches the API (line 405), then calls
repo.DeleteAPIByHandlewhich fetches it again (per repository implementation at line 522). Consider usingrepo.DeleteAPI(api.ID)directly since you already have the UUID.func (s *APIService) DeleteAPIByHandle(handle, orgId string) error { if handle == "" { return errors.New("API handle is required") } // Check if API exists api, err := s.apiRepo.GetAPIByHandle(handle, orgId) if err != nil { return err } if api == nil { return constants.ErrAPINotFound } // Delete API from repository - if err := s.apiRepo.DeleteAPIByHandle(handle, orgId); err != nil { + if err := s.apiRepo.DeleteAPI(api.ID); err != nil { return fmt.Errorf("failed to delete api: %w", err) } return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
platform-api/src/internal/constants/error.go(1 hunks)platform-api/src/internal/database/schema.sql(2 hunks)platform-api/src/internal/handler/api.go(13 hunks)platform-api/src/internal/model/api.go(1 hunks)platform-api/src/internal/repository/api.go(10 hunks)platform-api/src/internal/repository/interfaces.go(2 hunks)platform-api/src/internal/service/api.go(8 hunks)platform-api/src/internal/utils/api.go(2 hunks)platform-api/src/internal/utils/handle.go(1 hunks)platform-api/src/resources/openapi.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/handler/api.goplatform-api/src/internal/service/api.go
📚 Learning: 2025-11-06T07:04:01.101Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 105
File: platform-api/src/internal/model/api_publication.go:63-79
Timestamp: 2025-11-06T07:04:01.101Z
Learning: In the API Platform codebase (platform-api/src/internal/model/api_publication.go), the PublicationStateMachine intentionally disallows Published → Publishing transitions. The design requires APIs to be explicitly unpublished before they can be republished, enforcing the lifecycle: Published → Unpublishing → Unpublished → Publishing → Published. This prevents accidental overwrites and ensures proper cleanup.
Applied to files:
platform-api/src/internal/service/api.go
🧬 Code graph analysis (5)
platform-api/src/internal/utils/api.go (2)
platform-api/src/internal/model/api.go (2)
API(25-52)API(55-57)platform-api/src/internal/dto/api.go (1)
API(25-51)
platform-api/src/internal/repository/interfaces.go (1)
platform-api/src/internal/model/api.go (2)
API(25-52)API(55-57)
platform-api/src/internal/utils/handle.go (1)
platform-api/src/internal/constants/error.go (6)
ErrHandleEmpty(24-24)ErrHandleTooShort(25-25)ErrHandleTooLong(26-26)ErrInvalidHandle(27-27)ErrHandleSourceEmpty(29-29)ErrHandleGenerationFailed(28-28)
platform-api/src/internal/service/api.go (7)
platform-api/src/internal/utils/handle.go (2)
ValidateHandle(53-67)GenerateHandle(80-117)platform-api/src/internal/constants/error.go (2)
ErrHandleExists(23-23)ErrAPINotFound(45-45)platform-api/src/internal/model/api.go (3)
API(25-52)API(55-57)BackendService(132-144)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/client/devportal_client/errors.go (1)
ErrAPINotFound(73-73)platform-api/src/internal/dto/gateway.go (1)
APIGatewayListResponse(116-120)platform-api/src/internal/dto/devportal.go (1)
PublishToDevPortalRequest(129-134)
platform-api/src/internal/repository/api.go (1)
platform-api/src/internal/model/api.go (2)
API(25-52)API(55-57)
🔇 Additional comments (12)
platform-api/src/internal/handler/api.go (3)
205-256: Prevent identifier drift: Update/Delete by handle should not accept/derive handle from request body
Handlers correctly route by:apiId(handle). Ensure the service layer never uses request payloadidto change the handle implicitly during update.Also applies to: 258-287
289-372: Gateway association endpoints look consistent after migrating to handle-based APIs
The handle-based service calls are aligned across add/list gateway endpoints.
433-567: DevPortal publish/unpublish/publications: consistent handle-based routing
Nice consistency across publish/unpublish/list-publications flows using...ByHandle.platform-api/src/internal/utils/api.go (1)
42-75: Handle changes are already protected from updatesThe original concern about accidental handle renaming via DTOToModel is already mitigated by existing safeguards:
- UpdateAPIRequest struct has no ID field, making it structurally impossible to send a handle in update request bodies
- applyAPIUpdates never modifies the Handle field
- Repository UPDATE statements do not include handle in the SET clause
- Service explicitly preserves internal UUID before persistence (UpdateAPIByHandle:
updatedAPIModel.ID = existingAPIModel.ID)- Create flow properly sets model.API.ID (UUID) before persistence
All suggested guardrails are already in place. No additional changes needed.
platform-api/src/internal/repository/api.go (2)
155-193: LGTM! NewGetAPIByHandlemethod is well-implemented.The method correctly:
- Filters by both handle and organization_uuid for proper multi-tenancy
- Returns nil for non-existent APIs (consistent with
GetAPIByUUID)- Loads all related configurations
534-548: LGTM!CheckAPIExistsByHandleInOrganizationis correctly implemented.The existence check properly scopes by organization, which is essential for handle uniqueness within an organization.
platform-api/src/internal/service/api.go (6)
107-133: Handle generation and validation logic is well-implemented.The flow correctly:
- Validates user-provided handles using
utils.ValidateHandle- Checks for collisions within the organization before accepting
- Auto-generates handles from API name with collision detection when not provided
367-396: LGTM!UpdateAPIByHandlecorrectly preserves the internal UUID.Line 390 ensures the internal UUID is preserved before calling the repository method, which addresses the data integrity concern at the repository layer.
917-983: Good refactoring withapplyAPIUpdateshelper.Extracting the update logic into a shared helper reduces code duplication between
UpdateAPIandUpdateAPIByHandle, improving maintainability.
985-1022: LGTM!updateAPIBackendServicescorrectly handles backend service lifecycle.The method properly:
- Upserts backend services (creates new or updates existing)
- Removes old associations
- Creates new associations with correct default flags
1082-1105: LGTM!CreateAPIRequestupdated to support user-provided handles.The optional
IDfield allows users to specify their own handle while maintaining backward compatibility when omitted (auto-generation kicks in).
157-196: [Your rewritten review comment text here]
[Exactly ONE classification tag]
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
platform-api/src/internal/repository/api.go (4)
142-145: Handle JSON unmarshal errors.The error from
json.Unmarshalfor transport data is silently ignored. If the stored JSON is malformed, this could lead to unexpected behavior with empty transport arrays.Consider logging or returning the error:
// Parse transport JSON if transportJSON != "" { - json.Unmarshal([]byte(transportJSON), &api.Transport) + if err := json.Unmarshal([]byte(transportJSON), &api.Transport); err != nil { + return nil, fmt.Errorf("failed to unmarshal transport JSON: %w", err) + } }
155-193: Extract common logic to reduce duplication.
GetAPIByHandleandGetAPIByUUIDshare nearly identical logic for scanning, parsing transport JSON, and loading configurations (lines 166-193 vs. lines 126-153). This duplication makes maintenance harder.Consider extracting the common logic into a helper method:
func (r *APIRepo) scanAndLoadAPI(scanner interface{ Scan(...interface{}) error }) (*model.API, error) { api := &model.API{} var transportJSON string var securityEnabled bool err := scanner.Scan( &api.ID, &api.Handle, &api.Name, &api.DisplayName, &api.Description, &api.Context, &api.Version, &api.Provider, &api.ProjectID, &api.OrganizationID, &api.LifeCycleStatus, &api.HasThumbnail, &api.IsDefaultVersion, &api.IsRevision, &api.RevisionedAPIID, &api.RevisionID, &api.Type, &transportJSON, &securityEnabled, &api.CreatedAt, &api.UpdatedAt) if err != nil { if errors.Is(err, sql.ErrNoRows) { return nil, nil } return nil, err } // Parse transport JSON if transportJSON != "" { if err := json.Unmarshal([]byte(transportJSON), &api.Transport); err != nil { return nil, fmt.Errorf("failed to unmarshal transport: %w", err) } } // Load related configurations if err := r.loadAPIConfigurations(api); err != nil { return nil, err } return api, nil }Then both methods would simply execute their query and call this helper.
480-494: Consider using EXISTS for better performance.Using
COUNT(*)requires scanning all matching rows, whileEXISTScan short-circuit after finding the first match. For large tables, this can improve performance.func (r *APIRepo) CheckAPIExistsByHandleInOrganization(handle, orgId string) (bool, error) { query := ` - SELECT COUNT(*) FROM apis + SELECT EXISTS(SELECT 1 FROM apis WHERE handle = ? AND organization_uuid = ? + ) ` - var count int - err := r.db.QueryRow(query, handle, orgId).Scan(&count) + var exists bool + err := r.db.QueryRow(query, handle, orgId).Scan(&exists) if err != nil { return false, err } - return count > 0, nil + return exists, nil }
1185-1199: Parameter name doesn't match queried field.The method parameter is named
name, but the query uses thedisplay_namecolumn. This inconsistency could lead to confusion about which field is being checked.Consider renaming the parameter to match the field:
-func (r *APIRepo) CheckAPIExistsByNameAndVersionInOrganization(name, version, orgId string) (bool, error) { +func (r *APIRepo) CheckAPIExistsByNameAndVersionInOrganization(displayName, version, orgId string) (bool, error) { query := ` SELECT COUNT(*) FROM apis WHERE display_name = ? AND version = ? AND organization_uuid = ? ` var count int - err := r.db.QueryRow(query, name, version, orgId).Scan(&count) + err := r.db.QueryRow(query, displayName, version, orgId).Scan(&count) if err != nil { return false, err } return count > 0, nil }The same EXISTS optimization mentioned for other check methods applies here as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platform-api/src/internal/repository/api.go(10 hunks)platform-api/src/internal/utils/handle.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-api/src/internal/utils/handle.go
🧰 Additional context used
🧬 Code graph analysis (1)
platform-api/src/internal/repository/api.go (2)
platform-api/src/internal/model/api.go (2)
API(25-52)API(55-57)platform-api/src/internal/dto/api.go (1)
API(25-51)
🔇 Additional comments (4)
platform-api/src/internal/repository/api.go (4)
409-424: LGTM! Past review concern addressed.The
UpdateAPIByHandlemethod now correctly fetches the API UUID before delegating toUpdateAPI, addressing the critical issue raised in the previous review. The implementation properly handles the case whereapi.IDis not set by the caller.
465-478: LGTM!The
DeleteAPIByHandlemethod follows the correct pattern by resolving the handle to a UUID before delegating to the existingDeleteAPImethod. Error handling is appropriate.
1169-1183: Clarify "identifier" terminology.The method name
CheckAPIExistsByIdentifierInOrganizationsuggests it checks a general identifier, but it specifically queries thenamefield. Given the PR introduceshandleas a user-facing identifier, this naming could be confusing.Please verify the intended behavior:
- Should "identifier" refer to the
handlefield (the new user-provided identifier)?- Or should it check the
namefield (as currently implemented)?- Or is "identifier" meant to encompass both?
Consider renaming to
CheckAPIExistsByNameInOrganizationfor clarity if the current implementation is correct.Additionally, the same EXISTS optimization mentioned for
CheckAPIExistsByHandleInOrganizationapplies here.
60-75: The database schema already enforces handle uniqueness within an organization through a UNIQUE constraint defined on line 67 ofplatform-api/src/internal/database/schema.sql:UNIQUE(handle, organization_uuid). The application-level check suggested in the original comment is unnecessary, and theCreateAPImethod's implementation is correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
platform-api/src/internal/repository/api.go (1)
311-340: Consider addinghandleto the SELECT for consistency.
GetAPIsByGatewayIDdoes not select or populate thehandlefield, while otherGetAPIs*methods do. If callers expect a fully populatedmodel.API, this inconsistency could cause issues.If the
handlefield should be available to callers:query := ` - SELECT a.uuid, a.name, a.display_name, a.type, a.created_at, a.updated_at + SELECT a.uuid, a.handle, a.name, a.display_name, a.type, a.created_at, a.updated_at FROM apis a INNER JOIN api_deployments ad ON a.uuid = ad.api_uuid WHERE ad.gateway_uuid = ? AND a.organization_uuid = ? ORDER BY a.created_at DESC `And update the Scan accordingly:
err := rows.Scan( - &api.ID, &api.Name, &api.DisplayName, &api.Type, &api.CreatedAt, &api.UpdatedAt, + &api.ID, &api.Handle, &api.Name, &api.DisplayName, &api.Type, &api.CreatedAt, &api.UpdatedAt, )platform-api/src/internal/service/gateway_internal.go (2)
168-169: Update stale comment.The comment references
apiIDbut the parameter was renamed toapiHandle.newAPI := &model.API{ - Handle: apiHandle, // Use provided apiID as handle + Handle: apiHandle, // Use provided apiHandle as handle Name: notification.Configuration.Spec.Name,
193-199: Remove unnecessary database round trip.The
CreateAPImethod already assigns the generated UUID tonewAPI.ID(seeapi.ID = uuid.New().String()in repository/api.go line 56). AfterCreateAPIreturns successfully,newAPI.IDcontains the UUID, making this additional fetch redundant.err = s.apiRepo.CreateAPI(newAPI) if err != nil { return nil, fmt.Errorf("failed to create API: %w", err) } -// Retrieve the existing API or newly created API to get its ID -existingAPI, err = s.apiRepo.GetAPIByHandle(apiHandle, orgID) -if err != nil { - return nil, fmt.Errorf("failed to check existing API: %w", err) -} - -apiUUID = existingAPI.ID +apiUUID = newAPI.ID
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platform-api/src/internal/repository/api.go(12 hunks)platform-api/src/internal/service/gateway_internal.go(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/service/gateway_internal.go
🧬 Code graph analysis (2)
platform-api/src/internal/service/gateway_internal.go (3)
gateway/gateway-controller/pkg/utils/api_utils.go (1)
APIDeploymentNotification(201-210)platform-api/src/internal/dto/gateway_internal.go (2)
APIDeploymentNotification(23-32)GatewayAPIDeploymentResponse(63-68)platform-api/src/internal/constants/constants.go (1)
AssociationTypeGateway(106-106)
platform-api/src/internal/repository/api.go (2)
platform-api/src/internal/model/api.go (2)
API(25-52)API(55-57)platform-api/src/internal/dto/api.go (1)
API(25-51)
🔇 Additional comments (14)
platform-api/src/internal/repository/api.go (10)
29-31: LGTM!Clean import addition for UUID generation.
55-56: LGTM!Good approach to always generate a new UUID server-side, ensuring consistent ID generation regardless of client input.
64-76: LGTM!The INSERT statement correctly includes the
handlecolumn and value. The placeholder count matches the column count.
123-157: LGTM!
GetAPIByUUIDcorrectly selects and scans thehandlefield, and properly handlessql.ErrNoRowsby returningnil, nil.
159-197: LGTM!
GetAPIByHandleis well-implemented with proper error handling and configuration loading.
200-242: LGTM!
GetAPIsByProjectIDcorrectly includeshandlein the SELECT and Scan operations.
252-309: LGTM!
GetAPIsByOrganizationIDconsistently includes thehandlefield for both query branches.
413-428: LGTM!The previous review feedback was addressed. The method now correctly fetches the API by handle to obtain the UUID before delegating to
UpdateAPI.
469-482: LGTM!
DeleteAPIByHandlecorrectly looks up the API first to get the UUID, then delegates toDeleteAPI.
484-498: LGTM!
CheckAPIExistsByHandleInOrganizationis a clean, simple existence check.platform-api/src/internal/service/gateway_internal.go (4)
99-106: LGTM!Good refactoring to use
apiHandleparameter name, clarifying that this is a user-provided handle rather than a UUID.
133-136: LGTM!Correctly changed from
GetAPIByUUIDtoGetAPIByHandleto support the handle-based lookup flow.
201-216: LGTM!Correctly uses
apiUUIDfor backend service associations and handles the existing API case properly.
218-280: LGTM!All downstream operations (deployments, associations, response) correctly use
apiUUIDinstead of the handle.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
platform-api/src/internal/service/api.go (1)
266-277: Consider logging errors in HandleExistsCheck for observability.The current approach of returning
trueon error is safe for handle generation (prevents collisions), but silently swallowing the error makes debugging harder if there are underlying database connectivity issues.func (s *APIService) HandleExistsCheck(orgId string) func(string) bool { return func(handle string) bool { exists, err := s.apiRepo.CheckAPIExistsByHandleInOrganization(handle, orgId) if err != nil { // On error, assume it exists to be safe (will trigger retry) + log.Printf("[WARN] HandleExistsCheck failed for handle %s: %v", handle, err) return true } return exists } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
platform-api/src/internal/model/api.go(2 hunks)platform-api/src/internal/repository/api.go(11 hunks)platform-api/src/internal/repository/interfaces.go(2 hunks)platform-api/src/internal/service/api.go(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/service/api.go
🧬 Code graph analysis (2)
platform-api/src/internal/repository/interfaces.go (1)
platform-api/src/internal/model/api.go (3)
API(25-52)API(55-57)APIMetadata(60-66)
platform-api/src/internal/repository/api.go (2)
platform-api/src/internal/model/api.go (3)
API(25-52)API(55-57)APIMetadata(60-66)platform-api/src/internal/dto/api.go (1)
API(25-51)
🔇 Additional comments (13)
platform-api/src/internal/model/api.go (2)
27-27: LGTM!The
Handlefield is correctly added with appropriate JSON and DB tags, aligning with the PR objective to support user-provided identifiers for APIs.
59-66: LGTM!The
APIMetadatastruct is a well-designed lightweight projection type for efficient handle-to-UUID resolution, containing only the essential fields needed for lookups. NoTableName()method is required since this is used for SELECT queries against the existingapistable.platform-api/src/internal/repository/interfaces.go (2)
50-51: LGTM!The handle-based retrieval methods are well-designed:
GetAPIByHandlefor full API retrieval with configurationsGetAPIMetadataByHandlefor lightweight handle-to-UUID resolutionBoth follow the established naming convention and parameter ordering.
71-71: LGTM!The
CheckAPIExistsByHandleInOrganizationmethod is correctly added to support handle uniqueness validation during API creation, consistent with the existingCheckAPIExistsByIdentifierInOrganizationpattern.platform-api/src/internal/service/api.go (6)
107-133: LGTM!The handle validation and generation logic correctly handles both scenarios:
- User-provided handle: validates format and checks for uniqueness within the organization
- Auto-generated handle: derives from API name with collision detection via
HandleExistsCheckThe separation between internal UUID (
apiUUID) and user-facing handle is well-implemented.
385-414: LGTM!The
UpdateAPIByHandleimplementation correctly:
- Fetches the full API model by handle to get all fields including the internal UUID
- Applies updates using the shared
applyAPIUpdateshelper- Preserves the internal UUID (line 408) before calling
UpdateAPIThis addresses the concern from previous reviews about UUID handling in handle-based operations.
888-954: LGTM!The
applyAPIUpdateshelper centralizes update logic well:
- Validates the request before applying changes
- Properly handles optional fields with nil checks
- Correctly passes the internal UUID to
updateAPIBackendServices
956-993: LGTM!The
updateAPIBackendServiceshelper correctly manages backend service associations:
- Upserts backend services to get/create UUIDs
- Removes existing associations before adding new ones (full replacement semantics)
- Preserves the
isDefaultlogic
428-481: LGTM!The handle-based wrapper methods follow a consistent pattern:
- Resolve handle to internal UUID via
getAPIUUIDByHandle- Delegate to existing UUID-based implementation
This approach maximizes code reuse and maintains a single source of truth for business logic.
1054-1054: LGTM!The optional
IDfield inCreateAPIRequestallows users to provide their own handle while maintaining backward compatibility (auto-generation when omitted).platform-api/src/internal/repository/api.go (3)
159-197: LGTM!
GetAPIByHandlecorrectly implements handle-based retrieval:
- Queries by both handle and organization_uuid (ensuring org-scoped uniqueness)
- Parses transport JSON and loads full configurations
- Follows the same pattern as
GetAPIByUUID
199-216: LGTM!
GetAPIMetadataByHandleis a well-designed lightweight query that returns only the essential fields needed for handle-to-UUID resolution, avoiding the overhead of loading full configurations.
471-485: LGTM!
CheckAPIExistsByHandleInOrganizationcorrectly implements existence checking with a simple COUNT query scoped by organization.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
platform-api/src/internal/service/api.go (1)
195-196: Note: UUID assignment may be redundant.Line 196 sets
apiModel.ID = apiUUID, but the repository'sCreateAPIunconditionally generates a new UUID (platform-api/src/internal/repository/api.go:56). While redundant, this doesn't cause issues—the repository UUID will be authoritative. Consider removing this assignment for clarity, or document that the repository controls UUID generation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
platform-api/src/internal/repository/api.go(11 hunks)platform-api/src/internal/repository/interfaces.go(2 hunks)platform-api/src/internal/service/api.go(9 hunks)platform-api/src/internal/service/gateway_internal.go(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/service/api.go
🧬 Code graph analysis (4)
platform-api/src/internal/repository/api.go (2)
platform-api/src/internal/model/api.go (3)
APIMetadata(60-66)API(25-52)API(55-57)platform-api/src/internal/dto/api.go (1)
API(25-51)
platform-api/src/internal/repository/interfaces.go (1)
platform-api/src/internal/model/api.go (1)
APIMetadata(60-66)
platform-api/src/internal/service/gateway_internal.go (4)
gateway/gateway-controller/pkg/utils/api_utils.go (1)
APIDeploymentNotification(201-210)platform-api/src/internal/dto/gateway_internal.go (2)
APIDeploymentNotification(23-32)GatewayAPIDeploymentResponse(63-68)platform-api/src/internal/constants/error.go (1)
ErrAPINotFound(45-45)platform-api/src/internal/constants/constants.go (1)
AssociationTypeGateway(106-106)
platform-api/src/internal/service/api.go (7)
platform-api/src/internal/utils/handle.go (2)
ValidateHandle(53-67)GenerateHandle(80-120)platform-api/src/internal/constants/error.go (2)
ErrHandleExists(23-23)ErrAPINotFound(45-45)platform-api/src/internal/model/api.go (3)
API(25-52)API(55-57)BackendService(141-153)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/client/devportal_client/errors.go (1)
ErrAPINotFound(73-73)platform-api/src/internal/websocket/transport.go (1)
Transport(30-83)platform-api/src/internal/dto/backend_service.go (1)
BackendService(23-36)
🔇 Additional comments (19)
platform-api/src/internal/repository/interfaces.go (1)
50-50: LGTM: Handle-based repository methods added.The new interface methods
GetAPIMetadataByHandleandCheckAPIExistsByHandleInOrganizationfollow consistent naming conventions and parameter ordering with existing methods in the interface.Also applies to: 70-70
platform-api/src/internal/service/gateway_internal.go (6)
99-119: LGTM: Method signature updated to accept handle.The signature change from
apiIDtoapiHandleis consistent with the PR's goal of using handles as user-facing identifiers. Early gateway validation is good practice for fail-fast error handling.
132-141: LGTM: Efficient metadata-based API lookup.Using
GetAPIMetadataByHandlefor existence checking is more efficient than fetching the full API model, as it retrieves only minimal information needed for validation.
168-194: Verify: API UUID assignment relies on CreateAPI side effects.The code assumes
CreateAPIsetsnewAPI.ID(line 194), but past review comments flagged that the repository'sCreateAPIunconditionally generates a new UUID (platform-api/src/internal/repository/api.go:55-56), which could overwrite any pre-set ID. While this works for this flow where the handle is separate, ensure the pattern is documented so other callers understand the UUID is always repository-generated.
196-202: LGTM: Backend service associations use internal UUID.Backend service associations correctly use the internal
apiUUIDfor database foreign key relationships, maintaining proper separation between user-facing handles and internal identifiers.
205-211: LGTM: Proper organization validation for existing APIs.The organization check ensures that even if an API exists with the given handle, it must belong to the correct organization before proceeding with deployment.
213-276: LGTM: Consistent UUID usage throughout deployment flow.All database operations (deployments, associations) correctly use the internal
apiUUID. The response at line 271 returns the UUID, which is appropriate for internal gateway operations where stable identifiers are needed for tracking.platform-api/src/internal/repository/api.go (6)
55-56: LGTM: UUID generation pattern clarified.The unconditional UUID generation in
CreateAPIestablishes clear ownership: the repository layer always controls UUID assignment. This resolves the past review concern by making the behavior explicit and consistent.
64-76: LGTM: Handle persisted during API creation.The handle field is correctly included in the INSERT statement and bound at the appropriate position in the parameter list.
124-144: LGTM: Handle field retrieved in GetAPIByUUID.The handle is correctly added to the SELECT clause and scanned into the API model, maintaining consistency with the updated schema.
159-176: LGTM: Efficient metadata retrieval by handle.This lightweight method retrieves only essential API information (ID, handle, name, context, organizationID) for handle-to-UUID resolution, avoiding the overhead of loading full configurations.
178-288: LGTM: Handle field consistently retrieved across query methods.The handle field is properly included in SELECT statements and Scan operations for
GetAPIsByProjectIDandGetAPIsByOrganizationID, ensuring the handle is available wherever the full API model is returned.
431-445: LGTM: Handle existence check implementation.The
CheckAPIExistsByHandleInOrganizationmethod uses an efficient COUNT query with proper organization scoping to verify handle uniqueness within an organization.platform-api/src/internal/service/api.go (6)
107-133: LGTM: Robust handle generation with validation and collision detection.The handle logic correctly:
- Generates an internal UUID separate from the user-facing handle
- Validates user-provided handles via
utils.ValidateHandle- Checks for handle collision within the organization
- Auto-generates collision-free handles using
utils.GenerateHandlewhen not providedThe separation of internal UUID (for DB relationships) and handle (for user identification) is architecturally sound.
205-216: LGTM: Backend service associations use internal UUID.Backend service associations correctly use
apiUUIDfor database foreign key relationships, maintaining consistency with the internal identifier scheme.
248-268: LGTM: Clean handle-based helpers with safe error handling.
GetAPIByHandlefollows a simple delegation pattern that resolves the handle to UUID before calling the existing UUID-based methodHandleExistsCheckreturns a closure for collision detection with safe error handling (assumes handle exists on error to avoid collisions)These helpers promote code reuse and maintain consistent error handling.
270-286: LGTM: Efficient handle-to-UUID resolution.The
getAPIUUIDByHandlehelper provides lightweight UUID resolution usingGetAPIMetadataByHandle, avoiding the overhead of loading full API configurations when only the internal ID is needed.
376-450: LGTM: Consistent handle-based method wrappers.All handle-based wrapper methods (
UpdateAPIByHandle,DeleteAPIByHandle,AddGatewaysToAPIByHandle,GetAPIGatewaysByHandle,DeployAPIRevisionByHandle,PublishAPIToDevPortalByHandle,UnpublishAPIFromDevPortalByHandle,GetAPIPublicationsByHandle) follow a consistent pattern:
- Resolve handle to internal UUID via
getAPIUUIDByHandle- Delegate to existing UUID-based method
This promotes code reuse and maintains a single source of truth for business logic.
1023-1023: LGTM: CreateAPIRequest supports user-provided handles.Adding the
IDfield toCreateAPIRequestallows users to provide custom handles during API creation, aligning with the PR's goal of supporting user-defined identifiers.
| // applyAPIUpdates applies update request fields to an existing API model and handles backend services | ||
| func (s *APIService) applyAPIUpdates(existingAPIModel *model.API, req *UpdateAPIRequest, orgId string) (*dto.API, error) { | ||
| // Validate update request | ||
| if err := s.validateUpdateAPIRequest(req); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| existingAPI := s.apiUtil.ModelToDTO(existingAPIModel) | ||
|
|
||
| // Update fields (only allow certain fields to be updated) | ||
| if req.DisplayName != nil { | ||
| existingAPI.DisplayName = *req.DisplayName | ||
| } | ||
| if req.Description != nil { | ||
| existingAPI.Description = *req.Description | ||
| } | ||
| if req.Provider != nil { | ||
| existingAPI.Provider = *req.Provider | ||
| } | ||
| if req.LifeCycleStatus != nil { | ||
| existingAPI.LifeCycleStatus = *req.LifeCycleStatus | ||
| } | ||
| if req.HasThumbnail != nil { | ||
| existingAPI.HasThumbnail = *req.HasThumbnail | ||
| } | ||
| if req.IsDefaultVersion != nil { | ||
| existingAPI.IsDefaultVersion = *req.IsDefaultVersion | ||
| } | ||
| if req.IsRevision != nil { | ||
| existingAPI.IsRevision = *req.IsRevision | ||
| } | ||
| if req.RevisionedAPIID != nil { | ||
| existingAPI.RevisionedAPIID = *req.RevisionedAPIID | ||
| } | ||
| if req.RevisionID != nil { | ||
| existingAPI.RevisionID = *req.RevisionID | ||
| } | ||
| if req.Type != nil { | ||
| existingAPI.Type = *req.Type | ||
| } | ||
| if req.Transport != nil { | ||
| existingAPI.Transport = *req.Transport | ||
| } | ||
| if req.MTLS != nil { | ||
| existingAPI.MTLS = req.MTLS | ||
| } | ||
| if req.Security != nil { | ||
| existingAPI.Security = req.Security | ||
| } | ||
| if req.CORS != nil { | ||
| existingAPI.CORS = req.CORS | ||
| } | ||
| if req.BackendServices != nil { | ||
| if err := s.updateAPIBackendServices(existingAPIModel.ID, req.BackendServices, orgId); err != nil { | ||
| return nil, err | ||
| } | ||
| existingAPI.BackendServices = *req.BackendServices | ||
| } | ||
| if req.APIRateLimiting != nil { | ||
| existingAPI.APIRateLimiting = req.APIRateLimiting | ||
| } | ||
| if req.Operations != nil { | ||
| existingAPI.Operations = *req.Operations | ||
| } | ||
|
|
||
| return existingAPI, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
LGTM: Update logic refactored into reusable helper.
Extracting the update logic into applyAPIUpdates centralizes field-by-field update handling and promotes code reuse. The method properly delegates backend service updates to updateAPIBackendServices, maintaining clean separation of concerns.
🤖 Prompt for AI Agents
In platform-api/src/internal/service/api.go around lines 857 to 923, the
reviewer approved the refactor of update logic into applyAPIUpdates and noted
delegation to updateAPIBackendServices; no functional issues were raised so no
code changes are required—leave the implementation as-is.
| // updateAPIBackendServices handles backend service updates for an API | ||
| func (s *APIService) updateAPIBackendServices(apiUUID string, backendServices *[]dto.BackendService, orgId string) error { | ||
| // Process backend services: check if they exist, create or update them | ||
| var backendServiceUUIDs []string | ||
| for _, backendService := range *backendServices { | ||
| backendServiceUUID, err := s.upstreamService.UpsertBackendService(&backendService, orgId) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to process backend service '%s': %w", backendService.Name, err) | ||
| } | ||
| backendServiceUUIDs = append(backendServiceUUIDs, backendServiceUUID) | ||
| } | ||
|
|
||
| // Remove existing associations | ||
| existingBackendServices, err := s.upstreamService.GetBackendServicesByAPIID(apiUUID) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get existing backend services: %w", err) | ||
| } | ||
|
|
||
| for _, existingService := range existingBackendServices { | ||
| if err := s.upstreamService.DisassociateBackendServiceFromAPI(apiUUID, existingService.ID); err != nil { | ||
| return fmt.Errorf("failed to remove existing backend service association: %w", err) | ||
| } | ||
| } | ||
|
|
||
| // Add new associations | ||
| for i, backendServiceUUID := range backendServiceUUIDs { | ||
| isDefault := i == 0 | ||
| if len(*backendServices) > 0 && i < len(*backendServices) { | ||
| isDefault = (*backendServices)[i].IsDefault | ||
| } | ||
|
|
||
| if err := s.upstreamService.AssociateBackendServiceWithAPI(apiUUID, backendServiceUUID, isDefault); err != nil { | ||
| return fmt.Errorf("failed to associate backend service with API: %w", err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
LGTM: Backend service update logic properly isolated.
The updateAPIBackendServices helper correctly:
- Processes and upserts backend services
- Removes existing associations before adding new ones
- Uses the internal
apiUUIDfor all database operations - Provides detailed error messages for troubleshooting
This refactoring improves maintainability by isolating backend service handling.
🤖 Prompt for AI Agents
platform-api/src/internal/service/api.go around lines 925 to 962: no changes
required — the updateAPIBackendServices function correctly upserts backend
services, removes old associations, and re-associates new ones using the apiUUID
with clear error propagation; leave the implementation as-is.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
platform-api/src/internal/service/api.go (1)
254-265: Don’t silently treat repo errors as “handle exists” during GenerateHandle()
HandleExistsCheckreturnstrueon repo error, which can turn transient DB issues into repeated collisions and ultimatelyErrHandleGenerationFailedwith no signal of the real cause. At minimum, log the error (with orgId + handle) so operators can diagnose; ideally, consider aGenerateHandlevariant that can propagate the error instead of converting it into “exists”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platform-api/src/internal/service/api.go(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/service/api.go
🧬 Code graph analysis (1)
platform-api/src/internal/service/api.go (5)
platform-api/src/internal/utils/handle.go (2)
ValidateHandle(53-67)GenerateHandle(80-120)platform-api/src/internal/constants/error.go (2)
ErrHandleExists(23-23)ErrAPINotFound(45-45)platform-api/src/internal/model/api.go (3)
API(25-52)API(55-57)BackendService(141-153)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/client/devportal_client/errors.go (1)
ErrAPINotFound(73-73)
🔇 Additional comments (1)
platform-api/src/internal/service/api.go (1)
245-283: Handle→UUID resolution + wrapper methods look clean; verify org scoping and uniqueness assumptionsThe handle-based wrappers correctly resolve UUID then delegate to existing UUID-based methods (good for consistency and authorization checks). Please verify
apiRepo.GetAPIMetadataByHandle(handle, orgId)is scoped byorgIdand relies on the same uniqueness guarantee you intend (one handle per org).Also applies to: 373-447
| // Handle the API handle (user-facing identifier) | ||
| var handle string | ||
| if req.ID != "" { | ||
| // Validate user-provided handle | ||
| if err := utils.ValidateHandle(req.ID); err != nil { | ||
| return nil, err | ||
| } | ||
| // Check if handle already exists in the organization | ||
| handleExists, err := s.apiRepo.CheckAPIExistsByHandleInOrganization(req.ID, orgId) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if handleExists { | ||
| return nil, constants.ErrHandleExists | ||
| } | ||
| handle = req.ID | ||
| } else { | ||
| // Generate handle from API name with collision detection | ||
| var err error | ||
| handle, err = utils.GenerateHandle(req.Name, s.HandleExistsCheck(orgId)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
Enforce handle uniqueness at the DB/repo layer (TOCTOU) + clarify “ID == handle” semantics
The pre-check CheckAPIExistsByHandleInOrganization can race (two requests pass the check, then both try create). Please ensure there’s a unique constraint (at least (organization_uuid, handle)) and that apiRepo.CreateAPI(...) maps duplicate-handle DB errors to constants.ErrHandleExists (otherwise this returns a generic “failed to create api”). Also, since CreateAPIRequest.ID and dto.API.ID now carry the handle (not the internal UUID), consider renaming CreateAPIRequest.ID → Handle (or similar) to avoid future misuse.
Also applies to: 153-178, 190-221
| // Associate backend services with the API (use internal UUID) | ||
| for i, backendServiceUUID := range backendServiceIdList { | ||
| isDefault := i == 0 // First backend service is default | ||
| if len(req.BackendServices) > 0 && i < len(req.BackendServices) { | ||
| // Check if isDefault was explicitly set in the request | ||
| isDefault = req.BackendServices[i].IsDefault | ||
| } | ||
|
|
||
| if err := s.upstreamService.AssociateBackendServiceWithAPI(apiId, backendServiceUUID, isDefault); err != nil { | ||
| if err := s.upstreamService.AssociateBackendServiceWithAPI(apiUUID, backendServiceUUID, isDefault); err != nil { | ||
| return nil, fmt.Errorf("failed to associate backend service with API: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Default backend-service selection is not “explicit” (bool cannot express omitted); ensure exactly one default
Both create and update paths overwrite isDefault := i == 0 with BackendService.IsDefault, but IsDefault is a non-pointer bool, so “omitted” is indistinguishable from “explicit false”. Result: you can easily end up with no default backend service even when the user didn’t intend to set one. Prefer: if none of the provided backend services are marked default, force the first to be default (and ideally also prevent multiple defaults).
Suggested patch pattern (apply similarly in both loops):
+ // pick a default deterministically if none provided
+ defaultIdx := 0
+ foundDefault := false
+ for i, bs := range req.BackendServices { // or *backendServices in update
+ if bs.IsDefault {
+ defaultIdx = i
+ foundDefault = true
+ break
+ }
+ }
...
- isDefault := i == 0 // First backend service is default
- if len(req.BackendServices) > 0 && i < len(req.BackendServices) {
- // Check if isDefault was explicitly set in the request
- isDefault = req.BackendServices[i].IsDefault
- }
+ isDefault := i == defaultIdx
+ _ = foundDefault // (optional) use to validate “only one default” if desiredAlso applies to: 946-956
🤖 Prompt for AI Agents
platform-api/src/internal/service/api.go around lines 202-213 (also apply same
fix at 946-956): the loop currently overwrites the implicit default (isDefault
:= i == 0) with req.BackendServices[i].IsDefault where IsDefault is a
non-pointer bool so omission cannot be distinguished from explicit false, which
can leave zero or multiple defaults; fix by first scanning req.BackendServices
to detect how many entries explicitly set IsDefault==true, if zero then treat
the first backend as default, if more than one return a validation error (or
normalize by picking the first and rejecting others), then in the loop use that
computed decision (don’t rely on the raw non-pointer bool to indicate omission);
apply the same pre-scan-and-enforce logic in both the create and update loops.
2 participants
Purpose
#407
Goals
Approach
User stories
Documentation
Automation tests
Security checks
Samples
Related PRs
Test environment
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.