Skip to content

Commit

Permalink
SavedObjectsRepository code cleanup (bis) (#157363)
Browse files Browse the repository at this point in the history
## Summary

Follow-up of #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>
  • Loading branch information
pgayvallet and kibanamachine committed May 12, 2023
1 parent 069550d commit 8911bfa
Show file tree
Hide file tree
Showing 61 changed files with 100 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,16 @@ 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,
ExpectedBulkDeleteMultiNamespaceDocsParams,
ExpectedBulkDeleteResult,
NewBulkItemResponse,
ObjectToDeleteAliasesFor,
} from '../repository_bulk_delete_internal_types';
} from './internals/repository_bulk_delete_internal_types';

export interface PerformBulkDeleteParams<T = unknown> {
objects: SavedObjectsBulkDeleteObject[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
SavedObjectsBulkResponse,
SavedObjectsGetOptions,
} from '@kbn/core-saved-objects-api-server';
import { includedFields } from '../utils';
import {
Either,
errorContent,
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
* 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';

export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof findLegacyUrlAliases
>;

jest.mock('../../legacy_url_aliases', () => {
jest.mock('./find_legacy_url_aliases', () => {
return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,15 @@
*/

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';
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand All @@ -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,
}));
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }));
Original file line number Diff line number Diff line change
Expand Up @@ -3792,7 +3792,6 @@ describe('SavedObjectsRepository', () => {
'updated_at',
'created_at',
'originId',
'title',
],
}),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ 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,
isRootLevelAttribute,
rewriteRootLevelAttribute,
} from './validation_utils';
import { aggregationSchemas } from './aggs_types';
import { getRootFields } from '../included_fields';

const aggregationKeys = ['aggs', 'aggregations'];

Expand Down
Loading

0 comments on commit 8911bfa

Please sign in to comment.