Skip to content

Commit

Permalink
Merge branch 'master' into log-config-keys
Browse files Browse the repository at this point in the history
  • Loading branch information
@mshustov
mshustov committed Feb 11, 2021
2 parents a1f2820 + 619db36 commit 7838e4f
Show file tree
Hide file tree
Showing 112 changed files with 6,614 additions and 2,762 deletions.
77 changes: 31 additions & 46 deletions src/core/server/plugins/discovery/plugin_manifest_parser.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@
import { mockReadFile } from './plugin_manifest_parser.test.mocks';

import { PluginDiscoveryErrorType } from './plugin_discovery_error';
import { loggingSystemMock } from '../../logging/logging_system.mock';

import { resolve } from 'path';
import { parseManifest } from './plugin_manifest_parser';

const logger = loggingSystemMock.createLogger();
const pluginPath = resolve('path', 'existent-dir');
const pluginManifestPath = resolve(pluginPath, 'kibana.json');
const packageInfo = {
Expand All @@ -34,7 +32,7 @@ test('return error when manifest is empty', async () => {
cb(null, Buffer.from(''));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Unexpected end of JSON input (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -46,7 +44,7 @@ test('return error when manifest content is null', async () => {
cb(null, Buffer.from('null'));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin manifest must contain a JSON encoded object. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -58,7 +56,7 @@ test('return error when manifest content is not a valid JSON', async () => {
cb(null, Buffer.from('not-json'));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Unexpected token o in JSON at position 1 (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -70,7 +68,7 @@ test('return error when plugin id is missing', async () => {
cb(null, Buffer.from(JSON.stringify({ version: 'some-version' })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin manifest must contain an "id" property. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -82,45 +80,32 @@ test('return error when plugin id includes `.` characters', async () => {
cb(null, Buffer.from(JSON.stringify({ id: 'some.name', version: 'some-version' })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "id" must not include \`.\` characters. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});
});

test('logs warning if pluginId is not in camelCase format', async () => {
test('return error when pluginId is not in camelCase format', async () => {
expect.assertions(1);
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some_name', version: 'kibana', server: true })));
});

expect(loggingSystemMock.collect(logger).warn).toHaveLength(0);
await parseManifest(pluginPath, packageInfo, logger);
expect(loggingSystemMock.collect(logger).warn).toMatchInlineSnapshot(`
Array [
Array [
"Expect plugin \\"id\\" in camelCase, but found: some_name",
],
]
`);
});

test('does not log pluginId format warning in dist mode', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'some_name', version: 'kibana', server: true })));
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "id" must be camelCase, but found: some_name. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
});

expect(loggingSystemMock.collect(logger).warn).toHaveLength(0);
await parseManifest(pluginPath, { ...packageInfo, dist: true }, logger);
expect(loggingSystemMock.collect(logger).warn.length).toBe(0);
});

test('return error when plugin version is missing', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'someId' })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin manifest for "someId" must contain a "version" property. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -132,7 +117,7 @@ test('return error when plugin expected Kibana version is lower than actual vers
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '6.4.2' })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "someId" is only compatible with Kibana version "6.4.2", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.IncompatibleVersion,
path: pluginManifestPath,
Expand All @@ -147,7 +132,7 @@ test('return error when plugin expected Kibana version cannot be interpreted as
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "someId" is only compatible with Kibana version "non-sem-ver", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.IncompatibleVersion,
path: pluginManifestPath,
Expand All @@ -159,7 +144,7 @@ test('return error when plugin config path is not a string', async () => {
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', configPath: 2 })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `The "configPath" in plugin manifest for "someId" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -174,7 +159,7 @@ test('return error when plugin config path is an array that contains non-string
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `The "configPath" in plugin manifest for "someId" should either be a string or an array of strings. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -186,7 +171,7 @@ test('return error when plugin expected Kibana version is higher than actual ver
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.1' })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Plugin "someId" is only compatible with Kibana version "7.0.1", but used Kibana version is "7.0.0-alpha1". (incompatible-version, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.IncompatibleVersion,
path: pluginManifestPath,
Expand All @@ -198,7 +183,7 @@ test('return error when both `server` and `ui` are set to `false` or missing', a
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0' })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "someId", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -211,7 +196,7 @@ test('return error when both `server` and `ui` are set to `false` or missing', a
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Both "server" and "ui" are missing or set to "false" in plugin manifest for "someId", but at least one of these must be set to "true". (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -234,7 +219,7 @@ test('return error when manifest contains unrecognized properties', async () =>
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
await expect(parseManifest(pluginPath, packageInfo)).rejects.toMatchObject({
message: `Manifest for plugin "someId" contains the following unrecognized properties: unknownOne,unknownTwo. (invalid-manifest, ${pluginManifestPath})`,
type: PluginDiscoveryErrorType.InvalidManifest,
path: pluginManifestPath,
Expand All @@ -247,20 +232,20 @@ describe('configPath', () => {
cb(null, Buffer.from(JSON.stringify({ id: 'plugin', version: '7.0.0', server: true })));
});

const manifest = await parseManifest(pluginPath, packageInfo, logger);
const manifest = await parseManifest(pluginPath, packageInfo);
expect(manifest.configPath).toBe(manifest.id);
});

test('falls back to plugin id in snakeCase format', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(null, Buffer.from(JSON.stringify({ id: 'SomeId', version: '7.0.0', server: true })));
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', server: true })));
});

const manifest = await parseManifest(pluginPath, packageInfo, logger);
const manifest = await parseManifest(pluginPath, packageInfo);
expect(manifest.configPath).toBe('some_id');
});

test('not formated to snakeCase if defined explicitly as string', async () => {
test('not formatted to snakeCase if defined explicitly as string', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Expand All @@ -270,11 +255,11 @@ describe('configPath', () => {
);
});

const manifest = await parseManifest(pluginPath, packageInfo, logger);
const manifest = await parseManifest(pluginPath, packageInfo);
expect(manifest.configPath).toBe('somePath');
});

test('not formated to snakeCase if defined explicitly as an array of strings', async () => {
test('not formatted to snakeCase if defined explicitly as an array of strings', async () => {
mockReadFile.mockImplementation((path, cb) => {
cb(
null,
Expand All @@ -284,7 +269,7 @@ describe('configPath', () => {
);
});

const manifest = await parseManifest(pluginPath, packageInfo, logger);
const manifest = await parseManifest(pluginPath, packageInfo);
expect(manifest.configPath).toEqual(['somePath']);
});
});
Expand All @@ -294,7 +279,7 @@ test('set defaults for all missing optional fields', async () => {
cb(null, Buffer.from(JSON.stringify({ id: 'someId', version: '7.0.0', server: true })));
});

await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'someId',
configPath: 'some_id',
version: '7.0.0',
Expand Down Expand Up @@ -325,7 +310,7 @@ test('return all set optional fields as they are in manifest', async () => {
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'someId',
configPath: ['some', 'path'],
version: 'some-version',
Expand Down Expand Up @@ -355,7 +340,7 @@ test('return manifest when plugin expected Kibana version matches actual version
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'someId',
configPath: 'some-path',
version: 'some-version',
Expand Down Expand Up @@ -385,7 +370,7 @@ test('return manifest when plugin expected Kibana version is `kibana`', async ()
);
});

await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
await expect(parseManifest(pluginPath, packageInfo)).resolves.toEqual({
id: 'someId',
configPath: 'some_id',
version: 'some-version',
Expand Down
11 changes: 6 additions & 5 deletions src/core/server/plugins/discovery/plugin_manifest_parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { coerce } from 'semver';
import { promisify } from 'util';
import { snakeCase } from 'lodash';
import { isConfigPath, PackageInfo } from '../../config';
import { Logger } from '../../logging';
import { PluginManifest } from '../types';
import { PluginDiscoveryError } from './plugin_discovery_error';
import { isCamelCase } from './is_camel_case';
Expand Down Expand Up @@ -63,8 +62,7 @@ const KNOWN_MANIFEST_FIELDS = (() => {
*/
export async function parseManifest(
pluginPath: string,
packageInfo: PackageInfo,
log: Logger
packageInfo: PackageInfo
): Promise<PluginManifest> {
const manifestPath = resolve(pluginPath, MANIFEST_FILE_NAME);

Expand Down Expand Up @@ -105,8 +103,11 @@ export async function parseManifest(
);
}

if (!packageInfo.dist && !isCamelCase(manifest.id)) {
log.warn(`Expect plugin "id" in camelCase, but found: ${manifest.id}`);
if (!isCamelCase(manifest.id)) {
throw PluginDiscoveryError.invalidManifest(
manifestPath,
new Error(`Plugin "id" must be camelCase, but found: ${manifest.id}.`)
);
}

if (!manifest.version || typeof manifest.version !== 'string') {
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/plugins/discovery/plugins_discovery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ function createPlugin$(
coreContext: CoreContext,
instanceInfo: InstanceInfo
) {
return from(parseManifest(path, coreContext.env.packageInfo, log)).pipe(
return from(parseManifest(path, coreContext.env.packageInfo)).pipe(
map((manifest) => {
log.debug(`Successfully discovered plugin "${manifest.id}" at "${path}"`);
const opaqueId = Symbol(manifest.id);
Expand Down
9 changes: 8 additions & 1 deletion src/plugins/data/common/search/aggs/agg_type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { constant, noop, identity } from 'lodash';
import { i18n } from '@kbn/i18n';

import { ISearchSource } from 'src/plugins/data/public';
import { SerializedFieldFormat } from 'src/plugins/expressions/common';
import { DatatableColumnType, SerializedFieldFormat } from 'src/plugins/expressions/common';
import type { RequestAdapter } from 'src/plugins/inspector/common';

import { initParams } from './agg_params';
Expand All @@ -33,6 +33,7 @@ export interface AggTypeConfig<
ordered?: any;
hasNoDsl?: boolean;
params?: Array<Partial<TParam>>;
valueType?: DatatableColumnType;
getRequestAggs?: ((aggConfig: TAggConfig) => TAggConfig[]) | (() => TAggConfig[] | void);
getResponseAggs?: ((aggConfig: TAggConfig) => TAggConfig[]) | (() => TAggConfig[] | void);
customLabels?: boolean;
Expand Down Expand Up @@ -91,6 +92,11 @@ export class AggType<
* @type {string}
*/
title: string;
/**
* The type the values produced by this agg will have in the final data table.
* If not specified, the type of the field is used.
*/
valueType?: DatatableColumnType;
/**
* a function that will be called when this aggType is assigned to
* an aggConfig, and that aggConfig is being rendered (in a form, chart, etc.).
Expand Down Expand Up @@ -222,6 +228,7 @@ export class AggType<
this.dslName = config.dslName || config.name;
this.expressionName = config.expressionName;
this.title = config.title;
this.valueType = config.valueType;
this.makeLabel = config.makeLabel || constant(this.name);
this.ordered = config.ordered;
this.hasNoDsl = !!config.hasNoDsl;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/common/search/aggs/metrics/cardinality.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface AggParamsCardinality extends BaseAggParams {
export const getCardinalityMetricAgg = () =>
new MetricAggType({
name: METRIC_TYPES.CARDINALITY,
valueType: 'number',
expressionName: aggCardinalityFnName,
title: uniqueCountTitle,
makeLabel(aggConfig: IMetricAggConfig) {
Expand Down
Loading

0 comments on commit 7838e4f

Please sign in to comment.