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.

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

SavedObjectsRepository code cleanup (bis) #157363

Merged
merged 7 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.
Comment on lines -50 to -51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NIT: removed those comments because they didn't really bring any value (and the original comment from the repository file was removed in the previous PR)


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',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See previous comment (on the v5 field compat) for the reason of this test change

],
}),
}),
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