Code review updates

elastic · May 8, 2018 · 7538bf3 · 7538bf3
1 parent f208d3c
commit 7538bf3
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 59 deletions.
diff --git a/src/server/saved_objects/client/lib/search_dsl/query_params.js b/src/server/saved_objects/client/lib/search_dsl/query_params.js
@@ -27,19 +27,27 @@ function getFieldsForTypes(searchFields, types) {
  *  @param  {Object} type
  *  @param  {String} search
  *  @param  {Array<string>} searchFields
+ *  @param  {Array<object>} extraFilters
  *  @return {Object}
  */
-export function getQueryParams(mappings, type, search, searchFields) {
+export function getQueryParams(mappings, type, search, searchFields, extraFilters) {
   if (!type && !search) {
     return {};
   }
 
   const bool = {};
 
+  const filters = [];
   if (type) {
-    bool.filter = [
-      { term: { type } }
-    ];
+    filters.push({ term: { type } });
+  }
+
+  if (extraFilters) {
+    filters.push(...extraFilters);
+  }
+
+  if (filters.length > 0) {
+    bool.filter = filters;
   }
 
   if (search) {

diff --git a/src/server/saved_objects/client/lib/search_dsl/search_dsl.js b/src/server/saved_objects/client/lib/search_dsl/search_dsl.js
@@ -9,7 +9,8 @@ export function getSearchDsl(mappings, options = {}) {
     search,
     searchFields,
     sortField,
-    sortOrder
+    sortOrder,
+    extraFilters,
   } = options;
 
   if (!type && sortField) {
@@ -20,8 +21,12 @@ export function getSearchDsl(mappings, options = {}) {
     throw Boom.notAcceptable('sortOrder requires a sortField');
   }
 
+  if (extraFilters && !Array.isArray(extraFilters)) {
+    throw Boom.notAcceptable('extraFilters must be an array');
+  }
+
   return {
-    ...getQueryParams(mappings, type, search, searchFields),
+    ...getQueryParams(mappings, type, search, searchFields, extraFilters),
     ...getSortingParams(mappings, type, sortField, sortOrder),
   };
 }
diff --git a/src/server/saved_objects/client/saved_objects_client.js b/src/server/saved_objects/client/saved_objects_client.js
@@ -121,10 +121,10 @@ export class SavedObjectsClient {
         index: this._index,
         refresh: 'wait_for',
         body: {
+          ...extraBodyProperties,
           type,
           updated_at: time,
           [type]: attributes,
-          ...extraBodyProperties
         },
       });
 
@@ -275,7 +275,7 @@ export class SavedObjectsClient {
       sortField,
       sortOrder,
       fields,
-      queryDecorator,
+      extraFilters,
     } = options;
 
     if (searchFields && !Array.isArray(searchFields)) {
@@ -286,6 +286,10 @@ export class SavedObjectsClient {
       throw new TypeError('options.searchFields must be an array');
     }
 
+    if (extraFilters && !Array.isArray(extraFilters)) {
+      throw new TypeError('options.extraFilters must be an array');
+    }
+
     const esOptions = {
       index: this._index,
       size: perPage,
@@ -299,15 +303,12 @@ export class SavedObjectsClient {
           searchFields,
           type,
           sortField,
-          sortOrder
+          sortOrder,
+          extraFilters
         })
       }
     };
 
-    if (esOptions.body.query && typeof queryDecorator === 'function') {
-      esOptions.body.query = queryDecorator(esOptions.body.query);
-    }
-
     const response = await this._callCluster('search', esOptions);
 
     if (response.status === 404) {
@@ -372,6 +373,8 @@ export class SavedObjectsClient {
       docsToReturn = docs.filter(options.documentFilter);
     }
 
+    const { extraSourceProperties = [] } = options;
+
     return {
       saved_objects: docsToReturn.map((doc, i) => {
         const { id, type } = objects[i];
@@ -390,12 +393,14 @@ export class SavedObjectsClient {
           type,
           ...time && { updated_at: time },
           version: doc._version,
-          attributes: doc._source[type]
-        };
+          attributes: {
+            ...extraSourceProperties
+              .map(s => doc._source[s])
+              .reduce((acc, prop) => ({ ...acc, ...prop }), {}),
 
-        if (typeof options.resultDecorator === 'function') {
-          return options.resultDecorator(savedObject, doc);
-        }
+            ...doc._source[type],
+          }
+        };
 
         return savedObject;
       })
@@ -417,25 +422,29 @@ export class SavedObjectsClient {
       ignore: [404]
     });
 
-    if (typeof options.responseInterceptor === 'function') {
-      options.responseInterceptor(response);
-    }
-
     const docNotFound = response.found === false;
     const indexNotFound = response.status === 404;
     if (docNotFound || indexNotFound) {
       // see "404s from missing index" above
       throw errors.createGenericNotFoundError();
     }
 
+    const { extraSourceProperties = [] } = options;
+
     const { updated_at: updatedAt } = response._source;
 
     return {
       id,
       type,
       ...updatedAt && { updated_at: updatedAt },
       version: response._version,
-      attributes: response._source[type]
+      attributes: {
+        ...extraSourceProperties
+          .map(s => response._source[s])
+          .reduce((acc, prop) => ({ ...acc, ...prop }), {}),
+
+        ...response._source[type],
+      }
     };
   }
 
@@ -459,9 +468,9 @@ export class SavedObjectsClient {
       ignore: [404],
       body: {
         doc: {
+          ...options.extraBodyProperties,
           updated_at: time,
           [type]: attributes,
-          ...options.extraBodyProperties
         }
       },
     });

