Skip to content

Commit

Permalink
Add a publicationId suffix to storage key when enabled by flag (#3575)
Browse files Browse the repository at this point in the history 
* Add a publicationId suffix to storage key when enabled by flag

* fix lint errors

* Allowlist storage keys without publication id due to legacy reason

* Rename variable names to baseKey or finalKey for clarity.

* Consolidate constants for local storage keys into StorageKeys.

* Rename arguments for key generation functions to baseKey for clarity.

* Fix broken unit tests

* Add more unit tests to cover scenarios using new key

* Make internal functions privates

* Clean up unused storage functions

* Add test to cover pruneTimestamps function

* Add more test cases to cover StorageKeysWithoutPublicationIdSuffix

* Fix typo and add comments
  • Loading branch information
@oyj9109
oyj9109 authored Oct 16, 2024
1 parent 98cd00d commit 8a0d7a4
Show file tree
Hide file tree
Showing 21 changed files with 928 additions and 419 deletions.
4 changes: 2 additions & 2 deletions src/components/activities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from '../proto/api_messages';
import {INTERNAL_RUNTIME_VERSION} from '../constants';

import {Constants} from '../utils/constants';
import {StorageKeys} from '../utils/constants';
import {addQueryParam} from '../utils/url';

import {
Expand Down Expand Up @@ -273,7 +273,7 @@ export class ActivityPorts {

const swgUserToken = await this.deps_
.storage()
.get(Constants.USER_TOKEN, /* useLocalStorage= */ true);
.get(StorageKeys.USER_TOKEN, /* useLocalStorage= */ true);

const queryParams = new URL(url).searchParams;
if (swgUserToken && !queryParams.has('sut')) {
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/analytics-service-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import {
import {AnalyticsService} from './analytics-service';
import {ClientEventManager} from './client-event-manager';
import {ConfiguredRuntime} from './runtime';
import {Constants} from '../utils/constants';
import {PageConfig} from '../model/page-config';
import {StorageKeys} from '../utils/constants';
import {feUrl} from './services';
import {getStyle} from '../utils/style';
import {setExperimentsStringForTesting} from './experiments';
Expand Down Expand Up @@ -195,7 +195,7 @@ describes.realWin('AnalyticsService', (env) => {
analyticsService.setReadyForLogging();
storageMock
.expects('get')
.withExactArgs(Constants.USER_TOKEN)
.withExactArgs(StorageKeys.USER_TOKEN)
.resolves('swgUserToken')
.once();
analyticsService.start();
Expand Down
46 changes: 23 additions & 23 deletions src/runtime/audience-action-flow-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {AudienceActionIframeFlow} from './audience-action-flow';
import {AutoPromptType} from '../api/basic-subscriptions';
import {ClientEventManager} from './client-event-manager';
import {ConfiguredRuntime} from './runtime';
import {Constants} from '../utils/constants';
import {Constants, StorageKeys} from '../utils/constants';
import {MockActivityPort} from '../../test/mock-activity-port';
import {PageConfig} from '../model/page-config';
import {ProductType} from '../api/subscriptions';
Expand Down Expand Up @@ -325,11 +325,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

let toast;
Expand Down Expand Up @@ -370,11 +370,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

let toast;
Expand Down Expand Up @@ -415,11 +415,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

let toast;
Expand Down Expand Up @@ -458,11 +458,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

let toast;
Expand Down Expand Up @@ -504,11 +504,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

let toast;
Expand Down Expand Up @@ -548,11 +548,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

let toast;
Expand Down Expand Up @@ -594,11 +594,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);
const toastOpenStub = sandbox.stub(Toast.prototype, 'open');

Expand Down Expand Up @@ -627,11 +627,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);
const toastOpenStub = sandbox.stub(Toast.prototype, 'open');

Expand Down Expand Up @@ -661,11 +661,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);
const toastOpenStub = sandbox.stub(Toast.prototype, 'open');

Expand Down Expand Up @@ -701,11 +701,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);
const toastOpenStub = sandbox.stub(Toast.prototype, 'open');

Expand Down Expand Up @@ -741,11 +741,11 @@ describes.realWin('AudienceActionIframeFlow', (env) => {
entitlementsManagerMock.expects('getEntitlements').once();
storageMock
.expects('set')
.withExactArgs(Constants.USER_TOKEN, 'fake user token', true)
.withExactArgs(StorageKeys.USER_TOKEN, 'fake user token', true)
.exactly(1);
storageMock
.expects('set')
.withExactArgs(Constants.READ_TIME, EXPECTED_TIME_STRING, false)
.withExactArgs(StorageKeys.READ_TIME, EXPECTED_TIME_STRING, false)
.exactly(1);

const toastOpenStub = sandbox.stub(Toast.prototype, 'open');
Expand Down
13 changes: 8 additions & 5 deletions src/runtime/audience-action-flow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ import {
} from '../proto/api_messages';
import {AutoPromptType} from '../api/basic-subscriptions';
import {ClientConfigManager} from './client-config-manager';
import {Constants, StorageKeys} from '../utils/constants';
import {
Constants,
StorageKeys,
StorageKeysWithoutPublicationIdSuffix,
} from '../utils/constants';
import {Deps} from './deps';
import {DialogManager} from '../components/dialog-manager';
import {EntitlementsManager} from './entitlements-manager';
Expand Down Expand Up @@ -200,7 +204,7 @@ export class AudienceActionIframeFlow implements AudienceActionFlow {
this.entitlementsManager_.clear();
const userToken = response.getSwgUserToken();
if (userToken) {
this.deps_.storage().set(Constants.USER_TOKEN, userToken, true);
this.deps_.storage().set(StorageKeys.USER_TOKEN, userToken, true);
}
if (this.isOptIn(this.params_.action) && onResult) {
onResult({
Expand All @@ -226,7 +230,7 @@ export class AudienceActionIframeFlow implements AudienceActionFlow {
const now = Date.now().toString();
this.deps_
.storage()
.set(Constants.READ_TIME, now, /*useLocalStorage=*/ false);
.set(StorageKeys.READ_TIME, now, /*useLocalStorage=*/ false);
this.entitlementsManager_.getEntitlements();
}

Expand Down Expand Up @@ -346,7 +350,6 @@ export class AudienceActionIframeFlow implements AudienceActionFlow {
AnalyticsEvent.EVENT_SURVEY_DATA_TRANSFER_FAILED,
/* isFromUserAction */ false
);
this.storage_.storeEvent(StorageKeys.SURVEY_DATA_TRANSFER_FAILED);
}
const surveyDataTransferResponse = new SurveyDataTransferResponse();
const isPpsEligible = request.getStorePpsInLocalStorage();
Expand Down Expand Up @@ -389,7 +392,7 @@ export class AudienceActionIframeFlow implements AudienceActionFlow {
private async storePpsValuesFromSurveyAnswers(
request: SurveyDataTransferRequest
): Promise<void> {
const iabAudienceKey = StorageKeys.PPS_TAXONOMIES;
const iabAudienceKey = StorageKeysWithoutPublicationIdSuffix.PPS_TAXONOMIES;
// PPS value field is optional and category may not be populated
// in accordance to IAB taxonomies.
const ppsConfigParams = request
Expand Down
54 changes: 38 additions & 16 deletions src/runtime/audience-action-local-flow-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {AudienceActionLocalFlow} from './audience-action-local-flow';
import {AutoPromptType} from '../api/basic-subscriptions';
import {ConfiguredRuntime} from './runtime';
import {PageConfig} from '../model/page-config';
import {StorageKeys} from '../utils/constants';
import {Toast} from '../ui/toast';
import {tick} from '../../test/tick';

Expand Down Expand Up @@ -62,6 +63,7 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
let runtime;
let eventManager;
let entitlementsManager;
let storageMock;
let DEFAULT_PARAMS;

beforeEach(() => {
Expand All @@ -75,9 +77,6 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
/* config= */ undefined,
/* clientOptions= */ {}
);
env.win.localStorage.getItem = () => 'abc';
env.win.localStorage.setItem = sandbox.spy();
env.win.sessionStorage.setItem = sandbox.spy();
eventManager = {
logSwgEvent: sandbox.spy(),
};
Expand All @@ -89,6 +88,7 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
parseArticleExperimentConfigFlags: (_) => articleExperimentFlags,
};
runtime.entitlementsManager = () => entitlementsManager;
storageMock = sandbox.mock(runtime.storage());

DEFAULT_PARAMS = {
action: 'TYPE_REWARDED_AD',
Expand All @@ -102,6 +102,10 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
};
});

afterEach(() => {
storageMock.verify();
});

describe('start', () => {
it('invalid action renders with error view prompt when not closable', async () => {
const params = {
Expand Down Expand Up @@ -695,6 +699,14 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
});

it('renders thanks with rewardedSlotGranted', async () => {
storageMock
.expects('get')
.withArgs(StorageKeys.USER_TOKEN)
.resolves('abc')
.exactly(1);
storageMock.expects('set').withArgs(StorageKeys.USER_TOKEN).exactly(1);
storageMock.expects('set').withArgs(StorageKeys.READ_TIME).exactly(1);

await renderAndAssertRewardedAd(DEFAULT_PARAMS, DEFAULT_CONFIG);

const wrapper = await callReadyAndReturnWrapper();
Expand All @@ -720,16 +732,16 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
);
expect(entitlementsManager.clear).to.be.called;
expect(entitlementsManager.getEntitlements).to.be.called;
expect(env.win.localStorage.setItem).to.be.calledWith(
'subscribe.google.com:USER_TOKEN',
'xyz'
);
expect(env.win.sessionStorage.setItem).to.be.calledWith(
'subscribe.google.com:READ_TIME'
);
});

it('does not update entitlements when complete fails', async () => {
storageMock
.expects('get')
.withArgs(StorageKeys.USER_TOKEN)
.resolves('abc')
.exactly(1);
storageMock.expects('set').never();

await renderAndAssertRewardedAd(
DEFAULT_PARAMS,
DEFAULT_CONFIG,
Expand All @@ -754,11 +766,17 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
);
expect(entitlementsManager.clear).to.not.be.called;
expect(entitlementsManager.getEntitlements).to.not.be.called;
expect(env.win.localStorage.setItem).to.not.be.called;
expect(env.win.sessionStorage.setItem).to.not.be.called;
});

it('does not update token if non returned', async () => {
storageMock
.expects('get')
.withArgs(StorageKeys.USER_TOKEN)
.resolves('abc')
.exactly(1);
storageMock.expects('set').withArgs(StorageKeys.USER_TOKEN).never();
storageMock.expects('set').withArgs(StorageKeys.READ_TIME).exactly(1);

await renderAndAssertRewardedAd(
DEFAULT_PARAMS,
DEFAULT_CONFIG,
Expand All @@ -782,10 +800,6 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
);
expect(entitlementsManager.clear).to.be.called;
expect(entitlementsManager.getEntitlements).to.be.called;
expect(env.win.localStorage.setItem).to.not.be.called;
expect(env.win.sessionStorage.setItem).to.be.calledWith(
'subscribe.google.com:READ_TIME'
);
});

it('closes on thanks', async () => {
Expand Down Expand Up @@ -1235,6 +1249,14 @@ describes.realWin('AudienceActionLocalFlow', (env) => {
});

it('submit event triggers completion event', async () => {
storageMock
.expects('get')
.withArgs(StorageKeys.USER_TOKEN)
.resolves('abc')
.atLeast(0);
storageMock.expects('set').withArgs(StorageKeys.USER_TOKEN).exactly(1);
storageMock.expects('set').withArgs(StorageKeys.READ_TIME).exactly(1);

const state = await renderNewsletterPrompt(
NEWSLETTER_PARAMS,
NEWSLETTER_CONFIG
Expand Down
Loading

0 comments on commit 8a0d7a4

Please sign in to comment.