From 47dbf4e446ba937837825148cb8f5737f26e1763 Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Sat, 25 Jul 2026 17:27:51 +0200 Subject: [PATCH] store: scope kind assignments, laps, and samples via activity ownership None of these three tables gained their own user_id column -- they're always accessed through a specific activity, so ownership is checked via a join/subquery against activities.user_id instead. --- backend/internal/store/assignments.go | 109 ++++++++++++++++---------- backend/internal/store/laps.go | 35 ++++++--- backend/internal/store/reset_test.go | 8 +- backend/internal/store/samples.go | 32 ++++++-- backend/internal/store/store_test.go | 54 ++++++++++--- 5 files changed, 163 insertions(+), 75 deletions(-) diff --git a/backend/internal/store/assignments.go b/backend/internal/store/assignments.go index 3e4631a..0561fab 100644 --- a/backend/internal/store/assignments.go +++ b/backend/internal/store/assignments.go @@ -28,8 +28,18 @@ type KindAssignment struct { CreatedAt string } -// InsertKindAssignment appends a new assignment row for an activity. -func (db *DB) InsertKindAssignment(ctx context.Context, a KindAssignment) (int64, error) { +// InsertKindAssignment appends a new assignment row for an activity owned +// by userID. +func (db *DB) InsertKindAssignment(ctx context.Context, userID int64, a KindAssignment) (int64, error) { + var exists int + err := db.QueryRowContext(ctx, `SELECT 1 FROM activities WHERE id = ? AND user_id = ?`, a.ActivityID, userID).Scan(&exists) + if err == sql.ErrNoRows { + return 0, fmt.Errorf("insert kind assignment: activity %d not found for user %d", a.ActivityID, userID) + } + if err != nil { + return 0, fmt.Errorf("insert kind assignment for activity %d (user %d): %w", a.ActivityID, userID, err) + } + res, err := db.ExecContext(ctx, ` INSERT INTO kind_assignments (activity_id, workout_kind_id, assignment_source, status, confidence, candidate_kinds_json) VALUES (?,?,?,?,?,?)`, @@ -46,29 +56,37 @@ func scanKindAssignment(row interface{ Scan(...any) error }) (KindAssignment, er return a, err } -const kindAssignmentColumns = `id, activity_id, workout_kind_id, assignment_source, status, confidence, candidate_kinds_json, created_at` +const kindAssignmentColumns = `a.id, a.activity_id, a.workout_kind_id, a.assignment_source, a.status, a.confidence, a.candidate_kinds_json, a.created_at` -// CurrentAssignment returns the latest assignment for an activity, if any. -func (db *DB) CurrentAssignment(ctx context.Context, activityID int64) (KindAssignment, bool, error) { - row := db.QueryRowContext(ctx, `SELECT `+kindAssignmentColumns+` FROM current_kind_assignment WHERE activity_id = ?`, activityID) +// CurrentAssignment returns the latest assignment for an activity owned by +// userID, if any. +func (db *DB) CurrentAssignment(ctx context.Context, userID, activityID int64) (KindAssignment, bool, error) { + row := db.QueryRowContext(ctx, ` + SELECT `+kindAssignmentColumns+` + FROM current_kind_assignment a + JOIN activities ON activities.id = a.activity_id + WHERE a.activity_id = ? AND activities.user_id = ?`, activityID, userID) a, err := scanKindAssignment(row) if err == sql.ErrNoRows { return KindAssignment{}, false, nil } if err != nil { - return KindAssignment{}, false, fmt.Errorf("current assignment for activity %d: %w", activityID, err) + return KindAssignment{}, false, fmt.Errorf("current assignment for activity %d (user %d): %w", activityID, userID, err) } return a, true, nil } -// ReviewQueue returns activities whose current assignment status is -// needs_review, newest first. -func (db *DB) ReviewQueue(ctx context.Context) ([]KindAssignment, error) { +// ReviewQueue returns userID's activities whose current assignment status +// is needs_review, newest first. +func (db *DB) ReviewQueue(ctx context.Context, userID int64) ([]KindAssignment, error) { rows, err := db.QueryContext(ctx, ` - SELECT `+kindAssignmentColumns+` FROM current_kind_assignment - WHERE status = ? ORDER BY created_at DESC`, AssignmentStatusNeedsReview) + SELECT `+kindAssignmentColumns+` + FROM current_kind_assignment a + JOIN activities ON activities.id = a.activity_id + WHERE activities.user_id = ? AND a.status = ? + ORDER BY a.created_at DESC`, userID, AssignmentStatusNeedsReview) if err != nil { - return nil, fmt.Errorf("review queue: %w", err) + return nil, fmt.Errorf("review queue for user %d: %w", userID, err) } defer rows.Close() @@ -83,37 +101,44 @@ func (db *DB) ReviewQueue(ctx context.Context) ([]KindAssignment, error) { return assignments, rows.Err() } -// AllCurrentAssignments returns the latest assignment for every activity -// that has one, regardless of status or source -- the basis for deciding -// which activities a global reclassify pass is allowed to touch. -func (db *DB) AllCurrentAssignments(ctx context.Context) ([]KindAssignment, error) { - rows, err := db.QueryContext(ctx, `SELECT `+kindAssignmentColumns+` FROM current_kind_assignment`) - if err != nil { - return nil, fmt.Errorf("all current assignments: %w", err) - } - defer rows.Close() - - assignments := []KindAssignment{} - for rows.Next() { - a, err := scanKindAssignment(rows) - if err != nil { - return nil, fmt.Errorf("scan kind assignment row: %w", err) - } - assignments = append(assignments, a) - } - return assignments, rows.Err() -} - -// AssignmentsForKind returns every historical assignment where the given -// workout kind was the resolved kind (regardless of source), oldest first -- -// the basis for progression-over-time charts. -func (db *DB) AssignmentsForKind(ctx context.Context, workoutKindID int64) ([]KindAssignment, error) { +// AllCurrentAssignments returns the latest assignment for every one of +// userID's activities that has one, regardless of status or source -- the +// basis for deciding which activities a global reclassify pass may touch. +func (db *DB) AllCurrentAssignments(ctx context.Context, userID int64) ([]KindAssignment, error) { rows, err := db.QueryContext(ctx, ` - SELECT `+kindAssignmentColumns+` FROM current_kind_assignment - WHERE workout_kind_id = ? AND status = ? ORDER BY created_at ASC`, - workoutKindID, AssignmentStatusAssigned) + SELECT `+kindAssignmentColumns+` + FROM current_kind_assignment a + JOIN activities ON activities.id = a.activity_id + WHERE activities.user_id = ?`, userID) if err != nil { - return nil, fmt.Errorf("assignments for kind %d: %w", workoutKindID, err) + return nil, fmt.Errorf("all current assignments for user %d: %w", userID, err) + } + defer rows.Close() + + assignments := []KindAssignment{} + for rows.Next() { + a, err := scanKindAssignment(rows) + if err != nil { + return nil, fmt.Errorf("scan kind assignment row: %w", err) + } + assignments = append(assignments, a) + } + return assignments, rows.Err() +} + +// AssignmentsForKind returns every historical assignment (for userID's +// activities) where the given workout kind was the resolved kind (regardless +// of source), oldest first -- the basis for progression-over-time charts. +func (db *DB) AssignmentsForKind(ctx context.Context, userID, workoutKindID int64) ([]KindAssignment, error) { + rows, err := db.QueryContext(ctx, ` + SELECT `+kindAssignmentColumns+` + FROM current_kind_assignment a + JOIN activities ON activities.id = a.activity_id + WHERE activities.user_id = ? AND a.workout_kind_id = ? AND a.status = ? + ORDER BY a.created_at ASC`, + userID, workoutKindID, AssignmentStatusAssigned) + if err != nil { + return nil, fmt.Errorf("assignments for kind %d (user %d): %w", workoutKindID, userID, err) } defer rows.Close() diff --git a/backend/internal/store/laps.go b/backend/internal/store/laps.go index 5d72942..08ede5e 100644 --- a/backend/internal/store/laps.go +++ b/backend/internal/store/laps.go @@ -2,6 +2,7 @@ package store import ( "context" + "database/sql" "fmt" ) @@ -35,9 +36,19 @@ type Lap struct { RawJSON string } -// ReplaceLaps deletes any existing laps for activityID and inserts the given -// set, so re-syncing an activity's splits is idempotent. -func (db *DB) ReplaceLaps(ctx context.Context, activityID int64, laps []Lap) error { +// ReplaceLaps deletes any existing laps for activityID (owned by userID) +// and inserts the given set, so re-syncing an activity's splits is +// idempotent. +func (db *DB) ReplaceLaps(ctx context.Context, userID, activityID int64, laps []Lap) error { + var exists int + err := db.QueryRowContext(ctx, `SELECT 1 FROM activities WHERE id = ? AND user_id = ?`, activityID, userID).Scan(&exists) + if err == sql.ErrNoRows { + return fmt.Errorf("replace laps: activity %d not found for user %d", activityID, userID) + } + if err != nil { + return fmt.Errorf("replace laps for activity %d (user %d): %w", activityID, userID, err) + } + tx, err := db.BeginTx(ctx, nil) if err != nil { return fmt.Errorf("begin replace laps tx: %w", err) @@ -66,15 +77,19 @@ func (db *DB) ReplaceLaps(ctx context.Context, activityID int64, laps []Lap) err return tx.Commit() } -// LapsForActivity returns all laps for an activity, ordered by lap_index. -func (db *DB) LapsForActivity(ctx context.Context, activityID int64) ([]Lap, error) { +// LapsForActivity returns all laps for an activity owned by userID, +// ordered by lap_index. +func (db *DB) LapsForActivity(ctx context.Context, userID, activityID int64) ([]Lap, error) { rows, err := db.QueryContext(ctx, ` - SELECT id, activity_id, lap_index, avg_speed_mps, - intensity_type, hr_drift_bpm_per_min, hr_recovery_bpm_per_min, - target_pace_low_mps, target_pace_high_mps, target_hr_low_bpm, target_hr_high_bpm, raw_json - FROM laps WHERE activity_id = ? ORDER BY lap_index`, activityID) + SELECT laps.id, laps.activity_id, laps.lap_index, laps.avg_speed_mps, + laps.intensity_type, laps.hr_drift_bpm_per_min, laps.hr_recovery_bpm_per_min, + laps.target_pace_low_mps, laps.target_pace_high_mps, laps.target_hr_low_bpm, laps.target_hr_high_bpm, laps.raw_json + FROM laps + JOIN activities ON activities.id = laps.activity_id + WHERE laps.activity_id = ? AND activities.user_id = ? + ORDER BY laps.lap_index`, activityID, userID) if err != nil { - return nil, fmt.Errorf("laps for activity %d: %w", activityID, err) + return nil, fmt.Errorf("laps for activity %d (user %d): %w", activityID, userID, err) } defer rows.Close() diff --git a/backend/internal/store/reset_test.go b/backend/internal/store/reset_test.go index ab9d47b..23065b1 100644 --- a/backend/internal/store/reset_test.go +++ b/backend/internal/store/reset_test.go @@ -21,10 +21,10 @@ func TestResetAllSyncedData_DeletesActivitiesCascadesAndRewindsWatermark(t *test if err != nil { t.Fatalf("CreateWorkoutKind: %v", err) } - if err := db.ReplaceLaps(ctx, activityID, []Lap{{LapIndex: 1}}); err != nil { + if err := db.ReplaceLaps(ctx, userID, activityID, []Lap{{LapIndex: 1}}); err != nil { t.Fatalf("ReplaceLaps: %v", err) } - if _, err := db.InsertKindAssignment(ctx, KindAssignment{ + if _, err := db.InsertKindAssignment(ctx, userID, KindAssignment{ ActivityID: activityID, WorkoutKindID: &kindID, AssignmentSource: AssignmentSourceRuleEngine, Status: AssignmentStatusAssigned, CandidateKindsJSON: "[]", }); err != nil { @@ -45,14 +45,14 @@ func TestResetAllSyncedData_DeletesActivitiesCascadesAndRewindsWatermark(t *test if len(activities) != 0 { t.Errorf("expected 0 activities after reset, got %d", len(activities)) } - laps, err := db.LapsForActivity(ctx, activityID) + laps, err := db.LapsForActivity(ctx, userID, activityID) if err != nil { t.Fatalf("LapsForActivity: %v", err) } if len(laps) != 0 { t.Errorf("expected laps to cascade-delete, got %d", len(laps)) } - if _, ok, err := db.CurrentAssignment(ctx, activityID); err != nil || ok { + if _, ok, err := db.CurrentAssignment(ctx, userID, activityID); err != nil || ok { t.Errorf("expected kind assignment to cascade-delete, ok=%v err=%v", ok, err) } diff --git a/backend/internal/store/samples.go b/backend/internal/store/samples.go index 57d70a5..3fefa0f 100644 --- a/backend/internal/store/samples.go +++ b/backend/internal/store/samples.go @@ -2,6 +2,7 @@ package store import ( "context" + "database/sql" "fmt" ) @@ -15,9 +16,19 @@ type Sample struct { ElevationM *float64 } -// ReplaceActivitySamples deletes any existing samples for activityID and -// bulk-inserts the given set, so re-syncing an activity's details is idempotent. -func (db *DB) ReplaceActivitySamples(ctx context.Context, activityID int64, samples []Sample) error { +// ReplaceActivitySamples deletes any existing samples for activityID (owned +// by userID) and bulk-inserts the given set, so re-syncing an activity's +// details is idempotent. +func (db *DB) ReplaceActivitySamples(ctx context.Context, userID, activityID int64, samples []Sample) error { + var exists int + err := db.QueryRowContext(ctx, `SELECT 1 FROM activities WHERE id = ? AND user_id = ?`, activityID, userID).Scan(&exists) + if err == sql.ErrNoRows { + return fmt.Errorf("replace activity samples: activity %d not found for user %d", activityID, userID) + } + if err != nil { + return fmt.Errorf("replace activity samples for activity %d (user %d): %w", activityID, userID, err) + } + tx, err := db.BeginTx(ctx, nil) if err != nil { return fmt.Errorf("begin replace samples tx: %w", err) @@ -44,13 +55,18 @@ func (db *DB) ReplaceActivitySamples(ctx context.Context, activityID int64, samp return tx.Commit() } -// SamplesForActivity returns all samples for an activity, ordered by elapsed_seconds. -func (db *DB) SamplesForActivity(ctx context.Context, activityID int64) ([]Sample, error) { +// SamplesForActivity returns all samples for an activity owned by userID, +// ordered by elapsed_seconds. +func (db *DB) SamplesForActivity(ctx context.Context, userID, activityID int64) ([]Sample, error) { rows, err := db.QueryContext(ctx, ` - SELECT elapsed_seconds, timestamp_ms, heart_rate, speed_mps, distance_m, elevation_m - FROM activity_samples WHERE activity_id = ? ORDER BY elapsed_seconds`, activityID) + SELECT activity_samples.elapsed_seconds, activity_samples.timestamp_ms, activity_samples.heart_rate, + activity_samples.speed_mps, activity_samples.distance_m, activity_samples.elevation_m + FROM activity_samples + JOIN activities ON activities.id = activity_samples.activity_id + WHERE activity_samples.activity_id = ? AND activities.user_id = ? + ORDER BY activity_samples.elapsed_seconds`, activityID, userID) if err != nil { - return nil, fmt.Errorf("samples for activity %d: %w", activityID, err) + return nil, fmt.Errorf("samples for activity %d (user %d): %w", activityID, userID, err) } defer rows.Close() diff --git a/backend/internal/store/store_test.go b/backend/internal/store/store_test.go index 7a236a2..2526979 100644 --- a/backend/internal/store/store_test.go +++ b/backend/internal/store/store_test.go @@ -143,7 +143,7 @@ func TestKindAssignment_AppendOnlyHistoryAndCurrentView(t *testing.T) { } // First: rule engine says needs_review (ambiguous). - if _, err := db.InsertKindAssignment(ctx, KindAssignment{ + if _, err := db.InsertKindAssignment(ctx, userID, KindAssignment{ ActivityID: activityID, AssignmentSource: AssignmentSourceRuleEngine, Status: AssignmentStatusNeedsReview, @@ -152,7 +152,7 @@ func TestKindAssignment_AppendOnlyHistoryAndCurrentView(t *testing.T) { t.Fatalf("InsertKindAssignment (rule engine): %v", err) } - queue, err := db.ReviewQueue(ctx) + queue, err := db.ReviewQueue(ctx, userID) if err != nil { t.Fatalf("ReviewQueue: %v", err) } @@ -161,7 +161,7 @@ func TestKindAssignment_AppendOnlyHistoryAndCurrentView(t *testing.T) { } // Then: user manually resolves it. - if _, err := db.InsertKindAssignment(ctx, KindAssignment{ + if _, err := db.InsertKindAssignment(ctx, userID, KindAssignment{ ActivityID: activityID, WorkoutKindID: &kindID, AssignmentSource: AssignmentSourceManual, @@ -170,7 +170,7 @@ func TestKindAssignment_AppendOnlyHistoryAndCurrentView(t *testing.T) { t.Fatalf("InsertKindAssignment (manual): %v", err) } - queue, err = db.ReviewQueue(ctx) + queue, err = db.ReviewQueue(ctx, userID) if err != nil { t.Fatalf("ReviewQueue after resolve: %v", err) } @@ -178,7 +178,7 @@ func TestKindAssignment_AppendOnlyHistoryAndCurrentView(t *testing.T) { t.Fatalf("expected 0 items in review queue after manual resolve, got %d", len(queue)) } - current, ok, err := db.CurrentAssignment(ctx, activityID) + current, ok, err := db.CurrentAssignment(ctx, userID, activityID) if err != nil || !ok { t.Fatalf("CurrentAssignment: ok=%v err=%v", ok, err) } @@ -195,7 +195,7 @@ func TestKindAssignment_AppendOnlyHistoryAndCurrentView(t *testing.T) { t.Errorf("expected 2 historical assignment rows (rule engine + manual), got %d", historyCount) } - forKind, err := db.AssignmentsForKind(ctx, kindID) + forKind, err := db.AssignmentsForKind(ctx, userID, kindID) if err != nil { t.Fatalf("AssignmentsForKind: %v", err) } @@ -225,15 +225,15 @@ func TestReplaceLapsIsIdempotent(t *testing.T) { {LapIndex: 1, RawJSON: "{}"}, {LapIndex: 2, RawJSON: "{}"}, } - if err := db.ReplaceLaps(ctx, activityID, laps); err != nil { + if err := db.ReplaceLaps(ctx, userID, activityID, laps); err != nil { t.Fatalf("ReplaceLaps (first): %v", err) } // Re-sync with a different set (e.g. corrected data) should fully replace, not append. - if err := db.ReplaceLaps(ctx, activityID, laps[:1]); err != nil { + if err := db.ReplaceLaps(ctx, userID, activityID, laps[:1]); err != nil { t.Fatalf("ReplaceLaps (second): %v", err) } - got, err := db.LapsForActivity(ctx, activityID) + got, err := db.LapsForActivity(ctx, userID, activityID) if err != nil { t.Fatalf("LapsForActivity: %v", err) } @@ -309,7 +309,7 @@ func TestForeignKeyEnforcementPostMigration(t *testing.T) { } // Inserting a kind_assignment with a valid FK reference should succeed. - assignmentID, err := db.InsertKindAssignment(ctx, KindAssignment{ + assignmentID, err := db.InsertKindAssignment(ctx, userID, KindAssignment{ ActivityID: activityID, WorkoutKindID: &seedKindID, AssignmentSource: AssignmentSourceManual, @@ -325,7 +325,7 @@ func TestForeignKeyEnforcementPostMigration(t *testing.T) { // Inserting a kind_assignment with a nonexistent workout_kind_id should // be rejected by the FK constraint, proving FK enforcement is ON. nonexistentKindID := int64(999999) - _, err = db.InsertKindAssignment(ctx, KindAssignment{ + _, err = db.InsertKindAssignment(ctx, userID, KindAssignment{ ActivityID: activityID, WorkoutKindID: &nonexistentKindID, AssignmentSource: AssignmentSourceManual, @@ -568,6 +568,38 @@ func TestRebuildMigrationsPreserveForeignKeyReferences(t *testing.T) { } } +func TestCurrentAssignment_ScopedToOwningUser(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + userA, err := db.ProvisionUser(ctx, "sub-a", "A") + if err != nil { + t.Fatalf("ProvisionUser(a): %v", err) + } + userB, err := db.ProvisionUser(ctx, "sub-b", "B") + if err != nil { + t.Fatalf("ProvisionUser(b): %v", err) + } + + activityID, err := db.UpsertActivity(ctx, userA, Activity{GarminActivityID: 1, StartTimeUTC: "2026-07-11 05:00:00", RawJSON: "{}"}) + if err != nil { + t.Fatalf("UpsertActivity: %v", err) + } + if _, err := db.InsertKindAssignment(ctx, userA, KindAssignment{ + ActivityID: activityID, AssignmentSource: AssignmentSourceRuleEngine, Status: AssignmentStatusNeedsReview, CandidateKindsJSON: "[]", + }); err != nil { + t.Fatalf("InsertKindAssignment: %v", err) + } + + if _, found, err := db.CurrentAssignment(ctx, userB, activityID); err != nil || found { + t.Fatalf("expected userB to not see userA's activity assignment, found=%v err=%v", found, err) + } + if _, err := db.InsertKindAssignment(ctx, userB, KindAssignment{ + ActivityID: activityID, AssignmentSource: AssignmentSourceManual, Status: AssignmentStatusAssigned, CandidateKindsJSON: "[]", + }); err == nil { + t.Fatal("expected InsertKindAssignment to reject an activity that doesn't belong to userB") + } +} + func contains(s string, substrs ...string) bool { lower := strings.ToLower(s) for _, substr := range substrs {