Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uses SavedObjectAPI for CRUD operations #5

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ savedObjectManagementRegistry.register({
});

// This is the only thing that gets injected into controllers
module.service('savedDashboards', function (SavedDashboard, kbnIndex, esAdmin, kbnUrl) {
return new SavedObjectLoader(SavedDashboard, kbnIndex, esAdmin, kbnUrl);
module.service('savedDashboards', function (SavedDashboard, kbnIndex, esAdmin, kbnUrl, $http) {
return new SavedObjectLoader(SavedDashboard, kbnIndex, esAdmin, kbnUrl, $http);
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ savedObjectManagementRegistry.register({
title: 'searches'
});

module.service('savedSearches', function (Promise, config, kbnIndex, esAdmin, createNotifier, SavedSearch, kbnUrl) {
const savedSearchLoader = new SavedObjectLoader(SavedSearch, kbnIndex, esAdmin, kbnUrl);
module.service('savedSearches', function (Promise, config, kbnIndex, esAdmin, createNotifier, SavedSearch, kbnUrl, $http) {
const savedSearchLoader = new SavedObjectLoader(SavedSearch, kbnIndex, esAdmin, kbnUrl, $http);
// Customize loader properties since adding an 's' on type doesn't work for type 'search' .
savedSearchLoader.loaderProperties = {
name: 'searches',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ savedObjectManagementRegistry.register({
title: 'visualizations'
});

app.service('savedVisualizations', function (Promise, esAdmin, kbnIndex, SavedVis, Private, Notifier, kbnUrl) {
app.service('savedVisualizations', function (Promise, esAdmin, kbnIndex, SavedVis, Private, Notifier, kbnUrl, $http) {
const visTypes = Private(VisTypesRegistryProvider);
const notify = new Notifier({
location: 'Saved Visualization Service'
});

const saveVisualizationLoader = new SavedObjectLoader(SavedVis, kbnIndex, esAdmin, kbnUrl);
saveVisualizationLoader.mapHits = function (hit) {
const source = hit._source;
source.id = hit._id;
source.url = this.urlFor(hit._id);
const saveVisualizationLoader = new SavedObjectLoader(SavedVis, kbnIndex, esAdmin, kbnUrl, $http);

saveVisualizationLoader.mapHitSource = function (source, id) {
source.id = id;
source.url = this.urlFor(id);

let typeName = source.typeName;
if (source.visState) {
Expand All @@ -32,8 +32,8 @@ app.service('savedVisualizations', function (Promise, esAdmin, kbnIndex, SavedVi
}

if (!typeName || !visTypes.byName[typeName]) {
if (!typeName) notify.error('Visualization type is missing. Please add a type to this visualization.', hit);
else notify.error('Visualization type of "' + typeName + '" is invalid. Please change to a valid type.', hit);
if (!typeName) notify.error('Visualization type is missing. Please add a type to this visualization.', source);
else notify.error('Visualization type of "' + typeName + '" is invalid. Please change to a valid type.', source);
return kbnUrl.redirect('/management/kibana/objects/savedVisualizations/{{id}}', { id: source.id });
}

Expand All @@ -45,6 +45,5 @@ app.service('savedVisualizations', function (Promise, esAdmin, kbnIndex, SavedVi
saveVisualizationLoader.urlFor = function (id) {
return kbnUrl.eval('#/visualize/edit/{{id}}', { id: id });
};

return saveVisualizationLoader;
});
4 changes: 2 additions & 2 deletions src/core_plugins/timelion/public/services/saved_sheets.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ define(function (require) {
});

// This is the only thing that gets injected into controllers
module.service('savedSheets', function (Promise, SavedSheet, kbnIndex, esAdmin, kbnUrl) {
const savedSheetLoader = new SavedObjectLoader(SavedSheet, kbnIndex, esAdmin, kbnUrl);
module.service('savedSheets', function (Promise, SavedSheet, kbnIndex, esAdmin, kbnUrl, $http) {
const savedSheetLoader = new SavedObjectLoader(SavedSheet, kbnIndex, esAdmin, kbnUrl, $http);
savedSheetLoader.urlFor = function (id) {
return kbnUrl.eval('#/{{id}}', { id: id });
};
Expand Down
37 changes: 17 additions & 20 deletions src/ui/public/courier/saved_object/get_title_already_exists.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import _ from 'lodash';
import { find } from 'lodash';
/**
* Returns true if the given saved object has a title that already exists, false otherwise. Search is case
* insensitive.
Expand All @@ -7,30 +7,27 @@ import _ from 'lodash';
* @returns {Promise<string|undefined>} Returns the title that matches. Because this search is not case
* sensitive, it may not exactly match the title of the object.
*/
export function getTitleAlreadyExists(savedObject, esAdmin) {
const { index, title, id } = savedObject;
const esType = savedObject.getEsType();
export function getTitleAlreadyExists(savedObject, savedObjectsClient) {
const { title, id } = savedObject;
const type = savedObject.getEsType();
if (!title) {
throw new Error('Title must be supplied');
}

const body = {
query: {
bool: {
must: { match_phrase: { title } },
must_not: { match: { id } }
}
}
};

// Elastic search will return the most relevant results first, which means exact matches should come
// first, and so we shouldn't need to request everything. Using 10 just to be on the safe side.
const size = 10;
return esAdmin.search({ index, type: esType, body, size })
.then((response) => {
const match = _.find(response.hits.hits, function currentVersion(hit) {
return hit._source.title.toLowerCase() === title.toLowerCase();
});
return match ? match._source.title : undefined;
const perPage = 10;
return savedObjectsClient.find({
type,
perPage,
search: title,
searchFields: 'title',
fields: ['title']
}).then((response) => {
const match = find(response.savedObjects, (obj) => {
return obj.id !== id && obj.get('title').toLowerCase() === title.toLowerCase();
});

return match ? match.get('title') : undefined;
});
}
89 changes: 21 additions & 68 deletions src/ui/public/courier/saved_object/saved_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@

import angular from 'angular';
import _ from 'lodash';
import chrome from 'ui/chrome';

import { SavedObjectNotFound } from 'ui/errors';
import uuid from 'node-uuid';
import MappingSetupProvider from 'ui/utils/mapping_setup';

import { AdminDocSourceProvider } from '../data_source/admin_doc_source';
import { SearchSourceProvider } from '../data_source/search_source';
import { getTitleAlreadyExists } from './get_title_already_exists';
import { SavedObjectsClient } from 'ui/saved_objects';

/**
* An error message to be used when the user rejects a confirm overwrite.
Expand All @@ -40,20 +40,18 @@ function isErrorNonFatal(error) {
return error.message === OVERWRITE_REJECTED || error.message === SAVE_DUPLICATE_REJECTED;
}

export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifier, confirmModalPromise, indexPatterns) {
export function SavedObjectProvider($http, $q, esAdmin, kbnIndex, Promise, Private, Notifier, confirmModalPromise, indexPatterns) {

const DocSource = Private(AdminDocSourceProvider);
const SearchSource = Private(SearchSourceProvider);
const mappingSetup = Private(MappingSetupProvider);
const savedObjectsClient = new SavedObjectsClient($http, chrome.getBasePath(), $q);

function SavedObject(config) {
if (!_.isObject(config)) config = {};

/************
* Initialize config vars
************/
// the doc which is used to store this object
const docSource = new DocSource();

// type name for this object, used as the ES-type
const esType = config.type;
Expand All @@ -77,11 +75,6 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
this.isSaving = false;
this.defaults = config.defaults || {};

// Create a notifier for sending alerts
const notify = new Notifier({
location: 'Saved ' + this.getDisplayName()
});

// mapping definition for the fields that this object will expose
const mapping = mappingSetup.expandShorthand(config.mapping);

Expand Down Expand Up @@ -163,12 +156,6 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
// ensure that the esType is defined
if (!esType) throw new Error('You must define a type name to use SavedObject objects.');

// tell the docSource where to find the doc
docSource
.index(kbnIndex)
.type(esType)
.id(this.id);

// check that the mapping for this esType is defined
return mappingSetup.isDefined(esType)
.then(function (defined) {
Expand Down Expand Up @@ -198,7 +185,7 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
}

// fetch the object from ES
return docSource.fetch().then(this.applyESResp);
return savedObjectsClient.get(esType, this.id).then(this.applyESResp);
})
.then(() => {
return customInit.call(this);
Expand All @@ -210,12 +197,12 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
});

this.applyESResp = (resp) => {
this._source = _.cloneDeep(resp._source);
this._source = _.cloneDeep(resp._attributes);

if (resp.found != null && !resp.found) throw new SavedObjectNotFound(esType, this.id);
if (!resp._version) throw new SavedObjectNotFound(esType, this.id);

const meta = resp._source.kibanaSavedObjectMeta || {};
delete resp._source.kibanaSavedObjectMeta;
const meta = resp._attributes.kibanaSavedObjectMeta || {};
delete resp._attributes.kibanaSavedObjectMeta;

if (!config.indexPattern && this._source.indexPattern) {
config.indexPattern = this._source.indexPattern;
Expand All @@ -239,14 +226,9 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
return Promise.try(() => {
parseSearchSource(meta.searchSourceJSON);
return hydrateIndexPattern();
})
.then(() => {
return Promise.cast(afterESResp.call(this, resp));
})
.then(() => {
// Any time obj is updated, re-call applyESResp
docSource.onUpdate().then(this.applyESResp, notify.fatal);
});
}).then(() => {
return Promise.cast(afterESResp.call(this, resp));
});
};

/**
Expand Down Expand Up @@ -290,32 +272,6 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
return esAdmin.indices.refresh({ index: kbnIndex });
}

/**
* Attempts to create the current object using the serialized source. If an object already
* exists, a warning message requests an overwrite confirmation.
* @param source - serialized version of this object (return value from this.serialize())
* What will be indexed into elasticsearch.
* @returns {Promise} - A promise that is resolved with the objects id if the object is
* successfully indexed. If the overwrite confirmation was rejected, an error is thrown with
* a confirmRejected = true parameter so that case can be handled differently than
* a create or index error.
* @resolved {String} - The id of the doc
*/
const createSource = (source) => {
return docSource.doCreate(source)
.catch((err) => {
// record exists, confirm overwriting
if (_.get(err, 'origError.status') === 409) {
const confirmMessage = `Are you sure you want to overwrite ${this.title}?`;

return confirmModalPromise(confirmMessage, { confirmButtonText: `Overwrite ${this.getDisplayName()}` })
.then(() => docSource.doIndex(source))
.catch(() => Promise.reject(new Error(OVERWRITE_REJECTED)));
}
return Promise.reject(err);
});
};

/**
* Returns a promise that resolves to true if either the title is unique, or if the user confirmed they
* wished to save the duplicate title. Promise is rejected if the user rejects the confirmation.
Expand All @@ -327,7 +283,7 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
return Promise.resolve();
}

return getTitleAlreadyExists(this, esAdmin)
return getTitleAlreadyExists(this, savedObjectsClient)
.then((duplicateTitle) => {
if (!duplicateTitle) return true;
const confirmMessage =
Expand All @@ -347,11 +303,10 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
/**
* Saves this object.
*
* @param {SaveOptions} saveOptions?
* @return {Promise}
* @resolved {String} - The id of the doc
*/
this.save = (saveOptions = {}) => {
this.save = () => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can get rid of confirmOverwrite. In saved object management, we want to confirm overwrite on importing all the objects. I think with this new implementation it won't warn because it'll use the update call instead of create.

You can see where confirmOverwrite: true is passed in in kibana/src/core_plugins/kibana/public/management/sections/objects/_objects.js

// Save the original id in case the save fails.
const originalId = this.id;
// Read https://github.com/elastic/kibana/issues/9056 and
Expand All @@ -364,21 +319,20 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
this.id = null;
}

// Create a unique id for this object if it doesn't have one already.
this.id = this.id || uuid.v1();
// ensure that the docSource has the current id
docSource.id(this.id);

const source = this.serialize();

this.isSaving = true;

return warnIfDuplicateTitle()
.then(() => {
return saveOptions.confirmOverwrite ? createSource(source) : docSource.doIndex(source);
if (this.id) {
return savedObjectsClient.update(esType, this.id, source);
} else {
return savedObjectsClient.create(esType, source);
}
})
.then((id) => {
this.id = id;
.then((resp) => {
this.id = resp.id;
})
.then(refreshIndex)
.then(() => {
Expand All @@ -397,7 +351,6 @@ export function SavedObjectProvider(esAdmin, kbnIndex, Promise, Private, Notifie
};

this.destroy = () => {
docSource.cancelQueued();
if (this.searchSource) {
this.searchSource.cancelQueued();
}
Expand Down
Loading