-
Notifications
You must be signed in to change notification settings - Fork 103
Add tests for swagger datasource generation #6219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
77ab675
3418e92
b237068
8d5bf39
4471023
f593a8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,9 @@ package datasource | |
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
| "os" | ||
| "regexp" | ||
| "slices" | ||
| "strings" | ||
|
|
||
| "buf.build/go/protoyaml" | ||
|
|
@@ -63,12 +63,33 @@ func initDriverStruct() *minderv1.RestDataSource { | |
|
|
||
| // conver the title to a valid datasource name. It should only contain alphanumeric characters and dashes. | ||
| func swaggerTitleToDataSourceName(title string) string { | ||
| re := regexp.MustCompile("^[a-z][-_[:word:]]*$") | ||
| return re.ReplaceAllString(title, "-") | ||
| re := regexp.MustCompile(`[^a-z0-9_-]+`) | ||
| sanitized := strings.ToLower(strings.TrimSpace(title)) | ||
| sanitized = re.ReplaceAllString(sanitized, "-") | ||
| sanitized = strings.Trim(sanitized, "-") | ||
| if sanitized == "" { | ||
| return "datasource" | ||
| } | ||
| if sanitized[0] < 'a' || sanitized[0] > 'z' { | ||
| sanitized = "ds-" + sanitized | ||
| } | ||
|
|
||
| return sanitized | ||
| } | ||
|
|
||
| // swaggerToDataSource generates datasource code from an OpenAPI specification. | ||
| func swaggerToDataSource(cmd *cobra.Command, swagger *spec.Swagger) error { | ||
| // Ensure the generator respects Cobra's configured output writer. | ||
| if out := cmd.OutOrStdout(); out != os.Stdout { | ||
| if f, ok := out.(*os.File); ok { | ||
| prev := os.Stdout | ||
| os.Stdout = f | ||
| defer func() { | ||
| os.Stdout = prev | ||
| }() | ||
| } | ||
| } | ||
|
|
||
| if swagger.Info == nil { | ||
| return fmt.Errorf("info section is required in OpenAPI spec") | ||
| } | ||
|
|
@@ -84,14 +105,18 @@ func swaggerToDataSource(cmd *cobra.Command, swagger *spec.Swagger) error { | |
| } | ||
|
|
||
| for path, pathItem := range swagger.Paths.Paths { | ||
| p, err := url.JoinPath(basepath, path) | ||
| if err != nil { | ||
| cmd.PrintErrf("error joining path %s and basepath %s: %v\n Skipping", path, basepath, err) | ||
| continue | ||
| } | ||
| p := joinPaths(basepath, path) | ||
|
|
||
| for method, op := range operations(pathItem) { | ||
| opName := generateOpName(method, path) | ||
| if _, ok := drv.Def[opName]; ok { | ||
| return fmt.Errorf("duplicate generated operation name %q for %s %s", opName, method, path) | ||
| } | ||
|
|
||
| if err := validateParameters(op.Parameters); err != nil { | ||
| return fmt.Errorf("%s %s: %w", method, path, err) | ||
| } | ||
|
|
||
| // Create a new REST DataSource definition | ||
| def := &minderv1.RestDataSource_Def{ | ||
| Method: method, | ||
|
|
@@ -126,6 +151,16 @@ func swaggerToDataSource(cmd *cobra.Command, swagger *spec.Swagger) error { | |
| return writeDataSourceToFile(ds) | ||
| } | ||
|
|
||
| func joinPaths(basepath, path string) string { | ||
| basepath = strings.TrimSuffix(basepath, "/") | ||
| path = strings.TrimPrefix(path, "/") | ||
| if path == "" { | ||
| return basepath | ||
| } | ||
|
|
||
| return basepath + "/" + path | ||
| } | ||
|
Comment on lines
+154
to
+162
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // Generates an operation name for a data source. Note that these names | ||
| // must be unique within a data source. They also should be only alphanumeric | ||
| // characters and underscores | ||
|
|
@@ -158,6 +193,16 @@ func requiresMsgBody(method string) bool { | |
| return method == http.MethodPost || method == http.MethodPut || method == http.MethodPatch | ||
| } | ||
|
|
||
| func validateParameters(params []spec.Parameter) error { | ||
| for _, p := range params { | ||
| if !slices.Contains([]string{"path", "query"}, p.In) { | ||
| return fmt.Errorf("unsupported parameter %q in %q", p.Name, p.In) | ||
|
Comment on lines
+198
to
+199
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could support e.g. headers, but we don't at the moment, right? It seems worth a comment to indicate whether this is a limitation of the generation tool, or a limit of the underlying datasource API. |
||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func paramsToInputSchema(params []spec.Parameter) map[string]any { | ||
| if len(params) == 0 { | ||
| return nil | ||
|
|
@@ -176,10 +221,10 @@ func paramsToInputSchema(params []spec.Parameter) map[string]any { | |
|
|
||
| if p.Required { | ||
| if _, ok := is["required"]; !ok { | ||
| is["required"] = make([]string, 0) | ||
| is["required"] = make([]any, 0) | ||
| } | ||
|
|
||
| is["required"] = append(is["required"].([]string), p.Name) | ||
| is["required"] = append(is["required"].([]any), p.Name) | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than overwriting
os.Stdout, let's pass the file fromcmd.OutOrStdoutto the writers, and useCommand.SetOut()in the tests to override Stdout. This avoids mutating global state across tests run witht.Parallel().There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is just adding an argument to
writeDataSourceToFile