From 8be0f90dbc2317d7130402a710486b08e8f91519 Mon Sep 17 00:00:00 2001 From: Alexa <53471801+Alexa-RR@users.noreply.github.com> Date: Sun, 25 Oct 2026 13:59:30 +0200 Subject: [PATCH] fix: sub-room id sequence, restore sync, presence index TTL, permission races From the local CodeRabbit review: - Scene ids: the repair and room clone assigned max + 1, which never advanced the Postgres identity sequence (live: sequence 3412, max id 3466), so the next add / clone sub-room would collide on the primary key. The repair now moves the sequence past the highest id at startup, and both let the database assign ids. - Room-level restores (datahistory/restore and the admin restore) now also restore the entry sub-room, whose blob room details serve first. - TouchRoom refreshes the room's presence index too, so players kept alive only by heartbeats no longer drop out of the instance browser after 20 min. - Sub-room permission saves match cells exactly (as the unique index does) and retry when a concurrent editor inserted the same new cell. --- .../SubRoomsAndInstances2023Tests.cs | 29 ++++++ .../API/Rooms/V2/RoomsController.cs | 88 +++++++++++++------ .../Services/PlayerPresenceService.cs | 14 ++- .../Services/RoomPermissionMatrix.cs | 2 +- DorkNet.Server/Services/RoomService.cs | 39 +++++--- 5 files changed, 129 insertions(+), 43 deletions(-) diff --git a/DorkNet.Server.Tests/SubRoomsAndInstances2023Tests.cs b/DorkNet.Server.Tests/SubRoomsAndInstances2023Tests.cs index 092ea5f..cd4e056 100644 --- a/DorkNet.Server.Tests/SubRoomsAndInstances2023Tests.cs +++ b/DorkNet.Server.Tests/SubRoomsAndInstances2023Tests.cs @@ -118,6 +118,35 @@ public sealed class SubRoomsAndInstances2023Tests : IClassFixture(); + await db.RoomScenes.Where(s => s.RoomId == roomId) + .ExecuteUpdateAsync(s => s.SetProperty(x => x.DataBlobName, "blob-old")); + db.RoomDataBlobs.Add(new RoomDataBlobEntity { RoomId = roomId, BlobName = "blob-new", UploadedByPlayerId = owner.PlayerId }); + await db.SaveChangesAsync(); + } + + using var api = Client("api", owner); + using var response = await api.PostAsync("/api/rooms/v1/restore", new FormUrlEncodedContent(new Dictionary + { + ["roomId"] = roomId.ToString(), ["blobName"] = "blob-new", + })); + Assert.True(response.IsSuccessStatusCode, await response.Content.ReadAsStringAsync()); + + await using var check = _factory.Services.CreateAsyncScope(); + var checkDb = check.ServiceProvider.GetRequiredService(); + var blobs = await checkDb.RoomScenes.AsNoTracking().Where(s => s.RoomId == roomId) + .OrderBy(s => s.OrderIndex).Select(s => s.DataBlobName).ToListAsync(); + // The entry sub-room follows the restore; other sub-rooms keep theirs. + Assert.Equal(new[] { "blob-new", "blob-old" }, blobs); + } + [Fact] public async Task Startup_repair_restores_a_missing_entry_sub_room_next_to_others() { diff --git a/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs b/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs index 1a2541a..052a974 100644 --- a/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs +++ b/DorkNet.Server/Controllers/API/Rooms/V2/RoomsController.cs @@ -1406,6 +1406,7 @@ public class RoomsController( .ExecuteUpdateAsync(s => s .SetProperty(r => r.CurrentDataBlobName, blob.BlobName) .SetProperty(r => r.UpdatedAt, DateTime.UtcNow)); + if (!room.IsDormRoom) await SyncEntrySceneBlobAsync(room.Id, blob.BlobName); await db.SaveChangesAsync(); // Push the same SubscriptionUpdateRoom the save flow sends so @@ -1479,10 +1480,27 @@ public class RoomsController( .ExecuteUpdateAsync(s => s .SetProperty(r => r.CurrentDataBlobName, blob.BlobName) .SetProperty(r => r.UpdatedAt, DateTime.UtcNow)); + if (!room.IsDormRoom) await SyncEntrySceneBlobAsync(roomId, blob.BlobName); await db.SaveChangesAsync(); return Ok(new { restored = blob.BlobName }); } + /// A room-level restore also restores the entry sub-room + /// (OrderIndex 0): room details serve the entry scene's own blob ahead + /// of the room's, so updating only Rooms.CurrentDataBlobName + /// would leave the old version in place. The per-sub-room restore keeps + /// the two in step the other way round. Dorms keep their blob per + /// player (DormState) instead. + private Task SyncEntrySceneBlobAsync(long roomId, string blobName) + { + var now = DateTime.UtcNow; + return db.RoomScenes + .Where(s => s.RoomId == roomId && s.OrderIndex == 0) + .ExecuteUpdateAsync(s => s + .SetProperty(x => x.DataBlobName, blobName) + .SetProperty(x => x.DataModifiedAt, now)); + } + /// GET api/rooms/v{1,2}/personaldetails/{roomId} — /// per-caller bookmark/cheer flags for one room. Wire shape verified /// against RecNet.PersonalRoomDetails.Deserialize @@ -3829,35 +3847,51 @@ public class RoomsController( var scene = await GetOrCreateSceneForMutationAsync(room, subRoomId); if (scene is null) return NotFound(); - var existing = await db.RoomRolePermissions - .Where(p => p.RoomId == roomId && p.SubRoomId == subRoomId) - .ToListAsync(); var now = DateTime.UtcNow; - foreach (var cell in cells) - { - var row = existing.FirstOrDefault(p => - p.Role == cell.Role - && string.Equals(p.Permission, cell.Permission, StringComparison.OrdinalIgnoreCase)); - if (row is null) - { - row = new RoomRolePermissionEntity - { - RoomId = roomId, - SubRoomId = subRoomId, - Role = cell.Role, - Permission = cell.Permission, - }; - db.RoomRolePermissions.Add(row); - existing.Add(row); - } - row.Type = cell.Type; - row.Override = cell.Override; - row.Value = cell.Value; - row.UpdatedByPlayerId = pid; - row.UpdatedAt = now; - } scene.DataModifiedAt = now; - await db.SaveChangesAsync(); + for (var attempt = 1; ; attempt++) + { + var existing = await db.RoomRolePermissions + .Where(p => p.RoomId == roomId && p.SubRoomId == subRoomId) + .ToListAsync(); + foreach (var cell in cells) + { + // Exact match, like the unique index on + // (RoomId, SubRoomId, Role, Permission). + var row = existing.FirstOrDefault(p => + p.Role == cell.Role && string.Equals(p.Permission, cell.Permission, StringComparison.Ordinal)); + if (row is null) + { + row = new RoomRolePermissionEntity + { + RoomId = roomId, + SubRoomId = subRoomId, + Role = cell.Role, + Permission = cell.Permission, + }; + db.RoomRolePermissions.Add(row); + existing.Add(row); + } + row.Type = cell.Type; + row.Override = cell.Override; + row.Value = cell.Value; + row.UpdatedByPlayerId = pid; + row.UpdatedAt = now; + } + try + { + await db.SaveChangesAsync(); + break; + } + catch (DbUpdateException) when (attempt < 3) + { + // Another editor inserted the same new cell between our read + // and our insert (unique index). Forget our pending rows and + // apply the cells again, now as updates of theirs. + foreach (var entry in db.ChangeTracker.Entries().ToList()) + entry.State = EntityState.Detached; + } + } logger.LogInformation( "[subroom-permissions] room={Room} sub={Sub} by={By} cells={Count}", diff --git a/DorkNet.Server/Services/PlayerPresenceService.cs b/DorkNet.Server/Services/PlayerPresenceService.cs index fb93733..c0e11f9 100644 --- a/DorkNet.Server/Services/PlayerPresenceService.cs +++ b/DorkNet.Server/Services/PlayerPresenceService.cs @@ -141,7 +141,19 @@ public class PlayerPresenceService { try { - mux.GetDatabase().KeyExpire(Key(playerId), PresenceTtl); + var db = mux.GetDatabase(); + db.KeyExpire(Key(playerId), PresenceTtl); + // Keep the room's membership index alive as long as its + // members are: a player refreshed only through here would + // otherwise outlive the set (SetRoom's 2 × TTL) and drop out + // of the instance browser and room pushes. + var json = db.StringGet(Key(playerId)); + if (json.HasValue && + JsonSerializer.Deserialize(json.ToString(), JsonOpts) is { RoomId: > 0 } room) + { + db.SetAdd(RoomIndexKey(room.RoomId), playerId); + db.KeyExpire(RoomIndexKey(room.RoomId), PresenceTtl + PresenceTtl); + } } catch (Exception ex) { diff --git a/DorkNet.Server/Services/RoomPermissionMatrix.cs b/DorkNet.Server/Services/RoomPermissionMatrix.cs index 5b19a19..44b0a72 100644 --- a/DorkNet.Server/Services/RoomPermissionMatrix.cs +++ b/DorkNet.Server/Services/RoomPermissionMatrix.cs @@ -60,7 +60,7 @@ public static class RoomPermissionMatrix var cell = new Cell(row.Permission, row.Role, row.Override, row.Type, row.Value); var at = cells.FindIndex(c => c.Role == row.Role - && string.Equals(c.Permission, row.Permission, StringComparison.OrdinalIgnoreCase)); + && string.Equals(c.Permission, row.Permission, StringComparison.Ordinal)); if (at >= 0) cells[at] = cell; else cells.Add(cell); } diff --git a/DorkNet.Server/Services/RoomService.cs b/DorkNet.Server/Services/RoomService.cs index 5406386..6596323 100644 --- a/DorkNet.Server/Services/RoomService.cs +++ b/DorkNet.Server/Services/RoomService.cs @@ -570,6 +570,22 @@ public class RoomService(DorkNetDbContext db) DataModifiedAt = DateTime.UtcNow, }; + /// Postgres only: moves the RoomScenes id sequence past the + /// highest existing id. Rows written with explicit ids (seeds, older + /// builds of the clone and repair code) leave it behind, and the next + /// insert that relies on it (add / clone sub-room) then collides on the + /// primary key. A no-op on SQLite, which assigns max + 1 itself. + public async Task SyncRoomSceneIdSequenceAsync() + { + if (!db.Database.IsNpgsql()) return; + await db.Database.ExecuteSqlRawAsync( + """ + SELECT setval(pg_get_serial_sequence('"RoomScenes"', 'Id'), + GREATEST(COALESCE((SELECT MAX("Id") FROM "RoomScenes"), 0), 1), + EXISTS (SELECT 1 FROM "RoomScenes")); + """); + } + /// Startup self-heal for sub-rooms (idempotent). /// /// Every non-dorm room must own at least one real RoomScenes row, @@ -583,12 +599,12 @@ public class RoomService(DorkNetDbContext db) /// are left alone: their details are shaped per player and their single /// scene is synthesised on purpose. /// - /// Scene ids are assigned explicitly (max + 1) for the same reason - /// the clone handler does: seeded rows were inserted with manual ids - /// without advancing the Postgres identity sequence. Every service that + /// It first moves the Postgres id sequence of RoomScenes past the + /// highest id (), then lets the + /// database assign ids like every other scene insert. Every service that /// runs the database bootstrap calls this at startup, so on Postgres the - /// whole repair holds the bootstrap's advisory lock: replicas booting - /// together take turns instead of handing out the same ids. + /// whole repair holds the bootstrap's advisory lock and replicas booting + /// together take turns. /// Returns (rooms given an entry scene, scenes renamed). public async Task<(int ScenesAdded, int ScenesRenamed)> RepairRoomScenesAsync() { @@ -598,16 +614,13 @@ public class RoomService(DorkNetDbContext db) // The entry sub-room is OrderIndex 0 (its blob is the room's own front // door), so a room missing it is repaired even when it has others. + await SyncRoomSceneIdSequenceAsync(); + var roomsWithoutEntry = await db.Rooms .Where(r => !r.IsDormRoom && !db.RoomScenes.Any(s => s.RoomId == r.Id && s.OrderIndex == 0)) .ToListAsync(); - var nextSceneId = await db.RoomScenes.MaxAsync(s => (long?)s.Id) ?? 0L; foreach (var room in roomsWithoutEntry) - { - var scene = NewHomeScene(room); - scene.Id = ++nextSceneId; - db.RoomScenes.Add(scene); - } + db.RoomScenes.Add(NewHomeScene(room)); var unnamed = await db.RoomScenes .Where(s => s.Name == null || s.Name.Trim() == string.Empty) @@ -1144,9 +1157,7 @@ public class RoomService(DorkNetDbContext db) db.Rooms.Add(clone); // Every room owns a real entry sub-room (see RepairRoomScenesAsync). - var home = NewHomeScene(clone); - home.Id = (await db.RoomScenes.MaxAsync(s => (long?)s.Id) ?? 0L) + 1; - db.RoomScenes.Add(home); + db.RoomScenes.Add(NewHomeScene(clone)); await db.SaveChangesAsync(); return new CloneResult(0 /* Success */, clone); }