fix: key melody binary storage on pid instead of uid, add migration script
melody.uid is never actually populated anywhere in MelodyForm.jsx, so keying local .bsm storage on it (as the previous commit did) would silently break for every existing melody. pid is the correct key anyway: it identifies the underlying archetype binary, and multiple melodies legitimately share one pid (each remaps the same note sequence to different bells/speed/duration via its own settings). Deletion is now share-aware — a melody's binary is only removed from disk once no other melody still references its pid. Also adds backend/scripts/migrate_melody_binaries_to_local.py to backfill existing melodies from their old Firebase URLs to local storage, with --dry-run support and a warning list for pids whose melodies point at different source files (a pre-existing data issue, flagged for manual review via each melody's playback button rather than silently resolved). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -12,28 +12,74 @@ This does **not** affect the audio preview file (`information.previewURL`) — t
|
||||
only ever fetched by the browser admin UI, which is already HTTPS, so it stays on
|
||||
Firebase Storage unchanged.
|
||||
|
||||
## PID vs UID — why storage is keyed on pid, not uid
|
||||
|
||||
- **`pid`** ("Playback ID") identifies the underlying **archetype binary** — the raw
|
||||
note sequence (A, B, C...) baked into a `.bsm` file. Multiple melodies can
|
||||
legitimately share one `pid`: each melody remaps those notes to actual bells via its
|
||||
own `noteAssignments`, speed, and duration settings. One binary can back many
|
||||
different "remix" melodies. **Duplicate PIDs across melodies are correct and
|
||||
expected, not a bug.**
|
||||
- **`uid`** is meant to be each melody's own unique database identity. It is *not*
|
||||
currently populated anywhere in `MelodyForm.jsx` — this is a pre-existing, separate
|
||||
bug, deferred as its own task. Because of this, local binary storage is keyed on
|
||||
`pid`, not `uid`.
|
||||
|
||||
## What changed
|
||||
|
||||
- **Storage**: `.bsm` binaries are written to `./data/melody_binaries/{melody_uid}.bsm`
|
||||
on the host (mounted into the backend container at `/app/storage/melody_binaries`,
|
||||
same pattern as `./data/firmware` and `./data/built_melodies`).
|
||||
- **Storage**: `.bsm` binaries are written to `./data/melody_binaries/{pid}.bsm` on
|
||||
the host (mounted into the backend container at `/app/storage/melody_binaries`,
|
||||
same pattern as `./data/firmware` and `./data/built_melodies`). Because storage is
|
||||
keyed on `pid`, multiple melodies sharing a `pid` share one file — saving from any
|
||||
of them overwrites the same file, which is expected.
|
||||
- **Upload path**: `SelectArchetypeModal` / `BuildOnTheFlyModal` still call
|
||||
`POST /api/melodies/{melodyId}/upload/binary` exactly as before. The backend
|
||||
(`backend/melodies/service.py::save_binary_for_melody`) now writes the bytes to
|
||||
local disk instead of uploading to Firebase Storage, and returns a URL like
|
||||
`http://melodies.bellsystems.net/download/{pid}`.
|
||||
`http://melodies.bellsystems.net/download/{pid}`. The melody must have a `pid` set
|
||||
before uploading a binary (the endpoint returns 400 if not).
|
||||
- **Stored URL**: the melody's `url` field (Firestore + SQLite) now holds that
|
||||
plain-HTTP URL instead of a Firebase public URL. No schema change — `url` was
|
||||
already a plain string field.
|
||||
- **Download route**: `GET /api/melodies/download/{pid}` (`backend/melodies/router.py`)
|
||||
is unauthenticated (devices have no login token) and resolves `pid` → the melody's
|
||||
`{uid}.bsm` file on disk, same pattern as the existing firmware download route
|
||||
is unauthenticated (devices have no login token) and resolves `pid` directly to
|
||||
`{pid}.bsm` on disk, same pattern as the existing firmware download route
|
||||
(`backend/firmware/router.py::download_firmware`).
|
||||
- **PID uniqueness**: since `pid` is now the public lookup key for the download route,
|
||||
`backend/melodies/service.py::_check_pid_unique` rejects creating/renaming a melody
|
||||
to a PID already used by another melody (409 Conflict).
|
||||
- **Cleanup**: deleting a melody or its binary file now also deletes the local
|
||||
`.bsm` file (`service.delete_local_binary`).
|
||||
- **Deletion is share-aware**: deleting a melody or its binary
|
||||
(`service.delete_file` / `service._delete_storage_files`) only removes the local
|
||||
`.bsm` file if no *other* melody still references the same `pid`
|
||||
(`service._pid_used_by_other_melody`). This prevents one melody's deletion from
|
||||
breaking playback for other melodies sharing its archetype binary.
|
||||
|
||||
## Migrating existing melodies
|
||||
|
||||
Melodies created before this change still have a Firebase Storage URL in their `url`
|
||||
field — deploying this code does not touch existing data. They keep working exactly
|
||||
as before (still HTTPS to Firebase) until migrated.
|
||||
|
||||
Two ways to move a melody to the new plain-HTTP URL:
|
||||
|
||||
1. **Per-melody, via the UI**: open the melody and re-run "Select Archetype" or
|
||||
"Build on the Fly" — this naturally re-uploads through the same endpoint, which now
|
||||
writes to local disk and updates `url`.
|
||||
2. **Bulk, via script**: `backend/scripts/migrate_melody_binaries_to_local.py`
|
||||
downloads each melody's current Firebase binary and writes it to local disk under
|
||||
its `pid`, then updates `url` (SQLite + Firestore if published). Run with
|
||||
`--dry-run` first:
|
||||
|
||||
```
|
||||
docker exec -it bellsystems-backend python scripts/migrate_melody_binaries_to_local.py --dry-run
|
||||
docker exec -it bellsystems-backend python scripts/migrate_melody_binaries_to_local.py
|
||||
```
|
||||
|
||||
**Known caveat**: some existing melodies share a `pid` despite their Firebase URLs
|
||||
pointing at *different* source files (observed on real data — e.g. voice-count
|
||||
variant filenames like `1N_`/`2N_`/`3N_`/`4N_` all filed under one `pid`). The
|
||||
script downloads only the first melody's file for each `pid` and reuses it for
|
||||
every other melody sharing that `pid` — it does not attempt to detect which file is
|
||||
"correct". It prints a warning list of every `pid` where this happened; use each
|
||||
melody's playback button afterward to verify the right archetype plays, and
|
||||
manually re-assign the correct one via "Select Archetype" if it's wrong.
|
||||
|
||||
## Infrastructure (outside this repo)
|
||||
|
||||
@@ -57,8 +103,8 @@ the NPM proxy host exists.
|
||||
|
||||
## Verification
|
||||
|
||||
1. Build/select an archetype for a melody → confirm a file appears at
|
||||
`./data/melody_binaries/{uid}.bsm` and the melody's `url` becomes
|
||||
1. Build/select an archetype for a melody with a `pid` set → confirm a file appears
|
||||
at `./data/melody_binaries/{pid}.bsm` and the melody's `url` becomes
|
||||
`http://melodies.bellsystems.net/download/{pid}`.
|
||||
2. `curl http://localhost:8000/api/melodies/download/{pid}` (direct to backend,
|
||||
bypassing nginx) → returns the `.bsm` bytes, no auth header needed.
|
||||
@@ -66,6 +112,8 @@ the NPM proxy host exists.
|
||||
console server block) → same result.
|
||||
4. Once NPM is configured, `curl http://melodies.bellsystems.net/download/{pid}`
|
||||
→ same result, over plain HTTP.
|
||||
5. Delete the melody → confirm the local `.bsm` file is removed.
|
||||
5. Delete a melody whose `pid` is *not* shared with any other melody → confirm the
|
||||
local `.bsm` file is removed. Delete one of two melodies sharing a `pid` → confirm
|
||||
the file survives until the last one is deleted.
|
||||
6. Upload a preview audio file → confirm it still lands in Firebase Storage and
|
||||
`previewURL` still populates (regression check).
|
||||
|
||||
Reference in New Issue
Block a user