Skip to content

Commit

Permalink
Refactor new code into separate transform function
Browse files Browse the repository at this point in the history
Also added tests!
  • Loading branch information
Joe Portner committed Jun 2, 2022
1 parent e895f78 commit 110e043
Show file tree
Hide file tree
Showing 7 changed files with 218 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,16 @@
* Side Public License, v 1.
*/

import type { transformDefaultIndex } from '../saved_objects';
import type { getUpgradeableConfig } from './get_upgradeable_config';

export const mockTransformDefaultIndex = jest.fn() as jest.MockedFunction<
typeof transformDefaultIndex
>;
jest.mock('../saved_objects', () => ({
transformDefaultIndex: mockTransformDefaultIndex,
}));

export const mockGetUpgradeableConfig = jest.fn() as jest.MockedFunction<
typeof getUpgradeableConfig
>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
* Side Public License, v 1.
*/

import { mockGetUpgradeableConfig } from './create_or_upgrade_saved_config.test.mock';
import {
mockTransformDefaultIndex,
mockGetUpgradeableConfig,
} from './create_or_upgrade_saved_config.test.mock';
import { SavedObjectsErrorHelpers } from '../../saved_objects';
import { savedObjectsClientMock } from '../../saved_objects/service/saved_objects_client.mock';
import { loggingSystemMock } from '../../logging/logging_system.mock';
Expand Down Expand Up @@ -106,6 +109,36 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () {
);
});

it('should prefer transformed attributes when merging', async () => {
const { run, savedObjectsClient } = setup();
mockGetUpgradeableConfig.mockResolvedValue({
id: prevVersion,
attributes: {
buildNum: buildNum - 100,
defaultIndex: 'some-index',
},
});
mockTransformDefaultIndex.mockResolvedValue({
defaultIndex: 'another-index',
isDefaultIndexMigrated: true,
});

await run();

expect(mockGetUpgradeableConfig).toHaveBeenCalledTimes(1);
expect(mockTransformDefaultIndex).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.create).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.create).toHaveBeenCalledWith(
'config',
{
buildNum,
defaultIndex: 'another-index',
isDefaultIndexMigrated: true,
},
{ id: version }
);
});

