Skip to content

Commit 5ded458

Browse files
committed
Lint and update
1 parent d35e601 commit 5ded458

File tree

4 files changed

+63
-26
lines changed

4 files changed

+63
-26
lines changed

tools/cli/internal/openapi/errors/merge_conflict_error.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@
1414

1515
package errors
1616

17-
import "fmt"
17+
import (
18+
"encoding/json"
19+
"fmt"
20+
21+
"github.com/tufin/oasdiff/diff"
22+
)
1823

1924
type ParamConflictError struct {
2025
Entry string
@@ -56,3 +61,26 @@ type TagConflictError struct {
5661
func (e TagConflictError) Error() string {
5762
return fmt.Sprintf("there was a conflict with the Tag %q with the description: %q", e.Entry, e.Description)
5863
}
64+
65+
type PathDocsDiffConflictError struct {
66+
Entry string
67+
Diff *diff.Diff
68+
}
69+
70+
func (e PathDocsDiffConflictError) Error() string {
71+
var pathDiff []byte
72+
_, ok := e.Diff.PathsDiff.Modified[e.Entry]
73+
if ok {
74+
pathDiff, _ = json.MarshalIndent(e.Diff.PathsDiff.Modified[e.Entry], "", " ")
75+
}
76+
77+
return fmt.Sprintf("the path: %q is enabled for merge but it has a diff between the base and external spec. See the diff:\n%s", e.Entry, pathDiff)
78+
}
79+
80+
type AllowDocsDiffNotSupportedError struct {
81+
Entry string
82+
}
83+
84+
func (e AllowDocsDiffNotSupportedError) Error() string {
85+
return fmt.Sprintf("the path: %q is enabled for merge but the flag to allow docs diff is not supported", e.Entry)
86+
}

tools/cli/internal/openapi/oasdiff.go

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package openapi
1616

1717
import (
18-
"encoding/json"
1918
"log"
2019
"slices"
2120
"strings"
@@ -122,11 +121,7 @@ func (o OasDiff) mergePaths() error {
122121
if err != nil {
123122
return err
124123
}
125-
126-
// if diff is not allowed and there is a diff, then fail
127-
if !allOperationsAllowDocsDiff(originalPathData, path) {
128-
basePaths.Set(path, removeExternalRefs(externalPathData))
129-
}
124+
basePaths.Set(path, removeExternalRefs(externalPathData))
130125
}
131126
}
132127
o.base.Spec.Paths = basePaths
@@ -176,19 +171,33 @@ func (o OasDiff) handlePathConflict(basePath *openapi3.PathItem, basePathName st
176171
}
177172
}
178173

174+
var pathsAreIdentical bool
175+
var err error
176+
if pathsAreIdentical, err = o.arePathsIdenticalWithExcludeExtensions(basePathName); err != nil {
177+
return err
178+
}
179+
179180
log.Printf("Skipping conflict for path: %s", basePathName)
180-
if o.arePathsIdenticalWithExcludeExtensions(basePathName) {
181-
log.Printf("No doc diff detected for path %s, merging the paths", basePathName)
181+
if pathsAreIdentical {
182182
return nil
183183
}
184184

185-
if !allOperationsAllowDocsDiff(basePath, basePathName) {
186-
log.Printf("Doc diff detected failing as allowDocsDiff=true is not supported.")
187-
return errors.PathConflictError{
185+
// allowDocsDiff = true not supported
186+
if allOperationsAllowDocsDiff(basePath) {
187+
return errors.AllowDocsDiffNotSupportedError{
188188
Entry: basePathName,
189189
}
190190
}
191-
return nil
191+
192+
d, err := o.getDiffWithoutExtensions()
193+
if err != nil {
194+
return err
195+
}
196+
197+
return errors.PathDocsDiffConflictError{
198+
Entry: basePathName,
199+
Diff: d,
200+
}
192201
}
193202

194203
// shouldSkipConflict checks if the conflict should be skipped.
@@ -475,24 +484,24 @@ func (o OasDiff) areSchemaIdentical(name string) bool {
475484
}
476485

477486
// arePathsIdenticalWithExcludeExtensions checks if the paths are identical with the extensions excluded
478-
func (o OasDiff) arePathsIdenticalWithExcludeExtensions(name string) bool {
487+
func (o OasDiff) arePathsIdenticalWithExcludeExtensions(name string) (bool, error) {
479488
// If the diff only has extensions diff, then we consider the paths to be identical
480-
exclude := []string{"extensions"}
481-
customConfig := diff.NewConfig().WithExcludeElements(exclude)
482-
d, err := diff.Get(customConfig, o.base.Spec, o.external.Spec)
489+
d, err := o.getDiffWithoutExtensions()
483490
if err != nil {
484-
log.Fatalf("error in calculating the diff of the specs: %s", err)
491+
return false, err
485492
}
486493

487494
if d.Empty() || d.PathsDiff.Empty() {
488-
return true
495+
return true, nil
489496
}
490497
_, ok := d.PathsDiff.Modified[name]
491-
if ok {
492-
j, _ := json.MarshalIndent(d.PathsDiff.Modified[name], "", " ")
493-
log.Printf("arePathsIdenticalWithExcludeExtensions diff: %s", j)
494-
}
495-
return !ok
498+
return !ok, nil
499+
}
500+
501+
func (o OasDiff) getDiffWithoutExtensions() (*diff.Diff, error) {
502+
exclude := []string{"extensions"}
503+
customConfig := diff.NewConfig().WithExcludeElements(exclude)
504+
return diff.Get(customConfig, o.base.Spec, o.external.Spec)
496505
}
497506

498507
type ByName []*openapi3.Tag

tools/cli/internal/openapi/paths.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func getOperationExtensionWithName(operation *openapi3.Operation, extensionName
7474
return operation.Extensions[extensionName]
7575
}
7676

77-
func allOperationsAllowDocsDiff(basePath *openapi3.PathItem, basePathName string) bool {
77+
func allOperationsAllowDocsDiff(basePath *openapi3.PathItem) bool {
7878
if basePath.Operations() == nil || len(basePath.Operations()) == 0 {
7979
return false
8080
}

tools/cli/internal/openapi/paths_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func TestAllOperationsAllowDocsDiff(t *testing.T) {
177177

178178
for _, test := range tests {
179179
t.Run(test.name, func(t *testing.T) {
180-
actual := allOperationsAllowDocsDiff(test.input, "/test")
180+
actual := allOperationsAllowDocsDiff(test.input)
181181
if actual != test.expected {
182182
t.Errorf("Expected %t, got %t", test.expected, actual)
183183
}

0 commit comments

Comments
 (0)