Skip to content

Add #set and #toJSON to RC ServerTemplate #2522

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

Merged
merged 7 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions etc/firebase-admin.remote-config.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export interface InAppDefaultValue {

// @public
export interface InitServerTemplateOptions extends GetServerTemplateOptions {
template?: ServerTemplateData | string;
template?: ServerTemplateDataType;
}

// @public
Expand Down Expand Up @@ -183,9 +183,10 @@ export interface ServerConfig {

// @public
export interface ServerTemplate {
cache: ServerTemplateData;
evaluate(context?: EvaluationContext): ServerConfig;
load(): Promise<void>;
set(template: ServerTemplateDataType): void;
toJSON(): ServerTemplateData;
}

// @public
Expand All @@ -198,6 +199,9 @@ export interface ServerTemplateData {
version?: Version;
}

// @public
export type ServerTemplateDataType = ServerTemplateData | string;

// @public
export type TagColor = 'BLUE' | 'BROWN' | 'CYAN' | 'DEEP_ORANGE' | 'GREEN' | 'INDIGO' | 'LIME' | 'ORANGE' | 'PINK' | 'PURPLE' | 'TEAL';

Expand Down
1 change: 1 addition & 0 deletions src/remote-config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export {
ServerConfig,
ServerTemplate,
ServerTemplateData,
ServerTemplateDataType,
TagColor,
Value,
ValueSource,
Expand Down
28 changes: 19 additions & 9 deletions src/remote-config/remote-config-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,13 @@ export interface GetServerTemplateOptions {
defaultConfig?: DefaultConfig;
}

/**
* Represents the type of a Remote Config server template that can be set on
* {@link ServerTemplate}. This can either be a {@link ServerTemplateData} object
* or a template JSON string.
*/
export type ServerTemplateDataType = ServerTemplateData | string;

/**
* Represents optional arguments that can be used when instantiating
* {@link ServerTemplate} synchonously.
Expand All @@ -375,22 +382,14 @@ export interface InitServerTemplateOptions extends GetServerTemplateOptions {
* example, customers can reduce initialization latency by pre-fetching and
* caching template data and then using this option to initialize the SDK with
* that data.
* The template can be initialized with either a {@link ServerTemplateData}
* object or a JSON string.
*/
template?: ServerTemplateData|string,
template?: ServerTemplateDataType,
}

/**
* Represents a stateful abstraction for a Remote Config server template.
*/
export interface ServerTemplate {

/**
* Cached {@link ServerTemplateData}.
*/
cache: ServerTemplateData;

/**
* Evaluates the current template to produce a {@link ServerConfig}.
*/
Expand All @@ -401,6 +400,17 @@ export interface ServerTemplate {
* project's {@link ServerTemplate}.
*/
load(): Promise<void>;

/**
* Sets and caches a {@link ServerTemplateData} or a JSON string representing
* the server template
*/
set(template: ServerTemplateDataType): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious (because I think either is fine) so we need a separate type for this or should this just be ServerTemplateData | string here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either works, I originally had it as ServerTemplateData | string but we are defining it in quite a few places across multiple files, so I moved it to a type 😃


/**
* Returns a JSON representation of {@link ServerTemplateData}
*/
toJSON(): ServerTemplateData;
}

/**
Expand Down
46 changes: 32 additions & 14 deletions src/remote-config/remote-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
DefaultConfig,
GetServerTemplateOptions,
InitServerTemplateOptions,
ServerTemplateDataType,
} from './remote-config-api';
import { isString } from 'lodash';

Expand Down Expand Up @@ -200,20 +201,9 @@ export class RemoteConfig {
public initServerTemplate(options?: InitServerTemplateOptions): ServerTemplate {
const template = new ServerTemplateImpl(
this.client, new ConditionEvaluator(), options?.defaultConfig);

if (options?.template) {
// Check and instantiates the template via a json string
if (isString(options?.template)) {
try {
template.cache = new ServerTemplateDataImpl(JSON.parse(options?.template));
} catch (e) {
throw new FirebaseRemoteConfigError(
'invalid-argument',
`Failed to parse the JSON string: ${options?.template}. ` + e
);
}
} else {
template.cache = options?.template;
}
template.set(options?.template);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, intuitive reuse of set 👍

}
return template;
}
Expand Down Expand Up @@ -307,7 +297,7 @@ class RemoteConfigTemplateImpl implements RemoteConfigTemplate {
* Remote Config dataplane template data implementation.
*/
class ServerTemplateImpl implements ServerTemplate {
public cache: ServerTemplateData;
private cache: ServerTemplateData;
private stringifiedDefaultConfig: {[key: string]: string} = {};

constructor(
Expand All @@ -333,6 +323,27 @@ class ServerTemplateImpl implements ServerTemplate {
});
}

/**
* Parses a {@link ServerTemplateDataType} and caches it.
*/
public set(template: ServerTemplateDataType): void {
let parsed;
if (isString(template)) {
try {
parsed = JSON.parse(template);
} catch (e) {
// Transforms JSON parse errors to Firebase error.
throw new FirebaseRemoteConfigError(
'invalid-argument',
`Failed to parse the JSON string: ${template}. ` + e);
}
} else {
parsed = template;
}
// Throws template parse errors.
this.cache = new ServerTemplateDataImpl(parsed);
}

/**
* Evaluates the current template in cache to produce a {@link ServerConfig}.
*/
Expand Down Expand Up @@ -402,6 +413,13 @@ class ServerTemplateImpl implements ServerTemplate {

return new ServerConfigImpl(configValues);
}

/**
* @returns JSON representation of the server template
*/
public toJSON(): ServerTemplateData {
return this.cache;
}
}

class ServerConfigImpl implements ServerConfig {
Expand Down
Loading