Skip to content

Commit

Permalink
fix(session-replay-browser): make joined config generator immutable (#…
Browse files Browse the repository at this point in the history
…890)

This also helps remove an unnecesary need for a nullish check.
  • Loading branch information
lewgordon-amplitude authored Oct 3, 2024
1 parent 7ef4e4d commit 2489b20
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 43 deletions.
35 changes: 14 additions & 21 deletions packages/session-replay-browser/src/config/joined-config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { RemoteConfigFetch, createRemoteConfigFetch } from '@amplitude/analytics-remote-config';
import { Logger } from '@amplitude/analytics-types';
import { getDebugConfig } from '../helpers';
import { SessionReplayOptions } from '../typings/session-replay';
import { SessionReplayLocalConfig } from './local-config';
import {
Expand All @@ -7,8 +9,6 @@ import {
SessionReplayJoinedConfig,
SessionReplayRemoteConfig,
} from './types';
import { getDebugConfig } from '../helpers';
import { Logger } from '@amplitude/analytics-types';

export const removeInvalidSelectorsFromPrivacyConfig = (privacyConfig: PrivacyConfig, loggerProvider: Logger) => {
// This allows us to not search the DOM.
Expand Down Expand Up @@ -38,18 +38,12 @@ export const removeInvalidSelectorsFromPrivacyConfig = (privacyConfig: PrivacyCo
return privacyConfig;
};
export class SessionReplayJoinedConfigGenerator {
localConfig: ISessionReplayLocalConfig;
remoteConfigFetch: RemoteConfigFetch<SessionReplayRemoteConfig> | undefined;
private readonly localConfig: ISessionReplayLocalConfig;
private readonly remoteConfigFetch: RemoteConfigFetch<SessionReplayRemoteConfig>;

constructor(apiKey: string, options: SessionReplayOptions) {
this.localConfig = new SessionReplayLocalConfig(apiKey, options);
}

async initialize() {
this.remoteConfigFetch = await createRemoteConfigFetch<SessionReplayRemoteConfig>({
localConfig: this.localConfig,
configKeys: ['sessionReplay'],
});
constructor(remoteConfigFetch: RemoteConfigFetch<SessionReplayRemoteConfig>, localConfig: ISessionReplayLocalConfig) {
this.localConfig = localConfig;
this.remoteConfigFetch = remoteConfigFetch;
}

async generateJoinedConfig(sessionId?: number): Promise<SessionReplayJoinedConfig> {
Expand All @@ -61,11 +55,6 @@ export class SessionReplayJoinedConfigGenerator {
config.captureEnabled = true;
let remoteConfig: SessionReplayRemoteConfig | undefined;
try {
if (!this.remoteConfigFetch) {
config.captureEnabled = false;
return config;
}

const samplingConfig = await this.remoteConfigFetch.getRemoteConfig(
'sessionReplay',
'sr_sampling_config',
Expand Down Expand Up @@ -197,7 +186,11 @@ export class SessionReplayJoinedConfigGenerator {
}

export const createSessionReplayJoinedConfigGenerator = async (apiKey: string, options: SessionReplayOptions) => {
const joinedConfigGenerator = new SessionReplayJoinedConfigGenerator(apiKey, options);
await joinedConfigGenerator.initialize();
return joinedConfigGenerator;
const localConfig = new SessionReplayLocalConfig(apiKey, options);
const remoteConfigFetch = await createRemoteConfigFetch<SessionReplayRemoteConfig>({
localConfig,
configKeys: ['sessionReplay'],
});

return new SessionReplayJoinedConfigGenerator(remoteConfigFetch, localConfig);
};
34 changes: 12 additions & 22 deletions packages/session-replay-browser/test/config/joined-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '../../src/config/joined-config';
import { SessionReplayLocalConfig } from '../../src/config/local-config';
import { PrivacyConfig, SessionReplayRemoteConfig } from '../../src/config/types';
import { createRemoteConfigFetch } from '@amplitude/analytics-remote-config';

type MockedLogger = jest.Mocked<Logger>;
const samplingConfig = {
Expand Down Expand Up @@ -141,17 +142,6 @@ describe('SessionReplayJoinedConfigGenerator', () => {
});
});
});
describe('without first initializing', () => {
test('should set captureEnabled to true', async () => {
joinedConfigGenerator = new SessionReplayJoinedConfigGenerator('static_key', mockOptions);
const config = await joinedConfigGenerator.generateJoinedConfig(123);
expect(config).toEqual({
...mockLocalConfig,
optOut: mockLocalConfig.optOut,
captureEnabled: false,
});
});
});
describe('with successful privacy config fetch', () => {
const privacySelectorTest = async (
remotePrivacyConfig?: PrivacyConfig,
Expand All @@ -162,10 +152,12 @@ describe('SessionReplayJoinedConfigGenerator', () => {
const result = key === 'sr_privacy_config' ? remotePrivacyConfig : undefined;
return Promise.resolve(result);
});
if (configGenerator.remoteConfigFetch) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
configGenerator.remoteConfigFetch.getRemoteConfig = getRemoteConfigMock;
}
const remoteConfigFetch = await createRemoteConfigFetch<SessionReplayRemoteConfig>({
localConfig: mockLocalConfig,
configKeys: ['sessionReplay'],
});
remoteConfigFetch.getRemoteConfig = getRemoteConfigMock;
new SessionReplayJoinedConfigGenerator(remoteConfigFetch, mockLocalConfig);
return configGenerator.generateJoinedConfig(123);
};

Expand Down Expand Up @@ -263,15 +255,13 @@ describe('SessionReplayJoinedConfigGenerator', () => {
});

test('should update to remote config when local privacy config is undefined', async () => {
const configGenerator = await createSessionReplayJoinedConfigGenerator('static_key', {
...mockOptions,
privacyConfig: undefined,
const mockRemoteConfigFetch = await createRemoteConfigFetch<SessionReplayRemoteConfig>({
localConfig: mockLocalConfig,
configKeys: ['sessionReplay'],
});
getRemoteConfigMockImplementation({ privacyConfig });
if (configGenerator.remoteConfigFetch) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
configGenerator.remoteConfigFetch.getRemoteConfig = getRemoteConfigMock;
}
mockRemoteConfigFetch.getRemoteConfig = getRemoteConfigMock;
const configGenerator = new SessionReplayJoinedConfigGenerator(mockRemoteConfigFetch, mockLocalConfig);
const config = await configGenerator.generateJoinedConfig(123);
expect(config).toEqual({
...mockLocalConfig,
Expand Down

0 comments on commit 2489b20

Please sign in to comment.