Skip to content
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.

Sign up for GitHub

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

[SOR] Remove fields API #153447

Closed
wants to merge 2 commits into from
Closed

[SOR] Remove fields API #153447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
796ccf1
[SOR] Remove `fields` API
afharo Mar 22, 2023
529da57
Merge branch 'main' into sor/remove-fields-API
stratoula Mar 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/core/saved-objects/core-saved-objects-api-browser/src/saved_objects_client.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion ...aved-objects/core-saved-objects-api-server-internal/src/lib/find_shared_origin_objects.ts
View file
Open in desktop
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
43 changes: 1 addition & 42 deletions ...core/saved-objects/core-saved-objects-api-server-internal/src/lib/included_fields.test.ts
View file
Open in desktop
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']);
});
});
27 changes: 0 additions & 27 deletions ...ages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/included_fields.ts
View file
Open in desktop
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
}
33 changes: 0 additions & 33 deletions ...ages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.test.ts
View file
Open in desktop
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
16 changes: 3 additions & 13 deletions packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.ts
View file
Open in desktop
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
2 changes: 0 additions & 2 deletions packages/core/saved-objects/core-saved-objects-api-server/src/apis/bulk_get.ts
View file
Open in desktop
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
6 changes: 0 additions & 6 deletions packages/core/saved-objects/core-saved-objects-api-server/src/apis/find.ts
View file
Open in desktop
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
4 changes: 0 additions & 4 deletions ...s/core/saved-objects/core-saved-objects-browser-internal/src/saved_objects_client.test.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion packages/core/saved-objects/core-saved-objects-browser-internal/src/saved_objects_client.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion ...core-saved-objects-import-export-server-internal/src/import/lib/check_origin_conflicts.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion ...ore-saved-objects-import-export-server-internal/src/import/lib/check_reference_origins.ts
View file
Open in desktop
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to add an option for retrieving the root fields only

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps an option to return just "meta" fields like ID, createdAt, updatedAt?

sortField: 'updated_at',
sortOrder: 'desc' as const,
...(namespace && { namespaces: [namespace] }),
Expand Down
16 changes: 8 additions & 8 deletions ...re-saved-objects-import-export-server-internal/src/import/lib/validate_references.test.ts
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...ts/core-saved-objects-import-export-server-internal/src/import/lib/validate_references.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/bulk_get.ts
View file
Open in desktop
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
2 changes: 0 additions & 2 deletions packages/core/saved-objects/core-saved-objects-server-internal/src/routes/find.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion packages/core/saved-objects/core-saved-objects-server-internal/src/routes/utils.ts
View file
Open in desktop
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