Skip to content

Commit

Permalink
Addressing feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-buttner committed Apr 10, 2023
1 parent 770c786 commit ea37ca6
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 21 deletions.
5 changes: 0 additions & 5 deletions x-pack/plugins/cases/common/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,6 @@ export const CONNECTORS_URL = `${ACTION_URL}/connectors` as const;
*/
export const MAX_ALERTS_PER_CASE = 1000 as const;

/**
* Requests
*/
export const MAX_DELETE_CASE_IDS = 100 as const;

/**
* Searching
*/
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/cases/server/client/cases/delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,17 @@ describe('delete', () => {
});

it('returns the number of entities equal to the case ids times the max files per case limit', async () => {
const expectedEntities = Array.from(Array(numCaseIds * MAX_FILES_PER_CASE).keys()).map(
() => ({
id: '123',
owner: 'securitySolution',
})
);

const entities = await getFileEntities(caseIds, mockFileService);

expect(entities.length).toEqual(numCaseIds * MAX_FILES_PER_CASE);
expect(entities).toEqual(expectedEntities);
});
});
});
Expand Down
16 changes: 8 additions & 8 deletions x-pack/plugins/cases/server/client/cases/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
CASE_COMMENT_SAVED_OBJECT,
CASE_SAVED_OBJECT,
CASE_USER_ACTION_SAVED_OBJECT,
MAX_DELETE_CASE_IDS,
MAX_FILES_PER_CASE,
MAX_DOCS_PER_PAGE,
} from '../../../common/constants';
import type { CasesClientArgs } from '..';
Expand Down Expand Up @@ -95,22 +95,22 @@ export const getFileEntities = async (
): Promise<OwnerEntity[]> => {
// using 50 just to be safe, each case can have 100 files = 50 * 100 = 5000 which is half the max number of docs that
// the client can request
const chunkSize = MAX_DELETE_CASE_IDS / 2;
const chunkSize = MAX_FILES_PER_CASE / 2;
const chunkedIds = chunk(caseIds, chunkSize);

const fileResults = await pMap(chunkedIds, async (ids: string[]) => {
const files = await fileService.find({
const entityResults = await pMap(chunkedIds, async (ids: string[]) => {
const findRes = await fileService.find({
perPage: MAX_DOCS_PER_PAGE,
meta: {
caseIds: ids,
},
});

return files;
const fileEntities = createFileEntities(findRes.files);
return fileEntities;
});

const files = fileResults.flatMap((res) => res.files);
const fileEntities = createFileEntities(files);
const entities = entityResults.flatMap((res) => res);

return fileEntities;
return entities;
};
77 changes: 77 additions & 0 deletions x-pack/plugins/cases/server/client/files/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* 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 { createFileServiceMock } from '@kbn/files-plugin/server/mocks';
import { OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER } from '../../../common';
import { constructFileKindIdByOwner } from '../../../common/files';
import { createFileEntities, deleteFiles } from '.';

describe('server files', () => {
describe('createFileEntities', () => {
it('returns an empty array when passed no files', () => {
expect(createFileEntities([])).toEqual([]);
});

it('throws an error when the file kind is not valid', () => {
expect.assertions(1);

expect(() =>
createFileEntities([{ fileKind: 'abc', id: '1' }])
).toThrowErrorMatchingInlineSnapshot(`"File id 1 has invalid file kind abc"`);
});

it('throws an error when one of the file entities does not have a valid file kind', () => {
expect.assertions(1);

expect(() =>
createFileEntities([
{ fileKind: constructFileKindIdByOwner(SECURITY_SOLUTION_OWNER), id: '1' },
{ fileKind: 'abc', id: '2' },
])
).toThrowErrorMatchingInlineSnapshot(`"File id 2 has invalid file kind abc"`);
});

it('returns an array of entities when the file kind is valid', () => {
expect.assertions(1);

expect(
createFileEntities([
{ fileKind: constructFileKindIdByOwner(SECURITY_SOLUTION_OWNER), id: '1' },
{ fileKind: constructFileKindIdByOwner(OBSERVABILITY_OWNER), id: '2' },
])
).toEqual([
{ id: '1', owner: 'securitySolution' },
{ id: '2', owner: 'observability' },
]);
});
});

describe('deleteFiles', () => {
it('calls delete twice with the ids passed in', async () => {
const fileServiceMock = createFileServiceMock();

expect.assertions(2);
await deleteFiles(['1', '2'], fileServiceMock);

expect(fileServiceMock.delete).toBeCalledTimes(2);
expect(fileServiceMock.delete.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"id": "1",
},
],
Array [
Object {
"id": "2",
},
],
]
`);
});
});
});
4 changes: 3 additions & 1 deletion x-pack/plugins/cases/server/client/files/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import { constructOwnerFromFileKind } from '../../../common/files';
import { MAX_CONCURRENT_SEARCHES } from '../../../common/constants';
import type { OwnerEntity } from '../../authorization';

export const createFileEntities = (files: FileJSON[]): OwnerEntity[] => {
type FileEntityInfo = Pick<FileJSON, 'fileKind' | 'id'>;

export const createFileEntities = (files: FileEntityInfo[]): OwnerEntity[] => {
const fileEntities: OwnerEntity[] = [];

// It's possible that the owner array could have invalid information in it so we'll use the file kind for determining if the user
Expand Down
7 changes: 2 additions & 5 deletions x-pack/plugins/cases/server/routes/api/cases/delete_cases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { schema } from '@kbn/config-schema';

import { CASES_URL, MAX_DELETE_CASE_IDS } from '../../../../common/constants';
import { CASES_URL } from '../../../../common/constants';
import { createCaseError } from '../../../common/error';
import { createCasesRoute } from '../create_cases_route';

Expand All @@ -16,10 +16,7 @@ export const deleteCaseRoute = createCasesRoute({
path: CASES_URL,
params: {
query: schema.object({
ids: schema.arrayOf(schema.string({ minLength: 1 }), {
minSize: 1,
maxSize: MAX_DELETE_CASE_IDS,
}),
ids: schema.arrayOf(schema.string()),
}),
},
handler: async ({ context, request, response }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ import { fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';

import { FindQueryParamsRt, throwErrors, excess } from '../../../../common/api';
import { CASE_COMMENTS_URL } from '../../../../common/constants';
import { CASE_FIND_ATTACHMENTS_URL } from '../../../../common/constants';
import { createCasesRoute } from '../create_cases_route';
import { createCaseError } from '../../../common/error';

export const findCommentsRoute = createCasesRoute({
method: 'get',
path: `${CASE_COMMENTS_URL}/_find`,
path: CASE_FIND_ATTACHMENTS_URL,
params: {
params: schema.object({
case_id: schema.string(),
Expand Down

0 comments on commit ea37ca6

Please sign in to comment.