From b9f4ebe7e220daca57f4a1e7b7fd13485049f639 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Wed, 4 Mar 2026 16:25:34 +0100 Subject: [PATCH] poc: FOASCLI breaking change command with custom check --- ...ck_x_xgen_operation_id_override_updated.go | 71 +++++++ .../customcheckers/customcheckers.go | 57 ++++++ .../cli/breakingchanges/breakingchanges.go | 2 + .../breakingchanges/breakingchanges_test.go | 2 +- .../cli/breakingchanges/check/check.go | 175 ++++++++++++++++++ .../cli/breakingchanges/check/check_test.go | 111 +++++++++++ tools/cli/internal/cli/usage/usage.go | 1 + 7 files changed, 418 insertions(+), 1 deletion(-) create mode 100644 tools/cli/internal/breakingchanges/customcheckers/check_x_xgen_operation_id_override_updated.go create mode 100644 tools/cli/internal/breakingchanges/customcheckers/customcheckers.go create mode 100644 tools/cli/internal/cli/breakingchanges/check/check.go create mode 100644 tools/cli/internal/cli/breakingchanges/check/check_test.go diff --git a/tools/cli/internal/breakingchanges/customcheckers/check_x_xgen_operation_id_override_updated.go b/tools/cli/internal/breakingchanges/customcheckers/check_x_xgen_operation_id_override_updated.go new file mode 100644 index 0000000000..e2c203ae37 --- /dev/null +++ b/tools/cli/internal/breakingchanges/customcheckers/check_x_xgen_operation_id_override_updated.go @@ -0,0 +1,71 @@ +package customcheckers + +import ( + "github.com/oasdiff/oasdiff/checker" + "github.com/oasdiff/oasdiff/diff" +) + +const ( + XXgenOperationIdOverrideAddedId = "api-x-xgen-operation-id-override-added" + XXgenOperationIdOverrideRemovedId = "api-x-xgen-operation-id-override-removed" + XXgenOperationIdOverrideModifiedId = "api-x-xgen-operation-id-override-modified" +) + +func APIXXgenOperationIdOverrideUpdatedCheck(diffReport *diff.Diff, operationsSources *diff.OperationsSourcesMap, config *checker.Config) checker.Changes { + result := make(checker.Changes, 0) + if diffReport.PathsDiff == nil { + return result + } + + for path, pathItem := range diffReport.PathsDiff.Modified { + if pathItem.OperationsDiff == nil { + continue + } + + for operation, operationItem := range pathItem.OperationsDiff.Modified { + if operationItem.ExtensionsDiff.Empty() { + continue + } + + if operationItem.ExtensionsDiff.Modified["x-xgen-operation-id-override"] != nil { + result = append(result, checker.NewApiChange( + XXgenOperationIdOverrideModifiedId, + config, + []any{operationItem.Base.Extensions["x-xgen-operation-id-override"], operationItem.Revision.Extensions["x-xgen-operation-id-override"]}, + "", + operationsSources, + pathItem.Base.GetOperation(operation), + operation, + path, + )) + } + + if operationItem.ExtensionsDiff.Deleted.Contains("x-xgen-operation-id-override") { + result = append(result, checker.NewApiChange( + XXgenOperationIdOverrideRemovedId, + config, + []any{operationItem.Base.Extensions["x-xgen-operation-id-override"]}, + "", + operationsSources, + pathItem.Base.GetOperation(operation), + operation, + path, + )) + } + + if operationItem.ExtensionsDiff.Added.Contains("x-xgen-operation-id-override") { + result = append(result, checker.NewApiChange( + XXgenOperationIdOverrideAddedId, + config, + []any{operationItem.Revision.Extensions["x-xgen-operation-id-override"]}, + "", + operationsSources, + pathItem.Revision.GetOperation(operation), + operation, + path, + )) + } + } + } + return result +} diff --git a/tools/cli/internal/breakingchanges/customcheckers/customcheckers.go b/tools/cli/internal/breakingchanges/customcheckers/customcheckers.go new file mode 100644 index 0000000000..5a19ac6aba --- /dev/null +++ b/tools/cli/internal/breakingchanges/customcheckers/customcheckers.go @@ -0,0 +1,57 @@ +package customcheckers + +import ( + "fmt" + "github.com/oasdiff/oasdiff/checker" + "github.com/oasdiff/oasdiff/utils" +) + +func GetAllChecks() checker.BackwardCompatibilityChecks { + return rulesToChecks(getAllRules()) +} + +func GetAllRules() checker.BackwardCompatibilityRules { + rules := []checker.BackwardCompatibilityRule{ + newRule(XXgenOperationIdOverrideAddedId, checker.ERR, "", APIXXgenOperationIdOverrideUpdatedCheck, checker.DirectionNone, checker.LocationNone, checker.ActionAdd), + newRule(XXgenOperationIdOverrideRemovedId, checker.ERR, "", APIXXgenOperationIdOverrideUpdatedCheck, checker.DirectionNone, checker.LocationNone, checker.ActionRemove), + newRule(XXgenOperationIdOverrideModifiedId, checker.ERR, "", APIXXgenOperationIdOverrideUpdatedCheck, checker.DirectionNone, checker.LocationNone, checker.ActionChange), + } + + return rules +} + +func getAllRules() checker.BackwardCompatibilityRules { + rules := []checker.BackwardCompatibilityRule{ + newRule(XXgenOperationIdOverrideAddedId, checker.ERR, "", APIXXgenOperationIdOverrideUpdatedCheck, checker.DirectionNone, checker.LocationNone, checker.ActionAdd), + newRule(XXgenOperationIdOverrideRemovedId, checker.ERR, "", APIXXgenOperationIdOverrideUpdatedCheck, checker.DirectionNone, checker.LocationNone, checker.ActionRemove), + newRule(XXgenOperationIdOverrideModifiedId, checker.ERR, "", APIXXgenOperationIdOverrideUpdatedCheck, checker.DirectionNone, checker.LocationNone, checker.ActionChange), + } + + return rules +} + +func newRule(id string, level checker.Level, description string, check checker.BackwardCompatibilityCheck, direction checker.Direction, location checker.Location, action checker.Action) checker.BackwardCompatibilityRule { + return checker.BackwardCompatibilityRule{ + Id: id, + Level: level, + Description: description, + Handler: check, + Direction: direction, + Location: location, + Action: action, + } +} + +func rulesToChecks(rules checker.BackwardCompatibilityRules) checker.BackwardCompatibilityChecks { + result := checker.BackwardCompatibilityChecks{} + m := utils.StringSet{} + for _, rule := range rules { + // functions are not comparable, so we convert them to strings + pStr := fmt.Sprintf("%v", rule.Handler) + if !m.Contains(pStr) { + m.Add(pStr) + result = append(result, rule.Handler) + } + } + return result +} diff --git a/tools/cli/internal/cli/breakingchanges/breakingchanges.go b/tools/cli/internal/cli/breakingchanges/breakingchanges.go index d111c4a0ba..a5e8e36ec7 100644 --- a/tools/cli/internal/cli/breakingchanges/breakingchanges.go +++ b/tools/cli/internal/cli/breakingchanges/breakingchanges.go @@ -15,6 +15,7 @@ package breakingchanges import ( + "github.com/mongodb/openapi/tools/cli/internal/cli/breakingchanges/check" "github.com/mongodb/openapi/tools/cli/internal/cli/breakingchanges/exemptions" "github.com/spf13/cobra" ) @@ -26,6 +27,7 @@ func Builder() *cobra.Command { } cmd.AddCommand( + check.Builder(), exemptions.Builder(), ) diff --git a/tools/cli/internal/cli/breakingchanges/breakingchanges_test.go b/tools/cli/internal/cli/breakingchanges/breakingchanges_test.go index ba77c7f61f..733e523045 100644 --- a/tools/cli/internal/cli/breakingchanges/breakingchanges_test.go +++ b/tools/cli/internal/cli/breakingchanges/breakingchanges_test.go @@ -24,7 +24,7 @@ func TestBuilder(t *testing.T) { test.CmdValidator( t, Builder(), - 1, + 2, []string{}, ) } diff --git a/tools/cli/internal/cli/breakingchanges/check/check.go b/tools/cli/internal/cli/breakingchanges/check/check.go new file mode 100644 index 0000000000..eb5ff10022 --- /dev/null +++ b/tools/cli/internal/cli/breakingchanges/check/check.go @@ -0,0 +1,175 @@ +// Copyright 2024 MongoDB Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package check + +import ( + "encoding/json" + "fmt" + "github.com/mongodb/openapi/tools/cli/internal/breakingchanges/customcheckers" + + "github.com/mongodb/openapi/tools/cli/internal/cli/flag" + "github.com/mongodb/openapi/tools/cli/internal/cli/usage" + "github.com/mongodb/openapi/tools/cli/internal/openapi" + "github.com/oasdiff/oasdiff/checker" + "github.com/oasdiff/oasdiff/diff" + "github.com/spf13/afero" + "github.com/spf13/cobra" +) + +type Opts struct { + fs afero.Fs + basePath string + revisionPath string + outputPath string +} + +func (o *Opts) Run() error { + // Load the base spec + parser := openapi.NewOpenAPI3() + baseSpec, err := parser.CreateOpenAPISpecFromPath(o.basePath) + if err != nil { + return fmt.Errorf("failed to load base spec: %w", err) + } + + // Load the revision spec + revisionSpec, err := parser.CreateOpenAPISpecFromPath(o.revisionPath) + if err != nil { + return fmt.Errorf("failed to load revision spec: %w", err) + } + + // Create OasDiff instance + oasDiff := openapi.NewOasDiffWithSpecInfo(baseSpec, revisionSpec, &diff.Config{ + IncludePathParams: true, + }) + + // Get the flattened diff + diffResult, err := oasDiff.GetFlattenedDiff(baseSpec, revisionSpec) + if err != nil { + return fmt.Errorf("failed to get diff: %w", err) + } + + // Load default and custom checkers and rules + checks := append(checker.GetAllChecks(), customcheckers.GetAllChecks()...) + rules := append(checker.GetAllRules(), customcheckers.GetAllRules()...) + + // Set log levels for each rule + ids := make(map[string]checker.Level) + for _, rule := range rules { + ids[rule.Id] = rule.Level + } + + // Create checker config + config := &checker.Config{ + Checks: checks, + LogLevels: ids, + Attributes: make([]string, 0), + MinSunsetBetaDays: 365, // TODO: not sure what this does, it's used downstream + MinSunsetStableDays: 365, // TODO: not sure what this does, it's used downstream + } + + // Additional checks used downstream + breakingChangesAdditionalCheckers := []string{ + "response-non-success-status-removed", + "api-operation-id-removed", + "api-tag-removed", + "response-property-enum-value-removed", + "response-mediatype-enum-value-removed", + "request-body-enum-value-removed", + "api-schema-removed", + } + + // Add additional checkers used downstream, sets them to ERR level by default + config = config.WithOptionalChecks(breakingChangesAdditionalCheckers) + + // Check for breaking changes + breakingChanges := checker.CheckBackwardCompatibilityUntilLevel( + config, + diffResult.Report, + diffResult.SourceMap, + checker.ERR, // TODO: Use flag for setting level (default to WARN) + ) + + // Format the output as JSON + output, err := json.MarshalIndent(breakingChanges, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal breaking changes: %w", err) + } + + // Write to output file or print to terminal + if o.outputPath != "" { + if err := afero.WriteFile(o.fs, o.outputPath, output, 0o644); err != nil { + return fmt.Errorf("failed to write output file: %w", err) + } + fmt.Printf("Breaking changes written to %s\n", o.outputPath) + } else { + fmt.Println(string(output)) + } + + return nil +} + +func (o *Opts) PreRunE() error { + if o.basePath == "" { + return fmt.Errorf("base spec path is required") + } + + if o.revisionPath == "" { + return fmt.Errorf("revision spec path is required") + } + + // Validate that the files exist + if _, err := o.fs.Stat(o.basePath); err != nil { + return fmt.Errorf("base spec file not found: %w", err) + } + + if _, err := o.fs.Stat(o.revisionPath); err != nil { + return fmt.Errorf("revision spec file not found: %w", err) + } + + return nil +} + +// Builder builds the check command with the following signature: +// breaking-changes check -b base-spec -r revision-spec [-o output-file]. +func Builder() *cobra.Command { + opts := &Opts{ + fs: afero.NewOsFs(), + } + + cmd := &cobra.Command{ + Use: "check -b base-spec -r revision-spec [-o output-file]", + Short: "Check breaking changes between two OpenAPI specifications.", + Long: `Check breaking changes between two OpenAPI specifications using OasDiff. +The command compares a base specification with a revision specification and reports +any breaking changes found. By default, the results are printed to the terminal, +but can be written to a file using the --output flag.`, + Args: cobra.NoArgs, + PreRunE: func(_ *cobra.Command, _ []string) error { + return opts.PreRunE() + }, + RunE: func(_ *cobra.Command, _ []string) error { + return opts.Run() + }, + } + + cmd.Flags().StringVarP(&opts.basePath, flag.Base, flag.BaseShort, "", usage.Base) + cmd.Flags().StringVarP(&opts.revisionPath, flag.Revision, flag.RevisionShort, "", usage.Revision) + cmd.Flags().StringVarP(&opts.outputPath, flag.Output, flag.OutputShort, "", usage.Output) + + _ = cmd.MarkFlagRequired(flag.Base) + _ = cmd.MarkFlagRequired(flag.Revision) + + return cmd +} diff --git a/tools/cli/internal/cli/breakingchanges/check/check_test.go b/tools/cli/internal/cli/breakingchanges/check/check_test.go new file mode 100644 index 0000000000..27a8006203 --- /dev/null +++ b/tools/cli/internal/cli/breakingchanges/check/check_test.go @@ -0,0 +1,111 @@ +// Copyright 2024 MongoDB Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package check + +import ( + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" +) + +func TestCheckOpts_PreRunE(t *testing.T) { + testCases := []struct { + name string + basePath string + revisionPath string + wantErr require.ErrorAssertionFunc + setupFS func(fs afero.Fs) + }{ + { + name: "NoBasePath", + basePath: "", + revisionPath: "revision.json", + wantErr: require.Error, + setupFS: func(_ afero.Fs) {}, + }, + { + name: "NoRevisionPath", + basePath: "base.json", + revisionPath: "", + wantErr: require.Error, + setupFS: func(_ afero.Fs) {}, + }, + { + name: "BaseFileNotFound", + basePath: "base.json", + revisionPath: "revision.json", + wantErr: require.Error, + setupFS: func(fs afero.Fs) { + _ = afero.WriteFile(fs, "revision.json", []byte("{}"), 0o644) + }, + }, + { + name: "RevisionFileNotFound", + basePath: "base.json", + revisionPath: "revision.json", + wantErr: require.Error, + setupFS: func(fs afero.Fs) { + _ = afero.WriteFile(fs, "base.json", []byte("{}"), 0o644) + }, + }, + { + name: "Successful", + basePath: "base.json", + revisionPath: "revision.json", + wantErr: require.NoError, + setupFS: func(fs afero.Fs) { + _ = afero.WriteFile(fs, "base.json", []byte("{}"), 0o644) + _ = afero.WriteFile(fs, "revision.json", []byte("{}"), 0o644) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fs := afero.NewMemMapFs() + tc.setupFS(fs) + + opts := &Opts{ + fs: fs, + basePath: tc.basePath, + revisionPath: tc.revisionPath, + } + + tc.wantErr(t, opts.PreRunE()) + }) + } +} + +func TestCheckBuilder(t *testing.T) { + cmd := Builder() + + require.NotNil(t, cmd) + require.Equal(t, "check -b base-spec -r revision-spec [-o output-file]", cmd.Use) + require.Equal(t, "Check breaking changes between two OpenAPI specifications.", cmd.Short) + + // Verify required flags + baseFlag := cmd.Flags().Lookup("base") + require.NotNil(t, baseFlag) + require.Equal(t, "b", baseFlag.Shorthand) + + revisionFlag := cmd.Flags().Lookup("revision") + require.NotNil(t, revisionFlag) + require.Equal(t, "r", revisionFlag.Shorthand) + + outputFlag := cmd.Flags().Lookup("output") + require.NotNil(t, outputFlag) + require.Equal(t, "o", outputFlag.Shorthand) +} diff --git a/tools/cli/internal/cli/usage/usage.go b/tools/cli/internal/cli/usage/usage.go index d789adbe0f..12ecea0b07 100644 --- a/tools/cli/internal/cli/usage/usage.go +++ b/tools/cli/internal/cli/usage/usage.go @@ -16,6 +16,7 @@ package usage const ( Base = "Base OAS. The command will merge other OASes into it." + Revision = "Revision OAS to compare against the base OAS." External = "OASes that will be merged into the base OAS." Output = "File name or path where the command will store the output." Format = "Output format. Supported values are 'json', 'yaml' or 'all' which will generate one file for each supported format."