kriss/geniusrun
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(sync): remove dead Backfill/IncrementalSync exported wrappers

Browse Source 
Both were only reachable via the periodic background sync loop removed
in 4d2cbe4 -- nothing in production calls them anymore, only tests did.
FullSync already calls backfillCore/incrementalSyncCore directly.
Rewrite the affected tests to call the *Core functions (still exported
within the package) instead, dropping the now-redundant standalone
SyncRun-recording assertion covered by TestFullSync_RecordsOneCombinedSyncRun.
This commit is contained in:
Kriss Vila
2026-07-27 06:44:39 +02:00
parent 9d65c50068
commit 7204a48c1c
5 changed files with 57 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
backend/internal/store/isolation_test.go
View File

@@ -137,7 +137,7 @@ func TestIsolation_SyncStateAndRunsNeverLeakAcrossUsers(t *testing.T) {
t.Fatalf("userA's UpdateSyncState leaked into userB's sync_state: %+v", stateB) t.Fatalf("userA's UpdateSyncState leaked into userB's sync_state: %+v", stateB)
} }
runID, err := db.StartSyncRun(ctx, userA, SyncKindBackfill) runID, err := db.StartSyncRun(ctx, userA, SyncKindFull)
if err != nil { if err != nil {
t.Fatalf("StartSyncRun(a): %v", err) t.Fatalf("StartSyncRun(a): %v", err)
} }

10
backend/internal/store/syncruns.go
View File

