From 774289f4b2f42cc61a00410413660ab4448a4331 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 13 Sep 2021 16:00:27 -0700 Subject: [PATCH] Use savedObjectsClient.resolve in saved query service (#111229) * Use resolve instead of get for saved query service * Update tests * Update src/plugins/data/public/query/saved_query/saved_query_service.ts Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> * Revert step 4 * Fix test Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> --- .../saved_query/saved_query_service.test.ts | 113 ++++++++++++------ .../query/saved_query/saved_query_service.ts | 9 +- 2 files changed, 83 insertions(+), 39 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..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 @@ -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 (legacy URL alias conflict)]` + ); + }); }); describe('deleteSavedQuery', function () { 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..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 @@ -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} (legacy URL alias conflict)`); + } else if (savedObject.error) { throw new Error(savedObject.error.message); } return parseSavedQueryObject(savedObject);