Skip to content

amp-consent metadata values #28187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
micajuine-ho merged 4 commits into from
May 15, 2020
Merged

amp-consent metadata values #28187

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
003c48a
init
May 4, 2020
84da3ed
Merge branch 'master' into metadata
May 11, 2020
80dcaf3
suggested changed
May 11, 2020
eea2052
Suggested changes
May 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 49 additions & 8 deletions extensions/amp-consent/0.1/amp-consent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {dev, devAssert, user, userAssert} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {getData} from '../../../src/event-helper';
import {getServicePromiseForDoc} from '../../../src/service';
import {isEnumValue} from '../../../src/types';
import {isEnumValue, isObject} from '../../../src/types';
import {toggle} from '../../../src/style';

const CONSENT_STATE_MANAGER = 'consentStateManager';
Expand Down Expand Up @@ -255,6 +255,7 @@ export class AmpConsent extends AMP.BaseElement {
}

let consentString;
let metadata;
const data = getData(event);

if (!data || data['type'] != 'consent-response') {
Expand Down Expand Up @@ -287,14 +288,18 @@ export class AmpConsent extends AMP.BaseElement {
data['info'] = undefined;
}
consentString = data['info'];
metadata = this.configureMetadataByConsentString(
data['consentMetadata'],
consentString
);
}

const iframes = this.element.querySelectorAll('iframe');

