From a01fe561babf1ca606d6b6ec6e4ab1796b702a5a Mon Sep 17 00:00:00 2001 From: Xin Wei Date: Tue, 13 Feb 2024 10:49:01 -0800 Subject: [PATCH 01/13] [SSRC] Add API changes needed for SSRC --- src/remote-config/remote-config-api.ts | 80 +++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index 90f3bd4970..15d601ff6c 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -54,6 +54,27 @@ export interface RemoteConfigCondition { tagColor?: TagColor; } +/** + * Interface representing a Remote Config condition in the data-plane. + * A condition targets a specific group of users. A list of these conditions make up + * part of a Remote Config template. + */ +export interface RemoteConfigServerCondition { + + /** + * A non-empty and unique name of this condition. + */ + name: string; + + /** + * The logic of this condition. + * See the documentation on + * {@link https://firebase.google.com/docs/remote-config/condition-reference | condition expressions} + * for the expected syntax of this field. + */ + expression: string; +} + /** * Interface representing an explicit parameter value. */ @@ -135,7 +156,7 @@ export interface RemoteConfigParameterGroup { } /** - * Interface representing a Remote Config template. + * Interface representing a Remote Config client template. */ export interface RemoteConfigTemplate { /** @@ -167,6 +188,58 @@ export interface RemoteConfigTemplate { version?: Version; } +/** + * Interface representing the template data returned by the Remote Config API in the data-plane. + */ +export interface RemoteConfigServerTemplateData { + /** + * A list of conditions in descending order by priority. + */ + conditions: RemoteConfigServerCondition[]; + + /** + * Map of parameter keys to their optional default values and optional conditional values. + */ + parameters: { [key: string]: RemoteConfigParameter }; + + /** + * ETag of the current Remote Config template (readonly). + */ + readonly etag: string; + + /** + * Version information for the current Remote Config template. + */ + version?: Version; +} + +/** + * Interface representing non-API data for the Remote Config template in the data-plane. + */ +export interface RemoteConfigServerTemplate { + + /** + * Cached {@link RemoteConfigServerTemplateData} + */ + cache: RemoteConfigServerTemplateData; + + /** + * A {@link RemoteConfigServerConfig} containing default values for Config + */ + defaultConfig: RemoteConfigServerConfig; + + /** + * Evaluates the current template to produce a {@link RemoteConfigServerConfig} + */ + evaluate(): RemoteConfigServerConfig; + + /** + * Fetches and caches the current active version of the + * {@link RemoteConfigServerTemplate} of the project. + */ + load(): Promise; +} + /** * Interface representing a Remote Config user. */ @@ -289,3 +362,8 @@ export interface ListVersionsOptions { */ endTime?: Date | string; } + +/** + * Defines the type for the configuration produced by evaluating a server template. + */ +export type RemoteConfigServerConfig = { [key: string]: string | boolean | number } From e48a65ab9d113fb587cda9edb72eff4ab5b3b6e8 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 13 Feb 2024 11:45:05 -0800 Subject: [PATCH 02/13] Strip whitespace and improve documentation consistency --- src/remote-config/remote-config-api.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index 15d601ff6c..dd9f641034 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -189,7 +189,7 @@ export interface RemoteConfigTemplate { } /** - * Interface representing the template data returned by the Remote Config API in the data-plane. + * Interface representing the data in a Remote Config server template. */ export interface RemoteConfigServerTemplateData { /** @@ -214,7 +214,7 @@ export interface RemoteConfigServerTemplateData { } /** - * Interface representing non-API data for the Remote Config template in the data-plane. + * Interface representing a stateful abstraction for a Remote Config server template. */ export interface RemoteConfigServerTemplate { @@ -234,7 +234,7 @@ export interface RemoteConfigServerTemplate { evaluate(): RemoteConfigServerConfig; /** - * Fetches and caches the current active version of the + * Fetches and caches the current active version of the * {@link RemoteConfigServerTemplate} of the project. */ load(): Promise; @@ -364,6 +364,6 @@ export interface ListVersionsOptions { } /** - * Defines the type for the configuration produced by evaluating a server template. + * Type representing the configuration produced by evaluating a server template. */ export type RemoteConfigServerConfig = { [key: string]: string | boolean | number } From ab077c4a8ba8e2a2b9fad7e4d6ad5f12ee8103e0 Mon Sep 17 00:00:00 2001 From: Xin Wei Date: Tue, 13 Feb 2024 10:54:49 -0800 Subject: [PATCH 03/13] [SSRC] Add client internals and tests --- .../remote-config-api-client-internal.ts | 48 +++++++++++++++++-- .../remote-config-api-client.spec.ts | 36 +++++++++++++- 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/remote-config/remote-config-api-client-internal.ts b/src/remote-config/remote-config-api-client-internal.ts index b8cfe22fc4..8eab84c1bd 100644 --- a/src/remote-config/remote-config-api-client-internal.ts +++ b/src/remote-config/remote-config-api-client-internal.ts @@ -21,10 +21,16 @@ import { PrefixedFirebaseError } from '../utils/error'; import * as utils from '../utils/index'; import * as validator from '../utils/validator'; import { deepCopy } from '../utils/deep-copy'; -import { ListVersionsOptions, ListVersionsResult, RemoteConfigTemplate } from './remote-config-api'; +import { + ListVersionsOptions, + ListVersionsResult, + RemoteConfigTemplate, + RemoteConfigServerTemplateData +} from './remote-config-api'; // Remote Config backend constants -const FIREBASE_REMOTE_CONFIG_V1_API = 'https://firebaseremoteconfig.googleapis.com/v1'; +// Honors env param to enable URL override independent of binary change. +const FIREBASE_REMOTE_CONFIG_URL_BASE = process.env.FIREBASE_REMOTE_CONFIG_URL_BASE || 'https://firebaseremoteconfig.googleapis.com'; const FIREBASE_REMOTE_CONFIG_HEADERS = { 'X-Firebase-Client': `fire-admin-node/${utils.getSdkVersion()}`, // There is a known issue in which the ETag is not properly returned in cases where the request @@ -166,6 +172,24 @@ export class RemoteConfigApiClient { }); } + public getServerTemplate(): Promise { + return this.getUrl() + .then((url) => { + const request: HttpRequestConfig = { + method: 'GET', + url: `${url}/templates/server`, + headers: FIREBASE_REMOTE_CONFIG_HEADERS + }; + return this.httpClient.send(request); + }) + .then((resp) => { + return this.toRemoteConfigServerTemplate(resp); + }) + .catch((err) => { + throw this.toFirebaseError(err); + }); + } + private sendPutRequest(template: RemoteConfigTemplate, etag: string, validateOnly?: boolean): Promise { let path = 'remoteConfig'; if (validateOnly) { @@ -191,7 +215,7 @@ export class RemoteConfigApiClient { private getUrl(): Promise { return this.getProjectIdPrefix() .then((projectIdPrefix) => { - return `${FIREBASE_REMOTE_CONFIG_V1_API}/${projectIdPrefix}`; + return `${FIREBASE_REMOTE_CONFIG_URL_BASE}/v1/${projectIdPrefix}`; }); } @@ -255,6 +279,24 @@ export class RemoteConfigApiClient { }; } + /** + * Creates a RemoteConfigServerTemplate from the API response. + * If provided, customEtag is used instead of the etag returned in the API response. + * + * @param {HttpResponse} resp API response object. + * @param {string} customEtag A custom etag to replace the etag fom the API response (Optional). + */ + private toRemoteConfigServerTemplate(resp: HttpResponse, customEtag?: string): RemoteConfigServerTemplateData { + const etag = (typeof customEtag === 'undefined') ? resp.headers['etag'] : customEtag; + this.validateEtag(etag); + return { + conditions: resp.data.conditions, + parameters: resp.data.parameters, + etag, + version: resp.data.version, + }; + } + /** * Checks if the given RemoteConfigTemplate object is valid. * The object must have valid parameters, parameter groups, conditions, and an etag. diff --git a/test/unit/remote-config/remote-config-api-client.spec.ts b/test/unit/remote-config/remote-config-api-client.spec.ts index 9c66f78a41..89feaafe8a 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -33,6 +33,7 @@ import { getSdkVersion } from '../../../src/utils/index'; import { RemoteConfigTemplate, Version, ListVersionsResult, } from '../../../src/remote-config/index'; +import { RemoteConfigServerTemplateData } from '../../../src/remote-config/remote-config-api'; const expect = chai.expect; @@ -661,6 +662,36 @@ describe('RemoteConfigApiClient', () => { }); }); + describe('getServerTemplate', () => { + it('should reject when project id is not available', () => { + return clientWithoutProjectId.getServerTemplate() + .should.eventually.be.rejectedWith(noProjectId); + }); + + // // tests for api response validations + runEtagHeaderTests(() => apiClient.getServerTemplate()); + runErrorResponseTests(() => apiClient.getServerTemplate()); + + it('should resolve with the latest template on success', () => { + const stub = sinon + .stub(HttpClient.prototype, 'send') + .resolves(utils.responseFrom(TEST_RESPONSE, 200, { etag: 'etag-123456789012-1' })); + stubs.push(stub); + return apiClient.getServerTemplate() + .then((resp) => { + expect(resp.conditions).to.deep.equal(TEST_RESPONSE.conditions); + expect(resp.parameters).to.deep.equal(TEST_RESPONSE.parameters); + expect(resp.etag).to.equal('etag-123456789012-1'); + expect(resp.version).to.deep.equal(TEST_RESPONSE.version); + expect(stub).to.have.been.calledOnce.and.calledWith({ + method: 'GET', + url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/templates/server', + headers: EXPECTED_HEADERS, + }); + }); + }); + }); + function runTemplateVersionNumberTests(rcOperation: (v: string | number) => any): void { ['', null, NaN, true, [], {}].forEach((invalidVersion) => { it(`should reject if the versionNumber is: ${invalidVersion}`, () => { @@ -677,7 +708,7 @@ describe('RemoteConfigApiClient', () => { }); } - function runEtagHeaderTests(rcOperation: () => Promise): void { + function runEtagHeaderTests(rcOperation: () => Promise): void { it('should reject when the etag is not present in the response', () => { const stub = sinon .stub(HttpClient.prototype, 'send') @@ -690,7 +721,8 @@ describe('RemoteConfigApiClient', () => { }); } - function runErrorResponseTests(rcOperation: () => Promise): void { + function runErrorResponseTests( + rcOperation: () => Promise): void { it('should reject when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') From 8f145c672575aaea3cf057086ad06147144bf113 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 13 Feb 2024 13:10:31 -0800 Subject: [PATCH 04/13] Update URL to match API proposal --- src/remote-config/remote-config-api-client-internal.ts | 2 +- test/unit/remote-config/remote-config-api-client.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/remote-config/remote-config-api-client-internal.ts b/src/remote-config/remote-config-api-client-internal.ts index 8eab84c1bd..16d98946f8 100644 --- a/src/remote-config/remote-config-api-client-internal.ts +++ b/src/remote-config/remote-config-api-client-internal.ts @@ -177,7 +177,7 @@ export class RemoteConfigApiClient { .then((url) => { const request: HttpRequestConfig = { method: 'GET', - url: `${url}/templates/server`, + url: `${url}/namespaces/firebase-server/serverRemoteConfig`, headers: FIREBASE_REMOTE_CONFIG_HEADERS }; return this.httpClient.send(request); diff --git a/test/unit/remote-config/remote-config-api-client.spec.ts b/test/unit/remote-config/remote-config-api-client.spec.ts index 89feaafe8a..da2c87c639 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -668,7 +668,7 @@ describe('RemoteConfigApiClient', () => { .should.eventually.be.rejectedWith(noProjectId); }); - // // tests for api response validations + // tests for api response validations runEtagHeaderTests(() => apiClient.getServerTemplate()); runErrorResponseTests(() => apiClient.getServerTemplate()); @@ -685,7 +685,7 @@ describe('RemoteConfigApiClient', () => { expect(resp.version).to.deep.equal(TEST_RESPONSE.version); expect(stub).to.have.been.calledOnce.and.calledWith({ method: 'GET', - url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/templates/server', + url: 'https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/namespaces/firebase-server/serverRemoteConfig', headers: EXPECTED_HEADERS, }); }); From afec9ecb76c1f44677b8ac443600054ab1249375 Mon Sep 17 00:00:00 2001 From: Xin Wei Date: Tue, 13 Feb 2024 10:57:07 -0800 Subject: [PATCH 05/13] [SSRC] Add changes to RC --- src/remote-config/remote-config.ts | 176 +++++++ test/integration/remote-config.spec.ts | 59 +++ test/unit/remote-config/remote-config.spec.ts | 460 ++++++++++++++++++ 3 files changed, 695 insertions(+) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 27cbd05793..699cbb0a69 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -23,9 +23,15 @@ import { RemoteConfigCondition, RemoteConfigParameter, RemoteConfigParameterGroup, + RemoteConfigServerTemplate, RemoteConfigTemplate, RemoteConfigUser, Version, + ExplicitParameterValue, + InAppDefaultValue, + ParameterValueType, + RemoteConfigServerConfig, + RemoteConfigServerTemplateData } from './remote-config-api'; /** @@ -35,6 +41,12 @@ export class RemoteConfig { private readonly client: RemoteConfigApiClient; + /** + * An in-memory cache for the {@link RemoteConfigServerTemplate} of the project. + */ + public cachedServerTemplate: RemoteConfigServerTemplate; + + /** * @param app - The app for this RemoteConfig service. * @constructor @@ -168,6 +180,33 @@ export class RemoteConfig { return new RemoteConfigTemplateImpl(template); } + + /** + * Instantiates {@link RemoteConfigServerTemplate} and then fetches and caches the latest + * template version of the project. + */ + public async getServerTemplate(options?: { + defaultConfig?: RemoteConfigServerConfig, + template?: RemoteConfigServerTemplateData, + }): Promise { + const template = this.initServerTemplate(options); + await template.load(); + return template; + } + + /** + * Synchronously instantiates {@link RemoteConfigServerTemplate}. + */ + public initServerTemplate(options?: { + defaultConfig?: RemoteConfigServerConfig, + template?: RemoteConfigServerTemplateData, + }): RemoteConfigServerTemplate { + const template = new RemoteConfigServerTemplateImpl(this.client, options?.defaultConfig); + if (options?.template) { + template.cache = options?.template; + } + return template; + } } /** @@ -254,6 +293,143 @@ class RemoteConfigTemplateImpl implements RemoteConfigTemplate { } } +/** + * Remote Config data-plane template data implementation. + */ +class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { + public cache: RemoteConfigServerTemplateData; + + constructor( + private readonly apiClient: RemoteConfigApiClient, + public readonly defaultConfig: RemoteConfigServerConfig = {} + ) { } + + /** + * Fetches and caches the current active version of the {@link RemoteConfigServerTemplate} of the project. + */ + public load(): Promise { + return this.apiClient.getServerTemplate() + .then((template) => { + this.cache = new RemoteConfigServerTemplateDataImpl(template); + }); + } + + /** + * Evaluates the current template in cache to produce a {@link RemoteConfigServerConfig} + */ + public evaluate(): RemoteConfigServerConfig { + if (!this.cache) { + throw new FirebaseRemoteConfigError( + 'failed-precondition', + 'No Remote Config Server template in cache. Call load() before calling evaluate().'); + } + + const renderedConfig: RemoteConfigServerConfig = {}; + + for (const [key, parameter] of Object.entries(this.cache.parameters)) { + const { defaultValue, valueType } = parameter; + + if (!defaultValue) { + console.debug(`Filtering out parameter ${key} with no default value`); + continue; + } + + if ((defaultValue as InAppDefaultValue).useInAppDefault) { + console.debug(`Filtering out parameter ${key} with "use in-app default" value`); + continue; + } + + const parameterDefaultValue = (defaultValue as ExplicitParameterValue).value; + + renderedConfig[key] = this.parseRemoteConfigParameterValue(valueType, parameterDefaultValue); + } + + // Merges rendered config over default config. + const mergedConfig = Object.assign(this.defaultConfig, renderedConfig); + + // Enables config to be a convenient object, but with the ability to perform additional + // functionality when a value is retrieved. + const proxyHandler = { + get(target: RemoteConfigServerConfig, prop: string) { + return target[prop]; + } + }; + + return new Proxy(mergedConfig, proxyHandler); + } + + /** + * Private helper method to process and parse a parameter value based on {@link ParameterValueType} + */ + private parseRemoteConfigParameterValue(parameterType: ParameterValueType | undefined, + parameterDefaultValue: string): string | number | boolean { + const BOOLEAN_TRUTHY_VALUES = ['1', 'true', 't', 'yes', 'y', 'on']; + const DEFAULT_VALUE_FOR_NUMBER = 0; + const DEFAULT_VALUE_FOR_STRING = ''; + + if (parameterType === 'BOOLEAN') { + return BOOLEAN_TRUTHY_VALUES.indexOf(parameterDefaultValue) >= 0; + } else if (parameterType === 'NUMBER') { + const num = Number(parameterDefaultValue); + if (isNaN(num)) { + return DEFAULT_VALUE_FOR_NUMBER; + } + return num; + } else { + // Treat everything else as string + return parameterDefaultValue || DEFAULT_VALUE_FOR_STRING; + } + } +} + +/** + * Remote Config data-plane template data implementation. + */ +class RemoteConfigServerTemplateDataImpl implements RemoteConfigServerTemplateData { + public parameters: { [key: string]: RemoteConfigParameter }; + public parameterGroups: { [key: string]: RemoteConfigParameterGroup }; + public conditions: RemoteConfigCondition[]; + public readonly etag: string; + public version?: Version; + + constructor(template: RemoteConfigServerTemplateData) { + if (!validator.isNonNullObject(template) || + !validator.isNonEmptyString(template.etag)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + `Invalid Remote Config template: ${JSON.stringify(template)}`); + } + + this.etag = template.etag; + + if (typeof template.parameters !== 'undefined') { + if (!validator.isNonNullObject(template.parameters)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + 'Remote Config parameters must be a non-null object'); + } + this.parameters = template.parameters; + } else { + this.parameters = {}; + } + + if (typeof template.conditions !== 'undefined') { + if (!validator.isArray(template.conditions)) { + throw new FirebaseRemoteConfigError( + 'invalid-argument', + 'Remote Config conditions must be an array'); + } + this.conditions = template.conditions; + } else { + this.conditions = []; + } + + if (typeof template.version !== 'undefined') { + this.version = new VersionImpl(template.version); + } + } +} + /** * Remote Config Version internal implementation. */ diff --git a/test/integration/remote-config.spec.ts b/test/integration/remote-config.spec.ts index 79689a64e6..dff976f27b 100644 --- a/test/integration/remote-config.spec.ts +++ b/test/integration/remote-config.spec.ts @@ -23,6 +23,7 @@ import { RemoteConfigCondition, RemoteConfigTemplate, } from '../../lib/remote-config/index'; +import { RemoteConfigServerTemplate } from '../../src/remote-config/remote-config-api'; chai.should(); chai.use(chaiAsPromised); @@ -287,4 +288,62 @@ describe('admin.remoteConfig', () => { expect(newTemplate.parameterGroups).to.deep.equal(VALID_PARAMETER_GROUPS); }); }); + + describe('getServerTemplate', () => { + it('should return the most recently published template', () => { + // TODO: use global mock data once there's a REST API for server templates. + // Until then, copy/paste this into test server. + const VALID_TEMPLATE = { + 'conditions': [ + { + 'name': 'ios', + 'expression': "device.os == 'ios'" + }, + { + 'name': 'android', + 'expression': "device.os == 'android'" + } + ], + 'etag': 'etag-123-456', + 'parameters': { + 'holiday_promo_enabled': { + 'defaultValue': { + 'useInAppDefault': true + }, + 'description': 'promo indicator', + 'valueType': 'STRING' + }, + 'welcome_message': { + 'conditionalValues': { + 'android': { + 'value': 'welcome android text' + }, + 'ios': { + 'value': 'welcome ios text' + } + }, + 'defaultValue': { + 'value': 'welcome text 1706578886883' + }, + 'valueType': 'STRING' + } + }, + 'version': { + 'description': 'asd' + } + }; + + const rc = getRemoteConfig(); + + // Set FIREBASE_REMOTE_CONFIG_URL_BASE env var to point this at a local server, + // eg `FIREBASE_REMOTE_CONFIG_URL_BASE=http://localhost:3000 npm run integration` + // Note this applies to all RC endpoints, so filter to just this test if using a + // server that doesn't implement all endpoints. + return rc.getServerTemplate() + .then((template: RemoteConfigServerTemplate) => { + expect(template.cache).to.deep.equal(VALID_TEMPLATE); + expect(template.evaluate()).to.deep.equal({ welcome_message:'welcome text 1706578886883' }); + }); + }); + }); }); diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 5459ecd90c..5c50c16367 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -34,6 +34,9 @@ import { RemoteConfigApiClient } from '../../../src/remote-config/remote-config-api-client-internal'; import { deepCopy } from '../../../src/utils/deep-copy'; +import { + RemoteConfigServerCondition, RemoteConfigServerTemplate, RemoteConfigServerTemplateData +} from '../../../src/remote-config/remote-config-api'; const expect = chai.expect; @@ -98,6 +101,34 @@ describe('RemoteConfig', () => { version: VERSION_INFO, }; + const SERVER_REMOTE_CONFIG_RESPONSE: { + // This type is effectively a RemoteConfigServerTemplate, but with mutable fields + // to allow easier use from within the tests. An improvement would be to + // alter this into a helper that creates customized RemoteConfigTemplateContent based + // on the needs of the test, as that would ensure type-safety. + conditions?: Array<{ name: string; expression: string; }>; + parameters?: object | null; + etag: string; + version?: object; + } = { + conditions: [ + { + name: 'ios', + expression: 'device.os == \'ios\'' + }, + ], + parameters: { + holiday_promo_enabled: { + defaultValue: { value: 'true' }, + conditionalValues: { ios: { useInAppDefault: true } }, + description: 'this is a promo', + valueType: 'BOOLEAN', + }, + }, + etag: 'etag-123456789012-5', + version: VERSION_INFO, + }; + const REMOTE_CONFIG_TEMPLATE: RemoteConfigTemplate = { conditions: [{ name: 'ios', @@ -511,6 +542,435 @@ describe('RemoteConfig', () => { }); }); + describe('getServerTemplate', () => { + const operationName = 'getServerTemplate'; + + it('should propagate API errors', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .rejects(INTERNAL_ERROR); + stubs.push(stub); + + return remoteConfig.getServerTemplate().should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR); + }); + + it('should resolve a server template on success', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData); + stubs.push(stub); + + return remoteConfig.getServerTemplate() + .then((template) => { + expect(template.cache.conditions.length).to.equal(1); + expect(template.cache.conditions[0].name).to.equal('ios'); + expect(template.cache.conditions[0].expression).to.equal('device.os == \'ios\''); + expect(template.cache.etag).to.equal('etag-123456789012-5'); + + const version = template.cache.version!; + expect(version.versionNumber).to.equal('86'); + expect(version.updateOrigin).to.equal('ADMIN_SDK_NODE'); + expect(version.updateType).to.equal('INCREMENTAL_UPDATE'); + expect(version.updateUser).to.deep.equal({ + email: 'firebase-adminsdk@gserviceaccount.com' + }); + expect(version.description).to.equal('production version'); + expect(version.updateTime).to.equal('Mon, 15 Jun 2020 16:45:03 GMT'); + + const key = 'holiday_promo_enabled'; + const p1 = template.cache.parameters[key]; + expect(p1.defaultValue).deep.equals({ value: 'true' }); + expect(p1.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); + expect(p1.description).equals('this is a promo'); + expect(p1.valueType).equals('BOOLEAN'); + + const c = template.cache.conditions.find((c) => c.name === 'ios'); + expect(c).to.be.not.undefined; + const cond = c as RemoteConfigServerCondition; + expect(cond.name).to.equal('ios'); + expect(cond.expression).to.equal('device.os == \'ios\''); + + const parsed = JSON.parse(JSON.stringify(template.cache)); + const expectedTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + const expectedVersion = deepCopy(VERSION_INFO); + expectedVersion.updateTime = new Date(expectedVersion.updateTime).toUTCString(); + expectedTemplate.version = expectedVersion; + expect(parsed).deep.equals(expectedTemplate); + }); + }); + + it('should set defaultConfig when passed', () => { + const defaultConfig = { + holiday_promo_enabled: false, + holiday_promo_discount: 20, + }; + + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData); + stubs.push(stub); + + return remoteConfig.getServerTemplate({ defaultConfig }) + .then((template) => { + expect(template.defaultConfig.holiday_promo_enabled).to.equal(false); + expect(template.defaultConfig.holiday_promo_discount).to.equal(20); + }); + }); + }); + + describe('initServerTemplate', () => { + it('should set and instantiates template when passed', () => { + const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as RemoteConfigServerTemplateData; + template.parameters = { + dog_type: { + defaultValue: { + value: 'shiba' + }, + description: 'Type of dog breed', + valueType: 'STRING' + } + }; + const initializedTemplate = remoteConfig.initServerTemplate({ template }).cache; + const parsed = JSON.parse(JSON.stringify(initializedTemplate)); + expect(parsed).deep.equals(deepCopy(template)); + }); + }); + + describe('RemoteConfigServerTemplate', () => { + const SERVER_REMOTE_CONFIG_RESPONSE_2 = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + SERVER_REMOTE_CONFIG_RESPONSE_2.parameters = { + dog_type: { + defaultValue: { + value: 'corgi' + }, + description: 'Type of dog breed', + valueType: 'STRING' + }, + dog_type_enabled: { + defaultValue: { + value: 'true' + }, + description: 'It\'s true or false', + valueType: 'BOOLEAN' + }, + dog_age: { + defaultValue: { + value: '22' + }, + description: 'Age', + valueType: 'NUMBER' + }, + dog_jsonified: { + defaultValue: { + value: '{"name":"Taro","breed":"Corgi","age":1,"fluffiness":100}' + }, + description: 'Dog Json Response', + valueType: 'JSON' + }, + dog_use_inapp_default: { + defaultValue: { + useInAppDefault: true + }, + description: 'Use in-app default dog', + valueType: 'STRING' + }, + dog_no_remote_default_value: { + description: 'TIL: default values are optional!', + valueType: 'STRING' + } + }; + + describe('load', () => { + const operationName = 'getServerTemplate'; + + it('should propagate API errors', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .rejects(INTERNAL_ERROR); + stubs.push(stub); + + return remoteConfig.getServerTemplate().should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR); + }); + + it('should reject when API response is invalid', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(undefined); + stubs.push(stub); + return remoteConfig.getServerTemplate().should.eventually.be.rejected.and.have.property( + 'message', 'Invalid Remote Config template: undefined'); + }); + + it('should reject when API response does not contain an ETag', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + response.etag = ''; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', `Invalid Remote Config template: ${JSON.stringify(response)}`); + }); + + it('should reject when API response does not contain valid parameters', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + response.parameters = null; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', 'Remote Config parameters must be a non-null object'); + }); + + it('should reject when API response does not contain valid conditions', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + response.conditions = Object(); + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate() + .should.eventually.be.rejected.and.have.property( + 'message', 'Remote Config conditions must be an array'); + }); + + it('should resolve with parameters:{} when no parameters present in the response', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + response.parameters = undefined; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate() + .then((template) => { + // if parameters are not present in the response, we set it to an empty object. + expect(template.cache.parameters).deep.equals({}); + }); + }); + + it('should resolve with conditions:[] when no conditions present in the response', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + response.conditions = undefined; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate() + .then((template) => { + // if conditions are not present in the response, we set it to an empty array. + expect(template.cache.conditions).deep.equals([]); + }); + }); + + it('should resolve a server template on success', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData); + stubs.push(stub); + + return remoteConfig.getServerTemplate() + .then((template) => { + expect(template.cache.conditions.length).to.equal(1); + expect(template.cache.conditions[0].name).to.equal('ios'); + expect(template.cache.conditions[0].expression).to.equal('device.os == \'ios\''); + expect(template.cache.etag).to.equal('etag-123456789012-5'); + + const version = template.cache.version!; + expect(version.versionNumber).to.equal('86'); + expect(version.updateOrigin).to.equal('ADMIN_SDK_NODE'); + expect(version.updateType).to.equal('INCREMENTAL_UPDATE'); + expect(version.updateUser).to.deep.equal({ + email: 'firebase-adminsdk@gserviceaccount.com' + }); + expect(version.description).to.equal('production version'); + expect(version.updateTime).to.equal('Mon, 15 Jun 2020 16:45:03 GMT'); + + const key = 'holiday_promo_enabled'; + const p1 = template.cache.parameters[key]; + expect(p1.defaultValue).deep.equals({ value: 'true' }); + expect(p1.conditionalValues).deep.equals({ ios: { useInAppDefault: true } }); + expect(p1.description).equals('this is a promo'); + expect(p1.valueType).equals('BOOLEAN'); + + const c = template.cache.conditions.find((c) => c.name === 'ios'); + expect(c).to.be.not.undefined; + const cond = c as RemoteConfigServerCondition; + expect(cond.name).to.equal('ios'); + expect(cond.expression).to.equal('device.os == \'ios\''); + + const parsed = JSON.parse(JSON.stringify(template.cache)); + const expectedTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + const expectedVersion = deepCopy(VERSION_INFO); + expectedVersion.updateTime = new Date(expectedVersion.updateTime).toUTCString(); + expectedTemplate.version = expectedVersion; + expect(parsed).deep.equals(expectedTemplate); + }); + }); + + it('should resolve with template when Version updateTime contains 3 digits in fractional seconds', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + const versionInfo = deepCopy(VERSION_INFO); + versionInfo.updateTime = '2020-10-03T17:14:10.203Z'; + response.version = versionInfo; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + + return remoteConfig.getServerTemplate() + .then((template) => { + expect(template.cache.etag).to.equal('etag-123456789012-5'); + + const version = template.cache.version!; + expect(version.versionNumber).to.equal('86'); + expect(version.updateOrigin).to.equal('ADMIN_SDK_NODE'); + expect(version.updateType).to.equal('INCREMENTAL_UPDATE'); + expect(version.updateUser).to.deep.equal({ + email: 'firebase-adminsdk@gserviceaccount.com' + }); + expect(version.description).to.equal('production version'); + expect(version.updateTime).to.equal('Sat, 03 Oct 2020 17:14:10 GMT'); + }); + }); + + it('should resolve with template when Version updateTime contains 6 digits in fractional seconds', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + const versionInfo = deepCopy(VERSION_INFO); + versionInfo.updateTime = '2020-08-14T17:01:36.541527Z'; + response.version = versionInfo; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + + return remoteConfig.getServerTemplate() + .then((template) => { + expect(template.cache.etag).to.equal('etag-123456789012-5'); + + const version = template.cache.version!; + expect(version.versionNumber).to.equal('86'); + expect(version.updateOrigin).to.equal('ADMIN_SDK_NODE'); + expect(version.updateType).to.equal('INCREMENTAL_UPDATE'); + expect(version.updateUser).to.deep.equal({ + email: 'firebase-adminsdk@gserviceaccount.com' + }); + expect(version.description).to.equal('production version'); + expect(version.updateTime).to.equal('Fri, 14 Aug 2020 17:01:36 GMT'); + }); + }); + + it('should resolve with template when Version updateTime contains 9 digits in fractional seconds', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + const versionInfo = deepCopy(VERSION_INFO); + versionInfo.updateTime = '2020-11-15T06:57:26.342763941Z'; + response.version = versionInfo; + const stub = sinon + .stub(RemoteConfigApiClient.prototype, operationName) + .resolves(response as RemoteConfigServerTemplateData); + stubs.push(stub); + + return remoteConfig.getServerTemplate() + .then((template) => { + expect(template.cache.etag).to.equal('etag-123456789012-5'); + + const version = template.cache.version!; + expect(version.versionNumber).to.equal('86'); + expect(version.updateOrigin).to.equal('ADMIN_SDK_NODE'); + expect(version.updateType).to.equal('INCREMENTAL_UPDATE'); + expect(version.updateUser).to.deep.equal({ + email: 'firebase-adminsdk@gserviceaccount.com' + }); + expect(version.description).to.equal('production version'); + expect(version.updateTime).to.equal('Sun, 15 Nov 2020 06:57:26 GMT'); + }); + }); + }); + + describe('evaluate', () => { + it('returns a config when template is present in cache', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate() + .then((template: RemoteConfigServerTemplate) => { + const config = template.evaluate!(); + expect(config.dog_type).to.equal('corgi'); + expect(config.dog_type_enabled).to.equal(true); + expect(config.dog_age).to.equal(22); + expect(config.dog_jsonified).to.equal('{"name":"Taro","breed":"Corgi","age":1,"fluffiness":100}'); + }); + }); + + it('uses local default if parameter not in template', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate({ + defaultConfig: { + dog_coat: 'blue merle', + } + }) + .then((template: RemoteConfigServerTemplate) => { + const config = template.evaluate!(); + expect(config.dog_coat).to.equal(template.defaultConfig.dog_coat); + }); + }); + + it('uses local default when parameter is in template but default value is undefined', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate({ + defaultConfig: { + dog_no_remote_default_value: 'local default' + } + }) + .then((template: RemoteConfigServerTemplate) => { + const config = template.evaluate!(); + expect(config.dog_no_remote_default_value).to.equal(template.defaultConfig.dog_no_remote_default_value); + }); + }); + + it('uses local default when in-app default value specified', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate({ + defaultConfig: { + dog_use_inapp_default: '🐕' + } + }) + .then((template: RemoteConfigServerTemplate) => { + const config = template.evaluate!(); + expect(config.dog_use_inapp_default).to.equal(template.defaultConfig.dog_use_inapp_default); + }); + }); + + it('overrides local default when value exists', () => { + const stub = sinon + .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + stubs.push(stub); + return remoteConfig.getServerTemplate({ + defaultConfig: { + dog_type_enabled: false + } + }) + .then((template: RemoteConfigServerTemplate) => { + const config = template.evaluate!(); + expect(config.dog_type_enabled).to.equal(template.defaultConfig.dog_type_enabled); + }); + }); + }); + }); + function runInvalidResponseTests(rcOperation: () => Promise, operationName: any): void { it('should propagate API errors', () => { From 1bfb4b25dee3747f49a18f30405b456b54e59a6c Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 13 Feb 2024 15:39:48 -0800 Subject: [PATCH 06/13] Remove unused field, and rename another for consistency --- src/remote-config/remote-config.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 699cbb0a69..f8b4c91e08 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -41,12 +41,6 @@ export class RemoteConfig { private readonly client: RemoteConfigApiClient; - /** - * An in-memory cache for the {@link RemoteConfigServerTemplate} of the project. - */ - public cachedServerTemplate: RemoteConfigServerTemplate; - - /** * @param app - The app for this RemoteConfig service. * @constructor @@ -324,7 +318,7 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { 'No Remote Config Server template in cache. Call load() before calling evaluate().'); } - const renderedConfig: RemoteConfigServerConfig = {}; + const evaluatedConfig: RemoteConfigServerConfig = {}; for (const [key, parameter] of Object.entries(this.cache.parameters)) { const { defaultValue, valueType } = parameter; @@ -341,11 +335,11 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { const parameterDefaultValue = (defaultValue as ExplicitParameterValue).value; - renderedConfig[key] = this.parseRemoteConfigParameterValue(valueType, parameterDefaultValue); + evaluatedConfig[key] = this.parseRemoteConfigParameterValue(valueType, parameterDefaultValue); } // Merges rendered config over default config. - const mergedConfig = Object.assign(this.defaultConfig, renderedConfig); + const mergedConfig = Object.assign(this.defaultConfig, evaluatedConfig); // Enables config to be a convenient object, but with the ability to perform additional // functionality when a value is retrieved. From ca8093ac2e492a1896ec4e8b61e6bc2f6b6c156a Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 13 Feb 2024 18:13:46 -0800 Subject: [PATCH 07/13] Export types from index file --- src/remote-config/index.ts | 3 +++ test/integration/remote-config.spec.ts | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/remote-config/index.ts b/src/remote-config/index.ts index e4719b2e43..72f4343557 100644 --- a/src/remote-config/index.ts +++ b/src/remote-config/index.ts @@ -35,6 +35,9 @@ export { RemoteConfigParameterGroup, RemoteConfigParameterValue, RemoteConfigTemplate, + RemoteConfigServerConfig, + RemoteConfigServerTemplate, + RemoteConfigServerTemplateData, RemoteConfigUser, TagColor, Version, diff --git a/test/integration/remote-config.spec.ts b/test/integration/remote-config.spec.ts index dff976f27b..721a26f5c2 100644 --- a/test/integration/remote-config.spec.ts +++ b/test/integration/remote-config.spec.ts @@ -22,8 +22,10 @@ import { ParameterValueType, RemoteConfigCondition, RemoteConfigTemplate, + RemoteConfigServerConfig, + RemoteConfigServerTemplate, + RemoteConfigServerTemplateData } from '../../lib/remote-config/index'; -import { RemoteConfigServerTemplate } from '../../src/remote-config/remote-config-api'; chai.should(); chai.use(chaiAsPromised); @@ -341,8 +343,14 @@ describe('admin.remoteConfig', () => { // server that doesn't implement all endpoints. return rc.getServerTemplate() .then((template: RemoteConfigServerTemplate) => { - expect(template.cache).to.deep.equal(VALID_TEMPLATE); - expect(template.evaluate()).to.deep.equal({ welcome_message:'welcome text 1706578886883' }); + // Demonstrates explicit template data type usage. + const templateData: RemoteConfigServerTemplateData = template.cache; + expect(templateData).to.deep.equal(VALID_TEMPLATE); + // Demonstrates config type usage. + const config: RemoteConfigServerConfig = template.evaluate(); + expect(config).to.deep.equal({ + welcome_message: 'welcome text 1706578886883' + }); }); }); }); From 8c883bc8b8d8de1afcc8e7fa95b1434c9389de95 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 13 Feb 2024 19:08:23 -0800 Subject: [PATCH 08/13] Add RemoteConfigServerTemplateOptions type and update extracted API --- etc/firebase-admin.remote-config.api.md | 39 +++++++++++++++++++++++++ src/remote-config/index.ts | 2 ++ src/remote-config/remote-config-api.ts | 8 +++++ src/remote-config/remote-config.ts | 13 +++------ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/etc/firebase-admin.remote-config.api.md b/etc/firebase-admin.remote-config.api.md index fb07bfad76..93dfcc452f 100644 --- a/etc/firebase-admin.remote-config.api.md +++ b/etc/firebase-admin.remote-config.api.md @@ -46,8 +46,10 @@ export class RemoteConfig { // (undocumented) readonly app: App; createTemplateFromJSON(json: string): RemoteConfigTemplate; + getServerTemplate(options?: RemoteConfigServerTemplateOptions): Promise; getTemplate(): Promise; getTemplateAtVersion(versionNumber: number | string): Promise; + initServerTemplate(options?: RemoteConfigServerTemplateOptions): RemoteConfigServerTemplate; listVersions(options?: ListVersionsOptions): Promise; publishTemplate(template: RemoteConfigTemplate, options?: { force: boolean; @@ -84,6 +86,43 @@ export interface RemoteConfigParameterGroup { // @public export type RemoteConfigParameterValue = ExplicitParameterValue | InAppDefaultValue; +// @public +export interface RemoteConfigServerCondition { + expression: string; + name: string; +} + +// @public +export type RemoteConfigServerConfig = { + [key: string]: string | boolean | number; +}; + +// @public +export interface RemoteConfigServerTemplate { + cache: RemoteConfigServerTemplateData; + defaultConfig: RemoteConfigServerConfig; + evaluate(): RemoteConfigServerConfig; + load(): Promise; +} + +// @public +export interface RemoteConfigServerTemplateData { + conditions: RemoteConfigServerCondition[]; + readonly etag: string; + parameters: { + [key: string]: RemoteConfigParameter; + }; + version?: Version; +} + +// @public +export interface RemoteConfigServerTemplateOptions { + // (undocumented) + defaultConfig?: RemoteConfigServerConfig; + // (undocumented) + template?: RemoteConfigServerTemplateData; +} + // @public export interface RemoteConfigTemplate { conditions: RemoteConfigCondition[]; diff --git a/src/remote-config/index.ts b/src/remote-config/index.ts index 72f4343557..194929641c 100644 --- a/src/remote-config/index.ts +++ b/src/remote-config/index.ts @@ -35,9 +35,11 @@ export { RemoteConfigParameterGroup, RemoteConfigParameterValue, RemoteConfigTemplate, + RemoteConfigServerCondition, RemoteConfigServerConfig, RemoteConfigServerTemplate, RemoteConfigServerTemplateData, + RemoteConfigServerTemplateOptions, RemoteConfigUser, TagColor, Version, diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index dd9f641034..142bdd0718 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -213,6 +213,14 @@ export interface RemoteConfigServerTemplateData { version?: Version; } +/** + * Interface representing optional arguments for instantiating {@link RemoteConfigServerTemplate}. + */ +export interface RemoteConfigServerTemplateOptions { + defaultConfig?: RemoteConfigServerConfig, + template?: RemoteConfigServerTemplateData, +} + /** * Interface representing a stateful abstraction for a Remote Config server template. */ diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index f8b4c91e08..c975a1246a 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -31,7 +31,8 @@ import { InAppDefaultValue, ParameterValueType, RemoteConfigServerConfig, - RemoteConfigServerTemplateData + RemoteConfigServerTemplateData, + RemoteConfigServerTemplateOptions, } from './remote-config-api'; /** @@ -179,10 +180,7 @@ export class RemoteConfig { * Instantiates {@link RemoteConfigServerTemplate} and then fetches and caches the latest * template version of the project. */ - public async getServerTemplate(options?: { - defaultConfig?: RemoteConfigServerConfig, - template?: RemoteConfigServerTemplateData, - }): Promise { + public async getServerTemplate(options?: RemoteConfigServerTemplateOptions): Promise { const template = this.initServerTemplate(options); await template.load(); return template; @@ -191,10 +189,7 @@ export class RemoteConfig { /** * Synchronously instantiates {@link RemoteConfigServerTemplate}. */ - public initServerTemplate(options?: { - defaultConfig?: RemoteConfigServerConfig, - template?: RemoteConfigServerTemplateData, - }): RemoteConfigServerTemplate { + public initServerTemplate(options?: RemoteConfigServerTemplateOptions): RemoteConfigServerTemplate { const template = new RemoteConfigServerTemplateImpl(this.client, options?.defaultConfig); if (options?.template) { template.cache = options?.template; From 0cb0153b4d3429cc0719f3e8f60ca8d2f059f41c Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Wed, 14 Feb 2024 18:37:57 -0800 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: jen_h --- src/remote-config/remote-config-api.ts | 2 +- src/remote-config/remote-config.ts | 10 +++++----- test/integration/remote-config.spec.ts | 10 +++++++++- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index 142bdd0718..0fd3ed8099 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -214,7 +214,7 @@ export interface RemoteConfigServerTemplateData { } /** - * Interface representing optional arguments for instantiating {@link RemoteConfigServerTemplate}. + * Represents optional arguments that can be used when instantiating {@link RemoteConfigServerTemplate}. */ export interface RemoteConfigServerTemplateOptions { defaultConfig?: RemoteConfigServerConfig, diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index c975a1246a..91db665a50 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -283,7 +283,7 @@ class RemoteConfigTemplateImpl implements RemoteConfigTemplate { } /** - * Remote Config data-plane template data implementation. + * Remote Config dataplane template data implementation. */ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { public cache: RemoteConfigServerTemplateData; @@ -294,7 +294,7 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { ) { } /** - * Fetches and caches the current active version of the {@link RemoteConfigServerTemplate} of the project. + * Fetches and caches the current active version of the project's {@link RemoteConfigServerTemplate}. */ public load(): Promise { return this.apiClient.getServerTemplate() @@ -304,7 +304,7 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { } /** - * Evaluates the current template in cache to produce a {@link RemoteConfigServerConfig} + * Evaluates the current template in cache to produce a {@link RemoteConfigServerConfig}. */ public evaluate(): RemoteConfigServerConfig { if (!this.cache) { @@ -348,7 +348,7 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { } /** - * Private helper method to process and parse a parameter value based on {@link ParameterValueType} + * Private helper method that processes and parses a parameter value based on {@link ParameterValueType}. */ private parseRemoteConfigParameterValue(parameterType: ParameterValueType | undefined, parameterDefaultValue: string): string | number | boolean { @@ -372,7 +372,7 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { } /** - * Remote Config data-plane template data implementation. + * Remote Config dataplane template data implementation. */ class RemoteConfigServerTemplateDataImpl implements RemoteConfigServerTemplateData { public parameters: { [key: string]: RemoteConfigParameter }; diff --git a/test/integration/remote-config.spec.ts b/test/integration/remote-config.spec.ts index 721a26f5c2..c050880e1b 100644 --- a/test/integration/remote-config.spec.ts +++ b/test/integration/remote-config.spec.ts @@ -337,7 +337,15 @@ describe('admin.remoteConfig', () => { const rc = getRemoteConfig(); - // Set FIREBASE_REMOTE_CONFIG_URL_BASE env var to point this at a local server, + /** + * Set a `FIREBASE_REMOTE_CONFIG_URL_BASE` environment variable to direct + * template evaluation to a local server, for example: + * + * `FIREBASE_REMOTE_CONFIG_URL_BASE=http://localhost:3000 npm run integration` + * + * Note that this applies to all Remote Config endpoints, so if you're using a + * server that doesn't implement all endpoints, filter for this specific test. + */ // eg `FIREBASE_REMOTE_CONFIG_URL_BASE=http://localhost:3000 npm run integration` // Note this applies to all RC endpoints, so filter to just this test if using a // server that doesn't implement all endpoints. From 72fa8155598eafeff2a8e00564067c0ed1cb9384 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Wed, 14 Feb 2024 18:40:18 -0800 Subject: [PATCH 10/13] Remove getServerTemplate integration test --- test/integration/remote-config.spec.ts | 75 -------------------------- 1 file changed, 75 deletions(-) diff --git a/test/integration/remote-config.spec.ts b/test/integration/remote-config.spec.ts index c050880e1b..79689a64e6 100644 --- a/test/integration/remote-config.spec.ts +++ b/test/integration/remote-config.spec.ts @@ -22,9 +22,6 @@ import { ParameterValueType, RemoteConfigCondition, RemoteConfigTemplate, - RemoteConfigServerConfig, - RemoteConfigServerTemplate, - RemoteConfigServerTemplateData } from '../../lib/remote-config/index'; chai.should(); @@ -290,76 +287,4 @@ describe('admin.remoteConfig', () => { expect(newTemplate.parameterGroups).to.deep.equal(VALID_PARAMETER_GROUPS); }); }); - - describe('getServerTemplate', () => { - it('should return the most recently published template', () => { - // TODO: use global mock data once there's a REST API for server templates. - // Until then, copy/paste this into test server. - const VALID_TEMPLATE = { - 'conditions': [ - { - 'name': 'ios', - 'expression': "device.os == 'ios'" - }, - { - 'name': 'android', - 'expression': "device.os == 'android'" - } - ], - 'etag': 'etag-123-456', - 'parameters': { - 'holiday_promo_enabled': { - 'defaultValue': { - 'useInAppDefault': true - }, - 'description': 'promo indicator', - 'valueType': 'STRING' - }, - 'welcome_message': { - 'conditionalValues': { - 'android': { - 'value': 'welcome android text' - }, - 'ios': { - 'value': 'welcome ios text' - } - }, - 'defaultValue': { - 'value': 'welcome text 1706578886883' - }, - 'valueType': 'STRING' - } - }, - 'version': { - 'description': 'asd' - } - }; - - const rc = getRemoteConfig(); - - /** - * Set a `FIREBASE_REMOTE_CONFIG_URL_BASE` environment variable to direct - * template evaluation to a local server, for example: - * - * `FIREBASE_REMOTE_CONFIG_URL_BASE=http://localhost:3000 npm run integration` - * - * Note that this applies to all Remote Config endpoints, so if you're using a - * server that doesn't implement all endpoints, filter for this specific test. - */ - // eg `FIREBASE_REMOTE_CONFIG_URL_BASE=http://localhost:3000 npm run integration` - // Note this applies to all RC endpoints, so filter to just this test if using a - // server that doesn't implement all endpoints. - return rc.getServerTemplate() - .then((template: RemoteConfigServerTemplate) => { - // Demonstrates explicit template data type usage. - const templateData: RemoteConfigServerTemplateData = template.cache; - expect(templateData).to.deep.equal(VALID_TEMPLATE); - // Demonstrates config type usage. - const config: RemoteConfigServerConfig = template.evaluate(); - expect(config).to.deep.equal({ - welcome_message: 'welcome text 1706578886883' - }); - }); - }); - }); }); From 9a9a49e801476227ddfa2963b4c57b70dbfc2af5 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 20 Feb 2024 09:57:03 -0800 Subject: [PATCH 11/13] Use console.log, for consistency --- src/remote-config/remote-config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 91db665a50..47d65aaab4 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -319,12 +319,12 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { const { defaultValue, valueType } = parameter; if (!defaultValue) { - console.debug(`Filtering out parameter ${key} with no default value`); + console.log(`Filtering out parameter ${key} with no default value`); continue; } if ((defaultValue as InAppDefaultValue).useInAppDefault) { - console.debug(`Filtering out parameter ${key} with "use in-app default" value`); + console.log(`Filtering out parameter ${key} with "use in-app default" value`); continue; } From e6e577914f2b375961459e53aa515abf9604f429 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 20 Feb 2024 15:06:33 -0800 Subject: [PATCH 12/13] Apply suggestions from code review Co-authored-by: jen_h --- test/unit/remote-config/remote-config.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 5c50c16367..71fcf87f84 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -746,7 +746,7 @@ describe('RemoteConfig', () => { stubs.push(stub); return remoteConfig.getServerTemplate() .then((template) => { - // if parameters are not present in the response, we set it to an empty object. + // If parameters are not present in the response, we set it to an empty object. expect(template.cache.parameters).deep.equals({}); }); }); @@ -760,7 +760,7 @@ describe('RemoteConfig', () => { stubs.push(stub); return remoteConfig.getServerTemplate() .then((template) => { - // if conditions are not present in the response, we set it to an empty array. + // If conditions are not present in the response, we set it to an empty array. expect(template.cache.conditions).deep.equals([]); }); }); From 3da3f28c920cef0bbe99a1eaf340e39e0d0feeff Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 5 Mar 2024 13:59:08 -0800 Subject: [PATCH 13/13] Incorporate review feedback --- etc/firebase-admin.remote-config.api.md | 2 -- src/remote-config/remote-config-api.ts | 13 +++++++++++++ src/remote-config/remote-config.ts | 4 ++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/etc/firebase-admin.remote-config.api.md b/etc/firebase-admin.remote-config.api.md index 93dfcc452f..614fc5dfeb 100644 --- a/etc/firebase-admin.remote-config.api.md +++ b/etc/firebase-admin.remote-config.api.md @@ -117,9 +117,7 @@ export interface RemoteConfigServerTemplateData { // @public export interface RemoteConfigServerTemplateOptions { - // (undocumented) defaultConfig?: RemoteConfigServerConfig; - // (undocumented) template?: RemoteConfigServerTemplateData; } diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index 2392f33eaf..a5d0287d80 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -217,7 +217,20 @@ export interface RemoteConfigServerTemplateData { * Represents optional arguments that can be used when instantiating {@link RemoteConfigServerTemplate}. */ export interface RemoteConfigServerTemplateOptions { + + /** + * Defines in-app default parameter values, so that your app behaves as + * intended before it connects to the Remote Config backend, and so that + * default values are available if none are set on the backend. + */ defaultConfig?: RemoteConfigServerConfig, + + /** + * Enables integrations to use template data loaded independently. For + * 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. + */ template?: RemoteConfigServerTemplateData, } diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 47d65aaab4..cfd965d27b 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -319,12 +319,12 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { const { defaultValue, valueType } = parameter; if (!defaultValue) { - console.log(`Filtering out parameter ${key} with no default value`); + // TODO: add logging once we have a wrapped logger. continue; } if ((defaultValue as InAppDefaultValue).useInAppDefault) { - console.log(`Filtering out parameter ${key} with "use in-app default" value`); + // TODO: add logging once we have a wrapped logger. continue; }