Import and Create API from OpenAPI definition #162
Conversation
WalkthroughAdds OpenAPI import support: a new DTO for multipart requests, handler and service logic to accept URL or uploaded definition, utilities to parse/merge OpenAPI content, a new POST endpoint, and related gateway validation signature/test updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant Service
participant Utils
participant Storage
Client->>Handler: POST /api/v1/import/open-api (multipart: url/definition + api)
Handler->>Handler: Parse multipart form\nEnsure url or file present
Handler->>Service: ImportFromOpenAPI(req, orgId)
Service->>Utils: ValidateAndParseOpenAPI(content)
Utils-->>Service: extractedAPI / error
Service->>Utils: MergeAPIDetails(userAPI, extractedAPI)
Utils-->>Service: mergedAPI
Service->>Service: Re-validate mergedAPI
Service->>Storage: CreateAPI(mergedAPI, orgId)
Storage-->>Service: createdAPI
Service-->>Handler: createdAPI
Handler-->>Client: 201 Created (API)
Note over Handler,Service: Errors -> mapped to 400/401/404/409/500
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 5
🧹 Nitpick comments (3)
platform-api/src/internal/dto/openapi.go (1)
39-44: Clarify binding semantics forImportOpenAPIRequest.APICurrently
ImportOpenAPIRequest.APIis tagged withjson:"api", but the handler fills it via:apiJSON := c.PostForm("api") json.Unmarshal([]byte(apiJSON), &req.API)So JSON binding on the whole struct is not used, and the tag may be misleading. Two options:
- If you intend to keep manual multipart handling, consider adding
form:"api"and optionally dropping thejsontag, or- If you later move to
ShouldBind, ensure the handler and tags are aligned.Not critical, but tightening this now will avoid confusion for future maintainers.
platform-api/src/internal/service/api.go (1)
1379-1452: ImportFromOpenAPI flow is sound; consider small cleanups and error normalizationThe overall flow (fetch → validate+parse → merge → reuse CreateAPI) looks good and reuses existing validation and creation logic appropriately.
A couple of small points:
Redundant validation
You callvalidateCreateAPIRequest(createReq)here and thenCreateAPIimmediately calls the same validator again. You can safely drop the explicit call and letCreateAPIown request validation.Normalize fetch error text
The accumulated error message for URL failures is:"failed to fetch OpenAPI from URL: %s"while the handler checks for
"failed to fetch OpenAPI definition". Aligning these (or moving to a shared constant / sentinel error) will avoid future divergence and keep HTTP error mapping consistent with intent (see handler comment).These are small changes but would reduce duplication and make error handling more robust.
platform-api/src/internal/utils/api.go (1)
1452-1531: Merging semantics inMergeAPIDetailslook correct; consider a defensive nil checkThe merge logic (user overrides extracted values; required fields always from user; operations always from OpenAPI) is consistent with the contract-flow use case and keeps OpenAPI as the source of truth for operations.
Given the function signature accepts pointers, you might consider a defensive nil check to guard future callers:
if userAPI == nil || extractedAPI == nil { return nil }(or return
extractedAPIwhen userAPI is nil) to avoid panics if it’s ever reused outside the current controlled path.Not urgent, but cheap insurance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
platform-api/src/internal/dto/openapi.go(1 hunks)platform-api/src/internal/handler/api.go(4 hunks)platform-api/src/internal/service/api.go(1 hunks)platform-api/src/internal/service/gateway_test.go(2 hunks)platform-api/src/internal/utils/api.go(1 hunks)platform-api/src/resources/openapi.yaml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: thivindu
Repo: wso2/api-platform PR: 142
File: platform-api/src/resources/openapi.yaml:905-969
Timestamp: 2025-11-12T08:52:52.909Z
Learning: In the wso2/api-platform repository, the team follows an API-first development approach where OpenAPI specs may document planned features before backend implementation is complete, allowing frontend development to proceed against the intended API contract without requiring changes later.
🧬 Code graph analysis (4)
platform-api/src/internal/utils/api.go (1)
platform-api/src/internal/dto/api.go (1)
API(25-51)
platform-api/src/internal/handler/api.go (5)
platform-api/src/internal/middleware/auth.go (1)
GetOrganizationFromContext(169-176)platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/dto/openapi.go (1)
ImportOpenAPIRequest(40-44)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/constants/error.go (8)
ErrAPIAlreadyExists(41-41)ErrProjectNotFound(33-33)ErrInvalidAPIName(44-44)ErrInvalidAPIContext(42-42)ErrInvalidAPIVersion(43-43)ErrInvalidLifecycleState(45-45)ErrInvalidAPIType(46-46)ErrInvalidTransport(47-47)
platform-api/src/internal/service/api.go (3)
platform-api/src/internal/dto/openapi.go (1)
ImportOpenAPIRequest(40-44)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/model/api.go (2)
API(25-51)API(54-56)
platform-api/src/internal/service/gateway_test.go (1)
platform-api/src/internal/constants/constants.go (1)
GatewayFunctionalityTypeRegular(53-53)
🔇 Additional comments (4)
platform-api/src/internal/service/gateway_test.go (2)
21-21: LGTM!The import is necessary for accessing
constants.GatewayFunctionalityTypeRegularused throughout the test cases.
30-37: LGTM!The test struct correctly includes the new
functionalityTypefield to match the updatedvalidateGatewayInputsignature.platform-api/src/internal/handler/api.go (1)
973-977: New/api/v1/import/open-apiroute wiring looks consistentThe new route is correctly grouped under
/api/v1/importand wired toImportOpenAPI, consistent with the existingImportAPIProjectpattern.No issues from the handler/route wiring perspective.
platform-api/src/internal/utils/api.go (1)
1436-1450: Good reuse viaValidateAndParseOpenAPI
ValidateAndParseOpenAPIcleanly composes the existing validation and parsing steps and wraps errors with clear context strings. This keeps the service layer thinner and error messages more readable.No issues here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
platform-api/src/internal/utils/api.go (2)
1436-1450: Wrap helper is fine; consider avoiding double libopenapi parsing.
ValidateAndParseOpenAPIcurrently creates and inspects the libopenapi document twice viaValidateOpenAPIDefinitionandParseAPIDefinition. It’s correct but does extra work. If this becomes hot, consider refactoring the validation/parse helpers to share a singlelibopenapi.Document(e.g., a lower-level helper that builds the model once and returns both validation errors and a parsed DTO).
1452-1535: Preserve extracted config (MTLS/Security/CORS/RateLimiting) when user does not override.The merge behavior for basic fields and operations looks good, but config structs are only taken from
userAPI:
MTLS,Security,CORS,APIRateLimitingare set fromuserAPIif non‑nil; otherwise they remainnileven ifextractedAPIhas values.- The comment says “Use user-provided configuration if available”, which implies extracted values should be the default baseline.
To future‑proof this against later enhancements to
ParseAPIDefinition, consider falling back to extracted config when the user hasn’t provided one:- // Use user-provided configuration if available - if userAPI.MTLS != nil { - merged.MTLS = userAPI.MTLS - } - if userAPI.Security != nil { - merged.Security = userAPI.Security - } - if userAPI.CORS != nil { - merged.CORS = userAPI.CORS - } - if userAPI.APIRateLimiting != nil { - merged.APIRateLimiting = userAPI.APIRateLimiting - } + // Use user-provided configuration if available, otherwise keep extracted values + if userAPI.MTLS != nil { + merged.MTLS = userAPI.MTLS + } else { + merged.MTLS = extractedAPI.MTLS + } + if userAPI.Security != nil { + merged.Security = userAPI.Security + } else { + merged.Security = extractedAPI.Security + } + if userAPI.CORS != nil { + merged.CORS = userAPI.CORS + } else { + merged.CORS = extractedAPI.CORS + } + if userAPI.APIRateLimiting != nil { + merged.APIRateLimiting = userAPI.APIRateLimiting + } else { + merged.APIRateLimiting = extractedAPI.APIRateLimiting + }This keeps OpenAPI‑derived defaults while still letting the user override selectively.
platform-api/src/internal/service/gateway_test.go (1)
20-224: Functionality type coverage looks solid; consider constants for"ai"/"event".The extended table-driven tests for
functionalityType(empty, whitespace, invalid, valid"ai"/"event") give good coverage ofvalidateGatewayInput. One minor improvement would be to use constants (e.g.,constants.GatewayFunctionalityTypeAI/...Event) rather than string literals"ai"and"event"so tests stay aligned if valid values change inconstants.platform-api/src/internal/service/api.go (1)
1379-1450: ImportFromOpenAPI flow is correct; refine error messaging for URL‑only failures.The end‑to‑end flow (fetch content →
ValidateAndParseOpenAPI→MergeAPIDetails→CreateAPI) is sound and correctly reuses existing CreateAPI validation/creation logic.Two small polish points:
URL‑only failure message is slightly misleading.
When onlyreq.URLis set andFetchOpenAPIFromURLfails, the returned error combines:
"failed to fetch OpenAPI from URL: ..."and"either URL or definition file must be provided"
even though a URL was provided. Consider making the second part more explicit, e.g. “failed to fetch OpenAPI from URL and no definition file was provided”.Dropped URL error when a file is also provided.
If the URL fetch fails but the file upload succeeds,errorListretains the URL error but it’s never surfaced. If that’s intentional (file takes precedence), a brief comment or a debug log at the failure site would help future readers understand why the error is ignored.These are non‑blocking, but tightening them would make debugging import issues easier.
platform-api/src/internal/dto/openapi.go (1)
39-44: DTO aligns with service; optionally clarifyapiparsing in comments.
ImportOpenAPIRequestcleanly represents the multipart payload for the import flow and matchesAPIService.ImportFromOpenAPI’s expectations. Sinceapiis received as a JSON string form field and then unmarshaled into thisAPIstruct by the handler, consider adding a short comment noting that behavior so readers don’t assume automatic form binding is doing a deep object bind here.platform-api/src/resources/openapi.yaml (1)
2616-2659: ImportOpenAPIRequest schema matches payload; optionally document file‑over‑URL precedence.The
ImportOpenAPIRequestschema correctly models:
apias a required string containing JSON-encoded API details.urlanddefinitionas alternative OpenAPI sources viaanyOf(at least one required).- Multipart usage consistent with the path’s
multipart/form-datarequestBody.Two minor polish suggestions:
- The description currently says “Either
urlordefinitionmust be provided…”. Since the backend allows both and prefers the file when both are present, consider adding a line like: “If both are provided, the uploadeddefinitionfile takes precedence overurl.”- To help client authors, you might add
format: json(custom format) or similar onapito hint that the string is JSON-encoded API metadata.These tweaks keep the contract aligned with the actual behavior and make the intent clearer to tooling and consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
platform-api/src/internal/dto/openapi.go(1 hunks)platform-api/src/internal/handler/api.go(4 hunks)platform-api/src/internal/service/api.go(1 hunks)platform-api/src/internal/service/gateway_test.go(2 hunks)platform-api/src/internal/utils/api.go(1 hunks)platform-api/src/resources/openapi.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- platform-api/src/internal/handler/api.go
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: thivindu
Repo: wso2/api-platform PR: 142
File: platform-api/src/resources/openapi.yaml:905-969
Timestamp: 2025-11-12T08:52:52.909Z
Learning: In the wso2/api-platform repository, the team follows an API-first development approach where OpenAPI specs may document planned features before backend implementation is complete, allowing frontend development to proceed against the intended API contract without requiring changes later.
🧬 Code graph analysis (3)
platform-api/src/internal/service/api.go (2)
platform-api/src/internal/dto/openapi.go (1)
ImportOpenAPIRequest(40-44)platform-api/src/internal/dto/api.go (1)
API(25-51)
platform-api/src/internal/service/gateway_test.go (1)
platform-api/src/internal/constants/constants.go (1)
GatewayFunctionalityTypeRegular(53-53)
platform-api/src/internal/utils/api.go (2)
platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/model/api.go (2)
API(25-51)API(54-56)
3 participants
Purpose
The Management Portal requires API creation via Contract Flow via URL and OpenAPI definition file upload. Therefore, the provided OpenAPI definition needs to be validated, parsed and a API needs to be created using the API metadata sent from the frontend and the provided OpenAPI definition.
Fixes: wso2/api-platform/issues#161
Goals
Validate, parse and create an API from the provided API definition and the API metadata.
Approach
This PR adds a new endpoint, request/response models, service logic, and OpenAPI documentation for this functionality. The changes ensure that user-provided API details are merged with those parsed from the OpenAPI definition, with robust error handling and validation throughout the process.
New OpenAPI Import Feature
ImportOpenAPIRequeststruct inopenapi.goto represent the import request, supporting both URL and file upload for the OpenAPI definition, along with required API details.ImportOpenAPIhandler inapi.goto process multipart form requests, validate inputs, parse API details from JSON, and handle error scenarios (such as missing fields, invalid formats, and known conflicts)./api/v1/import/open-apiPOST route in the API handler.Service and Utility Enhancements
ImportFromOpenAPImethod toAPIServiceinapi.go, which fetches/parses the OpenAPI definition, validates it, merges user and extracted API details, and creates the API using existing logic.api.gofor validating/parsing OpenAPI content (ValidateAndParseOpenAPI) and merging user-provided and extracted API details (MergeAPIDetails).Documentation and OpenAPI Spec Updates
/import/open-apiendpoint in the OpenAPI spec, including request/response schemas, error responses, and usage details. [1] [2]Summary by CodeRabbit
New Features
Documentation