Skip to content

Commit 572f8bb

Browse files
authored
remove angular dependencies in SavedObjectClient (#20384)
* create saved object client that is native javascript * fix savedObjectClient unit tests * get saved object client from chrome when being used outside of angular * update error handlers to pull status code from FetchError * add some debug messages to failing functional test * revert changes to management/_objects * add clicks to import done in import objects test * take screenshots of test only failing in CI * remove functional test screenshot code since test is passing in CI now * remove unused file, clean up saved_objects_client test to not use global, add test to error_auto_create_index to ensure compatibility with kfetch errors * add body to kfetch error * update savedObjectClient.bulkCreate * add bulkCreate wrapper to SavedObjectsClientProvider * mark _createSavedObject and _processBatchQueue as private methods
1 parent 0baa57e commit 572f8bb

File tree

14 files changed

+375
-234
lines changed

14 files changed

+375
-234
lines changed

src/core_plugins/kibana/public/dashboard/dashboard_app.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import * as filterActions from 'ui/doc_table/actions/filter';
4747
import { FilterManagerProvider } from 'ui/filter_manager';
4848
import { EmbeddableFactoriesRegistryProvider } from 'ui/embeddable/embeddable_factories_registry';
4949
import { DashboardPanelActionsRegistryProvider } from 'ui/dashboard_panel_actions/dashboard_panel_actions_registry';
50-
import { SavedObjectsClientProvider } from 'ui/saved_objects';
5150
import { VisTypesRegistryProvider } from 'ui/registry/vis_types';
5251
import { timefilter } from 'ui/timefilter';
5352

@@ -86,7 +85,6 @@ app.directive('dashboardApp', function ($injector) {
8685

8786
panelActionsStore.initializeFromRegistry(panelActionsRegistry);
8887

89-
const savedObjectsClient = Private(SavedObjectsClientProvider);
9088
const visTypes = Private(VisTypesRegistryProvider);
9189
$scope.getEmbeddableFactory = panelType => embeddableFactories.byName[panelType];
9290

@@ -391,7 +389,7 @@ app.directive('dashboardApp', function ($injector) {
391389
const isLabsEnabled = config.get('visualize:enableLabs');
392390
const listingLimit = config.get('savedObjects:listingLimit');
393391

394-
showAddPanel(savedObjectsClient, dashboardStateManager.addNewPanel, addNewVis, listingLimit, isLabsEnabled, visTypes);
392+
showAddPanel(chrome.getSavedObjectsClient(), dashboardStateManager.addNewPanel, addNewVis, listingLimit, isLabsEnabled, visTypes);
395393
};
396394
updateViewMode(dashboardStateManager.getViewMode());
397395

src/core_plugins/metrics/public/kbn_vis_types/editor_controller.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@
1818
*/
1919

2020
import React from 'react';
21+
import chrome from 'ui/chrome';
2122
import { render, unmountComponentAtNode } from 'react-dom';
22-
import { SavedObjectsClientProvider } from 'ui/saved_objects';
2323
import { FetchFieldsProvider } from '../lib/fetch_fields';
2424
import { extractIndexPatterns } from '../lib/extract_index_patterns';
2525

2626
function ReactEditorControllerProvider(Private, config) {
2727
const fetchFields = Private(FetchFieldsProvider);
28-
const savedObjectsClient = Private(SavedObjectsClientProvider);
28+
const savedObjectsClient = chrome.getSavedObjectsClient();
2929

3030
class ReactEditorController {
3131
constructor(el, savedObj) {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { SavedObjectsClient } from '../../saved_objects';
21+
22+
export function initSavedObjectClient(chrome) {
23+
const savedObjectClient = new SavedObjectsClient();
24+
25+
chrome.getSavedObjectsClient = function () {
26+
return savedObjectClient;
27+
};
28+
}

src/ui/public/chrome/chrome.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import translationsApi from './api/translations';
4444
import { initChromeXsrfApi } from './api/xsrf';
4545
import { initUiSettingsApi } from './api/ui_settings';
4646
import { initLoadingCountApi } from './api/loading_count';
47+
import { initSavedObjectClient } from './api/saved_object_client';
4748

4849
export const chrome = {};
4950
const internals = _.defaults(
@@ -62,6 +63,7 @@ const internals = _.defaults(
6263
);
6364

6465
initUiSettingsApi(chrome);
66+
initSavedObjectClient(chrome);
6567
appsApi(chrome, internals);
6668
initChromeXsrfApi(chrome, internals);
6769
initChromeNavApi(chrome, internals);

src/ui/public/courier/saved_object/__tests__/saved_object.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ describe('Saved Object', function () {
8181
return savedObject.init();
8282
}
8383

84+
const mock409FetchError = {
85+
res: { status: 409 }
86+
};
87+
8488
beforeEach(ngMock.module(
8589
'kibana',
8690
StubIndexPatternsApiClientModule,
@@ -104,15 +108,15 @@ describe('Saved Object', function () {
104108
describe('with confirmOverwrite', function () {
105109
function stubConfirmOverwrite() {
106110
window.confirm = sinon.stub().returns(true);
107-
sinon.stub(esDataStub, 'create').returns(BluebirdPromise.reject({ status: 409 }));
111+
sinon.stub(esDataStub, 'create').returns(BluebirdPromise.reject(mock409FetchError));
108112
}
109113

110114
describe('when true', function () {
111115
it('requests confirmation and updates on yes response', function () {
112116
stubESResponse(getMockedDocResponse('myId'));
113117
return createInitializedSavedObject({ type: 'dashboard', id: 'myId' }).then(savedObject => {
114118
const createStub = sinon.stub(savedObjectsClientStub, 'create');
115-
createStub.onFirstCall().returns(BluebirdPromise.reject({ statusCode: 409 }));
119+
createStub.onFirstCall().returns(BluebirdPromise.reject(mock409FetchError));
116120
createStub.onSecondCall().returns(BluebirdPromise.resolve({ id: 'myId' }));
117121

118122
stubConfirmOverwrite();
@@ -135,7 +139,7 @@ describe('Saved Object', function () {
135139
return createInitializedSavedObject({ type: 'dashboard', id: 'HI' }).then(savedObject => {
136140
window.confirm = sinon.stub().returns(false);
137141

138-
sinon.stub(savedObjectsClientStub, 'create').returns(BluebirdPromise.reject({ statusCode: 409 }));
142+
sinon.stub(savedObjectsClientStub, 'create').returns(BluebirdPromise.reject(mock409FetchError));
139143

140144
savedObject.lastSavedTitle = 'original title';
141145
savedObject.title = 'new title';
@@ -154,7 +158,7 @@ describe('Saved Object', function () {
154158
return createInitializedSavedObject({ type: 'dashboard', id: 'myId' }).then(savedObject => {
155159
stubConfirmOverwrite();
156160

157-
sinon.stub(savedObjectsClientStub, 'create').returns(BluebirdPromise.reject({ statusCode: 409 }));
161+
sinon.stub(savedObjectsClientStub, 'create').returns(BluebirdPromise.reject(mock409FetchError));
158162

159163
return savedObject.save({ confirmOverwrite: true })
160164
.then(() => {

src/ui/public/courier/saved_object/saved_object.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ export function SavedObjectProvider(Promise, Private, Notifier, confirmModalProm
292292
return savedObjectsClient.create(esType, source, { id: this.id })
293293
.catch(err => {
294294
// record exists, confirm overwriting
295-
if (_.get(err, 'statusCode') === 409) {
295+
if (_.get(err, 'res.status') === 409) {
296296
const confirmMessage = `Are you sure you want to overwrite ${this.title}?`;
297297

298298
return confirmModalPromise(confirmMessage, { confirmButtonText: `Overwrite ${this.getDisplayName()}` })

src/ui/public/error_auto_create_index/error_auto_create_index.js

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,20 @@
2020
import { get } from 'lodash';
2121

2222
import uiRoutes from '../routes';
23-
import { KbnUrlProvider } from '../url';
2423

2524
import './error_auto_create_index.less';
2625
import template from './error_auto_create_index.html';
2726

2827
uiRoutes
2928
.when('/error/action.auto_create_index', { template });
3029

31-
export function ErrorAutoCreateIndexProvider(Private, Promise) {
32-
const kbnUrl = Private(KbnUrlProvider);
33-
34-
return new (class ErrorAutoCreateIndex {
35-
test(error) {
36-
return (
37-
error.statusCode === 503 &&
38-
get(error, 'body.code') === 'ES_AUTO_CREATE_INDEX_ERROR'
39-
);
40-
}
30+
export function isAutoCreateIndexError(error) {
31+
return (
32+
get(error, 'res.status') === 503 &&
33+
get(error, 'body.code') === 'ES_AUTO_CREATE_INDEX_ERROR'
34+
);
35+
}
4136

42-
takeover() {
43-
kbnUrl.change('/error/action.auto_create_index');
44-
return Promise.halt();
45-
}
46-
});
37+
export function showAutoCreateIndexErrorPage() {
38+
window.location.hash = '/error/action.auto_create_index';
4739
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Licensed to Elasticsearch B.V. under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch B.V. licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
jest.mock('../chrome', () => ({
21+
addBasePath: path => `myBase/${path}`,
22+
}));
23+
jest.mock('../metadata', () => ({
24+
metadata: {
25+
version: 'my-version',
26+
},
27+
}));
28+
29+
import fetchMock from 'fetch-mock';
30+
import { kfetch } from 'ui/kfetch';
31+
import { isAutoCreateIndexError } from './error_auto_create_index';
32+
33+
describe('isAutoCreateIndexError correctly handles FetchError thrown by kfetch', () => {
34+
describe('404', () => {
35+
beforeEach(() => {
36+
fetchMock.post({
37+
matcher: '*',
38+
response: {
39+
status: 404,
40+
},
41+
});
42+
});
43+
afterEach(() => fetchMock.restore());
44+
45+
test('should return false', async () => {
46+
let gotError = false;
47+
try {
48+
await kfetch({ method: 'POST', pathname: 'my/path' });
49+
} catch (fetchError) {
50+
gotError = true;
51+
expect(isAutoCreateIndexError(fetchError)).toBe(false);
52+
}
53+
54+
expect(gotError).toBe(true);
55+
});
56+
});
57+
58+
describe('503 error that is not ES_AUTO_CREATE_INDEX_ERROR', () => {
59+
beforeEach(() => {
60+
fetchMock.post({
61+
matcher: '*',
62+
response: {
63+
status: 503,
64+
},
65+
});
66+
});
67+
afterEach(() => fetchMock.restore());
68+
69+
test('should return false', async () => {
70+
let gotError = false;
71+
try {
72+
await kfetch({ method: 'POST', pathname: 'my/path' });
73+
} catch (fetchError) {
74+
gotError = true;
75+
expect(isAutoCreateIndexError(fetchError)).toBe(false);
76+
}
77+
78+
expect(gotError).toBe(true);
79+
});
80+
});
81+
82+
describe('503 error that is ES_AUTO_CREATE_INDEX_ERROR', () => {
83+
beforeEach(() => {
84+
fetchMock.post({
85+
matcher: '*',
86+
response: {
87+
body: {
88+
code: 'ES_AUTO_CREATE_INDEX_ERROR'
89+
},
90+
status: 503,
91+
},
92+
});
93+
});
94+
afterEach(() => fetchMock.restore());
95+
96+
test('should return true', async () => {
97+
let gotError = false;
98+
try {
99+
await kfetch({ method: 'POST', pathname: 'my/path' });
100+
} catch (fetchError) {
101+
gotError = true;
102+
expect(isAutoCreateIndexError(fetchError)).toBe(true);
103+
}
104+
105+
expect(gotError).toBe(true);
106+
});
107+
});
108+
});
109+

src/ui/public/error_auto_create_index/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@
1717
* under the License.
1818
*/
1919

20-
export { ErrorAutoCreateIndexProvider } from './error_auto_create_index';
20+
export { isAutoCreateIndexError, showAutoCreateIndexErrorPage } from './error_auto_create_index';

src/ui/public/kfetch/index.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ import { metadata } from '../metadata';
2424
import { merge } from 'lodash';
2525

2626
class FetchError extends Error {
27-
constructor(res) {
27+
constructor(res, body) {
2828
super(res.statusText);
2929
this.res = res;
30+
this.body = body;
3031
Error.captureStackTrace(this, FetchError);
3132
}
3233
}
@@ -59,7 +60,13 @@ export async function kfetch(fetchOptions, kibanaOptions) {
5960
const res = await fetch(fullUrl, combinedFetchOptions);
6061

6162
if (!res.ok) {
62-
throw new FetchError(res);
63+
let body;
64+
try {
65+
body = await res.json();
66+
} catch (err) {
67+
// ignore error, may not be able to get body for response that is not ok
68+
}
69+
throw new FetchError(res, body);
6370
}
6471

6572
return res.json();

0 commit comments

Comments
 (0)