mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-04 08:07:29 +00:00
fix: delete large file selections in batches the server accepts
Deleting files sent every selected id in one FileNode/set. A selection larger than the server's `maxObjectsInSet` was refused as a whole, so nothing was deleted. The ids now go out in batches of that size; a node already removed with its folder by an earlier batch counts as deleted, and a refused item no longer stops the remaining batches.
This commit is contained in:
@@ -8,6 +8,7 @@ vi.mock('../jmap-client', () => ({
|
||||
hasAccountCapability: vi.fn(() => false),
|
||||
getMaxSizeUpload: vi.fn(() => 0),
|
||||
getMaxObjectsInGet: vi.fn(() => 500),
|
||||
getMaxObjectsInSet: vi.fn(() => 500),
|
||||
getMaxCallsInRequest: vi.fn(() => 16),
|
||||
currentSession: null as unknown,
|
||||
},
|
||||
@@ -45,6 +46,7 @@ const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
|
||||
const mockHasCapability = jmapClient.hasCapability as ReturnType<typeof vi.fn>;
|
||||
const mockMaxObjectsInGet = jmapClient.getMaxObjectsInGet as ReturnType<typeof vi.fn>;
|
||||
const mockMaxCallsInRequest = jmapClient.getMaxCallsInRequest as ReturnType<typeof vi.fn>;
|
||||
const mockMaxObjectsInSet = jmapClient.getMaxObjectsInSet as ReturnType<typeof vi.fn>;
|
||||
|
||||
function setSession(session: unknown) {
|
||||
(jmapClient as { currentSession: unknown }).currentSession = session;
|
||||
@@ -53,6 +55,7 @@ function setSession(session: unknown) {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockMaxObjectsInGet.mockReturnValue(500);
|
||||
mockMaxObjectsInSet.mockReturnValue(500);
|
||||
mockMaxCallsInRequest.mockReturnValue(16);
|
||||
mockHasCapability.mockImplementation(
|
||||
(urn: string) =>
|
||||
@@ -125,6 +128,43 @@ describe('deleteFileNodes', () => {
|
||||
|
||||
await expect(deleteFileNodes(['f1'])).rejects.toThrow('forbidden');
|
||||
});
|
||||
|
||||
it("splits a large selection into batches of the server's maxObjectsInSet", async () => {
|
||||
mockMaxObjectsInSet.mockReturnValue(2);
|
||||
const destroyed = (ids: string[]) => ({ methodResponses: [['FileNode/set', { destroyed: ids }, '0']] });
|
||||
mockRequest
|
||||
.mockResolvedValueOnce(destroyed(['a', 'b']))
|
||||
.mockResolvedValueOnce(destroyed(['c', 'd']))
|
||||
.mockResolvedValueOnce(destroyed(['e']));
|
||||
|
||||
await deleteFileNodes(['a', 'b', 'c', 'd', 'e']);
|
||||
|
||||
expect(mockRequest.mock.calls.map((c) => c[0][0][1].destroy)).toEqual([['a', 'b'], ['c', 'd'], ['e']]);
|
||||
expect(mockRequest.mock.calls.every((c) => c[0][0][1].onDestroyRemoveChildren === true)).toBe(true);
|
||||
});
|
||||
|
||||
it('accepts a node that an earlier batch already removed with its folder', async () => {
|
||||
mockMaxObjectsInSet.mockReturnValue(1);
|
||||
mockRequest
|
||||
.mockResolvedValueOnce({ methodResponses: [['FileNode/set', { destroyed: ['folder'] }, '0']] })
|
||||
.mockResolvedValueOnce({
|
||||
methodResponses: [['FileNode/set', { notDestroyed: { child: { type: 'notFound' } } }, '0']],
|
||||
});
|
||||
|
||||
await expect(deleteFileNodes(['folder', 'child'])).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('still deletes the other batches when one item is refused', async () => {
|
||||
mockMaxObjectsInSet.mockReturnValue(1);
|
||||
mockRequest
|
||||
.mockResolvedValueOnce({
|
||||
methodResponses: [['FileNode/set', { notDestroyed: { a: { type: 'forbidden', description: 'forbidden' } } }, '0']],
|
||||
})
|
||||
.mockResolvedValueOnce({ methodResponses: [['FileNode/set', { destroyed: ['b'] }, '0']] });
|
||||
|
||||
await expect(deleteFileNodes(['a', 'b'])).rejects.toThrow('forbidden');
|
||||
expect(mockRequest).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('setFileNodeShare', () => {
|
||||
|
||||
+21
-16
@@ -294,23 +294,28 @@ export async function moveFileNode(id: string, parentId: string | null): Promise
|
||||
export async function deleteFileNodes(ids: string[]): Promise<void> {
|
||||
if (ids.length === 0) return;
|
||||
const accountId = filesAccountId();
|
||||
const res = await jmapClient.request(
|
||||
[['FileNode/set', {
|
||||
accountId,
|
||||
destroy: ids,
|
||||
// The server removes descendant nodes of destroyed folders.
|
||||
onDestroyRemoveChildren: true,
|
||||
}, '0']],
|
||||
fileUsing(),
|
||||
);
|
||||
const notDestroyed = requireMethodResult(res, '0', 'FileNode/set').notDestroyed as
|
||||
| Record<string, { description?: string }>
|
||||
| undefined;
|
||||
const failedIds = Object.keys(notDestroyed ?? {});
|
||||
if (failedIds.length > 0) {
|
||||
throw new Error(
|
||||
notDestroyed![failedIds[0]]?.description || `Failed to delete ${failedIds.length} item(s)`,
|
||||
// One FileNode/set per `maxObjectsInSet` ids: a request over the limit
|
||||
// fails whole (RFC 8620 §3.6.1), so a large selection deleted nothing.
|
||||
// Every batch runs; refusals are reported once all of them are done.
|
||||
const refused: Array<{ description?: string }> = [];
|
||||
for (const slice of batched(ids, jmapClient.getMaxObjectsInSet())) {
|
||||
const res = await jmapClient.request(
|
||||
[['FileNode/set', {
|
||||
accountId,
|
||||
destroy: slice,
|
||||
// The server removes descendant nodes of destroyed folders.
|
||||
onDestroyRemoveChildren: true,
|
||||
}, '0']],
|
||||
fileUsing(),
|
||||
);
|
||||
const notDestroyed = requireMethodResult(res, '0', 'FileNode/set').notDestroyed as
|
||||
| Record<string, { type?: string; description?: string }>
|
||||
| undefined;
|
||||
// A node inside a folder an earlier batch removed is already gone.
|
||||
refused.push(...Object.values(notDestroyed ?? {}).filter((err) => err?.type !== 'notFound'));
|
||||
}
|
||||
if (refused.length > 0) {
|
||||
throw new Error(refused[0]?.description || `Failed to delete ${refused.length} item(s)`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user