From abcdb94268390cf47693c68596498f8002e3cb64 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Fri, 22 Mar 2024 15:32:01 -0700 Subject: [PATCH 1/2] Remove defaultConfig from public API --- etc/firebase-admin.remote-config.api.md | 1 - src/remote-config/remote-config-api.ts | 5 -- src/remote-config/remote-config.ts | 2 +- test/unit/remote-config/remote-config.spec.ts | 83 ++++++++++++------- 4 files changed, 55 insertions(+), 36 deletions(-) diff --git a/etc/firebase-admin.remote-config.api.md b/etc/firebase-admin.remote-config.api.md index 6712175a60..1727c250a1 100644 --- a/etc/firebase-admin.remote-config.api.md +++ b/etc/firebase-admin.remote-config.api.md @@ -166,7 +166,6 @@ export type ServerConfig = { // @public export interface ServerTemplate { cache: ServerTemplateData; - defaultConfig: ServerConfig; evaluate(context?: EvaluationContext): ServerConfig; load(): Promise; } diff --git a/src/remote-config/remote-config-api.ts b/src/remote-config/remote-config-api.ts index e102094cde..e0f2af7738 100644 --- a/src/remote-config/remote-config-api.ts +++ b/src/remote-config/remote-config-api.ts @@ -382,11 +382,6 @@ export interface ServerTemplate { */ cache: ServerTemplateData; - /** - * A {@link ServerConfig} that contains default Config values. - */ - defaultConfig: ServerConfig; - /** * Evaluates the current template to produce a {@link ServerConfig}. */ diff --git a/src/remote-config/remote-config.ts b/src/remote-config/remote-config.ts index 1a720f9220..505cb7ac5a 100644 --- a/src/remote-config/remote-config.ts +++ b/src/remote-config/remote-config.ts @@ -296,7 +296,7 @@ class ServerTemplateImpl implements ServerTemplate { constructor( private readonly apiClient: RemoteConfigApiClient, private readonly conditionEvaluator: ConditionEvaluator, - public readonly defaultConfig: ServerConfig = {} + private readonly defaultConfig: ServerConfig = {} ) { } /** diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 81e34fa4b7..207a0f2779 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -610,20 +610,28 @@ describe('RemoteConfig', () => { }); it('should set defaultConfig when passed', () => { - const defaultConfig = { - holiday_promo_enabled: false, - holiday_promo_discount: 20, - }; + // Defines template with no parameters to demonstrate + // default config will be used instead, + const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData; + template.parameters = {}; const stub = sinon .stub(RemoteConfigApiClient.prototype, operationName) - .resolves(SERVER_REMOTE_CONFIG_RESPONSE as ServerTemplateData); + .resolves(template); stubs.push(stub); + const defaultConfig = { + holiday_promo_enabled: false, + holiday_promo_discount: 20, + }; + return remoteConfig.getServerTemplate({ defaultConfig }) .then((template) => { - expect(template.defaultConfig.holiday_promo_enabled).to.equal(false); - expect(template.defaultConfig.holiday_promo_discount).to.equal(20); + const config = template.evaluate(); + expect(config.holiday_promo_enabled).to.equal( + defaultConfig.holiday_promo_enabled); + expect(config.holiday_promo_discount).to.equal( + defaultConfig.holiday_promo_discount); }); }); }); @@ -1029,50 +1037,67 @@ describe('RemoteConfig', () => { }); it('uses local default if parameter not in template', () => { + // Defines a template without the parameter set by default config. + const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData; + template.parameters = {}; + const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') - .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); + .resolves(template); stubs.push(stub); - return remoteConfig.getServerTemplate({ - defaultConfig: { - dog_coat: 'blue merle', - } - }) + + const defaultConfig = { + dog_coat: 'blue merle', + }; + + return remoteConfig.getServerTemplate({ defaultConfig }) .then((template: ServerTemplate) => { - const config = template.evaluate!(); - expect(config.dog_coat).to.equal(template.defaultConfig.dog_coat); + const config = template.evaluate(); + expect(config.dog_coat).to.equal(defaultConfig.dog_coat); }); }); it('uses local default when parameter is in template but default value is undefined', () => { + const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData; + template.parameters = { + dog_no_remote_default_value: {} + }; + const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') - .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); + .resolves(template); stubs.push(stub); - return remoteConfig.getServerTemplate({ - defaultConfig: { - dog_no_remote_default_value: 'local default' - } - }) + + const defaultConfig = { + dog_no_remote_default_value: 'local default' + }; + + return remoteConfig.getServerTemplate({ defaultConfig }) .then((template: ServerTemplate) => { const config = template.evaluate!(); - expect(config.dog_no_remote_default_value).to.equal(template.defaultConfig.dog_no_remote_default_value); + expect(config.dog_no_remote_default_value).to.equal(defaultConfig.dog_no_remote_default_value); }); }); it('uses local default when in-app default value specified', () => { + const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData; + template.parameters = { + dog_no_remote_default_value: {} + }; + const stub = sinon .stub(RemoteConfigApiClient.prototype, 'getServerTemplate') - .resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData); + .resolves(template); stubs.push(stub); - return remoteConfig.getServerTemplate({ - defaultConfig: { - dog_use_inapp_default: '🐕' - } - }) + + const defaultConfig = { + dog_use_inapp_default: '🐕' + }; + + return remoteConfig.getServerTemplate({ defaultConfig }) .then((template: ServerTemplate) => { const config = template.evaluate!(); - expect(config.dog_use_inapp_default).to.equal(template.defaultConfig.dog_use_inapp_default); + expect(config.dog_use_inapp_default).to.equal(defaultConfig.dog_use_inapp_default); }); }); From 7227765ef7479cd36a0a334261ac0af240cc0159 Mon Sep 17 00:00:00 2001 From: Erik Eldridge Date: Fri, 22 Mar 2024 16:01:13 -0700 Subject: [PATCH 2/2] Remove extraneous comment --- test/unit/remote-config/remote-config.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/remote-config/remote-config.spec.ts b/test/unit/remote-config/remote-config.spec.ts index 207a0f2779..19a1aed3d2 100644 --- a/test/unit/remote-config/remote-config.spec.ts +++ b/test/unit/remote-config/remote-config.spec.ts @@ -1037,7 +1037,6 @@ describe('RemoteConfig', () => { }); it('uses local default if parameter not in template', () => { - // Defines a template without the parameter set by default config. const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as ServerTemplateData; template.parameters = {};