diff --git a/main.go b/main.go index 98bb707..7b73b2e 100644 --- a/main.go +++ b/main.go @@ -553,7 +553,7 @@ func (a *App) runImport(ctx context.Context, cmd *cli.Command) error { a.log.Info("Fetched existing records", slog.Int("count", len(existingRecords))) published, _ := storage.GetPublished(authClient.DID()) - newRecords := sync.FilterNew(records, existingRecords, published) + newRecords := sync.FilterNew(records, existingRecords, published, tolerance) skippedCount := len(records) - len(newRecords) a.log.Info("Filtered to new records", slog.Int("count", len(newRecords)), diff --git a/sync/publish_test.go b/sync/publish_test.go index 37ee409..f7b2e97 100644 --- a/sync/publish_test.go +++ b/sync/publish_test.go @@ -351,9 +351,13 @@ func TestIteratePublished(t *testing.T) { return } - for i, key := range gotKeys { - if key != tt.wantKeys[i] { - t.Errorf("IteratePublished() key[%d] = %s, want %s", i, key, tt.wantKeys[i]) + wantSet := make(map[string]bool) + for _, k := range tt.wantKeys { + wantSet[k] = true + } + for _, k := range gotKeys { + if !wantSet[k] { + t.Errorf("IteratePublished() got unexpected key %s", k) } } }) diff --git a/sync/record.go b/sync/record.go index 165801a..27054e1 100644 --- a/sync/record.go +++ b/sync/record.go @@ -149,22 +149,31 @@ func CreateRecordKeys(records []*PlayRecord) []string { return keys } -func FilterNew(records []*PlayRecord, existing []ExistingRecord, processed map[string]bool) []*PlayRecord { - existingKeys := make(map[string]bool) +func FilterNew(records []*PlayRecord, existing []ExistingRecord, processed map[string]bool, tolerance time.Duration) []*PlayRecord { + existingSet := make(map[*PlayRecord]bool) for _, rec := range existing { - key := CreateRecordKey(rec.Value) - if key == "|||" { - continue - } - existingKeys[key] = true + existingSet[rec.Value] = true } var newRecords []*PlayRecord for _, record := range records { - key := CreateRecordKey(record) - if !existingKeys[key] && !processed[key] { - newRecords = append(newRecords, record) + if processed != nil && processed[CreateRecordKey(record)] { + continue + } + + if len(existingSet) > 0 { + isDup := false + for existingRec := range existingSet { + if record.sameAs(existingRec, tolerance) { + isDup = true + break + } + } + if isDup { + continue + } } + newRecords = append(newRecords, record) } return newRecords } diff --git a/sync/sync_test.go b/sync/sync_test.go index aa4925a..7921184 100644 --- a/sync/sync_test.go +++ b/sync/sync_test.go @@ -126,88 +126,242 @@ func TestPrepareWritesManyCollisions(t *testing.T) { } } -func TestFilterNewExcludesExisting(t *testing.T) { - records := []*PlayRecord{ +func TestFilterNew(t *testing.T) { + baseTime := time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC) + processedKey := CreateRecordKey(&PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}) + + tests := []struct { + name string + records []*PlayRecord + existing []ExistingRecord + processed map[string]bool + tolerance time.Duration + wantNewCount int + wantNewTracks []string + }{ { - TrackName: "Song A", - Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, - PlayedTime: Timestamp{Time: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC)}, + name: "excludes exact matches", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + {TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Minute)}}, + {TrackName: "Song C", Artists: []PlayRecordArtist{{ArtistName: "Artist C"}}, PlayedTime: Timestamp{Time: baseTime.Add(2 * time.Minute)}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Minute)}}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 2, + wantNewTracks: []string{"Song A", "Song C"}, }, { - TrackName: "Song B", - Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, - PlayedTime: Timestamp{Time: time.Date(2024, 1, 15, 10, 1, 0, 0, time.UTC)}, + name: "returns all when none exist", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + {TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Minute)}}, + }, + existing: []ExistingRecord{}, + tolerance: 5 * time.Minute, + wantNewCount: 2, + wantNewTracks: []string{"Song A", "Song B"}, }, { - TrackName: "Song C", - Artists: []PlayRecordArtist{{ArtistName: "Artist C"}}, - PlayedTime: Timestamp{Time: time.Date(2024, 1, 15, 10, 2, 0, 0, time.UTC)}, + name: "detects duplicates within tolerance", + records: []*PlayRecord{ + {TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime.Add(30 * time.Second)}, MusicServiceBaseDomain: MusicServiceSpotify}, + {TrackName: "Different Song", Artists: []PlayRecordArtist{{ArtistName: "Different Artist"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Hour)}, MusicServiceBaseDomain: MusicServiceSpotify}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime}, MusicServiceBaseDomain: MusicServiceLastFM}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Different Song"}, }, - } - - existing := []ExistingRecord{ { - URI: "at://did:example:user/fm.teal.alpha.feed.play/abc123", - CID: "bafyreabc123", - Value: &PlayRecord{ - TrackName: "Song B", - Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, - PlayedTime: Timestamp{Time: time.Date(2024, 1, 15, 10, 1, 0, 0, time.UTC)}, + name: "excludes via processed map", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + {TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Minute)}}, + }, + existing: []ExistingRecord{}, + processed: map[string]bool{ + processedKey: true, }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song B"}, }, - } - - newRecords := FilterNew(records, existing, nil) - - if len(newRecords) != 2 { - t.Errorf("len(newRecords) = %d, want 2", len(newRecords)) - } - - foundSongA := false - foundSongB := false - foundSongC := false - for _, rec := range newRecords { - switch rec.TrackName { - case "Song A": - foundSongA = true - case "Song B": - foundSongB = true - case "Song C": - foundSongC = true - } - } - - if !foundSongA { - t.Error("Song A should be in new records") - } - if foundSongB { - t.Error("Song B should not be in new records (it exists)") - } - if !foundSongC { - t.Error("Song C should be in new records") - } -} - -func TestFilterNewReturnsAllWhenNoneExist(t *testing.T) { - records := []*PlayRecord{ { - TrackName: "Song A", - Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, - PlayedTime: Timestamp{Time: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC)}, + name: "empty records returns nothing", + records: []*PlayRecord{}, + existing: []ExistingRecord{{URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}}, + tolerance: 5 * time.Minute, + wantNewCount: 0, + wantNewTracks: []string{}, }, { - TrackName: "Song B", - Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, - PlayedTime: Timestamp{Time: time.Date(2024, 1, 15, 10, 1, 0, 0, time.UTC)}, + name: "nil processed map works", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + }, + existing: []ExistingRecord{}, + processed: nil, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song A"}, + }, + { + name: "nil existing records works", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + }, + existing: nil, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song A"}, + }, + { + name: "zero tolerance requires exact time match", + records: []*PlayRecord{ + {TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime}}, + {TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Second)}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + tolerance: 0, + wantNewCount: 1, + wantNewTracks: []string{"Same Song"}, + }, + { + name: "matches multiple existing records", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + {TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Minute)}}, + {TrackName: "Song C", Artists: []PlayRecordArtist{{ArtistName: "Artist C"}}, PlayedTime: Timestamp{Time: baseTime.Add(2 * time.Minute)}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}, + {URI: "at://did:example/user/play/def", Value: &PlayRecord{TrackName: "Song C", Artists: []PlayRecordArtist{{ArtistName: "Artist C"}}, PlayedTime: Timestamp{Time: baseTime.Add(2 * time.Minute)}}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song B"}, + }, + { + name: "time at exact tolerance boundary matches", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime.Add(5 * time.Minute)}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 0, + wantNewTracks: []string{}, + }, + { + name: "time just beyond tolerance does not match", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime.Add(5*time.Minute + time.Second)}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song A"}, + }, + { + name: "different artist does not match", + records: []*PlayRecord{ + {TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Same Song", Artists: []PlayRecordArtist{{ArtistName: "Different Artist"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Same Song"}, + }, + { + name: "different track does not match", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Same Artist"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song A"}, + }, + { + name: "processed takes precedence over existing check", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + processed: map[string]bool{ + processedKey: true, + }, + tolerance: 5 * time.Minute, + wantNewCount: 0, + wantNewTracks: []string{}, + }, + { + name: "same_record_processed_and_matches_existing_returns_nothing", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + processed: map[string]bool{ + processedKey: true, + }, + tolerance: 5 * time.Minute, + wantNewCount: 0, + wantNewTracks: []string{}, + }, + { + name: "processed_skips_record_regardless_of_existing", + records: []*PlayRecord{ + {TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}, + {TrackName: "Song B", Artists: []PlayRecordArtist{{ArtistName: "Artist B"}}, PlayedTime: Timestamp{Time: baseTime.Add(time.Minute)}}, + }, + existing: []ExistingRecord{ + {URI: "at://did:example/user/play/abc", Value: &PlayRecord{TrackName: "Song A", Artists: []PlayRecordArtist{{ArtistName: "Artist A"}}, PlayedTime: Timestamp{Time: baseTime}}}, + }, + processed: map[string]bool{ + processedKey: true, + }, + tolerance: 5 * time.Minute, + wantNewCount: 1, + wantNewTracks: []string{"Song B"}, }, } - existing := []ExistingRecord{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + newRecords := FilterNew(tt.records, tt.existing, tt.processed, tt.tolerance) - newRecords := FilterNew(records, existing, nil) + if len(newRecords) != tt.wantNewCount { + t.Errorf("FilterNew() returned %d records, want %d", len(newRecords), tt.wantNewCount) + } - if len(newRecords) != 2 { - t.Errorf("len(newRecords) = %d, want 2", len(newRecords)) + wantSet := make(map[string]bool) + for _, tr := range tt.wantNewTracks { + wantSet[tr] = true + } + for _, rec := range newRecords { + if !wantSet[rec.TrackName] { + t.Errorf("FilterNew() returned unexpected track %q", rec.TrackName) + } + } + }) } }