Skip to content

Commit f1c839d

Browse files
DT-1236DT-1236
authored
fix: prevent internal confirmation loop (#197)
* fix: prevent internal confirmation loop [AP-5710] the internal confirmation cids weren't included in the check to prevent re-adding them, causing an infinite loop
1 parent 424f5f3 commit f1c839d

File tree

3 files changed

+84
-14
lines changed

3 files changed

+84
-14
lines changed

‎src/context.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ export const CONTEXT_VALUE_ADDED = 'context.value.added';
99
export const CONTEXT_VALUE_CHANGED = 'context.value.changed';
1010
export const CONTEXT_DESTROYED = 'context.destroyed';
1111

12+
export const CONFIRMATIONS_KEY = 'experiments.confirmations';
13+
export const INTERNAL_CONFIRMATIONS_KEY = 'experiments.confirmationsInternal';
14+
1215
export const DEFAULT_QUEUE_LIMIT = 50;
1316

1417
/**

‎src/index.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import Context, {
22
CONTEXT_INITIALIZED,
33
CONTEXT_VALUE_ADDED,
44
CONTEXT_VALUE_CHANGED,
5-
CONTEXT_VALUE_REMOVED
5+
CONTEXT_VALUE_REMOVED,
6+
CONFIRMATIONS_KEY,
7+
INTERNAL_CONFIRMATIONS_KEY,
68
} from './context.js';
79

810
import Store, { EFFECTIVE_GENOME_UPDATED, REQUEST_FAILED } from './store.js';
@@ -261,8 +263,10 @@ function EvolvClient(opts) {
261263
return;
262264
}
263265

264-
const confirmations = context.get('experiments.confirmations') || [];
265-
const confirmedCids = confirmations.map(function(conf) {
266+
const existingConfirmations = context.get(CONFIRMATIONS_KEY) || [];
267+
const existingInternalConfirmations = context.get(INTERNAL_CONFIRMATIONS_KEY) || [];
268+
const combinedExistingConfirmations = existingConfirmations.concat(existingInternalConfirmations);
269+
const confirmedCids = combinedExistingConfirmations.map(function(conf) {
266270
return conf.cid;
267271
});
268272
const contaminations = context.get('experiments.contaminations') || [];
@@ -279,15 +283,17 @@ function EvolvClient(opts) {
279283
}
280284

281285
const timestamp = (new Date()).getTime();
282-
const contextConfirmations = confirmableAllocations.map(function(alloc) {
286+
const newConfirmations = confirmableAllocations.map(function(alloc) {
283287
return {
284288
cid: alloc.cid,
285289
timestamp: timestamp
286290
}
287291
});
288292

289-
const contextConfirmationsKey = store.isInternalUser() ? 'experiments.confirmationsInternal' : 'experiments.confirmations';
290-
context.set(contextConfirmationsKey, contextConfirmations.concat(confirmations));
293+
const isInternalUser = store.isInternalUser();
294+
const confirmationsKey = isInternalUser ? INTERNAL_CONFIRMATIONS_KEY : CONFIRMATIONS_KEY;
295+
const existingValues = isInternalUser ? existingInternalConfirmations : existingConfirmations;
296+
context.set(confirmationsKey, newConfirmations.concat(existingValues));
291297

292298
confirmableAllocations.forEach(function(alloc) {
293299
// Only confirm for non session based experiments -- session based use the analytics data

‎src/tests/index.test.js

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import _xhrMock from 'xhr-mock';
44
import { Crypto } from "@peculiar/webcrypto";
55
import Evolv from '../index.js';
66
import Store, { EFFECTIVE_GENOME_UPDATED } from '../store.js';
7-
import Context, { CONTEXT_INITIALIZED, CONTEXT_CHANGED } from '../context.js';
7+
import Context, { CONTEXT_INITIALIZED, CONTEXT_CHANGED, CONFIRMATIONS_KEY, INTERNAL_CONFIRMATIONS_KEY } from '../context.js';
88
import { waitFor, emit } from '../waitforit.js';
99
import base64 from '../ponyfills/base64.js';
1010
import { buildOptions } from '../build-options.js';
@@ -439,12 +439,42 @@ describe('Evolv client integration tests', () => {
439439

440440
await validateClient(evolv, options, uid);
441441

442-
const confirmations = evolv.context.get('experiments.confirmations');
442+
const confirmations = evolv.context.get(CONFIRMATIONS_KEY);
443443
expect(confirmations.length).to.equal(1);
444444
expect(confirmations[0].cid).to.equal('0cf8ffcedea2:0f39849197');
445445
});
446446

447447

448+
it('should only confirm once when evaluating a context for confirmations', async () => {
449+
xhrMock.get(`${endpoint}v${version}/${environment}/${uid}/configuration.json`, (_req, res) => {
450+
return res.status(200).body(JSON.stringify({
451+
_published: 1584475383.3865728,
452+
_client: {
453+
browser: 'chrome',
454+
platform: 'windows'
455+
},
456+
_experiments: experiments
457+
}));
458+
});
459+
460+
xhrMock.get(`${endpoint}v${version}/${environment}/${uid}/allocations`, (_req, res) => {
461+
return res.status(200).body(JSON.stringify(allocations));
462+
});
463+
464+
const options = {
465+
environment,
466+
endpoint,
467+
version
468+
};
469+
const evolv = new Evolv(options);
470+
const confirmedSpy = chai.spy();
471+
evolv.on(Evolv.CONFIRMED, confirmedSpy);
472+
473+
await validateClient(evolv, options, uid);
474+
475+
expect(confirmedSpy).to.have.been.called.exactly(1);
476+
});
477+
448478
it('should put in internal confirmations if the user is internal', async () => {
449479
xhrMock.get(`${endpoint}v${version}/${environment}/${uid}/configuration.json`, (_req, res) => {
450480
return res.status(200).body(JSON.stringify({
@@ -471,12 +501,43 @@ describe('Evolv client integration tests', () => {
471501

472502
await validateClient(evolv, options, uid);
473503

474-
expect(evolv.context.get('experiments.confirmations')).to.be.undefined;
475-
const internalConfirmations = evolv.context.get('experiments.confirmationsInternal');
504+
expect(evolv.context.get(CONFIRMATIONS_KEY)).to.be.undefined;
505+
const internalConfirmations = evolv.context.get(INTERNAL_CONFIRMATIONS_KEY);
476506
expect(internalConfirmations.length).to.equal(1);
477507
expect(internalConfirmations[0].cid).to.equal('0cf8ffcedea2:0f39849197');
478508
});
479509

510+
it('should only confirm once when evaluating a context for internal confirmations', async () => {
511+
xhrMock.get(`${endpoint}v${version}/${environment}/${uid}/configuration.json`, (_req, res) => {
512+
return res.status(200).body(JSON.stringify({
513+
_internal_user: true,
514+
_published: 1584475383.3865728,
515+
_client: {
516+
browser: 'chrome',
517+
platform: 'windows'
518+
},
519+
_experiments: experiments
520+
}));
521+
});
522+
523+
xhrMock.get(`${endpoint}v${version}/${environment}/${uid}/allocations`, (_req, res) => {
524+
return res.status(200).body(JSON.stringify(allocations));
525+
});
526+
527+
const options = {
528+
environment,
529+
endpoint,
530+
version
531+
};
532+
const evolv = new Evolv(options);
533+
const confirmedSpy = chai.spy();
534+
evolv.on(Evolv.CONFIRMED, confirmedSpy);
535+
536+
await validateClient(evolv, options, uid);
537+
538+
expect(confirmedSpy).to.have.been.called.exactly(1);
539+
});
540+
480541
it('should load variants and reevaluate context correctly with authentication', async () => {
481542
const id = 'mine';
482543
const secret = 'yep, lunch';
@@ -675,7 +736,7 @@ describe('Evolv client integration tests', () => {
675736
expect(messages[6]).to.be.a.message("context.value.added", "user_attributes.country", "usa");
676737
expect(messages[7]).to.be.a.message("context.value.changed", "keys.active", ["web.ab8numq2j.am94yhwo2"]);
677738
expect(messages[8]).to.be.a.message("context.value.changed", "variants.active", ["web.ab8numq2j.am94yhwo2:1486101989"]);
678-
expect(messages[9]).to.be.a.messageWithLength("context.value.added", "experiments.confirmations", 1);
739+
expect(messages[9]).to.be.a.messageWithLength("context.value.added", CONFIRMATIONS_KEY, 1);
679740
expect(messages[9].payload.value[0].cid).to.equal("0cf8ffcedea2:0f39849197")
680741
expect(messages[10]).to.be.a.messageWithLength("context.value.added", "events", 1);
681742
expect(messages[10].payload.value[0].type).to.equal("lunch-time")
@@ -823,7 +884,7 @@ describe('Evolv client integration tests', () => {
823884

824885
expect(messages[9]).to.be.a.messageWithLength("context.value.added", "confirmations", 1);
825886
expect(messages[9].payload.value[0].cid).to.equal("0cf8ffcedea2:0f39849197")
826-
expect(messages[10]).to.be.a.messageWithLength("context.value.added", "experiments.confirmations", 1);
887+
expect(messages[10]).to.be.a.messageWithLength("context.value.added", CONFIRMATIONS_KEY, 1);
827888
expect(messages[10].payload.value[0].cid).to.equal("0cf8ffcedea2:0f39849197")
828889
expect(messages[11]).to.be.a.messageWithLength("context.value.added", "events", 1);
829890
expect(messages[11].payload.value[0].type).to.equal("lunch-time")
@@ -1170,7 +1231,7 @@ describe('Evolv client unit tests', () => {
11701231

11711232
waitFor(context, Evolv.CONFIRMED, () => {
11721233
try {
1173-
const confirmations = context.get('experiments.confirmations');
1234+
const confirmations = context.get(CONFIRMATIONS_KEY);
11741235
expect(confirmations.length).to.be.equal(1);
11751236
expect(confirmations[0].cid).to.be.equal('5678');
11761237
done();
@@ -1196,7 +1257,7 @@ describe('Evolv client unit tests', () => {
11961257

11971258
waitFor(context, Evolv.CONFIRMED, () => {
11981259
try {
1199-
const confirmations = context.get('experiments.confirmations');
1260+
const confirmations = context.get(CONFIRMATIONS_KEY);
12001261
expect(confirmations.length).to.be.equal(1);
12011262
expect(confirmations[0].cid).to.be.equal('5678');
12021263
done();

0 commit comments

Comments
 (0)