From 53c2145e7e2d0cc1e74f55566d223c1792c21b16 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Fri, 15 Mar 2024 11:36:03 -0700 Subject: [PATCH 1/8] Remove product prefix from exported types --- src/remote-config/index.ts | 10 ++-- .../remote-config-api-client-internal.ts | 6 +-- src/remote-config/remote-config-api.ts | 32 ++++++------ src/remote-config/remote-config.ts | 36 ++++++------- .../remote-config-api-client.spec.ts | 6 +-- test/unit/remote-config/remote-config.spec.ts | 50 +++++++++---------- 6 files changed, 70 insertions(+), 70 deletions(-) diff --git a/src/remote-config/index.ts b/src/remote-config/index.ts index 194929641c..0f3e8904e3 100644 --- a/src/remote-config/index.ts +++ b/src/remote-config/index.ts @@ -29,17 +29,17 @@ export { InAppDefaultValue, ListVersionsOptions, ListVersionsResult, + NamedServerCondition, ParameterValueType, RemoteConfigCondition, RemoteConfigParameter, RemoteConfigParameterGroup, RemoteConfigParameterValue, RemoteConfigTemplate, - RemoteConfigServerCondition, - RemoteConfigServerConfig, - RemoteConfigServerTemplate, - RemoteConfigServerTemplateData, - RemoteConfigServerTemplateOptions, + ServerConfig as RemoteConfigServerConfig, + ServerTemplate as RemoteConfigServerTemplate, + ServerTemplateData as RemoteConfigServerTemplateData, + ServerTemplateOptions as RemoteConfigServerTemplateOptions, RemoteConfigUser, TagColor, Version, diff --git a/src/remote-config/remote-config-api-client-internal.ts b/src/remote-config/remote-config-api-client-internal.ts index 6331eaa1b1..f1a0ad1c10 100644 --- a/src/remote-config/remote-config-api-client-internal.ts +++ b/src/remote-config/remote-config-api-client-internal.ts @@ -25,7 +25,7 @@ import { ListVersionsOptions, ListVersionsResult, RemoteConfigTemplate, - RemoteConfigServerTemplateData + ServerTemplateData } from './remote-config-api'; // Remote Config backend constants @@ -175,7 +175,7 @@ export class RemoteConfigApiClient { }); } - public getServerTemplate(): Promise { + public getServerTemplate(): Promise { return this.getUrl() .then((url) => { const request: HttpRequestConfig = { @@ -289,7 +289,7 @@ export class RemoteConfigApiClient { * @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 { + private toRemoteConfigServerTemplate(resp: HttpResponse, customEtag?: string): ServerTemplateData { const etag = (typeof customEtag === 'undefined') ? resp.headers['etag'] : customEtag; this.validateEtag(etag); return { diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index a5d0287d80..88e1237907 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -59,7 +59,7 @@ export interface RemoteConfigCondition { * A condition targets a specific group of users. A list of these conditions make up * part of a Remote Config template. */ -export interface RemoteConfigServerCondition { +export interface NamedServerCondition { /** * A non-empty and unique name of this condition. @@ -191,11 +191,11 @@ export interface RemoteConfigTemplate { /** * Represents the data in a Remote Config server template. */ -export interface RemoteConfigServerTemplateData { +export interface ServerTemplateData { /** * A list of conditions in descending order by priority. */ - conditions: RemoteConfigServerCondition[]; + conditions: NamedServerCondition[]; /** * Map of parameter keys to their optional default values and optional conditional values. @@ -214,16 +214,16 @@ export interface RemoteConfigServerTemplateData { } /** - * Represents optional arguments that can be used when instantiating {@link RemoteConfigServerTemplate}. + * Represents optional arguments that can be used when instantiating {@link ServerTemplate}. */ -export interface RemoteConfigServerTemplateOptions { +export interface ServerTemplateOptions { /** * 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, + defaultConfig?: ServerConfig, /** * Enables integrations to use template data loaded independently. For @@ -231,32 +231,32 @@ export interface RemoteConfigServerTemplateOptions { * caching template data and then using this option to initialize the SDK with * that data. */ - template?: RemoteConfigServerTemplateData, + template?: ServerTemplateData, } /** * Represents a stateful abstraction for a Remote Config server template. */ -export interface RemoteConfigServerTemplate { +export interface ServerTemplate { /** - * Cached {@link RemoteConfigServerTemplateData}. + * Cached {@link ServerTemplateData}. */ - cache: RemoteConfigServerTemplateData; + cache: ServerTemplateData; /** - * A {@link RemoteConfigServerConfig} that contains default Config values. + * A {@link ServerConfig} that contains default Config values. */ - defaultConfig: RemoteConfigServerConfig; + defaultConfig: ServerConfig; /** - * Evaluates the current template to produce a {@link RemoteConfigServerConfig}. + * Evaluates the current template to produce a {@link ServerConfig}. */ - evaluate(): RemoteConfigServerConfig; + evaluate(): ServerConfig; /** * Fetches and caches the current active version of the - * project's {@link RemoteConfigServerTemplate}. + * project's {@link ServerTemplate}. */ load(): Promise; } @@ -387,4 +387,4 @@ export interface ListVersionsOptions { /** * Represents the configuration produced by evaluating a server template. */ -export type RemoteConfigServerConfig = { [key: string]: string | boolean | number } +export type ServerConfig = { [key: string]: string | boolean | number } diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index cfd965d27b..72fb276c56 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -23,16 +23,16 @@ import { RemoteConfigCondition, RemoteConfigParameter, RemoteConfigParameterGroup, - RemoteConfigServerTemplate, + ServerTemplate, RemoteConfigTemplate, RemoteConfigUser, Version, ExplicitParameterValue, InAppDefaultValue, ParameterValueType, - RemoteConfigServerConfig, - RemoteConfigServerTemplateData, - RemoteConfigServerTemplateOptions, + ServerConfig, + ServerTemplateData, + ServerTemplateOptions, } from './remote-config-api'; /** @@ -177,19 +177,19 @@ export class RemoteConfig { } /** - * Instantiates {@link RemoteConfigServerTemplate} and then fetches and caches the latest + * Instantiates {@link ServerTemplate} and then fetches and caches the latest * template version of the project. */ - public async getServerTemplate(options?: RemoteConfigServerTemplateOptions): Promise { + public async getServerTemplate(options?: ServerTemplateOptions): Promise { const template = this.initServerTemplate(options); await template.load(); return template; } /** - * Synchronously instantiates {@link RemoteConfigServerTemplate}. + * Synchronously instantiates {@link ServerTemplate}. */ - public initServerTemplate(options?: RemoteConfigServerTemplateOptions): RemoteConfigServerTemplate { + public initServerTemplate(options?: ServerTemplateOptions): ServerTemplate { const template = new RemoteConfigServerTemplateImpl(this.client, options?.defaultConfig); if (options?.template) { template.cache = options?.template; @@ -285,16 +285,16 @@ class RemoteConfigTemplateImpl implements RemoteConfigTemplate { /** * Remote Config dataplane template data implementation. */ -class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { - public cache: RemoteConfigServerTemplateData; +class RemoteConfigServerTemplateImpl implements ServerTemplate { + public cache: ServerTemplateData; constructor( private readonly apiClient: RemoteConfigApiClient, - public readonly defaultConfig: RemoteConfigServerConfig = {} + public readonly defaultConfig: ServerConfig = {} ) { } /** - * Fetches and caches the current active version of the project's {@link RemoteConfigServerTemplate}. + * Fetches and caches the current active version of the project's {@link ServerTemplate}. */ public load(): Promise { return this.apiClient.getServerTemplate() @@ -304,16 +304,16 @@ 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 ServerConfig}. */ - public evaluate(): RemoteConfigServerConfig { + public evaluate(): ServerConfig { if (!this.cache) { throw new FirebaseRemoteConfigError( 'failed-precondition', 'No Remote Config Server template in cache. Call load() before calling evaluate().'); } - const evaluatedConfig: RemoteConfigServerConfig = {}; + const evaluatedConfig: ServerConfig = {}; for (const [key, parameter] of Object.entries(this.cache.parameters)) { const { defaultValue, valueType } = parameter; @@ -339,7 +339,7 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { // 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) { + get(target: ServerConfig, prop: string) { return target[prop]; } }; @@ -374,14 +374,14 @@ class RemoteConfigServerTemplateImpl implements RemoteConfigServerTemplate { /** * Remote Config dataplane template data implementation. */ -class RemoteConfigServerTemplateDataImpl implements RemoteConfigServerTemplateData { +class RemoteConfigServerTemplateDataImpl implements ServerTemplateData { public parameters: { [key: string]: RemoteConfigParameter }; public parameterGroups: { [key: string]: RemoteConfigParameterGroup }; public conditions: RemoteConfigCondition[]; public readonly etag: string; public version?: Version; - constructor(template: RemoteConfigServerTemplateData) { + constructor(template: ServerTemplateData) { if (!validator.isNonNullObject(template) || !validator.isNonEmptyString(template.etag)) { throw new FirebaseRemoteConfigError( 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 da2c87c639..52abb968c1 100644 --- a/test/unit/remote-config/remote-config-api-client.spec.ts +++ b/test/unit/remote-config/remote-config-api-client.spec.ts @@ -33,7 +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'; +import { ServerTemplateData } from '../../../src/remote-config/remote-config-api'; const expect = chai.expect; @@ -708,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') @@ -722,7 +722,7 @@ describe('RemoteConfigApiClient', () => { } function runErrorResponseTests( - rcOperation: () => Promise): void { + rcOperation: () => Promise): void { it('should reject when a full platform error response is received', () => { const stub = sinon .stub(HttpClient.prototype, 'send') diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 71fcf87f84..0bc5a7f201 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -35,7 +35,7 @@ import { } from '../../../src/remote-config/remote-config-api-client-internal'; import { deepCopy } from '../../../src/utils/deep-copy'; import { - RemoteConfigServerCondition, RemoteConfigServerTemplate, RemoteConfigServerTemplateData + NamedServerCondition, ServerTemplate, ServerTemplateData } from '../../../src/remote-config/remote-config-api'; const expect = chai.expect; @@ -557,7 +557,7 @@ describe('RemoteConfig', () => { it('should resolve a server template on success', () => { const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() @@ -586,7 +586,7 @@ describe('RemoteConfig', () => { const c = template.cache.conditions.find((c) => c.name === 'ios'); expect(c).to.be.not.undefined; - const cond = c as RemoteConfigServerCondition; + const cond = c as NamedServerCondition; expect(cond.name).to.equal('ios'); expect(cond.expression).to.equal('device.os == \'ios\''); @@ -607,7 +607,7 @@ describe('RemoteConfig', () => { const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate({ defaultConfig }) @@ -620,7 +620,7 @@ describe('RemoteConfig', () => { describe('initServerTemplate', () => { it('should set and instantiates template when passed', () => { - const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as RemoteConfigServerTemplateData; + const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData; template.parameters = { dog_type: { defaultValue: { @@ -706,7 +706,7 @@ describe('RemoteConfig', () => { response.etag = ''; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() .should.eventually.be.rejected.and.have.property( @@ -718,7 +718,7 @@ describe('RemoteConfig', () => { response.parameters = null; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() .should.eventually.be.rejected.and.have.property( @@ -730,7 +730,7 @@ describe('RemoteConfig', () => { response.conditions = Object(); const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() .should.eventually.be.rejected.and.have.property( @@ -742,7 +742,7 @@ describe('RemoteConfig', () => { response.parameters = undefined; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() .then((template) => { @@ -756,7 +756,7 @@ describe('RemoteConfig', () => { response.conditions = undefined; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() .then((template) => { @@ -768,7 +768,7 @@ describe('RemoteConfig', () => { it('should resolve a server template on success', () => { const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() @@ -797,7 +797,7 @@ describe('RemoteConfig', () => { const c = template.cache.conditions.find((c) => c.name === 'ios'); expect(c).to.be.not.undefined; - const cond = c as RemoteConfigServerCondition; + const cond = c as NamedServerCondition; expect(cond.name).to.equal('ios'); expect(cond.expression).to.equal('device.os == \'ios\''); @@ -817,7 +817,7 @@ describe('RemoteConfig', () => { response.version = versionInfo; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() @@ -843,7 +843,7 @@ describe('RemoteConfig', () => { response.version = versionInfo; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() @@ -869,7 +869,7 @@ describe('RemoteConfig', () => { response.version = versionInfo; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(response as RemoteConfigServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() @@ -893,10 +893,10 @@ describe('RemoteConfig', () => { 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); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate() - .then((template: RemoteConfigServerTemplate) => { + .then((template: ServerTemplate) => { const config = template.evaluate!(); expect(config.dog_type).to.equal('corgi'); expect(config.dog_type_enabled).to.equal(true); @@ -908,14 +908,14 @@ describe('RemoteConfig', () => { it('uses local default if parameter not in template', () => { const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') - .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate({ defaultConfig: { dog_coat: 'blue merle', } }) - .then((template: RemoteConfigServerTemplate) => { + .then((template: ServerTemplate) => { const config = template.evaluate!(); expect(config.dog_coat).to.equal(template.defaultConfig.dog_coat); }); @@ -924,14 +924,14 @@ describe('RemoteConfig', () => { 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); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate({ defaultConfig: { dog_no_remote_default_value: 'local default' } }) - .then((template: RemoteConfigServerTemplate) => { + .then((template: ServerTemplate) => { const config = template.evaluate!(); expect(config.dog_no_remote_default_value).to.equal(template.defaultConfig.dog_no_remote_default_value); }); @@ -940,14 +940,14 @@ describe('RemoteConfig', () => { 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); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate({ defaultConfig: { dog_use_inapp_default: '🐕' } }) - .then((template: RemoteConfigServerTemplate) => { + .then((template: ServerTemplate) => { const config = template.evaluate!(); expect(config.dog_use_inapp_default).to.equal(template.defaultConfig.dog_use_inapp_default); }); @@ -956,14 +956,14 @@ describe('RemoteConfig', () => { it('overrides local default when value exists', () => { const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') - .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData); + .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); stubs.push(stub); return remoteConfig.getServerTemplate({ defaultConfig: { dog_type_enabled: false } }) - .then((template: RemoteConfigServerTemplate) => { + .then((template: ServerTemplate) => { const config = template.evaluate!(); expect(config.dog_type_enabled).to.equal(template.defaultConfig.dog_type_enabled); }); From ee675fb7e1c3a741631d79e50abe5c416d503f70 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Fri, 15 Mar 2024 14:33:32 -0700 Subject: [PATCH 2/8] Remove unused "expression" field from server condition --- src/remote-config/remote-config-api.ts | 8 -------- src/remote-config/remote-config.ts | 3 ++- test/unit/remote-config/remote-config.spec.ts | 9 ++------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index 88e1237907..62bb6d22a3 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -65,14 +65,6 @@ export interface NamedServerCondition { * 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; } /** diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 72fb276c56..4d30a8370e 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -33,6 +33,7 @@ import { ServerConfig, ServerTemplateData, ServerTemplateOptions, + NamedServerCondition, } from './remote-config-api'; /** @@ -377,7 +378,7 @@ class RemoteConfigServerTemplateImpl implements ServerTemplate { class RemoteConfigServerTemplateDataImpl implements ServerTemplateData { public parameters: { [key: string]: RemoteConfigParameter }; public parameterGroups: { [key: string]: RemoteConfigParameterGroup }; - public conditions: RemoteConfigCondition[]; + public conditions: NamedServerCondition[]; public readonly etag: string; public version?: Version; diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 0bc5a7f201..0f0821ef0c 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -106,15 +106,14 @@ describe('RemoteConfig', () => { // 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; }>; + conditions?: Array<{ name: string; }>; parameters?: object | null; etag: string; version?: object; } = { conditions: [ { - name: 'ios', - expression: 'device.os == \'ios\'' + name: 'ios' }, ], parameters: { @@ -564,7 +563,6 @@ describe('RemoteConfig', () => { .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!; @@ -588,7 +586,6 @@ describe('RemoteConfig', () => { expect(c).to.be.not.undefined; const cond = c as NamedServerCondition; 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); @@ -775,7 +772,6 @@ describe('RemoteConfig', () => { .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!; @@ -799,7 +795,6 @@ describe('RemoteConfig', () => { expect(c).to.be.not.undefined; const cond = c as NamedServerCondition; 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); From c9701eb2573af713ef73c034538e5a307347b696 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Fri, 15 Mar 2024 14:40:23 -0700 Subject: [PATCH 3/8] Extract API --- etc/firebase-admin.remote-config.api.md | 69 ++++++++++++------------- src/remote-config/index.ts | 8 +-- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/etc/firebase-admin.remote-config.api.md b/etc/firebase-admin.remote-config.api.md index 614fc5dfeb..c68097455a 100644 --- a/etc/firebase-admin.remote-config.api.md +++ b/etc/firebase-admin.remote-config.api.md @@ -38,6 +38,11 @@ export interface ListVersionsResult { versions: Version[]; } +// @public +export interface NamedServerCondition { + name: string; +} + // @public export type ParameterValueType = 'STRING' | 'BOOLEAN' | 'NUMBER' | 'JSON'; @@ -46,10 +51,10 @@ export class RemoteConfig { // (undocumented) readonly app: App; createTemplateFromJSON(json: string): RemoteConfigTemplate; - getServerTemplate(options?: RemoteConfigServerTemplateOptions): Promise; + getServerTemplate(options?: ServerTemplateOptions): Promise; getTemplate(): Promise; getTemplateAtVersion(versionNumber: number | string): Promise; - initServerTemplate(options?: RemoteConfigServerTemplateOptions): RemoteConfigServerTemplate; + initServerTemplate(options?: ServerTemplateOptions): ServerTemplate; listVersions(options?: ListVersionsOptions): Promise; publishTemplate(template: RemoteConfigTemplate, options?: { force: boolean; @@ -87,28 +92,12 @@ export interface RemoteConfigParameterGroup { 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[]; +export interface RemoteConfigTemplate { + conditions: RemoteConfigCondition[]; readonly etag: string; + parameterGroups: { + [key: string]: RemoteConfigParameterGroup; + }; parameters: { [key: string]: RemoteConfigParameter; }; @@ -116,18 +105,29 @@ export interface RemoteConfigServerTemplateData { } // @public -export interface RemoteConfigServerTemplateOptions { - defaultConfig?: RemoteConfigServerConfig; - template?: RemoteConfigServerTemplateData; +export interface RemoteConfigUser { + email: string; + imageUrl?: string; + name?: string; } // @public -export interface RemoteConfigTemplate { - conditions: RemoteConfigCondition[]; +export type ServerConfig = { + [key: string]: string | boolean | number; +}; + +// @public +export interface ServerTemplate { + cache: ServerTemplateData; + defaultConfig: ServerConfig; + evaluate(): ServerConfig; + load(): Promise; +} + +// @public +export interface ServerTemplateData { + conditions: NamedServerCondition[]; readonly etag: string; - parameterGroups: { - [key: string]: RemoteConfigParameterGroup; - }; parameters: { [key: string]: RemoteConfigParameter; }; @@ -135,10 +135,9 @@ export interface RemoteConfigTemplate { } // @public -export interface RemoteConfigUser { - email: string; - imageUrl?: string; - name?: string; +export interface ServerTemplateOptions { + defaultConfig?: ServerConfig; + template?: ServerTemplateData; } // @public diff --git a/src/remote-config/index.ts b/src/remote-config/index.ts index 0f3e8904e3..84eef33fe9 100644 --- a/src/remote-config/index.ts +++ b/src/remote-config/index.ts @@ -36,11 +36,11 @@ export { RemoteConfigParameterGroup, RemoteConfigParameterValue, RemoteConfigTemplate, - ServerConfig as RemoteConfigServerConfig, - ServerTemplate as RemoteConfigServerTemplate, - ServerTemplateData as RemoteConfigServerTemplateData, - ServerTemplateOptions as RemoteConfigServerTemplateOptions, RemoteConfigUser, + ServerConfig, + ServerTemplate, + ServerTemplateData, + ServerTemplateOptions, TagColor, Version, } from './remote-config-api'; From fa21a91f2d29039ff66fedfcf35bc03b7e61b9d6 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Fri, 15 Mar 2024 15:02:17 -0700 Subject: [PATCH 4/8] Remove prefix from impl classes, for consistency --- src/remote-config/remote-config.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 4d30a8370e..208bd630da 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -191,7 +191,7 @@ export class RemoteConfig { * Synchronously instantiates {@link ServerTemplate}. */ public initServerTemplate(options?: ServerTemplateOptions): ServerTemplate { - const template = new RemoteConfigServerTemplateImpl(this.client, options?.defaultConfig); + const template = new ServerTemplateImpl(this.client, options?.defaultConfig); if (options?.template) { template.cache = options?.template; } @@ -286,7 +286,7 @@ class RemoteConfigTemplateImpl implements RemoteConfigTemplate { /** * Remote Config dataplane template data implementation. */ -class RemoteConfigServerTemplateImpl implements ServerTemplate { +class ServerTemplateImpl implements ServerTemplate { public cache: ServerTemplateData; constructor( @@ -300,7 +300,7 @@ class RemoteConfigServerTemplateImpl implements ServerTemplate { public load(): Promise { return this.apiClient.getServerTemplate() .then((template) => { - this.cache = new RemoteConfigServerTemplateDataImpl(template); + this.cache = new ServerTemplateDataImpl(template); }); } @@ -375,7 +375,7 @@ class RemoteConfigServerTemplateImpl implements ServerTemplate { /** * Remote Config dataplane template data implementation. */ -class RemoteConfigServerTemplateDataImpl implements ServerTemplateData { +class ServerTemplateDataImpl implements ServerTemplateData { public parameters: { [key: string]: RemoteConfigParameter }; public parameterGroups: { [key: string]: RemoteConfigParameterGroup }; public conditions: NamedServerCondition[]; From 689c6aa69df58ed6849b783f5dd48b93c1492178 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Tue, 19 Mar 2024 12:59:15 -0700 Subject: [PATCH 5/8] Remove prefix from NamedCondition --- etc/firebase-admin.remote-config.api.md | 4 ++-- src/remote-config/index.ts | 2 +- src/remote-config/remote-config-api.ts | 4 ++-- src/remote-config/remote-config.ts | 4 ++-- test/unit/remote-config/remote-config.spec.ts | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/etc/firebase-admin.remote-config.api.md b/etc/firebase-admin.remote-config.api.md index c68097455a..98c1883ab1 100644 --- a/etc/firebase-admin.remote-config.api.md +++ b/etc/firebase-admin.remote-config.api.md @@ -39,7 +39,7 @@ export interface ListVersionsResult { } // @public -export interface NamedServerCondition { +export interface NamedCondition { name: string; } @@ -126,7 +126,7 @@ export interface ServerTemplate { // @public export interface ServerTemplateData { - conditions: NamedServerCondition[]; + conditions: NamedCondition[]; readonly etag: string; parameters: { [key: string]: RemoteConfigParameter; diff --git a/src/remote-config/index.ts b/src/remote-config/index.ts index 84eef33fe9..aa09a8e18a 100644 --- a/src/remote-config/index.ts +++ b/src/remote-config/index.ts @@ -29,7 +29,7 @@ export { InAppDefaultValue, ListVersionsOptions, ListVersionsResult, - NamedServerCondition, + NamedCondition, ParameterValueType, RemoteConfigCondition, RemoteConfigParameter, diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index 62bb6d22a3..2ad15eedd5 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -59,7 +59,7 @@ export interface RemoteConfigCondition { * A condition targets a specific group of users. A list of these conditions make up * part of a Remote Config template. */ -export interface NamedServerCondition { +export interface NamedCondition { /** * A non-empty and unique name of this condition. @@ -187,7 +187,7 @@ export interface ServerTemplateData { /** * A list of conditions in descending order by priority. */ - conditions: NamedServerCondition[]; + conditions: NamedCondition[]; /** * Map of parameter keys to their optional default values and optional conditional values. diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 208bd630da..afd9d68d3c 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -33,7 +33,7 @@ import { ServerConfig, ServerTemplateData, ServerTemplateOptions, - NamedServerCondition, + NamedCondition, } from './remote-config-api'; /** @@ -378,7 +378,7 @@ class ServerTemplateImpl implements ServerTemplate { class ServerTemplateDataImpl implements ServerTemplateData { public parameters: { [key: string]: RemoteConfigParameter }; public parameterGroups: { [key: string]: RemoteConfigParameterGroup }; - public conditions: NamedServerCondition[]; + public conditions: NamedCondition[]; public readonly etag: string; public version?: Version; diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 0f0821ef0c..a78febcc81 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -35,7 +35,7 @@ import { } from '../../../src/remote-config/remote-config-api-client-internal'; import { deepCopy } from '../../../src/utils/deep-copy'; import { - NamedServerCondition, ServerTemplate, ServerTemplateData + NamedCondition, ServerTemplate, ServerTemplateData } from '../../../src/remote-config/remote-config-api'; const expect = chai.expect; @@ -584,7 +584,7 @@ describe('RemoteConfig', () => { const c = template.cache.conditions.find((c) => c.name === 'ios'); expect(c).to.be.not.undefined; - const cond = c as NamedServerCondition; + const cond = c as NamedCondition; expect(cond.name).to.equal('ios'); const parsed = JSON.parse(JSON.stringify(template.cache)); @@ -793,7 +793,7 @@ describe('RemoteConfig', () => { const c = template.cache.conditions.find((c) => c.name === 'ios'); expect(c).to.be.not.undefined; - const cond = c as NamedServerCondition; + const cond = c as NamedCondition; expect(cond.name).to.equal('ios'); const parsed = JSON.parse(JSON.stringify(template.cache)); From 34fee9dabe318e57c4a1a573e6dc01fc00332d0a Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Wed, 20 Mar 2024 19:46:25 -0700 Subject: [PATCH 6/8] Fix incorrect use of Object.assign --- src/remote-config/remote-config.ts | 6 +- test/unit/remote-config/remote-config.spec.ts | 81 ++++++++++++++++++- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index afd9d68d3c..5a764f2f69 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -334,8 +334,10 @@ class ServerTemplateImpl implements ServerTemplate { evaluatedConfig[key] = this.parseRemoteConfigParameterValue(valueType, parameterDefaultValue); } - // Merges rendered config over default config. - const mergedConfig = Object.assign(this.defaultConfig, evaluatedConfig); + const mergedConfig = {}; + + // Merges rendered config and default config, prioritizing the former. + Object.assign(mergedConfig, this.defaultConfig, evaluatedConfig); // Enables config to be a convenient object, but with the ability to perform additional // functionality when a value is retrieved. diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index a78febcc81..90f7f58bbf 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -948,19 +948,92 @@ describe('RemoteConfig', () => { }); }); - it('overrides local default when value exists', () => { + it('uses local default when in-app default value specified after loading remote values', async () => { + // We had a bug caused by forgetting the first argument to + // Object.assign. This resulted in defaultConfig being overwritten + // by the remote values. So this test asserts we can use in-app + // default after loading remote values. + const template = remoteConfig.initServerTemplate({ + defaultConfig: { + dog_type: 'corgi' + } + }); + + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + + // response.parameters = { + // dog_type: { + // defaultValue: { + // useInAppDefault: true + // }, + // valueType: 'STRING' + // }, + // } + + // template.cache = response as ServerTemplateData; + + + + // expect(config.dog_type).to.equal(template.defaultConfig.dog_type); + + response.parameters = { + dog_type: { + defaultValue: { + value: 'pug' + }, + valueType: 'STRING' + }, + } + + template.cache = response as ServerTemplateData; + + let config = template.evaluate(); + + expect(config.dog_type).to.equal('pug'); + + response.parameters = { + dog_type: { + defaultValue: { + useInAppDefault: true + }, + valueType: 'STRING' + }, + } + + template.cache = response as ServerTemplateData; + + config = template.evaluate(); + + expect(config.dog_type).to.equal('corgi'); + }); + + it('overrides local default when remote value exists', () => { + const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); + response.parameters = { + dog_type_enabled: { + defaultValue: { + // Defines remote value + value: 'true' + }, + valueType: 'BOOLEAN' + }, + } + const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') - .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); + .resolves(response as ServerTemplateData); stubs.push(stub); + return remoteConfig.getServerTemplate({ defaultConfig: { + // Defines local default dog_type_enabled: false } }) .then((template: ServerTemplate) => { - const config = template.evaluate!(); - expect(config.dog_type_enabled).to.equal(template.defaultConfig.dog_type_enabled); + const config = template.evaluate(); + // Asserts remote value overrides local default. + expect(config.dog_type_enabled).to.be.true; }); }); }); From e87a8d5e2c93bb04827f0c73a52a4651069ef127 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Thu, 21 Mar 2024 11:46:36 -0700 Subject: [PATCH 7/8] Remove commented-out test code --- test/unit/remote-config/remote-config.spec.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 90f7f58bbf..39e7cf3b86 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -961,21 +961,6 @@ describe('RemoteConfig', () => { const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE); - // response.parameters = { - // dog_type: { - // defaultValue: { - // useInAppDefault: true - // }, - // valueType: 'STRING' - // }, - // } - - // template.cache = response as ServerTemplateData; - - - - // expect(config.dog_type).to.equal(template.defaultConfig.dog_type); - response.parameters = { dog_type: { defaultValue: { From 17f738a36b75daa12ab3d4f3f3c67c3088112cd6 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Thu, 21 Mar 2024 11:56:58 -0700 Subject: [PATCH 8/8] Update comment to match order of arguments --- src/remote-config/remote-config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 5a764f2f69..d603919a3a 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -336,7 +336,7 @@ class ServerTemplateImpl implements ServerTemplate { const mergedConfig = {}; - // Merges rendered config and default config, prioritizing the former. + // Merges default config and rendered config, prioritizing the latter. Object.assign(mergedConfig, this.defaultConfig, evaluatedConfig); // Enables config to be a convenient object, but with the ability to perform additional