mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 17:59:23 -04:00
[Files] Add ability to delete files on upload failure (#140716)
* added server-side mocks * added upload tests * added self destruct on abort capability * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
parent
10211c6df7
commit
5c11b65bb6
5 changed files with 161 additions and 2 deletions
|
@ -98,7 +98,7 @@ export type UpdateFileKindHttpEndpoint = HttpApiInterfaceEntryDefinition<
|
|||
|
||||
export type UploadFileKindHttpEndpoint = HttpApiInterfaceEntryDefinition<
|
||||
{ id: string },
|
||||
unknown,
|
||||
{ selfDestructOnAbort?: boolean },
|
||||
{ body: unknown },
|
||||
{
|
||||
ok: true;
|
||||
|
|
29
x-pack/plugins/files/server/mocks.ts
Normal file
29
x-pack/plugins/files/server/mocks.ts
Normal file
|
@ -0,0 +1,29 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||
* 2.0.
|
||||
*/
|
||||
import { KibanaRequest } from '@kbn/core/server';
|
||||
import { DeeplyMockedKeys } from '@kbn/utility-types-jest';
|
||||
import { FileServiceFactory, FileServiceStart } from '.';
|
||||
|
||||
export const createFileServiceMock = (): DeeplyMockedKeys<FileServiceStart> => ({
|
||||
create: jest.fn(),
|
||||
delete: jest.fn(),
|
||||
deleteShareObject: jest.fn(),
|
||||
find: jest.fn(),
|
||||
getById: jest.fn(),
|
||||
getByToken: jest.fn(),
|
||||
getShareObject: jest.fn(),
|
||||
getUsageMetrics: jest.fn(),
|
||||
list: jest.fn(),
|
||||
listShareObjects: jest.fn(),
|
||||
update: jest.fn(),
|
||||
updateShareObject: jest.fn(),
|
||||
});
|
||||
|
||||
export const createFileServiceFactoryMock = (): DeeplyMockedKeys<FileServiceFactory> => ({
|
||||
asInternal: jest.fn(createFileServiceMock),
|
||||
asScoped: jest.fn((_: KibanaRequest) => createFileServiceMock()),
|
||||
});
|
84
x-pack/plugins/files/server/routes/file_kind/upload.test.ts
Normal file
84
x-pack/plugins/files/server/routes/file_kind/upload.test.ts
Normal file
|
@ -0,0 +1,84 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||
* 2.0.
|
||||
*/
|
||||
|
||||
import { Readable } from 'stream';
|
||||
import { httpServerMock } from '@kbn/core/server/mocks';
|
||||
import { DeeplyMockedKeys } from '@kbn/utility-types-jest';
|
||||
import { kibanaResponseFactory } from '@kbn/core-http-router-server-internal';
|
||||
|
||||
import { FileServiceStart } from '../../file_service';
|
||||
|
||||
import { handler } from './upload';
|
||||
import { createFileKindsRequestHandlerContextMock } from '../test_utils';
|
||||
import { FileKindsRequestHandlerContext } from './types';
|
||||
import { File } from '../../file';
|
||||
import { AbortedUploadError } from '../../file/errors';
|
||||
|
||||
const createRequest = httpServerMock.createKibanaRequest;
|
||||
|
||||
describe('upload', () => {
|
||||
let ctx: FileKindsRequestHandlerContext;
|
||||
let fileService: DeeplyMockedKeys<FileServiceStart>;
|
||||
|
||||
let uploadContent: jest.Mock<ReturnType<File['uploadContent']>>;
|
||||
let deleteFn: jest.Mock<ReturnType<File['delete']>>;
|
||||
|
||||
const testErrorMessage = 'stop';
|
||||
const stopFn = async () => {
|
||||
throw new Error(testErrorMessage);
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
({ ctx, fileService } = createFileKindsRequestHandlerContextMock());
|
||||
uploadContent = jest.fn();
|
||||
deleteFn = jest.fn();
|
||||
fileService.getById.mockResolvedValueOnce({
|
||||
id: 'test',
|
||||
data: { size: 1 },
|
||||
uploadContent,
|
||||
delete: deleteFn,
|
||||
} as unknown as File);
|
||||
});
|
||||
|
||||
it('errors as expected', async () => {
|
||||
fileService.getById.mockReset();
|
||||
fileService.getById.mockImplementation(stopFn);
|
||||
const { status, payload } = await handler(
|
||||
ctx,
|
||||
createRequest({
|
||||
params: { id: 'test' },
|
||||
query: { selfDestructOnFailure: true },
|
||||
body: Readable.from(['test']),
|
||||
}),
|
||||
kibanaResponseFactory
|
||||
);
|
||||
expect(status).toBe(500);
|
||||
expect(payload).toEqual({ message: testErrorMessage });
|
||||
expect(deleteFn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('self-destruct on abort', () => {
|
||||
it('deletes a file on failure to upload', async () => {
|
||||
uploadContent.mockImplementationOnce(() => {
|
||||
throw new AbortedUploadError('Request aborted');
|
||||
});
|
||||
|
||||
const { status, payload } = await handler(
|
||||
ctx,
|
||||
createRequest({
|
||||
params: { id: 'test' },
|
||||
query: { selfDestructOnAbort: true },
|
||||
body: Readable.from(['test']),
|
||||
}),
|
||||
kibanaResponseFactory
|
||||
);
|
||||
expect(status).toBe(499);
|
||||
expect(payload).toEqual({ message: 'Request aborted' });
|
||||
expect(deleteFn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
|
@ -21,6 +21,11 @@ export const method = 'put' as const;
|
|||
export const bodySchema = schema.stream();
|
||||
type Body = TypeOf<typeof bodySchema>;
|
||||
|
||||
export const querySchema = schema.object({
|
||||
selfDestructOnAbort: schema.maybe(schema.boolean()),
|
||||
});
|
||||
type Query = Ensure<UploadFileKindHttpEndpoint['inputs']['query'], TypeOf<typeof querySchema>>;
|
||||
|
||||
export const paramsSchema = schema.object({
|
||||
id: schema.string(),
|
||||
});
|
||||
|
@ -28,7 +33,7 @@ type Params = Ensure<UploadFileKindHttpEndpoint['inputs']['params'], TypeOf<type
|
|||
|
||||
type Response = UploadFileKindHttpEndpoint['output'];
|
||||
|
||||
export const handler: FileKindsRequestHandler<Params, unknown, Body> = async (
|
||||
export const handler: FileKindsRequestHandler<Params, Query, Body> = async (
|
||||
{ files, fileKind },
|
||||
req,
|
||||
res
|
||||
|
@ -40,6 +45,7 @@ export const handler: FileKindsRequestHandler<Params, unknown, Body> = async (
|
|||
const sub = req.events.aborted$.subscribe(abort$);
|
||||
|
||||
const { fileService } = await files;
|
||||
const { logger } = fileService;
|
||||
const {
|
||||
body: stream,
|
||||
params: { id },
|
||||
|
@ -57,6 +63,12 @@ export const handler: FileKindsRequestHandler<Params, unknown, Body> = async (
|
|||
} else if (e instanceof fileErrors.AbortedUploadError) {
|
||||
fileService.usageCounter?.('UPLOAD_ERROR_ABORT');
|
||||
fileService.logger.error(e);
|
||||
if (req.query.selfDestructOnAbort) {
|
||||
logger.info(
|
||||
`File (id: ${file.id}) upload aborted. Deleting file due to self-destruct flag.`
|
||||
);
|
||||
file.delete(); // fire and forget
|
||||
}
|
||||
return res.customError({ body: { message: e.message }, statusCode: 499 });
|
||||
}
|
||||
throw e;
|
||||
|
|
34
x-pack/plugins/files/server/routes/test_utils.ts
Normal file
34
x-pack/plugins/files/server/routes/test_utils.ts
Normal file
|
@ -0,0 +1,34 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||
* 2.0.
|
||||
*/
|
||||
|
||||
import { loggingSystemMock } from '@kbn/core/server/mocks';
|
||||
import { createFileServiceMock } from '../mocks';
|
||||
import type { FileKindsRequestHandlerContext } from './file_kind/types';
|
||||
|
||||
export const createFileKindsRequestHandlerContextMock = (
|
||||
fileKind: string = 'test'
|
||||
): {
|
||||
fileService: ReturnType<typeof createFileServiceMock>;
|
||||
ctx: FileKindsRequestHandlerContext;
|
||||
} => {
|
||||
const fileService = createFileServiceMock();
|
||||
const ctx = {
|
||||
fileKind,
|
||||
files: Promise.resolve({
|
||||
fileService: {
|
||||
asCurrentUser: () => fileService,
|
||||
asInternalUser: () => fileService,
|
||||
logger: loggingSystemMock.createLogger(),
|
||||
},
|
||||
}),
|
||||
} as unknown as FileKindsRequestHandlerContext;
|
||||
|
||||
return {
|
||||
ctx,
|
||||
fileService,
|
||||
};
|
||||
};
|
Loading…
Add table
Add a link
Reference in a new issue