Implement validate and import API project from GitHub Repo #150
Conversation
WalkthroughAdds Git-backed API project import and validation: new DTOs, handlers, service and git provider methods to fetch and parse repo files, OpenAPI/WSO2 validation and YAML→DTO utilities, new error constants, OpenAPI spec endpoints, and go.mod updates for OpenAPI/YAML tooling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Handler as HTTP Handler
participant Service as API Service
participant GitSvc as Git Service
participant Provider as Git Provider Client
participant Util as Utils
Client->>Handler: POST /import or /validate\n(repoUrl, branch, path, ...)
Handler->>Handler: validate request & auth
Handler->>GitSvc: create provider-backed Git service
alt Import flow
Handler->>Service: ImportAPIProject(req, orgId, gitSvc)
Service->>GitSvc: ValidateAPIProject(repoUrl, branch, path)
GitSvc->>Provider: FetchFileContent(.api-platform/config.yaml)
Provider-->>GitSvc: config bytes
GitSvc->>GitSvc: parse config -> APIProjectConfig
GitSvc->>Provider: FetchFileContent(openapi path / wso2 artifact)
Provider-->>GitSvc: file bytes
GitSvc->>Util: parse artifact YAML -> APIDeploymentYAML
Service->>Util: ValidateOpenAPIDefinition(openapi bytes)
Service->>Util: ValidateWSO2Artifact(artifact)
Service->>Service: mergeAPIData(userInput, artifact)
Service->>Service: CreateAPI(merged) -> created API
Service-->>Handler: created API (201)
Handler-->>Client: 201 Created
else Validate flow
Handler->>Service: ValidateAndRetrieveAPIProject(req, gitSvc)
Service->>GitSvc: ValidateAPIProject(...)
GitSvc->>Provider: fetch referenced files
GitSvc->>Util: parse & validate YAML
Service->>Util: ValidateAPIDefinitionConsistency(...)
Service-->>Handler: APIProjectValidationResponse (200)
Handler-->>Client: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
platform-api/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
platform-api/src/go.mod(2 hunks)platform-api/src/internal/constants/error.go(1 hunks)platform-api/src/internal/dto/api-project.go(1 hunks)platform-api/src/internal/dto/api.go(1 hunks)platform-api/src/internal/handler/api.go(2 hunks)platform-api/src/internal/service/api.go(3 hunks)platform-api/src/internal/service/git.go(2 hunks)platform-api/src/internal/service/git_bitbucket_client.go(2 hunks)platform-api/src/internal/service/git_github_client.go(2 hunks)platform-api/src/internal/service/git_gitlab_client.go(1 hunks)platform-api/src/internal/service/git_provider.go(1 hunks)platform-api/src/internal/utils/api.go(2 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/dto/api.goplatform-api/src/internal/service/api.go
📚 Learning: 2025-11-05T05:26:25.128Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 101
File: platform-api/src/internal/handler/project.go:169-169
Timestamp: 2025-11-05T05:26:25.128Z
Learning: In platform-api/src/internal/handler/project.go UpdateProject handler, the UpdateProjectRequest allows empty description field by design. This is intentional to provide a means to clear/empty the description of a project. The frontend is responsible for populating the existing description value when the user is not modifying it, implementing a full-replacement update pattern rather than partial/PATCH-style updates.
Applied to files:
platform-api/src/internal/handler/api.goplatform-api/src/internal/dto/api-project.go
🧬 Code graph analysis (5)
platform-api/src/internal/handler/api.go (6)
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/api-project.go (2)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/service/git.go (1)
NewGitService(40-48)platform-api/src/internal/constants/error.go (10)
ErrAPIProjectNotFound(85-85)ErrMalformedAPIProject(86-86)ErrInvalidAPIProject(87-87)ErrConfigFileNotFound(88-88)ErrOpenAPIFileNotFound(89-89)ErrWSO2ArtifactNotFound(90-90)ErrAPIAlreadyExists(41-41)ErrProjectNotFound(33-33)ErrInvalidAPIName(44-44)ErrInvalidAPIContext(42-42)
platform-api/src/internal/service/git.go (3)
platform-api/src/internal/dto/api-project.go (1)
APIProjectConfig(30-34)platform-api/src/internal/dto/api.go (1)
APIDeploymentYAML(183-187)platform-api/src/internal/service/git_provider.go (1)
ParseRepositoryURL(80-116)
platform-api/src/internal/service/api.go (4)
platform-api/src/internal/dto/api-project.go (3)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)APIProjectValidationResponse(61-67)platform-api/src/internal/service/git.go (1)
GitService(26-33)platform-api/src/internal/dto/api.go (3)
API(25-51)APIYAMLData2(190-200)APIDeploymentYAML(183-187)platform-api/src/internal/constants/error.go (4)
ErrAPIProjectNotFound(85-85)ErrMalformedAPIProject(86-86)ErrInvalidAPIProject(87-87)ErrWSO2ArtifactNotFound(90-90)
platform-api/src/internal/utils/api.go (2)
platform-api/src/internal/dto/api.go (5)
APIDeploymentYAML(183-187)API(25-51)APIYAMLData2(190-200)Operation(134-138)OperationRequest(141-148)platform-api/src/internal/dto/backend_service.go (2)
BackendService(23-36)BackendEndpoint(39-45)
platform-api/src/internal/dto/api-project.go (1)
platform-api/src/internal/dto/api.go (1)
API(25-51)
🔇 Additional comments (14)
platform-api/src/go.mod (3)
3-5: LGTM!The Go directive and toolchain specifications enhance build reproducibility.
8-11: LGTM!The new OpenAPI dependencies support the API project validation and import features introduced in this PR.
26-75: LGTM!Indirect dependencies and version updates are appropriate for the OpenAPI validation tooling and appear to include routine security/bug fix updates.
platform-api/src/internal/service/git_gitlab_client.go (1)
261-265: LGTM!The placeholder implementation clearly indicates the limitation. The TODO and error message appropriately guide users to GitHub for this feature.
platform-api/src/internal/service/git_provider.go (1)
41-41: LGTM!The interface extension is well-designed and consistent with existing methods. All implementing clients provide the required method.
platform-api/src/internal/service/git_bitbucket_client.go (2)
23-23: LGTM!The io import is required for reading the response body.
119-148: LGTM!The implementation correctly uses the Bitbucket API v2 endpoint, handles errors comprehensively, and properly manages the response lifecycle.
platform-api/src/internal/dto/api.go (1)
184-186: LGTM!The required validation tags ensure the API deployment YAML structure is complete, supporting robust validation in the import workflow.
platform-api/src/internal/service/git_github_client.go (2)
21-24: LGTM!The new imports are required for decoding base64 content and reading large file downloads.
218-282: LGTM!The implementation correctly handles both GitHub API content delivery methods: base64-encoded inline content for small files and raw download URLs for large files. Error handling is comprehensive and resource management is proper.
platform-api/src/internal/handler/api.go (3)
601-717: LGTM!The ImportAPIProject handler follows established patterns with comprehensive validation and error handling. The creation of a new GitService instance per request ensures clean state, and the 201 Created response is appropriate for API creation.
719-757: LGTM!The ValidateAPIProject handler appropriately returns 200 OK with validation results in the response body, allowing the client to distinguish between validation failures and service errors. The absence of organization context checking is correct since validation doesn't require organizational access.
776-783: LGTM!The route registrations follow the existing pattern and provide clean, extensible endpoints for API project operations.
platform-api/src/internal/constants/error.go (1)
84-90: LGTM!The new error constants are well-defined with clear messages and align with the error handling in the ImportAPIProject handler.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
platform-api/src/internal/service/git.go (3)
144-145: Usepathpkg.Joinfor cleaner path construction.String concatenation for building file paths (e.g.,
path + "/.api-platform") is less idiomatic than usingpathpkg.Join, which handles edge cases like trailing slashes automatically.Apply this diff to use
pathpkg.Join:- apiPlatformPath := path + "/.api-platform" - configContent, err := s.FetchFileContent(repoURL, branch, apiPlatformPath+"/config.yaml") + configPath := pathpkg.Join(path, ".api-platform", "config.yaml") + configContent, err := s.FetchFileContent(repoURL, branch, configPath)- openAPIPath := path + "/" + api.OpenAPI + openAPIPath := pathpkg.Join(path, api.OpenAPI) _, err := s.FetchFileContent(repoURL, branch, openAPIPath)- wso2ArtifactPath := path + "/" + api.WSO2Artifact + wso2ArtifactPath := pathpkg.Join(path, api.WSO2Artifact) _, err = s.FetchFileContent(repoURL, branch, wso2ArtifactPath)Also applies to: 167-173
161-178: Consider adding API context to error messages.When validating multiple APIs in a loop, including the array index or an API identifier in error messages would make debugging easier if validation fails for one entry.
Example enhancement:
- for _, api := range config.APIs { + for i, api := range config.APIs { if api.OpenAPI == "" || api.WSO2Artifact == "" { - return nil, fmt.Errorf("malformed api project: apis.openapi and apis.wso2Artifact fields are required") + return nil, fmt.Errorf("malformed api project: apis[%d].openapi and apis[%d].wso2Artifact fields are required", i, i) } openAPIPath := path + "/" + api.OpenAPI _, err := s.FetchFileContent(repoURL, branch, openAPIPath) if err != nil { - return nil, fmt.Errorf("invalid api project: openapi file not found: %s", api.OpenAPI) + return nil, fmt.Errorf("invalid api project: apis[%d] openapi file not found: %s", i, api.OpenAPI) }
183-196: Include path in error messages for easier debugging.Adding the
pathparameter to error messages would provide better context when troubleshooting fetch or parse failures.Example:
content, err := s.FetchFileContent(repoURL, branch, path) if err != nil { - return nil, fmt.Errorf("failed to fetch WSO2 artifact file: %w", err) + return nil, fmt.Errorf("failed to fetch WSO2 artifact file at %s: %w", path, err) } var artifact dto.APIDeploymentYAML if err := yaml.Unmarshal(content, &artifact); err != nil { - return nil, fmt.Errorf("failed to parse WSO2 artifact file: %w", err) + return nil, fmt.Errorf("failed to parse WSO2 artifact file at %s: %w", path, err) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platform-api/src/internal/service/git.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
platform-api/src/internal/service/git.go (3)
platform-api/src/internal/dto/api-project.go (1)
APIProjectConfig(30-34)platform-api/src/internal/dto/api.go (1)
APIDeploymentYAML(183-187)platform-api/src/internal/service/git_provider.go (1)
ParseRepositoryURL(80-116)
🔇 Additional comments (1)
platform-api/src/internal/service/git.go (1)
114-139: Path normalization correctly implemented.The path normalization on lines 128-130 properly addresses the repo-root project issue flagged in the previous review. The combination of
TrimSpace,TrimPrefix, andpathpkg.Cleanensures that empty strings, ".", and paths with leading slashes are handled correctly before calling the provider API.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platform-api/src/internal/service/git.go(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-12T08:52:52.909Z
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.
Applied to files:
platform-api/src/internal/service/git.go
🧬 Code graph analysis (1)
platform-api/src/internal/service/git.go (3)
platform-api/src/internal/dto/api-project.go (1)
APIProjectConfig(30-34)platform-api/src/internal/dto/api.go (1)
APIDeploymentYAML(183-187)platform-api/src/internal/service/git_provider.go (1)
ParseRepositoryURL(80-116)
🔇 Additional comments (2)
platform-api/src/internal/service/git.go (2)
114-139: LGTM! Path normalization correctly handles repo-root projects.The implementation properly normalizes paths (trim whitespace, remove leading slashes, clean duplicates) before forwarding to the provider client, which resolves the repo-root project issue flagged in the previous review.
141-181: Consider deeper validation of OpenAPI and WSO2 artifact content.The PR objectives mention "validate them (including OpenAPI definitions and api.yaml)," but the current implementation only checks file existence, not content validity. While this may be sufficient for the current scope, consider whether you need to:
- Parse and validate OpenAPI spec structure/schema
- Validate WSO2 artifact YAML structure against expected schema
- Check consistency between OpenAPI and WSO2 artifact
If content validation is planned for a future PR, this is fine as-is. Otherwise, you may want to extend validation now.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
platform-api/src/internal/service/git.go (1)
205-208: Consider validating the Data field for completeness.While
KindandVersionare validated, theDatafield (also marked asbinding:"required"in the DTO) is not explicitly checked. SinceDatais a struct type, it will always be present after unmarshaling, but its fields might be empty or invalid. Consider adding validation to ensureDataand its essential fields are properly populated, or verify that this validation occurs downstream in the import workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platform-api/src/internal/service/git.go(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-12T08:52:52.909Z
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.
Applied to files:
platform-api/src/internal/service/git.go
🧬 Code graph analysis (1)
platform-api/src/internal/service/git.go (3)
platform-api/src/internal/dto/api-project.go (1)
APIProjectConfig(30-34)platform-api/src/internal/dto/api.go (1)
APIDeploymentYAML(183-187)platform-api/src/internal/service/git_provider.go (1)
ParseRepositoryURL(80-116)
🔇 Additional comments (2)
platform-api/src/internal/service/git.go (2)
114-139: Path normalization correctly implemented.The normalization logic properly handles edge cases including repo root (
"",".","/"), leading slashes, and duplicate separators before delegating to the provider.
141-191: Path traversal prevention correctly implemented.The sanitization logic properly cleans paths and rejects traversal attempts (
".."prefixes, absolute paths) before file operations, preventing unauthorized repository access.
thivindu
force-pushed
the
integrate-github
branch
from
d638ded to
8c10bda
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
platform-api/src/internal/service/git_provider.go (1)
41-41: Add documentation for the new interface method.The new
FetchFileContentmethod lacks a documentation comment. Public interface methods should include comments describing their purpose and parameters.Apply this diff to add documentation:
+ // FetchFileContent fetches the raw content of a file at the specified path from a repository branch FetchFileContent(owner, repo, branch, path string) ([]byte, error)platform-api/src/internal/handler/api.go (1)
628-663: Consider consolidating validation logic.The handler performs extensive manual validation after JSON binding, but many of these fields already have
binding:"required"tags in the DTO (RepoURL, Branch, Path, API). The nested API fields (Name, Context, Version, ProjectID) lack binding tags but are validated here.Consider either:
- Add
binding:"required"tags to the nested API struct fields and remove redundant handler checks- Remove DTO binding tags and centralize all validation in the handler
Mixing both approaches increases maintenance burden and risk of inconsistency.
platform-api/src/internal/service/api.go (1)
165-166: Consider retrieving timestamps from the database model.Setting timestamps on the DTO after database insertion may create a slight discrepancy with the actual database values. Consider either retrieving the created API from the database to get accurate timestamps, or ensuring the repository returns the model with timestamps set.
Example approach:
// Create API in repository if err := s.apiRepo.CreateAPI(apiModel); err != nil { return nil, fmt.Errorf("failed to create api: %w", err) } - api.CreatedAt = time.Now() - api.UpdatedAt = time.Now() + // Retrieve the created API to get accurate timestamps from DB + createdModel, err := s.apiRepo.GetAPIByUUID(apiId) + if err != nil { + return nil, fmt.Errorf("failed to retrieve created api: %w", err) + } + api.CreatedAt = createdModel.CreatedAt + api.UpdatedAt = createdModel.UpdatedAtplatform-api/src/internal/service/git.go (1)
32-34: Add documentation for new interface methods.The new interface methods lack documentation comments. Public interface methods should describe their purpose, parameters, and return values.
Apply this diff:
+ // FetchFileContent retrieves the raw content of a file at the specified path from a repository branch FetchFileContent(repoURL, branch, path string) ([]byte, error) + // ValidateAPIProject validates the structure and content of an API project at the specified path ValidateAPIProject(repoURL, branch, path string) (*dto.APIProjectConfig, error) + // FetchWSO2Artifact retrieves and parses the WSO2 API artifact file at the specified path FetchWSO2Artifact(repoURL, branch, path string) (*dto.APIDeploymentYAML, error)platform-api/src/internal/utils/api.go (1)
1072-1091: Consider consolidating version-specific validators.
ValidateOpenAPI3andValidateSwagger2duplicate the logic ofValidateOpenAPIDefinition, which already auto-detects and validates both versions. Unless callers need explicit version-specific validation, consider deprecating these in favor of the unifiedValidateOpenAPIDefinitionmethod to reduce API surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
platform-api/src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
platform-api/src/go.mod(3 hunks)platform-api/src/internal/constants/error.go(1 hunks)platform-api/src/internal/dto/api-project.go(1 hunks)platform-api/src/internal/dto/api.go(1 hunks)platform-api/src/internal/handler/api.go(2 hunks)platform-api/src/internal/service/api.go(3 hunks)platform-api/src/internal/service/git.go(2 hunks)platform-api/src/internal/service/git_bitbucket_client.go(2 hunks)platform-api/src/internal/service/git_github_client.go(2 hunks)platform-api/src/internal/service/git_gitlab_client.go(1 hunks)platform-api/src/internal/service/git_provider.go(1 hunks)platform-api/src/internal/utils/api.go(2 hunks)platform-api/src/resources/openapi.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- platform-api/src/internal/constants/error.go
- platform-api/src/resources/openapi.yaml
- platform-api/src/internal/service/git_gitlab_client.go
- platform-api/src/go.mod
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-05T05:26:25.128Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 101
File: platform-api/src/internal/handler/project.go:169-169
Timestamp: 2025-11-05T05:26:25.128Z
Learning: In platform-api/src/internal/handler/project.go UpdateProject handler, the UpdateProjectRequest allows empty description field by design. This is intentional to provide a means to clear/empty the description of a project. The frontend is responsible for populating the existing description value when the user is not modifying it, implementing a full-replacement update pattern rather than partial/PATCH-style updates.
Applied to files:
platform-api/src/internal/handler/api.goplatform-api/src/internal/dto/api-project.go
📚 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/dto/api.goplatform-api/src/internal/service/api.go
📚 Learning: 2025-11-12T08:52:52.909Z
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.
Applied to files:
platform-api/src/internal/service/git.go
🧬 Code graph analysis (5)
platform-api/src/internal/handler/api.go (6)
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/api-project.go (2)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/service/git.go (1)
NewGitService(42-50)platform-api/src/internal/constants/error.go (10)
ErrAPIProjectNotFound(88-88)ErrMalformedAPIProject(89-89)ErrInvalidAPIProject(90-90)ErrConfigFileNotFound(91-91)ErrOpenAPIFileNotFound(92-92)ErrWSO2ArtifactNotFound(93-93)ErrAPIAlreadyExists(41-41)ErrProjectNotFound(33-33)ErrInvalidAPIName(44-44)ErrInvalidAPIContext(42-42)
platform-api/src/internal/dto/api-project.go (1)
platform-api/src/internal/dto/api.go (1)
API(25-51)
platform-api/src/internal/utils/api.go (2)
platform-api/src/internal/dto/api.go (5)
APIDeploymentYAML(183-187)API(25-51)APIYAMLData2(190-200)Operation(134-138)OperationRequest(141-148)platform-api/src/internal/dto/backend_service.go (2)
BackendService(23-36)BackendEndpoint(39-45)
platform-api/src/internal/service/api.go (4)
platform-api/src/internal/dto/api-project.go (3)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)APIProjectValidationResponse(61-67)platform-api/src/internal/service/git.go (1)
GitService(28-35)platform-api/src/internal/dto/api.go (3)
API(25-51)APIYAMLData2(190-200)APIDeploymentYAML(183-187)platform-api/src/internal/constants/error.go (4)
ErrAPIProjectNotFound(88-88)ErrMalformedAPIProject(89-89)ErrInvalidAPIProject(90-90)ErrWSO2ArtifactNotFound(93-93)
platform-api/src/internal/service/git.go (3)
platform-api/src/internal/dto/api-project.go (1)
APIProjectConfig(30-34)platform-api/src/internal/dto/api.go (1)
APIDeploymentYAML(183-187)platform-api/src/internal/service/git_provider.go (1)
ParseRepositoryURL(80-116)
🔇 Additional comments (17)
platform-api/src/internal/service/git_bitbucket_client.go (1)
119-148: LGTM! FetchFileContent implementation is consistent.The implementation follows the established patterns for HTTP client methods in this file, with appropriate error handling for different status codes and proper resource cleanup.
platform-api/src/internal/service/git_github_client.go (1)
218-282: LGTM! GitHub FetchFileContent handles both small and large files.The implementation correctly handles GitHub's dual content delivery modes: base64-encoded inline content for small files and raw download URLs for large files. The newline removal before base64 decoding is appropriate for GitHub's API response format.
platform-api/src/internal/dto/api.go (1)
184-186: LGTM! Required field validation is properly enforced.The
binding:"required"tags ensure validation when this struct is bound from HTTP requests. The explicit validation afteryaml.Unmarshalis correctly implemented inplatform-api/src/internal/service/git.go(lines 205-208), addressing the limitation that Gin binding tags don't apply during YAML unmarshaling.platform-api/src/internal/handler/api.go (2)
731-769: LGTM! ValidateAPIProject follows appropriate validation endpoint pattern.The handler correctly returns 200 OK with validation results in the response body (even when validation fails), reserving 500 for actual service errors. This is appropriate for a validation endpoint that should always return structured results. Note that this endpoint does not require authentication, enabling pre-flight validation before import.
788-795: LGTM! Route registration is well-organized.The new endpoint groups follow RESTful conventions and maintain consistency with the existing API structure.
platform-api/src/internal/service/api.go (3)
951-1011: LGTM! ImportAPIProject follows a clean validation-then-creation flow.The method properly validates the project structure, fetches artifacts, merges data with user input, and delegates to the existing CreateAPI flow. The note about processing only the first API (line 968-971) appropriately documents the current limitation with a path for future enhancement.
1052-1062: Verify boolean and zero-value handling semantics.The merge logic unconditionally copies boolean fields from user data (lines 1052-1055), meaning
falsealways overwrites the artifact value. ForRevisionID, the check!= 0means a user-supplied value of0would be ignored (line 1060).Ensure this behavior aligns with your API semantics:
- Should explicit
falsevalues from the user override artifacttrue?- Is
RevisionID: 0a valid value that should be preserved?If not, consider documenting this behavior or using pointers to distinguish between "not provided" and explicit zero/false values.
1067-1142: LGTM! ValidateAPIProject provides comprehensive multi-layered validation.The method performs thorough validation across config structure, OpenAPI definitions, WSO2 artifacts, and cross-consistency checks. Collecting all errors before returning (rather than failing fast) provides better UX by showing all issues at once.
platform-api/src/internal/dto/api-project.go (1)
20-67: LGTM! API project DTOs are well-structured for their use cases.The DTOs appropriately use
yamltags for config parsing andjson/bindingtags for HTTP request/response handling. TheSpectralRulesetstruct (lines 44-50) is currently unused but provides a clear extension point for future Spectral linting integration.platform-api/src/internal/service/git.go (3)
114-139: LGTM! FetchFileContent properly normalizes file paths.The path normalization (lines 128-130) correctly handles repository root projects and prevents issues with leading slashes or "." paths. This addresses the concern from previous reviews.
141-191: LGTM! ValidateAPIProject implements proper path traversal protection.The path sanitization logic (lines 166-174) correctly prevents path traversal attacks by:
- Cleaning paths with
pathpkg.Clean- Rejecting paths starting with
".."- Rejecting absolute paths
This ensures users cannot access files outside the intended project directory.
193-211: LGTM! FetchWSO2Artifact includes proper post-unmarshal validation.The explicit validation of required fields (lines 205-208) after YAML unmarshaling correctly addresses the limitation that Gin's
binding:"required"tags don't apply toyaml.Unmarshal.platform-api/src/internal/utils/api.go (5)
25-27: LGTM! Good library choice for OpenAPI validation.The use of
libopenapiaddresses the concern raised in the past review comment about OpenAPI 3.1 support and provides broader spec coverage (3.0, 3.1, 3.2).
943-1026: LGTM! Comprehensive OpenAPI validation with proper version detection.The validation logic correctly handles both OpenAPI 3.x and Swagger 2.0, with a robust fallback mechanism when version detection fails. Required field validation covers the specification minimums.
1029-1069: LGTM! Proper Swagger 2.0 validation.The validation correctly checks required fields and ensures the swagger version is valid (2.x).
1111-1132: LGTM! Version consistency check is appropriate.The function correctly validates that OpenAPI and WSO2 artifact versions match. Note that
yaml.Unmarshalcan handle both YAML and JSON formats (since YAML is a superset of JSON), so this works for both.
876-889: Clarify intent: Are operation request fields intentionally dropped during conversion?The conversion at lines 893–906 copies only
MethodandPathfromyamlData.Operations[i]to the outputOperationRequest. However, the inputOperationRequeststruct includesBackendServices,Authentication,RequestPolicies, andResponsePoliciesfields that are silently discarded during conversion. Confirm whether this field loss is intentional simplification for the simplified YAML format or an oversight.The BackendService 1:1 mapping (lines 876–889) appears intentional for the simplified upstream format and mirrors the pattern in the service layer.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
platform-api/src/internal/handler/api.go (2)
628-663: Redundant manual validation with binding tags.The request struct
dto.ImportAPIProjectRequestalready usesbinding:"required"tags forRepoURL,Branch,Path, and the nestedAPIfields (see dto/api-project.go lines 20-26 and dto/api.go lines 24-50). Gin'sShouldBindJSONat line 623 automatically validates these, making the manual checks at lines 629-663 redundant.Consider removing the manual validation to reduce duplication:
var req dto.ImportAPIProjectRequest if err := c.ShouldBindJSON(&req); err != nil { c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", err.Error())) return } - // Validate required fields - if req.RepoURL == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Repository URL is required")) - return - } - if req.Branch == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Branch is required")) - return - } - if req.Path == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Path is required")) - return - } - if req.API.Name == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "API name is required")) - return - } - if req.API.Context == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "API context is required")) - return - } - if req.API.Version == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "API version is required")) - return - } - if req.API.ProjectID == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Project ID is required")) - return - } - // Create Git service gitService := service.NewGitService()If you prefer custom error messages, update the DTO binding tags to use custom messages or keep the manual validation and remove the
binding:"required"tags.
739-754: Redundant manual validation with binding tags.Similar to
ImportAPIProject, thedto.ValidateAPIProjectRequeststruct usesbinding:"required"tags forRepoURL,Branch, andPath(see dto/api-project.go lines 52-57). The manual checks at lines 740-754 duplicate Gin's automatic validation.Apply this diff to remove the redundant validation:
var req dto.ValidateAPIProjectRequest if err := c.ShouldBindJSON(&req); err != nil { c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", err.Error())) return } - // Validate required fields - if req.RepoURL == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Repository URL is required")) - return - } - if req.Branch == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Branch is required")) - return - } - if req.Path == "" { - c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", - "Path is required")) - return - } - // Create Git service gitService := service.NewGitService()platform-api/src/internal/utils/api.go (1)
1100-1122: LGTM! Version consistency check is a good starting point.The method validates that the version field matches between the OpenAPI definition and WSO2 artifact, preventing common misconfigurations. While additional field checks (e.g., name) could be added later, version consistency is the most critical validation.
Consider adding name validation for additional consistency:
// Check name consistency (optional) if title, exists := info["title"].(string); exists { if title != wso2Artifact.Data.Name { return fmt.Errorf("name mismatch between OpenAPI (%s) and WSO2 artifact (%s)", title, wso2Artifact.Data.Name) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platform-api/src/internal/handler/api.go(2 hunks)platform-api/src/internal/service/api.go(4 hunks)platform-api/src/internal/utils/api.go(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-05T05:26:25.128Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 101
File: platform-api/src/internal/handler/project.go:169-169
Timestamp: 2025-11-05T05:26:25.128Z
Learning: In platform-api/src/internal/handler/project.go UpdateProject handler, the UpdateProjectRequest allows empty description field by design. This is intentional to provide a means to clear/empty the description of a project. The frontend is responsible for populating the existing description value when the user is not modifying it, implementing a full-replacement update pattern rather than partial/PATCH-style updates.
Applied to files:
platform-api/src/internal/handler/api.go
📚 Learning: 2025-11-12T08:52:52.909Z
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.
Applied to files:
platform-api/src/internal/utils/api.go
📚 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 (3)
platform-api/src/internal/handler/api.go (6)
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/api-project.go (2)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)platform-api/src/internal/dto/api.go (1)
API(25-51)platform-api/src/internal/service/git.go (1)
NewGitService(42-50)platform-api/src/internal/constants/error.go (10)
ErrAPIProjectNotFound(88-88)ErrMalformedAPIProject(89-89)ErrInvalidAPIProject(90-90)ErrConfigFileNotFound(91-91)ErrOpenAPIFileNotFound(92-92)ErrWSO2ArtifactNotFound(93-93)ErrAPIAlreadyExists(41-41)ErrProjectNotFound(33-33)ErrInvalidAPIName(44-44)ErrInvalidAPIContext(42-42)
platform-api/src/internal/utils/api.go (3)
platform-api/src/internal/dto/api.go (5)
APIDeploymentYAML(183-187)API(25-51)APIYAMLData2(190-200)Operation(134-138)OperationRequest(141-148)platform-api/src/internal/model/api.go (6)
API(25-51)API(54-56)BackendService(131-143)BackendEndpoint(153-159)Operation(201-205)OperationRequest(208-215)platform-api/src/internal/dto/backend_service.go (2)
BackendService(23-36)BackendEndpoint(39-45)
platform-api/src/internal/service/api.go (5)
platform-api/src/internal/dto/api-project.go (3)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)APIProjectValidationResponse(61-67)platform-api/src/internal/service/git.go (1)
GitService(28-35)platform-api/src/internal/dto/api.go (3)
API(25-51)APIYAMLData2(190-200)APIDeploymentYAML(183-187)platform-api/src/internal/model/api.go (2)
API(25-51)API(54-56)platform-api/src/internal/constants/error.go (4)
ErrAPIProjectNotFound(88-88)ErrMalformedAPIProject(89-89)ErrInvalidAPIProject(90-90)ErrWSO2ArtifactNotFound(93-93)
🔇 Additional comments (8)
platform-api/src/internal/handler/api.go (2)
731-769: Consider whether organization context should be validated.
ImportAPIProjectvalidates organization context (lines 615-620), butValidateAPIProjectdoesn't. While validation might not require organization-specific access, this inconsistency could be confusing.Verify whether
ValidateAPIProjectshould enforce organization context for consistency and security. If validation should be organization-scoped (e.g., to prevent enumeration of other organizations' repositories), add the same context check as inImportAPIProject.
788-795: LGTM! Route registration is well-structured.The new route groups follow existing patterns and use appropriate HTTP methods. POST for validation is acceptable since it requires a request body.
platform-api/src/internal/service/api.go (2)
24-24: LGTM! New imports are appropriate.The
pathpkgalias for the standard librarypathpackage andgopkg.in/yaml.v3for YAML parsing are correctly added to support the new Git-based API project functionality.Also applies to: 36-36
1069-1147: LGTM! Comprehensive validation with appropriate error handling.The validation method follows a good pattern:
- Initializes response with default false/empty values
- Progressively validates project structure, OpenAPI definition, WSO2 artifact, and consistency
- Returns validation errors in the response body rather than as HTTP errors
- Uses path cleaning for security
The approach of returning validation errors in the response body (always returning
nilerror) aligns with the handler's design to return 200 OK with validation details.platform-api/src/internal/utils/api.go (4)
25-27: LGTM! Library choice addresses OpenAPI 3.1 support.The
pb33f/libopenapilibrary was chosen to support OpenAPI 3.0, 3.1, and 3.2 specifications, addressing the limitation ofkin-openapinoted in past review comments.
867-942: LGTM! Conversion logic is well-structured with clear documentation.The
APIYAMLData2ToDTOmethod:
- Properly converts Upstreams to BackendServices (each upstream becomes a backend service with a single endpoint)
- Marks the first backend service as default (reasonable convention)
- Copies all operation fields (past review comment was addressed)
- Sets sensible defaults for required fields
- Clearly documents which fields must be set by the caller
947-1073: LGTM! OpenAPI validation is comprehensive and robust.The validation implementation:
- Intelligently routes based on version string with fallback to structure detection
- Validates both OpenAPI 3.x and Swagger 2.0 formats
- Checks essential required fields (info.title, info.version, swagger version for 2.0)
- Provides clear, specific error messages
- Handles edge cases where version detection is ambiguous
1075-1098: LGTM! WSO2 artifact validation now includes top-level fields.The validation properly checks both top-level required fields (
Kind,Version) and data fields (Name,Context,Version), addressing the concern raised in previous reviews.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
platform-api/src/internal/service/api.go (1)
1062-1064: Consider whether RevisionID=0 should be a valid user override.Line 1062 skips the merge when
userAPIData.RevisionID == 0, which prevents explicitly setting the revision ID to zero. If zero is a valid value, use a pointer field in the DTO to distinguish "not provided" from "explicitly zero."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
platform-api/src/internal/service/api.go(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-05T05:26:25.128Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 101
File: platform-api/src/internal/handler/project.go:169-169
Timestamp: 2025-11-05T05:26:25.128Z
Learning: In platform-api/src/internal/handler/project.go UpdateProject handler, the UpdateProjectRequest allows empty description field by design. This is intentional to provide a means to clear/empty the description of a project. The frontend is responsible for populating the existing description value when the user is not modifying it, implementing a full-replacement update pattern rather than partial/PATCH-style updates.
Applied to files:
platform-api/src/internal/service/api.go
📚 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 (4)
platform-api/src/internal/dto/api-project.go (3)
ImportAPIProjectRequest(21-27)ValidateAPIProjectRequest(53-58)APIProjectValidationResponse(61-67)platform-api/src/internal/service/git.go (1)
GitService(28-35)platform-api/src/internal/dto/api.go (3)
API(25-51)APIYAMLData2(190-200)APIDeploymentYAML(183-187)platform-api/src/internal/constants/error.go (4)
ErrAPIProjectNotFound(88-88)ErrMalformedAPIProject(89-89)ErrInvalidAPIProject(90-90)ErrWSO2ArtifactNotFound(93-93)
🔇 Additional comments (2)
platform-api/src/internal/service/api.go (2)
24-24: LGTM: Clean import additions.The aliased
pathimport and YAML library are appropriately scoped for the new Git-based API import functionality.Also applies to: 36-36
166-167: LGTM: Timestamps now reflect database values.This correctly resolves the previous review concern by copying the database-generated timestamps from the persisted model back to the DTO response.
2 participants
Purpose
Users should be able to create APIs from API projects from public GitHub repositories.
Related To:
wso2/api-platform/issues#146
wso2/api-platform/issues#147
Goals
Pull API project from public GitHub repository and validate the API project. Validation includes the OpenAPI definition and api.yaml. The response includes whether each of the definitions are valid and the overall validity of the API, the metadata of the API and the resources.
Import the API project from a Git repository. Merge the API details sent from the Management Portal and the content in the API project and create a API.
Approach
This pull request introduces the ability to import and validate API projects directly from Git repositories, with supporting DTOs, error handling, and service logic. It also updates dependencies and adds new API endpoints to facilitate these features.
API Project Import & Validation Features:
api-project.goto represent import/validation requests, config structures, and validation responses for API projects.ImportAPIProjectandValidateAPIProjecthandler functions inapi.go, including error handling for various project validation scenarios.POST /api/v1/import/api-projectandPOST /api/v1/validate/api-projectfor importing and validating API projects.error.goto standardize error responses.Service Layer Enhancements:
GitServiceinterface to support fetching file content, validating API projects, and retrieving WSO2 artifacts, enabling deeper integration with Git-based workflows.api.goservice to import API projects by merging WSO2 artifact data with user input, and to validate API projects with comprehensive checks (YAML/JSON, artifact structure, and consistency between definitions).Dependency Updates:
go.modfor OpenAPI, YAML, and related tooling, supporting new validation and parsing requirements. [1] [2]Minor Improvements:
APIDeploymentYAMLstruct to ensure proper unmarshalling and validation.Summary by CodeRabbit
New Features
Bug Fixes
Chores