@@ -7,12 +7,10 @@ import (
) )
const ( const (
SyncKindBackfill = "backfill" // SyncKindFull is a manually-triggered "Sync now" pass: backfillCore
SyncKindIncremental = "incremental" // followed by incrementalSyncCore followed by FillPendingDetails,
// SyncKindFull is a manually-triggered "Sync now" pass: Backfill followed // recorded as one run so the reported activity count covers the whole
// by IncrementalSync followed by FillPendingDetails, recorded as one run // action instead of only whichever stage happened to finish last.
// so the reported activity count covers the whole action instead of only
// whichever stage happened to finish last.
SyncKindFull = "full" SyncKindFull = "full"
SyncStatusRunning = "running" SyncStatusRunning = "running"

2
backend/internal/store/users_test.go
View File

@@ -126,7 +126,7 @@ func TestDeleteUser_RemovesUserAndCascadesEverything(t *testing.T) {
if err := db.UpdateSyncState(ctx, userID, "2020-01-01", true); err != nil { if err := db.UpdateSyncState(ctx, userID, "2020-01-01", true); err != nil {
t.Fatalf("UpdateSyncState: %v", err) t.Fatalf("UpdateSyncState: %v", err)
} }
if _, err := db.StartSyncRun(ctx, userID, SyncKindBackfill); err != nil { if _, err := db.StartSyncRun(ctx, userID, SyncKindFull); err != nil {
t.Fatalf("StartSyncRun: %v", err) t.Fatalf("StartSyncRun: %v", err)
} }

64
backend/internal/sync/service.go
View File

@@ -96,7 +96,7 @@ func (s *Service) setProgress(done, total int) {
s.progressMu.Unlock() s.progressMu.Unlock()
} }
// Backfill pages backward in Config.BackfillWindowDays windows until // backfillCore pages backward in Config.BackfillWindowDays windows until
// Profile.BackfillHorizonDays is reached or Garmin returns an empty page. // Profile.BackfillHorizonDays is reached or Garmin returns an empty page.
// The horizon is read fresh from the profile on every call (not fixed at // The horizon is read fresh from the profile on every call (not fixed at
// server startup), so a user-edited value takes effect on the very next // server startup), so a user-edited value takes effect on the very next
@@ -106,27 +106,10 @@ func (s *Service) setProgress(done, total int) {
// completed backfill, or is a fast no-op if the configured horizon is // completed backfill, or is a fast no-op if the configured horizon is
// already fully covered -- it does not re-walk years of already-known // already fully covered -- it does not re-walk years of already-known
// history. Widening the horizon between calls resumes further back instead // history. Widening the horizon between calls resumes further back instead
// of re-fetching everything. // of re-fetching everything. Used by FullSync as one step of its single
func (s *Service) Backfill(ctx context.Context) error { // combined SyncRun; there is no standalone entrypoint for this anymore
runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindBackfill) // (the periodic background sync loop that used to call one is gone -- see
if err != nil { // 4d2cbe4 refactor: remove automatic background incremental sync).
return err
}
total, err := s.backfillCore(ctx)
if err != nil {
msg := err.Error()
s.db.FinishSyncRun(ctx, s.userID, runID, total, &msg)
return err
}
return s.db.FinishSyncRun(ctx, s.userID, runID, total, nil)
}
// backfillCore holds Backfill's actual fetch logic, without the SyncRun
// bookkeeping, so FullSync can run it as one step of a single combined run
// instead of its own separately-recorded one. The returned count reflects
// whatever was fetched even when an error is also returned, matching
// Backfill's own partial-progress-on-error behavior.
func (s *Service) backfillCore(ctx context.Context) (int, error) { func (s *Service) backfillCore(ctx context.Context) (int, error) {
profile, err := s.db.GetProfile(ctx, s.userID) profile, err := s.db.GetProfile(ctx, s.userID)
if err != nil { if err != nil {
@@ -191,25 +174,10 @@ func (s *Service) backfillCore(ctx context.Context) (int, error) {
return total, nil return total, nil
} }
// IncrementalSync fetches activities from just before the latest known // incrementalSyncCore fetches activities from just before the latest known
// activity (or a short recent window if none exist yet) through today. // activity (or a short recent window if none exist yet) through today. Used
func (s *Service) IncrementalSync(ctx context.Context) error { // by FullSync as one step of its single combined SyncRun -- see
runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindIncremental) // backfillCore's comment for why there's no standalone entrypoint.
if err != nil {
return err
}
n, err := s.incrementalSyncCore(ctx)
if err != nil {
msg := err.Error()
s.db.FinishSyncRun(ctx, s.userID, runID, n, &msg)
return err
}
return s.db.FinishSyncRun(ctx, s.userID, runID, n, nil)
}
// incrementalSyncCore holds IncrementalSync's actual fetch logic, without
// the SyncRun bookkeeping -- see backfillCore.
func (s *Service) incrementalSyncCore(ctx context.Context) (int, error) { func (s *Service) incrementalSyncCore(ctx context.Context) (int, error) {
start := s.now().AddDate(0, 0, -s.cfg.IncrementalOverlapDays) start := s.now().AddDate(0, 0, -s.cfg.IncrementalOverlapDays)
if latest, ok, err := s.db.LatestActivityStartTime(ctx, s.userID); err == nil && ok { if latest, ok, err := s.db.LatestActivityStartTime(ctx, s.userID); err == nil && ok {
@@ -221,15 +189,11 @@ func (s *Service) incrementalSyncCore(ctx context.Context) (int, error) {
return newCount, err return newCount, err
} }
// FullSync performs a complete manual "Sync now" pass -- Backfill (resumes // FullSync performs a complete manual "Sync now" pass -- backfillCore
// from the watermark), then IncrementalSync (catches anything new since the // (resumes from the watermark), then incrementalSyncCore (catches anything
// latest known activity), then FillPendingDetails -- recorded as a single // new since the latest known activity), then FillPendingDetails --
// SyncRun. Backfill and IncrementalSync each record their own SyncRun when // recorded as a single SyncRun so the reported activity count covers the
// called on their own (used by the periodic background loop), but a manual // whole action instead of only whichever stage happened to finish last.
// sync runs both back to back, and FillPendingDetails records no run at all;
// showing the user only the most recently *recorded* run (IncrementalSync's)
// would silently hide however many activities Backfill fetched. Recording
// one combined run makes the reported count match the whole action.
func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error { func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error {
runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindFull) runID, err := s.db.StartSyncRun(ctx, s.userID, store.SyncKindFull)
if err != nil { if err != nil {

78
backend/internal/sync/service_test.go
View File

@@ -52,7 +52,7 @@ func setBackfillHorizon(t *testing.T, db *store.DB, userID int64, days int) {
} }
} }
func TestBackfill_StoresActivitiesAndRecordsSyncRun(t *testing.T) { func TestBackfillCore_StoresActivities(t *testing.T) {
db := openTestDB(t) db := openTestDB(t)
ctx := context.Background() ctx := context.Background()
userID := provisionTestUser(t, db) userID := provisionTestUser(t, db)
@@ -63,8 +63,12 @@ func TestBackfill_StoresActivitiesAndRecordsSyncRun(t *testing.T) {
}} }}
svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svc.Backfill(ctx); err != nil { total, err := svc.backfillCore(ctx)
t.Fatalf("Backfill: %v", err) if err != nil {
t.Fatalf("backfillCore: %v", err)
}
if total != 1 {
t.Errorf("backfillCore returned total = %d, want 1", total)
} }
activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{})
@@ -77,14 +81,6 @@ func TestBackfill_StoresActivitiesAndRecordsSyncRun(t *testing.T) {
if activities[0].GarminActivityID != 1 { if activities[0].GarminActivityID != 1 {
t.Errorf("GarminActivityID = %d, want 1", activities[0].GarminActivityID) t.Errorf("GarminActivityID = %d, want 1", activities[0].GarminActivityID)
} }
runs, err := db.ListSyncRuns(ctx, userID, 10)
if err != nil {
t.Fatalf("ListSyncRuns: %v", err)
}
if len(runs) != 1 || runs[0].Status != store.SyncStatusSuccess {
t.Fatalf("expected 1 successful sync run, got %+v", runs)
}
} }
func TestAlignWorkoutTargets_MatchesLapsToFlattenedSteps(t *testing.T) { func TestAlignWorkoutTargets_MatchesLapsToFlattenedSteps(t *testing.T) {
@@ -195,7 +191,7 @@ func TestTargetHRRange_CustomRangeAndZoneNumberViaKarvonen(t *testing.T) {
} }
} }
func TestBackfill_SkipsNonRunningActivities(t *testing.T) { func TestBackfillCore_SkipsNonRunningActivities(t *testing.T) {
db := openTestDB(t) db := openTestDB(t)
ctx := context.Background() ctx := context.Background()
userID := provisionTestUser(t, db) userID := provisionTestUser(t, db)
@@ -212,8 +208,8 @@ func TestBackfill_SkipsNonRunningActivities(t *testing.T) {
}} }}
svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("Backfill: %v", err) t.Fatalf("backfillCore: %v", err)
} }
activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{})
@@ -258,8 +254,8 @@ func TestFillPendingDetailsAndClassify_EndToEnd(t *testing.T) {
} }
svc := NewService(m, db, userID, Config{MinConfidence: 0.5}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svc := NewService(m, db, userID, Config{MinConfidence: 0.5}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("Backfill: %v", err) t.Fatalf("backfillCore: %v", err)
} }
// A workout kind that should cleanly match the seeded activity's pace. // A workout kind that should cleanly match the seeded activity's pace.
@@ -328,8 +324,8 @@ func TestFillPendingDetails_ResolvesWorkoutTargetsOntoLaps(t *testing.T) {
} }
svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("Backfill: %v", err) t.Fatalf("backfillCore: %v", err)
} }
if err := svc.FillPendingDetails(ctx, 10); err != nil { if err := svc.FillPendingDetails(ctx, 10); err != nil {
t.Fatalf("FillPendingDetails: %v", err) t.Fatalf("FillPendingDetails: %v", err)
@@ -385,8 +381,8 @@ func TestFillPendingDetails_OneExtraTrailingLapKeepsOtherLapsTargets(t *testing.
} }
svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("Backfill: %v", err) t.Fatalf("backfillCore: %v", err)
} }
if err := svc.FillPendingDetails(ctx, 10); err != nil { if err := svc.FillPendingDetails(ctx, 10); err != nil {
t.Fatalf("FillPendingDetails: %v", err) t.Fatalf("FillPendingDetails: %v", err)
@@ -454,7 +450,7 @@ func TestBuildMetricContext_DerivesIsRace(t *testing.T) {
} }
} }
func TestBackfill_SecondRunIsANoOpOnceHorizonFullyCovered(t *testing.T) { func TestBackfillCore_SecondRunIsANoOpOnceHorizonFullyCovered(t *testing.T) {
db := openTestDB(t) db := openTestDB(t)
ctx := context.Background() ctx := context.Background()
userID := provisionTestUser(t, db) userID := provisionTestUser(t, db)
@@ -466,8 +462,8 @@ func TestBackfill_SecondRunIsANoOpOnceHorizonFullyCovered(t *testing.T) {
fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
setBackfillHorizon(t, db, userID, 10) setBackfillHorizon(t, db, userID, 10)
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("first Backfill: %v", err) t.Fatalf("first backfillCore: %v", err)
} }
firstCallCount := m.GetActivitiesCalls firstCallCount := m.GetActivitiesCalls
if firstCallCount == 0 { if firstCallCount == 0 {
@@ -482,11 +478,11 @@ func TestBackfill_SecondRunIsANoOpOnceHorizonFullyCovered(t *testing.T) {
t.Fatalf("expected backfill_complete=true after covering the full horizon, got %+v", state) t.Fatalf("expected backfill_complete=true after covering the full horizon, got %+v", state)
} }
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("second Backfill: %v", err) t.Fatalf("second backfillCore: %v", err)
} }
if m.GetActivitiesCalls != firstCallCount { if m.GetActivitiesCalls != firstCallCount {
t.Errorf("second Backfill made %d more GetActivities call(s); want 0 (should be a no-op once horizon is covered)", t.Errorf("second backfillCore made %d more GetActivities call(s); want 0 (should be a no-op once horizon is covered)",
m.GetActivitiesCalls-firstCallCount) m.GetActivitiesCalls-firstCallCount)
} }
} }
@@ -503,8 +499,8 @@ func TestResetAll_AllowsFreshBackfillAfterwards(t *testing.T) {
fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
setBackfillHorizon(t, db, userID, 10) setBackfillHorizon(t, db, userID, 10)
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("first Backfill: %v", err) t.Fatalf("first backfillCore: %v", err)
} }
firstCallCount := m.GetActivitiesCalls firstCallCount := m.GetActivitiesCalls
@@ -519,8 +515,8 @@ func TestResetAll_AllowsFreshBackfillAfterwards(t *testing.T) {
t.Fatalf("expected 0 activities after ResetAll, got %d", len(activities)) t.Fatalf("expected 0 activities after ResetAll, got %d", len(activities))
} }
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("Backfill after reset: %v", err) t.Fatalf("backfillCore after reset: %v", err)
} }
if m.GetActivitiesCalls <= firstCallCount { if m.GetActivitiesCalls <= firstCallCount {
t.Errorf("expected Backfill after ResetAll to call GetActivities again (fresh pull), call count stayed at %d", m.GetActivitiesCalls) t.Errorf("expected Backfill after ResetAll to call GetActivities again (fresh pull), call count stayed at %d", m.GetActivitiesCalls)
@@ -534,7 +530,7 @@ func TestResetAll_AllowsFreshBackfillAfterwards(t *testing.T) {
} }
} }
func TestBackfill_ResumesFromWatermarkWhenHorizonGrows(t *testing.T) { func TestBackfillCore_ResumesFromWatermarkWhenHorizonGrows(t *testing.T) {
db := openTestDB(t) db := openTestDB(t)
ctx := context.Background() ctx := context.Background()
userID := provisionTestUser(t, db) userID := provisionTestUser(t, db)
@@ -546,8 +542,8 @@ func TestBackfill_ResumesFromWatermarkWhenHorizonGrows(t *testing.T) {
svc := NewService(m, db, userID, Config{BackfillWindowDays: 10}, now) svc := NewService(m, db, userID, Config{BackfillWindowDays: 10}, now)
setBackfillHorizon(t, db, userID, 10) setBackfillHorizon(t, db, userID, 10)
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("first Backfill: %v", err) t.Fatalf("first backfillCore: %v", err)
} }
firstCallCount := m.GetActivitiesCalls firstCallCount := m.GetActivitiesCalls
@@ -556,8 +552,8 @@ func TestBackfill_ResumesFromWatermarkWhenHorizonGrows(t *testing.T) {
// make progress toward the new, deeper horizon. // make progress toward the new, deeper horizon.
setBackfillHorizon(t, db, userID, 30) setBackfillHorizon(t, db, userID, 30)
svc2 := NewService(m, db, userID, Config{BackfillWindowDays: 10}, now) svc2 := NewService(m, db, userID, Config{BackfillWindowDays: 10}, now)
if err := svc2.Backfill(ctx); err != nil { if _, err := svc2.backfillCore(ctx); err != nil {
t.Fatalf("second Backfill: %v", err) t.Fatalf("second backfillCore: %v", err)
} }
if m.GetActivitiesCalls <= firstCallCount { if m.GetActivitiesCalls <= firstCallCount {
t.Errorf("expected additional GetActivities calls when horizon grows, got %d total (was %d)", m.GetActivitiesCalls, firstCallCount) t.Errorf("expected additional GetActivities calls when horizon grows, got %d total (was %d)", m.GetActivitiesCalls, firstCallCount)
@@ -655,8 +651,8 @@ func TestFillPendingDetails_ReportsLiveProgress(t *testing.T) {
svc := NewService(m, db, userID, Config{InterCallDelay: 150 * time.Millisecond}, svc := NewService(m, db, userID, Config{InterCallDelay: 150 * time.Millisecond},
fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svc.Backfill(ctx); err != nil { if _, err := svc.backfillCore(ctx); err != nil {
t.Fatalf("Backfill: %v", err) t.Fatalf("backfillCore: %v", err)
} }
if p := svc.Progress(); p.Total != 0 { if p := svc.Progress(); p.Total != 0 {
@@ -704,11 +700,11 @@ func TestService_TwoUsersSyncIndependently(t *testing.T) {
svcA := NewService(mA, db, userA, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svcA := NewService(mA, db, userA, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
svcB := NewService(mB, db, userB, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) svcB := NewService(mB, db, userB, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC)))
if err := svcA.Backfill(ctx); err != nil { if _, err := svcA.backfillCore(ctx); err != nil {
t.Fatalf("Backfill(a): %v", err) t.Fatalf("backfillCore(a): %v", err)
} }
if err := svcB.Backfill(ctx); err != nil { if _, err := svcB.backfillCore(ctx); err != nil {
t.Fatalf("Backfill(b): %v", err) t.Fatalf("backfillCore(b): %v", err)
} }
activitiesA, err := db.ListActivities(ctx, userA, store.ActivityFilter{}) activitiesA, err := db.ListActivities(ctx, userA, store.ActivityFilter{})