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

Convert uiSettings service to TypeScript #47018

Merged
merged 18 commits into from
Oct 4, 2019
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
7 changes: 5 additions & 2 deletions src/legacy/server/kbn_server.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import { CallClusterWithRequest, ElasticsearchPlugin } from '../core_plugins/ela
import { CapabilitiesModifier } from './capabilities';
import { IndexPatternsServiceFactory } from './index_patterns';
import { Capabilities } from '../../core/public';
import { IUiSettingsClient } from '../../legacy/ui/ui_settings/ui_settings_service';
import { UiSettingsServiceFactoryOptions } from '../../legacy/ui/ui_settings/ui_settings_service_factory';

export interface KibanaConfig {
get<T>(key: string): T;
Expand Down Expand Up @@ -77,14 +79,15 @@ declare module 'hapi' {
name: string,
factoryFn: (request: Request) => Record<string, any>
) => void;
uiSettingsServiceFactory: (options: any) => any;
uiSettingsServiceFactory: (options?: UiSettingsServiceFactoryOptions) => IUiSettingsClient;
logWithMetadata: (tags: string[], message: string, meta: Record<string, any>) => void;
}

interface Request {
getSavedObjectsClient(options?: SavedObjectsClientProviderOptions): SavedObjectsClientContract;
getBasePath(): string;
getDefaultRoute(): Promise<string>;
getUiSettingsService(): any;
getUiSettingsService(): IUiSettingsClient;
getCapabilities(): Promise<Capabilities>;
}

Expand Down
8 changes: 8 additions & 0 deletions src/legacy/ui/ui_settings/create_objects_client_stub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export interface SavedObjectsClientStub {
update: sinon.SinonStub<any[], any>;
get: sinon.SinonStub<any[], any>;
create: sinon.SinonStub<any[], any>;
bulkCreate: sinon.SinonStub<any[], any>;
bulkGet: sinon.SinonStub<any[], any>;
delete: sinon.SinonStub<any[], any>;
find: sinon.SinonStub<any[], any>;
errors: typeof savedObjectsClientErrors;
}

Expand All @@ -35,6 +39,10 @@ export function createObjectsClientStub(esDocSource = {}): SavedObjectsClientStu
get: sinon.stub().returns({ attributes: esDocSource }),
create: sinon.stub(),
errors: savedObjectsClientErrors,
bulkCreate: sinon.stub(),
bulkGet: sinon.stub(),
delete: sinon.stub(),
find: sinon.stub(),
};
return savedObjectsClient;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import sinon from 'sinon';
import expect from '@kbn/expect';
import Chance from 'chance';

// @ts-ignore
import * as getUpgradeableConfigNS from './get_upgradeable_config';
// @ts-ignore
import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config';

const chance = new Chance();
Expand All @@ -45,7 +43,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() {
id: options.id,
version: 'foo',
})),
};
} as any; // mute until we have savedObjects mocks

