Skip to content

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

New issue

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

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

Already on GitHub? Sign in to your account

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