From 78f88d7a7495dfb6133ea354e11373b2d2de7f77 Mon Sep 17 00:00:00 2001 From: andreaangiolillo Date: Thu, 12 Dec 2024 14:03:27 +0000 Subject: [PATCH 1/3] CLOUDP-276602: FOASCLI: Improve the error message in the event of conflicts --- tools/cli/internal/openapi/errors/merge_conflict_error.go | 6 ++++-- tools/cli/internal/openapi/oasdiff.go | 4 +++- tools/cli/internal/openapi/openapi3.go | 1 + tools/cli/test/e2e/cli/merge_test.go | 4 +++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/cli/internal/openapi/errors/merge_conflict_error.go b/tools/cli/internal/openapi/errors/merge_conflict_error.go index 3953142d80..feabf63ba7 100644 --- a/tools/cli/internal/openapi/errors/merge_conflict_error.go +++ b/tools/cli/internal/openapi/errors/merge_conflict_error.go @@ -46,11 +46,13 @@ func (e ResponseConflictError) Error() string { } type SchemaConflictError struct { - Entry string + Entry string + BaseSpecLocation string + ExternalSpecLocation string } func (e SchemaConflictError) Error() string { - return fmt.Sprintf("there was a conflict on a Schema component: %q", e.Entry) + return fmt.Sprintf("there was a conflict on a Schema component: %q. Base Spec: %q, External Spec: %q", e.Entry, e.BaseSpecLocation, e.ExternalSpecLocation) } type TagConflictError struct { diff --git a/tools/cli/internal/openapi/oasdiff.go b/tools/cli/internal/openapi/oasdiff.go index e63ef5b1f1..5e9a61d7f4 100644 --- a/tools/cli/internal/openapi/oasdiff.go +++ b/tools/cli/internal/openapi/oasdiff.go @@ -419,7 +419,9 @@ func (o OasDiff) mergeSchemas() error { // The schemas have the same name but different definitions return errors.SchemaConflictError{ - Entry: k, + Entry: k, + BaseSpecLocation: o.base.Url, + ExternalSpecLocation: o.external.Url, } } } diff --git a/tools/cli/internal/openapi/openapi3.go b/tools/cli/internal/openapi/openapi3.go index 559933ea27..15da0e635c 100644 --- a/tools/cli/internal/openapi/openapi3.go +++ b/tools/cli/internal/openapi/openapi3.go @@ -48,6 +48,7 @@ func (o *OpenAPI3) WithExcludedPrivatePaths() *OpenAPI3 { func (o *OpenAPI3) CreateOpenAPISpecFromPath(path string) (*load.SpecInfo, error) { o.Loader.IsExternalRefsAllowed = o.IsExternalRefsAllowed spec, err := load.NewSpecInfo(o.Loader, load.NewSource(path)) + spec.Url = path if err != nil { return nil, err } diff --git a/tools/cli/test/e2e/cli/merge_test.go b/tools/cli/test/e2e/cli/merge_test.go index 66bb9a3d47..804f65d895 100644 --- a/tools/cli/test/e2e/cli/merge_test.go +++ b/tools/cli/test/e2e/cli/merge_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "fmt" "os" "os/exec" "testing" @@ -122,6 +123,7 @@ func TestMerge(t *testing.T) { resp, err := cmd.CombinedOutput() stringResponse := string(resp) require.Error(t, err, stringResponse) - assert.Contains(t, stringResponse, "Error: there was a conflict on a Schema component: \"ApiError\"") + assert.Contains(t, stringResponse, + fmt.Sprintf("Error: there was a conflict on a Schema component: \"ApiError\". Base Spec: %q, External Spec: %q", base, apiRegistrySpec)) }) } From 716eff59b04101c8051c8707b956a9258f658dcf Mon Sep 17 00:00:00 2001 From: andreaangiolillo Date: Thu, 12 Dec 2024 14:14:41 +0000 Subject: [PATCH 2/3] updated tag error message --- .../internal/openapi/errors/merge_conflict_error.go | 13 +++++++++---- tools/cli/internal/openapi/oasdiff.go | 6 ++++-- tools/cli/test/e2e/cli/merge_test.go | 8 ++++++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/tools/cli/internal/openapi/errors/merge_conflict_error.go b/tools/cli/internal/openapi/errors/merge_conflict_error.go index feabf63ba7..cef6923a03 100644 --- a/tools/cli/internal/openapi/errors/merge_conflict_error.go +++ b/tools/cli/internal/openapi/errors/merge_conflict_error.go @@ -52,16 +52,21 @@ type SchemaConflictError struct { } func (e SchemaConflictError) Error() string { - return fmt.Sprintf("there was a conflict on a Schema component: %q. Base Spec: %q, External Spec: %q", e.Entry, e.BaseSpecLocation, e.ExternalSpecLocation) + return fmt.Sprintf("there was a conflict on a Schema component: %q. Base Spec: %q, External Spec: %q", + e.Entry, e.BaseSpecLocation, e.ExternalSpecLocation) } type TagConflictError struct { - Entry string - Description string + Entry string + Description string + BaseSpecLocation string + ExternalSpecLocation string } func (e TagConflictError) Error() string { - return fmt.Sprintf("there was a conflict with the Tag %q with the description: %q", e.Entry, e.Description) + return fmt.Sprintf( + "there was a conflict with the Tag %q with the description: %q. Base Spec: %q, External Spec: %q", + e.Entry, e.Description, e.BaseSpecLocation, e.ExternalSpecLocation) } type PathDocsDiffConflictError struct { diff --git a/tools/cli/internal/openapi/oasdiff.go b/tools/cli/internal/openapi/oasdiff.go index 5e9a61d7f4..d330af6823 100644 --- a/tools/cli/internal/openapi/oasdiff.go +++ b/tools/cli/internal/openapi/oasdiff.go @@ -297,8 +297,10 @@ func (o OasDiff) mergeTags() error { baseTags = append(baseTags, v) } else { return errors.TagConflictError{ - Entry: v.Name, - Description: v.Description, + Entry: v.Name, + Description: v.Description, + BaseSpecLocation: o.base.Url, + ExternalSpecLocation: o.external.Url, } } } diff --git a/tools/cli/test/e2e/cli/merge_test.go b/tools/cli/test/e2e/cli/merge_test.go index 804f65d895..802eb8a7a3 100644 --- a/tools/cli/test/e2e/cli/merge_test.go +++ b/tools/cli/test/e2e/cli/merge_test.go @@ -101,7 +101,9 @@ func TestMerge(t *testing.T) { resp, err := cmd.CombinedOutput() stringResponse := string(resp) require.Error(t, err, stringResponse) - assert.Contains(t, stringResponse, "Error: there was a conflict with the Tag \"Events\" with the description: \"Returns information about the MongoDB Atlas Specification.\"") //nolint:lll // Line is over 120 characters + assert.Contains(t, stringResponse, fmt.Sprintf("Error: there was a conflict with the Tag \"Events\""+ + " with the description: \"Returns information about the MongoDB Atlas Specification.\"."+ + " Base Spec: %q, External Spec: %q", base, apiRegistrySpec)) }) t.Run("Expecting Error: not identical component", func(t *testing.T) { @@ -124,6 +126,8 @@ func TestMerge(t *testing.T) { stringResponse := string(resp) require.Error(t, err, stringResponse) assert.Contains(t, stringResponse, - fmt.Sprintf("Error: there was a conflict on a Schema component: \"ApiError\". Base Spec: %q, External Spec: %q", base, apiRegistrySpec)) + fmt.Sprintf( + "Error: there was a conflict on a Schema component: \"ApiError\". Base Spec: %q, "+ + "External Spec: %q", base, apiRegistrySpec)) }) } From 1a4684909d135d8e4f46b2f52c650cb068cf767f Mon Sep 17 00:00:00 2001 From: andreaangiolillo Date: Thu, 12 Dec 2024 14:17:41 +0000 Subject: [PATCH 3/3] Update merge_test.go --- tools/cli/test/e2e/cli/merge_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/cli/test/e2e/cli/merge_test.go b/tools/cli/test/e2e/cli/merge_test.go index 802eb8a7a3..b64ca02d30 100644 --- a/tools/cli/test/e2e/cli/merge_test.go +++ b/tools/cli/test/e2e/cli/merge_test.go @@ -103,7 +103,7 @@ func TestMerge(t *testing.T) { require.Error(t, err, stringResponse) assert.Contains(t, stringResponse, fmt.Sprintf("Error: there was a conflict with the Tag \"Events\""+ " with the description: \"Returns information about the MongoDB Atlas Specification.\"."+ - " Base Spec: %q, External Spec: %q", base, apiRegistrySpec)) + " Base Spec: %q, External Spec: %q", base, authnSpec)) }) t.Run("Expecting Error: not identical component", func(t *testing.T) {