it('should log a message for upgrades', async () => {
const { logger, run } = setup();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { SavedObjectsClientContract } from '../../saved_objects/types';
import { SavedObjectsErrorHelpers } from '../../saved_objects';

import { getUpgradeableConfig } from './get_upgradeable_config';
import { transformDefaultIndex } from '../saved_objects';

interface ConfigLogMeta extends LogMeta {
kibana: {
Expand All @@ -39,41 +40,15 @@ export async function createOrUpgradeSavedConfig(
version,
});

let defaultIndex: string | undefined;
let isDefaultIndexMigrated = false;
if (
upgradeableConfig?.attributes.defaultIndex &&
!upgradeableConfig.attributes.isDefaultIndexMigrated
) {
defaultIndex = upgradeableConfig.attributes.defaultIndex; // Keep the existing defaultIndex attribute if the data view is not found
try {
// The defaultIndex for this config object was created prior to 8.3, and it might refer to a data view ID that is no longer valid.
// We should try to resolve the data view and change the defaultIndex to the new ID, if necessary.
const resolvedIndex = await savedObjectsClient.resolve('index-pattern', defaultIndex);
if (resolvedIndex.outcome === 'aliasMatch' || resolvedIndex.outcome === 'conflict') {
// This resolved to an aliasMatch or conflict outcome; that means we should change the defaultIndex to the data view's new ID.
// Note, the alias_target_id field is guaranteed to exist iff the resolve outcome is aliasMatch or conflict.
defaultIndex = resolvedIndex.alias_target_id!;
isDefaultIndexMigrated = true;
}
} catch (err) {
// If the defaultIndex is not found at all, it will throw a Not Found error and we should mark the defaultIndex attribute as upgraded.
if (SavedObjectsErrorHelpers.isNotFoundError(err)) {
isDefaultIndexMigrated = true;
}
// For any other error, leave it unchanged so we can try to upgrade this attribute again in the future.
}
}

// default to the attributes of the upgradeableConfig if available
const attributes = defaults(
{
buildNum,
...(isDefaultIndexMigrated && {
// We migrated the defaultIndex attribute for this config, make sure these two fields take precedence over any in the old config
defaultIndex,
isDefaultIndexMigrated,
}),
// If there is an upgradeableConfig, attempt to transform the defaultIndex attribute to ensure it is set properly in 8.3.1+
...(await transformDefaultIndex({

This comment has been minimized.

Copy link
@TinaHeiligers

TinaHeiligers Jun 2, 2022

Contributor

I love it! The code is so much easier to understand.

savedObjectsClient,
configAttributes: upgradeableConfig?.attributes,
})),
},
upgradeableConfig?.attributes
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
*/

export { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config';
export type { UpgradeableConfigType } from './get_upgradeable_config';
1 change: 1 addition & 0 deletions src/core/server/ui_settings/saved_objects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
*/

export { uiSettingsType } from './ui_settings';
export { transformDefaultIndex } from './transforms';
101 changes: 101 additions & 0 deletions src/core/server/ui_settings/saved_objects/transforms.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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.
*/

import { savedObjectsClientMock } from '../../mocks';
import { SavedObjectsErrorHelpers } from '../../saved_objects';
import { SavedObject } from '../../types';
import { transformDefaultIndex } from './transforms';

describe('#transformDefaultIndex', () => {
const savedObjectsClient = savedObjectsClientMock.create();

beforeEach(() => {
jest.resetAllMocks();
});

it('should return early if configAttributes is undefined', async () => {
const result = await transformDefaultIndex({ savedObjectsClient, configAttributes: undefined });

expect(savedObjectsClient.resolve).not.toHaveBeenCalled();
expect(result).toEqual({ isDefaultIndexMigrated: true });
});

it('should return early if the defaultIndex attribute is undefined', async () => {
const result = await transformDefaultIndex({
savedObjectsClient,
configAttributes: { defaultIndex: undefined },
});

expect(savedObjectsClient.resolve).not.toHaveBeenCalled();
expect(result).toEqual({ isDefaultIndexMigrated: true });
});

describe('should resolve the data view for the defaultIndex and return the result according to the outcome', () => {
it('outcome: exactMatch', async () => {
savedObjectsClient.resolve.mockResolvedValue({
outcome: 'exactMatch',
alias_target_id: 'another-index', // This wouldn't realistically be set if the outcome is exactMatch, but we're including it in this test to assert that the returned defaultIndex will be 'some-index'
saved_object: {} as SavedObject, // Doesn't matter
});
const result = await transformDefaultIndex({
savedObjectsClient,
configAttributes: { defaultIndex: 'some-index' },
});

expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index');
expect(result).toEqual({ isDefaultIndexMigrated: true, defaultIndex: 'some-index' });
});

for (const outcome of ['aliasMatch' as const, 'conflict' as const]) {
it(`outcome: ${outcome}`, async () => {
savedObjectsClient.resolve.mockResolvedValue({
outcome,
alias_target_id: 'another-index',
saved_object: {} as SavedObject, // Doesn't matter
});
const result = await transformDefaultIndex({
savedObjectsClient,
configAttributes: { defaultIndex: 'some-index' },
});

expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index');
expect(result).toEqual({ isDefaultIndexMigrated: true, defaultIndex: 'another-index' });
});
}

it('returns the expected result if resolve fails with a Not Found error', async () => {
savedObjectsClient.resolve.mockRejectedValue(
SavedObjectsErrorHelpers.createGenericNotFoundError('Oh no!')
);
const result = await transformDefaultIndex({
savedObjectsClient,
configAttributes: { defaultIndex: 'some-index' },
});

expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index');
expect(result).toEqual({ isDefaultIndexMigrated: true, defaultIndex: 'some-index' });
});

it('returns the expected result if resolve fails with another error', async () => {
savedObjectsClient.resolve.mockRejectedValue(
SavedObjectsErrorHelpers.createIndexAliasNotFoundError('Oh no!')
);
const result = await transformDefaultIndex({
savedObjectsClient,
configAttributes: { defaultIndex: 'some-index' },
});

expect(savedObjectsClient.resolve).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.resolve).toHaveBeenCalledWith('index-pattern', 'some-index');
expect(result).toEqual({ isDefaultIndexMigrated: false, defaultIndex: 'some-index' });
});
});
});
67 changes: 67 additions & 0 deletions src/core/server/ui_settings/saved_objects/transforms.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* 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.
*/

import { SavedObjectsErrorHelpers } from '../../saved_objects';
import type { SavedObjectsClientContract } from '../../types';
import type { UpgradeableConfigType } from '../create_or_upgrade_saved_config';

interface Params {
savedObjectsClient: SavedObjectsClientContract;
configAttributes:
| Pick<UpgradeableConfigType, 'defaultIndex' | 'isDefaultIndexMigrated'>
| undefined;
}

/** The resulting parameters that should be used when upgrading the config object. */
interface ReturnType {
isDefaultIndexMigrated: boolean;
defaultIndex?: string;
}

/**
* This is applied during `createOrUpgradeSavedConfig` to optionally transform the `defaultIndex` attribute of a config saved object. The
* `defaultIndex` attribute points to a data view ID, but those saved object IDs were regenerated in the 8.0 upgrade. That resulted in a bug
* where the `defaultIndex` would be broken in custom spaces.
*
* We are fixing this bug after the fact, and we can't retroactively change a saved object that's already been migrated, so we use this
* transformation instead to ensure that the `defaultIndex` attributeis not broken.
*
* Note that what used to be called "index patterns" prior to 8.0 have been renamed to "data views", but the object type cannot be changed,
* so that type remains `index-pattern`.
*/
export async function transformDefaultIndex(params: Params): Promise<ReturnType> {
const { savedObjectsClient, configAttributes } = params;
if (!configAttributes || !configAttributes.defaultIndex) {
// If configAttributes is undefined (there's no config object being upgraded), set isDefaultIndexMigrated to true and return.
// If configAttributes is defined but the defaultIndex attribute is not set, set isDefaultIndexMigrated to true and return.
return { isDefaultIndexMigrated: true };
}

let defaultIndex = configAttributes.defaultIndex!; // Retain the existing defaultIndex attribute in case we run into a resolve error
let isDefaultIndexMigrated: boolean;
try {
// The defaultIndex for this config object was created prior to 8.3, and it might refer to a data view ID that is no longer valid.
// We should try to resolve the data view and change the defaultIndex to the new ID, if necessary.
const resolvedDataView = await savedObjectsClient.resolve('index-pattern', defaultIndex);
if (resolvedDataView.outcome === 'aliasMatch' || resolvedDataView.outcome === 'conflict') {
// This resolved to an aliasMatch or conflict outcome; that means we should change the defaultIndex to the data view's new ID.
// Note, the alias_target_id field is guaranteed to exist iff the resolve outcome is aliasMatch or conflict.
defaultIndex = resolvedDataView.alias_target_id!;
}
isDefaultIndexMigrated = true; // Regardless of the resolve outcome, we now consider this defaultIndex attribute to be migrated
} catch (err) {
// If the defaultIndex is not found at all, it will throw a Not Found error and we should mark the defaultIndex attribute as upgraded.
if (SavedObjectsErrorHelpers.isNotFoundError(err)) {
isDefaultIndexMigrated = true;
} else {
// For any other error, explicitly set isDefaultIndexMigrated to false so we can try this upgrade again in the future.
isDefaultIndexMigrated = false;
}
}
return { isDefaultIndexMigrated, defaultIndex };
}

0 comments on commit 110e043

Please sign in to comment.