Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixes incorrect use of Object.assign when backfilling SSRC config w…
…ith 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.
  • Loading branch information
erikeldridge authored Mar 21, 2024
commit 724652675311162934e26891d1653a92d45814a5
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