fix(service-storage): the upload commit door refuses a file whose bytes the storage backend does not hold

.changeset/24548-complete-holds-bytes.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/service-storage': minor
+---
+
+fix(service-storage): the single-shot upload commit refuses a file whose bytes the storage backend does not hold
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) What narrows is the single-shot upload commit door: a completion naming a file whose bytes the storage backend does not hold, or holds at another size than the upload declared, used to answer 200 and commit the row, and is now refused 409 with the row left pending. Nothing migrates: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, and no stored row is read, rewritten or converted, so there is nothing for `objectstack migrate meta` to rewrite. A refused client finishes the upload by sending its bytes, not by rewriting code or metadata. -->
+
+**What was wrong.** `POST /api/v1/storage/upload/complete` set the `sys_file` row to `committed` without asking the storage backend. A presigned upload whose byte `PUT` failed, or was never sent, therefore committed with the size it declared. The download doors serve any `committed` file, so `GET /api/v1/storage/files/:fileId/url` handed out a signed URL, and fetching it failed with `500`.
+
+**What the commit door does now.** Before it commits, it asks the backend through two members every adapter implements: `exists(key)`, and `getInfo(key).size` compared with the size the upload declared. When the backend holds no bytes for the file, or holds a different number of bytes, the door answers `409` with the standard code `RESOURCE_CONFLICT`. The row stays `pending`, so nothing is served for it. This is the answer the chunked completion already gives an upload that does not hold its declared bytes.
+
+**What changes for you.** Nothing, if your client sends the bytes before it completes, as the SDK's `storage.upload` and the console's upload provider do. A client that completes first now receives the `409`: send the file to its upload URL (start a new upload if that URL has expired), then complete again. If the storage adapter reports a fault while the door asks, the door answers `500`. The local adapter does not report an unreadable storage root as a fault: it reports it as holding no bytes, so on the local adapter the door answers `409`.
+
+The `eTag` a completion may carry is still stored as sent and not compared: no adapter reports the stored object's ETag.
+
+**The local adapter's download link.** `GET /api/v1/storage/_local/raw/:token` answers `404` `FILE_NOT_FOUND` when no file is stored under the link's key. Before, it answered `500` `INTERNAL` ("Download failed"). Every other read failure still answers `500`.

packages/plugins/organizations/src/storage-upload-door-ownership.wall.test.ts

@@ -105,6 +105,8 @@ describe('the upload doors on a booted organization wall (#22046)', () => {
   };
   const stored = (object: string, id: string) => ql.findOne(object, { where: { id } }, SYS);
 
+  /** The URL each presigned upload answered, by file id — where its bytes are sent. */
+  const uploadUrls = new Map<string, string>();
   const presign = async (): Promise<string> => {
     const res = await answer(
       await stack.apiAs(uploader, 'POST', '/storage/upload/presigned', {
@@ -114,8 +116,21 @@ describe('the upload doors on a booted organization wall (#22046)', () => {
       }),
     );
     expect(res.status).toBe(200);
+    uploadUrls.set(res.body.data.fileId, String(res.body.data.uploadUrl));
     return res.body.data.fileId;
   };
+  /**
+   * The upload's declared 5 bytes, PUT to the URL its presign answered — the
+   * commit door commits only a file whose bytes the backend holds (#24548).
+   */
+  const storeTheBytes = async (fileId: string) => {
+    const res = await stack.raw(uploadUrls.get(fileId)!.replace(/^https?:\/\/[^/]+/, ''), {
+      method: 'PUT',
+      headers: { 'Content-Type': 'text/plain' },
+      body: 'plan!',
+    });
+    expect(res.status, await res.clone().text()).toBe(200);
+  };
   const startChunked = async (): Promise<{ uploadId: string; fileId: string; resumeToken: string }> => {
     const res = await answer(
       await stack.apiAs(uploader, 'POST', '/storage/upload/chunked', {
@@ -248,6 +263,7 @@ describe('the upload doors on a booted organization wall (#22046)', () => {
 
     it('POSITIVE: the uploader commits', async () => {
       const fileId = await presign();
+      await storeTheBytes(fileId);
       const own = await answer(await stack.apiAs(uploader, 'POST', '/storage/upload/complete', { fileId, eTag: 'etag-uploader' }));
       expect(own.status).toBe(200);
       expect(own.body.data.fileId).toBe(fileId);

packages/plugins/organizations/src/storage-upload-organization-change.wall.test.ts

@@ -93,6 +93,8 @@ describe('the upload doors after the uploader switches organization, on a booted
   };
   const stored = (object: string, id: string) => ql.findOne(object, { where: { id } }, SYS);
 
+  /** The URL each presigned upload answered, by file id — where its bytes are sent. */
+  const uploadUrls = new Map<string, string>();
   const presign = async (): Promise<string> => {
     const res = await answer(
       await stack.apiAs(uploader, 'POST', '/storage/upload/presigned', {
@@ -102,8 +104,21 @@ describe('the upload doors after the uploader switches organization, on a booted
       }),
     );
     expect(res.status).toBe(200);
+    uploadUrls.set(res.body.data.fileId, String(res.body.data.uploadUrl));
     return res.body.data.fileId;
   };
+  /**
+   * The upload's declared 5 bytes, PUT to the URL its presign answered — the
+   * commit door commits only a file whose bytes the backend holds (#24548).
+   */
+  const storeTheBytes = async (fileId: string) => {
+    const res = await stack.raw(uploadUrls.get(fileId)!.replace(/^https?:\/\/[^/]+/, ''), {
+      method: 'PUT',
+      headers: { 'Content-Type': 'text/plain' },
+      body: 'plan!',
+    });
+    expect(res.status, await res.clone().text()).toBe(200);
+  };
   const startChunked = async (): Promise<{ uploadId: string; fileId: string; resumeToken: string }> => {
     const res = await answer(
       await stack.apiAs(uploader, 'POST', '/storage/upload/chunked', {
@@ -246,6 +261,7 @@ describe('the upload doors after the uploader switches organization, on a booted
     it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the row is unchanged; switching back commits it', async () => {
       await switchTo(startOrg);
       const fileId = await presign();
+      await storeTheBytes(fileId);
       const before = await stored('sys_file', fileId);
 
       await switchTo(otherOrg);
@@ -265,6 +281,7 @@ describe('the upload doors after the uploader switches organization, on a booted
     it('CONTROL — SAME ORGANIZATION: the uploader commits exactly as before', async () => {
       await switchTo(startOrg);
       const fileId = await presign();
+      await storeTheBytes(fileId);
       const own = await answer(await commit(fileId, 'etag-same'));
       expect(own.status, JSON.stringify(own.body)).toBe(200);
       expect(own.body.data.fileId).toBe(fileId);