Merge pull request #60 from DorkSquadRR/fix/room-clone-envelope

Creating a room no longer fails with a blank "Failed to copy room"

DorkNet.Server.Tests/PrivateRoomInstanceTests.cs

@@ -42,7 +42,7 @@ public sealed class PrivateRoomInstanceTests : IClassFixture<DorkNetServerFactor
         var text = await resp.Content.ReadAsStringAsync();
         Assert.True(resp.IsSuccessStatusCode, $"{(int)resp.StatusCode}: {text}");
         using var json = JsonDocument.Parse(text);
-        var cloneId = json.RootElement.GetProperty("RoomId").GetInt64();
+        var cloneId = json.RootElement.GetProperty("value").GetProperty("RoomId").GetInt64();
 
         await using var scope = _factory.Services.CreateAsyncScope();
         var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();

DorkNet.Server.Tests/RoomCloneShapeTests.cs

@@ -10,13 +10,15 @@ using Microsoft.Extensions.DependencyInjection;
 namespace DorkNet.Server.Tests;
 
 /// <summary>
-/// The 2023 "copy room" flow POSTs <c>rooms/{id}/clone</c> and deserialises the
-/// reply into <c>FGCPNAACHIK</c> — the SAME type <c>GET rooms/{id}</c> returns
-/// (RecNet.Runtime/NLDBPDCNNCF.txt:4676 declares
+/// The 2023 "copy room" flow POSTs <c>rooms/{id}/clone</c> and reads the reply
+/// as the <c>{success, value, error}</c> envelope (the same RecNet rooms helper
+/// as "save room"), whose <c>value</c> is <c>FGCPNAACHIK</c> — the SAME type
+/// <c>GET rooms/{id}</c> returns (RecNet.Runtime/NLDBPDCNNCF.txt:4676 declares
 /// <c>Task&lt;FGCPNAACHIK&gt; GDHIIAHCBMN(Int64, String, CancellationToken)</c>,
-/// with the route literal on :4802).
+/// with the route literal on :4802). A bare room with no <c>success</c> is
+/// rejected with an empty error.
 ///
-/// So the two responses must be shape-identical. When they drift, the server
+/// So <c>value</c> and the GET response must be shape-identical. When they drift, the server
 /// still answers 200 and the client throws while reading it — and because the
 /// clone promise's exception is never observed, it surfaces from the finalizer
 /// thread seconds later as a bare "Failed to copy room: Exception of type
@@ -63,8 +65,12 @@ public sealed class RoomCloneShapeTests : IClassFixture<DorkNetServerFactory>
         }
 
         var getBody = await ReadJsonAsync(client, HttpMethod.Get, $"/rooms/{sourceId}");