diff --git a/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.js b/x-pack/plugins/spaces/server/lib/saved_objects_client/spaces_saved_objects_client.js
@@ -50,22 +50,13 @@ export class SpacesSavedObjectsClient {
 
     if (this._isTypeSpaceAware(options.type)) {
       const spaceId = await this._getSpaceId();
-
-      spaceOptions.queryDecorator = (query) => {
-        const { bool = {} } = query;
-
-        if (!Array.isArray(bool.filter)) {
-          bool.filter = [];
-        }
-
-        bool.filter.push({
+      if (spaceId) {
+        spaceOptions.extraFilters = [{
           term: {
             spaceId
           }
-        });
-
-        return query;
-      };
+        }];
+      }
     }
 
     return await this._client.find({ ...options, ...spaceOptions });
@@ -76,6 +67,7 @@ export class SpacesSavedObjectsClient {
     const thisSpaceId = await this._getSpaceId();
 
     return await this._client.bulkGet(objects, {
+      extraSourceProperties: ['spaceId'],
       documentFilter: (doc) => {
         if (!doc.found) return true;
 
@@ -86,13 +78,6 @@ export class SpacesSavedObjectsClient {
         }
 
         return true;
-      },
-      resultDecorator(savedObject, doc) {
-        savedObject.attributes = {
-          ...savedObject.attributes,
-          spaceId: doc._source.spaceId
-        };
-        return savedObject;
       }
     });
   }
@@ -105,26 +90,21 @@ export class SpacesSavedObjectsClient {
       thisSpaceId = await this._getSpaceId();
     }
 
-    return await this._client.get(type, id, {
-      responseInterceptor: (response) => {
-        if (!this._isTypeSpaceAware(type)) {
-          return response;
-        }
+    const response = await this._client.get(type, id, {
+      extraSourceProperties: ['spaceId']
+    });
 
-        if (response.found && response.status !== 404) {
-          const { spaceId } = response._source;
-          if (spaceId !== thisSpaceId) {
-            response.found = false;
-            response._source = {};
-          }
-        }
+    const { spaceId: objectSpaceId } = response.attributes;
 
-        return response;
-      }
-    });
+    if (objectSpaceId !== thisSpaceId) {
+      throw this._client.errors.createGenericNotFoundError();
+    }
+
+    return response;
   }
 
   async update(type, id, attributes, options = {}) {
+    attributes.spaceId = await this._getSpaceId();
     return await this._client.update(type, id, attributes, options);
   }