for (let i = 0; i < iframes.length; i++) {
if (iframes[i].contentWindow === event.source) {
const action = data['action'];
this.handleAction_(action, consentString);
this.handleAction_(action, consentString, metadata);
return;
}
}
Expand Down Expand Up @@ -372,10 +377,12 @@ export class AmpConsent extends AMP.BaseElement {

/**
* Handler User action
*
* @param {string} action
* @param {string=} consentString
* @param {Object=} opt_consentMetadata
*/
handleAction_(action, consentString) {
handleAction_(action, consentString, opt_consentMetadata) {
if (!isEnumValue(ACTION_TYPE, action)) {
// Unrecognized action
return;
Expand All @@ -397,13 +404,15 @@ export class AmpConsent extends AMP.BaseElement {
//accept
this.consentStateManager_.updateConsentInstanceState(
CONSENT_ITEM_STATE.ACCEPTED,
consentString
consentString,
opt_consentMetadata
);
} else if (action == ACTION_TYPE.REJECT) {
// reject
this.consentStateManager_.updateConsentInstanceState(
CONSENT_ITEM_STATE.REJECTED,
consentString
consentString,
opt_consentMetadata
);
} else if (action == ACTION_TYPE.DISMISS) {
this.consentStateManager_.updateConsentInstanceState(
Expand Down Expand Up @@ -531,24 +540,35 @@ export class AmpConsent extends AMP.BaseElement {
) {
this.updateCacheIfNotNull_(
response['consentStateValue'],
response['consentString'] || undefined
response['consentString'] || undefined,
response['consentMetadata'] || undefined
);
}
});
}

/**
* Sync with local storage if consentRequired is true.
*
* @param {string=} responseStateValue
* @param {string=} responseConsentString
* @param {object=} opt_responseMetadata
*/
updateCacheIfNotNull_(responseStateValue, responseConsentString) {
updateCacheIfNotNull_(
responseStateValue,
responseConsentString,
opt_responseMetadata
) {
const consentStateValue = convertEnumValueToState(responseStateValue);
// consentStateValue and consentString are treated as a pair that will update together
if (consentStateValue !== null) {
this.consentStateManager_.updateConsentInstanceState(
consentStateValue,
responseConsentString
responseConsentString,
this.configureMetadataByConsentString(
opt_responseMetadata,
responseConsentString
)
);
}
}
Expand All @@ -571,6 +591,7 @@ export class AmpConsent extends AMP.BaseElement {
const request = /** @type {!JsonObject} */ ({
'consentInstanceId': this.consentId_,
'consentStateValue': getConsentStateValue(storedInfo['consentState']),
'consentMetadata': storedInfo['consentMetadata'],
'consentString': storedInfo['consentString'],
'isDirty': !!storedInfo['isDirty'],
'matchedGeoGroup': this.matchedGeoGroup_,
Expand Down Expand Up @@ -684,6 +705,26 @@ export class AmpConsent extends AMP.BaseElement {
getIsPromptUiOnForTesting() {
return this.isPromptUIOn_;
}

/**
* If consentString is undefined or invalid, don't
* include any metadata in update.
* @param {*} metadata
* @param {string=} consentString
* @return {Object=}
*/
configureMetadataByConsentString(metadata, consentString) {
if (!isObject(metadata) || !consentString) {
if (metadata) {
user().error(
TAG,
'metadata sent by CMP is invalid and will not update.'
);
}
return undefined;
}
return metadata;
}
}

AMP.extension('amp-consent', '0.1', (AMP) => {
Expand Down
70 changes: 68 additions & 2 deletions extensions/amp-consent/0.1/consent-info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,27 @@ import {isEnumValue, isObject} from '../../../src/types';
* STATE: Set when user accept or reject consent.
* STRING: Set when a consent string is used to store more granular consent info
* on vendors.
* METADATA: set when consent metadata is passed in to store more granular consent info
* on vendors.
* DITRYBIT: Set when the stored consent info need to be revoked next time.
* @enum {string}
*/
export const STORAGE_KEY = {
STATE: 's',
STRING: 'r',
IS_DIRTY: 'd',
METADATA: 'm',
};

/**
* Key values for retriving/storing metadata values within consent info
* TODO(micajuineho)
* GDPR_APPLIES: 'ga'
* CONSENT_STRING_TYPE: 'cst'
* @enum {string}
*/
export const METADATA_STORAGE_KEY = {};

/**
* @enum {number}
*/
Expand All @@ -50,6 +62,7 @@ export const CONSENT_ITEM_STATE = {
* @typedef {{
* consentState: CONSENT_ITEM_STATE,
* consentString: (string|undefined),
* consentMetadata: (Object|undefined),
* isDirty: (boolean|undefined),
* }}
*/
Expand All @@ -65,6 +78,7 @@ export function getStoredConsentInfo(value) {
return constructConsentInfo(
CONSENT_ITEM_STATE.UNKNOWN,
undefined,
undefined,
undefined
);
}
Expand All @@ -80,6 +94,7 @@ export function getStoredConsentInfo(value) {
return constructConsentInfo(
consentState,
value[STORAGE_KEY.STRING],
convertStorageMetadata(value[STORAGE_KEY.METADATA]),
value[STORAGE_KEY.IS_DIRTY] && value[STORAGE_KEY.IS_DIRTY] === 1
);
}
Expand Down Expand Up @@ -145,6 +160,12 @@ export function composeStoreValue(consentInfo) {
obj[STORAGE_KEY.IS_DIRTY] = 1;
}

if (consentInfo['consentMetadata']) {
obj[STORAGE_KEY.METADATA] = composeMetadataStoreValue(
consentInfo['consentMetadata']
);
}

if (Object.keys(obj) == 0) {
return null;
}
Expand Down Expand Up @@ -191,36 +212,55 @@ export function isConsentInfoStoredValueSame(infoA, infoB, opt_isDirty) {
} else {
isDirtyEqual = !!infoA['isDirty'] === !!infoB['isDirty'];
}
return stateEqual && stringEqual && isDirtyEqual;
const metadataEqual = isMetadataEqual(
Copy link
Contributor

@zhouyx zhouyx May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced this check to help prevent some unnecessary call to the storageAPI.
Now with the stored value getting more complicated, I am no longer sure if the check itself is worth it...

Copy link
Contributor Author

@micajuine-ho micajuine-ho May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's still worth it, since checking the metadata shouldn't be too much work compared to fetching and parsing info from local storage. Happy to remove the check if you think otherwise.

Copy link
Contributor Author

@micajuine-ho micajuine-ho May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked offline: we will measure performance of this later and determine if it should be removed or not.

infoA['consentMetadata'],
infoB['consentMetadata']
);
return stateEqual && stringEqual && metadataEqual && isDirtyEqual;
}
return false;
}

/**
* Compares metadata values from consentInfo
*
* @param {Object=} unusedMetadataA
* @param {Object=} unusedMetadataB
* @return {boolean}
*/
function isMetadataEqual(unusedMetadataA, unusedMetadataB) {
return true;
}

/**
* Convert the legacy boolean stored value to consentInfo object
* @param {boolean} value
* @return {!ConsentInfoDef}
*/
function getLegacyStoredConsentInfo(value) {
const state = convertValueToState(value);
return constructConsentInfo(state, undefined, undefined);
return constructConsentInfo(state);
}

/**
* Construct the consentInfo object from values
*
* @param {CONSENT_ITEM_STATE} consentState
* @param {string=} opt_consentString
* @param {Object=} opt_consentMetadata
* @param {boolean=} opt_isDirty
* @return {!ConsentInfoDef}
*/
export function constructConsentInfo(
consentState,
opt_consentString,
opt_consentMetadata,
Copy link
Contributor

@zhouyx zhouyx May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why you want to pass the consent metadata before the isDirty. Please double check that all calling to the #constructConsentInfo() have been updated.

Copy link
Contributor Author

@micajuine-ho micajuine-ho May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. Done

opt_isDirty
) {
return {
'consentState': consentState,
'consentString': opt_consentString,
'consentMetadata': opt_consentMetadata,
'isDirty': opt_isDirty,
};
}
Expand Down Expand Up @@ -286,3 +326,29 @@ export function getConsentStateValue(enumState) {

return 'unknown';
}

/**
* Converts metadata to stroage value:
* {'gdprApplies': true, 'consentStringType': 'tcf-v2'} =>
* {'ga': true, 'cst': 'tcf-v2'}
*
* @param {Object} unused
* @return {Object}
*/
function composeMetadataStoreValue(unused) {
const storageMetadata = map();
// TODO(micajuineho) if (metadata['gdprApplies']) {...}
Copy link
Contributor

@zhouyx zhouyx May 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: composeMetadataStoreValue and convertStorageMetadata are a pair of helper methods. What about we implement them together in a separate PR?

Copy link
Contributor Author

@micajuine-ho micajuine-ho May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my idea as well. Just wanted to lay the skeleton out here, and then add implementation as we add gdprApplies and consentStringType to metadata. Is that ok?

Copy link
Contributor

@zhouyx zhouyx May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! Correct me, but I think you have convertStorageMetadata() implemented?

Copy link
Contributor Author

@micajuine-ho micajuine-ho May 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, it shouldn't do anything, and I wanted to make sure my ideas were valid by fleshing it out. Should I remove it?

Will remove for now

Copy link
Contributor

@zhouyx zhouyx May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG. Thanks

return storageMetadata;
}

/**
* TODO(micajuineho) Converts stroage metadata to human readable object:
* {'ga': true, 'cst': 'tcf-v2'} =>
* {'gdprApplies': true, 'consentStringType': 'tcf-v2'}
*
* @param {Object=} unused
* @return {Object=}
*/
function convertStorageMetadata(unused) {
return undefined;
}
25 changes: 23 additions & 2 deletions extensions/amp-consent/0.1/consent-policy-manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ export class ConsentPolicyManager {

/** @private {?string} */
this.consentString_ = null;

/** @private {?Object|undefined} */
this.consentMetadata_ = null;
}

/**
Expand Down Expand Up @@ -179,9 +182,14 @@ export class ConsentPolicyManager {
consentStateChangeHandler_(info) {
const state = info['consentState'];
const consentStr = info['consentString'];
const {consentString_: prevConsentStr} = this;
const consentMetadata = info['consentMetadata'];
const {
consentString_: prevConsentStr,
consentMetadata_: prevConsentMetadata,
} = this;

this.consentString_ = consentStr;
this.consentMetadata_ = consentMetadata;
if (state === CONSENT_ITEM_STATE.UNKNOWN) {
// consent state has not been resolved yet.
return;
Expand All @@ -200,8 +208,9 @@ export class ConsentPolicyManager {
if (this.consentState_ === null) {
this.consentState_ = CONSENT_ITEM_STATE.UNKNOWN;
}
// consentString doesn't change with dismiss action
// consentString & consentMetadata doesn't change with dismiss action
this.consentString_ = prevConsentStr;
this.consentMetadata_ = prevConsentMetadata;
} else {
this.consentState_ = state;
}
Expand Down Expand Up @@ -295,6 +304,18 @@ export class ConsentPolicyManager {
});
}

/**
* Get the consent metadata value of a policy. Return a promise that resolves
* when the policy resolves.
* @param {string} policyId
* @return {!Promise<?Object|undefined>}
*/
getConsentMetadataInfo(policyId) {
return this.whenPolicyResolved(policyId).then(() => {
return this.consentMetadata_;
});
}

/**
* Wait for policy instance to be registered.
* @param {string} policyId
Expand Down
Loading