fix(server,flutter): sync wire format + systemVariant column (#357)
Two fixes in one commit because they're entangled — the systemVariant work would have been theater otherwise. ## The wire-format bug /api/library/sync was emitting PascalCase JSON for artist / album / track / playlist upserts (raw json.Marshal of sqlc-generated structs with no JSON tags — sqlc.yaml: emit_json_tags=false). Flutter's sync_controller _*FromJson reads snake_case keys, so all metadata sync rows landed in drift with empty strings / zero ints. The like_track / like_album / like_artist / playlist_track entities work because they're hand-built `map[string]string` payloads with snake_case keys — they sidestepped the bug. The 4 raw-marshal entities did not. Existing sync test caught zero of this — it asserts on len(upserts) not field shape. Fix: server-side view structs in library_sync_views.go with proper JSON tags + pgtype-flattening (UUID → 8-4-4-4-12 hex string, Date → "2006-01-02"). Mirrors the playlistRowView pattern from /api/playlists. New library_sync_views_test.go pins the wire keys so future field-name drift breaks loud. ## systemVariant column (closes #357 plan C v1 limitation) playlistSyncView now carries `system_variant` server → wire. Flutter drift schema bumped from 1 → 2 with onUpgrade adding the `systemVariant TEXT NULL` column to cached_playlists. Cursor reset to 0 in the migration so existing rows refresh with the new field on the next sync. playlistsListProvider now filters locally by systemVariant: - kind='user' → systemVariant IS NULL (the add-to-playlist sheet's intent) - kind='system' → systemVariant IS NOT NULL - kind='all' → no filter Closes the documented v1 limitation where the add-to-playlist sheet showed system playlists alongside user-created ones. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+2
-1
@@ -88,7 +88,7 @@ extension CachedPlaylistAdapter on CachedPlaylist {
|
||||
name: name,
|
||||
description: description,
|
||||
isPublic: isPublic,
|
||||
systemVariant: null, // not tracked in cache
|
||||
systemVariant: systemVariant,
|
||||
trackCount: trackCount,
|
||||
coverUrl: '', // server-derived; cache doesn't persist
|
||||
ownerUsername: ownerUsername,
|
||||
@@ -105,5 +105,6 @@ extension PlaylistDriftWrite on Playlist {
|
||||
description: drift.Value(description),
|
||||
isPublic: drift.Value(isPublic),
|
||||
trackCount: drift.Value(trackCount),
|
||||
systemVariant: drift.Value(systemVariant),
|
||||
);
|
||||
}
|
||||
|
||||
Vendored
+23
-1
@@ -76,6 +76,11 @@ class CachedPlaylists extends Table {
|
||||
TextColumn get coverPath => text().nullable()();
|
||||
IntColumn get trackCount => integer().withDefault(const Constant(0))();
|
||||
IntColumn get durationSec => integer().withDefault(const Constant(0))();
|
||||
/// Server's system_variant: null for user playlists, "for_you" /
|
||||
/// "songs_like_artist" / "discover" for system-generated mixes.
|
||||
/// Added in schemaVersion 2 to let the add-to-playlist sheet filter
|
||||
/// out system playlists locally without a REST round-trip.
|
||||
TextColumn get systemVariant => text().nullable()();
|
||||
DateTimeColumn get fetchedAt => dateTime().withDefault(currentDateAndTime)();
|
||||
@override
|
||||
Set<Column> get primaryKey => {id};
|
||||
@@ -126,5 +131,22 @@ class AppDb extends _$AppDb {
|
||||
AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache'));
|
||||
|
||||
@override
|
||||
int get schemaVersion => 1;
|
||||
int get schemaVersion => 2;
|
||||
|
||||
@override
|
||||
MigrationStrategy get migration => MigrationStrategy(
|
||||
onCreate: (m) => m.createAll(),
|
||||
onUpgrade: (m, from, to) async {
|
||||
if (from < 2) {
|
||||
// Schema 2: add CachedPlaylists.systemVariant. Existing
|
||||
// rows get null and the next sync rebuilds them with the
|
||||
// server's system_variant value.
|
||||
await m.addColumn(cachedPlaylists, cachedPlaylists.systemVariant);
|
||||
// Reset cursor so the next /api/library/sync re-emits all
|
||||
// playlists; otherwise the new column would stay null on
|
||||
// pre-existing rows until they happen to change server-side.
|
||||
await customStatement('UPDATE sync_metadata SET cursor = 0');
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -282,6 +282,7 @@ class SyncController extends AsyncNotifier<SyncResult?> {
|
||||
coverPath: drift.Value(j['cover_path'] as String?),
|
||||
trackCount: drift.Value((j['track_count'] as num?)?.toInt() ?? 0),
|
||||
durationSec: drift.Value((j['duration_sec'] as num?)?.toInt() ?? 0),
|
||||
systemVariant: drift.Value(j['system_variant'] as String?),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -17,12 +17,15 @@ final playlistsApiProvider = FutureProvider<PlaylistsApi>((ref) async {
|
||||
return PlaylistsApi(await ref.watch(dioProvider.future));
|
||||
});
|
||||
|
||||
/// Drift-first per #357 plan C. Returns all cached playlists for the
|
||||
/// current user (owned) + public playlists from others. The `kind`
|
||||
/// family arg only affects the REST cold-cache fetch shape — drift
|
||||
/// can't distinguish 'user' from 'system' kind because systemVariant
|
||||
/// isn't persisted (TODO: add column + schema bump in a follow-up).
|
||||
/// For v1 this means add-to-playlist sheet shows system playlists too.
|
||||
/// Drift-first per #357 plan C. Returns cached playlists filtered to
|
||||
/// match the `kind` family arg:
|
||||
/// - 'user' → only user-created (systemVariant null), owned by current user
|
||||
/// - 'system' → only system-generated (systemVariant non-null), owned
|
||||
/// - 'all' → everything: own user playlists + own system + others' public
|
||||
///
|
||||
/// systemVariant column added in drift schema v2 (#357 follow-up); previous
|
||||
/// behavior leaked system playlists into add-to-playlist sheet because we
|
||||
/// couldn't filter locally.
|
||||
final playlistsListProvider =
|
||||
StreamProvider.family<PlaylistsList, String>((ref, kind) {
|
||||
final db = ref.watch(appDbProvider);
|
||||
@@ -42,11 +45,16 @@ final playlistsListProvider =
|
||||
},
|
||||
toResult: (rows) {
|
||||
if (user == null) return PlaylistsList.empty();
|
||||
final owned = rows
|
||||
final filtered = rows.where((r) {
|
||||
if (kind == 'user') return r.systemVariant == null;
|
||||
if (kind == 'system') return r.systemVariant != null;
|
||||
return true; // 'all'
|
||||
});
|
||||
final owned = filtered
|
||||
.where((r) => r.userId == user.id)
|
||||
.map((r) => r.toRef())
|
||||
.toList();
|
||||
final pub = rows
|
||||
final pub = filtered
|
||||
.where((r) => r.userId != user.id && r.isPublic)
|
||||
.map((r) => r.toRef())
|
||||
.toList();
|
||||
|
||||
@@ -141,7 +141,7 @@ func (h *handlers) hydrateUpserts(
|
||||
return nil, err
|
||||
}
|
||||
for _, a := range artists {
|
||||
b, _ := json.Marshal(a)
|
||||
b, _ := json.Marshal(toArtistSyncView(a))
|
||||
out["artist"] = append(out["artist"], b)
|
||||
}
|
||||
}
|
||||
@@ -152,7 +152,7 @@ func (h *handlers) hydrateUpserts(
|
||||
return nil, err
|
||||
}
|
||||
for _, a := range albums {
|
||||
b, _ := json.Marshal(a)
|
||||
b, _ := json.Marshal(toAlbumSyncView(a))
|
||||
out["album"] = append(out["album"], b)
|
||||
}
|
||||
}
|
||||
@@ -163,7 +163,7 @@ func (h *handlers) hydrateUpserts(
|
||||
return nil, err
|
||||
}
|
||||
for _, t := range tracks {
|
||||
b, _ := json.Marshal(t)
|
||||
b, _ := json.Marshal(toTrackSyncView(t))
|
||||
out["track"] = append(out["track"], b)
|
||||
}
|
||||
}
|
||||
@@ -191,7 +191,7 @@ func (h *handlers) hydrateUpserts(
|
||||
if syncpkg.FormatUUID(p.UserID) != userIDStr && !p.IsPublic {
|
||||
continue
|
||||
}
|
||||
b, _ := json.Marshal(p)
|
||||
b, _ := json.Marshal(toPlaylistSyncView(p))
|
||||
out["playlist"] = append(out["playlist"], b)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
package api
|
||||
|
||||
// Wire shapes for /api/library/sync upserts. The sqlc-generated row
|
||||
// structs (dbq.Artist, dbq.Album, dbq.Track, dbq.Playlist) have no
|
||||
// JSON tags (sqlc.yaml: emit_json_tags=false), so a raw json.Marshal
|
||||
// of them produces PascalCase field names that the Flutter sync
|
||||
// controller (which reads snake_case keys) can't parse.
|
||||
//
|
||||
// These view structs add the JSON tag layer + flatten pgtype.UUID and
|
||||
// pgtype.Timestamptz / Date into strings, mirroring the pattern
|
||||
// playlistRowView already established for /api/playlists.
|
||||
//
|
||||
// Field names match flutter_client/lib/cache/sync_controller.dart's
|
||||
// _*FromJson helpers exactly. Adding a field server-side requires a
|
||||
// matching read in the Flutter helper or it'll be silently dropped.
|
||||
|
||||
import (
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
|
||||
)
|
||||
|
||||
type artistSyncView struct {
|
||||
ID string `json:"id"`
|
||||
Name string `json:"name"`
|
||||
SortName string `json:"sort_name"`
|
||||
Mbid *string `json:"mbid"`
|
||||
ArtistThumbPath *string `json:"artist_thumb_path"`
|
||||
ArtistFanartPath *string `json:"artist_fanart_path"`
|
||||
}
|
||||
|
||||
func toArtistSyncView(a dbq.Artist) artistSyncView {
|
||||
return artistSyncView{
|
||||
ID: syncpkg.FormatUUID(a.ID),
|
||||
Name: a.Name,
|
||||
SortName: a.SortName,
|
||||
Mbid: a.Mbid,
|
||||
ArtistThumbPath: a.ArtistThumbPath,
|
||||
ArtistFanartPath: a.ArtistFanartPath,
|
||||
}
|
||||
}
|
||||
|
||||
type albumSyncView struct {
|
||||
ID string `json:"id"`
|
||||
ArtistID string `json:"artist_id"`
|
||||
Title string `json:"title"`
|
||||
SortTitle string `json:"sort_title"`
|
||||
ReleaseDate *string `json:"release_date"`
|
||||
CoverArtPath *string `json:"cover_art_path"`
|
||||
Mbid *string `json:"mbid"`
|
||||
}
|
||||
|
||||
func toAlbumSyncView(a dbq.Album) albumSyncView {
|
||||
var releaseDate *string
|
||||
if a.ReleaseDate.Valid {
|
||||
s := a.ReleaseDate.Time.Format("2006-01-02")
|
||||
releaseDate = &s
|
||||
}
|
||||
return albumSyncView{
|
||||
ID: syncpkg.FormatUUID(a.ID),
|
||||
ArtistID: syncpkg.FormatUUID(a.ArtistID),
|
||||
Title: a.Title,
|
||||
SortTitle: a.SortTitle,
|
||||
ReleaseDate: releaseDate,
|
||||
CoverArtPath: a.CoverArtPath,
|
||||
Mbid: a.Mbid,
|
||||
}
|
||||
}
|
||||
|
||||
type trackSyncView struct {
|
||||
ID string `json:"id"`
|
||||
AlbumID string `json:"album_id"`
|
||||
ArtistID string `json:"artist_id"`
|
||||
Title string `json:"title"`
|
||||
DurationMs int32 `json:"duration_ms"`
|
||||
TrackNumber *int32 `json:"track_number"`
|
||||
DiscNumber *int32 `json:"disc_number"`
|
||||
FilePath string `json:"file_path"`
|
||||
FileFormat string `json:"file_format"`
|
||||
Genre *string `json:"genre"`
|
||||
}
|
||||
|
||||
func toTrackSyncView(t dbq.Track) trackSyncView {
|
||||
return trackSyncView{
|
||||
ID: syncpkg.FormatUUID(t.ID),
|
||||
AlbumID: syncpkg.FormatUUID(t.AlbumID),
|
||||
ArtistID: syncpkg.FormatUUID(t.ArtistID),
|
||||
Title: t.Title,
|
||||
DurationMs: t.DurationMs,
|
||||
TrackNumber: t.TrackNumber,
|
||||
DiscNumber: t.DiscNumber,
|
||||
FilePath: t.FilePath,
|
||||
FileFormat: t.FileFormat,
|
||||
Genre: t.Genre,
|
||||
}
|
||||
}
|
||||
|
||||
type playlistSyncView struct {
|
||||
ID string `json:"id"`
|
||||
UserID string `json:"user_id"`
|
||||
Name string `json:"name"`
|
||||
Description string `json:"description"`
|
||||
IsPublic bool `json:"is_public"`
|
||||
CoverPath *string `json:"cover_path"`
|
||||
TrackCount int32 `json:"track_count"`
|
||||
DurationSec int32 `json:"duration_sec"`
|
||||
SystemVariant *string `json:"system_variant"`
|
||||
}
|
||||
|
||||
func toPlaylistSyncView(p dbq.Playlist) playlistSyncView {
|
||||
return playlistSyncView{
|
||||
ID: syncpkg.FormatUUID(p.ID),
|
||||
UserID: syncpkg.FormatUUID(p.UserID),
|
||||
Name: p.Name,
|
||||
Description: p.Description,
|
||||
IsPublic: p.IsPublic,
|
||||
CoverPath: p.CoverPath,
|
||||
TrackCount: p.TrackCount,
|
||||
DurationSec: p.DurationSec,
|
||||
SystemVariant: p.SystemVariant,
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,101 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
)
|
||||
|
||||
// These tests pin the wire-format keys for /api/library/sync upserts.
|
||||
// Without them, sqlc model field-name drift or accidental
|
||||
// `json.Marshal(rawStruct)` regressions would silently break the
|
||||
// Flutter client (which reads snake_case keys via _*FromJson helpers
|
||||
// in flutter_client/lib/cache/sync_controller.dart).
|
||||
|
||||
// validUUID is a deterministic test UUID — not a real value, just
|
||||
// something that pgtype.UUID.Valid will accept.
|
||||
var validUUID = pgtype.UUID{
|
||||
Bytes: [16]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
|
||||
0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10},
|
||||
Valid: true,
|
||||
}
|
||||
|
||||
func assertJSONKeys(t *testing.T, label string, b []byte, want []string) {
|
||||
t.Helper()
|
||||
var m map[string]json.RawMessage
|
||||
if err := json.Unmarshal(b, &m); err != nil {
|
||||
t.Fatalf("%s: invalid json: %v\nbody: %s", label, err, b)
|
||||
}
|
||||
for _, k := range want {
|
||||
if _, ok := m[k]; !ok {
|
||||
t.Errorf("%s: missing key %q in %s", label, k, b)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestArtistSyncView_WireKeys(t *testing.T) {
|
||||
a := dbq.Artist{ID: validUUID, Name: "Aphex Twin", SortName: "Aphex Twin"}
|
||||
b, err := json.Marshal(toArtistSyncView(a))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
assertJSONKeys(t, "artist", b, []string{
|
||||
"id", "name", "sort_name", "mbid",
|
||||
"artist_thumb_path", "artist_fanart_path",
|
||||
})
|
||||
}
|
||||
|
||||
func TestAlbumSyncView_WireKeys(t *testing.T) {
|
||||
a := dbq.Album{ID: validUUID, ArtistID: validUUID, Title: "Drukqs", SortTitle: "Drukqs"}
|
||||
b, err := json.Marshal(toAlbumSyncView(a))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
assertJSONKeys(t, "album", b, []string{
|
||||
"id", "artist_id", "title", "sort_title",
|
||||
"release_date", "cover_art_path", "mbid",
|
||||
})
|
||||
}
|
||||
|
||||
func TestTrackSyncView_WireKeys(t *testing.T) {
|
||||
tr := dbq.Track{
|
||||
ID: validUUID, AlbumID: validUUID, ArtistID: validUUID,
|
||||
Title: "Avril 14th", DurationMs: 121_000,
|
||||
FilePath: "x", FileFormat: "flac",
|
||||
}
|
||||
b, err := json.Marshal(toTrackSyncView(tr))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
assertJSONKeys(t, "track", b, []string{
|
||||
"id", "album_id", "artist_id", "title", "duration_ms",
|
||||
"track_number", "disc_number", "file_path", "file_format", "genre",
|
||||
})
|
||||
}
|
||||
|
||||
func TestPlaylistSyncView_WireKeys(t *testing.T) {
|
||||
variant := "discover"
|
||||
p := dbq.Playlist{
|
||||
ID: validUUID, UserID: validUUID, Name: "Test",
|
||||
IsPublic: true, TrackCount: 5, DurationSec: 600,
|
||||
SystemVariant: &variant,
|
||||
}
|
||||
b, err := json.Marshal(toPlaylistSyncView(p))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
assertJSONKeys(t, "playlist", b, []string{
|
||||
"id", "user_id", "name", "description",
|
||||
"is_public", "cover_path", "track_count",
|
||||
"duration_sec", "system_variant",
|
||||
})
|
||||
// Sanity: system_variant carries the value, not just present.
|
||||
var m map[string]any
|
||||
_ = json.Unmarshal(b, &m)
|
||||
if got := m["system_variant"]; got != "discover" {
|
||||
t.Errorf("system_variant: want 'discover', got %v", got)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user