add swagger2.0#92
Merged
Merged
Conversation
Contributor
Reviewer's GuideThis pull request adds full Swagger 2.0 import support by introducing a new Converter in pkg/swagger (with JSON/YAML wrappers and comprehensive unit tests), wiring up a Swagger import HTTP handler and route in the API server, defining corresponding i18n messages, and updating module dependencies. Key OpenAPI tests are also relaxed by commenting out redundant assertions. Sequence Diagram for Swagger 2.0 Import ProcesssequenceDiagram
actor User
participant GinRouter as Gin Router
participant SwaggerHandler as handler.Swagger
participant SwaggerConverter as swagger.Converter
participant Store as storage.Store
participant Notifier as notifier.Notifier
User->>+GinRouter: POST /swagger/import (multipart/form-data with file)
GinRouter->>+SwaggerHandler: HandleImport(context)
SwaggerHandler->>SwaggerHandler: Get file from request
SwaggerHandler->>SwaggerConverter: NewConverter()
SwaggerHandler->>+SwaggerConverter: Convert(fileContent)
SwaggerConverter->>SwaggerConverter: Parse Swagger spec (loads.Analyzed)
SwaggerConverter->>SwaggerConverter: Transform to MCPConfig
SwaggerConverter-->>-SwaggerHandler: *config.MCPConfig
SwaggerHandler->>+Store: Create(context, mcpConfig)
Store-->>-SwaggerHandler: (success/error)
SwaggerHandler->>+Notifier: NotifyUpdate(context, mcpConfig)
Notifier-->>-SwaggerHandler: (success/error)
SwaggerHandler-->>-GinRouter: HTTP Response (success/error)
GinRouter-->>-User: HTTP Response
Class Diagram for Swagger Import ComponentsclassDiagram
namespace handler {
class Swagger {
-db database.Database
-store storage.Store
-notifier notifier.Notifier
-logger *zap.Logger
+NewSwagger(db, store, ntf, logger) *Swagger
+HandleImport(c *gin.Context)
}
}
namespace swagger {
class Converter {
+NewConverter() *Converter
+Convert(specData []byte) (*config.MCPConfig, error)
+ConvertFromJSON(jsonData []byte) (*config.MCPConfig, error)
+ConvertFromYAML(yamlData []byte) (*config.MCPConfig, error)
}
}
namespace config {
class MCPConfig {
+Name string
+Tenant string
+CreatedAt time.Time
+UpdatedAt time.Time
+Routers []RouterConfig
+Servers []ServerConfig
+Tools []ToolConfig
}
class ServerConfig {
+Name string
+Description string
+Config map[string]string
+AllowedTools []string
}
class RouterConfig {
+Server string
+Prefix string
+CORS *CORSConfig
}
class ToolConfig {
+Name string
+Description string
+Method string
+Endpoint string
+Headers map[string]string
+Args []ArgConfig
+RequestBody string
+ResponseBody string
}
class ArgConfig {
+Name string
+Position string
+Required bool
+Type string
+Description string
+Default string
}
class CORSConfig {
+AllowOrigins []string
+AllowMethods []string
+AllowHeaders []string
+ExposeHeaders []string
+AllowCredentials bool
}
}
handler.Swagger ..> swagger.Converter : uses
handler.Swagger ..> storage.Store : uses
handler.Swagger ..> notifier.Notifier : uses
handler.Swagger ..> database.Database : uses
swagger.Converter ..> config.MCPConfig : creates
config.MCPConfig *-- "0..*" config.ServerConfig : contains
config.MCPConfig *-- "0..*" config.RouterConfig : contains
config.MCPConfig *-- "0..*" config.ToolConfig : contains
config.RouterConfig *-- "1" config.CORSConfig : contains
config.ToolConfig *-- "0..*" config.ArgConfig : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @KamToHung - I've reviewed your changes - here's some feedback:
- The huge Convert method could be split into smaller helper functions (e.g. building server config, router config, and tools) to improve readability and maintainability.
- The use of lol.RandomString for name/route prefixes introduces unpredictability—consider making prefixes deterministic or configurable (and replacing the lol dependency with a small internal utility).
- There are multiple commented‐out assertions in the existing converter tests—either restore those checks or remove them to keep the test suite clear and up to date.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // Convert converts Swagger 2.0 specification to MCP configuration | ||
| func (c *Converter) Convert(specData []byte) (*config.MCPConfig, error) { | ||
| // Parse Swagger specification | ||
| doc, err := loads.Analyzed(specData, "") |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Incorrect use of loads.Analyzed for raw bytes
loads.Analyzed requires a *loads.Document, not raw []byte. First call loads.JSONSpec, loads.YAMLSpec, or loads.SpecFromData to get a Document, then pass it to loads.Analyzed.
|
|
||
| swaggerSpec := doc.Spec() | ||
|
|
||
| rs := lol.RandomString(4) |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Use a stronger unique ID generator
A 4-character random string risks collisions on large imports. Use a stronger generator like ULID or UUID (e.g., oklog/ulid or google/uuid) for non-colliding IDs.
Contributor
Author
|
PTAL |
Member
|
Awesome! LGTM! Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce full Swagger 2.0 support by adding a converter, handler, API route, tests, dependency updates, and associated i18n messages
New Features:
Enhancements:
Build:
Tests:
Chores: