Skip to content

Remove SSRC async initializer #2478

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.

Sign up for GitHub

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 1 commit into from
Mar 7, 2024
Merged

Remove SSRC async initializer #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Remove SSRC async initializer
  • Loading branch information
@erikeldridge
erikeldridge committed Feb 27, 2024
commit 6a06c790a5fc2e1870b5d6a3189c6ac1f37f744a
8 changes: 6 additions & 2 deletions etc/firebase-admin.remote-config.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export interface InAppDefaultValue {
useInAppDefault: boolean;
}

// @public
export interface InServerDefaultValue {
useInServerDefault: boolean;
}

// @public
export interface ListVersionsOptions {
endTime?: Date | string;
Expand All @@ -46,7 +51,6 @@ export class RemoteConfig {
// (undocumented)
readonly app: App;
createTemplateFromJSON(json: string): RemoteConfigTemplate;
getServerTemplate(options?: RemoteConfigServerTemplateOptions): Promise<RemoteConfigServerTemplate>;
getTemplate(): Promise<RemoteConfigTemplate>;
getTemplateAtVersion(versionNumber: number | string): Promise<RemoteConfigTemplate>;
initServerTemplate(options?: RemoteConfigServerTemplateOptions): RemoteConfigServerTemplate;
Expand Down Expand Up @@ -84,7 +88,7 @@ export interface RemoteConfigParameterGroup {
}

// @public
export type RemoteConfigParameterValue = ExplicitParameterValue | InAppDefaultValue;
export type RemoteConfigParameterValue = ExplicitParameterValue | InAppDefaultValue | InServerDefaultValue;

// @public
export interface RemoteConfigServerCondition {
Expand Down
10 changes: 0 additions & 10 deletions src/remote-config/remote-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,6 @@ export class RemoteConfig {
return new RemoteConfigTemplateImpl(template);
}

/**
* Instantiates {@link RemoteConfigServerTemplate} and then fetches and caches the latest
* template version of the project.
*/
public async getServerTemplate(options?: RemoteConfigServerTemplateOptions): Promise<RemoteConfigServerTemplate> {
const template = this.initServerTemplate(options);
await template.load();
return template;
}

Copy link
Member

@lahirumaramba lahirumaramba Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to update the internal API proposals to reflect this change?

Copy link
Contributor Author

@erikeldridge erikeldridge Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking this is a subset of the approved API, ie I'm not adding an unreviewed method, so it wasn't worth a full amendment, but I'll add a note to the doc, comment-in the approvers and see if they have any objections.

Copy link
Contributor Author

@erikeldridge erikeldridge Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amendment approved!

/**
* Synchronously instantiates {@link RemoteConfigServerTemplate}.
*/
Expand Down
170 changes: 51 additions & 119 deletions test/unit/remote-config/remote-config.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
} from '../../../src/remote-config/remote-config-api-client-internal';
import { deepCopy } from '../../../src/utils/deep-copy';
import {
RemoteConfigServerCondition, RemoteConfigServerTemplate, RemoteConfigServerTemplateData
RemoteConfigServerCondition, RemoteConfigServerTemplateData
} from '../../../src/remote-config/remote-config-api';

const expect = chai.expect;
Expand Down Expand Up @@ -542,82 +542,6 @@ describe('RemoteConfig', () => {
});
});

