Skip to content

Commit

Permalink
fix: wait to create cookie storage based on user options in init() (#579
Browse files Browse the repository at this point in the history
)

* fix: wait to create cookie storage based on user options in init()

* chore: add test for disableCookies cookie creation

* chore: add test for disableCookies=false cookie creation

* chore: remove duplicated test
  • Loading branch information
justin-fiedler committed Feb 16, 2023
1 parent e4b5e83 commit a682c5b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/amplitude-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ var AmplitudeClient = function AmplitudeClient(instanceName) {
plan: { ...DEFAULT_OPTIONS.plan },
trackingOptions: { ...DEFAULT_OPTIONS.trackingOptions },
};
this.cookieStorage = new cookieStorage().getStorage();
this._q = []; // queue for proxied functions before script load
this._sending = false;
this._updateScheduled = false;
Expand Down Expand Up @@ -127,6 +126,7 @@ AmplitudeClient.prototype.init = function init(apiKey, opt_userId, opt_config, o

this._cookieName = Constants.COOKIE_PREFIX + '_' + this._storageSuffixV5;

this.cookieStorage = new cookieStorage().getStorage(this.options.disableCookies);
this.cookieStorage.options({
expirationDays: this.options.cookieExpiration,
domain: this.options.domain,
Expand Down
4 changes: 2 additions & 2 deletions src/cookiestorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ var cookieStorage = function () {
this.storage = null;
};

cookieStorage.prototype.getStorage = function () {
cookieStorage.prototype.getStorage = function (disableCookies) {
if (this.storage !== null) {
return this.storage;
}

if (baseCookie.areCookiesEnabled()) {
if (!disableCookies && baseCookie.areCookiesEnabled()) {
this.storage = Cookie;
} else {
// if cookies disabled, fallback to localstorage
Expand Down
53 changes: 51 additions & 2 deletions test/amplitude-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ import { mockCookie, restoreCookie, getCookie } from './mock-cookie';
import { AmplitudeServerZone } from '../src/server-zone.js';
import Request from '../src/xhr';

const deleteAllCookies = () =>
document.cookie.split(';').forEach(function (c) {
document.cookie = c.replace(/^ +/, '').replace(/=.*/, '=;expires=' + new Date().toUTCString() + ';path=/');
});

const getAllCookies = () =>
document.cookie
.split(';')
.map((c) => c.trimStart())
.filter((c) => !utils.isEmptyString(c));

// maintain for testing backwards compatability
describe('AmplitudeClient', function () {
var apiKey = '000000';
Expand Down Expand Up @@ -888,11 +899,11 @@ describe('AmplitudeClient', function () {
var onErrorSpy = sinon.spy();

var amplitude = new AmplitudeClient();
sinon.stub(amplitude.cookieStorage, 'options').throws();
sinon.stub(amplitude, '_refreshDynamicConfig').throws();
amplitude.init(apiKey, null, { onError: onErrorSpy });
assert.isTrue(onErrorSpy.calledOnce);

amplitude.cookieStorage.options.restore();
amplitude['_refreshDynamicConfig'].restore();
});

it('should set observer plan options', function () {
Expand Down Expand Up @@ -2728,6 +2739,44 @@ describe('AmplitudeClient', function () {
});
});

it('should not create any cookies if disabledCookies = true', function () {
deleteAllCookies();
clock.tick(20);

var cookieArray = getAllCookies();
assert.equal(cookieArray.length, 0);

var deviceId = 'test_device_id';
var amplitude2 = new AmplitudeClient();

amplitude2.init(apiKey, null, {
deviceId: deviceId,
disableCookies: true,
});

cookieArray = getAllCookies();
assert.equal(cookieArray.length, 0);
});

it('should create cookies if disabledCookies = false', function () {
deleteAllCookies();
clock.tick(20);

var cookieArray = getAllCookies();
assert.equal(cookieArray.length, 0);

var deviceId = 'test_device_id';
var amplitude2 = new AmplitudeClient();

amplitude2.init(apiKey, null, {
deviceId: deviceId,
disableCookies: false,
});

cookieArray = getAllCookies();
assert.equal(cookieArray.length, 1);
});

it('should validate event properties', function () {
var e = new Error('oops');
clock.tick(1);
Expand Down

0 comments on commit a682c5b

Please sign in to comment.