Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions backend/app/services/workouts.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,27 @@ def __init__(self, existing_id: int, reason: str) -> None:
self.reason = reason


def find_duplicate(db: Session, user_id: int, parsed, file_hash: str) -> tuple[int, str] | None:
"""Find an existing workout that is the same session as `parsed`.
def find_identical(db: Session, user_id: int, file_hash: str) -> tuple[int, str] | None:
"""Find an existing workout stored from exactly these bytes.

Two questions, because they need different answers. Byte-identical files
are caught by their hash — exact, cheap, and enforced by the database.
The same ride exported from Garmin as TCX and from Strava as GPX has
different bytes and a different `source`, so it can only be recognised by
what it describes: same sport, starting at practically the same moment.
Exact, cheap, and enforced by the database — and answerable before the file
is parsed, which is what makes re-sending an export fast: parsing is most of
the cost of a file, and an export sent twice is almost entirely files
already here.
"""
exact = db.execute(
select(Workout.id).where(Workout.user_id == user_id, Workout.file_hash == file_hash)
).scalar_one_or_none()
if exact is not None:
return exact, "identical file"
return None if exact is None else (exact, "identical file")


def find_same_session(db: Session, user_id: int, parsed) -> tuple[int, str] | None:
"""Find an existing workout that is the same session as `parsed`.

The same ride exported from Garmin as TCX and from Strava as GPX has
different bytes and a different `source`, so it can only be recognised by
what it describes: same sport, starting at practically the same moment.
"""
window = timedelta(seconds=settings.duplicate_window_seconds)
near = db.execute(
select(Workout.id)
Expand All @@ -79,12 +85,13 @@ def store_workout(db: Session, user_id: int, filename: str | None, content: byte
if db.execute(select(User.id).where(User.id == user_id)).scalar_one_or_none() is None:
raise UserNotFoundError(user_id)

parsed = parse_file(filename, content)

file_hash = hashlib.sha256(content).hexdigest()
duplicate = find_duplicate(db, user_id, parsed, file_hash)
if duplicate is not None:
raise DuplicateWorkoutError(*duplicate)
if (identical := find_identical(db, user_id, file_hash)) is not None:
raise DuplicateWorkoutError(*identical)

parsed = parse_file(filename, content)
if (same_session := find_same_session(db, user_id, parsed)) is not None:
raise DuplicateWorkoutError(*same_session)

metrics = compute_metrics(parsed)
workout = Workout(
Expand Down
18 changes: 18 additions & 0 deletions backend/tests/test_duplicates.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ def test_the_same_bytes_are_rejected_under_a_different_name(client, user, sample
assert "identical file" in second.json()["detail"]


def test_identical_bytes_are_recognised_without_parsing(client, user, sample_gpx, monkeypatch):
"""Parsing is most of the cost of a file, and a re-sent export is all repeats.

Measured on a 150-activity archive, the second send took 130 s with nothing
to store — long enough to time out at the proxy on its own.
"""
assert upload(client, user["id"], sample_gpx, "run.gpx").status_code == 201

def parse_must_not_run(*_args, **_kwargs):
raise AssertionError("an identical file was parsed")

monkeypatch.setattr("app.services.workouts.parse_file", parse_must_not_run)
second = upload(client, user["id"], sample_gpx, "run-again.gpx")

assert second.status_code == 409
assert "identical file" in second.json()["detail"]


def test_the_same_file_is_allowed_for_a_different_user(client, user, other_user, sample_gpx):
"""The hash is scoped per user: two people may ride together."""
assert upload(client, user["id"], sample_gpx, "run.gpx").status_code == 201
Expand Down
5 changes: 5 additions & 0 deletions docs/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ within `DUPLICATE_WINDOW_SECONDS` — which no constraint can express, so the
check lives in the service layer. A brick session still works: a ride and a run
at the same time are different sports.

**The exact question is asked before the file is parsed.** Parsing is most of
what a file costs, and an export sent a second time is almost entirely files
already stored — so hashing first took re-sending a 150-activity export from
130 s to under a second.

**A backfill that can tell "not computed" from "computed, nothing there" only
visits an activity once.** The heart-rate histogram records an empty value for an
activity that carried no heart rate, so a strapless one is not rescanned on every
Expand Down
Loading