From ec64d17f1b61232b00c478624f1e4fe8e18705e1 Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Thu, 6 Aug 2020 16:38:31 -0600 Subject: [PATCH] Adds more end to end tests, fixes some message bugs in the REST routes, and adds some missing unit tests --- .../request/update_list_schema.mock.ts | 27 ++++ .../request/update_list_schema.test.ts | 48 ++++++ .../schemas/response/list_schema.mock.ts | 1 - .../lists/server/routes/patch_list_route.ts | 2 +- .../routes/update_exception_list_route.ts | 2 +- .../lists/server/routes/update_list_route.ts | 2 +- .../security_and_spaces/tests/create_lists.ts | 2 +- .../security_and_spaces/tests/delete_lists.ts | 79 ++++++++++ .../security_and_spaces/tests/find_lists.ts | 73 ++++++++++ .../security_and_spaces/tests/index.ts | 3 + .../security_and_spaces/tests/update_lists.ts | 137 ++++++++++++++++++ x-pack/test/lists_api_integration/utils.ts | 6 +- 12 files changed, 374 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/lists/common/schemas/request/update_list_schema.mock.ts create mode 100644 x-pack/plugins/lists/common/schemas/request/update_list_schema.test.ts create mode 100644 x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts create mode 100644 x-pack/test/lists_api_integration/security_and_spaces/tests/find_lists.ts create mode 100644 x-pack/test/lists_api_integration/security_and_spaces/tests/update_lists.ts diff --git a/x-pack/plugins/lists/common/schemas/request/update_list_schema.mock.ts b/x-pack/plugins/lists/common/schemas/request/update_list_schema.mock.ts new file mode 100644 index 00000000000000..b044d40a5d88f6 --- /dev/null +++ b/x-pack/plugins/lists/common/schemas/request/update_list_schema.mock.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { DESCRIPTION, LIST_ID, META, NAME, _VERSION } from '../../constants.mock'; + +import { UpdateListSchema } from './update_list_schema'; + +export const getUpdateListSchemaMock = (): UpdateListSchema => ({ + _version: _VERSION, + description: DESCRIPTION, + id: LIST_ID, + meta: META, + name: NAME, +}); + +/** + * Useful for end to end tests and other mechanisms which want to fill in the values + * after doing a get of the structure. + */ +export const getUpdateMinimalListSchemaMock = (): UpdateListSchema => ({ + description: DESCRIPTION, + id: LIST_ID, + name: NAME, +}); diff --git a/x-pack/plugins/lists/common/schemas/request/update_list_schema.test.ts b/x-pack/plugins/lists/common/schemas/request/update_list_schema.test.ts new file mode 100644 index 00000000000000..21d20a6b85bce6 --- /dev/null +++ b/x-pack/plugins/lists/common/schemas/request/update_list_schema.test.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { left } from 'fp-ts/lib/Either'; +import { pipe } from 'fp-ts/lib/pipeable'; + +import { exactCheck, foldLeftRight, getPaths } from '../../shared_imports'; + +import { UpdateListSchema, updateListSchema } from './update_list_schema'; +import { getUpdateListSchemaMock } from './update_list_schema.mock'; + +describe('update_list_schema', () => { + test('it should validate a typical list request', () => { + const payload = getUpdateListSchemaMock(); + const decoded = updateListSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + + test('it should accept an undefined for "meta" but strip it out', () => { + const payload = getUpdateListSchemaMock(); + const outputPayload = getUpdateListSchemaMock(); + delete payload.meta; + const decoded = updateListSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + delete outputPayload.meta; + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(outputPayload); + }); + + test('it should not allow an extra key to be sent in', () => { + const payload: UpdateListSchema & { + extraKey?: string; + } = getUpdateListSchemaMock(); + payload.extraKey = 'some new value'; + const decoded = updateListSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + expect(getPaths(left(message.errors))).toEqual(['invalid keys "extraKey"']); + expect(message.schema).toEqual({}); + }); +}); diff --git a/x-pack/plugins/lists/common/schemas/response/list_schema.mock.ts b/x-pack/plugins/lists/common/schemas/response/list_schema.mock.ts index 538ada8c6f51a3..4ae77e12a8294a 100644 --- a/x-pack/plugins/lists/common/schemas/response/list_schema.mock.ts +++ b/x-pack/plugins/lists/common/schemas/response/list_schema.mock.ts @@ -42,7 +42,6 @@ export const getListResponseMock = (): ListSchema => ({ * such as created_at, updated_at, and id. */ export const getListResponseMockWithoutAutoGeneratedValues = (): Partial => ({ - _version: 'WzAsMV0=', created_by: ELASTIC_USER, description: DESCRIPTION, immutable: IMMUTABLE, diff --git a/x-pack/plugins/lists/server/routes/patch_list_route.ts b/x-pack/plugins/lists/server/routes/patch_list_route.ts index e33d8d7c9c5986..763f3f495ca177 100644 --- a/x-pack/plugins/lists/server/routes/patch_list_route.ts +++ b/x-pack/plugins/lists/server/routes/patch_list_route.ts @@ -32,7 +32,7 @@ export const patchListRoute = (router: IRouter): void => { const list = await lists.updateList({ _version, description, id, meta, name, version }); if (list == null) { return siemResponse.error({ - body: `list id: "${id}" found found`, + body: `list id: "${id}" not found`, statusCode: 404, }); } else { diff --git a/x-pack/plugins/lists/server/routes/update_exception_list_route.ts b/x-pack/plugins/lists/server/routes/update_exception_list_route.ts index bead10802df4f2..8102210b8430d7 100644 --- a/x-pack/plugins/lists/server/routes/update_exception_list_route.ts +++ b/x-pack/plugins/lists/server/routes/update_exception_list_route.ts @@ -69,7 +69,7 @@ export const updateExceptionListRoute = (router: IRouter): void => { }); if (list == null) { return siemResponse.error({ - body: `exception list id: "${id}" found found`, + body: `exception list id: "${id}" not found`, statusCode: 404, }); } else { diff --git a/x-pack/plugins/lists/server/routes/update_list_route.ts b/x-pack/plugins/lists/server/routes/update_list_route.ts index 816ad13d3770ec..8d7d08be4130b6 100644 --- a/x-pack/plugins/lists/server/routes/update_list_route.ts +++ b/x-pack/plugins/lists/server/routes/update_list_route.ts @@ -32,7 +32,7 @@ export const updateListRoute = (router: IRouter): void => { const list = await lists.updateList({ _version, description, id, meta, name, version }); if (list == null) { return siemResponse.error({ - body: `list id: "${id}" found found`, + body: `list id: "${id}" not found`, statusCode: 404, }); } else { diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/create_lists.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/create_lists.ts index 44b587f567fdbd..99766ce20c0672 100644 --- a/x-pack/test/lists_api_integration/security_and_spaces/tests/create_lists.ts +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/create_lists.ts @@ -22,7 +22,7 @@ export default ({ getService }: FtrProviderContext) => { describe('create_lists', () => { describe('validation errors', () => { - it('should give an error that the index must exist first if it does not exist before creating a rule', async () => { + it('should give an error that the index must exist first if it does not exist before creating a list', async () => { const { body } = await supertest .post(LIST_URL) .set('kbn-xsrf', 'true') diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts new file mode 100644 index 00000000000000..a5a25cf4bd0431 --- /dev/null +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts @@ -0,0 +1,79 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; + +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { LIST_URL } from '../../../../plugins/lists/common/constants'; + +import { getCreateMinimalListSchemaMock } from '../../../../plugins/lists/common/schemas/request/create_list_schema.mock'; +import { createListsIndex, deleteListsIndex, removeServerGeneratedProperties } from '../../utils'; +import { getListResponseMockWithoutAutoGeneratedValues } from '../../../../plugins/lists/common/schemas/response/list_schema.mock'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const supertest = getService('supertest'); + + describe('delete_lists', () => { + describe('deleting lists', () => { + beforeEach(async () => { + await createListsIndex(supertest); + }); + + afterEach(async () => { + await deleteListsIndex(supertest); + }); + + it('should delete a single list with a list id', async () => { + // create a list + await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send(getCreateMinimalListSchemaMock()) + .expect(200); + + // delete the list by its list id + const { body } = await supertest + .delete(`${LIST_URL}?id=${getCreateMinimalListSchemaMock().id}`) + .set('kbn-xsrf', 'true') + .expect(200); + + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(getListResponseMockWithoutAutoGeneratedValues()); + }); + + it('should delete a single list using an auto generated id', async () => { + // add a list + const { body: bodyWithCreatedList } = await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send(getCreateMinimalListSchemaMock()) + .expect(200); + + // delete that list by its auto-generated id + const { body } = await supertest + .delete(`${LIST_URL}?id=${bodyWithCreatedList.id}`) + .set('kbn-xsrf', 'true') + .expect(200); + + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(getListResponseMockWithoutAutoGeneratedValues()); + }); + + it('should return an error if the id does not exist when trying to delete it', async () => { + const { body } = await supertest + .delete(`${LIST_URL}?id=c1e1b359-7ac1-4e96-bc81-c683c092436f`) + .set('kbn-xsrf', 'true') + .expect(404); + + expect(body).to.eql({ + message: 'list id: "c1e1b359-7ac1-4e96-bc81-c683c092436f" was not found', + status_code: 404, + }); + }); + }); + }); +}; diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/find_lists.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/find_lists.ts new file mode 100644 index 00000000000000..efd240a89b8ae8 --- /dev/null +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/find_lists.ts @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; + +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { LIST_URL } from '../../../../plugins/lists/common/constants'; + +import { getCreateMinimalListSchemaMock } from '../../../../plugins/lists/common/schemas/request/create_list_schema.mock'; +import { createListsIndex, deleteListsIndex, removeServerGeneratedProperties } from '../../utils'; +import { getListResponseMockWithoutAutoGeneratedValues } from '../../../../plugins/lists/common/schemas/response/list_schema.mock'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext): void => { + const supertest = getService('supertest'); + + describe('find_lists', () => { + describe('find lists', () => { + beforeEach(async () => { + await createListsIndex(supertest); + }); + + afterEach(async () => { + await deleteListsIndex(supertest); + }); + + it('should return an empty find body correctly if no lists are loaded', async () => { + const { body } = await supertest + .get(`${LIST_URL}/_find`) + .set('kbn-xsrf', 'true') + .send() + .expect(200); + + expect(body).to.eql({ + cursor: 'WzBd', + data: [], + page: 1, + per_page: 20, + total: 0, + }); + }); + + it('should return a single list when a single list is loaded from a find with defaults added', async () => { + // add a single list + await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send(getCreateMinimalListSchemaMock()) + .expect(200); + + // query the single list from _find + const { body } = await supertest + .get(`${LIST_URL}/_find`) + .set('kbn-xsrf', 'true') + .send() + .expect(200); + + body.data = [removeServerGeneratedProperties(body.data[0])]; + // cursor is a constant changing value so we have to delete it as well. + delete body.cursor; + expect(body).to.eql({ + data: [getListResponseMockWithoutAutoGeneratedValues()], + page: 1, + per_page: 20, + total: 1, + }); + }); + }); + }); +}; diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts index b2cb89df39d3ea..f380b2a860da76 100644 --- a/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts @@ -13,5 +13,8 @@ export default ({ loadTestFile }: FtrProviderContext): void => { loadTestFile(require.resolve('./create_lists')); loadTestFile(require.resolve('./read_lists')); + loadTestFile(require.resolve('./update_lists')); + loadTestFile(require.resolve('./delete_lists')); + loadTestFile(require.resolve('./find_lists')); }); }; diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/update_lists.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/update_lists.ts new file mode 100644 index 00000000000000..242705731d19d8 --- /dev/null +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/update_lists.ts @@ -0,0 +1,137 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; + +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { LIST_URL } from '../../../../plugins/lists/common/constants'; + +import { getCreateMinimalListSchemaMock } from '../../../../plugins/lists/common/schemas/request/create_list_schema.mock'; +import { createListsIndex, deleteListsIndex, removeServerGeneratedProperties } from '../../utils'; +import { getListResponseMockWithoutAutoGeneratedValues } from '../../../../plugins/lists/common/schemas/response/list_schema.mock'; +import { getUpdateMinimalListSchemaMock } from '../../../../plugins/lists/common/schemas/request/update_list_schema.mock'; +import { UpdateListSchema, ListSchema } from '../../../../plugins/lists/common/schemas'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const supertest = getService('supertest'); + + describe('update_lists', () => { + describe('update lists', () => { + beforeEach(async () => { + await createListsIndex(supertest); + }); + + afterEach(async () => { + await deleteListsIndex(supertest); + }); + + it('should update a single list property of name using an id', async () => { + // create a simple list + await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send(getCreateMinimalListSchemaMock()) + .expect(200); + + // update a simple list's name + const updatedList: UpdateListSchema = { + ...getUpdateMinimalListSchemaMock(), + name: 'some other name', + }; + + const { body } = await supertest + .put(LIST_URL) + .set('kbn-xsrf', 'true') + .send(updatedList) + .expect(200); + + const outputList: Partial = { + ...getListResponseMockWithoutAutoGeneratedValues(), + name: 'some other name', + version: 2, + }; + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(outputList); + }); + + it('should update a single list property of name using an auto-generated id', async () => { + const { id, ...listNoId } = getCreateMinimalListSchemaMock(); + // create a simple list with no id which will use an auto-generated id + const { body: createListBody } = await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send(listNoId) + .expect(200); + + // update a simple list's name + const updatedList: UpdateListSchema = { + ...getUpdateMinimalListSchemaMock(), + id: createListBody.id, + name: 'some other name', + }; + const { body } = await supertest + .put(LIST_URL) + .set('kbn-xsrf', 'true') + .send(updatedList) + .expect(200); + + const outputList: Partial = { + ...getListResponseMockWithoutAutoGeneratedValues(), + name: 'some other name', + version: 2, + }; + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(outputList); + }); + + it('should change the version of a list when it updates a property', async () => { + // create a simple list + await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send(getCreateMinimalListSchemaMock()) + .expect(200); + + // update a simple list property of name and description + const updatedList: UpdateListSchema = { + ...getUpdateMinimalListSchemaMock(), + name: 'some other name', + description: 'some other description', + }; + + const { body } = await supertest.put(LIST_URL).set('kbn-xsrf', 'true').send(updatedList); + + const outputList: Partial = { + ...getListResponseMockWithoutAutoGeneratedValues(), + name: 'some other name', + description: 'some other description', + version: 2, + }; + + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(outputList); + }); + + it('should give a 404 if it is given a fake id', async () => { + const simpleList: UpdateListSchema = { + ...getUpdateMinimalListSchemaMock(), + id: '5096dec6-b6b9-4d8d-8f93-6c2602079d9d', + }; + const { body } = await supertest + .put(LIST_URL) + .set('kbn-xsrf', 'true') + .send(simpleList) + .expect(404); + + expect(body).to.eql({ + status_code: 404, + message: 'list id: "5096dec6-b6b9-4d8d-8f93-6c2602079d9d" not found', + }); + }); + }); + }); +}; diff --git a/x-pack/test/lists_api_integration/utils.ts b/x-pack/test/lists_api_integration/utils.ts index 5d1d0e5b2c408c..bcadd894bd8001 100644 --- a/x-pack/test/lists_api_integration/utils.ts +++ b/x-pack/test/lists_api_integration/utils.ts @@ -60,10 +60,10 @@ export const deleteListsIndex = async ( /** * This will remove server generated properties such as date times, etc... - * @param rule Rule to pass in to remove typical server generated properties + * @param list List to pass in to remove typical server generated properties */ -export const removeServerGeneratedProperties = (rule: Partial): Partial => { +export const removeServerGeneratedProperties = (list: Partial): Partial => { /* eslint-disable-next-line @typescript-eslint/naming-convention */ - const { created_at, updated_at, id, tie_breaker_id, ...removedProperties } = rule; + const { created_at, updated_at, id, tie_breaker_id, _version, ...removedProperties } = list; return removedProperties; };