SavedObjectClient.find multiple types (#19231)

* Making the SavedObjectsClient.find accept a string or string[] for type

* Searching is now filtered to actual types

* Adding multi-type sort

* Changing another use of includeTypes

* Fixing test's reliance on type

* Changing the find route to take type as an array

* Can't sort on type... it's not a property on the object

* Only allowing sorting on multiple types if it's a root property

* Expanding indicator of root property object

* Sorting by type, now that it's allowed and one of the few sort columns

* Adjusting comment

* Throwing better error message

src/core_plugins/kibana/public/management/sections/objects/components/objects_table/__tests__/objects_table.test.js

@@ -96,8 +96,8 @@ $http.post = jest.fn().mockImplementation(() => ([]));
 const defaultProps = {
   savedObjectsClient: {
     find: jest.fn().mockImplementation(({ type }) => {
-      // We pass in type when fetching counts
-      if (type) {
+      // We pass in a single type when fetching counts
+      if (type && !Array.isArray(type)) {
         return {
           total: 1,
           savedObjects: [

src/core_plugins/kibana/public/management/sections/objects/components/objects_table/objects_table.js

@@ -92,13 +92,13 @@ export class ObjectsTable extends Component {
 
   fetchCounts = async () => {
     const { queryText, visibleTypes } = parseQuery(this.state.activeQuery);
-    const includeTypes = INCLUDED_TYPES.filter(
+    const type = INCLUDED_TYPES.filter(
       type => !visibleTypes || visibleTypes.includes(type)
     );
 
     const savedObjectCounts = await getSavedObjectCounts(
       this.props.$http,
-      includeTypes,
+      type,
       queryText
     );
 
@@ -130,7 +130,7 @@ export class ObjectsTable extends Component {
     let savedObjects = [];
     let totalItemCount = 0;
 
-    const includeTypes = INCLUDED_TYPES.filter(
+    const type = INCLUDED_TYPES.filter(
       type => !visibleTypes || visibleTypes.includes(type)
     );
 
@@ -144,7 +144,7 @@ export class ObjectsTable extends Component {
           sortField: 'type',
           fields: ['title', 'id'],
           searchFields: ['title'],
-          includeTypes,
+          type,
         });
 
         savedObjects = data.savedObjects.map(savedObject => ({

src/core_plugins/kibana/server/routes/api/management/saved_objects/scroll.js

@@ -65,7 +65,7 @@ export function registerScrollForCountRoute(server) {
     handler: async (req, reply) => {
       const savedObjectsClient = req.getSavedObjectsClient();
       const findOptions = {
-        includeTypes: req.payload.typesToInclude,
+        type: req.payload.typesToInclude,
         perPage: 1000,
       };
 

src/server/mappings/index.js

@@ -7,4 +7,5 @@ export {
   getRootType,
   getProperty,
   getRootProperties,
+  getRootPropertiesObjects,
 } from './lib';

src/server/mappings/lib/get_root_properties_objects.js

@@ -0,0 +1,28 @@
+import { getRootProperties } from './get_root_properties';
+
+/**
+ *  Get the property mappings for the root type in the EsMappingsDsl
+ *  where the properties are objects
+ *
+ *  If the mappings don't have a root type, or the root type is not
+ *  an object type (it's a keyword or something) this function will
+ *  throw an error.
+ *
+ *  This data can be found at `{indexName}.mappings.{typeName}.properties`
+ *  in the es indices.get() response where the properties are objects.
+ *
+ *  @param  {EsMappingsDsl} mappings
+ *  @return {EsPropertyMappings}
+ */
+export function getRootPropertiesObjects(mappings) {
+  const rootProperties = getRootProperties(mappings);
+  return Object.entries(rootProperties).reduce((acc, [key, value]) => {
+
+    // we consider the existence of the properties or type of object to designate that this is an object datatype
+    if (value.properties || value.type === 'object') {
+      acc[key] = value;
+    }
+
+    return acc;
+  }, {});
+}

src/server/mappings/lib/get_root_properties_objects.test.js

@@ -0,0 +1,166 @@
+import { getRootPropertiesObjects } from './get_root_properties_objects';
+
+test(`returns single object with properties`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          properties: {}
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({
+    foo: {
+      properties: {}
+    }
+  });
+});
+
+test(`returns single object with type === 'object'`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          type: 'object'
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({
+    foo: {
+      type: 'object'
+    }
+  });
+});
+
+test(`returns two objects with properties`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          properties: {}
+        },
+        bar: {
+          properties: {}
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({
+    foo: {
+      properties: {}
+    },
+    bar: {
+      properties: {}
+    }
+  });
+});
+
+test(`returns two objects with type === 'object'`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          type: 'object'
+        },
+        bar: {
+          type: 'object'
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({
+    foo: {
+      type: 'object'
+    },
+    bar: {
+      type: 'object'
+    }
+  });
+});
+
+test(`excludes objects without properties and type of keyword`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          type: 'keyword'
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({});
+});
+
+test(`excludes two objects without properties and type of keyword`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          type: 'keyword'
+        },
+        bar: {
+          type: 'keyword'
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({});
+});
+
+test(`includes one object with properties and excludes one object without properties`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          properties: {}
+        },
+        bar: {
+          type: 'keyword'
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({
+    foo: {
+      properties: {}
+    }
+  });
+});
+
+test(`includes one object with type === 'object' and excludes one object without properties`, () => {
+  const mappings = {
+    rootType: {
+      properties: {
+        foo: {
+          type: 'object'
+        },
+        bar: {
+          type: 'keyword'
+        }
+      }
+    }
+  };
+
+  const result = getRootPropertiesObjects(mappings);
+  expect(result).toEqual({
+    foo: {
+      type: 'object'
+    }
+  });
+});

src/server/mappings/lib/index.js

@@ -2,3 +2,4 @@ export { getProperty } from './get_property';
 export { getTypes } from './get_types';
 export { getRootType } from './get_root_type';
 export { getRootProperties } from './get_root_properties';
+export { getRootPropertiesObjects } from './get_root_properties_objects';

src/server/saved_objects/client/lib/repository.js

@@ -184,7 +184,7 @@ export class SavedObjectsRepository {
 
   /**
    * @param {object} [options={}]
-   * @property {string} [options.type]
+   * @property {(string|Array<string>)} [options.type]
    * @property {string} [options.search]
    * @property {Array<string>} [options.searchFields] - see Elasticsearch Simple Query String
    *                                        Query field argument for more information
@@ -205,7 +205,6 @@ export class SavedObjectsRepository {
       sortField,
       sortOrder,
       fields,
-      includeTypes,
     } = options;
 
     if (searchFields && !Array.isArray(searchFields)) {
@@ -228,7 +227,6 @@ export class SavedObjectsRepository {
           search,
           searchFields,
           type,
-          includeTypes,
           sortField,
           sortOrder
         })

src/server/saved_objects/client/lib/repository.test.js

@@ -376,7 +376,6 @@ describe('SavedObjectsRepository', () => {
         type: 'bar',
         sortField: 'name',
         sortOrder: 'desc',
-        includeTypes: ['index-pattern', 'dashboard'],
       };
 
       await savedObjectsRepository.find(relevantOpts);

src/server/saved_objects/client/lib/search_dsl/__snapshots__/sorting_params.test.js.snap

@@ -0,0 +1,9 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`searchDsl/getSortParams sort with direction sortField is non-root multi-field with multiple types returns correct params 1`] = `"Unable to sort multiple types by field title.raw, not a root field"`;
+
+exports[`searchDsl/getSortParams sort with direction sortFields is non-root simple property with multiple types returns correct params 1`] = `"Unable to sort multiple types by field title, not a root field"`;
+
+exports[`searchDsl/getSortParams sortField no direction sortField is not-root multi-field with multiple types returns correct params 1`] = `"Unable to sort multiple types by field title.raw, not a root field"`;
+
+exports[`searchDsl/getSortParams sortField no direction sortField is simple non-root property with multiple types returns correct params 1`] = `"Unable to sort multiple types by field title, not a root field"`;

src/server/saved_objects/client/lib/search_dsl/query_params.js

@@ -1,12 +1,31 @@
-import { getRootProperties } from '../../../../mappings';
+import { getRootPropertiesObjects } from '../../../../mappings';
+
+/**
+ * Gets the types based on the type. Uses mappings to support
+ * null type (all types), a single type string or an array
+ * @param {EsMapping} mappings
+ * @param {(string|Array<string>)} type
+ */
+function getTypes(mappings, type) {
+  if (!type) {
+    return Object.keys(getRootPropertiesObjects(mappings));
+  }
+
+  if (Array.isArray(type)) {
+    return type;
+  }
+
+  return [type];
+}
 
 /**
  *  Get the field params based on the types and searchFields
  *  @param  {Array<string>} searchFields
- *  @param  {Array<string>} types
+ *  @param  {string|Array<string>} types
  *  @return {Object}
  */
 function getFieldsForTypes(searchFields, types) {
+
   if (!searchFields || !searchFields.length) {
     return {
       all_fields: true
@@ -24,27 +43,13 @@ function getFieldsForTypes(searchFields, types) {
 /**
  *  Get the "query" related keys for the search body
  *  @param  {EsMapping} mapping mappings from Ui
- *  @param  {Object} type
+ *  @param  {(string|Array<string>)} type
  *  @param  {String} search
  *  @param  {Array<string>} searchFields
  *  @return {Object}
  */
-export function getQueryParams(mappings, type, search, searchFields, includeTypes) {
+export function getQueryParams(mappings, type, search, searchFields) {
   if (!type && !search) {
-    if (includeTypes) {
-      return {
-        query: {
-          bool: {
-            should: includeTypes.map(includeType => ({
-              term: {
-                type: includeType,
-              }
-            }))
-          }
-        }
-      };
-    }
-
     return {};
   }
 
@@ -52,21 +57,7 @@ export function getQueryParams(mappings, type, search, searchFields, includeType
 
   if (type) {
     bool.filter = [
-      { term: { type } }
-    ];
-  }
-
-  if (includeTypes) {
-    bool.must = [
-      {
-        bool: {
-          should: includeTypes.map(includeType => ({
-            term: {
-              type: includeType,
-            }
-          })),
-        }
-      }
+      { [Array.isArray(type) ? 'terms' : 'term']: { type } }
     ];
   }
 
@@ -78,7 +69,7 @@ export function getQueryParams(mappings, type, search, searchFields, includeType
           query: search,
           ...getFieldsForTypes(
             searchFields,
-            type ? [type] : Object.keys(getRootProperties(mappings)),
+            getTypes(mappings, type)
           )
         }
       }

src/server/saved_objects/client/lib/search_dsl/query_params.test.js

@@ -45,7 +45,7 @@ describe('searchDsl/queryParams', () => {
   });
 
   describe('{type}', () => {
-    it('includes just a terms filter', () => {
+    it('adds a term filter when a string', () => {
       expect(getQueryParams(MAPPINGS, 'saved'))
         .toEqual({
           query: {
@@ -59,6 +59,21 @@ describe('searchDsl/queryParams', () => {
           }
         });
     });
+
+    it('adds a terms filter when an array', () => {
+      expect(getQueryParams(MAPPINGS, ['saved', 'vis']))
+        .toEqual({
+          query: {
+            bool: {
+              filter: [
+                {
+                  terms: { type: ['saved', 'vis'] }
+                }
+              ]
+            }
+          }
+        });
+    });
   });
 
   describe('{search}', () => {
@@ -102,6 +117,26 @@ describe('searchDsl/queryParams', () => {
           }
         });
     });
+    it('includes bool with sqs query and terms filter for type', () => {
+      expect(getQueryParams(MAPPINGS, ['saved', 'vis'], 'y*'))
+        .toEqual({
+          query: {
+            bool: {
+              filter: [
+                { terms: { type: ['saved', 'vis'] } }
+              ],
+              must: [
+                {
+                  simple_query_string: {
+                    query: 'y*',
+                    all_fields: true
+                  }
+                }
+              ]
+            }
+          }
+        });
+    });
   });
 
   describe('{search,searchFields}', () => {
@@ -115,7 +150,6 @@ describe('searchDsl/queryParams', () => {
                   simple_query_string: {
                     query: 'y*',
                     fields: [
-                      'type.title',
                       'pending.title',
                       'saved.title'
                     ]
@@ -136,7 +170,6 @@ describe('searchDsl/queryParams', () => {
                   simple_query_string: {
                     query: 'y*',
                     fields: [
-                      'type.title^3',
                       'pending.title^3',
                       'saved.title^3'
                     ]
@@ -157,10 +190,8 @@ describe('searchDsl/queryParams', () => {
                   simple_query_string: {
                     query: 'y*',
                     fields: [
-                      'type.title',
                       'pending.title',
                       'saved.title',
-                      'type.title.raw',
                       'pending.title.raw',
                       'saved.title.raw',
                     ]
@@ -174,7 +205,7 @@ describe('searchDsl/queryParams', () => {
   });
 
   describe('{type,search,searchFields}', () => {
-    it('includes bool, and sqs with field list', () => {
+    it('includes bool, with term filter and sqs with field list', () => {
       expect(getQueryParams(MAPPINGS, 'saved', 'y*', ['title']))
         .toEqual({
           query: {
@@ -196,6 +227,29 @@ describe('searchDsl/queryParams', () => {
           }
         });
     });
+    it('includes bool, with terms filter and sqs with field list', () => {
+      expect(getQueryParams(MAPPINGS, ['saved', 'vis'], 'y*', ['title']))
+        .toEqual({
+          query: {
+            bool: {
+              filter: [
+                { terms: { type: ['saved', 'vis'] } }
+              ],
+              must: [
+                {
+                  simple_query_string: {
+                    query: 'y*',
+                    fields: [
+                      'saved.title',
+                      'vis.title'
+                    ]
+                  }
+                }
+              ]
+            }
+          }
+        });
+    });
     it('supports fields pointing to multi-fields', () => {
       expect(getQueryParams(MAPPINGS, 'saved', 'y*', ['title.raw']))
         .toEqual({

src/server/saved_objects/client/lib/search_dsl/search_dsl.js

@@ -6,14 +6,13 @@ import { getSortingParams } from './sorting_params';
 export function getSearchDsl(mappings, options = {}) {
   const {
     type,
-    includeTypes,
     search,
     searchFields,
     sortField,
     sortOrder
   } = options;
 
-  if (!type && !includeTypes && sortField) {
+  if (!type && sortField) {
     throw Boom.notAcceptable('Cannot sort without filtering by type');
   }
 
@@ -22,7 +21,7 @@ export function getSearchDsl(mappings, options = {}) {
   }
 
   return {
-    ...getQueryParams(mappings, type, search, searchFields, includeTypes),
+    ...getQueryParams(mappings, type, search, searchFields),
     ...getSortingParams(mappings, type, sortField, sortOrder),
   };
 }

src/server/saved_objects/client/lib/search_dsl/search_dsl.test.js

@@ -34,7 +34,6 @@ describe('getSearchDsl', () => {
         type: 'foo',
         search: 'bar',
         searchFields: ['baz'],
-        includeTypes: ['index-pattern', 'dashboard']
       };
 
       getSearchDsl(mappings, opts);
@@ -45,7 +44,6 @@ describe('getSearchDsl', () => {
         opts.type,
         opts.search,
         opts.searchFields,
-        opts.includeTypes,
       );
     });
 

src/server/saved_objects/client/lib/search_dsl/sorting_params.js

@@ -7,21 +7,35 @@ export function getSortingParams(mappings, type, sortField, sortOrder) {
     return {};
   }
 
-  const key = type ? `${type}.${sortField}` : sortField;
+  if (Array.isArray(type)) {
+    const rootField = getProperty(mappings, sortField);
+    if (!rootField) {
+      throw Boom.badRequest(`Unable to sort multiple types by field ${sortField}, not a root property`);
+    }
 
+    return {
+      sort: [{
+        [sortField]: {
+          order: sortOrder,
+          unmapped_type: rootField.type
+        }
+      }]
+    };
+  }
+
+
+  const key = `${type}.${sortField}`;
   const field = getProperty(mappings, key);
   if (!field) {
     throw Boom.badRequest(`Unknown sort field ${sortField}`);
   }
 
   return {
-    sort: [
-      {
-        [key]: {
-          order: sortOrder,
-          unmapped_type: field.type
-        }
+    sort: [{
+      [key]: {
+        order: sortOrder,
+        unmapped_type: field.type
       }
-    ]
+    }]
   };
 }

src/server/saved_objects/client/lib/search_dsl/sorting_params.test.js

@@ -3,10 +3,23 @@ import { getSortingParams } from './sorting_params';
 const MAPPINGS = {
   rootType: {
     properties: {
+      type: {
+        type: 'text',
+        fields: {
+          raw: {
+            type: 'keyword'
+          }
+        }
+      },
       pending: {
         properties: {
           title: {
             type: 'text',
+            fields: {
+              raw: {
+                type: 'keyword'
+              }
+            }
           }
         }
       },
@@ -55,8 +68,8 @@ describe('searchDsl/getSortParams', () => {
     });
   });
 
-  describe('search field no direction', () => {
-    describe('search field is simple property', () => {
+  describe('sortField no direction', () => {
+    describe('sortField is simple property with single type', () => {
       it('returns correct params', () => {
         expect(getSortingParams(MAPPINGS, 'saved', 'title'))
           .toEqual({
@@ -71,7 +84,27 @@ describe('searchDsl/getSortParams', () => {
           });
       });
     });
-    describe('search field is multi-field', () => {
+    describe('sortField is simple root property with multiple types', () => {
+      it('returns correct params', () => {
+        expect(getSortingParams(MAPPINGS, ['saved', 'pending'], 'type'))
+          .toEqual({
+            sort: [
+              {
+                'type': {
+                  order: undefined,
+                  unmapped_type: 'text'
+                }
+              }
+            ]
+          });
+      });
+    });
+    describe('sortField is simple non-root property with multiple types', () => {
+      it('returns correct params', () => {
+        expect(() => getSortingParams(MAPPINGS, ['saved', 'pending'], 'title')).toThrowErrorMatchingSnapshot();
+      });
+    });
+    describe('sortField is multi-field with single type', () => {
       it('returns correct params', () => {
         expect(getSortingParams(MAPPINGS, 'saved', 'title.raw'))
           .toEqual({
@@ -86,10 +119,30 @@ describe('searchDsl/getSortParams', () => {
           });
       });
     });
+    describe('sortField is root multi-field with multiple types', () => {
+      it('returns correct params', () => {
+        expect(getSortingParams(MAPPINGS, ['saved', 'pending'], 'type.raw'))
+          .toEqual({
+            sort: [
+              {
+                'type.raw': {
+                  order: undefined,
+                  unmapped_type: 'keyword'
+                }
+              }
+            ]
+          });
+      });
+    });
+    describe('sortField is not-root multi-field with multiple types', () => {
+      it('returns correct params', () => {
+        expect(() => getSortingParams(MAPPINGS, ['saved', 'pending'], 'title.raw')).toThrowErrorMatchingSnapshot();
+      });
+    });
   });
 
-  describe('search with direction', () => {
-    describe('search field is simple property', () => {
+  describe('sort with direction', () => {
+    describe('sortField is simple property with single type', () => {
       it('returns correct params', () => {
         expect(getSortingParams(MAPPINGS, 'saved', 'title', 'desc'))
           .toEqual({
@@ -104,7 +157,27 @@ describe('searchDsl/getSortParams', () => {
           });
       });
     });
-    describe('search field is multi-field', () => {
+    describe('sortField is root simple property with multiple type', () => {
+      it('returns correct params', () => {
+        expect(getSortingParams(MAPPINGS, ['saved', 'pending'], 'type', 'desc'))
+          .toEqual({
+            sort: [
+              {
+                'type': {
+                  order: 'desc',
+                  unmapped_type: 'text'
+                }
+              }
+            ]
+          });
+      });
+    });
+    describe('sortFields is non-root simple property with multiple types', () => {
+      it('returns correct params', () => {
+        expect(() => getSortingParams(MAPPINGS, ['saved', 'pending'], 'title', 'desc')).toThrowErrorMatchingSnapshot();
+      });
+    });
+    describe('sortField is multi-field with single type', () => {
       it('returns correct params', () => {
         expect(getSortingParams(MAPPINGS, 'saved', 'title.raw', 'asc'))
           .toEqual({
@@ -119,5 +192,25 @@ describe('searchDsl/getSortParams', () => {
           });
       });
     });
+    describe('sortField is root multi-field with multiple types', () => {
+      it('returns correct params', () => {
+        expect(getSortingParams(MAPPINGS, ['saved', 'pending'], 'type.raw', 'asc'))
+          .toEqual({
+            sort: [
+              {
+                'type.raw': {
+                  order: 'asc',
+                  unmapped_type: 'keyword'
+                }
+              }
+            ]
+          });
+      });
+    });
+    describe('sortField is non-root multi-field with multiple types', () => {
+      it('returns correct params', () => {
+        expect(() => getSortingParams(MAPPINGS, ['saved', 'pending'], 'title.raw', 'asc')).toThrowErrorMatchingSnapshot();
+      });
+    });
   });
 });

src/server/saved_objects/client/saved_objects_client.js

@@ -115,7 +115,7 @@ export class SavedObjectsClient {
 
   /**
    * @param {object} [options={}]
-   * @property {string} [options.type]
+   * @property {(string|Array<string>)} [options.type]
    * @property {string} [options.search]
    * @property {Array<string>} [options.searchFields] - see Elasticsearch Simple Query String
    *                                        Query field argument for more information

src/server/saved_objects/routes/find.js

@@ -10,8 +10,7 @@ export const createFindRoute = (prereqs) => ({
       query: Joi.object().keys({
         per_page: Joi.number().min(0).default(20),
         page: Joi.number().min(0).default(1),
-        type: Joi.string(),
-        include_types: Joi.array().items(Joi.string()).single(),
+        type: Joi.array().items(Joi.string()).single(),
         search: Joi.string().allow('').optional(),
         search_fields: Joi.array().items(Joi.string()).single(),
         sort_field: Joi.array().items(Joi.string()).single(),

src/server/saved_objects/routes/find.test.js

@@ -131,7 +131,7 @@ describe('GET /api/saved_objects/_find', () => {
     });
   });
 
-  it('accepts the type as a query parameter', async () => {
+  it('accepts the query parameter type as a string', async () => {
     const request = {
       method: 'GET',
       url: '/api/saved_objects/_find?type=index-pattern'
@@ -142,6 +142,20 @@ describe('GET /api/saved_objects/_find', () => {
     expect(savedObjectsClient.find.calledOnce).toBe(true);
 
     const options = savedObjectsClient.find.getCall(0).args[0];
-    expect(options).toEqual({ perPage: 20, page: 1, type: 'index-pattern' });
+    expect(options).toEqual({ perPage: 20, page: 1, type: [ 'index-pattern' ] });
+  });
+
+  it('accepts the query parameter type as an array', async () => {
+    const request = {
+      method: 'GET',
+      url: '/api/saved_objects/_find?type=index-pattern&type=visualization'
+    };
+
+    await server.inject(request);
+
+    expect(savedObjectsClient.find.calledOnce).toBe(true);
+
+    const options = savedObjectsClient.find.getCall(0).args[0];
+    expect(options).toEqual({ perPage: 20, page: 1, type: ['index-pattern', 'visualization'] });
   });
 });