From 8e489d089e1510d6bf8af72216501085185b3e23 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Sun, 25 Dec 2022 20:09:35 +0530 Subject: [PATCH 01/10] test: add tests for string representation of booleans in config Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index c0627a200b..b6179bd67e 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -22,6 +22,7 @@ const DATA = { one: 1, true: true, false: false, + stringFalse: 'false', null: null, string: 'string', emptyString: '', @@ -53,6 +54,7 @@ function expectValidValues(config: ConfigReader) { expect(config.getOptional('true')).toBe(true); expect(config.getBoolean('true')).toBe(true); expect(config.getBoolean('false')).toBe(false); + expect(config.getBoolean('stringFalse')).toBe(false); expect(config.getString('string')).toBe('string'); expect(config.get('strings')).toEqual(['string1', 'string2']); expect(config.getStringArray('strings')).toEqual(['string1', 'string2']); From ac1e91c252396a9fb2645c3553ab233162f711ab Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Sun, 25 Dec 2022 20:32:09 +0530 Subject: [PATCH 02/10] feat: handle string representation of boolean values in the config Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.test.ts | 5 ++++- packages/config/src/reader.ts | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index b6179bd67e..13ee243f3f 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -87,8 +87,11 @@ function expectValidValues(config: ConfigReader) { } function expectInvalidValues(config: ConfigReader) { + expect(() => config.getBoolean('zero')).toThrow( + "Invalid type in config for key 'zero' in 'ctx', got number, wanted boolean", + ); expect(() => config.getBoolean('string')).toThrow( - "Invalid type in config for key 'string' in 'ctx', got string, wanted boolean", + "Unable to convert config value for key 'string' in 'ctx' to a boolean", ); expect(() => config.getNumber('string')).toThrow( "Unable to convert config value for key 'string' in 'ctx' to a number", diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 1ce54590b5..743c20f95c 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -280,10 +280,26 @@ export class ConfigReader implements Config { /** {@inheritdoc Config.getOptionalBoolean} */ getOptionalBoolean(key: string): boolean | undefined { - return this.readConfigValue( + const value = this.readConfigValue( key, - value => typeof value === 'boolean' || { expected: 'boolean' }, + val => + typeof val === 'boolean' || + typeof val === 'string' || { expected: 'boolean' }, ); + if (typeof value === 'boolean' || value === undefined) { + return value; + } + let boolean; + if (value === 'true') { + boolean = true; + } else if (value === 'false') { + boolean = false; + } else { + throw new Error( + errors.convert(this.fullKey(key), this.context, 'boolean'), + ); + } + return boolean; } /** {@inheritdoc Config.getString} */ From ba2d69ee170db568016c68e109241c6d93450702 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Sun, 25 Dec 2022 20:38:15 +0530 Subject: [PATCH 03/10] feat: add changeset Signed-off-by: Sayak Mukhopadhyay --- .changeset/tidy-flies-cheer.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tidy-flies-cheer.md diff --git a/.changeset/tidy-flies-cheer.md b/.changeset/tidy-flies-cheer.md new file mode 100644 index 0000000000..d8c3780821 --- /dev/null +++ b/.changeset/tidy-flies-cheer.md @@ -0,0 +1,5 @@ +--- +'@backstage/config': patch +--- + +Handle a case when boolean configuration parameters are given a string type of 'true' or 'false'. This happens particularly when such parameters are used with environmental substitution as environment variables are always strings. From 485ccd98fb29d218a84b21fd8e6128f3f354f5ad Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Tue, 27 Dec 2022 16:31:45 +0530 Subject: [PATCH 04/10] test: add tests for coercing some strings and numbers to boolean Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 13ee243f3f..3c40ee95c2 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -22,6 +22,14 @@ const DATA = { one: 1, true: true, false: false, + yes: 'yes', + no: 'no', + y: 'y', + n: 'n', + on: 'on', + off: 'off', + zeroString: '0', + oneString: '1', stringFalse: 'false', null: null, string: 'string', @@ -55,6 +63,16 @@ function expectValidValues(config: ConfigReader) { expect(config.getBoolean('true')).toBe(true); expect(config.getBoolean('false')).toBe(false); expect(config.getBoolean('stringFalse')).toBe(false); + expect(config.getBoolean('zero')).toBe(false); + expect(config.getBoolean('one')).toBe(true); + expect(config.getBoolean('zeroString')).toBe(false); + expect(config.getBoolean('oneString')).toBe(true); + expect(config.getBoolean('yes')).toBe(true); + expect(config.getBoolean('no')).toBe(false); + expect(config.getBoolean('y')).toBe(true); + expect(config.getBoolean('n')).toBe(false); + expect(config.getBoolean('on')).toBe(true); + expect(config.getBoolean('off')).toBe(false); expect(config.getString('string')).toBe('string'); expect(config.get('strings')).toEqual(['string1', 'string2']); expect(config.getStringArray('strings')).toEqual(['string1', 'string2']); From 57e91a24b9eca919c3137aee17527e46f38d4071 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Tue, 27 Dec 2022 16:37:32 +0530 Subject: [PATCH 05/10] feat: coerce some strings and numbers into boolean Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 743c20f95c..6b0b418559 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -280,19 +280,20 @@ export class ConfigReader implements Config { /** {@inheritdoc Config.getOptionalBoolean} */ getOptionalBoolean(key: string): boolean | undefined { - const value = this.readConfigValue( + const value = this.readConfigValue( key, val => typeof val === 'boolean' || + typeof val === 'number' || typeof val === 'string' || { expected: 'boolean' }, ); if (typeof value === 'boolean' || value === undefined) { return value; } let boolean; - if (value === 'true') { + if (/^(?:y|yes|true|1|on)$/i.test(value as string) || value === 1) { boolean = true; - } else if (value === 'false') { + } else if (/^(?:n|no|false|0|off)$/i.test(value as string) || value === 0) { boolean = false; } else { throw new Error( From 74eed91f5b08e1df6fbb1aff22bcfb43c3cf9d57 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Wed, 28 Dec 2022 21:38:01 +0530 Subject: [PATCH 06/10] tests: remove test for invalid number 0 for boolean as it is coerced now Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 3c40ee95c2..c4c2a4ecbf 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -105,9 +105,6 @@ function expectValidValues(config: ConfigReader) { } function expectInvalidValues(config: ConfigReader) { - expect(() => config.getBoolean('zero')).toThrow( - "Invalid type in config for key 'zero' in 'ctx', got number, wanted boolean", - ); expect(() => config.getBoolean('string')).toThrow( "Unable to convert config value for key 'string' in 'ctx' to a boolean", ); From bfd1161dcfed4a0cd74d928469707d68e0f4d5ee Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Wed, 28 Dec 2022 22:03:19 +0530 Subject: [PATCH 07/10] refactor: simplify returns Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 6b0b418559..95924c45da 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -290,17 +290,13 @@ export class ConfigReader implements Config { if (typeof value === 'boolean' || value === undefined) { return value; } - let boolean; if (/^(?:y|yes|true|1|on)$/i.test(value as string) || value === 1) { - boolean = true; - } else if (/^(?:n|no|false|0|off)$/i.test(value as string) || value === 0) { - boolean = false; - } else { - throw new Error( - errors.convert(this.fullKey(key), this.context, 'boolean'), - ); + return true; } - return boolean; + if (/^(?:n|no|false|0|off)$/i.test(value as string) || value === 0) { + return false; + } + throw new Error(errors.convert(this.fullKey(key), this.context, 'boolean')); } /** {@inheritdoc Config.getString} */ From 94b4ab688d9a0b1f864cffab89c451bf525551f7 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Thu, 29 Dec 2022 17:06:33 +0530 Subject: [PATCH 08/10] refactor: simplify coersion of type to string Co-authored-by: Ben Lambert Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 95924c45da..fe3a8dbeaa 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -290,10 +290,12 @@ export class ConfigReader implements Config { if (typeof value === 'boolean' || value === undefined) { return value; } - if (/^(?:y|yes|true|1|on)$/i.test(value as string) || value === 1) { + const valueString = String(value).trim(); + + if (/^(?:y|yes|true|1|on)$/i.test(valueString)) { return true; } - if (/^(?:n|no|false|0|off)$/i.test(value as string) || value === 0) { + if (/^(?:n|no|false|0|off)$/i.test(valueString)) { return false; } throw new Error(errors.convert(this.fullKey(key), this.context, 'boolean')); From f8e8307c1f3cbcdec9e5ead3bddd5bb43264a3c3 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Thu, 29 Dec 2022 17:12:13 +0530 Subject: [PATCH 09/10] feat: update changeset to reflect broader scope Signed-off-by: Sayak Mukhopadhyay --- .changeset/tidy-flies-cheer.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/tidy-flies-cheer.md b/.changeset/tidy-flies-cheer.md index d8c3780821..78de902e63 100644 --- a/.changeset/tidy-flies-cheer.md +++ b/.changeset/tidy-flies-cheer.md @@ -2,4 +2,6 @@ '@backstage/config': patch --- -Handle a case when boolean configuration parameters are given a string type of 'true' or 'false'. This happens particularly when such parameters are used with environmental substitution as environment variables are always strings. +Adds the ability to coerce values to their boolean representatives. +Values such as `"true"` `1` `on` and `y` will become `true` when using `getBoolean` and the opposites `false`. +This happens particularly when such parameters are used with environmental substitution as environment variables are always strings. From bdfe792890ff87be023991a7dbc25865e7d7ea42 Mon Sep 17 00:00:00 2001 From: Sayak Mukhopadhyay Date: Thu, 29 Dec 2022 19:04:25 +0530 Subject: [PATCH 10/10] fix: prettyfying things Signed-off-by: Sayak Mukhopadhyay --- packages/config/src/reader.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index fe3a8dbeaa..4a570b6977 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -290,8 +290,8 @@ export class ConfigReader implements Config { if (typeof value === 'boolean' || value === undefined) { return value; } - const valueString = String(value).trim(); - + const valueString = String(value).trim(); + if (/^(?:y|yes|true|1|on)$/i.test(valueString)) { return true; }