From a6f3e0a9ddfab5d61bf39f8d27f1128199ee4e6f Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Wed, 8 Jun 2022 15:27:31 +0200 Subject: [PATCH] Swagger: Finish some TODOs and Add consistancy check for definition generation (#50119) * finish some todo and add consistancy check * sort parameters * revert parameter ordering * fix meaningless changes * remove go-generate tag also from alerting json * spec changes * update spec --- pkg/api/docs/merge/merge_specs.go | 51 +++++++++++++++++++++++++++---- public/api-merged.json | 28 ++++++++--------- public/api-spec.json | 28 ++++++++--------- 3 files changed, 73 insertions(+), 34 deletions(-) diff --git a/pkg/api/docs/merge/merge_specs.go b/pkg/api/docs/merge/merge_specs.go index 05fcc7d82b3..de0e366e5e7 100644 --- a/pkg/api/docs/merge/merge_specs.go +++ b/pkg/api/docs/merge/merge_specs.go @@ -8,12 +8,33 @@ import ( "fmt" "io/ioutil" "os" + "reflect" "strings" "github.com/go-openapi/loads" "github.com/go-openapi/spec" ) +func mergeVectors(a, b []string) []string { + for _, p := range b { + exist := false + for _, op := range a { + if op == p { + exist = true + break + } + } + if !exist { + a = append(a, p) + } + } + return a +} + +func compareDefinition(a, b spec.Schema) bool { + return reflect.DeepEqual(a.Type, b.Type) && a.Format == b.Format && reflect.DeepEqual(a.Properties, b.Properties) +} + // mergeSpecs merges OSS API spec with one or more other OpenAPI specs func mergeSpecs(output string, sources ...string) error { if len(sources) < 2 { @@ -41,15 +62,33 @@ func mergeSpecs(output string, sources ...string) error { return fmt.Errorf("failed to load spec from: %s: %w", s, err) } - //TODO: consumes, produces, schemes + // Merge consumes + specOSS.SwaggerProps.Consumes = mergeVectors(specOSS.SwaggerProps.Consumes, additionalSpec.OrigSpec().Consumes) - //TODO: check for conflicts - for k, d := range additionalSpec.OrigSpec().SwaggerProps.Definitions { - specOSS.SwaggerProps.Definitions[k] = d + // Merge produces + specOSS.SwaggerProps.Produces = mergeVectors(specOSS.SwaggerProps.Produces, additionalSpec.OrigSpec().Produces) + + // Merge schemes + specOSS.SwaggerProps.Schemes = mergeVectors(specOSS.SwaggerProps.Schemes, additionalSpec.OrigSpec().Schemes) + + //TODO: When there are conflict between definitions, we need to error out, but here we need to fix the existing conflict first + // there are false positives, we will have to fix those by regenerate alerting api spec + for k, ad := range additionalSpec.OrigSpec().SwaggerProps.Definitions { + if ossd, exists := specOSS.SwaggerProps.Definitions[k]; exists { + if !compareDefinition(ad, ossd) { + fmt.Printf("the definition of %s differs in specs!\n", k) + } + } + specOSS.SwaggerProps.Definitions[k] = ad } - for k, r := range additionalSpec.OrigSpec().SwaggerProps.Responses { - specOSS.SwaggerProps.Responses[k] = r + for k, ar := range additionalSpec.OrigSpec().SwaggerProps.Responses { + if ossr, exists := specOSS.SwaggerProps.Responses[k]; exists { + if !reflect.DeepEqual(ar, ossr) { + fmt.Printf("the definition of response %s differs in specs!\n", k) + } + } + specOSS.SwaggerProps.Responses[k] = ar } for k, p := range additionalSpec.OrigSpec().SwaggerProps.Parameters { diff --git a/public/api-merged.json b/public/api-merged.json index c34f9112ddc..aa81d429ce8 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -489,6 +489,13 @@ "summary": "Add a user role assignment.", "operationId": "addUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "name": "user_id", + "in": "path", + "required": true + }, { "name": "body", "in": "body", @@ -496,13 +503,6 @@ "schema": { "$ref": "#/definitions/AddUserRoleCommand" } - }, - { - "type": "integer", - "format": "int64", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { @@ -528,6 +528,13 @@ "summary": "Remove a user role assignment.", "operationId": "removeUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "name": "user_id", + "in": "path", + "required": true + }, { "type": "string", "name": "roleUID", @@ -539,13 +546,6 @@ "description": "A flag indicating if the assignment is global or not. If set to false, the default org ID of the authenticated user will be used from the request to remove assignment.", "name": "global", "in": "query" - }, - { - "type": "integer", - "format": "int64", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { diff --git a/public/api-spec.json b/public/api-spec.json index 2133d32b73b..14595a213f9 100644 --- a/public/api-spec.json +++ b/public/api-spec.json @@ -489,6 +489,13 @@ "summary": "Add a user role assignment.", "operationId": "addUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "name": "user_id", + "in": "path", + "required": true + }, { "name": "body", "in": "body", @@ -496,13 +503,6 @@ "schema": { "$ref": "#/definitions/AddUserRoleCommand" } - }, - { - "type": "integer", - "format": "int64", - "name": "user_id", - "in": "path", - "required": true } ], "responses": { @@ -528,6 +528,13 @@ "summary": "Remove a user role assignment.", "operationId": "removeUserRole", "parameters": [ + { + "type": "integer", + "format": "int64", + "name": "user_id", + "in": "path", + "required": true + }, { "type": "string", "name": "roleUID", @@ -539,13 +546,6 @@ "description": "A flag indicating if the assignment is global or not. If set to false, the default org ID of the authenticated user will be used from the request to remove assignment.", "name": "global", "in": "query" - }, - { - "type": "integer", - "format": "int64", - "name": "user_id", - "in": "path", - "required": true } ], "responses": {