Skip to content

Commit

Permalink
SavedObjectClient.find multiple types (elastic#19231)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kobelb committed May 24, 2018
1 parent e75b883 commit a3ea2f0
Show file tree
Hide file tree
Showing 19 changed files with 439 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);

Expand Down Expand Up @@ -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)
);

Expand All @@ -144,7 +144,7 @@ export class ObjectsTable extends Component {
sortField: 'type',
fields: ['title', 'id'],
searchFields: ['title'],
includeTypes,
type,
});

savedObjects = data.savedObjects.map(savedObject => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
1 change: 1 addition & 0 deletions src/server/mappings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ export {
getRootType,
getProperty,
getRootProperties,
getRootPropertiesObjects,
} from './lib';
28 changes: 28 additions & 0 deletions src/server/mappings/lib/get_root_properties_objects.js
Original file line number Diff line number Diff line change
@@ -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;
}, {});
}
166 changes: 166 additions & 0 deletions src/server/mappings/lib/get_root_properties_objects.test.js
Original file line number Diff line number Diff line change
@@ -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'
}
});
});
1 change: 1 addition & 0 deletions src/server/mappings/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
4 changes: 1 addition & 3 deletions src/server/saved_objects/client/lib/repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -205,7 +205,6 @@ export class SavedObjectsRepository {
sortField,
sortOrder,
fields,
includeTypes,
} = options;

if (searchFields && !Array.isArray(searchFields)) {
Expand All @@ -228,7 +227,6 @@ export class SavedObjectsRepository {
search,
searchFields,
type,
includeTypes,
sortField,
sortOrder
})
Expand Down
1 change: 0 additions & 1 deletion src/server/saved_objects/client/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ describe('SavedObjectsRepository', () => {
type: 'bar',
sortField: 'name',
sortOrder: 'desc',
includeTypes: ['index-pattern', 'dashboard'],
};

await savedObjectsRepository.find(relevantOpts);
Expand Down
Original file line number Diff line number Diff line change
@@ -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"`;
59 changes: 25 additions & 34 deletions src/server/saved_objects/client/lib/search_dsl/query_params.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -24,49 +43,21 @@ 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 {};
}

const bool = {};

if (type) {
bool.filter = [
{ term: { type } }
];
}

if (includeTypes) {
bool.must = [
{
bool: {
should: includeTypes.map(includeType => ({
term: {
type: includeType,
}
})),
}
}
{ [Array.isArray(type) ? 'terms' : 'term']: { type } }
];
}

Expand All @@ -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)
)
}
}
Expand Down
Loading

0 comments on commit a3ea2f0

Please sign in to comment.