describe('getServerTemplate', () => {
const operationName = 'getServerTemplate';

it('should propagate API errors', () => {
const stub = sinon
.stub(RemoteConfigApiClient.prototype, operationName)
.rejects(INTERNAL_ERROR);
stubs.push(stub);

return remoteConfig.getServerTemplate().should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR);
});

it('should resolve a server template on success', () => {
const stub = sinon
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData);
stubs.push(stub);

return remoteConfig.getServerTemplate()
.then((template) => {
expect(template.cache.conditions.length).to.equal(1);
expect(template.cache.conditions[0].name).to.equal('ios');
expect(template.cache.conditions[0].expression).to.equal('device.os == \'ios\'');
expect(template.cache.etag).to.equal('etag-123456789012-5');

const version = template.cache.version!;
expect(version.versionNumber).to.equal('86');
expect(version.updateOrigin).to.equal('ADMIN_SDK_NODE');
expect(version.updateType).to.equal('INCREMENTAL_UPDATE');
expect(version.updateUser).to.deep.equal({
email: '[email protected]'
});
expect(version.description).to.equal('production version');
expect(version.updateTime).to.equal('Mon, 15 Jun 2020 16:45:03 GMT');

const key = 'holiday_promo_enabled';
const p1 = template.cache.parameters[key];
expect(p1.defaultValue).deep.equals({ value: 'true' });
expect(p1.conditionalValues).deep.equals({ ios: { useInServerDefault: true } });
expect(p1.description).equals('this is a promo');
expect(p1.valueType).equals('BOOLEAN');

const c = template.cache.conditions.find((c) => c.name === 'ios');
expect(c).to.be.not.undefined;
const cond = c as RemoteConfigServerCondition;
expect(cond.name).to.equal('ios');
expect(cond.expression).to.equal('device.os == \'ios\'');

const parsed = JSON.parse(JSON.stringify(template.cache));
const expectedTemplate = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE);
const expectedVersion = deepCopy(VERSION_INFO);
expectedVersion.updateTime = new Date(expectedVersion.updateTime).toUTCString();
expectedTemplate.version = expectedVersion;
expect(parsed).deep.equals(expectedTemplate);
});
});

it('should set defaultConfig when passed', () => {
const defaultConfig = {
holiday_promo_enabled: false,
holiday_promo_discount: 20,
};

const stub = sinon
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData);
stubs.push(stub);

return remoteConfig.getServerTemplate({ defaultConfig })
.then((template) => {
expect(template.defaultConfig.holiday_promo_enabled).to.equal(false);
expect(template.defaultConfig.holiday_promo_discount).to.equal(20);
});
});
});

describe('initServerTemplate', () => {
it('should set and instantiates template when passed', () => {
const template = deepCopy(SERVER_REMOTE_CONFIG_RESPONSE) as RemoteConfigServerTemplateData;
Expand Down Expand Up @@ -688,16 +612,17 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.rejects(INTERNAL_ERROR);
stubs.push(stub);

return remoteConfig.getServerTemplate().should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR);
const template = remoteConfig.initServerTemplate();
return template.load().should.eventually.be.rejected.and.deep.equal(INTERNAL_ERROR);
});

it('should reject when API response is invalid', () => {
const stub = sinon
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(undefined);
stubs.push(stub);
return remoteConfig.getServerTemplate().should.eventually.be.rejected.and.have.property(
const template = remoteConfig.initServerTemplate();
return template.load().should.eventually.be.rejected.and.have.property(
'message', 'Invalid Remote Config template: undefined');
});

Expand All @@ -708,7 +633,8 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate()
const template = remoteConfig.initServerTemplate();
return template.load()
.should.eventually.be.rejected.and.have.property(
'message', `Invalid Remote Config template: ${JSON.stringify(response)}`);
});
Expand All @@ -720,7 +646,8 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate()
const template = remoteConfig.initServerTemplate();
return template.load()
.should.eventually.be.rejected.and.have.property(
'message', 'Remote Config parameters must be a non-null object');
});
Expand All @@ -732,7 +659,8 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate()
const template = remoteConfig.initServerTemplate();
return template.load()
.should.eventually.be.rejected.and.have.property(
'message', 'Remote Config conditions must be an array');
});
Expand All @@ -744,8 +672,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate()
.then((template) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
// If parameters are not present in the response, we set it to an empty object.
expect(template.cache.parameters).deep.equals({});
});
Expand All @@ -758,8 +687,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate()
.then((template) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
// If conditions are not present in the response, we set it to an empty array.
expect(template.cache.conditions).deep.equals([]);
});
Expand All @@ -770,9 +700,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(SERVER_REMOTE_CONFIG_RESPONSE as RemoteConfigServerTemplateData);
stubs.push(stub);

