[rooms] fixup room save endpoint to return same as room endpoint (no effect, just QOL)

2026-10-25 15:09:18 -07:00 · 2026-10-08 00:44:42 -04:00
parent 909bcebb38
commit e54224dbd0
4 changed files with 209 additions and 36 deletions
@@ -115,9 +115,13 @@ inconsistency here without checking the client first.
  `StagedSubRoomDataSaveId` carries no subroom context), and nothing is overwritten, so
  `…/saves` is real history and `publish_save` doubles as restore-a-save. There
  is no `GET …/subrooms/:sid/data`; only the POST (the room save) exists on that path.
-  `GET …/saves/:saveId` is the detail behind a list row, under the same gate, but in the
-  CAMELCASE projection the room save's response uses — not the PascalCase rows the list
-  serves. Three shapes of one save; keep them straight.
+  `GET …/saves/:saveId` is the detail behind a list row, under the same gate, and a
+  PASCALCASE row like the list serves — NOT the camelCase projection the room save's
+  response uses (served that, the client showed a save with every value missing). It
+  differs from the list row only in how it names bundles: each `UnitySubAssets` entry is
+  `{ UnityAssetId, UnityAsset, UnityAssetHash }`, the main bundle is lifted to top-level
+  `UnityAsset`/`UnityAssetHash`, and there is no `Tags`. Three shapes of one save; keep
+  them straight.
 - The save reads (`rooms`: `…/saves`, `…/saves/no_unity_assets` and `…/saves/:saveId`) are
  auth-gated and readable by the room's creator, a CO-OWNER, or anyone whose live `presence`
  row puts them in that room. They list unpublished staged saves, so they aren't public; but
