diff --git a/backend/internal/garmin/mock/mock.go b/backend/internal/garmin/mock/mock.go index 26e38d3..ddeac98 100644 --- a/backend/internal/garmin/mock/mock.go +++ b/backend/internal/garmin/mock/mock.go @@ -10,11 +10,17 @@ import ( // Client is a fake garmin.Client returning data supplied by the test/caller. type Client struct { - AuthResults []garmin.AuthResult // consumed in order by Authenticate/CompleteMFA calls - Activities []garmin.Activity - Splits map[int64]garmin.ActivitySplits - Details map[int64]garmin.ActivityDetails - Workouts map[int64]garmin.Workout + AuthResults []garmin.AuthResult // consumed in order by Authenticate/CompleteMFA calls + Activities []garmin.Activity + Splits map[int64]garmin.ActivitySplits + Details map[int64]garmin.ActivityDetails + Workouts map[int64]garmin.Workout + // WorkoutErrByID, if set for a given workout ID, makes GetWorkoutByID + // return that error for that ID specifically -- independent of the + // all-calls-fail Err field below -- so a test can simulate one + // activity's workout fetch failing while others in the same batch + // succeed. + WorkoutErrByID map[int64]error Err error // if set, every call returns this error authResultCursor int ClosedCalled bool @@ -79,6 +85,9 @@ func (c *Client) GetWorkoutByID(ctx context.Context, workoutID int64) (garmin.Wo if c.Err != nil { return garmin.Workout{}, c.Err } + if err, ok := c.WorkoutErrByID[workoutID]; ok { + return garmin.Workout{}, err + } return c.Workouts[workoutID], nil } diff --git a/backend/internal/sync/mapping.go b/backend/internal/sync/mapping.go index 0484ded..dbf2af6 100644 --- a/backend/internal/sync/mapping.go +++ b/backend/internal/sync/mapping.go @@ -94,9 +94,9 @@ func toLapRows(laps []garmin.Lap, samples []garmin.Sample, targets []*garmin.Wor return rows } -// alignWorkoutTargets zips an activity's recorded laps against its -// structured workout's flattened steps, returning one *garmin.WorkoutStep -// per lap (nil where unavailable). +// alignWorkoutTargets zips an activity's lap count against its structured +// workout's flattened steps, returning one *garmin.WorkoutStep per lap (nil +// where unavailable). // // Confirmed against a real activity (via Garmin Connect's own workout view) // that recording sometimes continues one lap past the end of the workout's @@ -110,11 +110,11 @@ func toLapRows(laps []garmin.Lap, samples []garmin.Sample, targets []*garmin.Wor // Any other mismatch (extra manual laps, auto-lap-by-distance also firing, // etc.) can't be trusted at all, so every entry comes back nil rather than // risk showing a target against the wrong lap. -func alignWorkoutTargets(laps []garmin.Lap, workout garmin.Workout) []*garmin.WorkoutStep { +func alignWorkoutTargets(lapCount int, workout garmin.Workout) []*garmin.WorkoutStep { steps := workout.FlattenSteps() - out := make([]*garmin.WorkoutStep, len(laps)) + out := make([]*garmin.WorkoutStep, lapCount) - switch len(laps) - len(steps) { + switch lapCount - len(steps) { case 0, 1: for i := range steps { s := steps[i] diff --git a/backend/internal/sync/service.go b/backend/internal/sync/service.go index 7be0f41..4d3297d 100644 --- a/backend/internal/sync/service.go +++ b/backend/internal/sync/service.go @@ -54,9 +54,23 @@ func (c Config) withDefaults() Config { return c } -// Progress reports how far a currently-running (or just-finished) -// FillPendingDetails pass has gotten, for a status banner to poll. +// Phase values reported by Progress.Phase. +const ( + PhaseIdle = "idle" + PhaseDiscovering = "discovering" + PhaseActivities = "activities" + PhaseWorkouts = "workouts" +) + +// Progress reports how far a currently-running (or just-finished) FullSync +// has gotten, for a status modal to poll. PhaseDiscovering (backfillCore/ +// incrementalSyncCore) has no meaningful Total -- discovering how many +// activities exist IS the act of fetching them, so it's reported as an +// indeterminate step (Done/Total both 0) rather than a fake percentage. +// PhaseActivities/PhaseWorkouts do have a real Total, taken once from a +// local DB count at the start of each pass (see FillPendingDetails). type Progress struct { + Phase string Done int Total int } @@ -80,19 +94,20 @@ func NewService(g garmin.Client, db *store.DB, userID int64, cfg Config, now fun if now == nil { now = time.Now } - return &Service{garmin: g, db: db, userID: userID, cfg: cfg.withDefaults(), now: now} + return &Service{garmin: g, db: db, userID: userID, cfg: cfg.withDefaults(), now: now, progress: Progress{Phase: PhaseIdle}} } -// Progress returns the current detail-fill progress (0/0 when idle). +// Progress returns the current sync progress (phase idle, 0/0 when nothing +// is running). func (s *Service) Progress() Progress { s.progressMu.Lock() defer s.progressMu.Unlock() return s.progress } -func (s *Service) setProgress(done, total int) { +func (s *Service) setProgress(phase string, done, total int) { s.progressMu.Lock() - s.progress = Progress{Done: done, Total: total} + s.progress = Progress{Phase: phase, Done: done, Total: total} s.progressMu.Unlock() } @@ -199,6 +214,11 @@ func (s *Service) FullSync(ctx context.Context, detailFillLimit int) error { if err != nil { return err } + // Reset to idle on every return path, including an early error return + // from backfillCore/incrementalSyncCore before FillPendingDetails (which + // otherwise owns its own idle-reset) ever runs. + s.setProgress(PhaseDiscovering, 0, 0) + defer s.setProgress(PhaseIdle, 0, 0) backfillCount, err := s.backfillCore(ctx) if err != nil { @@ -270,11 +290,22 @@ func (s *Service) fetchAndStoreWindow(ctx context.Context, startDate, endDate st return len(activities), newCount, nil } -// FillPendingDetails fetches get_activity_splits/get_activity_details for up -// to limit activities that don't have them yet, then (re)classifies each one. -// Calls are made sequentially with Config.InterCallDelay between them to +// FillPendingDetails fetches activity details/splits for up to limit +// activities missing them, then fetches workouts for up to limit activities +// missing those (independently -- see ActivitiesMissingWorkout), then +// (re)classifies every activity touched by the first pass. Each pass makes +// its Garmin calls sequentially with Config.InterCallDelay between them to // avoid Garmin/Cloudflare rate limiting. func (s *Service) FillPendingDetails(ctx context.Context, limit int) error { + defer s.setProgress(PhaseIdle, 0, 0) + + if err := s.fillPendingActivityDetails(ctx, limit); err != nil { + return err + } + return s.fillPendingWorkouts(ctx, limit) +} + +func (s *Service) fillPendingActivityDetails(ctx context.Context, limit int) error { pending, err := s.db.ActivitiesMissingDetails(ctx, s.userID, limit) if err != nil { return err @@ -284,8 +315,7 @@ func (s *Service) FillPendingDetails(ctx context.Context, limit int) error { return fmt.Errorf("load profile: %w", err) } - s.setProgress(0, len(pending)) - defer s.setProgress(0, 0) + s.setProgress(PhaseActivities, 0, len(pending)) for i, a := range pending { if i > 0 { @@ -301,7 +331,41 @@ func (s *Service) FillPendingDetails(ctx context.Context, limit int) error { if err := s.ClassifyActivity(ctx, a.ID); err != nil { return fmt.Errorf("classify activity %d: %w", a.GarminActivityID, err) } - s.setProgress(i+1, len(pending)) + s.setProgress(PhaseActivities, i+1, len(pending)) + } + return nil +} + +// fillPendingWorkouts fetches get_workout_by_id for up to limit activities +// missing it. Unlike fillPendingActivityDetails, one activity's workout +// fetch failing is non-fatal (logged, loop continues) -- workout target +// bands are enrichment, not core activity data, and workout_raw_json +// staying NULL means ActivitiesMissingWorkout will naturally retry it on +// the next sync. +func (s *Service) fillPendingWorkouts(ctx context.Context, limit int) error { + pending, err := s.db.ActivitiesMissingWorkout(ctx, s.userID, limit) + if err != nil { + return err + } + profile, err := s.db.GetProfile(ctx, s.userID) + if err != nil { + return fmt.Errorf("load profile: %w", err) + } + + s.setProgress(PhaseWorkouts, 0, len(pending)) + + for i, a := range pending { + if i > 0 { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(s.cfg.InterCallDelay): + } + } + if err := s.fillActivityWorkout(ctx, a, profile); err != nil { + log.Printf("sync: fill workout for activity %d failed, will retry next sync: %v", a.GarminActivityID, err) + } + s.setProgress(PhaseWorkouts, i+1, len(pending)) } return nil } @@ -316,23 +380,14 @@ func (s *Service) fillActivityDetails(ctx context.Context, a store.Activity, pro return fmt.Errorf("get_activity_details: %w", err) } - targets := make([]*garmin.WorkoutStep, len(splits.Laps)) - if a.WorkoutID != nil { - workout, err := s.garmin.GetWorkoutByID(ctx, *a.WorkoutID) - if err != nil { - log.Printf("sync: get_workout_by_id(%d) for activity %d failed, continuing without target zones: %v", *a.WorkoutID, a.GarminActivityID, err) - } else { - targets = alignWorkoutTargets(splits.Laps, workout) - if err := s.db.SetActivityWorkout(ctx, s.userID, a.ID, string(workout.Raw)); err != nil { - return err - } - } - } - samples := garmin.ExtractSamples(details) if err := s.db.ReplaceActivitySamples(ctx, s.userID, a.ID, toSampleRows(samples)); err != nil { return err } + // No workout-target alignment here -- that's fillActivityWorkout's job, + // run as its own later pass (see fillPendingWorkouts). targets is an + // all-nil placeholder the same length as splits.Laps. + targets := make([]*garmin.WorkoutStep, len(splits.Laps)) if err := s.db.ReplaceLaps(ctx, s.userID, a.ID, toLapRows(splits.Laps, samples, targets, profile)); err != nil { return err } @@ -342,6 +397,35 @@ func (s *Service) fillActivityDetails(ctx context.Context, a store.Activity, pro return s.db.SetActivitySplitsFetched(ctx, s.userID, a.ID) } +// fillActivityWorkout fetches a's structured workout and re-derives its +// laps' target pace/HR bands from it. Requires a.WorkoutID to be set -- +// only ever called for activities ActivitiesMissingWorkout returned, which +// already filters on that. Reads laps back from the DB (already written by +// fillActivityDetails, in some earlier pass or run) rather than needing the +// original garmin.Lap data again, since alignWorkoutTargets only needs a +// count. +func (s *Service) fillActivityWorkout(ctx context.Context, a store.Activity, profile store.Profile) error { + workout, err := s.garmin.GetWorkoutByID(ctx, *a.WorkoutID) + if err != nil { + return fmt.Errorf("get_workout_by_id: %w", err) + } + laps, err := s.db.LapsForActivity(ctx, s.userID, a.ID) + if err != nil { + return err + } + targets := alignWorkoutTargets(len(laps), workout) + for i := range laps { + if i < len(targets) && targets[i] != nil { + laps[i].TargetPaceLowMps, laps[i].TargetPaceHighMps = targetPaceRange(*targets[i]) + laps[i].TargetHRLowBpm, laps[i].TargetHRHighBpm = targetHRRange(*targets[i], profile) + } + } + if err := s.db.ReplaceLaps(ctx, s.userID, a.ID, laps); err != nil { + return err + } + return s.db.SetActivityWorkout(ctx, s.userID, a.ID, string(workout.Raw)) +} + // ClassifyActivity (re)runs the rule engine for one activity against the // currently active workout kinds and appends a new kind_assignments row. // Safe to call repeatedly (e.g. after editing a workout kind's rule). diff --git a/backend/internal/sync/service_test.go b/backend/internal/sync/service_test.go index f85d064..490053e 100644 --- a/backend/internal/sync/service_test.go +++ b/backend/internal/sync/service_test.go @@ -2,6 +2,7 @@ package sync import ( "context" + "fmt" "path/filepath" "testing" "time" @@ -92,7 +93,7 @@ func TestAlignWorkoutTargets_MatchesLapsToFlattenedSteps(t *testing.T) { }}, }} - targets := alignWorkoutTargets(laps, workout) + targets := alignWorkoutTargets(len(laps), workout) if len(targets) != 2 { t.Fatalf("expected 2 aligned targets, got %d", len(targets)) } @@ -118,7 +119,7 @@ func TestAlignWorkoutTargets_OneExtraTrailingLapKeepsOtherTargets(t *testing.T) }}, }} - targets := alignWorkoutTargets(laps, workout) + targets := alignWorkoutTargets(len(laps), workout) if len(targets) != 3 { t.Fatalf("expected 3 slots (one per lap), got %d", len(targets)) } @@ -141,7 +142,7 @@ func TestAlignWorkoutTargets_MismatchedCountYieldsAllNil(t *testing.T) { }}, }} - targets := alignWorkoutTargets(laps, workout) + targets := alignWorkoutTargets(len(laps), workout) if len(targets) != 3 { t.Fatalf("expected 3 slots (one per lap), got %d", len(targets)) } @@ -629,24 +630,27 @@ func TestFullSync_RecordsOneCombinedSyncRun(t *testing.T) { } } -func TestFillPendingDetails_ReportsLiveProgress(t *testing.T) { +func TestFillPendingDetails_ReportsPhaseAwareProgressThroughActivitiesAndWorkoutsPhases(t *testing.T) { db := openTestDB(t) ctx := context.Background() userID := provisionTestUser(t, db) + const n = 2 m := &mock.Client{ Activities: []garmin.Activity{}, Splits: map[int64]garmin.ActivitySplits{}, Details: map[int64]garmin.ActivityDetails{}, + Workouts: map[int64]garmin.Workout{}, } - const n = 3 for i := int64(1); i <= n; i++ { + workoutID := i + 100 m.Activities = append(m.Activities, garmin.Activity{ - ActivityID: i, ActivityType: garmin.ActivityType{TypeKey: "running"}, + ActivityID: i, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &workoutID, StartTimeGMT: "2026-07-0" + string(rune('0'+i)) + " 06:00:00", Distance: 5000, Duration: 1500, }) m.Splits[i] = garmin.ActivitySplits{ActivityID: i} m.Details[i] = garmin.ActivityDetails{ActivityID: i} + m.Workouts[workoutID] = garmin.Workout{} } svc := NewService(m, db, userID, Config{InterCallDelay: 150 * time.Millisecond}, @@ -655,27 +659,171 @@ func TestFillPendingDetails_ReportsLiveProgress(t *testing.T) { t.Fatalf("backfillCore: %v", err) } - if p := svc.Progress(); p.Total != 0 { - t.Fatalf("Progress before FillPendingDetails = %+v, want zero value", p) + if p := svc.Progress(); p.Phase != PhaseIdle { + t.Fatalf("Progress before FillPendingDetails = %+v, want PhaseIdle", p) } done := make(chan error, 1) go func() { done <- svc.FillPendingDetails(ctx, n) }() - time.Sleep(200 * time.Millisecond) // into the delay before the 2nd or 3rd item - mid := svc.Progress() - if mid.Total != n { - t.Errorf("mid-flight Progress().Total = %d, want %d", mid.Total, n) + // Timeline with InterCallDelay=150ms and n=2 per phase: activities pass + // does item0 (instant), delays ~150ms, does item1 -- landing here around + // t=75ms should be mid-delay in the activities phase. + time.Sleep(75 * time.Millisecond) + midActivities := svc.Progress() + if midActivities.Phase != PhaseActivities { + t.Errorf("mid-activities Progress().Phase = %q, want %q", midActivities.Phase, PhaseActivities) } - if mid.Done <= 0 || mid.Done >= n { - t.Errorf("mid-flight Progress().Done = %d, want strictly between 0 and %d (i.e. actually in progress)", mid.Done, n) + if midActivities.Total != n { + t.Errorf("mid-activities Progress().Total = %d, want %d", midActivities.Total, n) + } + if midActivities.Done != 1 { + t.Errorf("mid-activities Progress().Done = %d, want 1", midActivities.Done) + } + + // The activities phase finishes its own delay+item1 around t=150ms, then + // the workouts phase starts: item0 (instant), delay ~150ms, item1. + // Landing around t=150+75=225ms should be mid-delay in the workouts phase. + time.Sleep(150 * time.Millisecond) + midWorkouts := svc.Progress() + if midWorkouts.Phase != PhaseWorkouts { + t.Errorf("mid-workouts Progress().Phase = %q, want %q", midWorkouts.Phase, PhaseWorkouts) + } + if midWorkouts.Total != n { + t.Errorf("mid-workouts Progress().Total = %d, want %d", midWorkouts.Total, n) + } + if midWorkouts.Done != 1 { + t.Errorf("mid-workouts Progress().Done = %d, want 1", midWorkouts.Done) } if err := <-done; err != nil { t.Fatalf("FillPendingDetails: %v", err) } - if final := svc.Progress(); final.Total != 0 || final.Done != 0 { - t.Errorf("Progress after completion = %+v, want zero value (idle)", final) + if final := svc.Progress(); final.Phase != PhaseIdle || final.Total != 0 || final.Done != 0 { + t.Errorf("Progress after completion = %+v, want zero-value PhaseIdle", final) + } +} + +func TestFillPendingDetails_RetriesWorkoutFetchForActivityWithDetailsAlreadyFetched(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const garminActivityID = 77 + const workoutID = 888 + workoutIDPtr := int64(workoutID) + id, err := db.UpsertActivity(ctx, userID, store.Activity{ + GarminActivityID: garminActivityID, WorkoutID: &workoutIDPtr, + StartTimeUTC: "2026-07-01 06:00:00", RawJSON: "{}", + }) + if err != nil { + t.Fatalf("UpsertActivity: %v", err) + } + // Simulate a prior run that successfully fetched details/splits but + // whose workout fetch failed (workout_raw_json stays NULL). + if err := db.SetActivityDetails(ctx, userID, id, "{}"); err != nil { + t.Fatalf("SetActivityDetails: %v", err) + } + if err := db.SetActivitySplitsFetched(ctx, userID, id); err != nil { + t.Fatalf("SetActivitySplitsFetched: %v", err) + } + if err := db.ReplaceLaps(ctx, userID, id, []store.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}); err != nil { + t.Fatalf("ReplaceLaps: %v", err) + } + + m := &mock.Client{ + Workouts: map[int64]garmin.Workout{ + workoutID: {Segments: []garmin.WorkoutSegment{ + {Steps: []garmin.WorkoutStep{ + {Type: "ExecutableStepDTO", TargetType: garmin.WorkoutTargetType{TypeKey: "pace.zone"}, TargetValueOne: f(3.0), TargetValueTwo: f(3.5)}, + }}, + }}, + }, + } + svc := NewService(m, db, userID, Config{}, fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + + // A prior FillPendingDetails call (which had ActivitiesMissingDetails + // return nothing, since details are already fetched) still reaches this + // activity via ActivitiesMissingWorkout. + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("FillPendingDetails: %v", err) + } + + laps, err := db.LapsForActivity(ctx, userID, id) + if err != nil || len(laps) != 1 { + t.Fatalf("LapsForActivity: %v, %+v", err, laps) + } + if laps[0].TargetPaceLowMps == nil || *laps[0].TargetPaceLowMps != 3.0 { + t.Errorf("TargetPaceLowMps = %v, want 3.0 (workout fetch should have been retried)", laps[0].TargetPaceLowMps) + } +} + +func TestFillPendingDetails_OneWorkoutFetchFailureDoesNotAbortTheWorkoutsPass(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userID := provisionTestUser(t, db) + + const failingWorkoutID = 200 + const okWorkoutID = 201 + failingPtr, okPtr := int64(failingWorkoutID), int64(okWorkoutID) + m := &mock.Client{ + Activities: []garmin.Activity{ + {ActivityID: 1, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &failingPtr, + StartTimeGMT: "2026-07-01 06:00:00", Distance: 5000, Duration: 1500}, + {ActivityID: 2, ActivityType: garmin.ActivityType{TypeKey: "running"}, WorkoutID: &okPtr, + StartTimeGMT: "2026-07-02 06:00:00", Distance: 5000, Duration: 1500}, + }, + Splits: map[int64]garmin.ActivitySplits{ + 1: {ActivityID: 1, Laps: []garmin.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}}, + 2: {ActivityID: 2, Laps: []garmin.Lap{{LapIndex: 1, IntensityType: "ACTIVE"}}}, + }, + Details: map[int64]garmin.ActivityDetails{1: {ActivityID: 1}, 2: {ActivityID: 2}}, + Workouts: map[int64]garmin.Workout{ + okWorkoutID: {Segments: []garmin.WorkoutSegment{ + {Steps: []garmin.WorkoutStep{ + {Type: "ExecutableStepDTO", TargetType: garmin.WorkoutTargetType{TypeKey: "pace.zone"}, TargetValueOne: f(3.0), TargetValueTwo: f(3.5)}, + }}, + }}, + }, + WorkoutErrByID: map[int64]error{failingWorkoutID: fmt.Errorf("garmin says no")}, + } + // Not timing-sensitive -- keep InterCallDelay negligible so the 2 + // activities/2 workouts here don't cost real wall-clock seconds + // (Config{}'s default is 1s per gap). + svc := NewService(m, db, userID, Config{InterCallDelay: time.Millisecond}, + fixedNow(time.Date(2026, 7, 11, 0, 0, 0, 0, time.UTC))) + + if _, err := svc.backfillCore(ctx); err != nil { + t.Fatalf("backfillCore: %v", err) + } + if err := svc.FillPendingDetails(ctx, 10); err != nil { + t.Fatalf("FillPendingDetails should not return an error for a per-activity workout fetch failure: %v", err) + } + + remaining, err := db.CountActivitiesMissingWorkout(ctx, userID) + if err != nil { + t.Fatalf("CountActivitiesMissingWorkout: %v", err) + } + if remaining != 1 { + t.Errorf("CountActivitiesMissingWorkout = %d, want 1 (the failing activity stays pending, retryable next sync)", remaining) + } + + activities, err := db.ListActivities(ctx, userID, store.ActivityFilter{}) + if err != nil { + t.Fatalf("ListActivities: %v", err) + } + var okActivity store.Activity + for _, a := range activities { + if a.GarminActivityID == 2 { + okActivity = a + } + } + laps, err := db.LapsForActivity(ctx, userID, okActivity.ID) + if err != nil || len(laps) != 1 { + t.Fatalf("LapsForActivity: %v, %+v", err, laps) + } + if laps[0].TargetPaceLowMps == nil || *laps[0].TargetPaceLowMps != 3.0 { + t.Errorf("the succeeding activity's TargetPaceLowMps = %v, want 3.0 (should not be affected by the other activity's failure)", laps[0].TargetPaceLowMps) } }