From 583500e5261a05baa7c7be6789cf7853a4fd837a Mon Sep 17 00:00:00 2001
From: Mikhail Shustov <restrry@gmail.com>
Date: Mon, 18 Nov 2019 10:25:21 +0100
Subject: [PATCH] allows plugins to define validation schema for "enabled" flag
(#50286)
* validation error message gives a hint about error source
* allows plugins to define validation schema for "enabled" flag
---
src/core/server/config/config_service.test.ts | 85 +++++++++++++++++--
src/core/server/config/config_service.ts | 26 ++++--
2 files changed, 97 insertions(+), 14 deletions(-)
diff --git a/src/core/server/config/config_service.test.ts b/src/core/server/config/config_service.test.ts
index 61da9af7baa7c..131e1dd501792 100644
--- a/src/core/server/config/config_service.test.ts
+++ b/src/core/server/config/config_service.test.ts
@@ -55,7 +55,7 @@ test('throws if config at path does not match schema', async () => {
await expect(
configService.setSchema('key', schema.string())
).rejects.toThrowErrorMatchingInlineSnapshot(
- `"[key]: expected value of type [string] but got [number]"`
+ `"[config validation of [key]]: expected value of type [string] but got [number]"`
);
});
@@ -78,11 +78,11 @@ test('re-validate config when updated', async () => {
config$.next(new ObjectToConfigAdapter({ key: 123 }));
await expect(valuesReceived).toMatchInlineSnapshot(`
-Array [
- "value",
- [Error: [key]: expected value of type [string] but got [number]],
-]
-`);
+ Array [
+ "value",
+ [Error: [config validation of [key]]: expected value of type [string] but got [number]],
+ ]
+ `);
});
test("returns undefined if fetching optional config at a path that doesn't exist", async () => {
@@ -143,7 +143,7 @@ test("throws error if 'schema' is not defined for a key", async () => {
const configs = configService.atPath('key');
await expect(configs.pipe(first()).toPromise()).rejects.toMatchInlineSnapshot(
- `[Error: No validation schema has been defined for key]`
+ `[Error: No validation schema has been defined for [key]]`
);
});
@@ -153,7 +153,7 @@ test("throws error if 'setSchema' called several times for the same key", async
const addSchema = async () => await configService.setSchema('key', schema.string());
await addSchema();
await expect(addSchema()).rejects.toMatchInlineSnapshot(
- `[Error: Validation schema for key was already registered.]`
+ `[Error: Validation schema for [key] was already registered.]`
);
});
@@ -280,6 +280,33 @@ test('handles disabled path and marks config as used', async () => {
expect(unusedPaths).toEqual([]);
});
+test('does not throw if schema does not define "enabled" schema', async () => {
+ const initialConfig = {
+ pid: {
+ file: '/some/file.pid',
+ },
+ };
+
+ const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
+ const configService = new ConfigService(config$, defaultEnv, logger);
+ expect(
+ configService.setSchema(
+ 'pid',
+ schema.object({
+ file: schema.string(),
+ })
+ )
+ ).resolves.toBeUndefined();
+
+ const value$ = configService.atPath('pid');
+ const value: any = await value$.pipe(first()).toPromise();
+ expect(value.enabled).toBe(undefined);
+
+ const valueOptional$ = configService.optionalAtPath('pid');
+ const valueOptional: any = await valueOptional$.pipe(first()).toPromise();
+ expect(valueOptional.enabled).toBe(undefined);
+});
+
test('treats config as enabled if config path is not present in config', async () => {
const initialConfig = {};
@@ -292,3 +319,45 @@ test('treats config as enabled if config path is not present in config', async (
const unusedPaths = await configService.getUnusedPaths();
expect(unusedPaths).toEqual([]);
});
+
+test('read "enabled" even if its schema is not present', async () => {
+ const initialConfig = {
+ foo: {
+ enabled: true,
+ },
+ };
+
+ const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
+ const configService = new ConfigService(config$, defaultEnv, logger);
+
+ const isEnabled = await configService.isEnabledAtPath('foo');
+ expect(isEnabled).toBe(true);
+});
+
+test('allows plugins to specify "enabled" flag via validation schema', async () => {
+ const initialConfig = {};
+
+ const config$ = new BehaviorSubject(new ObjectToConfigAdapter(initialConfig));
+ const configService = new ConfigService(config$, defaultEnv, logger);
+
+ await configService.setSchema(
+ 'foo',
+ schema.object({ enabled: schema.boolean({ defaultValue: false }) })
+ );
+
+ expect(await configService.isEnabledAtPath('foo')).toBe(false);
+
+ await configService.setSchema(
+ 'bar',
+ schema.object({ enabled: schema.boolean({ defaultValue: true }) })
+ );
+
+ expect(await configService.isEnabledAtPath('bar')).toBe(true);
+
+ await configService.setSchema(
+ 'baz',
+ schema.object({ different: schema.boolean({ defaultValue: true }) })
+ );
+
+ expect(await configService.isEnabledAtPath('baz')).toBe(true);
+});
diff --git a/src/core/server/config/config_service.ts b/src/core/server/config/config_service.ts
index 8d3cc733cf250..c18a5b2000e01 100644
--- a/src/core/server/config/config_service.ts
+++ b/src/core/server/config/config_service.ts
@@ -54,7 +54,7 @@ export class ConfigService {
public async setSchema(path: ConfigPath, schema: Type<unknown>) {
const namespace = pathToString(path);
if (this.schemas.has(namespace)) {
- throw new Error(`Validation schema for ${path} was already registered.`);
+ throw new Error(`Validation schema for [${path}] was already registered.`);
}
this.schemas.set(namespace, schema);
@@ -98,14 +98,28 @@ export class ConfigService {
}
public async isEnabledAtPath(path: ConfigPath) {
- const enabledPath = createPluginEnabledPath(path);
+ const namespace = pathToString(path);
+
+ const validatedConfig = this.schemas.has(namespace)
+ ? await this.atPath<{ enabled?: boolean }>(path)
+ .pipe(first())
+ .toPromise()
+ : undefined;
+ const enabledPath = createPluginEnabledPath(path);
const config = await this.config$.pipe(first()).toPromise();
- if (!config.has(enabledPath)) {
+
+ // if plugin hasn't got a config schema, we try to read "enabled" directly
+ const isEnabled =
+ validatedConfig && validatedConfig.enabled !== undefined
+ ? validatedConfig.enabled
+ : config.get(enabledPath);
+
+ // not declared. consider that plugin is enabled by default
+ if (isEnabled === undefined) {
return true;
}
- const isEnabled = config.get(enabledPath);
if (isEnabled === false) {
// If the plugin is _not_ enabled, we mark the entire plugin path as
// handled, as it's expected that it won't be used.
@@ -138,7 +152,7 @@ export class ConfigService {
const namespace = pathToString(path);
const schema = this.schemas.get(namespace);
if (!schema) {
- throw new Error(`No validation schema has been defined for ${namespace}`);
+ throw new Error(`No validation schema has been defined for [${namespace}]`);
}
return schema.validate(
config,
@@ -147,7 +161,7 @@ export class ConfigService {
prod: this.env.mode.prod,
...this.env.packageInfo,
},
- namespace
+ `config validation of [${namespace}]`
);
}