@@ -175,10 +175,10 @@ export const LoadScreenDto = z.object({
 * no moderation state, no asset arrays; but `unityAsset`/`unityAssetHash`/`dataBlobHash`
 * that `CurrentSave` doesn't show). The two are deliberately not unified.
 *
- * Also what `GET …/subrooms/{subRoomId}/saves/{saveId}` answers — one save fetched by id
- * is the same thing the save that created it returned, so both go through
- * `toSaveResponse`. Note the `…/saves` LIST is the third shape here: it serves the raw
- * PascalCase rows ({@link SubRoomDataSaveDto}), not this.
+ * ONLY the room save answers this. `GET …/subrooms/{subRoomId}/saves/{saveId}` once did
+ * too, and the client rendered that save with every value missing: the detail is a
+ * PascalCase row ({@link SubRoomDataSaveDetailDto}), like the `…/saves` LIST serves
+ * ({@link SubRoomDataSaveDto}).
 */
 export const SubRoomDataSaveResponseDto = z.object({
 	subRoomDataSaveId: z.int(),
@@ -976,6 +976,51 @@ export const SubRoomDataSaveNoUnityAssetsDto = z.object({
 	UgcSubVersion: z.int(),
 })

+/**
+ * One save as `GET …/subrooms/{subRoomId}/saves/{saveId}` answers it — the PascalCase row
+ * of {@link SubRoomDataSaveDto}, with its bundles named the way the client's save-detail
+ * decoder reads them: each `UnitySubAssets` entry is `{ UnityAssetId, UnityAsset,
+ * UnityAssetHash }` rather than `Filename`/`Hash`, and the save's own main bundle is
+ * lifted to top-level `UnityAsset`/`UnityAssetHash`. No `Tags`; `UnityAssetId` is always
+ * present, null when the save carried none. NOT the camelCase
+ * {@link SubRoomDataSaveResponseDto} the room save returns — served that, the client
+ * showed a save with every value missing.
+ */
+export const SubRoomDataSaveDetailDto = z.object({
+	UnitySubAssets: z
+		.array(
+			z.object({
+				UnityAssetId: z.string().nullable(),
+				UnityAsset: z.string().nullable().describe('The bundle filename, from the CDN'),
+				UnityAssetHash: z.string().nullable().describe('Base64 SHA-256 of the bundle'),
+			})
+		)
+		.describe('Main Studio bundles for this save; empty for a maker-pen save'),
+	ReferencedUnityAssets: z.array(z.unknown()).describe('Always empty'),
+	UnityAsset: z
+		.string()
+		.nullable()
+		.describe(
+			'The main bundle for the named target (Windows when none); null for a maker-pen save'
+		),
+	UnityAssetHash: z.string().nullable().describe('Base64 SHA-256 of `UnityAsset`; null with it'),
+	SubRoomDataSaveId: z.int(),
+	SubRoomId: z.int(),
+	UnityAssetId: z.string().nullable().describe('Null unless the save carried one'),
+	ReferencedUnityAssetIds: z.array(z.string()).describe('Always empty — we record none'),
+	DataBlob: z.string().describe('The scene-data key the client downloads from the CDN'),
+	DataBlobHash: z.string().nullable(),
+	PersistenceVersion: z.int(),
+	OMVersion: z.int(),
+	SavedByAccountId: z.int().nullable(),
+	SavedOnPlatform: z.int().describe('0 — the save request carries no platform'),
+	SavedOnDeviceClass: z.int().describe('0 — the save request carries no device class'),
+	Description: z.string().describe('The save comment; empty string when none'),
+	ModerationState: z.int(),
+	CreatedAt: z.string(),
+	UgcSubVersion: z.int(),
+})
+
 /**
 * `GET /rooms/{roomId}/subrooms/{subRoomId}/saves/no_unity_assets` — the same history page
 * as {@link SubRoomSavesPage}, carrying the lighter rows. A BARE paged wrapper: no
@@ -170,7 +170,7 @@ import {
 	ShowcasedRooms,
 	stringQuery,
 	SubRoomAccessibilityRequest,
-	SubRoomDataSaveResponseDto,
+	SubRoomDataSaveDetailDto,
 	subRoomIdParam,
 	SubRoomPermissionsRequest,
 	SubRoomSavesNoUnityAssetsPage,
@@ -1010,6 +1010,62 @@ function toSaveWithoutUnityAssets(save: Record<string, unknown>) {
 	}
 }

+/**
+ * One save as `GET …/subrooms/:sid/saves/:saveId` answers it — the PascalCase row the
+ * `…/saves` list serves, NOT the camelCase `subRoomDataSave` the room save that created
+ * it returned (an earlier version served that, and the client rendered a save whose
+ * values were all missing). Two things differ from the list row, both observed against
+ * the reference:
+ *
+ * - Each `UnitySubAssets` entry is `{ UnityAssetId, UnityAsset, UnityAssetHash }` — the
+ *   stored `Filename`/`Hash` under the names the client's save-detail decoder reads —
+ *   and the save's own main bundle is lifted to top-level `UnityAsset`/`UnityAssetHash`
+ *   (the named target's, Windows when none was named; both null for a maker-pen save).
+ * - No `Tags`, and `UnityAssetId` is always present (null when the save carried none).
+ *
+ * Built key by key like {@link toSaveWithoutUnityAssets}, for the same reason.
+ */
+function toSaveDetail(save: Record<string, unknown>, target: number | null) {
+	const str = (v: unknown) => (typeof v === 'string' ? v : null)
+	const num = (v: unknown) => (typeof v === 'number' ? v : null)
+	const main = pickBakedBundle(save, target)
+	const bundles = (Array.isArray(save.UnitySubAssets) ? save.UnitySubAssets : []).filter(
+		(item): item is Record<string, unknown> =>
+			typeof item === 'object' &&
+			item !== null &&
+			(target === null || (item as { Target?: unknown }).Target === target)
+	)
+	return {
+		UnitySubAssets: bundles.map((row) => ({
+			UnityAssetId: str(row.UnityAssetId),
+			UnityAsset: str(row.Filename),
+			UnityAssetHash: str(row.Hash),
+		})),
+		ReferencedUnityAssets: Array.isArray(save.ReferencedUnityAssets)
+			? save.ReferencedUnityAssets
+			: [],
+		UnityAsset: main?.filename ?? null,
+		UnityAssetHash: main?.hash ?? null,
+		SubRoomDataSaveId: num(save.SubRoomDataSaveId),
+		SubRoomId: num(save.SubRoomId),
+		UnityAssetId: str(save.UnityAssetId),
+		ReferencedUnityAssetIds: Array.isArray(save.ReferencedUnityAssetIds)
+			? save.ReferencedUnityAssetIds
+			: [],
+		DataBlob: str(save.DataBlob) ?? '',
+		DataBlobHash: str(save.DataBlobHash),
+		PersistenceVersion: num(save.PersistenceVersion) ?? 0,
+		OMVersion: num(save.OMVersion) ?? 0,
+		SavedByAccountId: num(save.SavedByAccountId),
+		SavedOnPlatform: num(save.SavedOnPlatform) ?? 0,
+		SavedOnDeviceClass: num(save.SavedOnDeviceClass) ?? 0,
+		Description: str(save.Description) ?? '',
+		ModerationState: num(save.ModerationState) ?? 0,
+		CreatedAt: str(save.CreatedAt) ?? '',
+		UgcSubVersion: num(save.UgcSubVersion) ?? 0,
+	}
+}
+
 /** Client envelope for room mutations: `{ success, error, value }` (lowercase). */
 function roomEnvelope(c: Context<App>, value: unknown, error = '') {
 	return c.json({ success: error === '', error, value })
@@ -3670,20 +3726,34 @@ const app = new Hono<App>()
 			tags: ['Subrooms'],
 			summary: 'One of a subroom’s saves by id',
 			description: [
-				'A single save, in the SAME camelCase projection the room save that created it',
-				'returned — not the PascalCase rows `…/saves` lists. Save ids are globally',
-				'unique but resolved scoped to the subroom, so one subroom cannot read another’s',
-				'save by guessing an id: a save that belongs elsewhere is a 404, same as an unknown',
-				'one.',
+				'A single save as a PascalCase row like the ones `…/saves` lists — NOT the camelCase',
+				'`subRoomDataSave` the room save that created it returned (served that way, the',
+				'client showed a save with every value missing). It differs from the list row in',
+				'how it names bundles: each `UnitySubAssets` entry is `{ UnityAssetId, UnityAsset,',
+				'UnityAssetHash }`, the save’s main bundle is lifted to top-level `UnityAsset`/',
+				'`UnityAssetHash` (null for a maker-pen save), and there is no `Tags`. An integer',
+				'`unityAssetTarget` picks which target’s bundles those are (0 Windows, 2',
+				'Android/Quest); none named means every bundle in the list and Windows on top.',
+				'Save ids are globally unique but resolved scoped to the subroom, so one subroom',
+				'cannot read another’s save by guessing an id: a save that belongs elsewhere is a',
+				'404, same as an unknown one.',
 				'',
 				'Gated like the list it details — the room’s creator, a co-owner, or anyone whose',
 				'presence puts them in the room. A save id resolves whether or not it was ever published, so this',
 				'reads unpublished work.',
 			].join(' '),
 			security: AUTHED,
-			parameters: [roomIdParam, subRoomIdParam, saveIdParam],
+			parameters: [
+				roomIdParam,
+				subRoomIdParam,
+				saveIdParam,
+				stringQuery(
+					'unityAssetTarget',
+					'When an integer, the bundles are that target’s only (0 Windows, 2 Android/Quest)'
+				),
+			],
 			responses: {
-				200: json(SubRoomDataSaveResponseDto, 'The save'),
+				200: json(SubRoomDataSaveDetailDto, 'The save'),
 				401: UNAUTHORIZED_RESPONSE,
 				403: FORBIDDEN_RESPONSE,
 				404: { description: 'No such room, subroom, or save on that subroom' },
@@ -3704,7 +3774,7 @@ const app = new Hono<App>()
 			if (!(await canReadSaves(c, room, roomId, accountId))) return c.body(null, 403)

 			const save = await getSubRoomSaveById(c.env.DB, subRoomId, saveId)
-			return save ? c.json(toSaveResponse(save, unityAssetTargetQuery(c))) : c.notFound()
+			return save ? c.json(toSaveDetail(save, unityAssetTargetQuery(c))) : c.notFound()
 		}
 	)

@@ -5697,28 +5697,65 @@ describe('rooms endpoints', () => {

 		// Pick a real save off the history the previous test paged.
 		const list = (await (await get('/rooms/2/subrooms/2/saves', '1')).json()) as {
-			Results: Array<{ SubRoomDataSaveId: number; DataBlob: string; Description: string }>
+			Results: Array<{
+				SubRoomDataSaveId: number
+				DataBlob: string
+				Description: string
+				PersistenceVersion: number
+				UgcSubVersion: number
+			}>
 		}
 		const row = list.Results[0]!

 		const res = await get(`/rooms/2/subrooms/2/saves/${row.SubRoomDataSaveId}`, '1')
 		expect(res.status).toBe(200)
-		// The camelCase projection the room save returns — NOT the PascalCase row the list
-		// serves. Same field set, exactly: no persistence/OM/UGC versions, no asset arrays.
-		expect(await res.json()).toEqual({
-			subRoomDataSaveId: row.SubRoomDataSaveId,
-			subRoomId: 2,
-			unityAssetId: null,
-			unityAsset: null,
-			unityAssetHash: null,
-			dataBlob: row.DataBlob,
-			dataBlobHash: null,
-			savedByAccountId: expect.any(Number),
-			savedOnPlatform: 0,
-			savedOnDeviceClass: 0,
-			description: row.Description,
-			createdAt: expect.any(String),
+		// A PascalCase row like the list serves — NOT the camelCase projection the room save
+		// returns (served that, the client showed a save with every value missing). Its
+		// bundle fields are the detail's own: top-level `UnityAsset`/`UnityAssetHash`, no
+		// `Tags`, and `UnityAssetId` present-and-null rather than omitted.
+		const detailRow = (await res.json()) as Record<string, unknown>
+		expect(Object.keys(detailRow)).toEqual([
+			'UnitySubAssets',
+			'ReferencedUnityAssets',
+			'UnityAsset',
+			'UnityAssetHash',
+			'SubRoomDataSaveId',
+			'SubRoomId',
+			'UnityAssetId',
+			'ReferencedUnityAssetIds',
+			'DataBlob',
+			'DataBlobHash',
+			'PersistenceVersion',
+			'OMVersion',
+			'SavedByAccountId',
+			'SavedOnPlatform',
+			'SavedOnDeviceClass',
+			'Description',
+			'ModerationState',
+			'CreatedAt',
+			'UgcSubVersion',
+		])
+		expect(detailRow).toMatchObject({
+			UnitySubAssets: [],
+			ReferencedUnityAssets: [],
+			UnityAsset: null,
+			UnityAssetHash: null,
+			SubRoomDataSaveId: row.SubRoomDataSaveId,
+			SubRoomId: 2,
+			UnityAssetId: null,
+			ReferencedUnityAssetIds: [],
+			DataBlob: row.DataBlob,
+			DataBlobHash: null,
+			SavedByAccountId: expect.any(Number),
+			SavedOnPlatform: 0,
+			SavedOnDeviceClass: 0,
+			Description: row.Description,
+			ModerationState: 0,
+			CreatedAt: expect.any(String),
 		})
+		// The versions come off the row, not the room save's trimmed projection.
+		expect(detailRow.PersistenceVersion).toBe(row.PersistenceVersion)
+		expect(detailRow.UgcSubVersion).toBe(row.UgcSubVersion)

 		// Unknown save, and a save that exists but belongs to ANOTHER subroom (ids are
 		// global, so an unscoped lookup would happily resolve this one) — both 404.
@@ -6158,15 +6195,28 @@ describe('studio room bundles', () => {
 			{ headers: await bearer('1') }
 		)
 		expect(detail.status).toBe(200)
-		expect(await detail.json()).toMatchObject({ unityAsset: winName, unityAssetHash: winHash })
+		// The detail lifts the Windows bundle to the top level when no target is named, and
+		// names every bundle the client's way — `UnityAsset`/`UnityAssetHash`, not the stored
+		// `Filename`/`Hash`.
+		expect(await detail.json()).toMatchObject({
+			UnityAsset: winName,
+			UnityAssetHash: winHash,
+			UnitySubAssets: [
+				{ UnityAssetId: ASSET, UnityAsset: winName, UnityAssetHash: winHash },
+				{ UnityAssetId: ASSET, UnityAsset: androidName, UnityAssetHash: androidHash },
+			],
+		})

 		const questDetail = await SELF.fetch(
 			`${ORIGIN}/rooms/9901/subrooms/99011/saves/${current.SubRoomDataSaveId}?unityAssetTarget=2`,
 			{ headers: await bearer('1') }
 		)
 		expect(await questDetail.json()).toMatchObject({
-			unityAsset: androidName,
-			unityAssetHash: androidHash,
+			UnityAsset: androidName,
+			UnityAssetHash: androidHash,
+			UnitySubAssets: [
+				{ UnityAssetId: ASSET, UnityAsset: androidName, UnityAssetHash: androidHash },
+			],
 		})

 		const light = await SELF.fetch(`${ORIGIN}/rooms/9901/subrooms/99011/saves/no_unity_assets`, {
@@ -6181,7 +6231,11 @@ describe('studio room bundles', () => {
 			`${ORIGIN}/rooms/9902/subrooms/99021/saves/${makerDetailId}`,
 			{ headers: await bearer('1') }
 		)
-		expect(await makerDetail.json()).toMatchObject({ unityAsset: null, unityAssetHash: null })
+		expect(await makerDetail.json()).toMatchObject({
+			UnityAsset: null,
+			UnityAssetHash: null,
+			UnitySubAssets: [],
+		})
 	})
 })