SavedObjectsRepository code cleanup (bis) (#157363)

## Summary

Follow-up of https://github.com/elastic/kibana/pull/157154

Continue the cleanup started in the previous PR, by moving more things
around.

- Move everything search-related (dsl, aggregations, kql utils) to a
dedicated `search` folder
- Move a few more things to `/apis/internals`
- Remove the 'v5' field compatibility in the field list generation (see
comment)
- Cleanup some files a bit.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Pierre Gayvallet 2023-05-12 06:04:52 -04:00 committed by GitHub
parent 069550d72a
commit 8911bfabb0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
61 changed files with 100 additions and 93 deletions

View file

@ -32,8 +32,8 @@ import {
right,
} from './utils';
import type { ApiExecutionContext } from './types';
import { deleteLegacyUrlAliases } from '../legacy_url_aliases';
import {
import { deleteLegacyUrlAliases } from './internals/delete_legacy_url_aliases';
import type {
BulkDeleteExpectedBulkGetResult,
BulkDeleteItemErrorResult,
BulkDeleteParams,
@ -41,7 +41,7 @@ import {
ExpectedBulkDeleteResult,
NewBulkItemResponse,
ObjectToDeleteAliasesFor,
} from '../repository_bulk_delete_internal_types';
} from './internals/repository_bulk_delete_internal_types';
export interface PerformBulkDeleteParams<T = unknown> {
objects: SavedObjectsBulkDeleteObject[];

View file

@ -21,6 +21,7 @@ import {
SavedObjectsBulkResponse,
SavedObjectsGetOptions,
} from '@kbn/core-saved-objects-api-server';
import { includedFields } from '../utils';
import {
Either,
errorContent,
@ -32,13 +33,17 @@ import {
rawDocExistsInNamespaces,
} from './utils';
import { ApiExecutionContext } from './types';
import { includedFields } from '../included_fields';
export interface PerformBulkGetParams<T = unknown> {
objects: SavedObjectsBulkGetObject[];
options: SavedObjectsGetOptions;
}
type ExpectedBulkGetResult = Either<
{ type: string; id: string; error: Payload },
{ type: string; id: string; fields?: string[]; namespaces?: string[]; esRequestIndex: number }
>;
export const performBulkGet = async <T>(
{ objects, options }: PerformBulkGetParams<T>,
{ helpers, allowedTypes, client, serializer, registry, extensions = {} }: ApiExecutionContext
@ -75,10 +80,6 @@ export const performBulkGet = async <T>(
};
let bulkGetRequestIndexCounter = 0;
type ExpectedBulkGetResult = Either<
{ type: string; id: string; error: Payload },
{ type: string; id: string; fields?: string[]; namespaces?: string[]; esRequestIndex: number }
>;
const expectedBulkGetResults = await Promise.all(
objects.map<Promise<ExpectedBulkGetResult>>(async (object) => {
const { type, id, fields } = object;

View file

@ -11,7 +11,7 @@ import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server';
import { ALL_NAMESPACES_STRING } from '@kbn/core-saved-objects-utils-server';
import { SavedObjectsDeleteOptions } from '@kbn/core-saved-objects-api-server';
import { DEFAULT_REFRESH_SETTING } from '../constants';
import { deleteLegacyUrlAliases } from '../legacy_url_aliases';
import { deleteLegacyUrlAliases } from './internals/delete_legacy_url_aliases';
import { getExpectedVersionProperties } from './utils';
import { PreflightCheckNamespacesResult } from './helpers';
import type { ApiExecutionContext } from './types';

View file

@ -14,8 +14,8 @@ import {
getRootPropertiesObjects,
LEGACY_URL_ALIAS_TYPE,
} from '@kbn/core-saved-objects-base-server-internal';
import { getSearchDsl } from '../search';
import type { ApiExecutionContext } from './types';
import { getSearchDsl } from '../search_dsl';
export interface PerformDeleteByNamespaceParams<T = unknown> {
namespace: string;

View file

@ -28,10 +28,12 @@ import {
SavedObjectsFindResponse,
} from '@kbn/core-saved-objects-api-server';
import { ApiExecutionContext } from './types';
import { validateConvertFilterToKueryNode } from '../filter_utils';
import { validateAndConvertAggregations } from '../aggregations';
import { includedFields } from '../included_fields';
import { getSearchDsl } from '../search_dsl';
import {
validateConvertFilterToKueryNode,
getSearchDsl,
validateAndConvertAggregations,
} from '../search';
import { includedFields } from '../utils';
export interface PerformFindParams {
options: SavedObjectsFindOptions;

View file

@ -15,7 +15,7 @@ import type {
import { SavedObjectsUtils } from '@kbn/core-saved-objects-utils-server';
import { SavedObjectsErrorHelpers, SavedObjectsRawDocSource } from '@kbn/core-saved-objects-server';
import type { RepositoryEsClient } from '../../repository_es_client';
import type { PreflightCheckForBulkDeleteParams } from '../../repository_bulk_delete_internal_types';
import type { PreflightCheckForBulkDeleteParams } from '../internals/repository_bulk_delete_internal_types';
import type { CreatePointInTimeFinderFn } from '../../point_in_time_finder';
import {
getSavedObjectNamespaces,

View file

@ -6,7 +6,7 @@
* Side Public License, v 1.
*/
import type { findLegacyUrlAliases } from '../../legacy_url_aliases';
import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
import type { findSharedOriginObjects } from '../utils/find_shared_origin_objects';
import type * as InternalUtils from '../utils/internal_utils';
@ -14,7 +14,7 @@ export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof findLegacyUrlAliases
>;
jest.mock('../../legacy_url_aliases', () => {
jest.mock('./find_legacy_url_aliases', () => {
return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
});

View file

@ -22,8 +22,8 @@ import {
} from '@kbn/core-saved-objects-server';
import { SavedObjectsUtils } from '@kbn/core-saved-objects-utils-server';
import { getObjectKey, parseObjectKey } from '@kbn/core-saved-objects-base-server-internal';
import { findLegacyUrlAliases } from '../../legacy_url_aliases';
import { getRootFields } from '../../included_fields';
import { findLegacyUrlAliases } from './find_legacy_url_aliases';
import { getRootFields } from '../../utils';
import type { CreatePointInTimeFinderFn } from '../../point_in_time_finder';
import type { RepositoryEsClient } from '../../repository_es_client';
import {
@ -123,7 +123,9 @@ export async function collectMultiNamespaceReferences(
return { ...obj, spacesWithMatchingAliases, spacesWithMatchingOrigins };
});
if (!securityExtension) return { objects: results };
if (!securityExtension) {
return { objects: results };
}
// Now that we have *all* information for the object graph, if the Security extension is enabled, we can: check/enforce authorization,
// write audit events, filter the object graph, and redact spaces from the objects.

View file

@ -15,6 +15,10 @@ jest.mock('@kbn/core-elasticsearch-client-server-internal', () => {
});
// Mock this function to return empty results, as this simplifies test cases and we don't need to exercise alternate code paths for these
jest.mock('../search_dsl', () => {
return { getSearchDsl: jest.fn() };
jest.mock('../../search', () => {
const actual = jest.requireActual('../../search');
return {
...actual,
getSearchDsl: jest.fn(),
};
});

View file

@ -9,7 +9,6 @@
import { mockGetEsErrorMessage } from './delete_legacy_url_aliases.test.mock'; // Note: importing this file applies default mocks for other functions too
import { errors as EsErrors } from '@elastic/elasticsearch';
import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
import { ALL_NAMESPACES_STRING } from '@kbn/core-saved-objects-utils-server';
import { typeRegistryMock } from '@kbn/core-saved-objects-base-server-mocks';

View file

@ -7,7 +7,6 @@
*/
import * as esKuery from '@kbn/es-query';
import { getErrorMessage as getEsErrorMessage } from '@kbn/core-elasticsearch-client-server-internal';
import type { ISavedObjectTypeRegistry } from '@kbn/core-saved-objects-server';
import { ALL_NAMESPACES_STRING } from '@kbn/core-saved-objects-utils-server';
@ -15,8 +14,8 @@ import {
LEGACY_URL_ALIAS_TYPE,
type IndexMapping,
} from '@kbn/core-saved-objects-base-server-internal';
import type { RepositoryEsClient } from '../repository_es_client';
import { getSearchDsl } from '../search_dsl';
import type { RepositoryEsClient } from '../../repository_es_client';
import { getSearchDsl } from '../../search';
/** @internal */
export interface DeleteLegacyUrlAliasesParams {

View file

@ -7,15 +7,14 @@
*/
import { DeeplyMockedKeys } from '@kbn/utility-types-jest';
import {
type LegacyUrlAlias,
LEGACY_URL_ALIAS_TYPE,
} from '@kbn/core-saved-objects-base-server-internal';
import { CreatePointInTimeFinderFn, PointInTimeFinder } from '../point_in_time_finder';
import { findLegacyUrlAliases } from './find_legacy_url_aliases';
import { savedObjectsPointInTimeFinderMock } from '../../mocks';
import { SavedObjectsPointInTimeFinderClient } from '@kbn/core-saved-objects-api-server';
import { savedObjectsPointInTimeFinderMock } from '../../../mocks';
import { CreatePointInTimeFinderFn, PointInTimeFinder } from '../../point_in_time_finder';
import { findLegacyUrlAliases } from './find_legacy_url_aliases';
describe('findLegacyUrlAliases', () => {
let pitFinderClientMock: jest.Mocked<SavedObjectsPointInTimeFinderClient>;

View file

@ -12,7 +12,7 @@ import {
LEGACY_URL_ALIAS_TYPE,
getObjectKey,
} from '@kbn/core-saved-objects-base-server-internal';
import type { CreatePointInTimeFinderFn } from '../point_in_time_finder';
import type { CreatePointInTimeFinderFn } from '../../point_in_time_finder';
interface FindLegacyUrlAliasesObject {
type: string;

View file

@ -6,14 +6,14 @@
* Side Public License, v 1.
*/
import type { findLegacyUrlAliases } from '../../legacy_url_aliases';
import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
import type * as InternalUtils from '../utils/internal_utils';
export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof findLegacyUrlAliases
>;
jest.mock('../../legacy_url_aliases', () => {
jest.mock('./find_legacy_url_aliases', () => {
return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
});

View file

@ -12,7 +12,6 @@ import {
} from './preflight_check_for_create.test.mock';
import type { DeeplyMockedKeys } from '@kbn/utility-types-jest';
import type { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
import {

View file

@ -21,7 +21,7 @@ import {
getObjectKey,
type LegacyUrlAlias,
} from '@kbn/core-saved-objects-base-server-internal';
import { findLegacyUrlAliases } from '../../legacy_url_aliases';
import { findLegacyUrlAliases } from './find_legacy_url_aliases';
import type { CreatePointInTimeFinderFn } from '../../point_in_time_finder';
import type { RepositoryEsClient } from '../../repository_es_client';
import { left, right, isLeft, isRight, rawDocExistsInNamespaces, type Either } from '../utils';

View file

@ -12,8 +12,8 @@ import type {
ErrorCause,
} from '@elastic/elasticsearch/lib/api/types';
import type { estypes, TransportResult } from '@elastic/elasticsearch';
import type { Either } from './apis/utils';
import type { DeleteLegacyUrlAliasesParams } from './legacy_url_aliases';
import type { Either } from '../utils';
import type { DeleteLegacyUrlAliasesParams } from './delete_legacy_url_aliases';
/**
* @internal

View file

@ -7,7 +7,7 @@
*/
import type * as InternalUtils from '../utils/internal_utils';
import type { deleteLegacyUrlAliases } from '../../legacy_url_aliases';
import type { deleteLegacyUrlAliases } from './delete_legacy_url_aliases';
export const mockGetBulkOperationError = jest.fn() as jest.MockedFunction<
typeof InternalUtils['getBulkOperationError']
@ -32,6 +32,6 @@ jest.mock('../utils/internal_utils', () => {
export const mockDeleteLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof deleteLegacyUrlAliases
>;
jest.mock('../../legacy_url_aliases', () => ({
jest.mock('./delete_legacy_url_aliases', () => ({
deleteLegacyUrlAliases: mockDeleteLegacyUrlAliases,
}));

View file

@ -44,7 +44,7 @@ import type { RepositoryEsClient } from '../../repository_es_client';
import {
deleteLegacyUrlAliases,
type DeleteLegacyUrlAliasesParams,
} from '../../legacy_url_aliases';
} from './delete_legacy_url_aliases';
/**
* Parameters for the updateObjectsSpaces function.

View file

@ -12,8 +12,8 @@ import {
SavedObjectsRemoveReferencesToOptions,
SavedObjectsRemoveReferencesToResponse,
} from '@kbn/core-saved-objects-api-server';
import { ApiExecutionContext } from './types';
import { getSearchDsl } from '../search_dsl';
import { getSearchDsl } from '../search';
import type { ApiExecutionContext } from './types';
export interface PerformRemoveReferencesToParams {
type: string;

View file

@ -16,6 +16,11 @@ import type { IKibanaMigrator, IndexMapping } from '@kbn/core-saved-objects-base
import type { RepositoryHelpers } from './helpers';
import type { RepositoryEsClient } from '../repository_es_client';
/**
* Context passed from the SO repository to the API execution handlers.
*
* @internal
*/
export interface ApiExecutionContext {
registry: ISavedObjectTypeRegistry;
helpers: RepositoryHelpers;

View file

@ -9,5 +9,4 @@
export { SavedObjectsRepository } from './repository';
export { SavedObjectsClientProvider } from './scoped_client_provider';
export { PointInTimeFinder } from './point_in_time_finder';
export type { ISavedObjectsClientProvider } from './scoped_client_provider';

View file

@ -47,9 +47,6 @@ import {
} from '../test_helpers/repository.test.common';
import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock';
// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
describe('SavedObjectsRepository Encryption Extension', () => {
let client: ReturnType<typeof elasticsearchClientMock.createElasticsearchClient>;
let repository: SavedObjectsRepository;

View file

@ -65,9 +65,6 @@ import {
import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock';
import { arrayMapsAreEqual } from '@kbn/core-saved-objects-utils-server';
// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
describe('SavedObjectsRepository Security Extension', () => {
let client: ReturnType<typeof elasticsearchClientMock.createElasticsearchClient>;
let repository: SavedObjectsRepository;

View file

@ -62,9 +62,6 @@ import {
} from '../test_helpers/repository.test.common';
import { savedObjectsExtensionsMock } from '../mocks/saved_objects_extensions.mock';
// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
const ERROR_NAMESPACE_SPECIFIED = 'Spaces currently determines the namespaces';
describe('SavedObjectsRepository Spaces Extension', () => {

View file

@ -11,7 +11,7 @@ import type { internalBulkResolve } from './apis/internals/internal_bulk_resolve
import type * as InternalUtils from './apis/utils/internal_utils';
import type { preflightCheckForCreate } from './apis/internals/preflight_check_for_create';
import type { updateObjectsSpaces } from './apis/internals/update_objects_spaces';
import type { deleteLegacyUrlAliases } from './legacy_url_aliases';
import type { deleteLegacyUrlAliases } from './apis/internals/delete_legacy_url_aliases';
export const mockCollectMultiNamespaceReferences = jest.fn() as jest.MockedFunction<
typeof collectMultiNamespaceReferences
@ -66,9 +66,9 @@ jest.doMock('./point_in_time_finder', () => ({
export const mockDeleteLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof deleteLegacyUrlAliases
>;
jest.mock('./legacy_url_aliases', () => ({
jest.mock('./apis/internals/delete_legacy_url_aliases', () => ({
deleteLegacyUrlAliases: mockDeleteLegacyUrlAliases,
}));
export const mockGetSearchDsl = jest.fn();
jest.mock('./search_dsl/search_dsl', () => ({ getSearchDsl: mockGetSearchDsl }));
jest.mock('./search/search_dsl', () => ({ getSearchDsl: mockGetSearchDsl }));

View file

@ -3792,7 +3792,6 @@ describe('SavedObjectsRepository', () => {
'updated_at',
'created_at',
'originId',
'title',
],
}),
}),

View file

@ -10,7 +10,7 @@ import type { TransportRequestOptions } from '@elastic/elasticsearch';
import type { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
import { retryCallCluster } from '@kbn/core-elasticsearch-server-internal';
import { decorateEsError } from './decorate_es_error';
import { decorateEsError } from './utils';
const methods = [
'bulk',

View file

@ -11,6 +11,7 @@ import { ObjectType } from '@kbn/config-schema';
import { isPlainObject, isArray } from 'lodash';
import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal';
import { getRootFields } from '../../utils';
import {
isObjectTypeAttribute,
rewriteObjectTypeAttribute,
@ -18,7 +19,6 @@ import {
rewriteRootLevelAttribute,
} from './validation_utils';
import { aggregationSchemas } from './aggs_types';
import { getRootFields } from '../included_fields';
const aggregationKeys = ['aggs', 'aggregations'];

View file

@ -7,7 +7,7 @@
*/
import type { IndexMapping } from '@kbn/core-saved-objects-base-server-internal';
import { fieldDefined, hasFilterKeyError } from '../filter_utils';
import { fieldDefined, hasFilterKeyError } from '../utils/filter_utils';
/**
* Returns true if the given attribute path is a valid root level SO attribute path

View file

@ -6,7 +6,6 @@
* Side Public License, v 1.
*/
export { findLegacyUrlAliases } from './find_legacy_url_aliases';
export { deleteLegacyUrlAliases } from './delete_legacy_url_aliases';
export type { DeleteLegacyUrlAliasesParams } from './delete_legacy_url_aliases';
export { getSearchDsl } from './search_dsl';
export { validateAndConvertAggregations } from './aggregations';
export { validateConvertFilterToKueryNode } from './utils';

View file

@ -9,9 +9,6 @@
import { getReferencesFilterMock } from './query_params.tests.mocks';
import * as esKuery from '@kbn/es-query';
type KueryNode = any;
import {
ALL_NAMESPACES_STRING,
DEFAULT_NAMESPACE_STRING,
@ -19,6 +16,8 @@ import {
import { SavedObjectTypeRegistry } from '@kbn/core-saved-objects-base-server-internal';
import { getQueryParams } from './query_params';
type KueryNode = any;
const registerTypes = (registry: SavedObjectTypeRegistry) => {
registry.registerType({
name: 'pending',

View file

@ -8,9 +8,6 @@
import * as esKuery from '@kbn/es-query';
import type { SavedObjectTypeIdTuple } from '@kbn/core-saved-objects-common';
type KueryNode = any;
import type { ISavedObjectTypeRegistry } from '@kbn/core-saved-objects-server';
import {
ALL_NAMESPACES_STRING,
@ -18,6 +15,8 @@ import {
} from '@kbn/core-saved-objects-utils-server';
import { getReferencesFilter } from './references_filter';
type KueryNode = any;
/**
* Gets the types based on the type. Uses mappings to support
* null type (all types), a single type string or an array

View file

@ -7,7 +7,6 @@
*/
import type { SavedObjectTypeIdTuple } from '@kbn/core-saved-objects-common';
import type { SearchOperator } from './query_params';
export function getReferencesFilter({

View file

@ -0,0 +1,15 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export {
fieldDefined,
hasFilterKeyError,
isSavedObjectAttr,
validateConvertFilterToKueryNode,
validateFilterKueryNode,
} from './filter_utils';

View file

@ -38,34 +38,26 @@ describe('includedFields', () => {
it('accepts type and field as string', () => {
const fields = includedFields('config', 'foo');
expect(fields).toEqual(['config.foo', ...rootFields, 'foo']);
expect(fields).toEqual(['config.foo', ...rootFields]);
});
it('accepts type as array and field as string', () => {
const fields = includedFields(['config', 'secret'], 'foo');
expect(fields).toEqual(['config.foo', 'secret.foo', ...rootFields, 'foo']);
expect(fields).toEqual(['config.foo', 'secret.foo', ...rootFields]);
});
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']);
expect(fields).toEqual(['config.foo', 'config.bar', ...rootFields]);
});
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',
]);
expect(fields).toEqual(['config.foo', 'config.bar', 'secret.foo', 'secret.bar', ...rootFields]);
});
it('uses wildcard when type is not provided', () => {
const fields = includedFields(undefined, 'foo');
expect(fields).toEqual(['*.foo', ...rootFields, 'foo']);
expect(fields).toEqual(['*.foo', ...rootFields]);
});
});

View file

@ -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',
@ -39,14 +35,13 @@ export function includedFields(
return;
}
// convert to an array
const sourceFields = toArray(fields);
const sourceType = toArray(type);
const sourceFields = typeof fields === 'string' ? [fields] : fields;
const sourceType = typeof type === 'string' ? [type] : type;
return sourceType
.reduce((acc: string[], t) => {
return [...acc, ...sourceFields.map((f) => `${t}.${f}`)];
acc.push(...sourceFields.map((f) => `${t}.${f}`));
return acc;
}, [])
.concat(ROOT_FIELDS)
.concat(fields); // v5 compatibility
.concat(ROOT_FIELDS);
}

View file

@ -0,0 +1,10 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export { decorateEsError } from './decorate_es_error';
export { getRootFields, includedFields } from './included_fields';