-
Notifications
You must be signed in to change notification settings - Fork 396
Define type-specific getters for SSRC #2519
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
Changes from all commits
5732ea9
a59da2f
9bb2303
fbecfe7
a4b78f3
b6bbe9d
f2509b4
f0d4e53
9e0cb44
fac1259
a28252c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/*! | ||
* Copyright 2024 Google Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
'use strict'; | ||
|
||
import { | ||
Value, | ||
ValueSource, | ||
} from '../remote-config-api'; | ||
|
||
/** | ||
* Implements type-safe getters for parameter values. | ||
* | ||
* Visible for testing. | ||
* | ||
* @internal | ||
*/ | ||
export class ValueImpl implements Value { | ||
public static readonly DEFAULT_VALUE_FOR_BOOLEAN = false; | ||
public static readonly DEFAULT_VALUE_FOR_STRING = ''; | ||
public static readonly DEFAULT_VALUE_FOR_NUMBER = 0; | ||
public static readonly BOOLEAN_TRUTHY_VALUES = ['1', 'true', 't', 'yes', 'y', 'on']; | ||
constructor( | ||
private readonly source: ValueSource, | ||
private readonly value = ValueImpl.DEFAULT_VALUE_FOR_STRING) { } | ||
asString(): string { | ||
return this.value; | ||
} | ||
asBoolean(): boolean { | ||
if (this.source === 'static') { | ||
return ValueImpl.DEFAULT_VALUE_FOR_BOOLEAN; | ||
} | ||
return ValueImpl.BOOLEAN_TRUTHY_VALUES.indexOf(this.value.toLowerCase()) >= 0; | ||
} | ||
asNumber(): number { | ||
if (this.source === 'static') { | ||
return ValueImpl.DEFAULT_VALUE_FOR_NUMBER; | ||
} | ||
const num = Number(this.value); | ||
if (isNaN(num)) { | ||
return ValueImpl.DEFAULT_VALUE_FOR_NUMBER; | ||
} | ||
return num; | ||
} | ||
getSource(): ValueSource { | ||
return this.source; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -361,7 +361,7 @@ export interface GetServerTemplateOptions { | |
* intended before it connects to the Remote Config backend, and so that | ||
* default values are available if none are set on the backend. | ||
*/ | ||
defaultConfig?: ServerConfig, | ||
defaultConfig?: DefaultConfig; | ||
} | ||
|
||
/** | ||
|
@@ -539,4 +539,98 @@ export interface ListVersionsOptions { | |
/** | ||
* Represents the configuration produced by evaluating a server template. | ||
*/ | ||
export type ServerConfig = { [key: string]: string | boolean | number } | ||
export interface ServerConfig { | ||
|
||
/** | ||
* Gets the value for the given key as a boolean. | ||
* | ||
* Convenience method for calling <code>serverConfig.getValue(key).asBoolean()</code>. | ||
* | ||
* @param key - The name of the parameter. | ||
* | ||
* @returns The value for the given key as a boolean. | ||
*/ | ||
getBoolean(key: string): boolean; | ||
|
||
/** | ||
* Gets the value for the given key as a number. | ||
* | ||
* Convenience method for calling <code>serverConfig.getValue(key).asNumber()</code>. | ||
* | ||
* @param key - The name of the parameter. | ||
* | ||
* @returns The value for the given key as a number. | ||
*/ | ||
getNumber(key: string): number; | ||
|
||
/** | ||
* Gets the value for the given key as a string. | ||
* Convenience method for calling <code>serverConfig.getValue(key).asString()</code>. | ||
* | ||
* @param key - The name of the parameter. | ||
* | ||
* @returns The value for the given key as a string. | ||
*/ | ||
getString(key: string): string; | ||
|
||
/** | ||
* Gets the {@link Value} for the given key. | ||
* | ||
* Ensures application logic will always have a type-safe reference, | ||
* even if the parameter is removed remotely. | ||
* | ||
* @param key - The name of the parameter. | ||
* | ||
* @returns The value for the given key. | ||
*/ | ||
getValue(key: string): Value; | ||
} | ||
|
||
/** | ||
* Wraps a parameter value with metadata and type-safe getters. | ||
* | ||
* Type-safe getters insulate application logic from remote | ||
* changes to parameter names and types. | ||
*/ | ||
export interface Value { | ||
|
||
/** | ||
* Gets the value as a boolean. | ||
* | ||
* The following values (case insensitive) are interpreted as true: | ||
* "1", "true", "t", "yes", "y", "on". Other values are interpreted as false. | ||
*/ | ||
asBoolean(): boolean; | ||
|
||
/** | ||
* Gets the value as a number. Comparable to calling <code>Number(value) || 0</code>. | ||
*/ | ||
asNumber(): number; | ||
|
||
/** | ||
* Gets the value as a string. | ||
*/ | ||
asString(): string; | ||
|
||
/** | ||
* Gets the {@link ValueSource} for the given key. | ||
*/ | ||
getSource(): ValueSource; | ||
} | ||
|
||
/** | ||
* Indicates the source of a value. | ||
* | ||
* <ul> | ||
* <li>"static" indicates the value was defined by a static constant.</li> | ||
* <li>"default" indicates the value was defined by default config.</li> | ||
* <li>"remote" indicates the value was defined by config produced by | ||
* evaluating a template.</li> | ||
* </ul> | ||
*/ | ||
export type ValueSource = 'static' | 'default' | 'remote'; | ||
|
||
/** | ||
* Defines the format for in-app default parameter values. | ||
*/ | ||
export type DefaultConfig = { [key: string]: string | number | boolean }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this also contain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could add I don't think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have a 'DEFAULT_VALUE_FOR_BOOLEAN' for the static behavior? Similar to https://github.com/firebase/firebase-js-sdk/blob/master/packages/remote-config/src/value.ts#L36-L39?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it's inconsistent, but given the default value is an empty string when the source is static,
BOOLEAN_TRUTHY_VALUES.indexOf(this.value.toLowerCase())
will return false, so I'm not seeing a technical need ... That said, I can imagine how explicitly defining the default value for boolean would make our system easier to understand, so I'll make the change.