mirror of
https://github.com/hanzoai/firebase-js-sdk.git
synced 2026-09-24 16:59:10 +00:00
fix(remoteconfig) Support ABT realtime when variant value matches default value of parameter
When an experiment is added whose variant value matches default value, the parameter key is not listed as updated. This becomes a problem when the client app calls activate only on config change as the experiment will not get started Before fix - https://screencast.googleplex.com/cast/NDgxNTYwNjU0NDA3MjcwNHxiOWY1MmIyNy1iMQ After fix - https://screencast.googleplex.com/cast/NDcxODUwODQ0MDAyNzEzNnwzODc2YTRlMC01MA
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@firebase/remote-config': patch
|
||||
---
|
||||
|
||||
Support ABT realtime when variant value matches default value of parameter
|
||||
@@ -21,7 +21,8 @@ import {
|
||||
ConfigUpdate,
|
||||
ConfigUpdateObserver,
|
||||
FetchResponse,
|
||||
FirebaseRemoteConfigObject
|
||||
FirebaseRemoteConfigObject,
|
||||
FirebaseExperimentDescription
|
||||
} from '../public_types';
|
||||
import { calculateBackoffMillis, FirebaseError } from '@firebase/util';
|
||||
import { ERROR_FACTORY, ErrorCode } from '../errors';
|
||||
@@ -43,6 +44,7 @@ const NO_FAILED_REALTIME_STREAMS = 0;
|
||||
const REALTIME_DISABLED_KEY = 'featureDisabled';
|
||||
const REALTIME_RETRY_INTERVAL = 'retryIntervalSeconds';
|
||||
const TEMPLATE_VERSION_KEY = 'latestTemplateVersionNumber';
|
||||
const ROLLOUT_ID_PREFIX = '_exp_rollout';
|
||||
|
||||
export class RealtimeHandler {
|
||||
constructor(
|
||||
@@ -326,16 +328,36 @@ export class RealtimeHandler {
|
||||
*/
|
||||
private getChangedParams(
|
||||
newConfig: FirebaseRemoteConfigObject,
|
||||
oldConfig: FirebaseRemoteConfigObject
|
||||
oldConfig: FirebaseRemoteConfigObject,
|
||||
newFetchResponse: FetchResponse,
|
||||
oldFetchResponse: FetchResponse | undefined
|
||||
): Set<string> {
|
||||
const changedKeys = new Set<string>();
|
||||
const newKeys = new Set(Object.keys(newConfig || {}));
|
||||
const oldKeys = new Set(Object.keys(oldConfig || {}));
|
||||
|
||||
const newExperiments = newFetchResponse.experiments || [];
|
||||
const oldExperiments = oldFetchResponse?.experiments || [];
|
||||
const newExperimentsMap = this.createExperimentsMap(newExperiments);
|
||||
const oldExperimentsMap = this.createExperimentsMap(oldExperiments);
|
||||
|
||||
for (const key of newKeys) {
|
||||
if (!oldKeys.has(key) || newConfig[key] !== oldConfig[key]) {
|
||||
changedKeys.add(key);
|
||||
}
|
||||
if (newExperimentsMap.has(key) !== oldExperimentsMap.has(key)) {
|
||||
changedKeys.add(key);
|
||||
continue;
|
||||
}
|
||||
const newExperiment = newExperimentsMap.get(key);
|
||||
const oldExperiment = oldExperimentsMap.get(key);
|
||||
if (
|
||||
newExperiment &&
|
||||
oldExperiment &&
|
||||
!this.areExperimentsEqual(newExperiment, oldExperiment)
|
||||
) {
|
||||
changedKeys.add(key);
|
||||
}
|
||||
}
|
||||
|
||||
for (const key of oldKeys) {
|
||||
@@ -347,6 +369,37 @@ export class RealtimeHandler {
|
||||
return changedKeys;
|
||||
}
|
||||
|
||||
private areExperimentsEqual(
|
||||
newExperiment: FirebaseExperimentDescription,
|
||||
oldExperiment: FirebaseExperimentDescription
|
||||
): boolean {
|
||||
return (
|
||||
newExperiment.experimentId === oldExperiment.experimentId &&
|
||||
newExperiment.variantId === oldExperiment.variantId &&
|
||||
newExperiment.timeToLiveMillis === oldExperiment.timeToLiveMillis &&
|
||||
newExperiment.triggerTimeoutMillis === oldExperiment.triggerTimeoutMillis
|
||||
);
|
||||
}
|
||||
|
||||
/** Creates a map where the key is the config key and the value is the experiment description. */
|
||||
private createExperimentsMap(
|
||||
experimentDescriptions: FirebaseExperimentDescription[]
|
||||
): Map<string, FirebaseExperimentDescription> {
|
||||
const experimentsMap = new Map<string, FirebaseExperimentDescription>();
|
||||
for (const experimentDescription of experimentDescriptions) {
|
||||
if (
|
||||
!experimentDescription.affectedParameterKeys ||
|
||||
experimentDescription.experimentId.startsWith(ROLLOUT_ID_PREFIX)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
for (const key of experimentDescription.affectedParameterKeys) {
|
||||
experimentsMap.set(key, experimentDescription);
|
||||
}
|
||||
}
|
||||
return experimentsMap;
|
||||
}
|
||||
|
||||
private async fetchLatestConfig(
|
||||
remainingAttempts: number,
|
||||
targetVersion: number
|
||||
@@ -369,6 +422,8 @@ export class RealtimeHandler {
|
||||
fetchAttempt: currentAttempt
|
||||
};
|
||||
|
||||
const lastFetchResponse =
|
||||
await this.storage.getLastSuccessfulFetchResponse();
|
||||
const fetchResponse: FetchResponse = await this.cachingClient.fetch(
|
||||
fetchRequest
|
||||
);
|
||||
@@ -397,7 +452,9 @@ export class RealtimeHandler {
|
||||
|
||||
const updatedKeys = this.getChangedParams(
|
||||
fetchResponse.config,
|
||||
activatedConfigs
|
||||
activatedConfigs,
|
||||
fetchResponse,
|
||||
lastFetchResponse
|
||||
);
|
||||
|
||||
if (updatedKeys.size === 0) {
|
||||
|
||||
@@ -536,6 +536,170 @@ describe('RealtimeHandler', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should identify changed keys from updated experiment descriptions', async () => {
|
||||
mockStorage.getActiveConfig.resolves({
|
||||
keyA: 'valueA',
|
||||
keyB: 'valueB',
|
||||
keyC: 'valueC'
|
||||
});
|
||||
mockStorage.getLastSuccessfulFetchResponse.resolves({
|
||||
status: 200,
|
||||
config: { keyA: 'valueA', keyB: 'valueB', keyC: 'valueC' },
|
||||
experiments: [
|
||||
{
|
||||
experimentId: 'exp1',
|
||||
variantId: '1',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyA', 'keyB']
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
mockCachingClient.fetch.resolves({
|
||||
config: { keyA: 'valueA', keyB: 'valueB', keyC: 'valueC' },
|
||||
templateVersion: 2,
|
||||
status: 200,
|
||||
eTag: 'e',
|
||||
experiments: [
|
||||
{
|
||||
experimentId: 'exp1',
|
||||
variantId: '1',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyB', 'keyC']
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
await (realtime as any).fetchLatestConfig(MAXIMUM_FETCH_ATTEMPTS, 2);
|
||||
|
||||
expect(executeAllListenerCallbacksSpy).to.have.been.calledOnce;
|
||||
const configUpdate = executeAllListenerCallbacksSpy.getCall(0).args[0];
|
||||
expect(configUpdate.getUpdatedKeys()).to.deep.equal(
|
||||
new Set(['keyA', 'keyC'])
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore experiments if descriptions have not changed', async () => {
|
||||
mockStorage.getActiveConfig.resolves({ keyA: 'valueA', keyB: 'valueB' });
|
||||
const experiments = [
|
||||
{
|
||||
experimentId: 'exp1',
|
||||
variantId: '1',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyA', 'keyB']
|
||||
}
|
||||
];
|
||||
mockStorage.getLastSuccessfulFetchResponse.resolves({
|
||||
status: 200,
|
||||
config: { keyA: 'valueA', keyB: 'valueB' },
|
||||
experiments
|
||||
});
|
||||
|
||||
mockCachingClient.fetch.resolves({
|
||||
config: { keyA: 'valueA', keyB: 'valueB' },
|
||||
templateVersion: 2,
|
||||
status: 200,
|
||||
eTag: 'e',
|
||||
experiments
|
||||
});
|
||||
|
||||
await (realtime as any).fetchLatestConfig(MAXIMUM_FETCH_ATTEMPTS, 2);
|
||||
|
||||
expect(executeAllListenerCallbacksSpy).not.to.have.been.called;
|
||||
});
|
||||
|
||||
it('should identify changed keys when an experiment variant ID is updated', async () => {
|
||||
mockStorage.getActiveConfig.resolves({ keyA: 'valueA', keyB: 'valueB' });
|
||||
mockStorage.getLastSuccessfulFetchResponse.resolves({
|
||||
status: 200,
|
||||
config: { keyA: 'valueA', keyB: 'valueB' },
|
||||
experiments: [
|
||||
{
|
||||
experimentId: 'exp1',
|
||||
variantId: '1',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyA', 'keyB']
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
mockCachingClient.fetch.resolves({
|
||||
config: { keyA: 'valueA', keyB: 'valueB' },
|
||||
templateVersion: 2,
|
||||
status: 200,
|
||||
eTag: 'e',
|
||||
experiments: [
|
||||
{
|
||||
experimentId: 'exp1',
|
||||
variantId: '2',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyA', 'keyB']
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
await (realtime as any).fetchLatestConfig(MAXIMUM_FETCH_ATTEMPTS, 2);
|
||||
|
||||
expect(executeAllListenerCallbacksSpy).to.have.been.calledOnce;
|
||||
const configUpdate = executeAllListenerCallbacksSpy.getCall(0).args[0];
|
||||
expect(configUpdate.getUpdatedKeys()).to.deep.equal(
|
||||
new Set(['keyA', 'keyB'])
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore experiment descriptions starting with _exp_rollout', async () => {
|
||||
mockStorage.getActiveConfig.resolves({
|
||||
keyA: 'valueA',
|
||||
keyB: 'valueB',
|
||||
keyC: 'valueC'
|
||||
});
|
||||
mockStorage.getLastSuccessfulFetchResponse.resolves({
|
||||
status: 200,
|
||||
config: { keyA: 'valueA', keyB: 'valueB', keyC: 'valueC' },
|
||||
experiments: [
|
||||
{
|
||||
experimentId: '_exp_rollout_1',
|
||||
variantId: '1',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyA', 'keyB']
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
mockCachingClient.fetch.resolves({
|
||||
config: { keyA: 'valueA', keyB: 'valueB', keyC: 'valueC' },
|
||||
templateVersion: 2,
|
||||
status: 200,
|
||||
eTag: 'e',
|
||||
experiments: [
|
||||
{
|
||||
experimentId: '_exp_rollout_1',
|
||||
variantId: '1',
|
||||
experimentStartTime: '',
|
||||
triggerTimeoutMillis: '',
|
||||
timeToLiveMillis: '',
|
||||
affectedParameterKeys: ['keyB', 'keyC']
|
||||
}
|
||||
]
|
||||
});
|
||||
|
||||
await (realtime as any).fetchLatestConfig(MAXIMUM_FETCH_ATTEMPTS, 2);
|
||||
|
||||
expect(executeAllListenerCallbacksSpy).not.to.have.been.called;
|
||||
});
|
||||
|
||||
it('should handle null activatedConfigs gracefully', async () => {
|
||||
mockCachingClient.fetch.resolves({
|
||||
config: { newKey: 'value' },
|
||||
|
||||
Reference in New Issue
Block a user