diff --git a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.test.ts b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.test.ts index 5d78903812778..183396ef639c1 100644 --- a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.test.ts +++ b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.test.ts @@ -137,7 +137,12 @@ describe('[Snapshot and Restore API Routes] Repositories', () => { [name]: { type: '', settings: {} }, }; const mockEsSnapshotResponse = { - snapshots: [{}, {}], + responses: [ + { + repository: name, + snapshots: [{}, {}], + }, + ], }; const callWithRequest = jest .fn() diff --git a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.ts b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.ts index 70034b627598d..a01835d0e67c6 100644 --- a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.ts +++ b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/repositories.ts @@ -73,16 +73,27 @@ export const getOneHandler: RouterRouteHandler = async ( ): Promise<{ repository: Repository | {}; isManagedRepository?: boolean; - snapshots: { count: number | undefined } | {}; + snapshots: { count: number | null } | {}; }> => { const { name } = req.params; const managedRepository = await getManagedRepositoryName(); const repositoryByName = await callWithRequest('snapshot.getRepository', { repository: name }); - const { snapshots } = await callWithRequest('snapshot.get', { + const { + responses: snapshotResponses, + }: { + responses: Array<{ + repository: string; + snapshots: any[]; + }>; + } = await callWithRequest('snapshot.get', { repository: name, snapshot: '_all', }).catch(e => ({ - snapshots: null, + responses: [ + { + snapshots: null, + }, + ], })); if (repositoryByName[name]) { @@ -95,7 +106,10 @@ export const getOneHandler: RouterRouteHandler = async ( }, isManagedRepository: managedRepository === name, snapshots: { - count: snapshots ? snapshots.length : null, + count: + snapshotResponses && snapshotResponses[0] && snapshotResponses[0].snapshots + ? snapshotResponses[0].snapshots.length + : null, }, }; } else { diff --git a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.test.ts b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.test.ts index b5ef21963c0c5..522ad3f412fa6 100644 --- a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.test.ts +++ b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.test.ts @@ -38,17 +38,19 @@ describe('[Snapshot and Restore API Routes] Snapshots', () => { }; const mockGetSnapshotsFooResponse = Promise.resolve({ - snapshots: [ + responses: [ { - snapshot: 'snapshot1', + repository: 'fooRepository', + snapshots: [{ snapshot: 'snapshot1' }], }, ], }); const mockGetSnapshotsBarResponse = Promise.resolve({ - snapshots: [ + responses: [ { - snapshot: 'snapshot2', + repository: 'barRepository', + snapshots: [{ snapshot: 'snapshot2' }], }, ], }); @@ -105,7 +107,12 @@ describe('[Snapshot and Restore API Routes] Snapshots', () => { test('returns snapshot object with repository name if returned from ES', async () => { const mockSnapshotGetEsResponse = { - snapshots: [{ snapshot }], + responses: [ + { + repository, + snapshots: [{ snapshot }], + }, + ], }; const callWithRequest = jest.fn().mockReturnValue(mockSnapshotGetEsResponse); const expectedResponse = { @@ -118,10 +125,25 @@ describe('[Snapshot and Restore API Routes] Snapshots', () => { expect(response).toEqual(expectedResponse); }); - test('throws if ES error (including 404s)', async () => { - const callWithRequest = jest.fn().mockImplementation(() => { - throw new Error(); - }); + test('throws if ES error', async () => { + const mockSnapshotGetEsResponse = { + responses: [ + { + repository, + error: { + root_cause: [ + { + type: 'snapshot_missing_exception', + reason: `[${repository}:${snapshot}] is missing`, + }, + ], + type: 'snapshot_missing_exception', + reason: `[${repository}:${snapshot}] is missing`, + }, + }, + ], + }; + const callWithRequest = jest.fn().mockReturnValue(mockSnapshotGetEsResponse); await expect( getOneHandler(mockOneRequest, callWithRequest, mockResponseToolkit) diff --git a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.ts b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.ts index 02102b4cdaffb..4bb55ee641c97 100644 --- a/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.ts +++ b/x-pack/legacy/plugins/snapshot_restore/server/routes/api/snapshots.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { Router, RouterRouteHandler } from '../../../../../server/lib/create_router'; +import { wrapCustomError } from '../../../../../server/lib/create_router/error_wrappers'; import { SnapshotDetails } from '../../../common/types'; import { deserializeSnapshotDetails } from '../../lib'; import { SnapshotDetailsEs } from '../../types'; @@ -21,6 +22,11 @@ export const getAllHandler: RouterRouteHandler = async ( errors: any[]; repositories: string[]; }> => { + /* + * TODO: For 8.0, replace the logic in this handler with one call to `GET /_snapshot/_all/_all` + * when no repositories bug is fixed: https://github.com/elastic/elasticsearch/issues/43547 + */ + const repositoriesByName = await callWithRequest('snapshot.getRepository', { repository: '_all', }); @@ -39,16 +45,23 @@ export const getAllHandler: RouterRouteHandler = async ( try { // If any of these repositories 504 they will cost the request significant time. const { - snapshots: fetchedSnapshots, - }: { snapshots: SnapshotDetailsEs[] } = await callWithRequest('snapshot.get', { + responses: fetchedResponses, + }: { + responses: Array<{ + repository: 'string'; + snapshots: SnapshotDetailsEs[]; + }>; + } = await callWithRequest('snapshot.get', { repository, snapshot: '_all', ignore_unavailable: true, // Allow request to succeed even if some snapshots are unavailable. }); // Decorate each snapshot with the repository with which it's associated. - fetchedSnapshots.forEach((snapshot: SnapshotDetailsEs) => { - snapshots.push(deserializeSnapshotDetails(repository, snapshot)); + fetchedResponses.forEach(({ snapshots: fetchedSnapshots }) => { + fetchedSnapshots.forEach(snapshot => { + snapshots.push(deserializeSnapshotDetails(repository, snapshot)); + }); }); repositories.push(repository); @@ -73,11 +86,23 @@ export const getOneHandler: RouterRouteHandler = async ( callWithRequest ): Promise => { const { repository, snapshot } = req.params; - const { snapshots }: { snapshots: SnapshotDetailsEs[] } = await callWithRequest('snapshot.get', { + const { + responses: snapshotResponses, + }: { + responses: Array<{ + repository: string; + snapshots: SnapshotDetailsEs[]; + error?: any; + }>; + } = await callWithRequest('snapshot.get', { repository, snapshot, }); - // If the snapshot is missing the endpoint will return a 404, so we'll never get to this point. - return deserializeSnapshotDetails(repository, snapshots[0]); + if (snapshotResponses && snapshotResponses[0] && snapshotResponses[0].snapshots) { + return deserializeSnapshotDetails(repository, snapshotResponses[0].snapshots[0]); + } + + // If snapshot doesn't exist, ES will return 200 with an error object, so manually throw 404 here + throw wrapCustomError(new Error('Snapshot not found'), 404); };