From 299cf2127afb9f6b7ebd294dadc5334ef6ffa39d Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sun, 17 Nov 2024 22:06:19 -0800 Subject: [PATCH] fix: properly produce standard methods in ConvertToOpenAPI --- README.md | 1 + pkg/api/api_test.go | 8 ++--- pkg/api/openapi.go | 22 +++++++------ pkg/api/openapi_test.go | 72 +++++++++++++++++++++++++++++++++++------ pkg/openapi/openapi.go | 12 +++---- 5 files changed, 87 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 8b496d4..11a577a 100644 --- a/README.md +++ b/README.md @@ -6,5 +6,6 @@ It contains, among other things: - common utilities for case conversion - a service definition that can be extracted from aep-compliant or close to aep-compliant openapi specifications. +- a writer for compliant OpenAPI specifications. For usage of individual packages, please see the tests. diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 2bed997..3da6135 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -10,7 +10,7 @@ import ( var basicOpenAPI = &openapi.OpenAPI{ OpenAPI: "3.1.0", Servers: []openapi.Server{{URL: "https://api.example.com"}}, - Paths: map[string]openapi.PathItem{ + Paths: map[string]*openapi.PathItem{ "/widgets": { Get: &openapi.Operation{ Responses: map[string]openapi.Response{ @@ -200,7 +200,7 @@ func TestGetAPI(t *testing.T) { name: "resource with x-aep-resource annotation", api: &openapi.OpenAPI{ OpenAPI: "3.1.0", - Paths: map[string]openapi.PathItem{ + Paths: map[string]*openapi.PathItem{ "/widgets/{widget}": { Get: &openapi.Operation{ Responses: map[string]openapi.Response{ @@ -255,7 +255,7 @@ func TestGetAPI(t *testing.T) { api: &openapi.OpenAPI{ OpenAPI: "3.1.0", Servers: []openapi.Server{{URL: "https://api.example.com"}}, - Paths: map[string]openapi.PathItem{ + Paths: map[string]*openapi.PathItem{ "/widgets": { Post: &openapi.Operation{ Parameters: []openapi.Parameter{ @@ -295,7 +295,7 @@ func TestGetAPI(t *testing.T) { api: &openapi.OpenAPI{ Swagger: "2.0", Servers: []openapi.Server{{URL: "https://api.example.com"}}, - Paths: map[string]openapi.PathItem{ + Paths: map[string]*openapi.PathItem{ "/widgets/{widget}": { Get: &openapi.Operation{ Responses: map[string]openapi.Response{ diff --git a/pkg/api/openapi.go b/pkg/api/openapi.go index abb2bb5..6112327 100644 --- a/pkg/api/openapi.go +++ b/pkg/api/openapi.go @@ -22,7 +22,7 @@ func (api *API) ConvertToOpenAPIBytes() ([]byte, error) { } func ConvertToOpenAPI(api *API) (*openapi.OpenAPI, error) { - paths := map[string]openapi.PathItem{} + paths := map[string]*openapi.PathItem{} components := openapi.Components{ Schemas: map[string]openapi.Schema{}, } @@ -68,10 +68,10 @@ func ConvertToOpenAPI(api *API) (*openapi.OpenAPI, error) { }, } for _, pwp := range *parentPWPS { - resourcePath := fmt.Sprintf("%s/%s/{%s}", pwp.Pattern, collection, singular) + resourcePath := fmt.Sprintf("%s%s/{%s}", pwp.Pattern, collection, singular) patterns = append(patterns, resourcePath[1:]) if r.ListMethod != nil { - listPath := fmt.Sprintf("%s/%s", pwp.Pattern, collection) + listPath := fmt.Sprintf("%s%s", pwp.Pattern, collection) addMethodToPath(paths, listPath, "get", openapi.Operation{ Parameters: append(pwp.Params, openapi.Parameter{ @@ -110,7 +110,7 @@ func ConvertToOpenAPI(api *API) (*openapi.OpenAPI, error) { }) } if r.CreateMethod != nil { - createPath := fmt.Sprintf("%s/%s", pwp.Pattern, collection) + createPath := fmt.Sprintf("%s%s", pwp.Pattern, collection) params := pwp.Params if !r.CreateMethod.SupportsUserSettableCreate { params = append(params, openapi.Parameter{ @@ -255,7 +255,7 @@ func generateParentPatternsWithParams(r *Resource) (string, *[]PathWithParams) { // case 1: pattern elems are present, so we use them. // TODO(yft): support multiple patterns if len(r.PatternElems) > 0 { - collection := r.PatternElems[len(r.PatternElems)-2] + collection := fmt.Sprintf("/%s", r.PatternElems[len(r.PatternElems)-2]) params := []openapi.Parameter{} for i := 0; i < len(r.PatternElems)-2; i += 2 { pElem := r.PatternElems[i+1] @@ -266,12 +266,16 @@ func generateParentPatternsWithParams(r *Resource) (string, *[]PathWithParams) { Type: "string", }) } + pattern := strings.Join(r.PatternElems[0:len(r.PatternElems)-2], "/") + if pattern != "" { + pattern = fmt.Sprintf("/%s", pattern) + } return collection, &[]PathWithParams{ - {Pattern: fmt.Sprintf("/%s", strings.Join(r.PatternElems[0:len(r.PatternElems)-2], "/")), Params: params}, + {Pattern: pattern, Params: params}, } } // case 2: no pattern elems, so we need to generate the collection names - collection := CollectionName(r) + collection := fmt.Sprintf("/%s", CollectionName(r)) pwps := []PathWithParams{} for _, parent := range r.Parents { singular := parent.Singular @@ -299,10 +303,10 @@ func generateParentPatternsWithParams(r *Resource) (string, *[]PathWithParams) { return collection, &pwps } -func addMethodToPath(paths map[string]openapi.PathItem, path, method string, methodInfo openapi.Operation) { +func addMethodToPath(paths map[string]*openapi.PathItem, path, method string, methodInfo openapi.Operation) { methods, ok := paths[path] if !ok { - methods = openapi.PathItem{} + methods = &openapi.PathItem{} paths[path] = methods } switch method { diff --git a/pkg/api/openapi_test.go b/pkg/api/openapi_test.go index 1548a94..b2f7490 100644 --- a/pkg/api/openapi_test.go +++ b/pkg/api/openapi_test.go @@ -55,13 +55,13 @@ func TestToOpenAPI(t *testing.T) { "publisher": publisher, }, } - tests := []struct { - name string - api *API - expectedPaths []string - expectedSchemas []string - wantErr bool + name string + api *API + expectedPaths []string + expectedSchemas []string + expectedOperations map[string]openapi.PathItem + wantErr bool }{ { name: "Basic resource paths", @@ -75,6 +75,24 @@ func TestToOpenAPI(t *testing.T) { expectedSchemas: []string{ "account", }, + expectedOperations: map[string]openapi.PathItem{ + "/publishers": { + Get: &openapi.Operation{}, + Post: &openapi.Operation{}, + }, + "/publishers/{publisher}": { + Get: &openapi.Operation{}, + }, + "/publishers/{publisher}/books": { + Get: &openapi.Operation{}, + Post: &openapi.Operation{}, + }, + "/publishers/{publisher}/books/{book}": { + Get: &openapi.Operation{}, + Put: &openapi.Operation{}, + Delete: &openapi.Operation{}, + }, + }, wantErr: false, }, } @@ -113,6 +131,27 @@ func TestToOpenAPI(t *testing.T) { _, exists := openAPI.Components.Schemas[schema] assert.True(t, exists, "Expected schema %s not found", schema) } + + // Verify operations exist + for path, operations := range tt.expectedOperations { + pathItem, exists := openAPI.Paths[path] + assert.True(t, exists, "Expected path %s not found", path) + if operations.Get != nil { + assert.NotNil(t, pathItem.Get, "expected get operation for path %s", path) + } + if operations.Patch != nil { + assert.NotNil(t, pathItem.Patch, "expected patch operation for path %s", path) + } + if operations.Post != nil { + assert.NotNil(t, pathItem.Post, "expected post operation for path %s", path) + } + if operations.Put != nil { + assert.NotNil(t, operations.Put, "expected put operation for path %s", path) + } + if operations.Delete != nil { + assert.NotNil(t, pathItem.Delete, "expected delete operation for path %s", path) + } + } }) } } @@ -130,7 +169,7 @@ func TestGenerateParentPatternsWithParams(t *testing.T) { PatternElems: []string{"databases", "{database}", "tables", "{table}"}, Singular: "table", }, - wantCollection: "tables", + wantCollection: "/tables", wantPathParams: &[]PathWithParams{ { Pattern: "/databases/{database}", @@ -145,6 +184,21 @@ func TestGenerateParentPatternsWithParams(t *testing.T) { }, }, }, + { + name: "with pattern elements no nesting", + resource: &Resource{ + PatternElems: []string{"databases", "{database}"}, + Singular: "database", + }, + wantCollection: "/databases", + wantPathParams: &[]PathWithParams{ + { + Pattern: "", + Params: []openapi.Parameter{}, + }, + }, + }, + { name: "without pattern elements", resource: &Resource{ @@ -157,7 +211,7 @@ func TestGenerateParentPatternsWithParams(t *testing.T) { }, }, }, - wantCollection: "tables", + wantCollection: "/tables", wantPathParams: &[]PathWithParams{ { Pattern: "/databases/{database}", @@ -190,7 +244,7 @@ func TestGenerateParentPatternsWithParams(t *testing.T) { }, }, }, - wantCollection: "tables", + wantCollection: "/tables", wantPathParams: &[]PathWithParams{ { Pattern: "/accounts/{account}/databases/{database}", diff --git a/pkg/openapi/openapi.go b/pkg/openapi/openapi.go index 401109b..5eee203 100644 --- a/pkg/openapi/openapi.go +++ b/pkg/openapi/openapi.go @@ -19,12 +19,12 @@ const ( type OpenAPI struct { // oas 2.0 has swagger in the root.k - Swagger string `json:"swagger,omitempty"` - OpenAPI string `json:"openapi,omitempty"` - Servers []Server `json:"servers,omitempty"` - Info Info `json:"info"` - Paths map[string]PathItem `json:"paths"` - Components Components `json:"components,omitempty"` + Swagger string `json:"swagger,omitempty"` + OpenAPI string `json:"openapi,omitempty"` + Servers []Server `json:"servers,omitempty"` + Info Info `json:"info"` + Paths map[string]*PathItem `json:"paths"` + Components Components `json:"components,omitempty"` // oas 2.0 has definitions in the root. Definitions map[string]Schema `json:"definitions,omitempty"` }