Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes incorrect use of Object.assign when backfilling SSRC config with defaults. #2503

Merged
merged 9 commits into from
Mar 21, 2024
6 changes: 4 additions & 2 deletions src/remote-config/remote-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
// functionality when a value is retrieved.
Expand Down
66 changes: 62 additions & 4 deletions test/unit/remote-config/remote-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -948,19 +948,77 @@ 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: {
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;
});
});
});
Expand Down