async function run(options = {}) {
const resp = await createOrUpgradeSavedConfig({
Expand Down Expand Up @@ -103,7 +101,12 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() {
[chance.word()]: chance.sentence(),
};

getUpgradeableConfig.returns({ id: prevVersion, attributes: savedAttributes });
getUpgradeableConfig.resolves({
id: prevVersion,
attributes: savedAttributes,
type: '',
references: [],
});

await run();

Expand All @@ -125,7 +128,12 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() {
it('should log a message for upgrades', async () => {
const { getUpgradeableConfig, logWithMetadata, run } = setup();

getUpgradeableConfig.returns({ id: prevVersion, attributes: { buildNum: buildNum - 100 } });
getUpgradeableConfig.resolves({
id: prevVersion,
attributes: { buildNum: buildNum - 100 },
type: '',
references: [],
});

await run();
sinon.assert.calledOnce(logWithMetadata);
Expand All @@ -143,7 +151,12 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() {
it('does not log when upgrade fails', async () => {
const { getUpgradeableConfig, logWithMetadata, run, savedObjectsClient } = setup();

getUpgradeableConfig.returns({ id: prevVersion, attributes: { buildNum: buildNum - 100 } });
getUpgradeableConfig.resolves({
id: prevVersion,
attributes: { buildNum: buildNum - 100 },
type: '',
references: [],
});

savedObjectsClient.create.callsFake(async () => {
throw new Error('foo');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,38 @@
*/

import { defaults } from 'lodash';
import { SavedObjectsClientContract, SavedObjectAttribute } from 'src/core/server';
import { Legacy } from 'kibana';

import { getUpgradeableConfig } from './get_upgradeable_config';

export async function createOrUpgradeSavedConfig(options) {
const {
savedObjectsClient,
version,
buildNum,
logWithMetadata,
onWriteError,
} = options;
interface Options {
savedObjectsClient: SavedObjectsClientContract;
version: string;
buildNum: number;
logWithMetadata: Legacy.Server['logWithMetadata'];
onWriteError?: <T extends SavedObjectAttribute = any>(
error: Error,
attributes: Record<string, any>
) => Record<string, T> | undefined;
}
export async function createOrUpgradeSavedConfig<T extends SavedObjectAttribute = any>(
options: Options
): Promise<Record<string, T> | undefined> {
const { savedObjectsClient, version, buildNum, logWithMetadata, onWriteError } = options;
Copy link
Contributor

Choose a reason for hiding this comment

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

Move object destructure to function arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer not to do. there are already too many params to use positional arguments.


// try to find an older config we can upgrade
const upgradeableConfig = await getUpgradeableConfig({
savedObjectsClient,
version
version,
});

// default to the attributes of the upgradeableConfig if available
const attributes = defaults(
{ buildNum },
upgradeableConfig ? upgradeableConfig.attributes : {}
);
const attributes = defaults({ buildNum }, upgradeableConfig ? upgradeableConfig.attributes : {});

try {
// create the new SavedConfig
await savedObjectsClient.create(
'config',
attributes,
{ id: version }
);
await savedObjectsClient.create('config', attributes, { id: version });
} catch (error) {
if (onWriteError) {
return onWriteError(error, attributes);
Expand All @@ -58,9 +59,13 @@ export async function createOrUpgradeSavedConfig(options) {
}

if (upgradeableConfig) {
logWithMetadata(['plugin', 'elasticsearch'], `Upgrade config from ${upgradeableConfig.id} to ${version}`, {
prevVersion: upgradeableConfig.id,
newVersion: version
});
logWithMetadata(
['plugin', 'elasticsearch'],
`Upgrade config from ${upgradeableConfig.id} to ${version}`,
{
prevVersion: upgradeableConfig.id,
newVersion: version,
}
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/

import { SavedObjectsClientContract } from 'src/core/server';
import { isConfigVersionUpgradeable } from './is_config_version_upgradeable';

/**
Expand All @@ -26,18 +26,22 @@ import { isConfigVersionUpgradeable } from './is_config_version_upgradeable';
* @property {string} version
* @return {Promise<SavedConfig|undefined>}
*/
export async function getUpgradeableConfig({ savedObjectsClient, version }) {
export async function getUpgradeableConfig({
savedObjectsClient,
version,
}: {
savedObjectsClient: SavedObjectsClientContract;
version: string;
}) {
// attempt to find a config we can upgrade
const { saved_objects: savedConfigs } = await savedObjectsClient.find({
type: 'config',
page: 1,
perPage: 1000,
sortField: 'buildNum',
sortOrder: 'desc'
sortOrder: 'desc',
});

// try to find a config that we can upgrade
return savedConfigs.find(savedConfig => (
isConfigVersionUpgradeable(savedConfig.id, version)
));
return savedConfigs.find(savedConfig => isConfigVersionUpgradeable(savedConfig.id, version));
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { SavedObjectsClientContract } from 'src/core/server';

import KbnServer from '../../../../server/kbn_server';
import { createTestServers } from '../../../../../test_utils/kbn_server';
// @ts-ignore
import { createOrUpgradeSavedConfig } from '../create_or_upgrade_saved_config';

describe('createOrUpgradeSavedConfig()', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import expect from '@kbn/expect';

// @ts-ignore
import { isConfigVersionUpgradeable } from './is_config_version_upgradeable';
// @ts-ignore
import { pkg } from '../../../utils';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@
import semver from 'semver';
const rcVersionRegex = /^(\d+\.\d+\.\d+)\-rc(\d+)$/i;

function extractRcNumber(version) {
function extractRcNumber(version: string): [string, number] {
const match = version.match(rcVersionRegex);
return match
? [match[1], parseInt(match[2], 10)]
: [version, Infinity];
return match ? [match[1], parseInt(match[2], 10)] : [version, Infinity];
}

export function isConfigVersionUpgradeable(savedVersion, kibanaVersion) {
export function isConfigVersionUpgradeable(savedVersion: string, kibanaVersion: string): boolean {
if (
typeof savedVersion !== 'string' ||
typeof kibanaVersion !== 'string' ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ import expect from '@kbn/expect';
// @ts-ignore
import { Config } from '../../../server/config';

// @ts-ignore
import * as uiSettingsServiceFactoryNS from '../ui_settings_service_factory';
// @ts-ignore
import * as getUiSettingsServiceForRequestNS from '../ui_settings_service_for_request';
// @ts-ignore
import { uiSettingsMixin } from '../ui_settings_mixin';
Expand Down Expand Up @@ -123,7 +121,8 @@ describe('uiSettingsMixin()', () => {
foo: 'bar',
});
sinon.assert.calledOnce(uiSettingsServiceFactoryStub);
sinon.assert.calledWithExactly(uiSettingsServiceFactoryStub, server, {
sinon.assert.calledWithExactly(uiSettingsServiceFactoryStub, server as any, {
// @ts-ignore foo doesn't exist on Hapi.Server
foo: 'bar',
overrides: {
foo: 'bar',
Expand Down Expand Up @@ -162,7 +161,12 @@ describe('uiSettingsMixin()', () => {
sinon.assert.notCalled(getUiSettingsServiceForRequestStub);
const request = {};
decorations.request.getUiSettingsService.call(request);
sinon.assert.calledWith(getUiSettingsServiceForRequestStub, server, request);
sinon.assert.calledWith(getUiSettingsServiceForRequestStub, server as any, request as any, {
overrides: {
foo: 'bar',
},
getDefaults: sinon.match.func,
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,22 @@
* specific language governing permissions and limitations
* under the License.
*/
import { Legacy } from 'kibana';

async function handleRequest(request) {
async function handleRequest(request: Legacy.Request) {
const { key } = request.params;
const uiSettings = request.getUiSettingsService();

await uiSettings.remove(key);
return {
settings: await uiSettings.getUserProvided()
settings: await uiSettings.getUserProvided(),
};
}

export const deleteRoute = {
path: '/api/kibana/settings/{key}',
method: 'DELETE',
handler: async (request, h) => {
return h.response(await handleRequest(request));
}
handler: async (request: Legacy.Request) => {
return await handleRequest(request);
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
import { Legacy } from 'kibana';

async function handleRequest(request) {
async function handleRequest(request: Legacy.Request) {
const uiSettings = request.getUiSettingsService();
return {
settings: await uiSettings.getUserProvided()
settings: await uiSettings.getUserProvided(),
};
}

export const getRoute = {
path: '/api/kibana/settings',
method: 'GET',
handler: function (request) {
handler(request: Legacy.Request) {
return handleRequest(request);
}
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { SavedObjectsClientContract } from 'src/core/server';
import KbnServer from '../../../../../server/kbn_server';
import { createTestServers } from '../../../../../../test_utils/kbn_server';
import { CallCluster } from '../../../../../../legacy/core_plugins/elasticsearch';
import { IUiSettingsClient } from '../../../ui_settings_service';

let kbnServer: KbnServer;
let servers: ReturnType<typeof createTestServers>;
Expand All @@ -33,7 +34,7 @@ interface AllServices {
kbnServer: KbnServer;
savedObjectsClient: SavedObjectsClientContract;
callCluster: CallCluster;
uiSettings: any;
uiSettings: IUiSettingsClient;
deleteKibanaIndex: typeof deleteKibanaIndex;
}

Expand Down
Loading