refactor(profiles): remove dead registry surface
Delete unused registry members (ProfileConnectionRegistry.insertIfAbsent / removeAllForProfile, ProfileRegistry.reorder, ProfilesView.countFor, ConnectionRegistry.getDefault) and their orphan tests, refreshing the stale docs that named removeAllForProfile as the profile-delete cleanup path. Repair the "one default per profile" join-row invariant: the connectionId FK cascade (foreign_keys=ON) silently drops a profile's default row while its other rows survive, leaving it defaultless. Add promoteMissingDefaults, share a deterministic re-promotion helper with remove(), and re-promote in removeAllForConnection.
This commit is contained in:
@@ -1,7 +1,6 @@
|
|||||||
import 'dart:async';
|
import 'dart:async';
|
||||||
import 'dart:convert';
|
import 'dart:convert';
|
||||||
|
|
||||||
import 'package:collection/collection.dart';
|
|
||||||
import 'package:drift/drift.dart';
|
import 'package:drift/drift.dart';
|
||||||
|
|
||||||
import '../database/app_database.dart';
|
import '../database/app_database.dart';
|
||||||
@@ -41,17 +40,6 @@ class ConnectionRegistry {
|
|||||||
return _rowToConnection(row);
|
return _rowToConnection(row);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns the user's preferred connection — either the row marked
|
|
||||||
/// [Connections.isDefault], or the only row if exactly one exists, or
|
|
||||||
/// null when no connections are stored.
|
|
||||||
Future<Connection?> getDefault() async {
|
|
||||||
final rows = await _db.select(_db.connections).get();
|
|
||||||
if (rows.isEmpty) return null;
|
|
||||||
final flagged = rows.firstWhereOrNull((r) => r.isDefault);
|
|
||||||
final picked = flagged ?? (rows.length == 1 ? rows.single : null);
|
|
||||||
return picked == null ? null : _rowToConnection(picked);
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Insert or replace [connection]. If this is the first stored connection
|
/// Insert or replace [connection]. If this is the first stored connection
|
||||||
/// it is automatically marked default; re-upserting an existing row keeps
|
/// it is automatically marked default; re-upserting an existing row keeps
|
||||||
/// the row's current `isDefault` (so token/metadata refreshes don't clear
|
/// the row's current `isDefault` (so token/metadata refreshes don't clear
|
||||||
|
|||||||
@@ -193,9 +193,9 @@ class ProfileConnections extends Table {
|
|||||||
// No FK on profile_id: Plex Home profiles are virtual (built by
|
// No FK on profile_id: Plex Home profiles are virtual (built by
|
||||||
// Profile.virtualPlexHome from PlexHomeService's live cache, never
|
// Profile.virtualPlexHome from PlexHomeService's live cache, never
|
||||||
// persisted in `profiles`), so an FK here would reject every join row
|
// persisted in `profiles`), so an FK here would reject every join row
|
||||||
// they need. The two profile-delete sites clean up join rows manually
|
// they need. Profile deletion instead cleans up join rows explicitly
|
||||||
// via ProfileConnectionRegistry.removeAllForProfile before calling
|
// (removeAllProfileConnectionsAndCleanup in profile_connection_cleanup)
|
||||||
// ProfileRegistry.remove.
|
// before calling ProfileRegistry.remove.
|
||||||
TextColumn get profileId => text()();
|
TextColumn get profileId => text()();
|
||||||
TextColumn get connectionId => text().references(Connections, #id, onDelete: KeyAction.cascade)();
|
TextColumn get connectionId => text().references(Connections, #id, onDelete: KeyAction.cascade)();
|
||||||
TextColumn get userToken => text().withDefault(const Constant(''))();
|
TextColumn get userToken => text().withDefault(const Constant(''))();
|
||||||
|
|||||||
@@ -118,13 +118,6 @@ class ProfileConnectionRegistry {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Insert a new join row only if `(profileId, connectionId)` doesn't
|
|
||||||
/// already exist. Used by [ProfileSyncService] to surface new Plex Home
|
|
||||||
/// users without clobbering tokens cached by prior switches.
|
|
||||||
Future<void> insertIfAbsent(ProfileConnection pc) async {
|
|
||||||
await _db.into(_db.profileConnections).insert(await _companion(pc), mode: InsertMode.insertOrIgnore);
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Cache the freshly-acquired user token (e.g. after a `/home/users/switch`
|
/// Cache the freshly-acquired user token (e.g. after a `/home/users/switch`
|
||||||
/// call). Updates `tokenAcquiredAt` to now.
|
/// call). Updates `tokenAcquiredAt` to now.
|
||||||
Future<void> recordToken(String profileId, String connectionId, String token) async {
|
Future<void> recordToken(String profileId, String connectionId, String token) async {
|
||||||
@@ -149,12 +142,30 @@ class ProfileConnectionRegistry {
|
|||||||
await (_db.delete(
|
await (_db.delete(
|
||||||
_db.profileConnections,
|
_db.profileConnections,
|
||||||
)..where((t) => t.profileId.equals(profileId) & t.connectionId.equals(connectionId))).go();
|
)..where((t) => t.profileId.equals(profileId) & t.connectionId.equals(connectionId))).go();
|
||||||
// If we just removed the default, promote the oldest remaining row.
|
await _promoteDefaultIfMissing(profileId);
|
||||||
final remaining = await (_db.select(_db.profileConnections)..where((t) => t.profileId.equals(profileId))).get();
|
}
|
||||||
if (remaining.isNotEmpty && !remaining.any((r) => r.isDefault)) {
|
|
||||||
await (_db.update(_db.profileConnections)
|
/// Re-promote a default for [profileId] when it has join rows but none is
|
||||||
..where((t) => t.profileId.equals(profileId) & t.connectionId.equals(remaining.first.connectionId)))
|
/// flagged default — removing the default row would otherwise leave the
|
||||||
.write(const ProfileConnectionsCompanion(isDefault: Value(true)));
|
/// profile defaultless. Deterministic: picks the lowest connectionId,
|
||||||
|
/// matching [listForProfile]'s secondary ordering.
|
||||||
|
Future<void> _promoteDefaultIfMissing(String profileId) async {
|
||||||
|
final rows = await (_db.select(_db.profileConnections)..where((t) => t.profileId.equals(profileId))).get();
|
||||||
|
if (rows.isEmpty || rows.any((r) => r.isDefault)) return;
|
||||||
|
final pick = rows.map((r) => r.connectionId).reduce((a, b) => a.compareTo(b) <= 0 ? a : b);
|
||||||
|
await (_db.update(_db.profileConnections)
|
||||||
|
..where((t) => t.profileId.equals(profileId) & t.connectionId.equals(pick)))
|
||||||
|
.write(const ProfileConnectionsCompanion(isDefault: Value(true)));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Repair the "exactly one default per profile" invariant across every
|
||||||
|
/// profile. The `connectionId` FK cascade (PRAGMA foreign_keys=ON) deletes
|
||||||
|
/// join rows silently when a Connection is removed, so a profile can be left
|
||||||
|
/// with surviving rows but no default flag.
|
||||||
|
Future<void> promoteMissingDefaults() async {
|
||||||
|
final profileIds = (await _db.select(_db.profileConnections).get()).map((r) => r.profileId).toSet();
|
||||||
|
for (final profileId in profileIds) {
|
||||||
|
await _promoteDefaultIfMissing(profileId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -172,16 +183,14 @@ class ProfileConnectionRegistry {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Remove every join row referencing [connectionId] (e.g. when a Connection
|
/// Remove every join row referencing [connectionId] (e.g. when a Connection
|
||||||
/// is deleted). Drift's referential integrity isn't enabled by default for
|
/// is deleted). The `connectionId` FK (PRAGMA foreign_keys=ON) already
|
||||||
/// SQLite without `PRAGMA foreign_keys=ON`, so we cascade explicitly.
|
/// cascades these rows away when the Connection row itself is deleted; this
|
||||||
|
/// stays the explicit path for callers that drop the rows first, and either
|
||||||
|
/// way repairs any profile the removal left without a default.
|
||||||
Future<int> removeAllForConnection(String connectionId) async {
|
Future<int> removeAllForConnection(String connectionId) async {
|
||||||
return await (_db.delete(_db.profileConnections)..where((t) => t.connectionId.equals(connectionId))).go();
|
final removed = await (_db.delete(_db.profileConnections)..where((t) => t.connectionId.equals(connectionId))).go();
|
||||||
}
|
await promoteMissingDefaults();
|
||||||
|
return removed;
|
||||||
/// Wipe every join row for [profileId] (e.g. when a Plex Home profile's
|
|
||||||
/// parent connection is removed).
|
|
||||||
Future<int> removeAllForProfile(String profileId) async {
|
|
||||||
return await (_db.delete(_db.profileConnections)..where((t) => t.profileId.equals(profileId))).go();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Wipe the entire join table. Used by sign-out so a fresh sign-in starts
|
/// Wipe the entire join table. Used by sign-out so a fresh sign-in starts
|
||||||
|
|||||||
@@ -73,16 +73,6 @@ class ProfileRegistry {
|
|||||||
return (_db.delete(_db.profiles)..where((t) => t.kind.equals(ProfileKind.plexHome.id))).go();
|
return (_db.delete(_db.profiles)..where((t) => t.kind.equals(ProfileKind.plexHome.id))).go();
|
||||||
}
|
}
|
||||||
|
|
||||||
Future<void> reorder(List<String> idsInOrder) async {
|
|
||||||
await _db.transaction(() async {
|
|
||||||
for (var i = 0; i < idsInOrder.length; i++) {
|
|
||||||
await (_db.update(
|
|
||||||
_db.profiles,
|
|
||||||
)..where((t) => t.id.equals(idsInOrder[i]))).write(ProfilesCompanion(sortOrder: Value(i)));
|
|
||||||
}
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
Future<void> clear() async {
|
Future<void> clear() async {
|
||||||
await _db.delete(_db.profiles).go();
|
await _db.delete(_db.profiles).go();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,11 +28,6 @@ class ProfilesView {
|
|||||||
const ProfilesView({required this.profiles, required this.connectionsByProfile, required this.connectionsById});
|
const ProfilesView({required this.profiles, required this.connectionsByProfile, required this.connectionsById});
|
||||||
|
|
||||||
static const empty = ProfilesView(profiles: [], connectionsByProfile: {}, connectionsById: {});
|
static const empty = ProfilesView(profiles: [], connectionsByProfile: {}, connectionsById: {});
|
||||||
|
|
||||||
int countFor(Profile profile) {
|
|
||||||
if (profile.isPlexHome) return profile.parentConnectionId == null ? 0 : 1;
|
|
||||||
return connectionsByProfile[profile.id]?.length ?? 0;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Join-table rows that should be shown as explicit, user-manageable
|
/// Join-table rows that should be shown as explicit, user-manageable
|
||||||
|
|||||||
@@ -11,6 +11,15 @@ import 'package:plezy/services/plex_auth_service.dart';
|
|||||||
|
|
||||||
import '../test_helpers/prefs.dart';
|
import '../test_helpers/prefs.dart';
|
||||||
|
|
||||||
|
/// The id of the connection currently flagged default, read straight from the
|
||||||
|
/// row (the registry maintains the flag; there is no public reader).
|
||||||
|
Future<String?> _defaultConnectionId(AppDatabase db) async {
|
||||||
|
for (final row in await db.select(db.connections).get()) {
|
||||||
|
if (row.isDefault) return row.id;
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
JellyfinConnection _jellyfin({String id = 'srv-1', String userName = 'edde'}) {
|
JellyfinConnection _jellyfin({String id = 'srv-1', String userName = 'edde'}) {
|
||||||
return JellyfinConnection(
|
return JellyfinConnection(
|
||||||
id: id,
|
id: id,
|
||||||
@@ -71,7 +80,7 @@ void main() {
|
|||||||
group('ConnectionRegistry', () {
|
group('ConnectionRegistry', () {
|
||||||
test('list() returns empty when no connections stored', () async {
|
test('list() returns empty when no connections stored', () async {
|
||||||
expect(await registry.list(), isEmpty);
|
expect(await registry.list(), isEmpty);
|
||||||
expect(await registry.getDefault(), isNull);
|
expect(await _defaultConnectionId(db), isNull);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('first upserted connection becomes the default', () async {
|
test('first upserted connection becomes the default', () async {
|
||||||
@@ -80,8 +89,7 @@ void main() {
|
|||||||
expect(list.length, 1);
|
expect(list.length, 1);
|
||||||
expect(list.first.id, 'a');
|
expect(list.first.id, 'a');
|
||||||
|
|
||||||
final defaultConn = await registry.getDefault();
|
expect(await _defaultConnectionId(db), 'a');
|
||||||
expect(defaultConn?.id, 'a');
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test('upsert preserves type discriminator (Plex vs Jellyfin)', () async {
|
test('upsert preserves type discriminator (Plex vs Jellyfin)', () async {
|
||||||
@@ -141,10 +149,10 @@ void main() {
|
|||||||
await registry.upsert(_jellyfin(id: 'b'));
|
await registry.upsert(_jellyfin(id: 'b'));
|
||||||
// First is default by default; explicitly switch to b.
|
// First is default by default; explicitly switch to b.
|
||||||
await registry.setDefault('b');
|
await registry.setDefault('b');
|
||||||
expect((await registry.getDefault())?.id, 'b');
|
expect(await _defaultConnectionId(db), 'b');
|
||||||
// Switch back to a.
|
// Switch back to a.
|
||||||
await registry.setDefault('a');
|
await registry.setDefault('a');
|
||||||
expect((await registry.getDefault())?.id, 'a');
|
expect(await _defaultConnectionId(db), 'a');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('remove deletes a row and re-elects a default when needed', () async {
|
test('remove deletes a row and re-elects a default when needed', () async {
|
||||||
@@ -153,10 +161,10 @@ void main() {
|
|||||||
// a is default (first one in).
|
// a is default (first one in).
|
||||||
await registry.remove('a');
|
await registry.remove('a');
|
||||||
// b should now be the default.
|
// b should now be the default.
|
||||||
expect((await registry.getDefault())?.id, 'b');
|
expect(await _defaultConnectionId(db), 'b');
|
||||||
// Removing the last clears the default cleanly.
|
// Removing the last clears the default cleanly.
|
||||||
await registry.remove('b');
|
await registry.remove('b');
|
||||||
expect(await registry.getDefault(), isNull);
|
expect(await _defaultConnectionId(db), isNull);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('re-upsert preserves the existing default flag', () async {
|
test('re-upsert preserves the existing default flag', () async {
|
||||||
@@ -165,15 +173,15 @@ void main() {
|
|||||||
// wrote `isFirst` (false on update).
|
// wrote `isFirst` (false on update).
|
||||||
await registry.upsert(_jellyfin(id: 'a'));
|
await registry.upsert(_jellyfin(id: 'a'));
|
||||||
await registry.upsert(_jellyfin(id: 'b'));
|
await registry.upsert(_jellyfin(id: 'b'));
|
||||||
expect((await registry.getDefault())?.id, 'a');
|
expect(await _defaultConnectionId(db), 'a');
|
||||||
|
|
||||||
// Re-upsert the default with refreshed credentials.
|
// Re-upsert the default with refreshed credentials.
|
||||||
await registry.upsert(_jellyfin(id: 'a', userName: 'refreshed'));
|
await registry.upsert(_jellyfin(id: 'a', userName: 'refreshed'));
|
||||||
expect((await registry.getDefault())?.id, 'a');
|
expect(await _defaultConnectionId(db), 'a');
|
||||||
|
|
||||||
// And re-upserting a non-default row doesn't accidentally promote it.
|
// And re-upserting a non-default row doesn't accidentally promote it.
|
||||||
await registry.upsert(_jellyfin(id: 'b', userName: 'refreshed'));
|
await registry.upsert(_jellyfin(id: 'b', userName: 'refreshed'));
|
||||||
expect((await registry.getDefault())?.id, 'a');
|
expect(await _defaultConnectionId(db), 'a');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('recordAuthSuccess updates lastAuthenticatedAt without losing config', () async {
|
test('recordAuthSuccess updates lastAuthenticatedAt without losing config', () async {
|
||||||
|
|||||||
@@ -58,10 +58,10 @@ class _AppDatabaseTestSuite {
|
|||||||
|
|
||||||
test('ProfileConnections has no profile_id FK (virtual plex_home profiles)', () async {
|
test('ProfileConnections has no profile_id FK (virtual plex_home profiles)', () async {
|
||||||
// v20 dropped the profile_id FK so virtual Plex Home profiles can
|
// v20 dropped the profile_id FK so virtual Plex Home profiles can
|
||||||
// persist join rows without a parent `profiles` row. The two
|
// persist join rows without a parent `profiles` row. Profile deletion
|
||||||
// profile-delete sites (profile_detail_screen, profile_switch_screen)
|
// instead cleans up join rows explicitly (via the teardown flow's
|
||||||
// call ProfileConnectionRegistry.removeAllForProfile manually before
|
// removeAllProfileConnectionsAndCleanup) before deleting the profile,
|
||||||
// deleting the profile, so the cascade isn't needed.
|
// so the cascade isn't needed.
|
||||||
final now = DateTime.now().millisecondsSinceEpoch;
|
final now = DateTime.now().millisecondsSinceEpoch;
|
||||||
await db
|
await db
|
||||||
.into(db.connections)
|
.into(db.connections)
|
||||||
|
|||||||
@@ -144,16 +144,40 @@ void main() {
|
|||||||
expect(await registry.listForConnection('c1'), isEmpty);
|
expect(await registry.listForConnection('c1'), isEmpty);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('removeAllForProfile drops every row for a profile', () async {
|
test('removeAllForConnection re-promotes a default for a surviving profile', () async {
|
||||||
await registry.upsert(
|
await registry.upsert(
|
||||||
const ProfileConnection(profileId: 'p1', connectionId: 'c1', userToken: 't', userIdentifier: 'u'),
|
const ProfileConnection(profileId: 'p1', connectionId: 'c1', userToken: 't', userIdentifier: 'u'),
|
||||||
|
makeDefault: true,
|
||||||
);
|
);
|
||||||
await registry.upsert(
|
await registry.upsert(
|
||||||
const ProfileConnection(profileId: 'p1', connectionId: 'c2', userToken: 't', userIdentifier: 'u'),
|
const ProfileConnection(profileId: 'p1', connectionId: 'c2', userToken: 't', userIdentifier: 'u'),
|
||||||
);
|
);
|
||||||
final removed = await registry.removeAllForProfile('p1');
|
// c1 is default; removing it connection-wide must leave p1 with c2 as
|
||||||
expect(removed, 2);
|
// its new default rather than defaultless.
|
||||||
expect(await registry.listForProfile('p1'), isEmpty);
|
await registry.removeAllForConnection('c1');
|
||||||
|
final remaining = await registry.listForProfile('p1');
|
||||||
|
expect(remaining, hasLength(1));
|
||||||
|
expect(remaining.single.connectionId, 'c2');
|
||||||
|
expect(remaining.single.isDefault, isTrue);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('promoteMissingDefaults repairs a profile the FK cascade left defaultless', () async {
|
||||||
|
await registry.upsert(
|
||||||
|
const ProfileConnection(profileId: 'p1', connectionId: 'c1', userToken: 't', userIdentifier: 'u'),
|
||||||
|
makeDefault: true,
|
||||||
|
);
|
||||||
|
await registry.upsert(
|
||||||
|
const ProfileConnection(profileId: 'p1', connectionId: 'c2', userToken: 't', userIdentifier: 'u'),
|
||||||
|
);
|
||||||
|
// Deleting the connection cascades away p1's default join row (c1) via
|
||||||
|
// the connectionId FK, silently leaving p1 with no default flag.
|
||||||
|
await (db.delete(db.connections)..where((t) => t.id.equals('c1'))).go();
|
||||||
|
final beforeRepair = await registry.listForProfile('p1');
|
||||||
|
expect(beforeRepair.single.connectionId, 'c2');
|
||||||
|
expect(beforeRepair.single.isDefault, isFalse);
|
||||||
|
|
||||||
|
await registry.promoteMissingDefaults();
|
||||||
|
expect((await registry.listForProfile('p1')).single.isDefault, isTrue);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('upsert succeeds for a virtual plex_home profile id with no parent row', () async {
|
test('upsert succeeds for a virtual plex_home profile id with no parent row', () async {
|
||||||
|
|||||||
Reference in New Issue
Block a user