From d324204253f4b94dec7011acaf32626acd45846f Mon Sep 17 00:00:00 2001 From: Jon Snyder Date: Mon, 19 Sep 2022 09:36:27 -0600 Subject: [PATCH] Re-work dependencies to hande targetMigrationEnabled flag --- .../Personalization/createComponent.js | 5 +- .../Personalization/createFetchDataHandler.js | 9 ++- ...Enabled.js => createSetTargetMigration.js} | 10 +++- src/components/Personalization/index.js | 11 ++-- src/core/createCookieTransfer.js | 22 +++---- src/core/index.js | 9 ++- .../injectShouldTransferCookie.js} | 15 ++++- src/utils/index.js | 1 - .../Personalization/createComponent.spec.js | 11 ++-- ...ec.js => createSetTargetMigration.spec.js} | 43 +++++++------- .../specs/core/createCookieTransfer.spec.js | 8 ++- .../core/injectShouldTransferCookie.spec.js | 58 +++++++++++++++++++ .../specs/utils/isLegacyCookieName.spec.js | 38 ------------ 13 files changed, 139 insertions(+), 101 deletions(-) rename src/components/Personalization/{migration/setMigrationEnabled.js => createSetTargetMigration.js} (75%) rename src/{utils/isLegacyCookieName.js => core/injectShouldTransferCookie.js} (54%) rename test/unit/specs/components/Personalization/{migration/setMigrationEnabled.spec.js => createSetTargetMigration.spec.js} (50%) create mode 100644 test/unit/specs/core/injectShouldTransferCookie.spec.js delete mode 100644 test/unit/specs/utils/isLegacyCookieName.spec.js diff --git a/src/components/Personalization/createComponent.js b/src/components/Personalization/createComponent.js index cf2a6df12..3348a136a 100644 --- a/src/components/Personalization/createComponent.js +++ b/src/components/Personalization/createComponent.js @@ -16,7 +16,6 @@ import { AUTHORING_ENABLED } from "./constants/loggerMessage"; import validateApplyPropositionsOptions from "./validateApplyPropositionsOptions"; export default ({ - config, logger, fetchDataHandler, viewChangeHandler, @@ -26,12 +25,12 @@ export default ({ viewCache, showContainers, applyPropositions, - setMigrationEnabled + setTargetMigration }) => { return { lifecycle: { onBeforeRequest({ request }) { - setMigrationEnabled(config, request); + setTargetMigration(request); return Promise.resolve(); }, onBeforeEvent({ diff --git a/src/components/Personalization/createFetchDataHandler.js b/src/components/Personalization/createFetchDataHandler.js index 06e007335..df45aafc6 100644 --- a/src/components/Personalization/createFetchDataHandler.js +++ b/src/components/Personalization/createFetchDataHandler.js @@ -10,10 +10,13 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -export default ({ config, responseHandler, hideContainers, mergeQuery }) => { +export default ({ + prehidingStyle, + responseHandler, + hideContainers, + mergeQuery +}) => { return ({ decisionsDeferred, personalizationDetails, event, onResponse }) => { - const { prehidingStyle } = config; - if (personalizationDetails.isRenderDecisions()) { hideContainers(prehidingStyle); } diff --git a/src/components/Personalization/migration/setMigrationEnabled.js b/src/components/Personalization/createSetTargetMigration.js similarity index 75% rename from src/components/Personalization/migration/setMigrationEnabled.js rename to src/components/Personalization/createSetTargetMigration.js index a2b5ff02b..5471a30b4 100644 --- a/src/components/Personalization/migration/setMigrationEnabled.js +++ b/src/components/Personalization/createSetTargetMigration.js @@ -10,9 +10,13 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -export default (config, request) => { - const { targetMigrationEnabled } = config; +import { noop } from "../../utils"; + +export default ({ targetMigrationEnabled }) => { if (targetMigrationEnabled) { - request.getPayload().mergeMeta({ target: { migration: true } }); + return request => { + request.getPayload().mergeMeta({ target: { migration: true } }); + }; } + return noop; }; diff --git a/src/components/Personalization/index.js b/src/components/Personalization/index.js index d63523331..5021ba8e3 100644 --- a/src/components/Personalization/index.js +++ b/src/components/Personalization/index.js @@ -30,9 +30,10 @@ import createRedirectHandler from "./createRedirectHandler"; import createAutorenderingHandler from "./createAutoRenderingHandler"; import createNonRenderingHandler from "./createNonRenderingHandler"; import createApplyPropositions from "./createApplyPropositions"; -import setMigrationEnabled from "./migration/setMigrationEnabled"; +import createSetTargetMigration from "./createSetTargetMigration"; const createPersonalization = ({ config, logger, eventManager }) => { + const { targetMigrationEnabled, prehidingStyle } = config; const collect = createCollect({ eventManager, mergeDecisionsMeta }); const { @@ -72,7 +73,7 @@ const createPersonalization = ({ config, logger, eventManager }) => { showContainers }); const fetchDataHandler = createFetchDataHandler({ - config, + prehidingStyle, responseHandler, hideContainers, mergeQuery @@ -89,8 +90,10 @@ const createPersonalization = ({ config, logger, eventManager }) => { executeDecisions, viewCache }); + const setTargetMigration = createSetTargetMigration({ + targetMigrationEnabled + }); return createComponent({ - config, logger, fetchDataHandler, viewChangeHandler, @@ -100,7 +103,7 @@ const createPersonalization = ({ config, logger, eventManager }) => { viewCache, showContainers, applyPropositions, - setMigrationEnabled + setTargetMigration }); }; diff --git a/src/core/createCookieTransfer.js b/src/core/createCookieTransfer.js index e86228ec0..49ea5ce66 100644 --- a/src/core/createCookieTransfer.js +++ b/src/core/createCookieTransfer.js @@ -10,12 +10,16 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import { endsWith, isLegacyCookieName, isNamespacedCookieName } from "../utils"; +import { endsWith } from "../utils"; const STATE_STORE_HANDLE_TYPE = "state:store"; -export default ({ cookieJar, config, apexDomain, dateProvider }) => { - const { orgId } = config; +export default ({ + cookieJar, + shouldTransferCookie, + apexDomain, + dateProvider +}) => { return { /** * When sending to a third-party endpoint, the endpoint won't be able to @@ -37,17 +41,7 @@ export default ({ cookieJar, config, apexDomain, dateProvider }) => { const cookies = cookieJar.get(); const entries = Object.keys(cookies) - .filter(name => { - // We have a contract with the server that we will pass - // all cookies whose names are namespaced according to the - // logic in isNamespacedCookieName as well as any legacy - // cookie names (so that the server can handle migrating - // identities on websites previously using Visitor.js) - return ( - isNamespacedCookieName(orgId, name) || - isLegacyCookieName(name, config) - ); - }) + .filter(shouldTransferCookie) .map(qualifyingCookieName => { return { key: qualifyingCookieName, diff --git a/src/core/index.js b/src/core/index.js index 5f9c2628a..ddd86172f 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -42,6 +42,7 @@ import injectSendBeaconRequest from "./network/requestMethods/injectSendBeaconRe import createLogger from "./createLogger"; import createEventManager from "./createEventManager"; import createCookieTransfer from "./createCookieTransfer"; +import injectShouldTransferCookie from "./injectShouldTransferCookie"; import { createDataCollectionRequest, createDataCollectionRequestPayload @@ -90,10 +91,14 @@ export const createExecuteCommand = ({ logger, setDebugEnabled }); - const { orgId } = config; + const { orgId, targetMigrationEnabled } = config; + const shouldTransferCookie = injectShouldTransferCookie({ + orgId, + targetMigrationEnabled + }); const cookieTransfer = createCookieTransfer({ cookieJar: loggingCookieJar, - config, + shouldTransferCookie, apexDomain, dateProvider: () => new Date() }); diff --git a/src/utils/isLegacyCookieName.js b/src/core/injectShouldTransferCookie.js similarity index 54% rename from src/utils/isLegacyCookieName.js rename to src/core/injectShouldTransferCookie.js index 19bf9859c..bdfa6c03e 100644 --- a/src/utils/isLegacyCookieName.js +++ b/src/core/injectShouldTransferCookie.js @@ -10,9 +10,18 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ +import { isNamespacedCookieName } from "../utils"; import { AT_QA_MODE, MBOX } from "../constants/legacyCookies"; -export default (name, config) => { - const { targetMigrationEnabled } = config; - return name === AT_QA_MODE || (name === MBOX && targetMigrationEnabled); +export default ({ orgId, targetMigrationEnabled }) => name => { + // We have a contract with the server that we will pass + // all cookies whose names are namespaced according to the + // logic in isNamespacedCookieName as well as any legacy + // cookie names (so that the server can handle migrating + // identities on websites previously using Visitor.js) + return ( + isNamespacedCookieName(orgId, name) || + name === AT_QA_MODE || + (targetMigrationEnabled && name === MBOX) + ); }; diff --git a/src/utils/index.js b/src/utils/index.js index d01d91443..fe850a45b 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -41,7 +41,6 @@ export { default as isEmptyObject } from "./isEmptyObject"; export { default as isFunction } from "./isFunction"; export { default as isInteger } from "./isInteger"; export { default as isNamespacedCookieName } from "./isNamespacedCookieName"; -export { default as isLegacyCookieName } from "./isLegacyCookieName"; export { default as isNonEmptyArray } from "./isNonEmptyArray"; export { default as isNonEmptyString } from "./isNonEmptyString"; export { default as isNil } from "./isNil"; diff --git a/test/unit/specs/components/Personalization/createComponent.spec.js b/test/unit/specs/components/Personalization/createComponent.spec.js index b4977626b..9f1147351 100644 --- a/test/unit/specs/components/Personalization/createComponent.spec.js +++ b/test/unit/specs/components/Personalization/createComponent.spec.js @@ -23,11 +23,10 @@ describe("Personalization", () => { let mergeQuery; let event; let personalizationComponent; - let setMigrationEnabled; + let setTargetMigration; const build = () => { personalizationComponent = createComponent({ - config: {}, logger, fetchDataHandler, viewChangeHandler, @@ -36,7 +35,7 @@ describe("Personalization", () => { mergeQuery, viewCache, showContainers, - setMigrationEnabled + setTargetMigration }); }; @@ -59,7 +58,7 @@ describe("Personalization", () => { "isInitialized", "storeViews" ]); - setMigrationEnabled = jasmine.createSpy("setMigrationEnabled"); + setTargetMigration = jasmine.createSpy("setTargetMigration"); build(); }); @@ -162,13 +161,13 @@ describe("Personalization", () => { }); describe("onBeforeRequest", () => { - it("should always call setMigrationEnabled during onBeforeRequest", () => { + it("should always call setTargetMigration during onBeforeRequest", () => { const request = jasmine.createSpyObj("request", ["getPayload"]); personalizationComponent.lifecycle.onBeforeRequest({ request }); - expect(setMigrationEnabled).toHaveBeenCalled(); + expect(setTargetMigration).toHaveBeenCalledOnceWith(request); }); }); }); diff --git a/test/unit/specs/components/Personalization/migration/setMigrationEnabled.spec.js b/test/unit/specs/components/Personalization/createSetTargetMigration.spec.js similarity index 50% rename from test/unit/specs/components/Personalization/migration/setMigrationEnabled.spec.js rename to test/unit/specs/components/Personalization/createSetTargetMigration.spec.js index f5abbd270..765fd2f8b 100644 --- a/test/unit/specs/components/Personalization/migration/setMigrationEnabled.spec.js +++ b/test/unit/specs/components/Personalization/createSetTargetMigration.spec.js @@ -10,32 +10,33 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import setMigrationEnabled from "../../../../../../src/components/Personalization/migration/setMigrationEnabled"; +import createSetTargetMigration from "../../../../../src/components/Personalization/createSetTargetMigration"; -describe("Personalization::setMigrationEnabled", () => { - it("adds to request meta if targetMigrationEnabled=true is configured", () => { - const request = { - getPayload: jasmine - .createSpy("getPayload") - .and.returnValue(jasmine.createSpyObj("getPayloadObj", ["mergeMeta"])) - }; - const config = { - targetMigrationEnabled: true - }; +describe("Personalization::createSetTargetMigration", () => { + let request; + let payload; - setMigrationEnabled(config, request); + beforeEach(() => { + request = jasmine.createSpyObj("request", ["getPayload"]); + payload = jasmine.createSpyObj("payload", ["mergeMeta"]); + request.getPayload.and.returnValue(payload); + }); - expect(request.getPayload).toHaveBeenCalled(); + it("adds to request meta if targetMigrationEnabled=true is configured", () => { + const setTargetMigration = createSetTargetMigration({ + targetMigrationEnabled: true + }); + setTargetMigration(request); + expect(payload.mergeMeta).toHaveBeenCalledOnceWith({ + target: { migration: true } + }); }); it("does not add to request meta if targetMigrationEnabled is not configured", () => { - const request = { - getPayload: jasmine.createSpy("getPayload") - }; - const config = {}; - - setMigrationEnabled(config, request); - - expect(request.getPayload).not.toHaveBeenCalled(); + const setTargetMigration = createSetTargetMigration({ + targetMigrationEnabled: false + }); + setTargetMigration(request); + expect(payload.mergeMeta).not.toHaveBeenCalled(); }); }); diff --git a/test/unit/specs/core/createCookieTransfer.spec.js b/test/unit/specs/core/createCookieTransfer.spec.js index 630a9f607..bdb3d2c1a 100644 --- a/test/unit/specs/core/createCookieTransfer.spec.js +++ b/test/unit/specs/core/createCookieTransfer.spec.js @@ -13,22 +13,23 @@ governing permissions and limitations under the License. import createCookieTransfer from "../../../../src/core/createCookieTransfer"; describe("createCookieTransfer", () => { - const orgId = "ABC@CustomOrg"; const apexDomain = "example.com"; const endpointDomain = "thirdparty.com"; + let shouldTransferCookie; let payload; let cookieJar; let cookieTransfer; const date = new Date(); const dateProvider = () => date; - const config = { orgId }; beforeEach(() => { + shouldTransferCookie = jasmine.createSpy("shouldTransferCookie"); + shouldTransferCookie.and.returnValue(false); payload = jasmine.createSpyObj("payload", ["mergeState"]); cookieJar = jasmine.createSpyObj("cookieJar", ["get", "set"]); cookieTransfer = createCookieTransfer({ cookieJar, - config, + shouldTransferCookie, apexDomain, dateProvider }); @@ -60,6 +61,7 @@ describe("createCookieTransfer", () => { at_qa_mode: '{"token":"QATokenString","listedActivitiesOnly":true,"evaluateAsTrueAudienceIds":["2480042"],"previewIndexes":[{"activityIndex":1,"experienceIndex":1}]}' }); + shouldTransferCookie.and.returnValues(true, false, true, true); cookieTransfer.cookiesToPayload(payload, endpointDomain); expect(payload.mergeState).toHaveBeenCalledWith({ domain: apexDomain, diff --git a/test/unit/specs/core/injectShouldTransferCookie.spec.js b/test/unit/specs/core/injectShouldTransferCookie.spec.js new file mode 100644 index 000000000..d073f17cc --- /dev/null +++ b/test/unit/specs/core/injectShouldTransferCookie.spec.js @@ -0,0 +1,58 @@ +/* +Copyright 2022 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ + +import injectShouldTransferCookie from "../../../../src/core/injectShouldTransferCookie"; + +describe("shouldTransferCookie", () => { + let targetMigrationEnabled; + let orgId; + let shouldTransferCookie; + + beforeEach(() => { + targetMigrationEnabled = false; + orgId = "ABC@CustomOrg"; + shouldTransferCookie = null; + }); + const build = () => { + shouldTransferCookie = injectShouldTransferCookie({ + targetMigrationEnabled, + orgId + }); + }; + + it("returns true if it's at_qa_mode cookie", () => { + build(); + expect(shouldTransferCookie("at_qa_mode")).toBeTrue(); + }); + + it("returns true if it's mbox cookie and targetMigrationEnabled=true", () => { + targetMigrationEnabled = true; + build(); + expect(shouldTransferCookie("mbox")).toBeTrue(); + }); + + it("returns false if it's mbox cookie and targetMigrationEnabled=false", () => { + build(); + expect(shouldTransferCookie("mbox")).toBeFalse(); + }); + + it("returns false if it's not a legacy cookie name", () => { + targetMigrationEnabled = true; + build(); + expect(shouldTransferCookie("foo")).toBeFalse(); + }); + + it("returns true for kndctr cookies", () => { + build(); + expect(shouldTransferCookie("kndctr_ABC_CustomOrg_mynewcookie")).toBeTrue(); + }); +}); diff --git a/test/unit/specs/utils/isLegacyCookieName.spec.js b/test/unit/specs/utils/isLegacyCookieName.spec.js deleted file mode 100644 index 97956e6ff..000000000 --- a/test/unit/specs/utils/isLegacyCookieName.spec.js +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2022 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ - -import isLegacyCookieName from "../../../../src/utils/isLegacyCookieName"; - -describe("isLegacyCookieName", () => { - const config = {}; - - it("returns true if it's at_qa_mode cookie", () => { - const result = isLegacyCookieName("at_qa_mode", config); - expect(result).toBeTrue(); - }); - - it("returns true if it's mbox cookie and targetMigrationEnabled=true", () => { - const targetMigrationEnabledConfig = { - targetMigrationEnabled: true - }; - expect(isLegacyCookieName("mbox", targetMigrationEnabledConfig)).toBeTrue(); - }); - - it("returns false if it's mbox cookie and targetMigrationEnabled=false", () => { - expect(isLegacyCookieName("mbox", config)).toBeFalsy(); - }); - - it("returns false if it's not a legacy cookie name", () => { - const result = isLegacyCookieName("ABC@CustomOrg", config); - expect(result).toBeFalse(); - }); -});