From 872facd7cadc9135508abe9bcbde01cb55746dac Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 3 Sep 2021 16:57:19 -0700 Subject: [PATCH 1/6] Use resolve instead of get for saved query service --- .../data/public/query/saved_query/saved_query_service.ts | 9 +++++++-- src/plugins/data/server/saved_objects/query.ts | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/plugins/data/public/query/saved_query/saved_query_service.ts b/src/plugins/data/public/query/saved_query/saved_query_service.ts index 0f3da8f80a5ec6..5fc02e24e69baf 100644 --- a/src/plugins/data/public/query/saved_query/saved_query_service.ts +++ b/src/plugins/data/public/query/saved_query/saved_query_service.ts @@ -105,8 +105,13 @@ export const createSavedQueryService = ( }; const getSavedQuery = async (id: string): Promise => { - const savedObject = await savedObjectsClient.get('query', id); - if (savedObject.error) { + const { + saved_object: savedObject, + outcome, + } = await savedObjectsClient.resolve('query', id); + if (outcome === 'conflict') { + throw new Error(`Multiple saved queries found with ID: ${id}`); + } else if (savedObject.error) { throw new Error(savedObject.error.message); } return parseSavedQueryObject(savedObject); diff --git a/src/plugins/data/server/saved_objects/query.ts b/src/plugins/data/server/saved_objects/query.ts index 2e8b80cf3f080d..56ef73eb541d94 100644 --- a/src/plugins/data/server/saved_objects/query.ts +++ b/src/plugins/data/server/saved_objects/query.ts @@ -11,7 +11,7 @@ import { SavedObjectsType } from 'kibana/server'; export const querySavedObjectType: SavedObjectsType = { name: 'query', hidden: false, - namespaceType: 'single', + namespaceType: 'multiple-isolated', management: { icon: 'search', defaultSearchField: 'title', From 7b85d047bd8d5c49c2a31b3617eff1756afb04f9 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 3 Sep 2021 17:02:47 -0700 Subject: [PATCH 2/6] Update tests --- .../saved_query/saved_query_service.test.ts | 113 ++++++++++++------ 1 file changed, 76 insertions(+), 37 deletions(-) diff --git a/src/plugins/data/public/query/saved_query/saved_query_service.test.ts b/src/plugins/data/public/query/saved_query/saved_query_service.test.ts index a74f81b6a046d1..25bbc6dc08401a 100644 --- a/src/plugins/data/public/query/saved_query/saved_query_service.test.ts +++ b/src/plugins/data/public/query/saved_query/saved_query_service.test.ts @@ -54,7 +54,7 @@ const mockSavedObjectsClient = { create: jest.fn(), error: jest.fn(), find: jest.fn(), - get: jest.fn(), + resolve: jest.fn(), delete: jest.fn(), }; @@ -74,7 +74,7 @@ describe('saved query service', () => { afterEach(() => { mockSavedObjectsClient.create.mockReset(); mockSavedObjectsClient.find.mockReset(); - mockSavedObjectsClient.get.mockReset(); + mockSavedObjectsClient.resolve.mockReset(); mockSavedObjectsClient.delete.mockReset(); }); @@ -267,29 +267,44 @@ describe('saved query service', () => { describe('getSavedQuery', function () { it('should retrieve a saved query by id', async () => { - mockSavedObjectsClient.get.mockReturnValue({ id: 'foo', attributes: savedQueryAttributes }); + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'foo', + attributes: savedQueryAttributes, + }, + outcome: 'exactMatch', + }); const response = await getSavedQuery('foo'); expect(response).toEqual({ id: 'foo', attributes: savedQueryAttributes }); }); it('should only return saved queries', async () => { - mockSavedObjectsClient.get.mockReturnValue({ id: 'foo', attributes: savedQueryAttributes }); + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'foo', + attributes: savedQueryAttributes, + }, + outcome: 'exactMatch', + }); await getSavedQuery('foo'); - expect(mockSavedObjectsClient.get).toHaveBeenCalledWith('query', 'foo'); + expect(mockSavedObjectsClient.resolve).toHaveBeenCalledWith('query', 'foo'); }); it('should parse a json query', async () => { - mockSavedObjectsClient.get.mockReturnValue({ - id: 'food', - attributes: { - title: 'food', - description: 'bar', - query: { - language: 'kuery', - query: '{"x": "y"}', + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'food', + attributes: { + title: 'food', + description: 'bar', + query: { + language: 'kuery', + query: '{"x": "y"}', + }, }, }, + outcome: 'exactMatch', }); const response = await getSavedQuery('food'); @@ -297,16 +312,19 @@ describe('saved query service', () => { }); it('should handle null string', async () => { - mockSavedObjectsClient.get.mockReturnValue({ - id: 'food', - attributes: { - title: 'food', - description: 'bar', - query: { - language: 'kuery', - query: 'null', + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'food', + attributes: { + title: 'food', + description: 'bar', + query: { + language: 'kuery', + query: 'null', + }, }, }, + outcome: 'exactMatch', }); const response = await getSavedQuery('food'); @@ -314,16 +332,19 @@ describe('saved query service', () => { }); it('should handle null quoted string', async () => { - mockSavedObjectsClient.get.mockReturnValue({ - id: 'food', - attributes: { - title: 'food', - description: 'bar', - query: { - language: 'kuery', - query: '"null"', + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'food', + attributes: { + title: 'food', + description: 'bar', + query: { + language: 'kuery', + query: '"null"', + }, }, }, + outcome: 'exactMatch', }); const response = await getSavedQuery('food'); @@ -331,21 +352,39 @@ describe('saved query service', () => { }); it('should not lose quotes', async () => { - mockSavedObjectsClient.get.mockReturnValue({ - id: 'food', - attributes: { - title: 'food', - description: 'bar', - query: { - language: 'kuery', - query: '"Bob"', + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'food', + attributes: { + title: 'food', + description: 'bar', + query: { + language: 'kuery', + query: '"Bob"', + }, }, }, + outcome: 'exactMatch', }); const response = await getSavedQuery('food'); expect(response.attributes.query.query).toEqual('"Bob"'); }); + + it('should throw if conflict', async () => { + mockSavedObjectsClient.resolve.mockReturnValue({ + saved_object: { + id: 'foo', + attributes: savedQueryAttributes, + }, + outcome: 'conflict', + }); + + const result = getSavedQuery('food'); + expect(result).rejects.toMatchInlineSnapshot( + `[Error: Multiple saved queries found with ID: food]` + ); + }); }); describe('deleteSavedQuery', function () { From 1e14b3b850ca3f9cf3c09044a88fe45c9c953f68 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 7 Sep 2021 13:30:59 -0700 Subject: [PATCH 3/6] Update src/plugins/data/public/query/saved_query/saved_query_service.ts Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> --- .../data/public/query/saved_query/saved_query_service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/public/query/saved_query/saved_query_service.ts b/src/plugins/data/public/query/saved_query/saved_query_service.ts index 5fc02e24e69baf..21a34e0f136a20 100644 --- a/src/plugins/data/public/query/saved_query/saved_query_service.ts +++ b/src/plugins/data/public/query/saved_query/saved_query_service.ts @@ -110,7 +110,7 @@ export const createSavedQueryService = ( outcome, } = await savedObjectsClient.resolve('query', id); if (outcome === 'conflict') { - throw new Error(`Multiple saved queries found with ID: ${id}`); + throw new Error(`Multiple saved queries found with ID: ${id} (legacy URL alias conflict)`); } else if (savedObject.error) { throw new Error(savedObject.error.message); } From 09e3b7bcc5bf46c8d9b46b78f04033f83b214581 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 7 Sep 2021 13:31:48 -0700 Subject: [PATCH 4/6] Revert step 4 --- src/plugins/data/server/saved_objects/query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/server/saved_objects/query.ts b/src/plugins/data/server/saved_objects/query.ts index 56ef73eb541d94..2e8b80cf3f080d 100644 --- a/src/plugins/data/server/saved_objects/query.ts +++ b/src/plugins/data/server/saved_objects/query.ts @@ -11,7 +11,7 @@ import { SavedObjectsType } from 'kibana/server'; export const querySavedObjectType: SavedObjectsType = { name: 'query', hidden: false, - namespaceType: 'multiple-isolated', + namespaceType: 'single', management: { icon: 'search', defaultSearchField: 'title', From a1d6cb78f6f691172add3fb84b7b22c12e5191b5 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 7 Sep 2021 13:33:45 -0700 Subject: [PATCH 5/6] Make saved queries share-capable --- src/plugins/data/server/saved_objects/query.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/data/server/saved_objects/query.ts b/src/plugins/data/server/saved_objects/query.ts index 2e8b80cf3f080d..bc6225255b5e6c 100644 --- a/src/plugins/data/server/saved_objects/query.ts +++ b/src/plugins/data/server/saved_objects/query.ts @@ -11,7 +11,8 @@ import { SavedObjectsType } from 'kibana/server'; export const querySavedObjectType: SavedObjectsType = { name: 'query', hidden: false, - namespaceType: 'single', + namespaceType: 'multiple-isolated', + convertToMultiNamespaceTypeVersion: '8.0.0', management: { icon: 'search', defaultSearchField: 'title', From 5f2596bdc6e5c764ce1adc39d44251d433db7cc2 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 8 Sep 2021 14:03:48 -0700 Subject: [PATCH 6/6] Fix test --- .../data/public/query/saved_query/saved_query_service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/public/query/saved_query/saved_query_service.test.ts b/src/plugins/data/public/query/saved_query/saved_query_service.test.ts index 25bbc6dc08401a..673a86df98881d 100644 --- a/src/plugins/data/public/query/saved_query/saved_query_service.test.ts +++ b/src/plugins/data/public/query/saved_query/saved_query_service.test.ts @@ -382,7 +382,7 @@ describe('saved query service', () => { const result = getSavedQuery('food'); expect(result).rejects.toMatchInlineSnapshot( - `[Error: Multiple saved queries found with ID: food]` + `[Error: Multiple saved queries found with ID: food (legacy URL alias conflict)]` ); }); });