diff --git a/README.md b/README.md index a37cd10..caeaa6f 100644 --- a/README.md +++ b/README.md @@ -224,7 +224,7 @@ sn get notes --title "Meeting Notes" --rich # Edit a note in $EDITOR sn edit note --title "Meeting Notes" -# See which notes the app marked as copies, then delete them +# Review duplicate notes, then delete all but the newest of each set sn delete duplicates --dry-run sn delete duplicates diff --git a/cmd/sncli/duplicates.go b/cmd/sncli/duplicates.go index 117a4cd..0c39766 100644 --- a/cmd/sncli/duplicates.go +++ b/cmd/sncli/duplicates.go @@ -10,9 +10,9 @@ import ( "github.com/urfave/cli/v2" ) -// maxDuplicatesListed caps how many duplicates are listed before the rest are -// summarised, so a large account does not fill the terminal. -const maxDuplicatesListed = 20 +// maxDuplicateGroupsListed caps how many sets of duplicates are listed before +// the rest are summarised, so a large account does not fill the terminal. +const maxDuplicateGroupsListed = 20 func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error { sess, _, err := cache.GetSession(common.NewHTTPClient(), opts.useSession, opts.sessKey, opts.server, opts.debug) @@ -35,21 +35,23 @@ func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error { return fmt.Errorf("failed to find duplicate notes: %w", err) } - writeDuplicatesKept(c, found.KeptOriginalMissing) + writeDuplicatesKept(c, found.KeptNoOriginal) - if len(found.Deleted) == 0 { + toDelete := found.Deleted() + + if len(toDelete) == 0 { _, _ = fmt.Fprintln(c.App.Writer, "no duplicate notes found") return nil } - writeDuplicatesFound(c, found.Deleted, dryRun) + writeDuplicateGroups(c, found.Groups, len(toDelete), dryRun) if dryRun { return nil } - if !c.Bool("yes") && !confirmDeleteDuplicates(c, len(found.Deleted)) { + if !c.Bool("yes") && !confirmDeleteDuplicates(c, len(toDelete)) { return nil } @@ -60,41 +62,47 @@ func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error { return fmt.Errorf("failed to delete duplicate notes: %w", err) } - _, _ = fmt.Fprintln(c.App.Writer, color.Green.Sprintf("%d duplicate notes deleted", len(out.Deleted))) + _, _ = fmt.Fprintln(c.App.Writer, color.Green.Sprintf("%d duplicate notes deleted", len(out.Deleted()))) return nil } -func writeDuplicatesFound(c *cli.Context, dups []sncli.DuplicateNote, dryRun bool) { - heading := fmt.Sprintf("%d duplicate notes to delete:", len(dups)) +func writeDuplicateGroups(c *cli.Context, groups []sncli.DuplicateGroup, numToDelete int, dryRun bool) { + heading := fmt.Sprintf("%d notes to delete, in %d sets of duplicates:", numToDelete, len(groups)) if dryRun { - heading = fmt.Sprintf("%d duplicate notes would be deleted:", len(dups)) + heading = fmt.Sprintf("%d notes would be deleted, in %d sets of duplicates:", numToDelete, len(groups)) } _, _ = fmt.Fprintln(c.App.Writer, heading) - for x, dup := range dups { - if x == maxDuplicatesListed { - _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more", len(dups)-maxDuplicatesListed)) + for x, group := range groups { + if x == maxDuplicateGroupsListed { + _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more sets", len(groups)-maxDuplicateGroupsListed)) break } - _, _ = fmt.Fprintf(c.App.Writer, " %s %s\n", dup.UUID, dup.Title) + _, _ = fmt.Fprintf(c.App.Writer, " keep %s %s %s\n", + group.Keep.UpdatedDate(), group.Keep.UUID, group.Keep.Title) + + for _, dup := range group.Delete { + _, _ = fmt.Fprintf(c.App.Writer, " %s %s %s %s\n", + color.Red.Sprint("delete"), dup.UpdatedDate(), dup.UUID, dup.Title) + } } } -func writeDuplicatesKept(c *cli.Context, kept []sncli.DuplicateNote) { +func writeDuplicatesKept(c *cli.Context, kept []sncli.NoteSummary) { if len(kept) == 0 { return } _, _ = fmt.Fprintln(c.App.Writer, color.Yellow.Sprintf( - "%d duplicates kept, as the note each was copied from is no longer in the account:", len(kept))) + "%d copies kept, as the note each was made from is no longer in the account:", len(kept))) for x, dup := range kept { - if x == maxDuplicatesListed { - _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more", len(kept)-maxDuplicatesListed)) + if x == maxDuplicateGroupsListed { + _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more", len(kept)-maxDuplicateGroupsListed)) break } @@ -104,7 +112,7 @@ func writeDuplicatesKept(c *cli.Context, kept []sncli.DuplicateNote) { } func confirmDeleteDuplicates(c *cli.Context, num int) bool { - _, _ = fmt.Fprintf(c.App.Writer, "delete %d duplicate notes? ", num) + _, _ = fmt.Fprintf(c.App.Writer, "delete %d notes? ", num) var input string diff --git a/internal/sncli/duplicates.go b/internal/sncli/duplicates.go index 86eedc2..094d67e 100644 --- a/internal/sncli/duplicates.go +++ b/internal/sncli/duplicates.go @@ -2,61 +2,156 @@ package sncli import ( "fmt" + "sort" + "time" "github.com/jonhadfield/gosn-v2/cache" - "github.com/jonhadfield/gosn-v2/common" "github.com/jonhadfield/gosn-v2/items" ) -// DuplicateNote is a note that Standard Notes marked as a copy of another, -// by setting duplicate_of when the note was duplicated. -type DuplicateNote struct { - UUID string - // Title of the duplicate itself +// NoteSummary identifies a note in duplicate output. +type NoteSummary struct { + UUID string Title string - // OriginalUUID is the note this one was copied from - OriginalUUID string - // OriginalPresent reports whether that original is still in the account. - // When it is not, deleting the duplicate would lose the content, so the - // duplicate is kept. - OriginalPresent bool + // UpdatedAt is the note's last-updated timestamp, as the API reports it. + UpdatedAt int64 + // UpdatedAtText is the same time as text, for display. + UpdatedAtText string + // Marked reports whether the note carries duplicate_of, meaning Standard + // Notes created it by duplicating another note. + Marked bool } -// FindDuplicateNotes returns the notes marked as duplicates, and whether the -// note each was copied from still exists. -func FindDuplicateNotes(in items.Items) []DuplicateNote { - live := make(map[string]struct{}) +// UpdatedDate returns the note's last-updated time as a date and time for +// display, or the raw value if it cannot be parsed. +func (n NoteSummary) UpdatedDate() string { + t, err := time.Parse(timeLayout, n.UpdatedAtText) + if err != nil { + return n.UpdatedAtText + } + + return t.Format("2006-01-02 15:04") +} + +// DuplicateGroup is a note and the copies made from it, linked by +// duplicate_of. The most recently updated note is kept and the rest deleted, +// so that editing a copy after duplicating it does not lose that work. +type DuplicateGroup struct { + Keep NoteSummary + Delete []NoteSummary +} + +func summariseNote(note *items.Note) NoteSummary { + return NoteSummary{ + UUID: note.GetUUID(), + Title: note.Content.GetTitle(), + UpdatedAt: note.GetUpdatedAtTimestamp(), + UpdatedAtText: note.GetUpdatedAt(), + Marked: note.GetDuplicateOf() != "", + } +} + +// newerThan reports whether a should be kept in preference to b. The most +// recently updated wins; on a tie an original is preferred over a copy, and +// then the lower UUID, so the choice is stable. +func newerThan(a, b NoteSummary) bool { + switch { + case a.UpdatedAt != b.UpdatedAt: + return a.UpdatedAt > b.UpdatedAt + case a.Marked != b.Marked: + return !a.Marked + default: + return a.UUID < b.UUID + } +} + +// FindDuplicateGroups groups notes with the copies made from them and reports +// which to keep. It also returns copies whose original is no longer in the +// account: those are the only remaining copy of their content, so they are +// left alone. +func FindDuplicateGroups(in items.Items) (groups []DuplicateGroup, keptNoOriginal []NoteSummary) { + live := make(map[string]*items.Note) for _, item := range in { - if item.GetContentType() == common.SNItemTypeNote && !item.IsDeleted() { - live[item.GetUUID()] = struct{}{} + if note, isNote := item.(*items.Note); isNote && !note.IsDeleted() { + live[note.GetUUID()] = note } } - var duplicates []DuplicateNote + // union-find over the live notes, joining each copy to the note it was + // made from, so that a copy of a copy ends up in one group + parent := make(map[string]string, len(live)) - for _, item := range in { - note, isNote := item.(*items.Note) - if !isNote || note.IsDeleted() || note.GetDuplicateOf() == "" { - continue + var find func(string) string + + find = func(u string) string { + if parent[u] != u { + parent[u] = find(parent[u]) } + return parent[u] + } + + for uuid := range live { + parent[uuid] = uuid + } + + for uuid, note := range live { original := note.GetDuplicateOf() - _, originalPresent := live[original] + if original == "" { + continue + } + + if _, ok := live[original]; !ok { + keptNoOriginal = append(keptNoOriginal, summariseNote(note)) + + continue + } - duplicates = append(duplicates, DuplicateNote{ - UUID: note.GetUUID(), - Title: note.Content.GetTitle(), - OriginalUUID: original, - OriginalPresent: originalPresent, - }) + parent[find(uuid)] = find(original) } - return duplicates + members := make(map[string][]NoteSummary) + + for uuid, note := range live { + root := find(uuid) + members[root] = append(members[root], summariseNote(note)) + } + + for _, group := range members { + if len(group) < 2 { + continue + } + + keep := group[0] + + for _, candidate := range group[1:] { + if newerThan(candidate, keep) { + keep = candidate + } + } + + var toDelete []NoteSummary + + for _, candidate := range group { + if candidate.UUID != keep.UUID { + toDelete = append(toDelete, candidate) + } + } + + sort.Slice(toDelete, func(i, j int) bool { return toDelete[i].UUID < toDelete[j].UUID }) + + groups = append(groups, DuplicateGroup{Keep: keep, Delete: toDelete}) + } + + sort.Slice(groups, func(i, j int) bool { return groups[i].Keep.UUID < groups[j].Keep.UUID }) + sort.Slice(keptNoOriginal, func(i, j int) bool { return keptNoOriginal[i].UUID < keptNoOriginal[j].UUID }) + + return groups, keptNoOriginal } -// DeleteDuplicateNotesConfig deletes notes marked as duplicates of another -// note. +// DeleteDuplicateNotesConfig deletes the superseded notes in each set of +// duplicates. type DeleteDuplicateNotesConfig struct { Session *cache.Session // DryRun reports what would be deleted without deleting anything. @@ -66,15 +161,27 @@ type DeleteDuplicateNotesConfig struct { // DeleteDuplicateNotesOutput describes what was, or would be, deleted. type DeleteDuplicateNotesOutput struct { - // Deleted holds the duplicates removed, or, for a dry run, those that - // would be removed. - Deleted []DuplicateNote - // KeptOriginalMissing holds duplicates left alone because the note they - // were copied from is no longer in the account. - KeptOriginalMissing []DuplicateNote + // Groups holds each set of duplicates, with the note kept and those + // deleted. + Groups []DuplicateGroup + // KeptNoOriginal holds copies left alone because the note they were made + // from is no longer in the account. + KeptNoOriginal []NoteSummary +} + +// Deleted returns every note deleted, across all groups. +func (o DeleteDuplicateNotesOutput) Deleted() []NoteSummary { + var all []NoteSummary + + for _, group := range o.Groups { + all = append(all, group.Delete...) + } + + return all } -// Run finds duplicates and, unless this is a dry run, deletes them. +// Run finds duplicates and, unless this is a dry run, deletes all but the most +// recently updated note in each set. func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) { var out DeleteDuplicateNotesOutput @@ -103,17 +210,11 @@ func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) { } } - for _, dup := range FindDuplicateNotes(all) { - if dup.OriginalPresent { - out.Deleted = append(out.Deleted, dup) - - continue - } + out.Groups, out.KeptNoOriginal = FindDuplicateGroups(all) - out.KeptOriginalMissing = append(out.KeptOriginalMissing, dup) - } + toDelete := out.Deleted() - if i.DryRun || len(out.Deleted) == 0 { + if i.DryRun || len(toDelete) == 0 { _ = i.Session.CacheDB.Close() return out, nil @@ -121,8 +222,8 @@ func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) { var notesToDelete items.Notes - for _, dup := range out.Deleted { - note := byUUID[dup.UUID] + for _, summary := range toDelete { + note := byUUID[summary.UUID] if note == nil || note.GetContent() == nil { continue } diff --git a/internal/sncli/duplicates_test.go b/internal/sncli/duplicates_test.go index f4a648e..b859f90 100644 --- a/internal/sncli/duplicates_test.go +++ b/internal/sncli/duplicates_test.go @@ -8,13 +8,14 @@ import ( "github.com/stretchr/testify/require" ) -func testNote(uuid, title, duplicateOf string, deleted bool) *items.Note { +func testNote(uuid, title, duplicateOf string, updatedAt int64, deleted bool) *items.Note { note := &items.Note{ ItemCommon: items.ItemCommon{ - UUID: uuid, - ContentType: common.SNItemTypeNote, - DuplicateOf: duplicateOf, - Deleted: deleted, + UUID: uuid, + ContentType: common.SNItemTypeNote, + DuplicateOf: duplicateOf, + UpdatedAtTimestamp: updatedAt, + Deleted: deleted, }, } note.Content.SetTitle(title) @@ -22,78 +23,154 @@ func testNote(uuid, title, duplicateOf string, deleted bool) *items.Note { return note } -func TestFindDuplicateNotesMarkedOnly(t *testing.T) { +func TestFindDuplicateGroupsKeepsNewerCopy(t *testing.T) { + // the copy was edited after it was made, so it holds the latest work in := items.Items{ - testNote("original", "Shopping", "", false), - testNote("copy", "Shopping copy", "original", false), - testNote("unrelated", "Ideas", "", false), + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 200, false), } - dups := FindDuplicateNotes(in) + groups, kept := FindDuplicateGroups(in) - require.Len(t, dups, 1) - require.Equal(t, "copy", dups[0].UUID) - require.Equal(t, "Shopping copy", dups[0].Title) - require.Equal(t, "original", dups[0].OriginalUUID) - require.True(t, dups[0].OriginalPresent) + require.Empty(t, kept) + require.Len(t, groups, 1) + require.Equal(t, "copy", groups[0].Keep.UUID) + require.Len(t, groups[0].Delete, 1) + require.Equal(t, "original", groups[0].Delete[0].UUID) } -// A duplicate whose original is gone is the only copy of that content left, -// so it must not be treated as safe to delete. -func TestFindDuplicateNotesOriginalMissing(t *testing.T) { +func TestFindDuplicateGroupsKeepsNewerOriginal(t *testing.T) { + // the usual case: the copy was never touched after being made in := items.Items{ - testNote("copy", "Shopping copy", "gone", false), + testNote("original", "Shopping", "", 300, false), + testNote("copy", "Shopping copy", "original", 200, false), } - dups := FindDuplicateNotes(in) + groups, _ := FindDuplicateGroups(in) - require.Len(t, dups, 1) - require.False(t, dups[0].OriginalPresent) + require.Len(t, groups, 1) + require.Equal(t, "original", groups[0].Keep.UUID) + require.Equal(t, "copy", groups[0].Delete[0].UUID) } -func TestFindDuplicateNotesOriginalDeleted(t *testing.T) { +func TestFindDuplicateGroupsTiePrefersOriginal(t *testing.T) { + // identical timestamps: keep the note that is not marked as a copy in := items.Items{ - testNote("original", "Shopping", "", true), - testNote("copy", "Shopping copy", "original", false), + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 100, false), } - dups := FindDuplicateNotes(in) + groups, _ := FindDuplicateGroups(in) - require.Len(t, dups, 1) - require.False(t, dups[0].OriginalPresent, "a deleted original should not count as present") + require.Len(t, groups, 1) + require.Equal(t, "original", groups[0].Keep.UUID) } -func TestFindDuplicateNotesSkipsDeletedDuplicates(t *testing.T) { +// A copy whose original is gone is the only note holding that content, so it +// must not be deleted. +func TestFindDuplicateGroupsOriginalMissing(t *testing.T) { in := items.Items{ - testNote("original", "Shopping", "", false), - testNote("copy", "Shopping copy", "original", true), + testNote("copy", "Shopping copy", "gone", 100, false), } - require.Empty(t, FindDuplicateNotes(in)) + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, groups) + require.Len(t, kept, 1) + require.Equal(t, "copy", kept[0].UUID) } -func TestFindDuplicateNotesNone(t *testing.T) { +func TestFindDuplicateGroupsOriginalDeleted(t *testing.T) { in := items.Items{ - testNote("a", "One", "", false), - testNote("b", "Two", "", false), + testNote("original", "Shopping", "", 100, true), + testNote("copy", "Shopping copy", "original", 200, false), } - require.Empty(t, FindDuplicateNotes(in)) + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, groups, "a deleted original is not a live duplicate") + require.Len(t, kept, 1) } -func TestFindDuplicateNotesChain(t *testing.T) { - // a copy of a copy: both are duplicates, and both originals are present +func TestFindDuplicateGroupsSkipsDeletedCopies(t *testing.T) { in := items.Items{ - testNote("original", "Shopping", "", false), - testNote("copy", "Shopping copy", "original", false), - testNote("copy2", "Shopping copy 2", "copy", false), + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 200, true), } - dups := FindDuplicateNotes(in) + groups, kept := FindDuplicateGroups(in) - require.Len(t, dups, 2) + require.Empty(t, groups) + require.Empty(t, kept) +} - for _, dup := range dups { - require.True(t, dup.OriginalPresent) +func TestFindDuplicateGroupsNone(t *testing.T) { + in := items.Items{ + testNote("a", "One", "", 100, false), + testNote("b", "Two", "", 200, false), } + + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, groups) + require.Empty(t, kept) +} + +// A copy of a copy belongs to one set, so only the newest of the three is +// kept. +func TestFindDuplicateGroupsChain(t *testing.T) { + in := items.Items{ + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 400, false), + testNote("copy2", "Shopping copy 2", "copy", 200, false), + } + + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, kept) + require.Len(t, groups, 1) + require.Equal(t, "copy", groups[0].Keep.UUID) + require.Len(t, groups[0].Delete, 2) + require.Equal(t, []string{"copy2", "original"}, + []string{groups[0].Delete[0].UUID, groups[0].Delete[1].UUID}) +} + +// Two unrelated sets of duplicates stay separate. +func TestFindDuplicateGroupsSeparateSets(t *testing.T) { + in := items.Items{ + testNote("a", "One", "", 100, false), + testNote("a-copy", "One copy", "a", 200, false), + testNote("b", "Two", "", 300, false), + testNote("b-copy", "Two copy", "b", 100, false), + } + + groups, _ := FindDuplicateGroups(in) + + require.Len(t, groups, 2) + + keep := map[string]string{} + for _, g := range groups { + keep[g.Keep.UUID] = g.Delete[0].UUID + } + + require.Equal(t, map[string]string{"a-copy": "a", "b": "b-copy"}, keep) +} + +func TestDeleteDuplicateNotesOutputDeleted(t *testing.T) { + out := DeleteDuplicateNotesOutput{ + Groups: []DuplicateGroup{ + {Keep: NoteSummary{UUID: "a"}, Delete: []NoteSummary{{UUID: "a-copy"}}}, + {Keep: NoteSummary{UUID: "b"}, Delete: []NoteSummary{{UUID: "b-copy"}, {UUID: "b-copy2"}}}, + }, + } + + require.Len(t, out.Deleted(), 3) +} + +func TestNoteSummaryUpdatedDate(t *testing.T) { + require.Equal(t, "2026-09-21 14:30", + NoteSummary{UpdatedAtText: "2026-09-21T14:30:00.000Z"}.UpdatedDate()) + + // an unparseable value is shown as it came from the API + require.Equal(t, "not a time", NoteSummary{UpdatedAtText: "not a time"}.UpdatedDate()) }