fix(connections): keep a connection's creation time across re-authentication
ConnectionRegistry.upsert already preserved isDefault on conflict but rewrote created_at from the in-memory model. Re-signing in rebuilds the connection with DateTime.now() under the same stable id, so the row's creation time jumped forward on every reauth. That was cosmetic while created_at only drove list ordering. It is now behaviour: it picks which connection lends a profile its picture, so re-adding the originally-first connection could hand the avatar to a later one. remove() also promotes the oldest remaining row to default and was reading the same restamped value. Preserve the existing row's created_at on conflict, reusing the lookup upsert already performs for isDefault.
This commit is contained in:
@@ -42,14 +42,22 @@ class ConnectionRegistry {
|
|||||||
|
|
||||||
/// 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` and `createdAt` (so token/metadata
|
||||||
/// the default flag).
|
/// refreshes don't clear the default flag or restamp creation order).
|
||||||
|
///
|
||||||
|
/// Creation order is behaviour, not bookkeeping: it decides which
|
||||||
|
/// connection lends a profile its picture, and `remove` promotes the oldest
|
||||||
|
/// remaining row to default. Re-authenticating rebuilds the model with
|
||||||
|
/// `DateTime.now()` and reuses the same stable id, so without this the
|
||||||
|
/// originally-first connection would jump to last on every re-sign-in.
|
||||||
Future<void> upsert(Connection connection) async {
|
Future<void> upsert(Connection connection) async {
|
||||||
await _db.runIdentityMutation(() async {
|
await _db.runIdentityMutation(() async {
|
||||||
final existing = await (_db.select(_db.connections)..where((t) => t.id.equals(connection.id))).getSingleOrNull();
|
final existing = await (_db.select(_db.connections)..where((t) => t.id.equals(connection.id))).getSingleOrNull();
|
||||||
final bool isDefault;
|
final bool isDefault;
|
||||||
|
final int createdAt;
|
||||||
if (existing != null) {
|
if (existing != null) {
|
||||||
isDefault = existing.isDefault;
|
isDefault = existing.isDefault;
|
||||||
|
createdAt = existing.createdAt;
|
||||||
} else {
|
} else {
|
||||||
final any =
|
final any =
|
||||||
await (_db.selectOnly(_db.connections)
|
await (_db.selectOnly(_db.connections)
|
||||||
@@ -57,6 +65,7 @@ class ConnectionRegistry {
|
|||||||
..limit(1))
|
..limit(1))
|
||||||
.getSingleOrNull();
|
.getSingleOrNull();
|
||||||
isDefault = any == null;
|
isDefault = any == null;
|
||||||
|
createdAt = connection.createdAt.millisecondsSinceEpoch;
|
||||||
}
|
}
|
||||||
final protectedConfig = await CredentialVault.protectConnectionConfig(
|
final protectedConfig = await CredentialVault.protectConnectionConfig(
|
||||||
connection.kind.id,
|
connection.kind.id,
|
||||||
@@ -68,7 +77,7 @@ class ConnectionRegistry {
|
|||||||
displayName: Value(connection.displayName),
|
displayName: Value(connection.displayName),
|
||||||
configJson: Value(jsonEncode(protectedConfig)),
|
configJson: Value(jsonEncode(protectedConfig)),
|
||||||
isDefault: Value(isDefault),
|
isDefault: Value(isDefault),
|
||||||
createdAt: Value(connection.createdAt.millisecondsSinceEpoch),
|
createdAt: Value(createdAt),
|
||||||
lastAuthenticatedAt: Value(connection.lastAuthenticatedAt?.millisecondsSinceEpoch),
|
lastAuthenticatedAt: Value(connection.lastAuthenticatedAt?.millisecondsSinceEpoch),
|
||||||
);
|
);
|
||||||
await _db.into(_db.connections).insertOnConflictUpdate(row);
|
await _db.into(_db.connections).insertOnConflictUpdate(row);
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ Future<String?> _defaultConnectionId(AppDatabase db) async {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
JellyfinConnection _jellyfin({String id = 'srv-1', String userName = 'edde'}) {
|
JellyfinConnection _jellyfin({String id = 'srv-1', String userName = 'edde', int createdAtMs = 1_000_000}) {
|
||||||
return JellyfinConnection(
|
return JellyfinConnection(
|
||||||
id: id,
|
id: id,
|
||||||
baseUrl: 'https://jellyfin.local',
|
baseUrl: 'https://jellyfin.local',
|
||||||
@@ -30,7 +30,7 @@ JellyfinConnection _jellyfin({String id = 'srv-1', String userName = 'edde'}) {
|
|||||||
userName: userName,
|
userName: userName,
|
||||||
accessToken: 'tok-$id',
|
accessToken: 'tok-$id',
|
||||||
deviceId: 'dev-1',
|
deviceId: 'dev-1',
|
||||||
createdAt: DateTime.fromMillisecondsSinceEpoch(1_000_000),
|
createdAt: DateTime.fromMillisecondsSinceEpoch(createdAtMs),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -184,6 +184,28 @@ void main() {
|
|||||||
expect(await _defaultConnectionId(db), 'a');
|
expect(await _defaultConnectionId(db), 'a');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('re-upsert preserves the original creation order', () async {
|
||||||
|
// Regression: creation order decides which connection lends a profile
|
||||||
|
// its picture (issue #1667) and which row `remove` promotes to default.
|
||||||
|
// Re-authenticating rebuilds the model with `DateTime.now()` under the
|
||||||
|
// same stable id, so an unguarded writer restamped the originally-first
|
||||||
|
// connection and shuffled it to last.
|
||||||
|
await registry.upsert(_jellyfin(id: 'first', createdAtMs: 1_000_000));
|
||||||
|
await registry.upsert(_jellyfin(id: 'second', createdAtMs: 2_000_000));
|
||||||
|
|
||||||
|
await registry.upsert(_jellyfin(id: 'first', createdAtMs: 9_000_000));
|
||||||
|
|
||||||
|
final list = await registry.list();
|
||||||
|
expect(list.map((c) => c.id).toList(), ['first', 'second']);
|
||||||
|
expect(list.first.createdAt, DateTime.fromMillisecondsSinceEpoch(1_000_000));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a genuinely new connection keeps the creation time it was built with', () async {
|
||||||
|
await registry.upsert(_jellyfin(id: 'a', createdAtMs: 5_000_000));
|
||||||
|
|
||||||
|
expect((await registry.list()).single.createdAt, DateTime.fromMillisecondsSinceEpoch(5_000_000));
|
||||||
|
});
|
||||||
|
|
||||||
test('recordAuthSuccess updates lastAuthenticatedAt without losing config', () async {
|
test('recordAuthSuccess updates lastAuthenticatedAt without losing config', () async {
|
||||||
await registry.upsert(_jellyfin(id: 'a'));
|
await registry.upsert(_jellyfin(id: 'a'));
|
||||||
final at = DateTime.fromMillisecondsSinceEpoch(2_000_000);
|
final at = DateTime.fromMillisecondsSinceEpoch(2_000_000);
|
||||||
|
|||||||
Reference in New Issue
Block a user