return remoteConfig.getServerTemplate()
.then((template) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
expect(template.cache.conditions.length).to.equal(1);
expect(template.cache.conditions[0].name).to.equal('ios');
expect(template.cache.conditions[0].expression).to.equal('device.os == \'ios\'');
Expand Down Expand Up @@ -819,9 +749,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);

return remoteConfig.getServerTemplate()
.then((template) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
expect(template.cache.etag).to.equal('etag-123456789012-5');

const version = template.cache.version!;
Expand All @@ -845,9 +775,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);

return remoteConfig.getServerTemplate()
.then((template) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
expect(template.cache.etag).to.equal('etag-123456789012-5');

const version = template.cache.version!;
Expand All @@ -871,9 +801,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, operationName)
.resolves(response as RemoteConfigServerTemplateData);
stubs.push(stub);

return remoteConfig.getServerTemplate()
.then((template) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
expect(template.cache.etag).to.equal('etag-123456789012-5');

const version = template.cache.version!;
Expand All @@ -895,8 +825,9 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, 'getServerTemplate')
.resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate()
.then((template: RemoteConfigServerTemplate) => {
const template = remoteConfig.initServerTemplate();
return template.load()
.then(() => {
const config = template.evaluate!();
expect(config.dog_type).to.equal('corgi');
expect(config.dog_type_enabled).to.equal(true);
Expand All @@ -910,12 +841,13 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, 'getServerTemplate')
.resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate({
const template = remoteConfig.initServerTemplate({
defaultConfig: {
dog_coat: 'blue merle',
}
})
.then((template: RemoteConfigServerTemplate) => {
});
return template.load()
.then(() => {
const config = template.evaluate!();
expect(config.dog_coat).to.equal(template.defaultConfig.dog_coat);
});
Expand All @@ -926,47 +858,47 @@ describe('RemoteConfig', () => {
.stub(RemoteConfigApiClient.prototype, 'getServerTemplate')
.resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate({
const template = remoteConfig.initServerTemplate({
defaultConfig: {
dog_no_remote_default_value: 'local default'
}
})
.then((template: RemoteConfigServerTemplate) => {
const config = template.evaluate!();
expect(config.dog_no_remote_default_value).to.equal(template.defaultConfig.dog_no_remote_default_value);
});
return template.load().then(() => {
const config = template.evaluate!();
expect(config.dog_no_remote_default_value).to.equal(template.defaultConfig.dog_no_remote_default_value);
});
});

it('uses local default when in-app default value specified', () => {
const stub = sinon
.stub(RemoteConfigApiClient.prototype, 'getServerTemplate')
.resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate({
const template = remoteConfig.initServerTemplate({
defaultConfig: {
dog_use_inapp_default: '🐕'
}
})
.then((template: RemoteConfigServerTemplate) => {
const config = template.evaluate!();
expect(config.dog_use_inapp_default).to.equal(template.defaultConfig.dog_use_inapp_default);
});
return template.load().then(() => {
const config = template.evaluate!();
expect(config.dog_use_inapp_default).to.equal(template.defaultConfig.dog_use_inapp_default);
});
});

it('overrides local default when value exists', () => {
const stub = sinon
.stub(RemoteConfigApiClient.prototype, 'getServerTemplate')
.resolves(SERVER_REMOTE_CONFIG_RESPONSE_2 as RemoteConfigServerTemplateData);
stubs.push(stub);
return remoteConfig.getServerTemplate({
const template = remoteConfig.initServerTemplate({
defaultConfig: {
dog_type_enabled: false
}
})
.then((template: RemoteConfigServerTemplate) => {
const config = template.evaluate!();
expect(config.dog_type_enabled).to.equal(template.defaultConfig.dog_type_enabled);
});
return template.load().then(() => {
const config = template.evaluate!();
expect(config.dog_type_enabled).to.equal(template.defaultConfig.dog_type_enabled);
});
});
});
});
Expand Down