-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Saved Object Namespaces #22357
Saved Object Namespaces #22357
Changes from 23 commits
a8c38b7
a3c92ad
5e37485
c90476b
6ec276f
7cfc737
27c60c0
5b937bb
709dffd
e06a0ee
ff08700
57487af
f877510
c9059e7
2c4f135
636d583
c05b369
ad190b9
3bb65dd
a2c19ec
c424230
607b807
002e3b0
d213cd8
95ef4b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`#addNamespaceAgnosticType can't add Symbol type 1`] = `"type must be a string"`; | ||
|
||
exports[`#addNamespaceAgnosticType can't add a bool type 1`] = `"type must be a string"`; | ||
|
||
exports[`#addNamespaceAgnosticType can't add function type 1`] = `"type must be a string"`; | ||
|
||
exports[`#addNamespaceAgnosticType can't add null type 1`] = `"type must be a string"`; | ||
|
||
exports[`#addNamespaceAgnosticType can't add number type 1`] = `"type must be a string"`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,12 +35,14 @@ export class SavedObjectsRepository { | |
const { | ||
index, | ||
mappings, | ||
schema, | ||
callCluster, | ||
onBeforeWrite = () => { }, | ||
} = options; | ||
|
||
this._index = index; | ||
this._mappings = mappings; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would make sense for mappings and schema to merge at some point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I completely agree. |
||
this._schema = schema; | ||
this._type = getRootType(this._mappings); | ||
this._onBeforeWrite = onBeforeWrite; | ||
this._unwrappedCallCluster = callCluster; | ||
|
@@ -54,35 +56,35 @@ export class SavedObjectsRepository { | |
* @param {object} [options={}] | ||
* @property {string} [options.id] - force id on creation, not recommended | ||
* @property {boolean} [options.overwrite=false] | ||
* @property {object} [options.extraDocumentProperties={}] - extra properties to append to the document body, outside of the object's type property | ||
* @property {string} [options.namespace] | ||
* @returns {promise} - { id, type, version, attributes } | ||
*/ | ||
async create(type, attributes = {}, options = {}) { | ||
const { | ||
id, | ||
extraDocumentProperties = {}, | ||
overwrite = false | ||
overwrite = false, | ||
namespace, | ||
} = options; | ||
|
||
const method = id && !overwrite ? 'create' : 'index'; | ||
const time = this._getCurrentTime(); | ||
|
||
try { | ||
const response = await this._writeToCluster(method, { | ||
id: this._generateEsId(type, id), | ||
id: this._generateEsId(namespace, type, id), | ||
type: this._type, | ||
index: this._index, | ||
refresh: 'wait_for', | ||
body: { | ||
...extraDocumentProperties, | ||
...namespace && !this._schema.isNamespaceAgnostic(type) && { namespace }, | ||
type, | ||
updated_at: time, | ||
[type]: attributes, | ||
}, | ||
}); | ||
|
||
return { | ||
id: trimIdPrefix(response._id, type), | ||
id: trimIdPrefix(this._schema, response._id, namespace, type), | ||
type, | ||
updated_at: time, | ||
version: response._version, | ||
|
@@ -101,14 +103,16 @@ export class SavedObjectsRepository { | |
/** | ||
* Creates multiple documents at once | ||
* | ||
* @param {array} objects - [{ type, id, attributes, extraDocumentProperties }] | ||
* @param {array} objects - [{ type, id, attributes }] | ||
* @param {object} [options={}] | ||
* @property {boolean} [options.overwrite=false] - overwrites existing documents | ||
* @property {string} [options.namespace] | ||
* @returns {promise} - {saved_objects: [[{ id, type, version, attributes, error: { message } }]} | ||
*/ | ||
async bulkCreate(objects, options = {}) { | ||
const { | ||
overwrite = false | ||
overwrite = false, | ||
namespace | ||
} = options; | ||
const time = this._getCurrentTime(); | ||
const objectToBulkRequest = (object) => { | ||
|
@@ -117,12 +121,12 @@ export class SavedObjectsRepository { | |
return [ | ||
{ | ||
[method]: { | ||
_id: this._generateEsId(object.type, object.id), | ||
_id: this._generateEsId(namespace, object.type, object.id), | ||
_type: this._type, | ||
} | ||
}, | ||
{ | ||
...object.extraDocumentProperties, | ||
... namespace && !this._schema.isNamespaceAgnostic(object.type) && { namespace }, | ||
type: object.type, | ||
updated_at: time, | ||
[object.type]: object.attributes, | ||
|
@@ -186,11 +190,17 @@ export class SavedObjectsRepository { | |
* | ||
* @param {string} type | ||
* @param {string} id | ||
* @param {object} [options={}] | ||
* @property {string} [options.namespace] | ||
* @returns {promise} | ||
*/ | ||
async delete(type, id) { | ||
async delete(type, id, options = {}) { | ||
const { | ||
namespace | ||
} = options; | ||
|
||
const response = await this._writeToCluster('delete', { | ||
id: this._generateEsId(type, id), | ||
id: this._generateEsId(namespace, type, id), | ||
type: this._type, | ||
index: this._index, | ||
refresh: 'wait_for', | ||
|
@@ -220,12 +230,12 @@ export class SavedObjectsRepository { | |
* @property {string} [options.search] | ||
* @property {Array<string>} [options.searchFields] - see Elasticsearch Simple Query String | ||
* Query field argument for more information | ||
* @property {object} [options.filters] - ES Query filters to append | ||
* @property {integer} [options.page=1] | ||
* @property {integer} [options.perPage=20] | ||
* @property {string} [options.sortField] | ||
* @property {string} [options.sortOrder] | ||
* @property {Array<string>} [options.fields] | ||
* @property {string} [options.namespace] | ||
* @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page } | ||
*/ | ||
async find(options = {}) { | ||
|
@@ -238,7 +248,7 @@ export class SavedObjectsRepository { | |
sortField, | ||
sortOrder, | ||
fields, | ||
filters, | ||
namespace, | ||
} = options; | ||
|
||
if (searchFields && !Array.isArray(searchFields)) { | ||
|
@@ -249,10 +259,6 @@ export class SavedObjectsRepository { | |
throw new TypeError('options.searchFields must be an array'); | ||
} | ||
|
||
if (filters && !Array.isArray(filters)) { | ||
throw new TypeError('options.filters must be an array'); | ||
} | ||
|
||
const esOptions = { | ||
index: this._index, | ||
size: perPage, | ||
|
@@ -261,13 +267,13 @@ export class SavedObjectsRepository { | |
ignore: [404], | ||
body: { | ||
version: true, | ||
...getSearchDsl(this._mappings, { | ||
...getSearchDsl(this._mappings, this._schema, { | ||
namespace, | ||
search, | ||
searchFields, | ||
type, | ||
sortField, | ||
sortOrder, | ||
filters | ||
}) | ||
} | ||
}; | ||
|
@@ -292,7 +298,7 @@ export class SavedObjectsRepository { | |
saved_objects: response.hits.hits.map(hit => { | ||
const { type, updated_at: updatedAt } = hit._source; | ||
return { | ||
id: trimIdPrefix(hit._id, type), | ||
id: trimIdPrefix(this._schema, hit._id, namespace, type), | ||
type, | ||
...updatedAt && { updated_at: updatedAt }, | ||
version: hit._version, | ||
|
@@ -306,8 +312,8 @@ export class SavedObjectsRepository { | |
* Returns an array of objects by id | ||
* | ||
* @param {array} objects - an array ids, or an array of objects containing id and optionally type | ||
* @param {object} [options = {}] | ||
* @param {array} [options.extraDocumentProperties = []] - an array of extra properties to return from the underlying document | ||
* @param {object} [options={}] | ||
* @property {string} [options.namespace] | ||
* @returns {promise} - { saved_objects: [{ id, type, version, attributes }] } | ||
* @example | ||
* | ||
|
@@ -317,6 +323,10 @@ export class SavedObjectsRepository { | |
* ]) | ||
*/ | ||
async bulkGet(objects = [], options = {}) { | ||
const { | ||
namespace | ||
} = options; | ||
|
||
if (objects.length === 0) { | ||
return { saved_objects: [] }; | ||
} | ||
|
@@ -325,16 +335,14 @@ export class SavedObjectsRepository { | |
index: this._index, | ||
body: { | ||
docs: objects.map(object => ({ | ||
_id: this._generateEsId(object.type, object.id), | ||
_id: this._generateEsId(namespace, object.type, object.id), | ||
_type: this._type, | ||
})) | ||
} | ||
}); | ||
|
||
const { docs } = response; | ||
|
||
const { extraDocumentProperties = [] } = options; | ||
|
||
return { | ||
saved_objects: docs.map((doc, i) => { | ||
const { id, type } = objects[i]; | ||
|
@@ -353,9 +361,6 @@ export class SavedObjectsRepository { | |
type, | ||
...time && { updated_at: time }, | ||
version: doc._version, | ||
...extraDocumentProperties | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, Now, we are still restricting to the current space, but we aren't using this property to do so. We are using the composite document id to perform this check instead. This is more efficient than what I had, but it deviates from the property being the authoritative source of this information. In other words, we aren't using the I'm not necessarily opposed to changing our direction, but if I understand this right, the On the other hand, adding the code back in to check the What are your thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I shared your hesitation when initially making these changes, and it does remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense -- thanks for drawing the parallel to how we use the |
||
.map(s => ({ [s]: doc._source[s] })) | ||
.reduce((acc, prop) => ({ ...acc, ...prop }), {}), | ||
attributes: { | ||
...doc._source[type], | ||
} | ||
|
@@ -371,13 +376,17 @@ export class SavedObjectsRepository { | |
* | ||
* @param {string} type | ||
* @param {string} id | ||
* @param {object} [options = {}] | ||
* @param {array} [options.extraDocumentProperties = []] - an array of extra properties to return from the underlying document | ||
* @param {object} [options={}] | ||
* @property {string} [options.namespace] | ||
* @returns {promise} - { id, type, version, attributes } | ||
*/ | ||
async get(type, id, options = {}) { | ||
const { | ||
namespace | ||
} = options; | ||
|
||
const response = await this._callCluster('get', { | ||
id: this._generateEsId(type, id), | ||
id: this._generateEsId(namespace, type, id), | ||
type: this._type, | ||
index: this._index, | ||
ignore: [404] | ||
|
@@ -390,18 +399,13 @@ export class SavedObjectsRepository { | |
throw errors.createGenericNotFoundError(type, id); | ||
} | ||
|
||
const { extraDocumentProperties = [] } = options; | ||
|
||
const { updated_at: updatedAt } = response._source; | ||
|
||
return { | ||
id, | ||
type, | ||
...updatedAt && { updated_at: updatedAt }, | ||
version: response._version, | ||
...extraDocumentProperties | ||
.map(s => ({ [s]: response._source[s] })) | ||
.reduce((acc, prop) => ({ ...acc, ...prop }), {}), | ||
attributes: { | ||
...response._source[type], | ||
} | ||
|
@@ -415,21 +419,26 @@ export class SavedObjectsRepository { | |
* @param {string} id | ||
* @param {object} [options={}] | ||
* @property {integer} options.version - ensures version matches that of persisted object | ||
* @param {array} [options.extraDocumentProperties = {}] - an object of extra properties to write into the underlying document | ||
* @property {string} [options.namespace] | ||
* @returns {promise} | ||
*/ | ||
async update(type, id, attributes, options = {}) { | ||
const { | ||
version, | ||
namespace | ||
} = options; | ||
|
||
const time = this._getCurrentTime(); | ||
const response = await this._writeToCluster('update', { | ||
id: this._generateEsId(type, id), | ||
id: this._generateEsId(namespace, type, id), | ||
type: this._type, | ||
index: this._index, | ||
version: options.version, | ||
version, | ||
refresh: 'wait_for', | ||
ignore: [404], | ||
body: { | ||
doc: { | ||
...options.extraDocumentProperties, | ||
...namespace && !this._schema.isNamespaceAgnostic(type) && { namespace }, | ||
updated_at: time, | ||
[type]: attributes, | ||
} | ||
|
@@ -467,8 +476,9 @@ export class SavedObjectsRepository { | |
} | ||
} | ||
|
||
_generateEsId(type, id) { | ||
return `${type}:${id || uuid.v1()}`; | ||
_generateEsId(namespace, type, id) { | ||
const namespacePrefix = namespace && !this._schema.isNamespaceAgnostic(type) ? `${namespace}:` : ''; | ||
return `${namespacePrefix}${type}:${id || uuid.v1()}`; | ||
} | ||
|
||
_getCurrentTime() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the base mappings have a
namespace
property, we can probably remove thespaceId
property from the Spaces plugin mappings.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and all associated esArchiver mappings 😉