Skip to content

Commit 3be6d50

Browse files
committed
fix(service-storage): ask the chunk door and the progress door's expiry stamp for the starting organization before writing
The chunk door and the progress door's expiry stamp write the upload session by id under the acting organization; for an uploader who switched organization that scoped write misses and used to answer 500 INTERNAL with the data-engine outage text. Both now ask requireStartingOrganization first and answer 409 RESOURCE_CONFLICT. The progress door asks only when the expiry stamp is due. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent f4bed58 commit 3be6d50

4 files changed

Lines changed: 365 additions & 25 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/service-storage': patch
3+
---
4+
5+
fix(service-storage): sending a chunk, or polling an expired upload's progress, after switching active organization answers `409 RESOURCE_CONFLICT` naming the change, not a `500` that reads as a data-engine outage
6+
7+
Clause-②: no
8+
9+
`PUT /api/v1/storage/upload/chunked/:uploadId/chunk/:chunkIndex` records each chunk on the upload session, and `GET /api/v1/storage/upload/chunked/:uploadId/progress` stamps a session past its `expires_at` as `expired`. Both writes go by id under the caller's active organization, and they are scoped to it. When the uploader's active organization is no longer the one the upload was started in, the write cannot reach the session row. Both doors used to answer that with `500 INTERNAL` and a message diagnosing a data-engine outage ("Restore the data engine …").
10+
11+
- **Now:** each door answers `409 RESOURCE_CONFLICT` in the standard error envelope, with the same message the commit and chunked-completion doors give: the upload was started in a different organization than the active one, and switching back finishes it. The chunk door answers it before the chunk reaches the storage backend and before any write. The progress door answers it only when the expiry stamp is due, before stamping. The server log carries one warning naming the door, the upload, the organization it was started in and the active one.
12+
- **Unchanged:** which organization an upload belongs to (switching back finishes it there); a chunk or a progress read from the starting organization; the progress of a live session, or of one already `expired`, which is a read and still answers `200` from any organization; a real data-engine failure, which still answers `500` with its consequence and fix; the resume-token check on the chunk door and the uploader-only rule on the progress door, which run first, so a caller who cannot act on the upload learns nothing about organizations; sessions stamped with no organization; routes with no session resolver; and a store with no data engine wired.
13+
14+
What changes for you: when either door answers `409 RESOURCE_CONFLICT`, switch the active organization back to the one the upload was started in and retry the same call.

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

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@
1616
* organization an upload belongs to is NOT changed: switching back finishes it,
1717
* from where it started.
1818
*
19+
* #22218 asks the same question at two more doors. The chunk door
20+
* (`PUT …/upload/chunked/:uploadId/chunk/:chunkIndex`) writes the session's
21+
* progress after every chunk, and the progress door
22+
* (`GET …/upload/chunked/:uploadId/progress`) stamps a session past its
23+
* `expires_at` as `expired`. Both writes are scoped the same way and used to
24+
* answer the same `500`. The progress door asks only when that stamp is due: a
25+
* live session's progress is a read, and it still answers the uploader from
26+
* any organization.
27+
*
1928
* Same boot as `storage-upload-door-ownership.wall.test.ts` (and the same
2029
* reason this file lives in this package: the wall is raised by THIS
2130
* package's `OrganizationsPlugin`, which ADR-0132 forbids every other
@@ -88,7 +97,7 @@ describe('the upload doors after the uploader switches organization, on a booted
8897
expect(res.status).toBe(200);
8998
return res.body.data.fileId;
9099
};
91-
const startChunked = async (): Promise<{ uploadId: string; fileId: string }> => {
100+
const startChunked = async (): Promise<{ uploadId: string; fileId: string; resumeToken: string }> => {
92101
const res = await answer(
93102
await stack.apiAs(uploader, 'POST', '/storage/upload/chunked', {
94103
filename: 'org-change-chunked.bin',
@@ -97,7 +106,40 @@ describe('the upload doors after the uploader switches organization, on a booted
97106
}),
98107
);
99108
expect(res.status).toBe(200);
100-
return { uploadId: res.body.data.uploadId, fileId: res.body.data.fileId };
109+
return {
110+
uploadId: res.body.data.uploadId,
111+
fileId: res.body.data.fileId,
112+
resumeToken: res.body.data.resumeToken,
113+
};
114+
};
115+
/** One binary chunk, with the resume token the start door handed back. */
116+
const putChunk = (uploadId: string, resumeToken: string) =>
117+
stack.api(`/storage/upload/chunked/${uploadId}/chunk/0`, {
118+
method: 'PUT',
119+
headers: {
120+
Authorization: `Bearer ${uploader}`,
121+
'Content-Type': 'application/octet-stream',
122+
'x-resume-token': resumeToken,
123+
},
124+
body: new TextEncoder().encode('chunk-0'),
125+
});
126+
const progress = (uploadId: string) =>
127+
stack.apiAs(uploader, 'GET', `/storage/upload/chunked/${uploadId}/progress`);
128+
/**
129+
* Moves a session's own `expires_at` into the past, as SYSTEM and in the
130+
* organization it was started in, so its expiry stamp is due. The row's
131+
* organization is untouched.
132+
*/
133+
const pastDeadline = async (uploadId: string): Promise<void> => {
134+
await ql.update(
135+
'sys_upload_session',
136+
{ expires_at: new Date(Date.now() - 60_000).toISOString() },
137+
{ where: { id: uploadId }, context: { isSystem: true, tenantId: startOrg } },
138+
);
139+
const row = await stored('sys_upload_session', uploadId);
140+
expect(Date.parse(row.expires_at)).toBeLessThan(Date.now());
141+
expect(row.status).toBe('in_progress');
142+
expect(row.organization_id).toBe(startOrg);
101143
};
102144

103145
/**
@@ -251,4 +293,64 @@ describe('the upload doors after the uploader switches organization, on a booted
251293
expect(progress.body.data.status).toBe('in_progress');
252294
await switchTo(startOrg);
253295
});
296+
297+
describe('the chunk door (#22218)', () => {
298+
it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the session is unchanged; switching back lands the chunk', async () => {
299+
await switchTo(startOrg);
300+
const { uploadId, resumeToken } = await startChunked();
301+
const before = await stored('sys_upload_session', uploadId);
302+
303+
await switchTo(otherOrg);
304+
expectOrganizationChanged(await answer(await putChunk(uploadId, resumeToken)), 'chunk after switching');
305+
expect(await stored('sys_upload_session', uploadId)).toEqual(before);
306+
307+
// The upload still belongs where it started, and takes its chunks there.
308+
await switchTo(startOrg);
309+
const home = await answer(await putChunk(uploadId, resumeToken));
310+
expect(home.status, JSON.stringify(home.body)).toBe(200);
311+
const row = await stored('sys_upload_session', uploadId);
312+
expect(row.uploaded_chunks).toBe(1);
313+
expect(row.organization_id).toBe(startOrg);
314+
});
315+
316+
it('CONTROL — SAME ORGANIZATION: the uploader sends a chunk exactly as before', async () => {
317+
await switchTo(startOrg);
318+
const { uploadId, resumeToken } = await startChunked();
319+
const own = await answer(await putChunk(uploadId, resumeToken));
320+
expect(own.status, JSON.stringify(own.body)).toBe(200);
321+
expect(own.body.data.chunkIndex).toBe(0);
322+
expect((await stored('sys_upload_session', uploadId)).uploaded_chunks).toBe(1);
323+
});
324+
});
325+
326+
describe('the progress door, once the expiry stamp is due (#22218)', () => {
327+
it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the session is not stamped; switching back reports it expired', async () => {
328+
await switchTo(startOrg);
329+
const { uploadId } = await startChunked();
330+
await pastDeadline(uploadId);
331+
const before = await stored('sys_upload_session', uploadId);
332+
333+
await switchTo(otherOrg);
334+
expectOrganizationChanged(await answer(await progress(uploadId)), 'progress past the deadline after switching');
335+
expect(await stored('sys_upload_session', uploadId)).toEqual(before);
336+
337+
await switchTo(startOrg);
338+
const home = await answer(await progress(uploadId));
339+
expect(home.status, JSON.stringify(home.body)).toBe(200);
340+
expect(home.body.data.status).toBe('expired');
341+
const row = await stored('sys_upload_session', uploadId);
342+
expect(row.status).toBe('expired');
343+
expect(row.organization_id).toBe(startOrg);
344+
});
345+
346+
it('CONTROL — SAME ORGANIZATION: the uploader is told `expired`, and the session is stamped, exactly as before', async () => {
347+
await switchTo(startOrg);
348+
const { uploadId } = await startChunked();
349+
await pastDeadline(uploadId);
350+
const own = await answer(await progress(uploadId));
351+
expect(own.status, JSON.stringify(own.body)).toBe(200);
352+
expect(own.body.data.status).toBe('expired');
353+
expect((await stored('sys_upload_session', uploadId)).status).toBe('expired');
354+
});
355+
});
254356
});

0 commit comments

Comments
 (0)