Adopt code review feedback for config constraints

This change adopts the code review suggestion to use a bag of options
for config constraints rather than having overloaded function names.
This is a much cleaner approach, lets us use more descriptive names,
and is far more future proof in case we decide to add more capabilities.
This commit is contained in:
joeduffy 2018-08-28 18:05:24 -07:00
parent 834987bcd3
commit c2258be408
3 changed files with 112 additions and 247 deletions

View file

@ -47,87 +47,33 @@ export class Config {
* get loads an optional configuration value by its key, or undefined if it doesn't exist.
*
* @param key The key to lookup.
* @param opts An options bag to constrain legal values.
*/
public get(key: string): string | undefined {
return getConfig(this.fullKey(key));
}
/**
* getEnum loads an optional configuration value by its key, or undefined if it doesn't exist. If the value is not
* within the array of legal values, an error will be thrown.
*
* @param key The key to lookup.
* @param values The legal enum values.
*/
public getEnum(key: string, values: string[]): string | undefined {
public get(key: string, opts?: StringConfigOptions): string | undefined {
const v = getConfig(this.fullKey(key));
if (v !== undefined && values.indexOf(v) === -1) {
throw new ConfigEnumError(this.fullKey(key), v, values);
if (v === undefined) {
return undefined;
}
return v;
}
/**
* getMinMax loads an optional string configuration value by its key, or undefined if it doesn't exist. If the
* value's length is less than or greater than the specified number of characters, this function throws.
*
* @param key The key to lookup.
* @param min The minimum string length.
* @param max The maximum string length.
*/
public getMinMax(key: string, min: number, max: number): string | undefined {
const v = getConfig(this.fullKey(key));
if (v !== undefined && (v.length < min || v.length > max)) {
throw new ConfigRangeError(this.fullKey(key), v, min, max);
}
return v;
}
/**
* getMinMaxPattern loads an optional string configuration value by its key, or undefined if it doesn't exist. If
* the value's length is less than or greater than the specified number of characters, or the string does not match
* the supplied regular expression, this function throws.
*
* @param key The key to lookup.
* @param min The minimum string length.
* @param max The maximum string length.
* @param regexp A regular expression the string must match.
*/
public getMinMaxPattern(key: string, min: number, max: number, regexp: string | RegExp): string | undefined {
if (typeof regexp === "string") {
regexp = new RegExp(regexp);
}
const v = getConfig(this.fullKey(key));
if (v !== undefined) {
if (v.length < min || v.length > max) {
throw new ConfigRangeError(this.fullKey(key), v, min, max);
} else if (!regexp.test(v)) {
throw new ConfigPatternError(this.fullKey(key), v, regexp);
if (opts) {
if (opts.allowedValues !== undefined && opts.allowedValues.indexOf(v) === -1) {
throw new ConfigEnumError(this.fullKey(key), v, opts.allowedValues);
} else if (opts.minLength !== undefined && v.length < opts.minLength) {
throw new ConfigRangeError(this.fullKey(key), v, opts.minLength, undefined);
} else if (opts.maxLength !== undefined && v.length > opts.maxLength) {
throw new ConfigRangeError(this.fullKey(key), v, undefined, opts.maxLength);
} else if (opts.pattern !== undefined) {
let pattern = opts.pattern;
if (typeof pattern === "string") {
pattern = new RegExp(pattern);
}
if (!pattern.test(v)) {
throw new ConfigPatternError(this.fullKey(key), v, pattern);
}
}
}
return v;
}
/**
* getPattern loads an optional string configuration value by its key, or undefined if it doesn't exist. If the
* value doesn't match the regular expression pattern, the function throws.
*
* @param key The key to lookup.
* @param regexp A regular expression the string must match.
*/
public getPattern(key: string, regexp: string | RegExp): string | undefined {
if (typeof regexp === "string") {
regexp = new RegExp(regexp);
}
const v = getConfig(this.fullKey(key));
if (v !== undefined && !regexp.test(v)) {
throw new ConfigPatternError(this.fullKey(key), v, regexp);
}
return v;
}
/**
* getBoolean loads an optional configuration value, as a boolean, by its key, or undefined if it doesn't exist.
* If the configuration value isn't a legal boolean, this function will throw an error.
@ -151,8 +97,9 @@ export class Config {
* If the configuration value isn't a legal number, this function will throw an error.
*
* @param key The key to lookup.
* @param opts An options bag to constrain legal values.
*/
public getNumber(key: string): number | undefined {
public getNumber(key: string, opts?: NumberConfigOptions): number | undefined {
const v: string | undefined = this.get(key);
if (v === undefined) {
return undefined;
@ -161,24 +108,14 @@ export class Config {
if (isNaN(f)) {
throw new ConfigTypeError(this.fullKey(key), v, "number");
}
return f;
}
/**
* getNumberMinMax loads an optional configuration value, as a number, by its key, or undefined if it doesn't exist.
* If the configuration value isn't a legal number, this function will throw an error. The range is a pair of min
* and max values that, should there be a value, the number must fall within, inclusively, else an error is thrown.
*
* @param key The key to lookup.
* @param min The minimum value the number may be, inclusive.
* @param max The maximum value the number may be, inclusive.
*/
public getNumberMinMax(key: string, min: number, max: number): number | undefined {
const v: number | undefined = this.getNumber(key);
if (v !== undefined && (v < min || v > max)) {
throw new ConfigRangeError(this.fullKey(key), v, min, max);
if (opts) {
if (opts.min !== undefined && f < opts.min) {
throw new ConfigRangeError(this.fullKey(key), f, opts.min, undefined);
} else if (opts.max !== undefined && f > opts.max) {
throw new ConfigRangeError(this.fullKey(key), f, undefined, opts.max);
}
}
return v;
return f;
}
/**
@ -204,72 +141,10 @@ export class Config {
* require loads a configuration value by its given key. If it doesn't exist, an error is thrown.
*
* @param key The key to lookup.
* @param opts An options bag to constrain legal values.
*/
public require(key: string): string {
const v: string | undefined = this.get(key);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
return v;
}
/**
* requireEnum loads a configuration value by its given key. If it doesn't exist, an error is thrown. If the value
* is not within the array of legal values, an error will be thrown.
*
* @param key The key to lookup.
* @param values The legal enum values.
*/
public requireEnum(key: string, values: string[]): string {
const v: string | undefined = this.getEnum(key, values);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
return v;
}
/**
* requireMinMax loads a string configuration value by its key. If it doesn't exist, an error is thrown. If the
* value's length is less than or greater than the specified number of characters, this function throws.
*
* @param key The key to lookup.
* @param min The minimum string length.
* @param max The maximum string length.
*/
public requireMinMax(key: string, min: number, max: number): string {
const v = this.getMinMax(key, min, max);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
return v;
}
/**
* requireMinMaxPattern loads a string configuration value by its key. If it doesn't exist, an error is thrown. If
* the value's length is less than or greater than the specified number of characters, this function throws.
*
* @param key The key to lookup.
* @param min The minimum string length.
* @param max The maximum string length.
* @param regexp A regular expression the string must match.
*/
public requireMinMaxPattern(key: string, min: number, max: number, pattern: string | RegExp): string {
const v = this.getMinMaxPattern(key, min, max, pattern);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
return v;
}
/**
* requirePattern loads a string configuration value by its key. If it doesn't exist, an error is thrown. If the
* value's length is less than or greater than the specified number of characters, this function throws.
*
* @param key The key to lookup.
* @param regexp A regular expression the string must match.
*/
public requirePattern(key: string, pattern: string | RegExp): string {
const v = this.getPattern(key, pattern);
public require(key: string, opts?: StringConfigOptions): string {
const v: string | undefined = this.get(key, opts);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
@ -290,31 +165,15 @@ export class Config {
return v;
}
/**
* requireNumberMinMax loads a configuration value, as a number, by its given key. If it doesn't exist, or the
* configuration value is not a legal number, an error is thrown. The range is a pair of min and max values that,
* should there be a value, the number must fall within, inclusively, else an error is thrown.
*
* @param key The key to lookup.
* @param min The minimum value the number may be, inclusive.
* @param max The maximum value the number may be, inclusive.
*/
public requireNumberMinMax(key: string, min: number, max: number): number {
const v: number | undefined = this.getNumberMinMax(key, min, max);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
return v;
}
/**
* requireNumberMinMax loads a configuration value, as a number, by its given key. If it doesn't exist, or the
* configuration value is not a legal number, an error is thrown.
*
* @param key The key to lookup.
* @param opts An options bag to constrain legal values.
*/
public requireNumber(key: string): number {
const v: number | undefined = this.getNumber(key);
public requireNumber(key: string, opts?: NumberConfigOptions): number {
const v: number | undefined = this.getNumber(key, opts);
if (v === undefined) {
throw new ConfigMissingError(this.fullKey(key));
}
@ -345,6 +204,42 @@ export class Config {
}
}
/**
* StringConfigOptions may be used to constrain the set of legal values a string config value may contain.
*/
interface StringConfigOptions {
/**
* The legal enum values. If it does not match, a ConfigEnumError is thrown.
*/
allowedValues?: string[];
/**
* The minimum string length. If the string is not this long, a ConfigRangeError is thrown.
*/
minLength?: number;
/**
* The maximum string length. If the string is longer than this, a ConfigRangeError is thrown.
*/
maxLength?: number;
/**
* A regular expression the string must match. If it does not match, a ConfigPatternError is thrown.
*/
pattern?: string | RegExp;
}
/**
* NumberConfigOptions may be used to constrain the set of legal values a number config value may contain.
*/
interface NumberConfigOptions {
/**
* The minimum number value, inclusive. If the number is less than this, a ConfigRangeError is thrown.
*/
min?: number;
/**
* The maximum number value, inclusive. If the number is greater than this, a ConfigRangeError is thrown.
*/
max?: number;
}
/**
* ConfigTypeError is used when a configuration value is of the wrong type.
*/

View file

@ -65,57 +65,57 @@ describe("config", () => {
it("does enums", () => {
runtime.setConfig("pkg:color", "orange");
const config = new Config("pkg");
assert.strictEqual("orange", config.getEnum("color", [ "purple", "orange", "blue" ]));
assert.strictEqual(undefined, config.getEnum("missing", [ "purple", "orange", "blue" ]));
assert.strictEqual("orange", config.requireEnum("color", [ "purple", "orange", "blue" ]));
assert.throws(() => { config.getEnum("color", [ "purple", "black", "blue" ]); });
assert.throws(() => { config.requireEnum("color", [ "purple", "black", "blue" ]); });
assert.throws(() => { config.requireEnum("missing", [ "purple", "orange", "blue" ]); });
assert.strictEqual("orange", config.get("color", { allowedValues: [ "purple", "orange", "blue" ] }));
assert.strictEqual(undefined, config.get("missing", { allowedValues: [ "purple", "orange", "blue" ] }));
assert.strictEqual("orange", config.require("color", { allowedValues: [ "purple", "orange", "blue" ] }));
assert.throws(() => { config.get("color", { allowedValues: [ "purple", "black", "blue" ] }); });
assert.throws(() => { config.require("color", { allowedValues: [ "purple", "black", "blue" ] }); });
assert.throws(() => { config.require("missing", { allowedValues: [ "purple", "orange", "blue" ] }); });
});
it("does min/max (strlen)", () => {
runtime.setConfig("pkg:strlen", "abcdefgh");
const config = new Config("pkg");
assert.strictEqual("abcdefgh", config.getMinMax("strlen", 0, 8));
assert.strictEqual("abcdefgh", config.getMinMax("strlen", 8, 8));
assert.strictEqual("abcdefgh", config.getMinMax("strlen", 0, 16));
assert.strictEqual(undefined, config.getMinMax("missing", 0, 8));
assert.strictEqual("abcdefgh", config.requireMinMax("strlen", 0, 8));
assert.strictEqual("abcdefgh", config.requireMinMax("strlen", 8, 8));
assert.strictEqual("abcdefgh", config.requireMinMax("strlen", 0, 16));
assert.throws(() => { config.getMinMax("strlen", 9, 16); });
assert.throws(() => { config.getMinMax("strlen", 0, 7); });
assert.throws(() => { config.requireMinMax("strlen", 9, 16); });
assert.throws(() => { config.requireMinMax("strlen", 0, 7); });
assert.throws(() => { config.requireMinMax("missing", 0, 8); });
assert.strictEqual("abcdefgh", config.get("strlen", { minLength: 0, maxLength: 8 }));
assert.strictEqual("abcdefgh", config.get("strlen", { minLength: 8, maxLength: 8 }));
assert.strictEqual("abcdefgh", config.get("strlen", { minLength: 0, maxLength: 16 }));
assert.strictEqual(undefined, config.get("missing", { minLength: 0, maxLength: 8 }));
assert.strictEqual("abcdefgh", config.require("strlen", { minLength: 0, maxLength: 8 }));
assert.strictEqual("abcdefgh", config.require("strlen", { minLength: 8, maxLength: 8 }));
assert.strictEqual("abcdefgh", config.require("strlen", { minLength: 0, maxLength: 16 }));
assert.throws(() => { config.get("strlen", { minLength: 9, maxLength: 16 }); });
assert.throws(() => { config.get("strlen", { minLength: 0, maxLength: 7 }); });
assert.throws(() => { config.require("strlen", { minLength: 9, maxLength: 16 }); });
assert.throws(() => { config.require("strlen", { minLength: 0, maxLength: 7 }); });
assert.throws(() => { config.require("missing", { minLength: 0, maxLength: 8 }); });
});
it("does pattern matching", () => {
runtime.setConfig("pkg:pattern", "aBcDeFgH");
const config = new Config("pkg");
assert.strictEqual("aBcDeFgH", config.getPattern("pattern", /^[a-zA-Z]*$/));
assert.strictEqual("aBcDeFgH", config.getPattern("pattern", "^[a-zA-Z]*$"));
assert.strictEqual(undefined, config.getPattern("missing", /^[a-zA-Z]*$/));
assert.strictEqual("aBcDeFgH", config.requirePattern("pattern", /^[a-zA-Z]*$/));
assert.strictEqual("aBcDeFgH", config.requirePattern("pattern", "^[a-zA-Z]*$"));
assert.throws(() => { config.getPattern("pattern", /^[a-z]*$/); }, "bad pattern: get");
assert.throws(() => { config.getPattern("pattern", "/^[a-z]*$/"); }, "bad pattern (string): get");
assert.throws(() => { config.requirePattern("pattern", /^[a-z]*$/); }, "bad pattern: require");
assert.throws(() => { config.requirePattern("pattern", "/^[a-z]*$/"); }, "bad pattern (string): require");
assert.throws(() => { config.requirePattern("missing", /^[a-z]*$/); }, "missing");
assert.strictEqual("aBcDeFgH", config.get("pattern", { pattern: /^[a-zA-Z]*$/ }));
assert.strictEqual("aBcDeFgH", config.get("pattern", { pattern: "^[a-zA-Z]*$" }));
assert.strictEqual(undefined, config.get("missing", { pattern: /^[a-zA-Z]*$/ }));
assert.strictEqual("aBcDeFgH", config.require("pattern", { pattern: /^[a-zA-Z]*$/ }));
assert.strictEqual("aBcDeFgH", config.require("pattern", { pattern: "^[a-zA-Z]*$" }));
assert.throws(() => { config.get("pattern", { pattern: /^[a-z]*$/ }); }, "bad pattern: get");
assert.throws(() => { config.get("pattern", { pattern: "/^[a-z]*$/" }); }, "bad pattern (string): get");
assert.throws(() => { config.require("pattern", { pattern: /^[a-z]*$/ }); }, "bad pattern: require");
assert.throws(() => { config.require("pattern", { pattern: "/^[a-z]*$/" }); }, "bad pattern (string): require");
assert.throws(() => { config.require("missing", { pattern: /^[a-z]*$/ }); }, "missing");
});
it("does min/max (numbers)", () => {
runtime.setConfig("pkg:quantity", "8");
const config = new Config("pkg");
assert.strictEqual(8, config.getNumberMinMax("quantity", 0, 8));
assert.strictEqual(8, config.getNumberMinMax("quantity", 8, 8));
assert.strictEqual(8, config.getNumberMinMax("quantity", 0, 16));
assert.strictEqual(undefined, config.getNumberMinMax("missing", 0, 8));
assert.strictEqual(8, config.requireNumberMinMax("quantity", 0, 8));
assert.strictEqual(8, config.requireNumberMinMax("quantity", 8, 8));
assert.strictEqual(8, config.requireNumberMinMax("quantity", 0, 16));
assert.throws(() => { config.getNumberMinMax("quantity", 9, 16); });
assert.throws(() => { config.getNumberMinMax("quantity", 0, 7); });
assert.throws(() => { config.requireNumberMinMax("quantity", 9, 16); });
assert.throws(() => { config.requireNumberMinMax("quantity", 0, 7); });
assert.throws(() => { config.requireNumberMinMax("missing", 0, 8); });
assert.strictEqual(8, config.getNumber("quantity", { min: 0, max: 8 }));
assert.strictEqual(8, config.getNumber("quantity", { min: 8, max: 8 }));
assert.strictEqual(8, config.getNumber("quantity", { min: 0, max: 16 }));
assert.strictEqual(undefined, config.getNumber("missing", { min: 0, max: 8 }));
assert.strictEqual(8, config.requireNumber("quantity", { min: 0, max: 8 }));
assert.strictEqual(8, config.requireNumber("quantity", { min: 8, max: 8 }));
assert.strictEqual(8, config.requireNumber("quantity", { min: 0, max: 16 }));
assert.throws(() => { config.getNumber("quantity", { min: 9, max: 16 }); });
assert.throws(() => { config.getNumber("quantity", { min: 0, max: 7 }); });
assert.throws(() => { config.requireNumber("quantity", { min: 9, max: 16 }); });
assert.throws(() => { config.requireNumber("quantity", { min: 0, max: 7 }); });
assert.throws(() => { config.requireNumber("missing", { min: 0, max: 8 }); });
});
});

View file

@ -4829,11 +4829,8 @@ var __runtime = {getConfig: __getConfig, getProject: __0_getProject};
var __metadata_1 = {getProject: __getProject};
__f1.prototype = __testConfig_proto;
Object.defineProperty(__testConfig_proto, "constructor", { configurable: true, writable: true, value: __f1 });
Object.defineProperty(__testConfig_proto, "get", { configurable: true, writable: true, value: __f2 });
(...)
Object.defineProperty(__testConfig_proto, "require", { configurable: true, writable: true, value: __f17 });
(...)
Object.defineProperty(__testConfig_proto, "fullKey", { configurable: true, writable: true, value: __f27 });
Object.defineProperty(__testConfig_proto, "fullKey", { configurable: true, writable: true, value: __f17 });
var __testConfig = Object.create(__testConfig_proto);
__testConfig.name = "test";
@ -4894,18 +4891,6 @@ return function /*constructor*/(name) {
}
}).apply(undefined, undefined).apply(this, arguments);
}
function __f2() {
return (function() {
with({ runtime_1: __runtime }) {
return function /*get*/(key) {
return runtime_1.getConfig(this.fullKey(key));
};
}
}).apply(undefined, undefined).apply(this, arguments);
}
(...)
function __f0() {
return (function() {
@ -4934,11 +4919,8 @@ var __runtime = {getProject: __0_getProject, getConfig: __getConfig};
var __metadata_1 = {getProject: __getProject};
var __f1_prototype = {};
Object.defineProperty(__f1_prototype, "constructor", { configurable: true, writable: true, value: __f1 });
Object.defineProperty(__f1_prototype, "get", { configurable: true, writable: true, value: __f2 });
(...)
Object.defineProperty(__f1_prototype, "require", { configurable: true, writable: true, value: __f17 });
(...)
Object.defineProperty(__f1_prototype, "fullKey", { configurable: true, writable: true, value: __f27 });
Object.defineProperty(__f1_prototype, "fullKey", { configurable: true, writable: true, value: __f17 });
__f1.prototype = __f1_prototype;
var __pulumi = {Config: __f1};
@ -4999,18 +4981,6 @@ return function /*constructor*/(name) {
}
}).apply(undefined, undefined).apply(this, arguments);
}
function __f2() {
return (function() {
with({ runtime_1: __runtime }) {
return function /*get*/(key) {
return runtime_1.getConfig(this.fullKey(key));
};
}
}).apply(undefined, undefined).apply(this, arguments);
}
(...)
function __f0() {
return (function() {