Skip to content

Commit 7246526

Browse files
authored
Fixes incorrect use of Object.assign when backfilling SSRC config with defaults. (#2503)
In the logic where we backfill config with defaults, the first argument to Object.assign should be an object to assign to, but the code passed the object containing the defaults.
1 parent f89632a commit 7246526

File tree

2 files changed

+66
-6
lines changed

2 files changed

+66
-6
lines changed

src/remote-config/remote-config.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,10 @@ class ServerTemplateImpl implements ServerTemplate {
334334
evaluatedConfig[key] = this.parseRemoteConfigParameterValue(valueType, parameterDefaultValue);
335335
}
336336

337-
// Merges rendered config over default config.
338-
const mergedConfig = Object.assign(this.defaultConfig, evaluatedConfig);
337+
const mergedConfig = {};
338+
339+
// Merges default config and rendered config, prioritizing the latter.
340+
Object.assign(mergedConfig, this.defaultConfig, evaluatedConfig);
339341

340342
// Enables config to be a convenient object, but with the ability to perform additional
341343
// functionality when a value is retrieved.

test/unit/remote-config/remote-config.spec.ts

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -948,19 +948,77 @@ describe('RemoteConfig', () => {
948948
});
949949
});
950950

951-
it('overrides local default when value exists', () => {
951+
it('uses local default when in-app default value specified after loading remote values', async () => {
952+
// We had a bug caused by forgetting the first argument to
953+
// Object.assign. This resulted in defaultConfig being overwritten
954+
// by the remote values. So this test asserts we can use in-app
955+
// default after loading remote values.
956+
const template = remoteConfig.initServerTemplate({
957+
defaultConfig: {
958+
dog_type: 'corgi'
959+
}
960+
});
961+
962+
const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
963+
964+
response.parameters = {
965+
dog_type: {
966+
defaultValue: {
967+
value: 'pug'
968+
},
969+
valueType: 'STRING'
970+
},
971+
}
972+
973+
template.cache = response as ServerTemplateData;
974+
975+
let config = template.evaluate();
976+
977+
expect(config.dog_type).to.equal('pug');
978+
979+
response.parameters = {
980+
dog_type: {
981+
defaultValue: {
982+
useInAppDefault: true
983+
},
984+
valueType: 'STRING'
985+
},
986+
}
987+
988+
template.cache = response as ServerTemplateData;
989+
990+
config = template.evaluate();
991+
992+
expect(config.dog_type).to.equal('corgi');
993+
});
994+
995+
it('overrides local default when remote value exists', () => {
996+
const response = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
997+
response.parameters = {
998+
dog_type_enabled: {
999+
defaultValue: {
1000+
// Defines remote value
1001+
value: 'true'
1002+
},
1003+
valueType: 'BOOLEAN'
1004+
},
1005+
}
1006+
9521007
const stub = sinon
9531008
.stub(RemoteConfigApiClient.prototype, 'getServerTemplate')
954-
.resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as ServerTemplateData);
1009+
.resolves(response as ServerTemplateData);
9551010
stubs.push(stub);
1011+
9561012
return remoteConfig.getServerTemplate({
9571013
defaultConfig: {
1014+
// Defines local default
9581015
dog_type_enabled: false
9591016
}
9601017
})
9611018
.then((template: ServerTemplate) => {
962-
const config = template.evaluate!();
963-
expect(config.dog_type_enabled).to.equal(template.defaultConfig.dog_type_enabled);
1019+
const config = template.evaluate();
1020+
// Asserts remote value overrides local default.
1021+
expect(config.dog_type_enabled).to.be.true;
9641022
});
9651023
});
9661024
});

0 commit comments

Comments
 (0)