-        var cloneBody = await ReadJsonAsync(client, HttpMethod.Post, $"/rooms/{sourceId}/clone",
+        var cloneEnvelope = await ReadJsonAsync(client, HttpMethod.Post, $"/rooms/{sourceId}/clone",
             new FormUrlEncodedContent([new KeyValuePair<string, string>("name", $"Copy{Guid.NewGuid():N}"[..20])]));
+        // The client reads clone through the {success, value, error} envelope.
+        Assert.True(cloneEnvelope.GetProperty("success").GetBoolean(), cloneEnvelope.ToString());
+        Assert.Equal("", cloneEnvelope.GetProperty("error").GetString());
+        var cloneBody = cloneEnvelope.GetProperty("value");
 
         var expected = getBody.EnumerateObject().Select(p => p.Name).ToHashSet(StringComparer.Ordinal);
         var actual = cloneBody.EnumerateObject().Select(p => p.Name).ToHashSet(StringComparer.Ordinal);
@@ -157,7 +163,7 @@ public sealed class RoomCloneShapeTests : IClassFixture<DorkNetServerFactory>
 
         var clone = await ReadJsonAsync(client, HttpMethod.Post, $"/rooms/{sourceId}/clone",
             new FormUrlEncodedContent([new KeyValuePair<string, string>("name", $"Copy{Guid.NewGuid():N}"[..20])]));
-        var cloneId = clone.GetProperty("RoomId").GetInt64();
+        var cloneId = clone.GetProperty("value").GetProperty("RoomId").GetInt64();
 
         await using (var scope = factory.Services.CreateAsyncScope())
         {
@@ -203,6 +209,24 @@ public sealed class RoomCloneShapeTests : IClassFixture<DorkNetServerFactory>
             Task.FromResult(_objects.ContainsKey($"{bucket}/{key}"));
     }
 
+    [Fact]
+    public async Task Clone_refusal_is_a_200_envelope_with_a_message()
+    {
+        using var client = Client();
+        var session = await GameClientSessionFactory.CreateAsync(client, _factory.ApexDomain);
+        client.DefaultRequestHeaders.Authorization =
+            new AuthenticationHeaderValue("Bearer", session.AccessToken);
+
+        // No room has this id, so the clone is refused; the client shows
+        // `error`, which a bare 404 would leave empty.
+        var envelope = await ReadJsonAsync(client, HttpMethod.Post, "/rooms/987654321987/clone",
+            new FormUrlEncodedContent([new KeyValuePair<string, string>("name", "NoSuchSource")]));
+
+        Assert.False(envelope.GetProperty("success").GetBoolean());
+        Assert.False(string.IsNullOrEmpty(envelope.GetProperty("error").GetString()));
+        Assert.Equal(JsonValueKind.Null, envelope.GetProperty("value").ValueKind);
+    }
+
     private static async Task<JsonElement> ReadJsonAsync(
         HttpClient client, HttpMethod method, string path, HttpContent? content = null)
     {

DorkNet.Server.Tests/RoomSave2023Tests.cs

@@ -267,8 +267,10 @@ public sealed class RoomSave2023Tests : IClassFixture<DorkNetServerFactory>
         Assert.True(resp.IsSuccessStatusCode, $"clone of {sourceId} -> {(int)resp.StatusCode}: {text}");
 
         using var json = JsonDocument.Parse(text);
-        var root = json.RootElement;
-        // Full room-details shape (FGCPNAACHIK), not a status wrapper.
+        // {success, value, error} envelope around the full room-details
+        // shape (FGCPNAACHIK).
+        Assert.True(json.RootElement.GetProperty("success").GetBoolean(), text);
+        var root = json.RootElement.GetProperty("value");
         foreach (var key in new[] { "RoomId", "SubRooms", "Roles" })
             Assert.True(root.TryGetProperty(key, out _), $"clone response missing '{key}': {text}");
         return root.GetProperty("RoomId").GetInt64();

DorkNet.Server/Controllers/API/Rooms/V2/RoomsModerationController.cs

@@ -1375,13 +1375,13 @@ public class RoomsModerationController(
     {
         var newName = await ReadCloneNameAsync();
         var source = await db.Rooms.FirstOrDefaultAsync(r => r.Id == roomId);
-        if (source is null) return NotFound();
+        if (source is null) return CloneResult(null, "That room doesn't exist.");
         // RRO/AG rooms are first-party templates meant to be cloned — the
         // room-creation "base room" picker starts a new build by cloning one
         // (RecCenter, MakerRoom, …). Their seed rows have CloningAllowed=false,
-        // so without the IsAGRoom exception every base-room create 403s.
+        // so without the IsAGRoom exception every base-room create is refused.
         if (!source.CloningAllowed && !source.IsAGRoom && source.CreatorPlayerId != Me)
-            return Forbid();
+            return CloneResult(null, "You can't clone this room!");
 
         if (string.IsNullOrWhiteSpace(newName))
             newName = $"{source.Name}Copy";
@@ -1517,17 +1517,22 @@ public class RoomsModerationController(
 
         await GiveCloneItsOwnBlobsAsync(clone, sourceScenes, clonedScenes, Me);
 
-        // The 2023 client's clone (RecNet.Runtime NLDBPDCNNCF.GDHIIAHCBMN)
-        // deserializes the response as the FULL room-details object
-        // (FGCPNAACHIK — the same type get-by-id / rename return on the
-        // roomserver host), NOT the slim list-room shape. This is the
-        // roomserver-family path (rooms/{id}/clone), so use
-        // BuildRoomServerDetails — the exact shape GET rooms/{id} returns for
-        // the 2023 client. Returning ToWireRoom (or the wrong details variant)
-        // made the strict reader fail → "Failed to copy room: Failed to clone
-        // room".
-        return Ok(RoomsController.BuildRoomServerDetails(clone, clonedScenes));
+        // The value is the FULL room-details object (FGCPNAACHIK), the exact
+        // shape GET rooms/{id} returns for the 2023 client — not the slim
+        // list-room shape, which made the strict reader fail.
+        return CloneResult(RoomsController.BuildRoomServerDetails(clone, clonedScenes));
     }
+    /// <summary>Reply for <c>rooms/{id}/clone</c>. The 2023 client sends the
+    /// clone through the same RecNet rooms helper as "save room" and "restore
+    /// subroom save" (NLDBPDCNNCF.BDBHCEGKGDC with an op name), which reads the
+    /// legacy <c>{success, value, error}</c> envelope. A bare room-details body
+    /// has no <c>success</c>, so the client rejects it with an empty error:
+    /// "Failed to copy room: Exception of type 'CEMNLBKJABA' was thrown", even
+    /// though the room was created. Refusals are 200 with success=false and a
+    /// message the client shows.</summary>
+    private OkObjectResult CloneResult(object? room, string error = "") =>
+        Ok(new { success = error.Length == 0, value = room, error });
+
     /// <summary>Give a freshly-cloned room its OWN copy of the source's save
     /// data instead of pointing its scenes at the source's blob.
     ///

docs/recroom-2023-client-api-complete.md

@@ -659,7 +659,7 @@ Diffed all 45 real HTTP routes of the 2023-03-21 client's rooms subsystem agains
 | GET + POST | `rooms/{0}/bans` | GET: none. POST 'ban players': form fields id (repeated Int32 account ids) + banMask (Int32 enum DCDPJPHBHOA) | GET → [IBHAKOOKEEE {"AccountId": Int32, "BannedByAccountId": Int32, "BanStartTime": DateTime string}]. POST → none parsed. |
 | POST | `rooms/{0}/bans/import` | form field: sourceRoomId (Int64) | none parsed; return 200 |
 | DELETE | `rooms/{0}/bans/{1}` | form field: banMask (Int32 enum) | none parsed; return 200 |
-| POST | `rooms/{0}/clone` | x-www-form-urlencoded field: name (String) — new room name | FGCPNAACHIK FULL room details of the clone (not a status wrapper); client 404→empty-body surfaces as message-less 'Failed to copy room' |
+| POST | `rooms/{0}/clone` | x-www-form-urlencoded field: name (String) — new room name | `{success, value, error}` envelope with `value` = FGCPNAACHIK FULL room details of the clone (same helper as "save room"; a bare room has no `success` and is rejected with an empty error). Client 404→empty-body also surfaces as message-less 'Failed to copy room' |
 | PUT | `rooms/{0}/cloning` | form field: cloningAllowed (Boolean) | FGCPNAACHIK full room details |
 | PUT | `rooms/{0}/comments` | form field: disable (Boolean) — key is 'disable', maps to DisableRoomComments | FGCPNAACHIK full room details |
 | PUT | `rooms/{0}/creator` | form field: accountId (Int32) | FGCPNAACHIK full room details |

docs/recroom-2023-room-save.md

@@ -89,8 +89,15 @@ body — the same `roomserver/` prefix as every other room mutation from
 that client. A bare-only `rooms/{id}/clone` route 404s with an empty
 body, which the client's promise layer surfaces as a message-less
 `Failed to copy room: Exception of type 'CEMNLBKJABA' was thrown`.
-The response must be the FULL room-details object (`FGCPNAACHIK`,
-same shape as `GET roomserver/rooms/{id}`), not a status wrapper.
+The response is the legacy `{success, value, error}` envelope, the same
+one "save room" and "restore subroom save" use: the clone goes through the
+same RecNet rooms helper (`NLDBPDCNNCF.BDBHCEGKGDC`, which takes an op
+name). `value` is the FULL room-details object (`FGCPNAACHIK`, same shape
+as `GET roomserver/rooms/{id}`). A bare room-details body has no `success`,
+so the client rejects it with an empty error and shows the same
+message-less `Failed to copy room`, even though the room was created.
+Refusals (unknown source, cloning not allowed) are HTTP 200 with
+`success: false` and the message in `error`.
 
 Blob semantics on clone (`RoomsModerationController.BareClone`):