Skip to content

feat: add to and from flags to FOASCLI sunset diff command #1243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lovisaberggren merged 6 commits into from
Apr 27, 2026
Merged

feat: add to and from flags to FOASCLI sunset diff command #1243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
92ce1d4
feat: add to and from flags to FOASCLI sunset diff command
lovisaberggren Apr 22, 2026
aa36715
fix: lint
lovisaberggren Apr 22, 2026
e085695
fix: lint
lovisaberggren Apr 22, 2026
2e4011f
fix: use path.join
lovisaberggren Apr 23, 2026
185b3e5
fix: allow only one flag of to/from and add test
lovisaberggren Apr 23, 2026
b63147e
fix: address comments
lovisaberggren Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 89 additions & 10 deletions tools/cli/internal/cli/sunset/diff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"sort"
"strings"
"time"

"github.com/mongodb/openapi/tools/cli/internal/cli/flag"
"github.com/mongodb/openapi/tools/cli/internal/cli/usage"
Expand All @@ -35,6 +36,10 @@ type DiffOpts struct {
specPath string
outputPath string
format string
from string
to string
toDate *time.Time
fromDate *time.Time
}

type Diff struct {
Expand Down Expand Up @@ -68,7 +73,10 @@ func (o *DiffOpts) Run() error {
specSunsets := sunset.NewListFromSpec(specInfo)

// Find differences
var diffs = findDiffs(baseSunsets, specSunsets, o.basePath, o.specPath)
diffs, err := o.findDiffs(baseSunsets, specSunsets)
if err != nil {
return err
}

// Write to output
bytes, err := o.newSunsetDiffBytes(diffs)
Expand All @@ -84,7 +92,7 @@ func (o *DiffOpts) Run() error {
return nil
}

func findDiffs(baseSunsets, specSunsets []*sunset.Sunset, baseSpecPath, specPath string) []*Diff {
func (o *DiffOpts) findDiffs(baseSunsets, specSunsets []*sunset.Sunset) ([]*Diff, error) {
// Create maps for easy lookup
baseMap := make(map[string]*sunset.Sunset)
for _, s := range baseSunsets {
Expand All @@ -106,15 +114,14 @@ func findDiffs(baseSunsets, specSunsets []*sunset.Sunset, baseSpecPath, specPath
if specSunset, exists := specMap[key]; exists {
// Endpoint exists in both specs
if baseSunset.SunsetDate != specSunset.SunsetDate {
// Different sunset dates
diffs = append(diffs, &Diff{
Operation: baseSunset.Operation,
Path: baseSunset.Path,
Version: baseSunset.Version,
BaseSunsetDate: baseSunset.SunsetDate,
SpecSunsetDate: specSunset.SunsetDate,
BaseSpec: baseSpecPath,
Spec: specPath,
BaseSpec: o.basePath,
Spec: o.specPath,
Team: baseSunset.Team,
})
}
Expand All @@ -126,8 +133,8 @@ func findDiffs(baseSunsets, specSunsets []*sunset.Sunset, baseSpecPath, specPath
Version: baseSunset.Version,
BaseSunsetDate: baseSunset.SunsetDate,
SpecSunsetDate: "",
BaseSpec: baseSpecPath,
Spec: specPath,
BaseSpec: o.basePath,
Spec: o.specPath,
Team: baseSunset.Team,
})
}
Expand All @@ -142,8 +149,8 @@ func findDiffs(baseSunsets, specSunsets []*sunset.Sunset, baseSpecPath, specPath
Version: specSunset.Version,
BaseSunsetDate: "",
SpecSunsetDate: specSunset.SunsetDate,
BaseSpec: baseSpecPath,
Spec: specPath,
BaseSpec: o.basePath,
Spec: o.specPath,
Team: specSunset.Team,
})
}
Expand All @@ -156,7 +163,50 @@ func findDiffs(baseSunsets, specSunsets []*sunset.Sunset, baseSpecPath, specPath
return iKey < jKey
})

return diffs
// Filter diffs by date range if specified
filteredDiffs, err := o.diffsInRange(diffs)
if err != nil {
return nil, err
}

Copy link
Copy Markdown
Collaborator

@saisundar saisundar Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diffsInRange returns error but I don't think any branch ever sets it — the time.Parse failures just fall back to time.Time{}. Woud it be cleaner to drop the return type and the matching if err != nil plumbing in findDiffs / Run(), or actually propagate a wrapped error when the date string is malformed?

Copy link
Copy Markdown
Collaborator Author

@lovisaberggren lovisaberggren Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this is probably left from some changes I made.

I think we would want to distinguish between the date beling Nil/empty, and invalid. In the case of invalid we should probably return an error instead of silently fail. I'll make some changes 👍

return filteredDiffs, nil
}

func (o *DiffOpts) diffsInRange(diffs []*Diff) ([]*Diff, error) {
var out []*Diff

if o.from == "" && o.to == "" {
return diffs, nil
}

for _, d := range diffs {
baseSunsetDate, err := parseSunsetDate(d.BaseSunsetDate)
if err != nil {
return nil, err
}

specSunsetDate, err := parseSunsetDate(d.SpecSunsetDate)
if err != nil {
return nil, err
}

if isDateInRange(baseSunsetDate, o.fromDate, o.toDate) || isDateInRange(specSunsetDate, o.fromDate, o.toDate) {
out = append(out, d)
}
}

return out, nil
Copy link
Copy Markdown
Collaborator

@maks-m-mongo-leaf maks-m-mongo-leaf Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we support open intervals here (either of the date is not provided)? If this code returns a nil slice, would something break?
Do we have tests to cover ☝️ ☝️ ?

Copy link
Copy Markdown
Collaborator Author

@lovisaberggren lovisaberggren Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Qs!

Do we support open intervals here (either of the date is not provided)?

Hmm, we do however I don't think it's currently working as expected. When only one of the flags is provided, for example only from, I'd expect the behaviour to be "any date from the from date forward" and the other way around. I'll add tests to cover this case to ensure it works as expected

If this code returns a nil slice, would something break?

It wouldn't, it would either be an empty slice or one populated with values. err can be an error or nil

Copy link
Copy Markdown
Collaborator Author

@lovisaberggren lovisaberggren Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a small adjustment to make it work as I described above, and added tests to cover those cases. Let me know what you think! Ty

}

func parseSunsetDate(dateStr string) (*time.Time, error) {
if dateStr == "" {
return nil, nil
}
parsedDate, err := time.Parse("2006-01-02", dateStr)
if err != nil {
return nil, err
}
return &parsedDate, err
}

func makeKey(path, operation, version string) string {
Expand Down Expand Up @@ -186,6 +236,30 @@ func (o *DiffOpts) newSunsetDiffBytes(diffs []*Diff) ([]byte, error) {
return yamlData, nil
}

func (o *DiffOpts) validate() error {
Copy link
Copy Markdown
Collaborator

@saisundar saisundar Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick one: validate() doesn't check that from <= to. An inverted range (someone swaps the flags in the workflow YAML) will just return an empty diff, which looks the same as "no drift" and the CI job passes silently. Worth an early error like --from must not be after --to?

Copy link
Copy Markdown
Collaborator Author

@lovisaberggren lovisaberggren Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll add a check 👍

if o.from != "" {
value, err := time.Parse("2006-01-02", o.from)
if err != nil {
return err
}
o.fromDate = &value
}

if o.to != "" {
value, err := time.Parse("2006-01-02", o.to)
if err != nil {
return err
}
o.toDate = &value
}

if o.from != "" && o.to != "" && o.fromDate.After(*o.toDate) {
return fmt.Errorf("%s date cannot be after %s date", flag.From, flag.To)
}

return nil
}

// DiffBuilder builds the diff command with the following signature:
// sunset diff --base base-spec.json --spec spec.json.
func DiffBuilder() *cobra.Command {
Expand All @@ -197,6 +271,9 @@ func DiffBuilder() *cobra.Command {
Use: "diff --base spec1.json --spec spec2.json",
Short: "List API endpoints with different sunset dates between two OpenAPI specs.",
Args: cobra.NoArgs,
PreRunE: func(_ *cobra.Command, _ []string) error {
return opts.validate()
},
RunE: func(_ *cobra.Command, _ []string) error {
return opts.Run()
},
Expand All @@ -206,6 +283,8 @@ func DiffBuilder() *cobra.Command {
cmd.Flags().StringVarP(&opts.specPath, flag.Spec, flag.SpecShort, "", usage.Spec)
cmd.Flags().StringVarP(&opts.outputPath, flag.Output, flag.OutputShort, "", usage.Output)
cmd.Flags().StringVarP(&opts.format, flag.Format, flag.FormatShort, "json", usage.Format)
cmd.Flags().StringVar(&opts.from, flag.From, "", usage.From)
cmd.Flags().StringVar(&opts.to, flag.To, "", usage.To)

_ = cmd.MarkFlagRequired(flag.Base)
_ = cmd.MarkFlagRequired(flag.Spec)
Expand Down
Loading
Loading