Skip to content

Commit

Permalink
[SOR] Remove fields API
Browse files Browse the repository at this point in the history
  • Loading branch information
afharo committed Mar 24, 2023
1 parent 728efb3 commit 72b8f87
Show file tree
Hide file tree
Showing 86 changed files with 39 additions and 530 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ export interface SavedObjectsClientContract {
* Query field argument for more information
* @property {integer} [options.page=1]
* @property {integer} [options.perPage=20]
* @property {array} options.fields
* @property {object} [options.hasReference] - { type, id }
* @returns A find result with objects matching the specified search.
* @deprecated See https://github.com/elastic/kibana/issues/149098
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export async function findSharedOriginObjects(
type: [...uniqueObjectTypes],
perPage,
filter,
fields: ['not-a-field'], // Specify a non-existent field to avoid fetching all type-level fields (we only care about root-level fields)
namespaces: [ALL_NAMESPACES_STRING], // We need to search across all spaces to have accurate results
},
undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { getRootFields, includedFields } from './included_fields';
import { getRootFields } from './included_fields';

describe('getRootFields', () => {
it('returns copy of root fields', () => {
Expand All @@ -27,44 +27,3 @@ describe('getRootFields', () => {
`);
});
});

describe('includedFields', () => {
const rootFields = getRootFields();

it('returns undefined if fields are not provided', () => {
expect(includedFields()).toBe(undefined);
});

it('accepts type and field as string', () => {
const fields = includedFields('config', 'foo');
expect(fields).toEqual(['config.foo', ...rootFields, 'foo']);
});

it('accepts type as array and field as string', () => {
const fields = includedFields(['config', 'secret'], 'foo');
expect(fields).toEqual(['config.foo', 'secret.foo', ...rootFields, 'foo']);
});

it('accepts type as string and field as array', () => {
const fields = includedFields('config', ['foo', 'bar']);
expect(fields).toEqual(['config.foo', 'config.bar', ...rootFields, 'foo', 'bar']);
});

it('accepts type as array and field as array', () => {
const fields = includedFields(['config', 'secret'], ['foo', 'bar']);
expect(fields).toEqual([
'config.foo',
'config.bar',
'secret.foo',
'secret.bar',
...rootFields,
'foo',
'bar',
]);
});

it('uses wildcard when type is not provided', () => {
const fields = includedFields(undefined, 'foo');
expect(fields).toEqual(['*.foo', ...rootFields, 'foo']);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@
* Side Public License, v 1.
*/

function toArray(value: string | string[]): string[] {
return typeof value === 'string' ? [value] : value;
}

const ROOT_FIELDS = [
'namespace',
'namespaces',
Expand All @@ -26,26 +22,3 @@ const ROOT_FIELDS = [
export function getRootFields() {
return [...ROOT_FIELDS];
}

/**
* Provides an array of paths for ES source filtering
*/
export function includedFields(
type: string | string[] = '*',
fields?: string[] | string
): string[] | undefined {
if (!fields || fields.length === 0) {
return;
}

// convert to an array
const sourceFields = toArray(fields);
const sourceType = toArray(type);

return sourceType
.reduce((acc: string[], t) => {
return [...acc, ...sourceFields.map((f) => `${t}.${f}`)];
}, [])
.concat(ROOT_FIELDS)
.concat(fields); // v5 compatibility
}
Original file line number Diff line number Diff line change
Expand Up @@ -3525,31 +3525,6 @@ describe('SavedObjectsRepository', () => {
);
});

it(`can filter by fields`, async () => {
await findSuccess(client, repository, { type, fields: ['title'] });
expect(client.search).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({
_source: [
`${type}.title`,
'namespace',
'namespaces',
'type',
'references',
'migrationVersion',
'coreMigrationVersion',
'typeMigrationVersion',
'updated_at',
'created_at',
'originId',
'title',
],
}),
}),
expect.anything()
);
});

it(`should set rest_total_hits_as_int to true on a request`, async () => {
await findSuccess(client, repository, { type });
expect(client.search).toHaveBeenCalledWith(
Expand Down Expand Up @@ -3596,14 +3571,6 @@ describe('SavedObjectsRepository', () => {
expect(client.search).not.toHaveBeenCalled();
});

it(`throws when fields is defined but not an array`, async () => {
// @ts-expect-error fields is an array
await expect(repository.find({ type, fields: 'string' })).rejects.toThrowError(
'options.fields must be an array'
);
expect(client.search).not.toHaveBeenCalled();
});

it(`throws when a preference is provided with pit`, async () => {
await expect(
repository.find({ type: 'foo', pit: { id: 'abc123' }, preference: 'hi' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ import pMap from 'p-map';
import { PointInTimeFinder } from './point_in_time_finder';
import { createRepositoryEsClient, type RepositoryEsClient } from './repository_es_client';
import { getSearchDsl } from './search_dsl';
import { includedFields } from './included_fields';
import { internalBulkResolve, isBulkResolveError } from './internal_bulk_resolve';
import { validateConvertFilterToKueryNode } from './filter_utils';
import { validateAndConvertAggregations } from './aggregations';
Expand Down Expand Up @@ -1290,7 +1289,6 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
searchAfter,
sortField,
sortOrder,
fields,
type,
filter,
preference,
Expand All @@ -1317,10 +1315,6 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
throw SavedObjectsErrorHelpers.createBadRequestError('options.searchFields must be an array');
}

if (fields && !Array.isArray(fields)) {
throw SavedObjectsErrorHelpers.createBadRequestError('options.fields must be an array');
}

let kueryNode;
if (filter) {
try {
Expand Down Expand Up @@ -1389,15 +1383,13 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
index: pit ? undefined : this.getIndicesForTypes(allowedTypes),
// If `searchAfter` is provided, we drop `from` as it will not be used for pagination.
from: searchAfter ? undefined : perPage * (page - 1),
_source: includedFields(allowedTypes, fields),
preference,
rest_total_hits_as_int: true,
size: perPage,
body: {
size: perPage,
seq_no_primary_term: true,
from: perPage * (page - 1),
_source: includedFields(allowedTypes, fields),
...(aggsObject ? { aggs: aggsObject } : {}),
...getSearchDsl(this._mappings, this._registry, {
search,
Expand Down Expand Up @@ -1508,11 +1500,11 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
let bulkGetRequestIndexCounter = 0;
type ExpectedBulkGetResult = Either<
{ type: string; id: string; error: Payload },
{ type: string; id: string; fields?: string[]; namespaces?: string[]; esRequestIndex: number }
{ type: string; id: string; namespaces?: string[]; esRequestIndex: number }
>;
const expectedBulkGetResults = await Promise.all(
objects.map<Promise<ExpectedBulkGetResult>>(async (object) => {
const { type, id, fields } = object;
const { type, id } = object;

let error: DecoratedError | undefined;
if (!this._allowedTypes.includes(type)) {
Expand Down Expand Up @@ -1541,7 +1533,6 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {
value: {
type,
id,
fields,
namespaces,
esRequestIndex: bulkGetRequestIndexCounter++,
},
Expand All @@ -1562,10 +1553,9 @@ export class SavedObjectsRepository implements ISavedObjectsRepository {

const getNamespaceId = (namespaces?: string[]) =>
namespaces !== undefined ? SavedObjectsUtils.namespaceStringToId(namespaces[0]) : namespace;
const bulkGetDocs = validObjects.map(({ value: { type, id, fields, namespaces } }) => ({
const bulkGetDocs = validObjects.map(({ value: { type, id, namespaces } }) => ({
_id: this._serializer.generateRawId(getNamespaceId(namespaces), type, id), // the namespace prefix is only used for single-namespace object types
_index: this.getIndexForType(type),
_source: { includes: includedFields(type, fields) },
}));
const bulkGetResponse = bulkGetDocs.length
? await this.client.mget<SavedObjectsRawDocSource>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ export interface SavedObjectsBulkGetObject {
id: string;
/** Type of the object to get */
type: string;
/** SavedObject fields to include in the response */
fields?: string[];
/**
* Optional namespace(s) for the object to be retrieved in. If this is defined, it will supersede the namespace ID that is in the
* top-level options.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@ export interface SavedObjectsFindOptions {
sortField?: string;
/** sort order, ascending or descending */
sortOrder?: SortOrder;
/**
* An array of fields to include in the results
* @example
* SavedObjects.find({type: 'dashboard', fields: ['attributes.name', 'attributes.location']})
*/
fields?: string[];
/** Search documents using the Elasticsearch Simple Query String syntax. See Elasticsearch Simple Query String `query` argument for more information */
search?: string;
/** The fields to perform the parsed query against. See Elasticsearch Simple Query String `fields` argument for more information */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,6 @@ describe('SavedObjectsClient', () => {
test('makes HTTP call correctly mapping options into snake case query parameters', () => {
const options = {
defaultSearchOperator: 'OR' as const,
fields: ['title'],
hasReference: { id: '1', type: 'reference' },
hasNoReference: { id: '1', type: 'reference' },
page: 10,
Expand All @@ -631,9 +630,6 @@ describe('SavedObjectsClient', () => {
"method": "GET",
"query": Object {
"default_search_operator": "OR",
"fields": Array [
"title",
],
"has_no_reference": "{\\"id\\":\\"1\\",\\"type\\":\\"reference\\"}",
"has_reference": "{\\"id\\":\\"1\\",\\"type\\":\\"reference\\"}",
"page": 10,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ export class SavedObjectsClient implements SavedObjectsClientContract {
const path = this.getPath(['_find']);
const renameMap = {
defaultSearchOperator: 'default_search_operator',
fields: 'fields',
hasReference: 'has_reference',
hasReferenceOperator: 'has_reference_operator',
hasNoReference: 'has_no_reference',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ const checkOriginConflict = async (
rootSearchFields: ['_id', 'originId'],
page: 1,
perPage: 10,
fields: ['title'],
sortField: 'updated_at',
sortOrder: 'desc' as const,
...(namespace && { namespaces: [namespace] }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ async function checkOrigin(
rootSearchFields: ['_id', 'originId'],
page: 1,
perPage: 1, // we only need one result for now
fields: ['title'], // we don't actually need the object's title, we just specify one field so we don't fetch *all* fields
sortField: 'updated_at',
sortOrder: 'desc' as const,
...(namespace && { namespaces: [namespace] }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ describe('validateReferences()', () => {
expect(params.savedObjectsClient.bulkGet).toHaveBeenCalledTimes(1);
expect(params.savedObjectsClient.bulkGet).toHaveBeenCalledWith(
[
{ type: 'index-pattern', id: '3', fields: ['id'] },
{ type: 'index-pattern', id: '5', fields: ['id'] },
{ type: 'index-pattern', id: '6', fields: ['id'] },
{ type: 'search', id: '7', fields: ['id'] },
{ type: 'search', id: '8', fields: ['id'] },
{ type: 'index-pattern', id: '3' },
{ type: 'index-pattern', id: '5' },
{ type: 'index-pattern', id: '6' },
{ type: 'search', id: '7' },
{ type: 'search', id: '8' },
],
{ namespace: undefined }
);
Expand Down Expand Up @@ -154,7 +154,7 @@ describe('validateReferences()', () => {
expect(result).toEqual([]);
expect(params.savedObjectsClient.bulkGet).toHaveBeenCalledTimes(1);
expect(params.savedObjectsClient.bulkGet).toHaveBeenCalledWith(
[{ type: 'index-pattern', id: '1', fields: ['id'] }],
[{ type: 'index-pattern', id: '1' }],
{ namespace: undefined }
);
});
Expand Down Expand Up @@ -211,9 +211,9 @@ describe('validateReferences()', () => {
expect(params.savedObjectsClient.bulkGet).toHaveBeenCalledTimes(1);
expect(params.savedObjectsClient.bulkGet).toHaveBeenCalledWith(
[
{ type: 'index-pattern', id: '1', fields: ['id'] },
{ type: 'index-pattern', id: '1' },
// foo:2 is not included in the cluster call
{ type: 'search', id: '3', fields: ['id'] },
{ type: 'search', id: '3' },
],
{ namespace: undefined }
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ async function getNonExistingReferenceAsKeys({
}

// Fetch references to see if they exist
const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, fields: ['id'] }));
const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj }));
const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace });

// Error handling
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const registerBulkGetRoute = (
schema.object({
type: schema.string(),
id: schema.string(),
fields: schema.maybe(schema.arrayOf(schema.string())),
namespaces: schema.maybe(schema.arrayOf(schema.string())),
})
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export const registerFindRoute = (
schema.oneOf([referenceSchema, schema.arrayOf(referenceSchema)])
),
has_no_reference_operator: searchOperatorSchema,
fields: schema.maybe(schema.oneOf([schema.string(), schema.arrayOf(schema.string())])),
filter: schema.maybe(schema.string()),
aggs: schema.maybe(schema.string()),
namespaces: schema.maybe(
Expand Down Expand Up @@ -119,7 +118,6 @@ export const registerFindRoute = (
hasReferenceOperator: query.has_reference_operator,
hasNoReference: query.has_no_reference,
hasNoReferenceOperator: query.has_no_reference_operator,
fields: typeof query.fields === 'string' ? [query.fields] : query.fields,
filter: query.filter,
aggs,
namespaces,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ export function throwIfAnyTypeNotVisibleByAPI(
export interface BulkGetItem {
type: string;
id: string;
fields?: string[];
namespaces?: string[];
}

Expand Down
Loading

0 comments on commit 72b8f87

Please sign in to comment.