From 4ff11d6f6ee17a382dc26ca5993300ca20dc2e95 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 3 Nov 2021 07:12:58 +0100 Subject: [PATCH 01/22] migrate scoped_client_provider tests --- .../scoped_client_provider.test.js.snap | 3 -- ...test.js => scoped_client_provider.test.ts} | 30 ++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) delete mode 100644 src/core/server/saved_objects/service/lib/__snapshots__/scoped_client_provider.test.js.snap rename src/core/server/saved_objects/service/lib/{scoped_client_provider.test.js => scoped_client_provider.test.ts} (88%) diff --git a/src/core/server/saved_objects/service/lib/__snapshots__/scoped_client_provider.test.js.snap b/src/core/server/saved_objects/service/lib/__snapshots__/scoped_client_provider.test.js.snap deleted file mode 100644 index de53d6ca69bd3..0000000000000 --- a/src/core/server/saved_objects/service/lib/__snapshots__/scoped_client_provider.test.js.snap +++ /dev/null @@ -1,3 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`throws error when more than one scoped saved objects client factory is set 1`] = `"custom client factory is already set, unable to replace the current one"`; diff --git a/src/core/server/saved_objects/service/lib/scoped_client_provider.test.js b/src/core/server/saved_objects/service/lib/scoped_client_provider.test.ts similarity index 88% rename from src/core/server/saved_objects/service/lib/scoped_client_provider.test.js rename to src/core/server/saved_objects/service/lib/scoped_client_provider.test.ts index 506c354d5adf0..f8a81e48bbbc3 100644 --- a/src/core/server/saved_objects/service/lib/scoped_client_provider.test.js +++ b/src/core/server/saved_objects/service/lib/scoped_client_provider.test.ts @@ -7,12 +7,13 @@ */ import { SavedObjectsClientProvider } from './scoped_client_provider'; +import { httpServerMock } from '../../../http/http_server.mocks'; import { typeRegistryMock } from '../../saved_objects_type_registry.mock'; test(`uses default client factory when one isn't set`, () => { const returnValue = Symbol(); const defaultClientFactoryMock = jest.fn().mockReturnValue(returnValue); - const request = Symbol(); + const request = httpServerMock.createKibanaRequest(); const clientProvider = new SavedObjectsClientProvider({ defaultClientFactory: defaultClientFactoryMock, @@ -29,7 +30,7 @@ test(`uses default client factory when one isn't set`, () => { test(`uses custom client factory when one is set`, () => { const defaultClientFactoryMock = jest.fn(); - const request = Symbol(); + const request = httpServerMock.createKibanaRequest(); const returnValue = Symbol(); const customClientFactoryMock = jest.fn().mockReturnValue(returnValue); @@ -49,11 +50,20 @@ test(`uses custom client factory when one is set`, () => { }); test(`throws error when more than one scoped saved objects client factory is set`, () => { - const clientProvider = new SavedObjectsClientProvider({}); - clientProvider.setClientFactory(() => {}); + const defaultClientFactory = jest.fn(); + const clientFactory = jest.fn(); + + const clientProvider = new SavedObjectsClientProvider({ + defaultClientFactory, + typeRegistry: typeRegistryMock.create(), + }); + + clientProvider.setClientFactory(clientFactory); expect(() => { - clientProvider.setClientFactory(() => {}); - }).toThrowErrorMatchingSnapshot(); + clientProvider.setClientFactory(clientFactory); + }).toThrowErrorMatchingInlineSnapshot( + `"custom client factory is already set, unable to replace the current one"` + ); }); test(`throws error when registering a wrapper with a duplicate id`, () => { @@ -83,7 +93,7 @@ test(`invokes and uses wrappers in specified order`, () => { const firstClientWrapperFactoryMock = jest.fn().mockReturnValue(firstWrappedClient); const secondWrapperClient = Symbol('second client'); const secondClientWrapperFactoryMock = jest.fn().mockReturnValue(secondWrapperClient); - const request = Symbol(); + const request = httpServerMock.createKibanaRequest(); clientProvider.addClientWrapperFactory(1, 'foo', secondClientWrapperFactoryMock); clientProvider.addClientWrapperFactory(0, 'bar', firstClientWrapperFactoryMock); @@ -114,7 +124,7 @@ test(`does not invoke or use excluded wrappers`, () => { const firstClientWrapperFactoryMock = jest.fn().mockReturnValue(firstWrappedClient); const secondWrapperClient = Symbol('second client'); const secondClientWrapperFactoryMock = jest.fn().mockReturnValue(secondWrapperClient); - const request = Symbol(); + const request = httpServerMock.createKibanaRequest(); clientProvider.addClientWrapperFactory(1, 'foo', secondClientWrapperFactoryMock); clientProvider.addClientWrapperFactory(0, 'bar', firstClientWrapperFactoryMock); @@ -143,7 +153,7 @@ test(`allows all wrappers to be excluded`, () => { const firstClientWrapperFactoryMock = jest.fn().mockReturnValue(firstWrappedClient); const secondWrapperClient = Symbol('second client'); const secondClientWrapperFactoryMock = jest.fn().mockReturnValue(secondWrapperClient); - const request = Symbol(); + const request = httpServerMock.createKibanaRequest(); clientProvider.addClientWrapperFactory(1, 'foo', secondClientWrapperFactoryMock); clientProvider.addClientWrapperFactory(0, 'bar', firstClientWrapperFactoryMock); @@ -164,7 +174,7 @@ test(`allows hidden typed to be included`, () => { defaultClientFactory: defaultClientFactoryMock, typeRegistry: typeRegistryMock.create(), }); - const request = Symbol(); + const request = httpServerMock.createKibanaRequest(); const actualClient = clientProvider.getClient(request, { includedHiddenTypes: ['task'], From 441e6d1a13f7df0b35344656e768d92e449bcfde Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 3 Nov 2021 08:12:02 +0100 Subject: [PATCH 02/22] migrate saved_objects_client to ts --- .../service/saved_objects_client.test.js | 290 ----------------- .../service/saved_objects_client.test.ts | 298 ++++++++++++++++++ 2 files changed, 298 insertions(+), 290 deletions(-) delete mode 100644 src/core/server/saved_objects/service/saved_objects_client.test.js create mode 100644 src/core/server/saved_objects/service/saved_objects_client.test.ts diff --git a/src/core/server/saved_objects/service/saved_objects_client.test.js b/src/core/server/saved_objects/service/saved_objects_client.test.js deleted file mode 100644 index 736e6e06da905..0000000000000 --- a/src/core/server/saved_objects/service/saved_objects_client.test.js +++ /dev/null @@ -1,290 +0,0 @@ -/* - * 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 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { SavedObjectsClient } from './saved_objects_client'; - -test(`#create`, async () => { - const returnValue = Symbol(); - const mockRepository = { - create: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const attributes = Symbol(); - const options = Symbol(); - const result = await client.create(type, attributes, options); - - expect(mockRepository.create).toHaveBeenCalledWith(type, attributes, options); - expect(result).toBe(returnValue); -}); - -test(`#checkConflicts`, async () => { - const returnValue = Symbol(); - const mockRepository = { - checkConflicts: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const objects = Symbol(); - const options = Symbol(); - const result = await client.checkConflicts(objects, options); - - expect(mockRepository.checkConflicts).toHaveBeenCalledWith(objects, options); - expect(result).toBe(returnValue); -}); - -test(`#bulkCreate`, async () => { - const returnValue = Symbol(); - const mockRepository = { - bulkCreate: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const objects = Symbol(); - const options = Symbol(); - const result = await client.bulkCreate(objects, options); - - expect(mockRepository.bulkCreate).toHaveBeenCalledWith(objects, options); - expect(result).toBe(returnValue); -}); - -describe(`#createPointInTimeFinder`, () => { - test(`calls repository with options and default dependencies`, () => { - const returnValue = Symbol(); - const mockRepository = { - createPointInTimeFinder: jest.fn().mockReturnValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const options = Symbol(); - const result = client.createPointInTimeFinder(options); - - expect(mockRepository.createPointInTimeFinder).toHaveBeenCalledWith(options, { - client, - }); - expect(result).toBe(returnValue); - }); - - test(`calls repository with options and custom dependencies`, () => { - const returnValue = Symbol(); - const mockRepository = { - createPointInTimeFinder: jest.fn().mockReturnValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const options = Symbol(); - const dependencies = { - client: { - find: Symbol(), - openPointInTimeForType: Symbol(), - closePointInTime: Symbol(), - }, - }; - const result = client.createPointInTimeFinder(options, dependencies); - - expect(mockRepository.createPointInTimeFinder).toHaveBeenCalledWith(options, dependencies); - expect(result).toBe(returnValue); - }); -}); - -test(`#delete`, async () => { - const returnValue = Symbol(); - const mockRepository = { - delete: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const id = Symbol(); - const options = Symbol(); - const result = await client.delete(type, id, options); - - expect(mockRepository.delete).toHaveBeenCalledWith(type, id, options); - expect(result).toBe(returnValue); -}); - -test(`#find`, async () => { - const returnValue = Symbol(); - const mockRepository = { - find: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const options = Symbol(); - const result = await client.find(options); - - expect(mockRepository.find).toHaveBeenCalledWith(options); - expect(result).toBe(returnValue); -}); - -test(`#bulkGet`, async () => { - const returnValue = Symbol(); - const mockRepository = { - bulkGet: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const objects = Symbol(); - const options = Symbol(); - const result = await client.bulkGet(objects, options); - - expect(mockRepository.bulkGet).toHaveBeenCalledWith(objects, options); - expect(result).toBe(returnValue); -}); - -test(`#get`, async () => { - const returnValue = Symbol(); - const mockRepository = { - get: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const id = Symbol(); - const options = Symbol(); - const result = await client.get(type, id, options); - - expect(mockRepository.get).toHaveBeenCalledWith(type, id, options); - expect(result).toBe(returnValue); -}); - -test(`#openPointInTimeForType`, async () => { - const returnValue = Symbol(); - const mockRepository = { - openPointInTimeForType: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const options = Symbol(); - const result = await client.openPointInTimeForType(type, options); - - expect(mockRepository.openPointInTimeForType).toHaveBeenCalledWith(type, options); - expect(result).toBe(returnValue); -}); - -test(`#closePointInTime`, async () => { - const returnValue = Symbol(); - const mockRepository = { - closePointInTime: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const id = Symbol(); - const options = Symbol(); - const result = await client.closePointInTime(id, options); - - expect(mockRepository.closePointInTime).toHaveBeenCalledWith(id, options); - expect(result).toBe(returnValue); -}); - -test(`#bulkResolve`, async () => { - const returnValue = Symbol(); - const mockRepository = { - bulkResolve: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const objects = Symbol(); - const options = Symbol(); - const result = await client.bulkResolve(objects, options); - - expect(mockRepository.bulkResolve).toHaveBeenCalledWith(objects, options); - expect(result).toBe(returnValue); -}); - -test(`#resolve`, async () => { - const returnValue = Symbol(); - const mockRepository = { - resolve: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const id = Symbol(); - const options = Symbol(); - const result = await client.resolve(type, id, options); - - expect(mockRepository.resolve).toHaveBeenCalledWith(type, id, options); - expect(result).toBe(returnValue); -}); - -test(`#update`, async () => { - const returnValue = Symbol(); - const mockRepository = { - update: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const id = Symbol(); - const attributes = Symbol(); - const options = Symbol(); - const result = await client.update(type, id, attributes, options); - - expect(mockRepository.update).toHaveBeenCalledWith(type, id, attributes, options); - expect(result).toBe(returnValue); -}); - -test(`#bulkUpdate`, async () => { - const returnValue = Symbol(); - const mockRepository = { - bulkUpdate: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const type = Symbol(); - const id = Symbol(); - const attributes = Symbol(); - const version = Symbol(); - const namespace = Symbol(); - const result = await client.bulkUpdate([{ type, id, attributes, version }], { namespace }); - - expect(mockRepository.bulkUpdate).toHaveBeenCalledWith([{ type, id, attributes, version }], { - namespace, - }); - expect(result).toBe(returnValue); -}); - -test(`#collectMultiNamespaceReferences`, async () => { - const returnValue = Symbol(); - const mockRepository = { - collectMultiNamespaceReferences: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const objects = Symbol(); - const options = Symbol(); - const result = await client.collectMultiNamespaceReferences(objects, options); - - expect(mockRepository.collectMultiNamespaceReferences).toHaveBeenCalledWith(objects, options); - expect(result).toBe(returnValue); -}); - -test(`#updateObjectsSpaces`, async () => { - const returnValue = Symbol(); - const mockRepository = { - updateObjectsSpaces: jest.fn().mockResolvedValue(returnValue), - }; - const client = new SavedObjectsClient(mockRepository); - - const objects = Symbol(); - const spacesToAdd = Symbol(); - const spacesToRemove = Symbol(); - const options = Symbol(); - const result = await client.updateObjectsSpaces(objects, spacesToAdd, spacesToRemove, options); - - expect(mockRepository.updateObjectsSpaces).toHaveBeenCalledWith( - objects, - spacesToAdd, - spacesToRemove, - options - ); - expect(result).toBe(returnValue); -}); diff --git a/src/core/server/saved_objects/service/saved_objects_client.test.ts b/src/core/server/saved_objects/service/saved_objects_client.test.ts new file mode 100644 index 0000000000000..52d5e5c161c9d --- /dev/null +++ b/src/core/server/saved_objects/service/saved_objects_client.test.ts @@ -0,0 +1,298 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { + SavedObjectsBulkCreateObject, + SavedObjectsBulkGetObject, + SavedObjectsBulkResolveObject, + SavedObjectsCheckConflictsObject, + SavedObjectsClient, + SavedObjectsClosePointInTimeOptions, + SavedObjectsCreateOptions, + SavedObjectsDeleteOptions, + SavedObjectsOpenPointInTimeOptions, + SavedObjectsUpdateOptions, +} from './saved_objects_client'; +import { savedObjectsRepositoryMock } from './lib/repository.mock'; +import { savedObjectsClientMock } from './saved_objects_client.mock'; +import { + SavedObjectsBaseOptions, + SavedObjectsCollectMultiNamespaceReferencesObject, + SavedObjectsCollectMultiNamespaceReferencesOptions, + SavedObjectsCreatePointInTimeFinderOptions, + SavedObjectsFindOptions, + SavedObjectsUpdateObjectsSpacesObject, + SavedObjectsUpdateObjectsSpacesOptions, +} from 'kibana/server'; + +describe('', () => { + let mockRepository: ReturnType; + + beforeEach(() => { + mockRepository = savedObjectsRepositoryMock.create(); + }); + + test(`#create`, async () => { + const returnValue = Symbol() as any; + mockRepository.create.mockResolvedValueOnce(returnValue); + const client = new SavedObjectsClient(mockRepository); + + const type = 'foo'; + const attributes = { string: 'str', number: 12 }; + const options: SavedObjectsCreateOptions = { id: 'id', namespace: 'bar' }; + const result = await client.create(type, attributes, options); + + expect(mockRepository.create).toHaveBeenCalledWith(type, attributes, options); + expect(result).toBe(returnValue); + }); + + test(`#checkConflicts`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const objects: SavedObjectsCheckConflictsObject[] = [ + { id: '1', type: 'foo' }, + { id: '2', type: 'bar' }, + ]; + const options: SavedObjectsBaseOptions = { namespace: 'ns-1' }; + const result = await client.checkConflicts(objects, options); + + expect(mockRepository.checkConflicts).toHaveBeenCalledWith(objects, options); + expect(result).toBe(returnValue); + }); + + test(`#bulkCreate`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const objects: SavedObjectsBulkCreateObject[] = [ + { type: 'foo', attributes: { hello: 'dolly' } }, + { type: 'bar', attributes: { answer: 42 } }, + ]; + const options: SavedObjectsCreateOptions = { namespace: 'new-ns', refresh: true }; + const result = await client.bulkCreate(objects, options); + + expect(mockRepository.bulkCreate).toHaveBeenCalledWith(objects, options); + expect(result).toBe(returnValue); + }); + + describe(`#createPointInTimeFinder`, () => { + test(`calls repository with options and default dependencies`, () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const options: SavedObjectsCreatePointInTimeFinderOptions = { + perPage: 50, + search: 'candy', + type: 'foo', + }; + const result = client.createPointInTimeFinder(options); + + expect(mockRepository.createPointInTimeFinder).toHaveBeenCalledWith(options, { + client, + }); + expect(result).toBe(returnValue); + }); + + test(`calls repository with options and custom dependencies`, () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const options: SavedObjectsCreatePointInTimeFinderOptions = { + perPage: 50, + search: 'candy', + type: 'foo', + }; + const dependencies = { + client: savedObjectsClientMock.create(), + }; + const result = client.createPointInTimeFinder(options, dependencies); + + expect(mockRepository.createPointInTimeFinder).toHaveBeenCalledWith(options, dependencies); + expect(result).toBe(returnValue); + }); + }); + + test(`#delete`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const type = 'foo'; + const id = '12'; + const options: SavedObjectsDeleteOptions = { force: true, refresh: false }; + const result = await client.delete(type, id, options); + + expect(mockRepository.delete).toHaveBeenCalledWith(type, id, options); + expect(result).toBe(returnValue); + }); + + test(`#find`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const options: SavedObjectsFindOptions = { search: 'something', type: ['a', 'b'], perPage: 42 }; + const result = await client.find(options); + + expect(mockRepository.find).toHaveBeenCalledWith(options); + expect(result).toBe(returnValue); + }); + + test(`#bulkGet`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const objects: SavedObjectsBulkGetObject[] = [ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + ]; + const options: SavedObjectsBaseOptions = { namespace: 'ns-1' }; + const result = await client.bulkGet(objects, options); + + expect(mockRepository.bulkGet).toHaveBeenCalledWith(objects, options); + expect(result).toBe(returnValue); + }); + + test(`#get`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const type = 'foo'; + const id = '12'; + const options: SavedObjectsBaseOptions = { namespace: 'ns-1' }; + const result = await client.get(type, id, options); + + expect(mockRepository.get).toHaveBeenCalledWith(type, id, options); + expect(result).toBe(returnValue); + }); + + test(`#openPointInTimeForType`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const type = 'search'; + const options: SavedObjectsOpenPointInTimeOptions = { + namespaces: ['ns-1', 'ns-2'], + preference: 'pref', + }; + const result = await client.openPointInTimeForType(type, options); + + expect(mockRepository.openPointInTimeForType).toHaveBeenCalledWith(type, options); + expect(result).toBe(returnValue); + }); + + test(`#closePointInTime`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const id = '42'; + const options: SavedObjectsClosePointInTimeOptions = { namespace: 'ns-42' }; + const result = await client.closePointInTime(id, options); + + expect(mockRepository.closePointInTime).toHaveBeenCalledWith(id, options); + expect(result).toBe(returnValue); + }); + + test(`#bulkResolve`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const objects: SavedObjectsBulkResolveObject[] = [ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + ]; + const options: SavedObjectsBaseOptions = { namespace: 'ns-1' }; + const result = await client.bulkResolve(objects, options); + + expect(mockRepository.bulkResolve).toHaveBeenCalledWith(objects, options); + expect(result).toBe(returnValue); + }); + + test(`#resolve`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const type = 'foo'; + const id = '9000'; + const options: SavedObjectsBaseOptions = { namespace: 'ns-3' }; + const result = await client.resolve(type, id, options); + + expect(mockRepository.resolve).toHaveBeenCalledWith(type, id, options); + expect(result).toBe(returnValue); + }); + + test(`#update`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const type = 'some-type'; + const id = '90'; + const attributes = { attr1: 91, attr2: 'some string' }; + const options: SavedObjectsUpdateOptions = { namespace: 'ns-1', version: '12' }; + const result = await client.update(type, id, attributes, options); + + expect(mockRepository.update).toHaveBeenCalledWith(type, id, attributes, options); + expect(result).toBe(returnValue); + }); + + test(`#bulkUpdate`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const type = 'some-type'; + const id = '42'; + const attributes = { attr1: 'value' }; + const version = '12.1'; + const namespace = 'ns-1'; + const result = await client.bulkUpdate([{ type, id, attributes, version }], { namespace }); + + expect(mockRepository.bulkUpdate).toHaveBeenCalledWith([{ type, id, attributes, version }], { + namespace, + }); + expect(result).toBe(returnValue); + }); + + test(`#collectMultiNamespaceReferences`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const objects: SavedObjectsCollectMultiNamespaceReferencesObject[] = [ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + ]; + const options: SavedObjectsCollectMultiNamespaceReferencesOptions = { + namespace: 'ns-1', + purpose: 'collectMultiNamespaceReferences', + }; + const result = await client.collectMultiNamespaceReferences(objects, options); + + expect(mockRepository.collectMultiNamespaceReferences).toHaveBeenCalledWith(objects, options); + expect(result).toBe(returnValue); + }); + + test(`#updateObjectsSpaces`, async () => { + const returnValue = Symbol(); + const client = new SavedObjectsClient(mockRepository); + + const objects: SavedObjectsUpdateObjectsSpacesObject[] = [ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + ]; + const spacesToAdd = ['to-add', 'to-add-2']; + const spacesToRemove = ['to-remove', 'to-remove-2']; + const options: SavedObjectsUpdateObjectsSpacesOptions = { namespace: 'ns-1', refresh: false }; + const result = await client.updateObjectsSpaces(objects, spacesToAdd, spacesToRemove, options); + + expect(mockRepository.updateObjectsSpaces).toHaveBeenCalledWith( + objects, + spacesToAdd, + spacesToRemove, + options + ); + expect(result).toBe(returnValue); + }); +}); From 62f2fc34e0589cc913a454f74f3b4e518f25b204 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 3 Nov 2021 08:21:30 +0100 Subject: [PATCH 03/22] fix return values assertions --- .../service/saved_objects_client.test.ts | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/src/core/server/saved_objects/service/saved_objects_client.test.ts b/src/core/server/saved_objects/service/saved_objects_client.test.ts index 52d5e5c161c9d..3c24df6dcc1a4 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.test.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.test.ts @@ -38,7 +38,7 @@ describe('', () => { }); test(`#create`, async () => { - const returnValue = Symbol() as any; + const returnValue: any = Symbol(); mockRepository.create.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); @@ -52,7 +52,8 @@ describe('', () => { }); test(`#checkConflicts`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.checkConflicts.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const objects: SavedObjectsCheckConflictsObject[] = [ @@ -67,7 +68,8 @@ describe('', () => { }); test(`#bulkCreate`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.bulkCreate.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const objects: SavedObjectsBulkCreateObject[] = [ @@ -83,7 +85,8 @@ describe('', () => { describe(`#createPointInTimeFinder`, () => { test(`calls repository with options and default dependencies`, () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.createPointInTimeFinder.mockReturnValue(returnValue); const client = new SavedObjectsClient(mockRepository); const options: SavedObjectsCreatePointInTimeFinderOptions = { @@ -100,7 +103,8 @@ describe('', () => { }); test(`calls repository with options and custom dependencies`, () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.createPointInTimeFinder.mockReturnValue(returnValue); const client = new SavedObjectsClient(mockRepository); const options: SavedObjectsCreatePointInTimeFinderOptions = { @@ -119,7 +123,8 @@ describe('', () => { }); test(`#delete`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.delete.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const type = 'foo'; @@ -132,7 +137,8 @@ describe('', () => { }); test(`#find`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.find.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const options: SavedObjectsFindOptions = { search: 'something', type: ['a', 'b'], perPage: 42 }; @@ -143,7 +149,8 @@ describe('', () => { }); test(`#bulkGet`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.bulkGet.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const objects: SavedObjectsBulkGetObject[] = [ @@ -158,7 +165,8 @@ describe('', () => { }); test(`#get`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.get.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const type = 'foo'; @@ -171,7 +179,8 @@ describe('', () => { }); test(`#openPointInTimeForType`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.openPointInTimeForType.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const type = 'search'; @@ -186,7 +195,8 @@ describe('', () => { }); test(`#closePointInTime`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.closePointInTime.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const id = '42'; @@ -198,7 +208,8 @@ describe('', () => { }); test(`#bulkResolve`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.bulkResolve.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const objects: SavedObjectsBulkResolveObject[] = [ @@ -213,7 +224,8 @@ describe('', () => { }); test(`#resolve`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.resolve.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const type = 'foo'; @@ -226,7 +238,8 @@ describe('', () => { }); test(`#update`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.update.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const type = 'some-type'; @@ -240,7 +253,8 @@ describe('', () => { }); test(`#bulkUpdate`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.bulkUpdate.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const type = 'some-type'; @@ -257,7 +271,8 @@ describe('', () => { }); test(`#collectMultiNamespaceReferences`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.collectMultiNamespaceReferences.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const objects: SavedObjectsCollectMultiNamespaceReferencesObject[] = [ @@ -275,7 +290,8 @@ describe('', () => { }); test(`#updateObjectsSpaces`, async () => { - const returnValue = Symbol(); + const returnValue: any = Symbol(); + mockRepository.updateObjectsSpaces.mockResolvedValueOnce(returnValue); const client = new SavedObjectsClient(mockRepository); const objects: SavedObjectsUpdateObjectsSpacesObject[] = [ From ed543a992869f01f30a1c7335bdc73f247c4da01 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 4 Nov 2021 18:01:23 +0100 Subject: [PATCH 04/22] start fixing violations for repository.test.ts --- ...{repository.test.js => repository.test.ts} | 157 +++++++++++------- 1 file changed, 99 insertions(+), 58 deletions(-) rename src/core/server/saved_objects/service/lib/{repository.test.js => repository.test.ts} (97%) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.ts similarity index 97% rename from src/core/server/saved_objects/service/lib/repository.test.js rename to src/core/server/saved_objects/service/lib/repository.test.ts index f61a79ca9de66..8221dfef13b95 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -6,6 +6,8 @@ * Side Public License, v 1. */ +/* eslint-disable @typescript-eslint/no-shadow */ + import { pointInTimeFinderMock, mockCollectMultiNamespaceReferences, @@ -17,6 +19,7 @@ import { mockDeleteLegacyUrlAliases, } from './repository.test.mock'; +import { SavedObjectsType, SavedObject, SavedObjectReference } from '../../types'; import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -32,6 +35,7 @@ import { LEGACY_URL_ALIAS_TYPE } from '../../object_types'; import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks'; import * as esKuery from '@kbn/es-query'; import { errors as EsErrors } from '@elastic/elasticsearch'; +import { SavedObjectsCreateOptions, SavedObjectsUpdateOptions } from '../saved_objects_client'; const { nodeTypes } = esKuery; @@ -40,22 +44,27 @@ jest.mock('./search_dsl/search_dsl', () => ({ getSearchDsl: jest.fn() })); // BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository // so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient. -const createBadRequestError = (...args) => - SavedObjectsErrorHelpers.createBadRequestError(...args).output.payload; -const createConflictError = (...args) => - SavedObjectsErrorHelpers.createConflictError(...args).output.payload; -const createGenericNotFoundError = (...args) => - SavedObjectsErrorHelpers.createGenericNotFoundError(...args).output.payload; -const createUnsupportedTypeError = (...args) => - SavedObjectsErrorHelpers.createUnsupportedTypeError(...args).output.payload; +interface TypeIdTuple { + id: string; + type: string; +} + +const createBadRequestError = (reason?: string) => + SavedObjectsErrorHelpers.createBadRequestError(reason).output.payload; +const createConflictError = (type: string, id: string, reason?: string) => + SavedObjectsErrorHelpers.createConflictError(type, id, reason).output.payload; +const createGenericNotFoundError = (type: string | null = null, id: string | null = null) => + SavedObjectsErrorHelpers.createGenericNotFoundError(type, id).output.payload; +const createUnsupportedTypeError = (type: string) => + SavedObjectsErrorHelpers.createUnsupportedTypeError(type).output.payload; describe('SavedObjectsRepository', () => { - let client; - let savedObjectsRepository; - let migrator; - let logger; + let client: ReturnType; + let savedObjectsRepository: SavedObjectsRepository; + let migrator: ReturnType; + let logger: ReturnType; + let serializer: jest.Mocked; - let serializer; const mockTimestamp = '2017-08-14T15:49:14.886Z'; const mockTimestampFields = { updated_at: mockTimestamp }; const mockVersionProps = { _seq_no: 1, _primary_term: 1 }; @@ -152,52 +161,59 @@ describe('SavedObjectsRepository', () => { }, }; - const createType = (type) => ({ + const createType = (type: string, parts: Partial = {}): SavedObjectsType => ({ name: type, + hidden: false, + namespaceType: 'single', mappings: { properties: mappings.properties[type].properties }, migrations: { '1.1.1': (doc) => doc }, + ...parts, }); const registry = new SavedObjectTypeRegistry(); registry.registerType(createType('config')); registry.registerType(createType('index-pattern')); registry.registerType(createType('dashboard')); - registry.registerType({ - ...createType(CUSTOM_INDEX_TYPE), - indexPattern: 'custom', - }); - registry.registerType({ - ...createType(NAMESPACE_AGNOSTIC_TYPE), - namespaceType: 'agnostic', - }); - registry.registerType({ - ...createType(MULTI_NAMESPACE_TYPE), - namespaceType: 'multiple', - }); - registry.registerType({ - ...createType(MULTI_NAMESPACE_ISOLATED_TYPE), - namespaceType: 'multiple-isolated', - }); - registry.registerType({ - ...createType(MULTI_NAMESPACE_CUSTOM_INDEX_TYPE), - namespaceType: 'multiple', - indexPattern: 'custom', - }); - registry.registerType({ - ...createType(HIDDEN_TYPE), - hidden: true, - namespaceType: 'agnostic', - }); + registry.registerType(createType(CUSTOM_INDEX_TYPE, { indexPattern: 'custom' })); + registry.registerType(createType(NAMESPACE_AGNOSTIC_TYPE, { namespaceType: 'agnostic' })); + registry.registerType(createType(MULTI_NAMESPACE_TYPE, { namespaceType: 'multiple' })); + registry.registerType( + createType(MULTI_NAMESPACE_ISOLATED_TYPE, { namespaceType: 'multiple-isolated' }) + ); + registry.registerType( + createType(MULTI_NAMESPACE_CUSTOM_INDEX_TYPE, { + namespaceType: 'multiple', + indexPattern: 'custom', + }) + ); + registry.registerType( + createType(HIDDEN_TYPE, { + hidden: true, + namespaceType: 'agnostic', + }) + ); const documentMigrator = new DocumentMigrator({ typeRegistry: registry, kibanaVersion: KIBANA_VERSION, - log: {}, + log: loggerMock.create(), }); const getMockGetResponse = ( - { type, id, references, namespace: objectNamespace, originId }, - namespace + { + type, + id, + references, + namespace: objectNamespace, + originId, + }: { + type: string; + id: string; + namespace?: string; + originId?: string; + references?: SavedObjectReference[]; + }, + namespace?: string | string[] ) => { let namespaces; if (objectNamespace) { @@ -229,7 +245,7 @@ describe('SavedObjectsRepository', () => { }; }; - const getMockMgetResponse = (objects, namespace) => ({ + const getMockMgetResponse = (objects, namespace?: string) => ({ docs: objects.map((obj) => obj.found === false ? obj : getMockGetResponse(obj, obj.initialNamespaces ?? namespace) ), @@ -244,18 +260,27 @@ describe('SavedObjectsRepository', () => { } }, }); - const expectSuccess = ({ type, id }) => expect.toBeDocumentWithoutError(type, id); - const expectError = ({ type, id }) => ({ type, id, error: expect.any(Object) }); - const expectErrorResult = ({ type, id }, error, overrides = {}) => ({ + const expectSuccess = ({ type, id }: { type: string; id: string }) => + expect.toBeDocumentWithoutError(type, id); + const expectError = ({ type, id }: { type: string; id: string }) => ({ + type, + id, + error: expect.any(Object), + }); + const expectErrorResult = ( + { type, id }: TypeIdTuple, + error: Record, + overrides = {} + ) => ({ type, id, error: { ...error, ...overrides }, }); - const expectErrorNotFound = (obj, overrides) => + const expectErrorNotFound = (obj: TypeIdTuple, overrides) => expectErrorResult(obj, createGenericNotFoundError(obj.type, obj.id), overrides); - const expectErrorConflict = (obj, overrides) => + const expectErrorConflict = (obj: TypeIdTuple, overrides) => expectErrorResult(obj, createConflictError(obj.type, obj.id), overrides); - const expectErrorInvalidType = (obj, overrides) => + const expectErrorInvalidType = (obj: TypeIdTuple, overrides) => expectErrorResult(obj, createUnsupportedTypeError(obj.type, obj.id), overrides); const expectMigrationArgs = (args, contains = true, n = 1) => { @@ -358,7 +383,7 @@ describe('SavedObjectsRepository', () => { }; }; - const bulkCreateSuccess = async (objects, options) => { + const bulkCreateSuccess = async (objects, options?: SavedObjectsCreateOptions) => { const response = getMockBulkCreateResponse(objects, options?.namespace); client.bulk.mockResolvedValue( elasticsearchClientMock.createSuccessTransportRequestPromise(response) @@ -391,7 +416,14 @@ describe('SavedObjectsRepository', () => { ); }; - const expectObjArgs = ({ type, attributes, references }, overrides) => [ + const expectObjArgs = ( + { + type, + attributes, + references, + }: { type: string; attributes: unknown; references?: SavedObjectReference[] }, + overrides + ) => [ expect.any(Object), expect.objectContaining({ [type]: attributes, @@ -622,7 +654,7 @@ describe('SavedObjectsRepository', () => { }); it(`doesn't add namespaces to request body for any types that are not multi-namespace`, async () => { - const test = async (namespace) => { + const test = async (namespace?: string) => { const objects = [obj1, { ...obj2, type: NAMESPACE_AGNOSTIC_TYPE }]; await bulkCreateSuccess(objects, { namespace, overwrite: true }); const expected = expect.not.objectContaining({ namespaces: expect.anything() }); @@ -755,7 +787,7 @@ describe('SavedObjectsRepository', () => { }); it(`returns error when initialNamespaces is used with a space-isolated object and does not specify a single space`, async () => { - const doTest = async (objType, initialNamespaces) => { + const doTest = async (objType: string, initialNamespaces: string[]) => { const obj = { ...obj3, type: objType, initialNamespaces }; await bulkCreateError( obj, @@ -1906,9 +1938,12 @@ describe('SavedObjectsRepository', () => { }, ]; - const createSuccess = async (type, attributes, options) => { - const result = await savedObjectsRepository.create(type, attributes, options); - return result; + const createSuccess = async ( + type: string, + attributes: T, + options?: SavedObjectsCreateOptions + ) => { + return await savedObjectsRepository.create(type, attributes, options); }; describe('client calls', () => { @@ -4072,7 +4107,13 @@ describe('SavedObjectsRepository', () => { ); }; - const updateSuccess = async (type, id, attributes, options, internalOptions = {}) => { + const updateSuccess = async ( + type: string, + id: string, + attributes: T, + options?: SavedObjectsUpdateOptions, + internalOptions = {} + ) => { const { mockGetResponseValue, includeOriginId } = internalOptions; if (registry.isMultiNamespace(type)) { const mockGetResponse = From 62ed850fd69ddf97e4025426c866d9b23c1f434c Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 4 Nov 2021 23:37:45 +0100 Subject: [PATCH 05/22] fix ~750 violations, 201 left --- .../service/lib/repository.test.ts | 288 ++++++++++++------ 1 file changed, 187 insertions(+), 101 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 8221dfef13b95..824d80708717b 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -19,14 +19,25 @@ import { mockDeleteLegacyUrlAliases, } from './repository.test.mock'; -import { SavedObjectsType, SavedObject, SavedObjectReference } from '../../types'; -import { SavedObjectsRepository } from './repository'; +import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { + SavedObjectsType, + SavedObject, + SavedObjectReference, + SavedObjectsBaseOptions, + SavedObjectsFindOptions, +} from '../../types'; +import { + SavedObjectsDeleteByNamespaceOptions, + SavedObjectsIncrementCounterOptions, + SavedObjectsRepository, +} from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; import { PointInTimeFinder } from './point_in_time_finder'; import { ALL_NAMESPACES_STRING } from './utils'; import { loggerMock } from '../../../logging/logger.mock'; -import { SavedObjectsSerializer } from '../../serialization'; +import { SavedObjectsSerializer, SavedObjectUnsanitizedDoc } from '../../serialization'; import { encodeHitVersion } from '../../version'; import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { DocumentMigrator } from '../../migrations/core/document_migrator'; @@ -35,7 +46,14 @@ import { LEGACY_URL_ALIAS_TYPE } from '../../object_types'; import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks'; import * as esKuery from '@kbn/es-query'; import { errors as EsErrors } from '@elastic/elasticsearch'; -import { SavedObjectsCreateOptions, SavedObjectsUpdateOptions } from '../saved_objects_client'; +import { + SavedObjectsBulkUpdateOptions, + SavedObjectsCreateOptions, + SavedObjectsDeleteOptions, + SavedObjectsOpenPointInTimeOptions, + SavedObjectsUpdateOptions, +} from '../saved_objects_client'; +import { SavedObjectsMappingProperties, SavedObjectsTypeMappingDefinition } from '../../mappings'; const { nodeTypes } = esKuery; @@ -97,11 +115,13 @@ describe('SavedObjectsRepository', () => { const MULTI_NAMESPACE_CUSTOM_INDEX_TYPE = 'multiNamespaceTypeCustomIndex'; const HIDDEN_TYPE = 'hiddenType'; - const mappings = { + const mappings: SavedObjectsTypeMappingDefinition = { properties: { config: { properties: { - type: 'keyword', + otherField: { + type: 'keyword', + }, }, }, 'index-pattern': { @@ -120,7 +140,9 @@ describe('SavedObjectsRepository', () => { }, [CUSTOM_INDEX_TYPE]: { properties: { - type: 'keyword', + otherField: { + type: 'keyword', + }, }, }, [NAMESPACE_AGNOSTIC_TYPE]: { @@ -165,7 +187,9 @@ describe('SavedObjectsRepository', () => { name: type, hidden: false, namespaceType: 'single', - mappings: { properties: mappings.properties[type].properties }, + mappings: { + properties: mappings.properties[type].properties! as SavedObjectsMappingProperties, + }, migrations: { '1.1.1': (doc) => doc }, ...parts, }); @@ -242,7 +266,7 @@ describe('SavedObjectsRepository', () => { specialProperty: 'specialValue', ...mockTimestampFields, }, - }; + } as estypes.GetResponse; }; const getMockMgetResponse = (objects, namespace?: string) => ({ @@ -270,46 +294,52 @@ describe('SavedObjectsRepository', () => { const expectErrorResult = ( { type, id }: TypeIdTuple, error: Record, - overrides = {} + overrides: Record = {} ) => ({ type, id, error: { ...error, ...overrides }, }); - const expectErrorNotFound = (obj: TypeIdTuple, overrides) => + const expectErrorNotFound = (obj: TypeIdTuple, overrides?: Record) => expectErrorResult(obj, createGenericNotFoundError(obj.type, obj.id), overrides); - const expectErrorConflict = (obj: TypeIdTuple, overrides) => + const expectErrorConflict = (obj: TypeIdTuple, overrides?: Record) => expectErrorResult(obj, createConflictError(obj.type, obj.id), overrides); - const expectErrorInvalidType = (obj: TypeIdTuple, overrides) => - expectErrorResult(obj, createUnsupportedTypeError(obj.type, obj.id), overrides); + const expectErrorInvalidType = (obj: TypeIdTuple, overrides?: Record) => + expectErrorResult(obj, createUnsupportedTypeError(obj.type), overrides); - const expectMigrationArgs = (args, contains = true, n = 1) => { + const expectMigrationArgs = (args: unknown, contains = true, n = 1) => { const obj = contains ? expect.objectContaining(args) : expect.not.objectContaining(args); expect(migrator.migrateDocument).toHaveBeenNthCalledWith(n, obj); }; + const createSpySerializer = () => { + const spyInstance = { + isRawSavedObject: jest.fn(), + rawToSavedObject: jest.fn(), + savedObjectToRaw: jest.fn(), + generateRawId: jest.fn(), + generateRawLegacyUrlAliasId: jest.fn(), + trimIdPrefix: jest.fn(), + }; + const realInstance = new SavedObjectsSerializer(registry); + Object.keys(spyInstance).forEach((key) => { + spyInstance[key].mockImplementation((...args) => realInstance[key](...args)); + }); + + return spyInstance as unknown as jest.Mocked; + }; + beforeEach(() => { pointInTimeFinderMock.mockClear(); client = elasticsearchClientMock.createElasticsearchClient(); migrator = mockKibanaMigrator.create(); documentMigrator.prepareMigrations(); migrator.migrateDocument = jest.fn().mockImplementation(documentMigrator.migrate); - migrator.runMigrations = async () => ({ status: 'skipped' }); + migrator.runMigrations = jest.fn().mockResolvedValue([{ status: 'skipped' }]); logger = loggerMock.create(); // create a mock serializer "shim" so we can track function calls, but use the real serializer's implementation - serializer = { - isRawSavedObject: jest.fn(), - rawToSavedObject: jest.fn(), - savedObjectToRaw: jest.fn(), - generateRawId: jest.fn(), - generateRawLegacyUrlAliasId: jest.fn(), - trimIdPrefix: jest.fn(), - }; - const _serializer = new SavedObjectsSerializer(registry); - Object.keys(serializer).forEach((key) => { - serializer[key].mockImplementation((...args) => _serializer[key](...args)); - }); + serializer = createSpySerializer(); const allTypes = registry.getAllTypes().map((type) => type.name); const allowedTypes = [...new Set(allTypes.filter((type) => !registry.isHidden(type)))]; @@ -330,7 +360,7 @@ describe('SavedObjectsRepository', () => { }); const mockMigrationVersion = { foo: '2.3.4' }; - const mockMigrateDocument = (doc) => ({ + const mockMigrateDocument = (doc: SavedObjectUnsanitizedDoc) => ({ ...doc, attributes: { ...doc.attributes, @@ -344,7 +374,7 @@ describe('SavedObjectsRepository', () => { beforeEach(() => { mockPreflightCheckForCreate.mockReset(); mockPreflightCheckForCreate.mockImplementation(({ objects }) => { - return objects.map(({ type, id }) => ({ type, id })); // respond with no errors by default + return Promise.resolve(objects.map(({ type, id }) => ({ type, id }))); // respond with no errors by default }); }); @@ -363,7 +393,7 @@ describe('SavedObjectsRepository', () => { }; const namespace = 'foo-namespace'; - const getMockBulkCreateResponse = (objects, namespace) => { + const getMockBulkCreateResponse = (objects: SavedObject[], namespace?: string) => { return { items: objects.map(({ type, id, originId, attributes, references, migrationVersion }) => ({ create: { @@ -395,7 +425,11 @@ describe('SavedObjectsRepository', () => { // bulk create calls have two objects for each source -- the action, and the source const expectClientCallArgsAction = ( objects, - { method, _index = expect.any(String), getId = () => expect.any(String) } + { + method, + _index = expect.any(String), + getId = () => expect.any(String), + }: { method: string; _index?: string; getId?: (type: string, id: string) => string } ) => { const body = []; for (const { type, id, if_primary_term: ifPrimaryTerm, if_seq_no: ifSeqNo } of objects) { @@ -434,7 +468,7 @@ describe('SavedObjectsRepository', () => { }), ]; - const expectSuccessResult = (obj) => ({ + const expectSuccessResult = (obj: SavedObjectUnsanitizedDoc) => ({ ...obj, migrationVersion: { [obj.type]: '1.1.1' }, coreMigrationVersion: KIBANA_VERSION, @@ -582,7 +616,7 @@ describe('SavedObjectsRepository', () => { }); it(`adds initialNamespaces instead of namespace`, async () => { - const test = async (namespace) => { + const test = async (namespace?: string) => { const ns2 = 'bar-namespace'; const ns3 = 'baz-namespace'; const objects = [ @@ -636,7 +670,7 @@ describe('SavedObjectsRepository', () => { }); it(`normalizes initialNamespaces from 'default' to undefined`, async () => { - const test = async (namespace) => { + const test = async (namespace?: string) => { const objects = [{ ...obj1, type: 'dashboard', initialNamespaces: ['default'] }]; await bulkCreateSuccess(objects, { namespace, overwrite: true }); const body = [ @@ -694,19 +728,19 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const getId = (type: string, id: string) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkCreateSuccess([obj1, obj2], { namespace }); expectClientCallArgsAction([obj1, obj2], { method: 'create', getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkCreateSuccess([obj1, obj2]); expectClientCallArgsAction([obj1, obj2], { method: 'create', getId }); }); it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) const objects = [ { ...obj1, type: NAMESPACE_AGNOSTIC_TYPE }, { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE }, @@ -1039,12 +1073,12 @@ describe('SavedObjectsRepository', () => { }; const namespace = 'foo-namespace'; - const bulkGet = async (objects, options) => + const bulkGet = async (objects, options?: SavedObjectsBaseOptions) => savedObjectsRepository.bulkGet( objects.map(({ type, id, namespaces }) => ({ type, id, namespaces })), // bulkGet only uses type, id, and optionally namespaces options ); - const bulkGetSuccess = async (objects, options) => { + const bulkGetSuccess = async (objects, options?: SavedObjectsBaseOptions) => { const response = getMockMgetResponse(objects, options?.namespace); client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) @@ -1055,8 +1089,11 @@ describe('SavedObjectsRepository', () => { }; const _expectClientCallArgs = ( - objects, - { _index = expect.any(String), getId = () => expect.any(String) } + objects: TypeIdTuple[], + { + _index = expect.any(String), + getId = () => expect.any(String), + }: { _index?: string; getId?: (type: string, id: string) => string } ) => { expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ @@ -1075,32 +1112,32 @@ describe('SavedObjectsRepository', () => { describe('client calls', () => { it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const getId = (type: string, id: string) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkGetSuccess([obj1, obj2], { namespace }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`prepends namespace to the id when providing namespaces for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const getId = (type: string, id: string) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) const objects = [obj1, obj2].map((obj) => ({ ...obj, namespaces: [namespace] })); await bulkGetSuccess(objects, { namespace: 'some-other-ns' }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkGetSuccess([obj1, obj2]); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`normalizes options.namespace from 'default' to undefined`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkGetSuccess([obj1, obj2], { namespace: 'default' }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) let objects = [obj1, obj2].map((obj) => ({ ...obj, type: NAMESPACE_AGNOSTIC_TYPE })); await bulkGetSuccess(objects, { namespace }); _expectClientCallArgs(objects, { getId }); @@ -1116,7 +1153,7 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const bulkGetError = async (obj, isBulkError, expectedErrorResult) => { + const bulkGetError = async (obj, isBulkError: boolean, expectedErrorResult) => { let response; if (isBulkError) { // mock the bulk error for only the second object @@ -1208,7 +1245,7 @@ describe('SavedObjectsRepository', () => { }); describe('returns', () => { - const expectSuccessResult = ({ type, id }, doc) => ({ + const expectSuccessResult = ({ type, id }: TypeIdTuple, doc) => ({ type, id, namespaces: doc._source.namespaces ?? ['default'], @@ -1339,7 +1376,11 @@ describe('SavedObjectsRepository', () => { const originId = 'some-origin-id'; const namespace = 'foo-namespace'; - const getMockBulkUpdateResponse = (objects, options, includeOriginId) => ({ + const getMockBulkUpdateResponse = ( + objects: TypeIdTuple[], + options?: SavedObjectsBulkUpdateOptions, + includeOriginId?: boolean + ) => ({ items: objects.map(({ type, id }) => ({ update: { _id: `${ @@ -1358,7 +1399,11 @@ describe('SavedObjectsRepository', () => { })), }); - const bulkUpdateSuccess = async (objects, options, includeOriginId) => { + const bulkUpdateSuccess = async ( + objects, + options?: SavedObjectsBulkUpdateOptions, + includeOriginId?: boolean + ) => { const multiNamespaceObjects = objects.filter(({ type }) => registry.isMultiNamespace(type)); if (multiNamespaceObjects?.length) { const response = getMockMgetResponse(multiNamespaceObjects, options?.namespace); @@ -1366,7 +1411,7 @@ describe('SavedObjectsRepository', () => { elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); } - const response = getMockBulkUpdateResponse(objects, options?.namespace, includeOriginId); + const response = getMockBulkUpdateResponse(objects, options, includeOriginId); client.bulk.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -1377,8 +1422,18 @@ describe('SavedObjectsRepository', () => { // bulk create calls have two objects for each source -- the action, and the source const expectClientCallArgsAction = ( - objects, - { method, _index = expect.any(String), getId = () => expect.any(String), overrides } + objects: TypeIdTuple[], + { + method, + _index = expect.any(String), + getId = () => expect.any(String), + overrides = {}, + }: { + method: string; + _index?: string; + getId?: (type: string, id: string) => string; + overrides?: Record; + } ) => { const body = []; for (const { type, id } of objects) { @@ -1481,7 +1536,7 @@ describe('SavedObjectsRepository', () => { }); it(`doesn't accept custom references if not an array`, async () => { - const test = async (references) => { + const test = async (references: unknown) => { const objects = [obj1, obj2].map((obj) => ({ ...obj, references })); await bulkUpdateSuccess(objects); const expected = { doc: expect.not.objectContaining({ references: expect.anything() }) }; @@ -1553,7 +1608,7 @@ describe('SavedObjectsRepository', () => { }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkUpdateSuccess([obj1, obj2]); expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); @@ -1570,13 +1625,13 @@ describe('SavedObjectsRepository', () => { }); it(`normalizes options.namespace from 'default' to undefined`, async () => { - const getId = (type, id) => `${type}:${id}`; + const getId = (type: string, id: string) => `${type}:${id}`; await bulkUpdateSuccess([obj1, obj2], { namespace: 'default' }); expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); }); it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) const overrides = { // bulkUpdate uses a preflight `get` request for multi-namespace saved objects, and specifies that version on `update` // we aren't testing for this here, but we need to include Jest assertions so this test doesn't fail @@ -1791,12 +1846,15 @@ describe('SavedObjectsRepository', () => { const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; const namespace = 'foo-namespace'; - const checkConflicts = async (objects, options) => + const checkConflicts = async (objects: TypeIdTuple[], options?: SavedObjectsBaseOptions) => savedObjectsRepository.checkConflicts( objects.map(({ type, id }) => ({ type, id })), // checkConflicts only uses type and id options ); - const checkConflictsSuccess = async (objects, options) => { + const checkConflictsSuccess = async ( + objects: TypeIdTuple[], + options?: SavedObjectsBaseOptions + ) => { const response = getMockMgetResponse(objects, options?.namespace); client.mget.mockResolvedValue( elasticsearchClientMock.createSuccessTransportRequestPromise(response) @@ -1808,7 +1866,10 @@ describe('SavedObjectsRepository', () => { const _expectClientCallArgs = ( objects, - { _index = expect.any(String), getId = () => expect.any(String) } + { + _index = expect.any(String), + getId = () => expect.any(String), + }: { _index?: string; getId?: (type: string, id: string) => string } ) => { expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ @@ -1832,25 +1893,25 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const getId = (type: string, id: string) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await checkConflictsSuccess([obj1, obj2], { namespace }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await checkConflictsSuccess([obj1, obj2]); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`normalizes options.namespace from 'default' to undefined`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await checkConflictsSuccess([obj1, obj2], { namespace: 'default' }); _expectClientCallArgs([obj1, obj2], { getId }); }); it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) // obj3 is multi-namespace, and obj6 is namespace-agnostic await checkConflictsSuccess([obj3, obj6], { namespace }); _expectClientCallArgs([obj3, obj6], { getId }); @@ -2430,7 +2491,12 @@ describe('SavedObjectsRepository', () => { const id = 'logstash-*'; const namespace = 'foo-namespace'; - const deleteSuccess = async (type, id, options, internalOptions = {}) => { + const deleteSuccess = async ( + type: string, + id: string, + options?: SavedObjectsDeleteOptions, + internalOptions = {} + ) => { const { mockGetResponseValue } = internalOptions; if (registry.isMultiNamespace(type)) { const mockGetResponse = @@ -2584,7 +2650,11 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const expectNotFoundError = async (type, id, options) => { + const expectNotFoundError = async ( + type: string, + id: string, + options?: SavedObjectsDeleteOptions + ) => { await expect(savedObjectsRepository.delete(type, id, options)).rejects.toThrowError( createGenericNotFoundError(type, id) ); @@ -2718,7 +2788,10 @@ describe('SavedObjectsRepository', () => { failures: [], }; - const deleteByNamespaceSuccess = async (namespace, options) => { + const deleteByNamespaceSuccess = async ( + namespace: string, + options?: SavedObjectsDeleteByNamespaceOptions + ) => { client.updateByQuery.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(mockUpdateResults) ); @@ -2940,7 +3013,7 @@ describe('SavedObjectsRepository', () => { }); describe('#find', () => { - const generateSearchResults = (namespace) => { + const generateSearchResults = (namespace?: string) => { return { hits: { total: 4, @@ -3008,13 +3081,13 @@ describe('SavedObjectsRepository', () => { }, ], }, - }; + } as estypes.SearchResponse; }; const type = 'index-pattern'; const namespace = 'foo-namespace'; - const findSuccess = async (options, namespace) => { + const findSuccess = async (options?: SavedObjectsFindOptions, namespace?: string) => { client.search.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( generateSearchResults(namespace) @@ -3100,7 +3173,7 @@ describe('SavedObjectsRepository', () => { }); it(`should not make a client call when attempting to find only invalid or hidden types`, async () => { - const test = async (types) => { + const test = async (types: string | string[]) => { await savedObjectsRepository.find({ type: types }); expect(client.search).not.toHaveBeenCalled(); }; @@ -3148,12 +3221,14 @@ describe('SavedObjectsRepository', () => { it(`throws when searchFields is defined but not an array`, async () => { await expect( + // @ts-expect-error searchFields is an array savedObjectsRepository.find({ type, searchFields: 'string' }) ).rejects.toThrowError('options.searchFields must be an array'); expect(client.search).not.toHaveBeenCalled(); }); it(`throws when fields is defined but not an array`, async () => { + // @ts-expect-error fields is an array await expect(savedObjectsRepository.find({ type, fields: 'string' })).rejects.toThrowError( 'options.fields must be an array' ); @@ -3279,7 +3354,7 @@ describe('SavedObjectsRepository', () => { }); describe('search dsl', () => { - const commonOptions = { + const commonOptions: SavedObjectsFindOptions = { type: [type], // cannot be used when `typeToNamespacesMap` is present namespaces: [namespace], // cannot be used when `typeToNamespacesMap` is present search: 'foo*', @@ -3291,7 +3366,6 @@ describe('SavedObjectsRepository', () => { type: 'foo', id: '1', }, - kueryNode: undefined, }; it(`passes mappings, registry, and search options to getSearchDsl`, async () => { @@ -3315,7 +3389,7 @@ describe('SavedObjectsRepository', () => { }); it(`accepts hasReferenceOperator`, async () => { - const relevantOpts = { + const relevantOpts: SavedObjectsFindOptions = { ...commonOptions, hasReferenceOperator: 'AND', }; @@ -3328,20 +3402,20 @@ describe('SavedObjectsRepository', () => { }); it(`accepts searchAfter`, async () => { - const relevantOpts = { + const relevantOpts: SavedObjectsFindOptions = { ...commonOptions, - searchAfter: [1, 'a'], + searchAfter: ['1', 'a'], }; await findSuccess(relevantOpts, namespace); expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { ...relevantOpts, - searchAfter: [1, 'a'], + searchAfter: ['1', 'a'], }); }); it(`accepts pit`, async () => { - const relevantOpts = { + const relevantOpts: SavedObjectsFindOptions = { ...commonOptions, pit: { id: 'abc123', keepAlive: '2m' }, }; @@ -3354,7 +3428,7 @@ describe('SavedObjectsRepository', () => { }); it(`accepts KQL expression filter and passes KueryNode to getSearchDsl`, async () => { - const findOpts = { + const findOpts: SavedObjectsFindOptions = { namespaces: [namespace], search: 'foo*', searchFields: ['foo'], @@ -3366,7 +3440,6 @@ describe('SavedObjectsRepository', () => { type: 'foo', id: '1', }, - indexPattern: undefined, filter: 'dashboard.attributes.otherField: *', }; @@ -3395,7 +3468,7 @@ describe('SavedObjectsRepository', () => { }); it(`accepts KQL KueryNode filter and passes KueryNode to getSearchDsl`, async () => { - const findOpts = { + const findOpts: SavedObjectsFindOptions = { namespaces: [namespace], search: 'foo*', searchFields: ['foo'], @@ -3407,7 +3480,6 @@ describe('SavedObjectsRepository', () => { type: 'foo', id: '1', }, - indexPattern: undefined, filter: nodeTypes.function.buildNode('is', `dashboard.attributes.otherField`, '*'), }; @@ -3482,7 +3554,12 @@ describe('SavedObjectsRepository', () => { const namespace = 'foo-namespace'; const originId = 'some-origin-id'; - const getSuccess = async (type, id, options, includeOriginId) => { + const getSuccess = async ( + type: string, + id: string, + options?: SavedObjectsBaseOptions, + includeOriginId?: boolean + ) => { const response = getMockGetResponse( { type, @@ -3558,7 +3635,11 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const expectNotFoundError = async (type, id, options) => { + const expectNotFoundError = async ( + type: string, + id: string, + options?: SavedObjectsBaseOptions + ) => { await expect(savedObjectsRepository.get(type, id, options)).rejects.toThrowError( createGenericNotFoundError(type, id) ); @@ -3582,7 +3663,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }, undefined) + elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) ); await expectNotFoundError(type, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -3668,14 +3749,14 @@ describe('SavedObjectsRepository', () => { const expectedResult = { type: 'obj-type', id: 'obj-id', error }; mockInternalBulkResolve.mockResolvedValue({ resolved_objects: [expectedResult] }); - await expect(savedObjectsRepository.resolve()).rejects.toEqual(error); + await expect(savedObjectsRepository.resolve('foo', '2')).rejects.toEqual(error); }); it('throws when internalBulkResolve throws', async () => { const error = new Error('Oh no!'); mockInternalBulkResolve.mockRejectedValue(error); - await expect(savedObjectsRepository.resolve()).rejects.toEqual(error); + await expect(savedObjectsRepository.resolve('foo', '2')).rejects.toEqual(error); }); }); @@ -3686,7 +3767,13 @@ describe('SavedObjectsRepository', () => { const namespace = 'foo-namespace'; const originId = 'some-origin-id'; - const incrementCounterSuccess = async (type, id, fields, options, internalOptions = {}) => { + const incrementCounterSuccess = async ( + type: string, + id: string, + fields: string[], + options?: SavedObjectsIncrementCounterOptions, + internalOptions = {} + ) => { const { mockGetResponseValue } = internalOptions; const isMultiNamespace = registry.isMultiNamespace(type); if (isMultiNamespace) { @@ -3879,8 +3966,9 @@ describe('SavedObjectsRepository', () => { }); it(`throws when counterField is not CounterField type`, async () => { - const test = async (field) => { + const test = async (field: unknown[]) => { await expect( + // @ts-expect-error field is of wrong type savedObjectsRepository.incrementCounter(type, id, field) ).rejects.toThrowError( `"counterFields" argument must be of type Array` @@ -4364,8 +4452,8 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const expectNotFoundError = async (type, id) => { - await expect(savedObjectsRepository.update(type, id)).rejects.toThrowError( + const expectNotFoundError = async (type: string, id: string) => { + await expect(savedObjectsRepository.update(type, id, {})).rejects.toThrowError( createGenericNotFoundError(type, id) ); }; @@ -4407,9 +4495,7 @@ describe('SavedObjectsRepository', () => { client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); - await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id, { - namespace: 'bar-namespace', - }); + await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); }); @@ -4496,8 +4582,8 @@ describe('SavedObjectsRepository', () => { describe('#openPointInTimeForType', () => { const type = 'index-pattern'; - const generateResults = (id) => ({ id: id || null }); - const successResponse = async (type, options) => { + const generateResults = (id?: string) => ({ id: id || null }); + const successResponse = async (type: string, options?: SavedObjectsOpenPointInTimeOptions) => { client.openPointInTime.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(generateResults()) ); @@ -4544,7 +4630,7 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const expectNotFoundError = async (types) => { + const expectNotFoundError = async (types: string | string[]) => { await expect(savedObjectsRepository.openPointInTimeForType(types)).rejects.toThrowError( createGenericNotFoundError() ); @@ -4559,7 +4645,7 @@ describe('SavedObjectsRepository', () => { }); it(`should return generic not found error when attempting to find only invalid or hidden types`, async () => { - const test = async (types) => { + const test = async (types: string | string[]) => { await expectNotFoundError(types); expect(client.openPointInTime).not.toHaveBeenCalled(); }; @@ -4585,7 +4671,7 @@ describe('SavedObjectsRepository', () => { describe('#closePointInTime', () => { const generateResults = () => ({ succeeded: true, num_freed: 3 }); - const successResponse = async (id) => { + const successResponse = async (id: string) => { client.closePointInTime.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(generateResults()) ); @@ -4615,7 +4701,7 @@ describe('SavedObjectsRepository', () => { describe('returns', () => { it(`returns response body from ES`, async () => { - const results = generateResults('abc123'); + const results = generateResults(); client.closePointInTime.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(results) ); From f0ca78a6900ba45778f8c94b1f6a3e0cca224d8b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 08:39:33 +0100 Subject: [PATCH 06/22] 184 left --- .../service/lib/repository.test.mock.ts | 3 + .../service/lib/repository.test.ts | 105 ++++++++++-------- 2 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.mock.ts b/src/core/server/saved_objects/service/lib/repository.test.mock.ts index 3ec2cb0a5d8b9..9ca156605d638 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.mock.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.mock.ts @@ -68,3 +68,6 @@ export const mockDeleteLegacyUrlAliases = jest.fn() as jest.MockedFunction< jest.mock('./legacy_url_aliases', () => ({ deleteLegacyUrlAliases: mockDeleteLegacyUrlAliases, })); + +export const mockGetSearchDsl = jest.fn(); +jest.mock('./search_dsl/search_dsl', () => ({ getSearchDsl: mockGetSearchDsl })); diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 824d80708717b..0d5db9ba7051b 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -17,6 +17,7 @@ import { mockGetCurrentTime, mockPreflightCheckForCreate, mockDeleteLegacyUrlAliases, + mockGetSearchDsl, } from './repository.test.mock'; import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; @@ -32,12 +33,15 @@ import { SavedObjectsIncrementCounterOptions, SavedObjectsRepository, } from './repository'; -import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; import { PointInTimeFinder } from './point_in_time_finder'; import { ALL_NAMESPACES_STRING } from './utils'; import { loggerMock } from '../../../logging/logger.mock'; -import { SavedObjectsSerializer, SavedObjectUnsanitizedDoc } from '../../serialization'; +import { + SavedObjectsRawDocSource, + SavedObjectsSerializer, + SavedObjectUnsanitizedDoc, +} from '../../serialization'; import { encodeHitVersion } from '../../version'; import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import { DocumentMigrator } from '../../migrations/core/document_migrator'; @@ -47,6 +51,7 @@ import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks'; import * as esKuery from '@kbn/es-query'; import { errors as EsErrors } from '@elastic/elasticsearch'; import { + SavedObjectsBulkCreateObject, SavedObjectsBulkUpdateOptions, SavedObjectsCreateOptions, SavedObjectsDeleteOptions, @@ -54,11 +59,10 @@ import { SavedObjectsUpdateOptions, } from '../saved_objects_client'; import { SavedObjectsMappingProperties, SavedObjectsTypeMappingDefinition } from '../../mappings'; +import { BulkResponseItem } from "@elastic/elasticsearch/lib/api/typesWithBodyKey"; const { nodeTypes } = esKuery; -jest.mock('./search_dsl/search_dsl', () => ({ getSearchDsl: jest.fn() })); - // BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository // so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient. @@ -356,7 +360,7 @@ describe('SavedObjectsRepository', () => { }); mockGetCurrentTime.mockReturnValue(mockTimestamp); - getSearchDslNS.getSearchDsl.mockClear(); + mockGetSearchDsl.mockClear(); }); const mockMigrationVersion = { foo: '2.3.4' }; @@ -378,14 +382,14 @@ describe('SavedObjectsRepository', () => { }); }); - const obj1 = { + const obj1: SavedObject = { type: 'config', id: '6.0.0-alpha1', attributes: { title: 'Test One' }, references: [{ name: 'ref_0', type: 'test', id: '1' }], originId: 'some-origin-id', // only one of the object args has an originId, this is intentional to test both a positive and negative case }; - const obj2 = { + const obj2: SavedObject = { type: 'index-pattern', id: 'logstash-*', attributes: { title: 'Test Two' }, @@ -393,10 +397,17 @@ describe('SavedObjectsRepository', () => { }; const namespace = 'foo-namespace'; - const getMockBulkCreateResponse = (objects: SavedObject[], namespace?: string) => { + const getMockBulkCreateResponse = ( + objects: SavedObjectsBulkCreateObject[], + namespace?: string + ) => { return { + errors: false, + took: 1, items: objects.map(({ type, id, originId, attributes, references, migrationVersion }) => ({ create: { + // status: 1, + // _index: '.kibana', _id: `${namespace ? `${namespace}:` : ''}${type}:${id}`, _source: { [type]: attributes, @@ -410,16 +421,18 @@ describe('SavedObjectsRepository', () => { ...mockVersionProps, }, })), - }; + } as estypes.BulkResponse; }; - const bulkCreateSuccess = async (objects, options?: SavedObjectsCreateOptions) => { + const bulkCreateSuccess = async ( + objects: SavedObjectsBulkCreateObject[], + options?: SavedObjectsCreateOptions + ) => { const response = getMockBulkCreateResponse(objects, options?.namespace); client.bulk.mockResolvedValue( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); - const result = await savedObjectsRepository.bulkCreate(objects, options); - return result; + return await savedObjectsRepository.bulkCreate(objects, options); }; // bulk create calls have two objects for each source -- the action, and the source @@ -456,7 +469,7 @@ describe('SavedObjectsRepository', () => { attributes, references, }: { type: string; attributes: unknown; references?: SavedObjectReference[] }, - overrides + overrides: Record = {} ) => [ expect.any(Object), expect.objectContaining({ @@ -589,7 +602,7 @@ describe('SavedObjectsRepository', () => { }); it(`adds namespaces to request body for any types that are multi-namespace`, async () => { - const test = async (namespace) => { + const test = async (namespace?: string) => { const objects = [obj1, obj2].map((x) => ({ ...x, type: MULTI_NAMESPACE_ISOLATED_TYPE })); const [o1, o2] = objects; mockPreflightCheckForCreate.mockResolvedValueOnce([ @@ -597,7 +610,7 @@ describe('SavedObjectsRepository', () => { { type: o2.type, id: o2.id, - existingDocument: { _source: { namespaces: ['*'] } }, // second object does have an existing document to overwrite + existingDocument: { _id: o2.id, _source: { namespaces: ['*'], type: o2.type } }, // second object does have an existing document to overwrite }, ]); await bulkCreateSuccess(objects, { namespace, overwrite: true }); @@ -1046,7 +1059,7 @@ describe('SavedObjectsRepository', () => { }); describe('#bulkGet', () => { - const obj1 = { + const obj1: SavedObject = { type: 'config', id: '6.0.0-alpha1', attributes: { title: 'Testing' }, @@ -1059,7 +1072,7 @@ describe('SavedObjectsRepository', () => { ], originId: 'some-origin-id', // only one of the results has an originId, this is intentional to test both a positive and negative case }; - const obj2 = { + const obj2: SavedObject = { type: 'index-pattern', id: 'logstash-*', attributes: { title: 'Testing' }, @@ -1073,12 +1086,12 @@ describe('SavedObjectsRepository', () => { }; const namespace = 'foo-namespace'; - const bulkGet = async (objects, options?: SavedObjectsBaseOptions) => + const bulkGet = async (objects: SavedObject[], options?: SavedObjectsBaseOptions) => savedObjectsRepository.bulkGet( objects.map(({ type, id, namespaces }) => ({ type, id, namespaces })), // bulkGet only uses type, id, and optionally namespaces options ); - const bulkGetSuccess = async (objects, options?: SavedObjectsBaseOptions) => { + const bulkGetSuccess = async (objects: SavedObject[], options?: SavedObjectsBaseOptions) => { const response = getMockMgetResponse(objects, options?.namespace); client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) @@ -2796,7 +2809,7 @@ describe('SavedObjectsRepository', () => { elasticsearchClientMock.createSuccessTransportRequestPromise(mockUpdateResults) ); const result = await savedObjectsRepository.deleteByNamespace(namespace, options); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(1); + expect(mockGetSearchDsl).toHaveBeenCalledTimes(1); expect(client.updateByQuery).toHaveBeenCalledTimes(1); return result; }; @@ -2845,7 +2858,7 @@ describe('SavedObjectsRepository', () => { it(`constructs a query using all multi-namespace types, and another using all single-namespace types`, async () => { await deleteByNamespaceSuccess(namespace); const allTypes = registry.getAllTypes().map((type) => type.name); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, { namespaces: [namespace], type: [ ...allTypes.filter((type) => !registry.isNamespaceAgnostic(type)), @@ -2891,7 +2904,7 @@ describe('SavedObjectsRepository', () => { it('merges output of getSearchDsl into es request body', async () => { const query = { query: 1, aggregations: 2 }; - getSearchDslNS.getSearchDsl.mockReturnValue(query); + mockGetSearchDsl.mockReturnValue(query); await removeReferencesToSuccess({ type }); expect(client.updateByQuery).toHaveBeenCalledWith( @@ -2945,16 +2958,12 @@ describe('SavedObjectsRepository', () => { describe('search dsl', () => { it(`passes mappings and registry to getSearchDsl`, async () => { await removeReferencesToSuccess(); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( - mappings, - registry, - expect.anything() - ); + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, expect.anything()); }); it('passes namespace to getSearchDsl', async () => { await removeReferencesToSuccess({ namespace: 'some-ns' }); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( + expect(mockGetSearchDsl).toHaveBeenCalledWith( mappings, registry, expect.objectContaining({ @@ -2965,7 +2974,7 @@ describe('SavedObjectsRepository', () => { it('passes hasReference to getSearchDsl', async () => { await removeReferencesToSuccess(); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( + expect(mockGetSearchDsl).toHaveBeenCalledWith( mappings, registry, expect.objectContaining({ @@ -2979,7 +2988,7 @@ describe('SavedObjectsRepository', () => { it('passes all known types to getSearchDsl', async () => { await removeReferencesToSuccess(); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( + expect(mockGetSearchDsl).toHaveBeenCalledWith( mappings, registry, expect.objectContaining({ @@ -3015,6 +3024,9 @@ describe('SavedObjectsRepository', () => { describe('#find', () => { const generateSearchResults = (namespace?: string) => { return { + took: 1, + timed_out: false, + _shards: {} as any, hits: { total: 4, hits: [ @@ -3081,20 +3093,20 @@ describe('SavedObjectsRepository', () => { }, ], }, - } as estypes.SearchResponse; + } as estypes.SearchResponse; }; const type = 'index-pattern'; const namespace = 'foo-namespace'; - const findSuccess = async (options?: SavedObjectsFindOptions, namespace?: string) => { + const findSuccess = async (options: SavedObjectsFindOptions, namespace?: string) => { client.search.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( generateSearchResults(namespace) ) ); const result = await savedObjectsRepository.find(options); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(1); + expect(mockGetSearchDsl).toHaveBeenCalledTimes(1); expect(client.search).toHaveBeenCalledTimes(1); return result; }; @@ -3107,7 +3119,7 @@ describe('SavedObjectsRepository', () => { it(`merges output of getSearchDsl into es request body`, async () => { const query = { query: 1, aggregations: 2 }; - getSearchDslNS.getSearchDsl.mockReturnValue(query); + mockGetSearchDsl.mockReturnValue(query); await findSuccess({ type }); expect(client.search).toHaveBeenCalledWith( @@ -3243,7 +3255,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when KQL filter syntax is invalid`, async () => { - const findOpts = { + const findOpts: SavedObjectsFindOptions = { namespaces: [namespace], search: 'foo*', searchFields: ['foo'], @@ -3255,7 +3267,6 @@ describe('SavedObjectsRepository', () => { type: 'foo', id: '1', }, - indexPattern: undefined, filter: 'dashboard.attributes.otherField:<', }; @@ -3264,7 +3275,7 @@ describe('SavedObjectsRepository', () => { dashboard.attributes.otherField:< --------------------------------^: Bad Request] `); - expect(getSearchDslNS.getSearchDsl).not.toHaveBeenCalled(); + expect(mockGetSearchDsl).not.toHaveBeenCalled(); expect(client.search).not.toHaveBeenCalled(); }); }); @@ -3370,7 +3381,7 @@ describe('SavedObjectsRepository', () => { it(`passes mappings, registry, and search options to getSearchDsl`, async () => { await findSuccess(commonOptions, namespace); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, commonOptions); + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, commonOptions); }); it(`accepts typeToNamespacesMap`, async () => { @@ -3382,7 +3393,7 @@ describe('SavedObjectsRepository', () => { }; await findSuccess(relevantOpts, namespace); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, { ...relevantOpts, type: [type], }); @@ -3395,7 +3406,7 @@ describe('SavedObjectsRepository', () => { }; await findSuccess(relevantOpts, namespace); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, { ...relevantOpts, hasReferenceOperator: 'AND', }); @@ -3408,7 +3419,7 @@ describe('SavedObjectsRepository', () => { }; await findSuccess(relevantOpts, namespace); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, { ...relevantOpts, searchAfter: ['1', 'a'], }); @@ -3421,7 +3432,7 @@ describe('SavedObjectsRepository', () => { }; await findSuccess(relevantOpts, namespace); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith(mappings, registry, { + expect(mockGetSearchDsl).toHaveBeenCalledWith(mappings, registry, { ...relevantOpts, pit: { id: 'abc123', keepAlive: '2m' }, }); @@ -3444,7 +3455,7 @@ describe('SavedObjectsRepository', () => { }; await findSuccess(findOpts, namespace); - const { kueryNode } = getSearchDslNS.getSearchDsl.mock.calls[0][2]; + const { kueryNode } = mockGetSearchDsl.mock.calls[0][2]; expect(kueryNode).toMatchInlineSnapshot(` Object { "arguments": Array [ @@ -3484,7 +3495,7 @@ describe('SavedObjectsRepository', () => { }; await findSuccess(findOpts, namespace); - const { kueryNode } = getSearchDslNS.getSearchDsl.mock.calls[0][2]; + const { kueryNode } = mockGetSearchDsl.mock.calls[0][2]; expect(kueryNode).toMatchInlineSnapshot(` Object { "arguments": Array [ @@ -3511,7 +3522,7 @@ describe('SavedObjectsRepository', () => { const types = ['config', 'index-pattern']; await findSuccess({ type: types }); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( + expect(mockGetSearchDsl).toHaveBeenCalledWith( mappings, registry, expect.objectContaining({ @@ -3524,7 +3535,7 @@ describe('SavedObjectsRepository', () => { const types = ['config', 'unknownType', 'index-pattern']; await findSuccess({ type: types }); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( + expect(mockGetSearchDsl).toHaveBeenCalledWith( mappings, registry, expect.objectContaining({ @@ -3537,7 +3548,7 @@ describe('SavedObjectsRepository', () => { const types = ['config', HIDDEN_TYPE, 'index-pattern']; await findSuccess({ type: types }); - expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledWith( + expect(mockGetSearchDsl).toHaveBeenCalledWith( mappings, registry, expect.objectContaining({ From bde53ae357e08f9222bc2bc1dd3ab05df1e37efb Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 09:01:09 +0100 Subject: [PATCH 07/22] 169 left --- .../service/lib/repository.test.ts | 64 +++++++++++++------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 0d5db9ba7051b..9a19d67fec6c7 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -20,6 +20,7 @@ import { mockGetSearchDsl, } from './repository.test.mock'; +import type { Payload } from '@hapi/boom'; import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { SavedObjectsType, @@ -59,7 +60,6 @@ import { SavedObjectsUpdateOptions, } from '../saved_objects_client'; import { SavedObjectsMappingProperties, SavedObjectsTypeMappingDefinition } from '../../mappings'; -import { BulkResponseItem } from "@elastic/elasticsearch/lib/api/typesWithBodyKey"; const { nodeTypes } = esKuery; @@ -71,14 +71,22 @@ interface TypeIdTuple { type: string; } +interface ExpectedErrorResult { + type: string; + id: string; + error: Record; +} + +type ErrorPayload = Error & Payload; + const createBadRequestError = (reason?: string) => - SavedObjectsErrorHelpers.createBadRequestError(reason).output.payload; + SavedObjectsErrorHelpers.createBadRequestError(reason).output.payload as ErrorPayload; const createConflictError = (type: string, id: string, reason?: string) => - SavedObjectsErrorHelpers.createConflictError(type, id, reason).output.payload; + SavedObjectsErrorHelpers.createConflictError(type, id, reason).output.payload as ErrorPayload; const createGenericNotFoundError = (type: string | null = null, id: string | null = null) => - SavedObjectsErrorHelpers.createGenericNotFoundError(type, id).output.payload; + SavedObjectsErrorHelpers.createGenericNotFoundError(type, id).output.payload as ErrorPayload; const createUnsupportedTypeError = (type: string) => - SavedObjectsErrorHelpers.createUnsupportedTypeError(type).output.payload; + SavedObjectsErrorHelpers.createUnsupportedTypeError(type).output.payload as ErrorPayload; describe('SavedObjectsRepository', () => { let client: ReturnType; @@ -295,11 +303,12 @@ describe('SavedObjectsRepository', () => { id, error: expect.any(Object), }); + const expectErrorResult = ( { type, id }: TypeIdTuple, error: Record, overrides: Record = {} - ) => ({ + ): ExpectedErrorResult => ({ type, id, error: { ...error, ...overrides }, @@ -382,14 +391,14 @@ describe('SavedObjectsRepository', () => { }); }); - const obj1: SavedObject = { + const obj1: SavedObjectsBulkCreateObject = { type: 'config', id: '6.0.0-alpha1', attributes: { title: 'Test One' }, references: [{ name: 'ref_0', type: 'test', id: '1' }], originId: 'some-origin-id', // only one of the object args has an originId, this is intentional to test both a positive and negative case }; - const obj2: SavedObject = { + const obj2: SavedObjectsBulkCreateObject = { type: 'index-pattern', id: 'logstash-*', attributes: { title: 'Test Two' }, @@ -645,7 +654,8 @@ describe('SavedObjectsRepository', () => { type: o3.type, id: o3.id, existingDocument: { - _source: { namespaces: [namespace ?? 'default', 'something-else'] }, // third object does have an existing document to overwrite + _id: o3.id, + _source: { type: o3.type, namespaces: [namespace ?? 'default', 'something-else'] }, // third object does have an existing document to overwrite }, }, ]); @@ -775,7 +785,11 @@ describe('SavedObjectsRepository', () => { references: [{ name: 'ref_0', type: 'test', id: '2' }], }; - const bulkCreateError = async (obj, isBulkError, expectedErrorResult) => { + const bulkCreateError = async ( + obj: SavedObjectsBulkCreateObject, + isBulkError: boolean | undefined, + expectedErrorResult: ExpectedErrorResult + ) => { let response; if (isBulkError) { // mock the bulk error for only the second object @@ -921,7 +935,11 @@ describe('SavedObjectsRepository', () => { }); it(`returns bulk error`, async () => { - const expectedErrorResult = { type: obj3.type, id: obj3.id, error: 'Oh no, a bulk error!' }; + const expectedErrorResult = { + type: obj3.type, + id: obj3.id, + error: { error: 'Oh no, a bulk error!' }, + }; await bulkCreateError(obj3, true, expectedErrorResult); }); }); @@ -1166,12 +1184,16 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const bulkGetError = async (obj, isBulkError: boolean, expectedErrorResult) => { + const bulkGetError = async ( + obj: SavedObject, + isBulkError: boolean, + expectedErrorResult: ExpectedErrorResult + ) => { let response; if (isBulkError) { // mock the bulk error for only the second object mockGetBulkOperationError.mockReturnValueOnce(undefined); - mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error); + mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error as Payload); response = getMockMgetResponse([obj1, obj, obj2]); } else { response = getMockMgetResponse([obj1, obj2]); @@ -4182,7 +4204,12 @@ describe('SavedObjectsRepository', () => { ]; const originId = 'some-origin-id'; - const mockUpdateResponse = (type, id, options, includeOriginId) => { + const mockUpdateResponse = ( + type: string, + id: string, + options?: SavedObjectsBaseOptions, + includeOriginId?: boolean + ) => { client.update.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( { @@ -4233,7 +4260,7 @@ describe('SavedObjectsRepository', () => { beforeEach(() => { mockPreflightCheckForCreate.mockReset(); mockPreflightCheckForCreate.mockImplementation(({ objects }) => { - return objects.map(({ type, id }) => ({ type, id })); // respond with no errors by default + return Promise.resolve(objects.map(({ type, id }) => ({ type, id }))); // respond with no errors by default }); }); @@ -4276,7 +4303,7 @@ describe('SavedObjectsRepository', () => { }); it(`accepts custom references array`, async () => { - const test = async (references) => { + const test = async (references: SavedObjectReference[]) => { await updateSuccess(type, id, attributes, { references }); expect(client.update).toHaveBeenCalledWith( expect.objectContaining({ @@ -4287,7 +4314,7 @@ describe('SavedObjectsRepository', () => { client.update.mockClear(); }; await test(references); - await test(['string']); + await test([{ type: 'foo', id: '42', name: 'some ref' }]); await test([]); }); @@ -4352,7 +4379,8 @@ describe('SavedObjectsRepository', () => { }); it(`doesn't accept custom references if not an array`, async () => { - const test = async (references) => { + const test = async (references: unknown) => { + // @ts-expect-error references is unknown await updateSuccess(type, id, attributes, { references }); expect(client.update).toHaveBeenCalledWith( expect.objectContaining({ From 14793ec74dd5ffb57c3074eeb769c4a93ee27eb9 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 09:32:14 +0100 Subject: [PATCH 08/22] 150 left --- .../service/lib/repository.test.ts | 106 +++++++++++++----- 1 file changed, 79 insertions(+), 27 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 9a19d67fec6c7..0bd3cb86db474 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -29,13 +29,18 @@ import { SavedObjectsBaseOptions, SavedObjectsFindOptions, } from '../../types'; +import type { SavedObjectsUpdateObjectsSpacesResponse } from './update_objects_spaces'; import { SavedObjectsDeleteByNamespaceOptions, SavedObjectsIncrementCounterOptions, SavedObjectsRepository, } from './repository'; import { SavedObjectsErrorHelpers } from './errors'; -import { PointInTimeFinder } from './point_in_time_finder'; +import { + PointInTimeFinder, + SavedObjectsCreatePointInTimeFinderDependencies, + SavedObjectsCreatePointInTimeFinderOptions, +} from './point_in_time_finder'; import { ALL_NAMESPACES_STRING } from './utils'; import { loggerMock } from '../../../logging/logger.mock'; import { @@ -60,6 +65,12 @@ import { SavedObjectsUpdateOptions, } from '../saved_objects_client'; import { SavedObjectsMappingProperties, SavedObjectsTypeMappingDefinition } from '../../mappings'; +import { + SavedObjectsCollectMultiNamespaceReferencesObject, + SavedObjectsCollectMultiNamespaceReferencesResponse, + SavedObjectsUpdateObjectsSpacesObject, + SavedObjectsUpdateObjectsSpacesOptions, +} from 'kibana/server'; const { nodeTypes } = esKuery; @@ -4207,7 +4218,7 @@ describe('SavedObjectsRepository', () => { const mockUpdateResponse = ( type: string, id: string, - options?: SavedObjectsBaseOptions, + options?: SavedObjectsUpdateOptions, includeOriginId?: boolean ) => { client.update.mockResolvedValueOnce( @@ -4227,7 +4238,7 @@ describe('SavedObjectsRepository', () => { ...(includeOriginId && { originId }), }, }, - }, + } as estypes.UpdateResponse, { statusCode: 200 } ) ); @@ -4238,7 +4249,10 @@ describe('SavedObjectsRepository', () => { id: string, attributes: T, options?: SavedObjectsUpdateOptions, - internalOptions = {} + internalOptions: { + includeOriginId?: boolean; + mockGetResponseValue?: estypes.GetResponse; + } = {} ) => { const { mockGetResponseValue, includeOriginId } = internalOptions; if (registry.isMultiNamespace(type)) { @@ -4285,7 +4299,7 @@ describe('SavedObjectsRepository', () => { id, attributes, { upsert: true }, - { mockGetResponseValue: { found: false } } + { mockGetResponseValue: { found: false } as estypes.GetResponse } ); expect(client.get).toHaveBeenCalledTimes(1); expect(mockPreflightCheckForCreate).toHaveBeenCalledTimes(1); @@ -4515,7 +4529,10 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }, undefined) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { found: false } as estypes.GetResponse, + undefined + ) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -4523,7 +4540,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + elasticsearchClientMock.createSuccessTransportRequestPromise({} as estypes.GetResponse, { + statusCode: 404, + }) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -4540,11 +4559,25 @@ describe('SavedObjectsRepository', () => { it(`throws when there is an alias conflict from preflightCheckForCreate`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + found: false, + } as estypes.GetResponse) ); - mockPreflightCheckForCreate.mockResolvedValue([{ error: { type: 'aliasConflict' } }]); + mockPreflightCheckForCreate.mockResolvedValue([ + { type: 'type', id: 'id', error: { type: 'aliasConflict' } }, + ]); await expect( - savedObjectsRepository.update(MULTI_NAMESPACE_ISOLATED_TYPE, id, {}, { upsert: true }) + savedObjectsRepository.update( + MULTI_NAMESPACE_ISOLATED_TYPE, + id, + { attr: 'value' }, + { + upsert: { + upsertAttr: 'val', + attr: 'value', + }, + } + ) ).rejects.toThrowError(createConflictError(MULTI_NAMESPACE_ISOLATED_TYPE, id)); expect(client.get).toHaveBeenCalledTimes(1); expect(mockPreflightCheckForCreate).toHaveBeenCalledTimes(1); @@ -4552,13 +4585,15 @@ describe('SavedObjectsRepository', () => { }); it(`does not throw when there is a different error from preflightCheckForCreate`, async () => { - mockPreflightCheckForCreate.mockResolvedValue([{ error: { type: 'something-else' } }]); + mockPreflightCheckForCreate.mockResolvedValue([ + { type: 'type', id: 'id', error: { type: 'conflict' } }, + ]); await updateSuccess( MULTI_NAMESPACE_ISOLATED_TYPE, id, attributes, { upsert: true }, - { mockGetResponseValue: { found: false } } + { mockGetResponseValue: { found: false } as estypes.GetResponse } ); expect(client.get).toHaveBeenCalledTimes(1); expect(mockPreflightCheckForCreate).toHaveBeenCalledTimes(1); @@ -4621,7 +4656,7 @@ describe('SavedObjectsRepository', () => { describe('#openPointInTimeForType', () => { const type = 'index-pattern'; - const generateResults = (id?: string) => ({ id: id || null }); + const generateResults = (id?: string) => ({ id: id || 'id' }); const successResponse = async (type: string, options?: SavedObjectsOpenPointInTimeOptions) => { client.openPointInTime.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(generateResults()) @@ -4677,7 +4712,10 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index`, async () => { client.openPointInTime.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { id: 'error' }, + { statusCode: 404 } + ) ); await expectNotFoundError(type); expect(client.openPointInTime).toHaveBeenCalledTimes(1); @@ -4752,17 +4790,19 @@ describe('SavedObjectsRepository', () => { describe('#createPointInTimeFinder', () => { it('returns a new PointInTimeFinder instance', async () => { - const result = await savedObjectsRepository.createPointInTimeFinder({}, {}); + const result = await savedObjectsRepository.createPointInTimeFinder({ type: 'PIT' }); expect(result).toBeInstanceOf(PointInTimeFinder); }); it('calls PointInTimeFinder with the provided options and dependencies', async () => { - const options = Symbol(); - const dependencies = { + const options: SavedObjectsCreatePointInTimeFinderOptions = { + type: 'my-type', + }; + const dependencies: SavedObjectsCreatePointInTimeFinderDependencies = { client: { - find: Symbol(), - openPointInTimeForType: Symbol(), - closePointInTime: Symbol(), + find: jest.fn(), + openPointInTimeForType: jest.fn(), + closePointInTime: jest.fn(), }, }; @@ -4783,8 +4823,12 @@ describe('SavedObjectsRepository', () => { }); it('passes arguments to the collectMultiNamespaceReferences module and returns the result', async () => { - const objects = Symbol(); - const expectedResult = Symbol(); + const objects: SavedObjectsCollectMultiNamespaceReferencesObject[] = [ + { type: 'foo', id: 'bar' }, + ]; + const expectedResult: SavedObjectsCollectMultiNamespaceReferencesResponse = { + objects: [{ type: 'foo', id: 'bar', spaces: ['ns-1'], inboundReferences: [] }], + }; mockCollectMultiNamespaceReferences.mockResolvedValue(expectedResult); await expect( @@ -4812,11 +4856,19 @@ describe('SavedObjectsRepository', () => { }); it('passes arguments to the updateObjectsSpaces module and returns the result', async () => { - const objects = Symbol(); - const spacesToAdd = Symbol(); - const spacesToRemove = Symbol(); - const options = Symbol(); - const expectedResult = Symbol(); + const objects: SavedObjectsUpdateObjectsSpacesObject[] = [{ type: 'type', id: 'id' }]; + const spacesToAdd = ['to-add', 'also-to-add']; + const spacesToRemove = ['to-remove']; + const options: SavedObjectsUpdateObjectsSpacesOptions = { namespace: 'ns-1' }; + const expectedResult: SavedObjectsUpdateObjectsSpacesResponse = { + objects: [ + { + type: 'type', + id: 'id', + spaces: ['foo', 'bar'], + }, + ], + }; mockUpdateObjectsSpaces.mockResolvedValue(expectedResult); await expect( From aa5f959d448da5bc11ebab386e368ded64fa84e1 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 09:55:21 +0100 Subject: [PATCH 09/22] 132 left --- .../service/lib/repository.test.ts | 63 +++++++++++++------ 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 0bd3cb86db474..bd1e9758a452c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -32,6 +32,7 @@ import { import type { SavedObjectsUpdateObjectsSpacesResponse } from './update_objects_spaces'; import { SavedObjectsDeleteByNamespaceOptions, + SavedObjectsIncrementCounterField, SavedObjectsIncrementCounterOptions, SavedObjectsRepository, } from './repository'; @@ -62,6 +63,7 @@ import { SavedObjectsCreateOptions, SavedObjectsDeleteOptions, SavedObjectsOpenPointInTimeOptions, + SavedObjectsResolveResponse, SavedObjectsUpdateOptions, } from '../saved_objects_client'; import { SavedObjectsMappingProperties, SavedObjectsTypeMappingDefinition } from '../../mappings'; @@ -71,6 +73,7 @@ import { SavedObjectsUpdateObjectsSpacesObject, SavedObjectsUpdateObjectsSpacesOptions, } from 'kibana/server'; +import { InternalBulkResolveError } from "./internal_bulk_resolve"; const { nodeTypes } = esKuery; @@ -3707,7 +3710,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + found: false, + } as estypes.GetResponse) ); await expectNotFoundError(type, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -3715,7 +3720,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + elasticsearchClientMock.createSuccessTransportRequestPromise({} as estypes.GetResponse, { + statusCode: 404, + }) ); await expectNotFoundError(type, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -3776,7 +3783,10 @@ describe('SavedObjectsRepository', () => { }); it('passes arguments to the internalBulkResolve module and returns the result', async () => { - const expectedResult = { saved_object: 'mock-object', outcome: 'exactMatch' }; + const expectedResult: SavedObjectsResolveResponse = { + saved_object: { type: 'type', id: 'id', attributes: {}, references: [] }, + outcome: 'exactMatch', + }; mockInternalBulkResolve.mockResolvedValue({ resolved_objects: [expectedResult] }); await expect(savedObjectsRepository.resolve('obj-type', 'obj-id')).resolves.toEqual( @@ -3789,8 +3799,8 @@ describe('SavedObjectsRepository', () => { }); it('throws when internalBulkResolve result is an error', async () => { - const error = new Error('Oh no!'); - const expectedResult = { type: 'obj-type', id: 'obj-id', error }; + const error = SavedObjectsErrorHelpers.decorateBadRequestError(new Error('Oh no!')); + const expectedResult: InternalBulkResolveError = { type: 'obj-type', id: 'obj-id', error }; mockInternalBulkResolve.mockResolvedValue({ resolved_objects: [expectedResult] }); await expect(savedObjectsRepository.resolve('foo', '2')).rejects.toEqual(error); @@ -3814,9 +3824,9 @@ describe('SavedObjectsRepository', () => { const incrementCounterSuccess = async ( type: string, id: string, - fields: string[], + fields: Array, options?: SavedObjectsIncrementCounterOptions, - internalOptions = {} + internalOptions: { mockGetResponseValue?: estypes.GetResponse } = {} ) => { const { mockGetResponseValue } = internalOptions; const isMultiNamespace = registry.isMultiNamespace(type); @@ -3839,14 +3849,14 @@ describe('SavedObjectsRepository', () => { ...mockTimestampFields, [type]: { ...fields.reduce((acc, field) => { - acc[field] = 8468; + acc[typeof field === 'string' ? field : field.fieldName] = 8468; return acc; - }, {}), + }, {} as Record), defaultIndex: 'logstash-*', }, }, }, - }) + } as estypes.UpdateResponse) ); const result = await savedObjectsRepository.incrementCounter(type, id, fields, options); @@ -3857,7 +3867,7 @@ describe('SavedObjectsRepository', () => { beforeEach(() => { mockPreflightCheckForCreate.mockReset(); mockPreflightCheckForCreate.mockImplementation(({ objects }) => { - return objects.map(({ type, id }) => ({ type, id })); // respond with no errors by default + return Promise.resolve(objects.map(({ type, id }) => ({ type, id }))); // respond with no errors by default }); }); @@ -3884,7 +3894,7 @@ describe('SavedObjectsRepository', () => { id, counterFields, { namespace }, - { mockGetResponseValue: { found: false } } + { mockGetResponseValue: { found: false } as estypes.GetResponse } ); expect(client.get).toHaveBeenCalledTimes(1); expect(mockPreflightCheckForCreate).toHaveBeenCalledTimes(1); @@ -3981,7 +3991,11 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { - const expectUnsupportedTypeError = async (type, id, field) => { + const expectUnsupportedTypeError = async ( + type: string, + id: string, + field: Array + ) => { await expect(savedObjectsRepository.incrementCounter(type, id, field)).rejects.toThrowError( createUnsupportedTypeError(type) ); @@ -3996,8 +4010,9 @@ describe('SavedObjectsRepository', () => { }); it(`throws when type is not a string`, async () => { - const test = async (type) => { + const test = async (type: unknown) => { await expect( + // @ts-expect-error type is supposed to be a string savedObjectsRepository.incrementCounter(type, id, counterFields) ).rejects.toThrowError(`"type" argument must be a string`); expect(client.update).not.toHaveBeenCalled(); @@ -4063,9 +4078,13 @@ describe('SavedObjectsRepository', () => { it(`throws when there is an alias conflict from preflightCheckForCreate`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + found: false, + } as estypes.GetResponse) ); - mockPreflightCheckForCreate.mockResolvedValue([{ error: { type: 'aliasConflict' } }]); + mockPreflightCheckForCreate.mockResolvedValue([ + { type: 'foo', id: 'bar', error: { type: 'aliasConflict' } }, + ]); await expect( savedObjectsRepository.incrementCounter( MULTI_NAMESPACE_ISOLATED_TYPE, @@ -4081,15 +4100,19 @@ describe('SavedObjectsRepository', () => { it(`does not throw when there is a different error from preflightCheckForCreate`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + found: false, + } as estypes.GetResponse) ); - mockPreflightCheckForCreate.mockResolvedValue([{ error: { type: 'something-else' } }]); + mockPreflightCheckForCreate.mockResolvedValue([ + { type: 'foo', id: 'bar', error: { type: 'conflict' } }, + ]); await incrementCounterSuccess( MULTI_NAMESPACE_ISOLATED_TYPE, id, counterFields, { namespace }, - { mockGetResponseValue: { found: false } } + { mockGetResponseValue: { found: false } as estypes.GetResponse } ); expect(client.get).toHaveBeenCalledTimes(1); expect(mockPreflightCheckForCreate).toHaveBeenCalledTimes(1); @@ -4134,7 +4157,7 @@ describe('SavedObjectsRepository', () => { originId, }, }, - }) + } as estypes.UpdateResponse) ); const response = await savedObjectsRepository.incrementCounter( From f8d0b74705df898aba3260bb6e75d3a3a556249b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 10:00:39 +0100 Subject: [PATCH 10/22] fixed `find` violations, 117 left --- .../service/lib/repository.test.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index bd1e9758a452c..940eb5c6f8259 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -3234,6 +3234,7 @@ describe('SavedObjectsRepository', () => { describe('errors', () => { it(`throws when type is not defined`, async () => { + // @ts-expect-error type should be defined await expect(savedObjectsRepository.find({})).rejects.toThrowError( 'options.type must be a string or an array of strings' ); @@ -3257,7 +3258,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when type is not an empty array and typeToNamespacesMap is defined`, async () => { - const test = async (args) => { + const test = async (args: SavedObjectsFindOptions) => { await expect(savedObjectsRepository.find(args)).rejects.toThrowError( 'options.namespaces must be an empty array when options.typeToNamespacesMap is used' ); @@ -3332,14 +3333,14 @@ describe('SavedObjectsRepository', () => { noNamespaceSearchResults.hits.hits.forEach((doc, i) => { expect(response.saved_objects[i]).toEqual({ id: doc._id.replace(/(index-pattern|config|globalType)\:/, ''), - type: doc._source.type, - originId: doc._source.originId, + type: doc._source!.type, + originId: doc._source!.originId, ...mockTimestampFields, version: mockVersion, score: doc._score, - attributes: doc._source[doc._source.type], + attributes: doc._source![doc._source!.type], references: [], - namespaces: doc._source.type === NAMESPACE_AGNOSTIC_TYPE ? undefined : ['default'], + namespaces: doc._source!.type === NAMESPACE_AGNOSTIC_TYPE ? undefined : ['default'], }); }); }); @@ -3359,20 +3360,20 @@ describe('SavedObjectsRepository', () => { namespacedSearchResults.hits.hits.forEach((doc, i) => { expect(response.saved_objects[i]).toEqual({ id: doc._id.replace(/(foo-namespace\:)?(index-pattern|config|globalType)\:/, ''), - type: doc._source.type, - originId: doc._source.originId, + type: doc._source!.type, + originId: doc._source!.originId, ...mockTimestampFields, version: mockVersion, score: doc._score, - attributes: doc._source[doc._source.type], + attributes: doc._source![doc._source!.type], references: [], - namespaces: doc._source.type === NAMESPACE_AGNOSTIC_TYPE ? undefined : [namespace], + namespaces: doc._source!.type === NAMESPACE_AGNOSTIC_TYPE ? undefined : [namespace], }); }); }); it(`should return empty results when attempting to find only invalid or hidden types`, async () => { - const test = async (types) => { + const test = async (types: string | string[]) => { const result = await savedObjectsRepository.find({ type: types }); expect(result).toEqual(expect.objectContaining({ saved_objects: [] })); expect(client.search).not.toHaveBeenCalled(); @@ -3384,7 +3385,7 @@ describe('SavedObjectsRepository', () => { }); it(`should return empty results when attempting to find only invalid or hidden types using typeToNamespacesMap`, async () => { - const test = async (types) => { + const test = async (types: string[]) => { const result = await savedObjectsRepository.find({ typeToNamespacesMap: new Map(types.map((x) => [x, undefined])), type: '', From 1bf858a8822a2be2028675b46c0c96de11da977d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 10:03:45 +0100 Subject: [PATCH 11/22] fixed `removeReferencesTo` violations, 114 left --- .../saved_objects/service/lib/repository.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 940eb5c6f8259..bb864c08bf81c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -73,7 +73,7 @@ import { SavedObjectsUpdateObjectsSpacesObject, SavedObjectsUpdateObjectsSpacesOptions, } from 'kibana/server'; -import { InternalBulkResolveError } from "./internal_bulk_resolve"; +import { InternalBulkResolveError } from './internal_bulk_resolve'; const { nodeTypes } = esKuery; @@ -2869,7 +2869,8 @@ describe('SavedObjectsRepository', () => { describe('errors', () => { it(`throws when namespace is not a string or is '*'`, async () => { - const test = async (namespace) => { + const test = async (namespace: unknown) => { + // @ts-expect-error namespace is unknown await expect(savedObjectsRepository.deleteByNamespace(namespace)).rejects.toThrowError( `namespace is required, and must be a string` ); @@ -3046,7 +3047,10 @@ describe('SavedObjectsRepository', () => { client.updateByQuery.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise({ updated: 7, - failures: ['failure', 'another-failure'], + failures: [ + { id: 'failure' } as estypes.BulkIndexByScrollFailure, + { id: 'another-failure' } as estypes.BulkIndexByScrollFailure, + ], }) ); From 6039ac02a8030b41c5e346d070fc89cb4911f907 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 10:22:21 +0100 Subject: [PATCH 12/22] fixed `delete` violations, 105 left --- .../service/lib/repository.test.ts | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index bb864c08bf81c..2b4f861074c63 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -291,8 +291,8 @@ describe('SavedObjectsRepository', () => { references, specialProperty: 'specialValue', ...mockTimestampFields, - }, - } as estypes.GetResponse; + } as SavedObjectsRawDocSource, + } as estypes.GetResponse; }; const getMockMgetResponse = (objects, namespace?: string) => ({ @@ -2544,7 +2544,7 @@ describe('SavedObjectsRepository', () => { type: string, id: string, options?: SavedObjectsDeleteOptions, - internalOptions = {} + internalOptions: { mockGetResponseValue?: estypes.GetResponse } = {} ) => { const { mockGetResponseValue } = internalOptions; if (registry.isMultiNamespace(type)) { @@ -2555,7 +2555,9 @@ describe('SavedObjectsRepository', () => { ); } client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'deleted' }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + result: 'deleted', + } as estypes.DeleteResponse) ); const result = await savedObjectsRepository.delete(type, id, options); expect(client.get).toHaveBeenCalledTimes(registry.isMultiNamespace(type) ? 1 : 0); @@ -2686,7 +2688,9 @@ describe('SavedObjectsRepository', () => { ) ); client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'deleted' }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + result: 'deleted', + } as estypes.DeleteResponse) ); mockDeleteLegacyUrlAliases.mockRejectedValueOnce(new Error('Oh no!')); await savedObjectsRepository.delete(MULTI_NAMESPACE_ISOLATED_TYPE, id, { namespace }); @@ -2727,7 +2731,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }, undefined) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + found: false, + } as estypes.GetResponse) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -2735,7 +2741,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + elasticsearchClientMock.createSuccessTransportRequestPromise({} as estypes.GetResponse, { + statusCode: 404, + }) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -2754,7 +2762,7 @@ describe('SavedObjectsRepository', () => { it(`throws when the type is multi-namespace and the document has multiple namespaces and the force option is not enabled`, async () => { const response = getMockGetResponse({ type: MULTI_NAMESPACE_ISOLATED_TYPE, id, namespace }); - response._source.namespaces = [namespace, 'bar-namespace']; + response._source!.namespaces = [namespace, 'bar-namespace']; client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -2768,7 +2776,7 @@ describe('SavedObjectsRepository', () => { it(`throws when the type is multi-namespace and the document has all namespaces and the force option is not enabled`, async () => { const response = getMockGetResponse({ type: MULTI_NAMESPACE_ISOLATED_TYPE, id, namespace }); - response._source.namespaces = ['*']; + response._source!.namespaces = ['*']; client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -2782,7 +2790,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during delete`, async () => { client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' }) + elasticsearchClientMock.createSuccessTransportRequestPromise({ + result: 'not_found', + } as estypes.DeleteResponse) ); await expectNotFoundError(type, id); expect(client.delete).toHaveBeenCalledTimes(1); @@ -2791,8 +2801,9 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during delete`, async () => { client.delete.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise({ + // @elastic/elasticsearch doesn't declare error on DeleteResponse error: { type: 'index_not_found_exception' }, - }) + } as unknown as estypes.DeleteResponse) ); await expectNotFoundError(type, id); expect(client.delete).toHaveBeenCalledTimes(1); @@ -2801,8 +2812,8 @@ describe('SavedObjectsRepository', () => { it(`throws when ES returns an unexpected response`, async () => { client.delete.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise({ - result: 'something unexpected', - }) + result: 'something unexpected' as estypes.Result, + } as estypes.DeleteResponse) ); await expect(savedObjectsRepository.delete(type, id)).rejects.toThrowError( 'Unexpected Elasticsearch DELETE response' From 95ad9dfc5c88597d33f7473eca780cd6b770eebc Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 10:35:03 +0100 Subject: [PATCH 13/22] fixed `create` violations, 93 left --- .../service/lib/repository.test.ts | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 2b4f861074c63..466a855f00db2 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -2025,13 +2025,13 @@ describe('SavedObjectsRepository', () => { beforeEach(() => { mockPreflightCheckForCreate.mockReset(); mockPreflightCheckForCreate.mockImplementation(({ objects }) => { - return objects.map(({ type, id }) => ({ type, id })); // respond with no errors by default + return Promise.resolve(objects.map(({ type, id }) => ({ type, id }))); // respond with no errors by default }); client.create.mockImplementation((params) => elasticsearchClientMock.createSuccessTransportRequestPromise({ _id: params.id, ...mockVersionProps, - }) + } as estypes.CreateResponse) ); }); @@ -2113,24 +2113,34 @@ describe('SavedObjectsRepository', () => { it(`defaults to empty references array`, async () => { await createSuccess(type, attributes, { id }); - expect(client.create.mock.calls[0][0].body.references).toEqual([]); + expect( + (client.create.mock.calls[0][0] as estypes.CreateRequest).body! + .references + ).toEqual([]); }); it(`accepts custom references array`, async () => { - const test = async (references) => { + const test = async (references: SavedObjectReference[]) => { await createSuccess(type, attributes, { id, references }); - expect(client.create.mock.calls[0][0].body.references).toEqual(references); + expect( + (client.create.mock.calls[0][0] as estypes.CreateRequest) + .body!.references + ).toEqual(references); client.create.mockClear(); }; await test(references); - await test(['string']); + await test([{ type: 'type', id: 'id', name: 'some ref' }]); await test([]); }); it(`doesn't accept custom references if not an array`, async () => { - const test = async (references) => { + const test = async (references: unknown) => { + // @ts-expect-error references is unknown await createSuccess(type, attributes, { id, references }); - expect(client.create.mock.calls[0][0].body.references).not.toBeDefined(); + expect( + (client.create.mock.calls[0][0] as estypes.CreateRequest) + .body!.references + ).not.toBeDefined(); client.create.mockClear(); }; await test('string'); @@ -2233,7 +2243,10 @@ describe('SavedObjectsRepository', () => { { type: MULTI_NAMESPACE_TYPE, id, - existingDocument: { _source: { namespaces: ['*'] } }, // second object does have an existing document to overwrite + existingDocument: { + _id: id, + _source: { type: MULTI_NAMESPACE_TYPE, namespaces: ['*'] }, + }, // second object does have an existing document to overwrite }, ]); await createSuccess(MULTI_NAMESPACE_ISOLATED_TYPE, attributes, { @@ -2297,7 +2310,10 @@ describe('SavedObjectsRepository', () => { { type: MULTI_NAMESPACE_ISOLATED_TYPE, id, - existingDocument: { _source: { namespaces: ['something-else'] } }, // third object does have an existing document to overwrite + existingDocument: { + _id: id, + _source: { type: MULTI_NAMESPACE_ISOLATED_TYPE, namespaces: ['something-else'] }, + }, // third object does have an existing document to overwrite }, ]); await savedObjectsRepository.create(MULTI_NAMESPACE_ISOLATED_TYPE, attributes, { @@ -2403,7 +2419,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws when options.initialNamespaces is used with a space-isolated object and does not specify a single space`, async () => { - const doTest = async (objType, initialNamespaces) => { + const doTest = async (objType: string, initialNamespaces?: string[]) => { await expect( savedObjectsRepository.create(objType, attributes, { initialNamespaces }) ).rejects.toThrowError( From 2ae62c5ada26a54aa70398e5f2f180b34c26d6f7 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 10:40:45 +0100 Subject: [PATCH 14/22] fixed `checkConflicts` violations, 89 left --- src/core/server/saved_objects/service/lib/repository.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 466a855f00db2..465a637a14e22 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -1914,7 +1914,7 @@ describe('SavedObjectsRepository', () => { }; const _expectClientCallArgs = ( - objects, + objects: TypeIdTuple[], { _index = expect.any(String), getId = () => expect.any(String), @@ -1981,7 +1981,6 @@ describe('SavedObjectsRepository', () => { const hiddenTypeObj = { type: HIDDEN_TYPE, id: 'three' }; const objects = [unknownTypeObj, hiddenTypeObj, obj1, obj2, obj3, obj4, obj5, obj6, obj7]; const response = { - status: 200, docs: [ getMockGetResponse(obj1), { found: false }, @@ -1991,7 +1990,7 @@ describe('SavedObjectsRepository', () => { getMockGetResponse(obj6), { found: false }, ], - }; + } as estypes.MgetResponse; client.mget.mockResolvedValue( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); From f114b9f2b38f580125e9eaea50c4c37283e5a99d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 11:05:59 +0100 Subject: [PATCH 15/22] fixed most `bulkUpdate` violations, 60 left --- .../service/lib/repository.test.ts | 96 ++++++++++++------- 1 file changed, 62 insertions(+), 34 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 465a637a14e22..1ed39ca3ff0d5 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -59,6 +59,7 @@ import * as esKuery from '@kbn/es-query'; import { errors as EsErrors } from '@elastic/elasticsearch'; import { SavedObjectsBulkCreateObject, + SavedObjectsBulkUpdateObject, SavedObjectsBulkUpdateOptions, SavedObjectsCreateOptions, SavedObjectsDeleteOptions, @@ -1411,12 +1412,12 @@ describe('SavedObjectsRepository', () => { }); describe('#bulkUpdate', () => { - const obj1 = { + const obj1: SavedObjectsBulkUpdateObject = { type: 'config', id: '6.0.0-alpha1', attributes: { title: 'Test One' }, }; - const obj2 = { + const obj2: SavedObjectsBulkUpdateObject = { type: 'index-pattern', id: 'logstash-*', attributes: { title: 'Test Two' }, @@ -1429,27 +1430,28 @@ describe('SavedObjectsRepository', () => { objects: TypeIdTuple[], options?: SavedObjectsBulkUpdateOptions, includeOriginId?: boolean - ) => ({ - items: objects.map(({ type, id }) => ({ - update: { - _id: `${ - registry.isSingleNamespace(type) && options?.namespace ? `${options?.namespace}:` : '' - }${type}:${id}`, - ...mockVersionProps, - get: { - _source: { - // "includeOriginId" is not an option for the operation; however, if the existing saved object contains an originId attribute, the - // operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response. - ...(includeOriginId && { originId }), + ) => + ({ + items: objects.map(({ type, id }) => ({ + update: { + _id: `${ + registry.isSingleNamespace(type) && options?.namespace ? `${options?.namespace}:` : '' + }${type}:${id}`, + ...mockVersionProps, + get: { + _source: { + // "includeOriginId" is not an option for the operation; however, if the existing saved object contains an originId attribute, the + // operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response. + ...(includeOriginId && { originId }), + }, }, + result: 'updated', }, - result: 'updated', - }, - })), - }); + })), + } as estypes.BulkResponse); const bulkUpdateSuccess = async ( - objects, + objects: SavedObjectsBulkUpdateObject[], options?: SavedObjectsBulkUpdateOptions, includeOriginId?: boolean ) => { @@ -1501,7 +1503,7 @@ describe('SavedObjectsRepository', () => { ); }; - const expectObjArgs = ({ type, attributes }) => [ + const expectObjArgs = ({ type, attributes }: { type: string; attributes: unknown }) => [ expect.any(Object), { doc: expect.objectContaining({ @@ -1568,7 +1570,7 @@ describe('SavedObjectsRepository', () => { }); it(`accepts custom references array`, async () => { - const test = async (references) => { + const test = async (references: SavedObjectReference[]) => { const objects = [obj1, obj2].map((obj) => ({ ...obj, references })); await bulkUpdateSuccess(objects); const expected = { doc: expect.objectContaining({ references }) }; @@ -1580,13 +1582,14 @@ describe('SavedObjectsRepository', () => { client.bulk.mockClear(); }; await test(references); - await test(['string']); + await test([{ type: 'type', id: 'id', name: 'some ref' }]); await test([]); }); it(`doesn't accept custom references if not an array`, async () => { const test = async (references: unknown) => { const objects = [obj1, obj2].map((obj) => ({ ...obj, references })); + // @ts-expect-error references is unknown await bulkUpdateSuccess(objects); const expected = { doc: expect.not.objectContaining({ references: expect.anything() }) }; const body = [expect.any(Object), expected, expect.any(Object), expected]; @@ -1643,7 +1646,7 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const getId = (type: string, id: string) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkUpdateSuccess([obj1, obj2], { namespace }); expectClientCallArgsAction([obj1, obj2], { method: 'update', getId }); @@ -1694,7 +1697,7 @@ describe('SavedObjectsRepository', () => { expectClientCallArgsAction([_obj1], { method: 'update', getId }); client.bulk.mockClear(); await bulkUpdateSuccess([_obj2], { namespace }); - expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }, 2); + expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }); jest.clearAllMocks(); // test again with object namespace string that supersedes the operation's namespace ID @@ -1702,7 +1705,7 @@ describe('SavedObjectsRepository', () => { expectClientCallArgsAction([_obj1], { method: 'update', getId }); client.bulk.mockClear(); await bulkUpdateSuccess([{ ..._obj2, namespace }]); - expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }, 2); + expectClientCallArgsAction([_obj2], { method: 'update', getId, overrides }); }); }); @@ -1711,18 +1714,23 @@ describe('SavedObjectsRepository', () => { mockGetBulkOperationError.mockReset(); }); - const obj = { + const obj: SavedObjectsBulkUpdateObject = { type: 'dashboard', id: 'three', + attributes: {}, }; - const bulkUpdateError = async (obj, isBulkError, expectedErrorResult) => { + const bulkUpdateError = async ( + obj: SavedObjectsBulkUpdateObject, + isBulkError: boolean, + expectedErrorResult: ExpectedErrorResult + ) => { const objects = [obj1, obj, obj2]; const mockResponse = getMockBulkUpdateResponse(objects); if (isBulkError) { // mock the bulk error for only the second object mockGetBulkOperationError.mockReturnValueOnce(undefined); - mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error); + mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error as Payload); } client.bulk.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(mockResponse) @@ -1741,14 +1749,18 @@ describe('SavedObjectsRepository', () => { }); }; - const bulkUpdateMultiError = async ([obj1, _obj, obj2], options, mgetResponse) => { + const bulkUpdateMultiError = async ( + [obj1, _obj, obj2]: SavedObjectsBulkUpdateObject[], + options: SavedObjectsBulkUpdateOptions | undefined, + mgetResponse: estypes.MgetResponse + ) => { client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(mgetResponse, { statusCode: mgetResponse.statusCode, }) ); - const bulkResponse = getMockBulkUpdateResponse([obj1, obj2], namespace); + const bulkResponse = getMockBulkUpdateResponse([obj1, obj2], { namespace }); client.bulk.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(bulkResponse) ); @@ -1817,7 +1829,14 @@ describe('SavedObjectsRepository', () => { }); describe('returns', () => { - const expectSuccessResult = ({ type, id, attributes, references, namespaces, originId }) => ({ + const expectSuccessResult = ({ + type, + id, + attributes, + references, + namespaces, + originId, + }: SavedObjectsBulkUpdateObject) => ({ type, id, originId, @@ -1844,9 +1863,10 @@ describe('SavedObjectsRepository', () => { }); it(`handles a mix of successful updates and errors`, async () => { - const obj = { + const obj: SavedObjectsBulkUpdateObject = { type: 'unknownType', id: 'three', + attributes: {}, }; const objects = [obj1, obj, obj2]; const mockResponse = getMockBulkUpdateResponse(objects); @@ -1862,7 +1882,11 @@ describe('SavedObjectsRepository', () => { }); it(`includes namespaces property for single-namespace and multi-namespace documents`, async () => { - const obj = { type: MULTI_NAMESPACE_ISOLATED_TYPE, id: 'three' }; + const obj: SavedObjectsBulkUpdateObject = { + type: MULTI_NAMESPACE_ISOLATED_TYPE, + id: 'three', + attributes: {}, + }; const result = await bulkUpdateSuccess([obj1, obj]); expect(result).toEqual({ saved_objects: [ @@ -1873,7 +1897,11 @@ describe('SavedObjectsRepository', () => { }); it(`includes originId property if present in cluster call response`, async () => { - const obj = { type: MULTI_NAMESPACE_ISOLATED_TYPE, id: 'three' }; + const obj: SavedObjectsBulkUpdateObject = { + type: MULTI_NAMESPACE_ISOLATED_TYPE, + id: 'three', + attributes: {}, + }; const result = await bulkUpdateSuccess([obj1, obj], {}, true); expect(result).toEqual({ saved_objects: [ From c70b54b70c852acd9248509a06627945bdacf1b7 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 11:28:09 +0100 Subject: [PATCH 16/22] fixed remaining `bulkUpdate` violations, 54 left --- .../service/lib/repository.test.ts | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 1ed39ca3ff0d5..bb6cca3d53b5f 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -1642,7 +1642,7 @@ describe('SavedObjectsRepository', () => { ]; await bulkUpdateSuccess(objects); const overrides = { if_seq_no: 100, if_primary_term: 200 }; - expectClientCallArgsAction(objects, { method: 'update', overrides }, 2); + expectClientCallArgsAction(objects, { method: 'update', overrides }); }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { @@ -1752,11 +1752,12 @@ describe('SavedObjectsRepository', () => { const bulkUpdateMultiError = async ( [obj1, _obj, obj2]: SavedObjectsBulkUpdateObject[], options: SavedObjectsBulkUpdateOptions | undefined, - mgetResponse: estypes.MgetResponse + mgetResponse: estypes.MgetResponse, + mgetOptions?: { statusCode?: number } ) => { client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(mgetResponse, { - statusCode: mgetResponse.statusCode, + statusCode: mgetOptions?.statusCode, }) ); @@ -1812,8 +1813,9 @@ describe('SavedObjectsRepository', () => { it(`returns error when ES is unable to find the index (mget)`, async () => { const _obj = { ...obj, type: MULTI_NAMESPACE_ISOLATED_TYPE }; - const mgetResponse = { statusCode: 404 }; - await bulkUpdateMultiError([obj1, _obj, obj2], { namespace }, mgetResponse); + await bulkUpdateMultiError([obj1, _obj, obj2], { namespace }, {} as estypes.MgetResponse, { + statusCode: 404, + }); }); it(`returns error when there is a conflict with an existing multi-namespace saved object (mget)`, async () => { @@ -1823,7 +1825,11 @@ describe('SavedObjectsRepository', () => { }); it(`returns bulk error`, async () => { - const expectedErrorResult = { type: obj.type, id: obj.id, error: 'Oh no, a bulk error!' }; + const expectedErrorResult = { + type: obj.type, + id: obj.id, + error: { message: 'Oh no, a bulk error!' }, + }; await bulkUpdateError(obj, true, expectedErrorResult); }); }); @@ -1834,16 +1840,13 @@ describe('SavedObjectsRepository', () => { id, attributes, references, - namespaces, - originId, }: SavedObjectsBulkUpdateObject) => ({ type, id, - originId, attributes, references, version: mockVersion, - namespaces: namespaces ?? ['default'], + namespaces: ['default'], ...mockTimestampFields, }); From 7515f6878c23b128ccdf3a0ef02115f82a1e7488 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 11:31:46 +0100 Subject: [PATCH 17/22] fixed `bulkResolve` violations, 52 left --- .../server/saved_objects/service/lib/repository.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index bb6cca3d53b5f..dab2e8caa287b 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -1366,7 +1366,10 @@ describe('SavedObjectsRepository', () => { it('passes arguments to the internalBulkResolve module and returns the expected results', async () => { mockInternalBulkResolve.mockResolvedValue({ resolved_objects: [ - { saved_object: 'mock-object', outcome: 'exactMatch' }, + { + saved_object: { type: 'mock', id: 'mock-object', attributes: {}, references: [] }, + outcome: 'exactMatch', + }, { type: 'obj-type', id: 'obj-id-2', @@ -1407,7 +1410,7 @@ describe('SavedObjectsRepository', () => { const error = new Error('Oh no!'); mockInternalBulkResolve.mockRejectedValue(error); - await expect(savedObjectsRepository.resolve()).rejects.toEqual(error); + await expect(savedObjectsRepository.resolve('some-type', 'some-id')).rejects.toEqual(error); }); }); From e8a55f2c79c6fde2fc52c7685795a7258f8fdf1e Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 11:49:28 +0100 Subject: [PATCH 18/22] fixed `bulkGet` violations, 37 left --- .../service/lib/repository.test.ts | 82 +++++++++++++------ 1 file changed, 56 insertions(+), 26 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index dab2e8caa287b..3816cd51a7f8c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -59,6 +59,7 @@ import * as esKuery from '@kbn/es-query'; import { errors as EsErrors } from '@elastic/elasticsearch'; import { SavedObjectsBulkCreateObject, + SavedObjectsBulkGetObject, SavedObjectsBulkUpdateObject, SavedObjectsBulkUpdateOptions, SavedObjectsCreateOptions, @@ -296,11 +297,15 @@ describe('SavedObjectsRepository', () => { } as estypes.GetResponse; }; - const getMockMgetResponse = (objects, namespace?: string) => ({ - docs: objects.map((obj) => - obj.found === false ? obj : getMockGetResponse(obj, obj.initialNamespaces ?? namespace) - ), - }); + const getMockMgetResponse = ( + objects: Array, + namespace?: string + ) => + ({ + docs: objects.map((obj) => + obj.found === false ? obj : getMockGetResponse(obj, obj.initialNamespaces ?? namespace) + ), + } as estypes.MgetResponse); expect.extend({ toBeDocumentWithoutError(received, type, id) { @@ -1119,7 +1124,10 @@ describe('SavedObjectsRepository', () => { }; const namespace = 'foo-namespace'; - const bulkGet = async (objects: SavedObject[], options?: SavedObjectsBaseOptions) => + const bulkGet = async ( + objects: SavedObjectsBulkGetObject[], + options?: SavedObjectsBaseOptions + ) => savedObjectsRepository.bulkGet( objects.map(({ type, id, namespaces }) => ({ type, id, namespaces })), // bulkGet only uses type, id, and optionally namespaces options @@ -1200,7 +1208,7 @@ describe('SavedObjectsRepository', () => { describe('errors', () => { const bulkGetError = async ( - obj: SavedObject, + obj: SavedObjectsBulkGetObject & { found?: boolean }, isBulkError: boolean, expectedErrorResult: ExpectedErrorResult ) => { @@ -1236,7 +1244,7 @@ describe('SavedObjectsRepository', () => { const obj = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'three', namespaces: [] }; await bulkGetError( obj, - undefined, + false, expectErrorResult( obj, createBadRequestError('"namespaces" cannot be used on space-agnostic types') @@ -1245,11 +1253,11 @@ describe('SavedObjectsRepository', () => { }); it(`returns error when namespaces is used with a space-isolated object and does not specify a single space`, async () => { - const doTest = async (objType, namespaces) => { + const doTest = async (objType: string, namespaces?: string[]) => { const obj = { type: objType, id: 'three', namespaces }; await bulkGetError( obj, - undefined, + false, expectErrorResult( obj, createBadRequestError( @@ -1265,22 +1273,31 @@ describe('SavedObjectsRepository', () => { }); it(`returns error when type is invalid`, async () => { - const obj = { type: 'unknownType', id: 'three' }; - await bulkGetError(obj, undefined, expectErrorInvalidType(obj)); + const obj: SavedObjectsBulkGetObject = { type: 'unknownType', id: 'three' }; + await bulkGetError(obj, false, expectErrorInvalidType(obj)); }); it(`returns error when type is hidden`, async () => { - const obj = { type: HIDDEN_TYPE, id: 'three' }; - await bulkGetError(obj, undefined, expectErrorInvalidType(obj)); + const obj: SavedObjectsBulkGetObject = { type: HIDDEN_TYPE, id: 'three' }; + await bulkGetError(obj, false, expectErrorInvalidType(obj)); }); it(`returns error when document is not found`, async () => { - const obj = { type: 'dashboard', id: 'three', found: false }; + const obj: SavedObjectsBulkGetObject & { found: boolean } = { + type: 'dashboard', + id: 'three', + found: false, + }; await bulkGetError(obj, true, expectErrorNotFound(obj)); }); it(`handles missing ids gracefully`, async () => { - const obj = { type: 'dashboard', id: undefined, found: false }; + const obj: SavedObjectsBulkGetObject & { found: boolean } = { + type: 'dashboard', + // @ts-expect-error id is undefined + id: undefined, + found: false, + }; await bulkGetError(obj, true, expectErrorNotFound(obj)); }); @@ -1295,16 +1312,19 @@ describe('SavedObjectsRepository', () => { }); describe('returns', () => { - const expectSuccessResult = ({ type, id }: TypeIdTuple, doc) => ({ + const expectSuccessResult = ( + { type, id }: TypeIdTuple, + doc: estypes.MgetHit + ) => ({ type, id, - namespaces: doc._source.namespaces ?? ['default'], - ...(doc._source.originId && { originId: doc._source.originId }), - ...(doc._source.updated_at && { updated_at: doc._source.updated_at }), + namespaces: doc._source!.namespaces ?? ['default'], + ...(doc._source!.originId && { originId: doc._source!.originId }), + ...(doc._source!.updated_at && { updated_at: doc._source!.updated_at }), version: encodeHitVersion(doc), - attributes: doc._source[type], - references: doc._source.references || [], - migrationVersion: doc._source.migrationVersion, + attributes: doc._source![type], + references: doc._source!.references || [], + migrationVersion: doc._source!.migrationVersion, }); it(`returns early for empty objects argument`, async () => { @@ -1333,7 +1353,12 @@ describe('SavedObjectsRepository', () => { client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); - const obj = { type: 'unknownType', id: 'three' }; + const obj: SavedObject = { + type: 'unknownType', + id: 'three', + attributes: {}, + references: [], + }; const result = await bulkGet([obj1, obj, obj2]); expect(client.mget).toHaveBeenCalledTimes(1); expect(result).toEqual({ @@ -1346,7 +1371,12 @@ describe('SavedObjectsRepository', () => { }); it(`includes namespaces property for single-namespace and multi-namespace documents`, async () => { - const obj = { type: MULTI_NAMESPACE_ISOLATED_TYPE, id: 'three' }; + const obj: SavedObject = { + type: MULTI_NAMESPACE_ISOLATED_TYPE, + id: 'three', + attributes: {}, + references: [], + }; const result = await bulkGetSuccess([obj1, obj]); expect(result).toEqual({ saved_objects: [ @@ -1385,7 +1415,7 @@ describe('SavedObjectsRepository', () => { await expect(savedObjectsRepository.bulkResolve(objects)).resolves.toEqual({ resolved_objects: [ { - saved_object: 'mock-object', + saved_object: { type: 'mock', id: 'mock-object', attributes: {}, references: [] }, outcome: 'exactMatch', }, { From 635a78e593fed8057e16798227bc2a2d9512e7f0 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 12:08:56 +0100 Subject: [PATCH 19/22] fixed most `bulkCreate` violations, 14 left --- .../service/lib/repository.test.ts | 45 ++++++++++--------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 3816cd51a7f8c..47552f657eae4 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -411,14 +411,14 @@ describe('SavedObjectsRepository', () => { }); }); - const obj1: SavedObjectsBulkCreateObject = { + const obj1 = { type: 'config', id: '6.0.0-alpha1', attributes: { title: 'Test One' }, references: [{ name: 'ref_0', type: 'test', id: '1' }], originId: 'some-origin-id', // only one of the object args has an originId, this is intentional to test both a positive and negative case }; - const obj2: SavedObjectsBulkCreateObject = { + const obj2 = { type: 'index-pattern', id: 'logstash-*', attributes: { title: 'Test Two' }, @@ -466,12 +466,12 @@ describe('SavedObjectsRepository', () => { // bulk create calls have two objects for each source -- the action, and the source const expectClientCallArgsAction = ( - objects, + objects: Array<{ type: string; id?: string; if_primary_term?: string; if_seq_no?: string }>, { method, _index = expect.any(String), getId = () => expect.any(String), - }: { method: string; _index?: string; getId?: (type: string, id: string) => string } + }: { method: string; _index?: string; getId?: (type: string, id?: string) => string } ) => { const body = []; for (const { type, id, if_primary_term: ifPrimaryTerm, if_seq_no: ifSeqNo } of objects) { @@ -510,7 +510,11 @@ describe('SavedObjectsRepository', () => { }), ]; - const expectSuccessResult = (obj: SavedObjectUnsanitizedDoc) => ({ + const expectSuccessResult = (obj: { + type: string; + namespace?: string; + namespaces?: string[]; + }) => ({ ...obj, migrationVersion: { [obj.type]: '1.1.1' }, coreMigrationVersion: KIBANA_VERSION, @@ -635,11 +639,11 @@ describe('SavedObjectsRepository', () => { const objects = [obj1, obj2].map((x) => ({ ...x, type: MULTI_NAMESPACE_ISOLATED_TYPE })); const [o1, o2] = objects; mockPreflightCheckForCreate.mockResolvedValueOnce([ - { type: o1.type, id: o1.id }, // first object does not have an existing document to overwrite + { type: o1.type, id: o1.id! }, // first object does not have an existing document to overwrite { type: o2.type, - id: o2.id, - existingDocument: { _id: o2.id, _source: { namespaces: ['*'], type: o2.type } }, // second object does have an existing document to overwrite + id: o2.id!, + existingDocument: { _id: o2.id!, _source: { namespaces: ['*'], type: o2.type } }, // second object does have an existing document to overwrite }, ]); await bulkCreateSuccess(objects, { namespace, overwrite: true }); @@ -669,12 +673,12 @@ describe('SavedObjectsRepository', () => { const [o1, o2, o3] = objects; mockPreflightCheckForCreate.mockResolvedValueOnce([ // first object does not get passed in to preflightCheckForCreate at all - { type: o2.type, id: o2.id }, // second object does not have an existing document to overwrite + { type: o2.type, id: o2.id! }, // second object does not have an existing document to overwrite { type: o3.type, - id: o3.id, + id: o3.id!, existingDocument: { - _id: o3.id, + _id: o3.id!, _source: { type: o3.type, namespaces: [namespace ?? 'default', 'something-else'] }, // third object does have an existing document to overwrite }, }, @@ -771,19 +775,19 @@ describe('SavedObjectsRepository', () => { }); it(`prepends namespace to the id when providing namespace for single-namespace type`, async () => { - const getId = (type: string, id: string) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) + const getId = (type: string, id: string = '') => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix) await bulkCreateSuccess([obj1, obj2], { namespace }); expectClientCallArgsAction([obj1, obj2], { method: 'create', getId }); }); it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => { - const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string = '') => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) await bulkCreateSuccess([obj1, obj2]); expectClientCallArgsAction([obj1, obj2], { method: 'create', getId }); }); it(`doesn't prepend namespace to the id when not using single-namespace type`, async () => { - const getId = (type: string, id: string) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) + const getId = (type: string, id: string = '') => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix) const objects = [ { ...obj1, type: NAMESPACE_AGNOSTIC_TYPE }, { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE }, @@ -814,7 +818,7 @@ describe('SavedObjectsRepository', () => { if (isBulkError) { // mock the bulk error for only the second object mockGetBulkOperationError.mockReturnValueOnce(undefined); - mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error); + mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error as Payload); response = getMockBulkCreateResponse([obj1, obj, obj2]); } else { response = getMockBulkCreateResponse([obj1, obj2]); @@ -909,15 +913,15 @@ describe('SavedObjectsRepository', () => { const [o1, o2, o3, o4, o5] = objects; mockPreflightCheckForCreate.mockResolvedValueOnce([ // first and last objects do not get passed in to preflightCheckForCreate at all - { type: o2.type, id: o2.id, error: { type: 'conflict' } }, + { type: o2.type, id: o2.id!, error: { type: 'conflict' } }, { type: o3.type, - id: o3.id, + id: o3.id!, error: { type: 'unresolvableConflict', metadata: { isNotOverwritable: true } }, }, { type: o4.type, - id: o4.id, + id: o4.id!, error: { type: 'aliasConflict', metadata: { spacesWithConflictingAliases: ['foo'] } }, }, ]); @@ -1038,9 +1042,10 @@ describe('SavedObjectsRepository', () => { it.todo(`should return objects in the same order regardless of type`); it(`handles a mix of successful creates and errors`, async () => { - const obj = { + const obj: SavedObjectsBulkCreateObject = { type: 'unknownType', id: 'three', + attributes: {}, }; const objects = [obj1, obj, obj2]; const response = getMockBulkCreateResponse([obj1, obj2]); @@ -1072,7 +1077,7 @@ describe('SavedObjectsRepository', () => { expect(serializer.rawToSavedObject).toHaveBeenNthCalledWith(1, { ...response.items[0].create, _source: { - ...response.items[0].create._source, + ...response.items[0].create!._source, coreMigrationVersion: '2.0.0', // the document migrator adds this to all objects before creation namespaces: response.items[0].create._source.namespaces, }, From 854c6ff7fa9c493f22c3393387faf406964b343b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 12:13:28 +0100 Subject: [PATCH 20/22] fixed last `bulkCreate` violations, 5 left --- .../saved_objects/service/lib/repository.test.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 47552f657eae4..3a2792ef7c04c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -450,7 +450,7 @@ describe('SavedObjectsRepository', () => { ...mockVersionProps, }, })), - } as estypes.BulkResponse; + } as unknown as estypes.BulkResponse; }; const bulkCreateSuccess = async ( @@ -1042,7 +1042,7 @@ describe('SavedObjectsRepository', () => { it.todo(`should return objects in the same order regardless of type`); it(`handles a mix of successful creates and errors`, async () => { - const obj: SavedObjectsBulkCreateObject = { + const obj = { type: 'unknownType', id: 'three', attributes: {}, @@ -1063,7 +1063,10 @@ describe('SavedObjectsRepository', () => { // Test for fix to https://github.com/elastic/kibana/issues/65088 where // we returned raw ID's when an object without an id was created. const namespace = 'myspace'; - const response = getMockBulkCreateResponse([obj1, obj2], namespace); + // FIXME: this test is based on a gigantic hack to have the bulk operation return the source + // of the document when it actually does not, forcing to cast to any as BulkResponse + // does not contains _source + const response = getMockBulkCreateResponse([obj1, obj2], namespace) as any; client.bulk.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise(response) ); @@ -1077,7 +1080,7 @@ describe('SavedObjectsRepository', () => { expect(serializer.rawToSavedObject).toHaveBeenNthCalledWith(1, { ...response.items[0].create, _source: { - ...response.items[0].create!._source, + ...response.items[0].create._source, coreMigrationVersion: '2.0.0', // the document migrator adds this to all objects before creation namespaces: response.items[0].create._source.namespaces, }, From 68b1d38d41ce40a839dd62d398c3a8f12ec8e518 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 12:25:25 +0100 Subject: [PATCH 21/22] Everything but the SOR constructor --- .../server/saved_objects/service/lib/repository.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 3a2792ef7c04c..075afa38ae8c1 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -316,8 +316,11 @@ describe('SavedObjectsRepository', () => { } }, }); - const expectSuccess = ({ type, id }: { type: string; id: string }) => - expect.toBeDocumentWithoutError(type, id); + const expectSuccess = ({ type, id }: { type: string; id: string }) => { + // @ts-expect-error TS is not aware of the extension + return expect.toBeDocumentWithoutError(type, id); + }; + const expectError = ({ type, id }: { type: string; id: string }) => ({ type, id, @@ -356,6 +359,7 @@ describe('SavedObjectsRepository', () => { }; const realInstance = new SavedObjectsSerializer(registry); Object.keys(spyInstance).forEach((key) => { + // @ts-expect-error no proper way to do this with typing support spyInstance[key].mockImplementation((...args) => realInstance[key](...args)); }); From 516ba2f19f7fdbada27eb50ae964ce98540cb3c5 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 5 Nov 2021 12:52:51 +0100 Subject: [PATCH 22/22] ignore warning on SOR private constructor --- src/core/server/saved_objects/service/lib/repository.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/server/saved_objects/service/lib/repository.test.ts b/src/core/server/saved_objects/service/lib/repository.test.ts index 075afa38ae8c1..8a9f099314b8c 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/repository.test.ts @@ -381,6 +381,7 @@ describe('SavedObjectsRepository', () => { const allTypes = registry.getAllTypes().map((type) => type.name); const allowedTypes = [...new Set(allTypes.filter((type) => !registry.isHidden(type)))]; + // @ts-expect-error must use the private constructor to use the mocked serializer savedObjectsRepository = new SavedObjectsRepository({ index: '.kibana-test', mappings,