Skip to content

Commit 44aee84

Browse files
authored
Merge 76fd2ad into 17126e9
2 parents 17126e9 + 76fd2ad commit 44aee84

File tree

7 files changed

+90
-19
lines changed

7 files changed

+90
-19
lines changed

packages/remote-config/src/abt/experiment.ts

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,22 @@
1616
*/
1717
import { Storage } from '../storage/storage';
1818
import { FirebaseExperimentDescription } from '../public_types';
19+
import { Provider } from '@firebase/component';
20+
import { FirebaseAnalyticsInternalName } from '@firebase/analytics-interop-types';
21+
import { Logger } from '@firebase/logger';
22+
import { RemoteConfig } from '../remote_config';
23+
import { ERROR_FACTORY, ErrorCode } from '../errors';
1924

2025
export class Experiment {
21-
constructor(private readonly storage: Storage) {}
26+
private storage: Storage;
27+
private logger: Logger;
28+
private analyticsProvider: Provider<FirebaseAnalyticsInternalName>;
29+
30+
constructor(rc: RemoteConfig) {
31+
this.storage = rc._storage;
32+
this.logger = rc._logger;
33+
this.analyticsProvider = rc._analyticsProvider;
34+
}
2235

2336
async updateActiveExperiments(
2437
latestExperiments: FirebaseExperimentDescription[]
@@ -45,32 +58,47 @@ export class Experiment {
4558
currentActiveExperiments: Set<string>,
4659
experimentInfoMap: Map<string, FirebaseExperimentDescription>
4760
): void {
61+
const customProperty: Record<string, string | null> = {};
4862
for (const [experimentId, experimentInfo] of experimentInfoMap.entries()) {
4963
if (!currentActiveExperiments.has(experimentId)) {
50-
this.addExperimentToAnalytics(experimentId, experimentInfo.variantId);
64+
customProperty[experimentId] = experimentInfo.variantId;
5165
}
5266
}
67+
this.addExperimentToAnalytics(customProperty);
5368
}
5469

5570
private removeInactiveExperiments(
5671
currentActiveExperiments: Set<string>,
5772
experimentInfoMap: Map<string, FirebaseExperimentDescription>
5873
): void {
74+
const customProperty: Record<string, string | null> = {};
5975
for (const experimentId of currentActiveExperiments) {
6076
if (!experimentInfoMap.has(experimentId)) {
61-
this.removeExperimentFromAnalytics(experimentId);
77+
customProperty[experimentId] = null;
6278
}
6379
}
80+
this.addExperimentToAnalytics(customProperty);
6481
}
6582

6683
private addExperimentToAnalytics(
67-
_experimentId: string,
68-
_variantId: string
84+
customProperty: Record<string, string | null>
6985
): void {
70-
// TODO
71-
}
72-
73-
private removeExperimentFromAnalytics(_experimentId: string): void {
74-
// TODO
86+
if (Object.keys(customProperty).length === 0) {
87+
return;
88+
}
89+
try {
90+
const analytics = this.analyticsProvider.getImmediate({ optional: true });
91+
if (analytics) {
92+
analytics.setUserProperties({ properties: customProperty });
93+
} else {
94+
// TODO: Update warning message
95+
this.logger.warn(`Analytics is not imported correctly`);
96+
}
97+
} catch (error) {
98+
// TODO: Update error message
99+
throw ERROR_FACTORY.create(ErrorCode.ANALYTICS_UNAVAILABLE, {
100+
originalErrorMessage: (error as Error)?.message
101+
});
102+
}
75103
}
76104
}

packages/remote-config/src/api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export async function activate(remoteConfig: RemoteConfig): Promise<boolean> {
111111
// config.
112112
return false;
113113
}
114-
const experiment = new Experiment(rc._storage);
114+
const experiment = new Experiment(rc);
115115
const updateActiveExperiments = lastSuccessfulFetchResponse.experiments
116116
? experiment.updateActiveExperiments(
117117
lastSuccessfulFetchResponse.experiments

packages/remote-config/src/errors.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ export const enum ErrorCode {
3737
CONFIG_UPDATE_STREAM_ERROR = 'stream-error',
3838
CONFIG_UPDATE_UNAVAILABLE = 'realtime-unavailable',
3939
CONFIG_UPDATE_MESSAGE_INVALID = 'update-message-invalid',
40-
CONFIG_UPDATE_NOT_FETCHED = 'update-not-fetched'
40+
CONFIG_UPDATE_NOT_FETCHED = 'update-not-fetched',
41+
ANALYTICS_UNAVAILABLE = 'analytics-unavailable'
4142
}
4243

4344
const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
@@ -84,7 +85,9 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
8485
[ErrorCode.CONFIG_UPDATE_MESSAGE_INVALID]:
8586
'The stream invalidation message was unparsable: {$originalErrorMessage}',
8687
[ErrorCode.CONFIG_UPDATE_NOT_FETCHED]:
87-
'Unable to fetch the latest config: {$originalErrorMessage}'
88+
'Unable to fetch the latest config: {$originalErrorMessage}',
89+
[ErrorCode.ANALYTICS_UNAVAILABLE]:
90+
'Connection to firebase analytics failed: {$originalErrorMessage}'
8891
};
8992

9093
// Note this is effectively a type system binding a code to params. This approach overlaps with the
@@ -108,6 +111,7 @@ interface ErrorParams {
108111
[ErrorCode.CONFIG_UPDATE_UNAVAILABLE]: { originalErrorMessage: string };
109112
[ErrorCode.CONFIG_UPDATE_MESSAGE_INVALID]: { originalErrorMessage: string };
110113
[ErrorCode.CONFIG_UPDATE_NOT_FETCHED]: { originalErrorMessage: string };
114+
[ErrorCode.ANALYTICS_UNAVAILABLE]: { originalErrorMessage: string };
111115
}
112116

113117
export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(

packages/remote-config/src/register.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export function registerRemoteConfig(): void {
6666
const installations = container
6767
.getProvider('installations-internal')
6868
.getImmediate();
69+
const analyticsProvider = container.getProvider('analytics-internal');
6970

7071
// Normalizes optional inputs.
7172
const { projectId, apiKey, appId } = app.options;
@@ -127,7 +128,8 @@ export function registerRemoteConfig(): void {
127128
storageCache,
128129
storage,
129130
logger,
130-
realtimeHandler
131+
realtimeHandler,
132+
analyticsProvider
131133
);
132134

133135
// Starts warming cache.

packages/remote-config/src/remote_config.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import { StorageCache } from './storage/storage_cache';
2525
import { RemoteConfigFetchClient } from './client/remote_config_fetch_client';
2626
import { Storage } from './storage/storage';
2727
import { Logger } from '@firebase/logger';
28+
import { FirebaseAnalyticsInternalName } from '@firebase/analytics-interop-types';
29+
import { Provider } from '@firebase/component';
2830
import { RealtimeHandler } from './client/realtime_handler';
2931

3032
const DEFAULT_FETCH_TIMEOUT_MILLIS = 60 * 1000; // One minute
@@ -88,6 +90,10 @@ export class RemoteConfig implements RemoteConfigType {
8890
/**
8991
* @internal
9092
*/
91-
readonly _realtimeHandler: RealtimeHandler
93+
readonly _realtimeHandler: RealtimeHandler,
94+
/**
95+
* @internal
96+
*/
97+
readonly _analyticsProvider: Provider<FirebaseAnalyticsInternalName>
9298
) {}
9399
}

packages/remote-config/test/abt/experiment.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,32 @@ import * as sinon from 'sinon';
2020
import { Experiment } from '../../src/abt/experiment';
2121
import { FirebaseExperimentDescription } from '../../src/public_types';
2222
import { Storage } from '../../src/storage/storage';
23+
import { Provider } from '@firebase/component';
24+
import { FirebaseAnalyticsInternalName } from '@firebase/analytics-interop-types';
25+
import { Logger } from '@firebase/logger';
26+
import { RemoteConfig } from '../../src/remote_config';
2327

2428
describe('Experiment', () => {
2529
const storage = {} as Storage;
26-
const experiment = new Experiment(storage);
30+
const analyticsProvider = {} as Provider<FirebaseAnalyticsInternalName>;
31+
const logger = {} as Logger;
32+
const rc = {
33+
_storage: storage,
34+
_analyticsProvider: analyticsProvider,
35+
_logger: logger
36+
} as RemoteConfig;
37+
const experiment = new Experiment(rc);
2738

2839
describe('updateActiveExperiments', () => {
2940
beforeEach(() => {
3041
storage.getActiveExperiments = sinon.stub();
3142
storage.setActiveExperiments = sinon.stub();
43+
analyticsProvider.getImmediate = sinon.stub().returns({
44+
setUserProperties: sinon.stub()
45+
});
3246
});
3347

34-
it('adds mew experiments to storage', async () => {
48+
it('adds new experiments to storage', async () => {
3549
const latestExperiments: FirebaseExperimentDescription[] = [
3650
{
3751
experimentId: '_exp_3',
@@ -59,12 +73,16 @@ describe('Experiment', () => {
5973
storage.getActiveExperiments = sinon
6074
.stub()
6175
.returns(new Set(['_exp_1', '_exp_2']));
76+
const analytics = analyticsProvider.getImmediate();
6277

6378
await experiment.updateActiveExperiments(latestExperiments);
6479

6580
expect(storage.setActiveExperiments).to.have.been.calledWith(
6681
expectedStoredExperiments
6782
);
83+
expect(analytics.setUserProperties).to.have.been.calledWith({
84+
properties: { '_exp_3': '1' }
85+
});
6886
});
6987

7088
it('removes missing experiment in fetch response from storage', async () => {
@@ -81,12 +99,16 @@ describe('Experiment', () => {
8199
storage.getActiveExperiments = sinon
82100
.stub()
83101
.returns(new Set(['_exp_1', '_exp_2']));
102+
const analytics = analyticsProvider.getImmediate();
84103

85104
await experiment.updateActiveExperiments(latestExperiments);
86105

87106
expect(storage.setActiveExperiments).to.have.been.calledWith(
88107
expectedStoredExperiments
89108
);
109+
expect(analytics.setUserProperties).to.have.been.calledWith({
110+
properties: { '_exp_2': null }
111+
});
90112
});
91113
});
92114
});

packages/remote-config/test/remote_config.test.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ import {
4646
import * as api from '../src/api';
4747
import { fetchAndActivate } from '../src';
4848
import { restore } from 'sinon';
49-
import { RealtimeHandler } from '../src/client/realtime_handler';
5049
import { Experiment } from '../src/abt/experiment';
50+
import { Provider } from '@firebase/component';
51+
import { FirebaseAnalyticsInternalName } from '@firebase/analytics-interop-types';
52+
import { RealtimeHandler } from '../src/client/realtime_handler';
5153

5254
describe('RemoteConfig', () => {
5355
const ACTIVE_CONFIG = {
@@ -71,6 +73,7 @@ describe('RemoteConfig', () => {
7173
let logger: Logger;
7274
let realtimeHandler: RealtimeHandler;
7375
let rc: RemoteConfigType;
76+
let analyticsProvider: Provider<FirebaseAnalyticsInternalName>;
7477

7578
let getActiveConfigStub: sinon.SinonStub;
7679
let loggerDebugSpy: sinon.SinonSpy;
@@ -82,6 +85,7 @@ describe('RemoteConfig', () => {
8285
client = {} as RemoteConfigFetchClient;
8386
storageCache = {} as StorageCache;
8487
storage = {} as Storage;
88+
analyticsProvider = {} as Provider<FirebaseAnalyticsInternalName>;
8589
realtimeHandler = {} as RealtimeHandler;
8690
logger = new Logger('package-name');
8791
getActiveConfigStub = sinon.stub().returns(undefined);
@@ -94,7 +98,8 @@ describe('RemoteConfig', () => {
9498
storageCache,
9599
storage,
96100
logger,
97-
realtimeHandler
101+
realtimeHandler,
102+
analyticsProvider
98103
);
99104
});
100105

@@ -439,6 +444,10 @@ describe('RemoteConfig', () => {
439444
sandbox.restore();
440445
});
441446

447+
afterEach(() => {
448+
sandbox.restore();
449+
});
450+
442451
it('does not activate if last successful fetch response is undefined', async () => {
443452
getLastSuccessfulFetchResponseStub.returns(Promise.resolve());
444453
getActiveConfigEtagStub.returns(Promise.resolve(ETAG));

0 commit comments

Comments
 (0)