From 3e1095e9fb3d9689ec6481ae0520dafdd390afee Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Sat, 1 Jul 2017 21:34:40 -0700 Subject: [PATCH] Type check amp-access and make type checking mandatory for extensions. Part of #9876 --- build-system/tasks/compile.js | 4 + .../amp-access-laterpay/0.1/laterpay-impl.js | 127 +++++++++--------- .../amp-access/0.1/amp-access-client.js | 10 +- extensions/amp-access/0.1/amp-access-other.js | 6 +- .../amp-access/0.1/amp-access-server-jwt.js | 26 ++-- .../amp-access/0.1/amp-access-server.js | 22 +-- .../amp-access/0.1/amp-access-vendor.js | 9 +- extensions/amp-access/0.1/amp-access.js | 65 ++++----- extensions/amp-access/0.1/jwt.js | 12 +- extensions/amp-access/0.1/login-dialog.js | 35 ++--- extensions/amp-access/0.1/signin.js | 47 ++++--- .../0.1/test/test-amp-analytics.js | 4 +- gulpfile.js | 13 +- src/service/url-replacements-impl.js | 6 +- src/services.js | 14 +- 15 files changed, 210 insertions(+), 190 deletions(-) diff --git a/build-system/tasks/compile.js b/build-system/tasks/compile.js index 7787dd2db07de..349fa8c4189e3 100644 --- a/build-system/tasks/compile.js +++ b/build-system/tasks/compile.js @@ -154,6 +154,8 @@ function compile(entryModuleFilenames, outputDir, 'extensions/amp-bind/**/*.js', // Needed to access form impl from other extensions 'extensions/amp-form/**/*.js', + // Needed for AccessService + 'extensions/amp-access/**/*.js', // Needed to access UserNotificationManager from other extensions 'extensions/amp-user-notification/**/*.js', 'src/*.js', @@ -283,6 +285,8 @@ function compile(entryModuleFilenames, outputDir, 'build/patched-module/', // Can't seem to suppress `(0, win.eval)` suspicious code warning '3p/environment.js', + // Generated code. + 'extensions/amp-access/0.1/access-expr-impl.js', ], jscomp_error: [], } diff --git a/extensions/amp-access-laterpay/0.1/laterpay-impl.js b/extensions/amp-access-laterpay/0.1/laterpay-impl.js index f8eb15ecbfdb4..96e075f19e8d6 100644 --- a/extensions/amp-access-laterpay/0.1/laterpay-impl.js +++ b/extensions/amp-access-laterpay/0.1/laterpay-impl.js @@ -19,6 +19,7 @@ import {dev, user} from '../../../src/log'; import {installStyles} from '../../../src/style-installer'; import {installStylesForShadowRoot} from '../../../src/shadow-embed'; import {getMode} from '../../../src/mode'; +import {dict} from '../../../src/utils/object'; import {listen} from '../../../src/event-helper'; import {removeChildren} from '../../../src/dom'; import {timerFor} from '../../../src/services'; @@ -46,13 +47,13 @@ const DEFAULT_MESSAGES = { /** * @typedef {{ - * articleTitleSelector: !string, - * configUrl: string=, - * articleId: string=, - * scrollToTopAfterAuth: boolean=, - * locale: string=, - * localeMessages: object=, - * sandbox: boolean=, + * articleTitleSelector: string, + * configUrl: (string|undefined), + * articleId: (string|undefined), + * scrollToTopAfterAuth: (boolean|undefined), + * locale: (string|undefined), + * localeMessages: (Object|undefined), + * sandbox: (boolean|undefined), * }} */ let LaterpayConfigDef; @@ -76,35 +77,35 @@ let PurchaseOptionDef; * access: boolean, * apl: string, * premiumcontent: !PurchaseOptionDef, - * timepasses: Array=, - * subscriptions: Array= + * timepasses: (!Array|undefined), + * subscriptions: (!Array|undefined) * }} */ let PurchaseConfigDef; /** - * @implements {AccessVendor} + * @implements {../../amp-access/0.1/access-vendor.AccessVendor} */ export class LaterpayVendor { /** - * @param {!AccessService} accessService + * @param {!../../amp-access/0.1/amp-access.AccessService} accessService */ constructor(accessService) { /** @const */ this.ampdoc = accessService.ampdoc; - /** @const @private {!AccessService} */ + /** @const @private {!../../amp-access/0.1/amp-access.AccessService} */ this.accessService_ = accessService; - /** @private @const {!Viewport} */ + /** @private @const {!../../../src/service/viewport-impl.Viewport} */ this.viewport_ = viewportForDoc(this.ampdoc); - /** @const @private {!LaterpayConfigDef} */ + /** @const @private {!JsonObject} For shape see LaterpayConfigDef */ this.laterpayConfig_ = this.accessService_.getAdapterConfig(); - /** @private {?PurchaseConfigDef} */ + /** @private {?JsonObject} For shape see PurchaseConfigDef */ this.purchaseConfig_ = null; /** @private {?Function} */ @@ -113,7 +114,7 @@ export class LaterpayVendor { /** @private {?Function} */ this.alreadyPurchasedListener_ = null; - /** @const @private {!Array} */ + /** @const @private {!Array} */ this.purchaseOptionListeners_ = []; /** @private {!boolean} */ @@ -129,27 +130,27 @@ export class LaterpayVendor { this.purchaseButton_ = null; /** @private {string} */ - this.currentLocale_ = this.laterpayConfig_.locale || 'en'; + this.currentLocale_ = this.laterpayConfig_['locale'] || 'en'; - /** @private {Object} */ - this.i18n_ = Object.assign({}, DEFAULT_MESSAGES, - this.laterpayConfig_.localeMessages || {}); + /** @private {!JsonObject} */ + this.i18n_ = /** @type {!JsonObject} */ (Object.assign(dict(), + DEFAULT_MESSAGES, this.laterpayConfig_['localeMessages'] || dict())); /** @private {string} */ this.purchaseConfigBaseUrl_ = this.getConfigUrl_() + CONFIG_BASE_PATH; - const articleId = this.laterpayConfig_.articleId; + const articleId = this.laterpayConfig_['articleId']; if (articleId) { this.purchaseConfigBaseUrl_ += '&article_id=' + encodeURIComponent(articleId); } - /** @const @private {!Timer} */ + /** @const @private {!../../../src/service/timer-impl.Timer} */ this.timer_ = timerFor(this.ampdoc.win); - /** @const @private {!Vsync} */ + /** @const @private {!../../../src/service/vsync-impl.Vsync} */ this.vsync_ = vsyncFor(this.ampdoc.win); - /** @const @private {!Xhr} */ + /** @const @private {!../../../src/service/xhr-impl.Xhr} */ this.xhr_ = xhrFor(this.ampdoc.win); // Install styles. @@ -169,10 +170,10 @@ export class LaterpayVendor { getConfigUrl_() { if ( (getMode().localDev || getMode().development) && - this.laterpayConfig_.configUrl + this.laterpayConfig_['configUrl'] ) { - return this.laterpayConfig_.configUrl; - } else if (this.laterpayConfig_.sandbox) { + return this.laterpayConfig_['configUrl']; + } else if (this.laterpayConfig_['sandbox']) { return SANDBOX_CONFIG_URL; } else { return CONFIG_URL; @@ -191,7 +192,7 @@ export class LaterpayVendor { 'article, or no paid content configurations are setup.'); } - if (this.laterpayConfig_.scrollToTopAfterAuth) { + if (this.laterpayConfig_['scrollToTopAfterAuth']) { this.vsync_.mutate(() => this.viewport_.setScrollTop(0)); } this.emptyContainer_(); @@ -250,23 +251,23 @@ export class LaterpayVendor { */ getArticleTitle_() { const title = this.ampdoc.getRootNode().querySelector( - this.laterpayConfig_.articleTitleSelector); + this.laterpayConfig_['articleTitleSelector']); user().assert( title, 'No article title element found with selector %s', - this.laterpayConfig_.articleTitleSelector); + this.laterpayConfig_['articleTitleSelector']); return title.textContent.trim(); } /** - * @return {!Node} + * @return {!Element} * @private */ getContainer_() { const id = TAG + '-dialog'; const dialogContainer = this.ampdoc.getElementById(id); - return user().assert( + return user().assertElement( dialogContainer, - 'No element found with id %s', id + 'No element found with id ' + id ); } @@ -305,46 +306,48 @@ export class LaterpayVendor { const dialogContainer = this.getContainer_(); this.innerContainer_ = this.createElement_('div'); this.innerContainer_.className = TAG + '-container'; - if (this.laterpayConfig_.sandbox) { + if (this.laterpayConfig_['sandbox']) { this.renderTextBlock_('sandbox'); } this.renderTextBlock_('header'); const listContainer = this.createElement_('ul'); - this.purchaseConfig_.premiumcontent['tp_title'] = - this.i18n_.premiumContentTitle; - this.purchaseConfig_.premiumcontent.description = this.getArticleTitle_(); + this.purchaseConfig_['premiumcontent']['tp_title'] = + this.i18n_['premiumContentTitle']; + this.purchaseConfig_['premiumcontent']['description'] = + this.getArticleTitle_(); listContainer.appendChild( - this.createPurchaseOption_(this.purchaseConfig_.premiumcontent) + this.createPurchaseOption_(this.purchaseConfig_['premiumcontent']) ); - this.purchaseConfig_.timepasses.forEach(timepass => { + this.purchaseConfig_['timepasses'].forEach(timepass => { listContainer.appendChild(this.createPurchaseOption_(timepass)); }); - this.purchaseConfig_.subscriptions.forEach(subscription => { + this.purchaseConfig_['subscriptions'].forEach(subscription => { listContainer.appendChild(this.createPurchaseOption_(subscription)); }); const purchaseButton = this.createElement_('button'); purchaseButton.className = TAG + '-purchase-button'; - purchaseButton.textContent = this.i18n_.defaultButton; + purchaseButton.textContent = this.i18n_['defaultButton']; this.purchaseButton_ = purchaseButton; this.purchaseButtonListener_ = listen(purchaseButton, 'click', ev => { const value = this.selectedPurchaseOption_.value; - const purchaseType = this.selectedPurchaseOption_.dataset.purchaseType; + const purchaseType = this.selectedPurchaseOption_.dataset['purchaseType']; this.handlePurchase_(ev, value, purchaseType); }); this.innerContainer_.appendChild(listContainer); this.innerContainer_.appendChild(purchaseButton); this.innerContainer_.appendChild( - this.createAlreadyPurchasedLink_(this.purchaseConfig_.apl)); + this.createAlreadyPurchasedLink_(this.purchaseConfig_['apl'])); this.renderTextBlock_('footer'); dialogContainer.appendChild(this.innerContainer_); dialogContainer.appendChild(this.createLaterpayBadge_()); this.containerEmpty_ = false; - this.preselectFirstOption_(listContainer.firstElementChild); + this.preselectFirstOption_( + dev().assertElement(listContainer.firstElementChild)); } /** * @private - * @param {!Node} firstOption + * @param {!Element} firstOption */ preselectFirstOption_(firstOption) { const firstInput = firstOption.querySelector('input[type="radio"]'); @@ -367,7 +370,7 @@ export class LaterpayVendor { /** * @private - * @return {!Node} + * @return {!Element} */ createLaterpayBadge_() { const a = this.createElement_('a'); @@ -382,42 +385,42 @@ export class LaterpayVendor { } /** - * @param {!PurchaseOptionDef} option - * @return {!Node} + * @param {!JsonObject} option Shape: PurchaseOptionDef + * @return {!Element} * @private */ createPurchaseOption_(option) { const li = this.createElement_('li'); const control = this.createElement_('label'); - control.for = option.tp_title; + control.for = option['tp_title']; control.appendChild(this.createRadioControl_(option)); const metadataContainer = this.createElement_('div'); metadataContainer.className = TAG + '-metadata'; const title = this.createElement_('span'); title.className = TAG + '-title'; - title.textContent = option.tp_title; + title.textContent = option['tp_title']; metadataContainer.appendChild(title); const description = this.createElement_('p'); description.className = TAG + '-description'; - description.textContent = option.description; + description.textContent = option['description']; metadataContainer.appendChild(description); control.appendChild(metadataContainer); li.appendChild(control); - li.appendChild(this.createPrice_(option.price)); + li.appendChild(this.createPrice_(option['price'])); return li; } /** - * @param {!PurchaseOptionDef} option - * @return {!Node} + * @param {!JsonObject} option Shape: PurchaseOptionDef + * @return {!Element} * @private */ createRadioControl_(option) { const radio = this.createElement_('input'); radio.name = 'purchaseOption'; radio.type = 'radio'; - radio.id = option.tp_title; - radio.value = option.purchase_url; + radio.id = option['tp_title']; + radio.value = option['purchase_url']; const purchaseType = option['purchase_type'] === 'ppu' ? 'payLater' : 'payNow'; @@ -432,7 +435,7 @@ export class LaterpayVendor { /** * @param {!Object} price - * @return {!Node} + * @return {!Element} * @private */ createPrice_(price) { @@ -467,14 +470,14 @@ export class LaterpayVendor { /** * @param {!string} href - * @return {!Node} + * @return {!Element} */ createAlreadyPurchasedLink_(href) { const p = this.createElement_('p'); p.className = TAG + '-already-purchased-link-container'; const a = this.createElement_('a'); a.href = href; - a.textContent = this.i18n_.alreadyPurchasedLink; + a.textContent = this.i18n_['alreadyPurchasedLink']; this.alreadyPurchasedListener_ = listen(a, 'click', ev => { this.handlePurchase_(ev, href, 'alreadyPurchased'); }); @@ -488,17 +491,17 @@ export class LaterpayVendor { */ handlePurchaseOptionSelection_(ev) { ev.preventDefault(); - this.selectPurchaseOption_(ev.target); + this.selectPurchaseOption_(dev().assertElement(ev.target)); } /** - * @param {!Node} target + * @param {!Element} target * @private */ selectPurchaseOption_(target) { const selectedOptionClassname = TAG + '-selected'; const prevPurchaseOption = this.selectedPurchaseOption_; - const purchaseActionLabel = target.dataset.purchaseActionLabel; + const purchaseActionLabel = target.dataset['purchaseActionLabel']; if (prevPurchaseOption && prevPurchaseOption.classList.contains(selectedOptionClassname)) { prevPurchaseOption.classList.remove(selectedOptionClassname); diff --git a/extensions/amp-access/0.1/amp-access-client.js b/extensions/amp-access/0.1/amp-access-client.js index cd8038df9407e..e50bb6fe5b340 100644 --- a/extensions/amp-access/0.1/amp-access-client.js +++ b/extensions/amp-access/0.1/amp-access-client.js @@ -27,19 +27,19 @@ const TAG = 'amp-access-client'; const DEFAULT_AUTHORIZATION_TIMEOUT = 3000; -/** @implements {AccessTypeAdapterDef} */ +/** @implements {./amp-access.AccessTypeAdapterDef} */ export class AccessClientAdapter { /** * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!JsonObject} configJson - * @param {!AccessTypeAdapterContextDef} context + * @param {!./amp-access.AccessTypeAdapterContextDef} context */ constructor(ampdoc, configJson, context) { /** @const */ this.ampdoc = ampdoc; - /** @const @private {!AccessTypeAdapterContextDef} */ + /** @const @private {!./amp-access.AccessTypeAdapterContextDef} */ this.context_ = context; /** @const @private {string} */ @@ -61,10 +61,10 @@ export class AccessClientAdapter { this.authorizationTimeout_ = this.buildConfigAuthorizationTimeout_( configJson); - /** @const @private {!Xhr} */ + /** @const @private {!../../../src/service/xhr-impl.Xhr} */ this.xhr_ = xhrFor(ampdoc.win); - /** @const @private {!Timer} */ + /** @const @private {!../../../src/service/timer-impl.Timer} */ this.timer_ = timerFor(ampdoc.win); } diff --git a/extensions/amp-access/0.1/amp-access-other.js b/extensions/amp-access/0.1/amp-access-other.js index f11bb3f232bbc..dcfc41d698c09 100644 --- a/extensions/amp-access/0.1/amp-access-other.js +++ b/extensions/amp-access/0.1/amp-access-other.js @@ -21,19 +21,19 @@ import {isProxyOrigin} from '../../../src/url'; const TAG = 'amp-access-other'; -/** @implements {AccessTypeAdapterDef} */ +/** @implements {./amp-access.AccessTypeAdapterDef} */ export class AccessOtherAdapter { /** * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!JsonObject} configJson - * @param {!AccessTypeAdapterContextDef} context + * @param {!./amp-access.AccessTypeAdapterContextDef} context */ constructor(ampdoc, configJson, context) { /** @const */ this.ampdoc = ampdoc; - /** @const @private {!AccessTypeAdapterContextDef} */ + /** @const @private {!./amp-access.AccessTypeAdapterContextDef} */ this.context_ = context; /** @private {?JsonObject} */ diff --git a/extensions/amp-access/0.1/amp-access-server-jwt.js b/extensions/amp-access/0.1/amp-access-server-jwt.js index 229f642ce5b81..fbf519fbb1a10 100644 --- a/extensions/amp-access/0.1/amp-access-server-jwt.js +++ b/extensions/amp-access/0.1/amp-access-server-jwt.js @@ -17,6 +17,7 @@ import {AccessClientAdapter} from './amp-access-client'; import {JwtHelper} from './jwt'; import {assertHttpsUrl} from '../../../src/url'; +import {dict} from '../../../src/utils/object'; import {getMode} from '../../../src/mode'; import {isArray} from '../../../src/types'; import {isExperimentOn} from '../../../src/experiments'; @@ -72,35 +73,35 @@ const AMP_AUD = 'ampproject.org'; * \/ * Apply authorization response * - * @implements {AccessTypeAdapterDef} + * @implements {./amp-access.AccessTypeAdapterDef} */ export class AccessServerJwtAdapter { /** * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!JsonObject} configJson - * @param {!AccessTypeAdapterContextDef} context + * @param {!./amp-access.AccessTypeAdapterContextDef} context */ constructor(ampdoc, configJson, context) { /** @const */ this.ampdoc = ampdoc; - /** @const @private {!AccessTypeAdapterContextDef} */ + /** @const @private {!./amp-access.AccessTypeAdapterContextDef} */ this.context_ = context; /** @private @const */ this.clientAdapter_ = new AccessClientAdapter(ampdoc, configJson, context); - /** @private @const {!Viewer} */ + /** @private @const {!../../../src/service/viewer-impl.Viewer} */ this.viewer_ = viewerForDoc(ampdoc); - /** @const @private {!Xhr} */ + /** @const @private {!../../../src/service/xhr-impl.Xhr} */ this.xhr_ = xhrFor(ampdoc.win); - /** @const @private {!Timer} */ + /** @const @private {!../../../src/service/timer-impl.Timer} */ this.timer_ = timerFor(ampdoc.win); - /** @const @private {!Vsync} */ + /** @const @private {!../../../src/service/vsync-impl.Vsync} */ this.vsync_ = vsyncFor(ampdoc.win); const stateElement = ampdoc.getRootNode().querySelector( @@ -183,7 +184,7 @@ export class AccessServerJwtAdapter { } /** - * @return {!Promise<{encoded:string, jwt:!JSONObject}>} + * @return {!Promise<{encoded:string, jwt:!JsonObject}>} * @private */ fetchJwt_() { @@ -235,7 +236,8 @@ export class AccessServerJwtAdapter { if (this.key_) { return Promise.resolve(this.key_); } - return this.xhr_.fetchText(this.keyUrl_).then(res => res.text()); + return this.xhr_.fetchText(dev().assertString(this.keyUrl_)) + .then(res => res.text()); } /** @@ -247,7 +249,7 @@ export class AccessServerJwtAdapter { } /** - * @param {!JSONObject} jwt + * @param {!JsonObject} jwt * @private */ validateJwt_(jwt) { @@ -298,11 +300,11 @@ export class AccessServerJwtAdapter { const encoded = resp.encoded; const jwt = resp.jwt; const accessData = jwt['amp_authdata']; - const request = serializeQueryString({ + const request = serializeQueryString(dict({ 'url': removeFragment(this.ampdoc.win.location.href), 'state': this.serverState_, 'jwt': encoded, - }); + })); dev().fine(TAG, 'Authorization request: ', this.serviceUrl_, request); dev().fine(TAG, '- access data: ', accessData); // Note that `application/x-www-form-urlencoded` is used to avoid diff --git a/extensions/amp-access/0.1/amp-access-server.js b/extensions/amp-access/0.1/amp-access-server.js index 272de88e80669..d2094d7eb4d47 100644 --- a/extensions/amp-access/0.1/amp-access-server.js +++ b/extensions/amp-access/0.1/amp-access-server.js @@ -18,6 +18,8 @@ import {AccessClientAdapter} from './amp-access-client'; import {isExperimentOn} from '../../../src/experiments'; import {isProxyOrigin, removeFragment} from '../../../src/url'; import {dev} from '../../../src/log'; +import {dict} from '../../../src/utils/object'; +import {parseJson} from '../../../src/json'; import {timerFor} from '../../../src/services'; import {viewerForDoc} from '../../../src/services'; import {vsyncFor} from '../../../src/services'; @@ -54,35 +56,35 @@ const TAG = 'amp-access-server'; * \/ * Apply authorization response * - * @implements {AccessTypeAdapterDef} + * @implements {./amp-access.AccessTypeAdapterDef} */ export class AccessServerAdapter { /** * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc * @param {!JsonObject} configJson - * @param {!AccessTypeAdapterContextDef} context + * @param {!./amp-access.AccessTypeAdapterContextDef} context */ constructor(ampdoc, configJson, context) { /** @const */ this.ampdoc = ampdoc; - /** @const @private {!AccessTypeAdapterContextDef} */ + /** @const @private {!./amp-access.AccessTypeAdapterContextDef} */ this.context_ = context; /** @private @const */ this.clientAdapter_ = new AccessClientAdapter(ampdoc, configJson, context); - /** @private @const {!Viewer} */ + /** @private @const {!../../../src/service/viewer-impl.Viewer} */ this.viewer_ = viewerForDoc(ampdoc); - /** @const @private {!Xhr} */ + /** @const @private {!../../../src/service/xhr-impl.Xhr} */ this.xhr_ = xhrFor(ampdoc.win); - /** @const @private {!Timer} */ + /** @const @private {!../../../src/service/timer-impl.Timer} */ this.timer_ = timerFor(ampdoc.win); - /** @const @private {!Vsync} */ + /** @const @private {!../../../src/service/vsync-impl.Vsync} */ this.vsync_ = vsyncFor(ampdoc.win); const stateElement = ampdoc.getRootNode().querySelector( @@ -142,11 +144,11 @@ export class AccessServerAdapter { requestVars[k] = String(vars[k]); } } - const request = { + const request = dict({ 'url': removeFragment(this.ampdoc.win.location.href), 'state': this.serverState_, 'vars': requestVars, - }; + }); dev().fine(TAG, 'Authorization request: ', this.serviceUrl_, request); // Note that `application/x-www-form-urlencoded` is used to avoid // CORS preflight request. @@ -165,7 +167,7 @@ export class AccessServerAdapter { const accessDataString = dev().assert( responseDoc.querySelector('script[id="amp-access-data"]'), 'No authorization data available').textContent; - const accessData = JSON.parse(accessDataString); + const accessData = parseJson(accessDataString); dev().fine(TAG, '- access data: ', accessData); return this.replaceSections_(responseDoc).then(() => { diff --git a/extensions/amp-access/0.1/amp-access-vendor.js b/extensions/amp-access/0.1/amp-access-vendor.js index d56833d26a066..7d65ad2b7ae88 100644 --- a/extensions/amp-access/0.1/amp-access-vendor.js +++ b/extensions/amp-access/0.1/amp-access-vendor.js @@ -15,6 +15,7 @@ */ import {dev, user} from '../../../src/log'; +import './access-vendor'; /** @const {string} */ const TAG = 'amp-access-vendor'; @@ -25,7 +26,7 @@ const TAG = 'amp-access-vendor'; * interface and delivered via a separate extension. The vendor implementation * mainly requires two method: `authorize` and `pingback`. The actual * extension is registered via `registerVendor` method. - * @implements {AccessTypeAdapterDef} + * @implements {./amp-access.AccessTypeAdapterDef} */ export class AccessVendorAdapter { @@ -47,9 +48,11 @@ export class AccessVendorAdapter { /** @const @private {boolean} */ this.isPingbackEnabled_ = !configJson['noPingback']; + /** @private {?function(!./access-vendor.AccessVendor)|undefined} */ + this.vendorResolve_ = null; + /** @const @private {!Promise} */ this.vendorPromise_ = new Promise(resolve => { - /** @private {function(!./access-vendor.AccessVendor)|undefined} */ this.vendorResolve_ = resolve; }); } @@ -61,7 +64,7 @@ export class AccessVendorAdapter { /** * @param {string} name - * @param {./access-vendor.AccessVendor} vendor + * @param {!./access-vendor.AccessVendor} vendor */ registerVendor(name, vendor) { user().assert(this.vendorResolve_, 'Vendor has already been registered'); diff --git a/extensions/amp-access/0.1/amp-access.js b/extensions/amp-access/0.1/amp-access.js index 0c0cbffba9d56..ae74c7106f8c8 100644 --- a/extensions/amp-access/0.1/amp-access.js +++ b/extensions/amp-access/0.1/amp-access.js @@ -34,6 +34,7 @@ import {isExperimentOn} from '../../../src/experiments'; import {isObject} from '../../../src/types'; import {listenOnce} from '../../../src/event-helper'; import {dev, user} from '../../../src/log'; +import {dict} from '../../../src/utils/object'; import {getLoginUrl, openLoginDialog} from './login-dialog'; import {parseQueryString} from '../../../src/url'; import {performanceForOrNull} from '../../../src/services'; @@ -103,17 +104,19 @@ export class AccessService { this.isJwtEnabled_ = isExperimentOn(ampdoc.win, 'amp-access-jwt'); /** @const @private {!Element} */ - this.accessElement_ = accessElement; + this.accessElement_ = dev().assertElement(accessElement); const configJson = tryParseJson(this.accessElement_.textContent, e => { throw user().createError('Failed to parse "amp-access" JSON: ' + e); }); /** @const @private {!AccessType} */ - this.type_ = this.buildConfigType_(configJson); + this.type_ = this.buildConfigType_(/** @type {!JsonObject} */ ( + configJson)); - /** @const @private {!Object} */ - this.loginConfig_ = this.buildConfigLoginMap_(configJson); + /** @const @private {!JsonObject} */ + this.loginConfig_ = this.buildConfigLoginMap_(/** @type {!JsonObject} */ ( + configJson)); /** @const @private {!JsonObject} */ this.authorizationFallbackResponse_ = @@ -125,33 +128,33 @@ export class AccessService { /** @const @private {string} */ this.pubOrigin_ = getSourceOrigin(ampdoc.win.location); - /** @const @private {!Timer} */ + /** @const @private {!../../../src/service/timer-impl.Timer} */ this.timer_ = timerFor(ampdoc.win); - /** @const @private {!Vsync} */ + /** @const @private {!../../../src/service/vsync-impl.Vsync} */ this.vsync_ = vsyncFor(ampdoc.win); - /** @const @private {!UrlReplacements} */ + /** @const @private {!../../../src/service/url-replacements-impl.UrlReplacements} */ this.urlReplacements_ = urlReplacementsForDoc(ampdoc); // TODO(dvoytenko, #3742): This will refer to the ampdoc once AccessService // is migrated to ampdoc as well. - /** @private @const {!Cid} */ + /** @private @const {!Promise} */ this.cid_ = cidForDoc(ampdoc); - /** @private @const {!Viewer} */ + /** @private @const {!../../../src/service/viewer-impl.Viewer} */ this.viewer_ = viewerForDoc(ampdoc); - /** @private @const {!Viewport} */ + /** @private @const {!../../../src/service/viewport-impl.Viewport} */ this.viewport_ = viewportForDoc(ampdoc); - /** @private @const {!Templates} */ + /** @private @const {!../../../src/service/template-impl.Templates} */ this.templates_ = templatesFor(ampdoc.win); - /** @private @const {!Resources} */ + /** @private @const {!../../../src/service/resources-impl.Resources} */ this.resources_ = resourcesForDoc(ampdoc); - /** @private @const {?Performance} */ + /** @private @const {?../../../src/service/performance-impl.Performance} */ this.performance_ = performanceForOrNull(ampdoc.win); /** @private @const {function(string):Promise} */ @@ -167,9 +170,11 @@ export class AccessService { this.signIn_ = new SignInProtocol(ampdoc, this.viewer_, this.pubOrigin_, configJson); + /** @private {?Function} */ + this.firstAuthorizationResolver_ = null; + /** @const @private {!Promise} */ this.firstAuthorizationPromise_ = new Promise(resolve => { - /** @private {!Promise} */ this.firstAuthorizationResolver_ = resolve; }); @@ -200,7 +205,7 @@ export class AccessService { /** * @param {string} name - * @param {./access-vendor.AccessVendor} vendor + * @param {!./access-vendor.AccessVendor} vendor */ registerVendor(name, vendor) { user().assert(this.type_ == AccessType.VENDOR, @@ -232,7 +237,7 @@ export class AccessService { } return new AccessServerAdapter(this.ampdoc, configJson, context); case AccessType.VENDOR: - return new AccessVendorAdapter(this.ampdoc, configJson, context); + return new AccessVendorAdapter(this.ampdoc, configJson); case AccessType.OTHER: return new AccessOtherAdapter(this.ampdoc, configJson, context); } @@ -274,12 +279,12 @@ export class AccessService { /** * @param {!JsonObject} configJson - * @return {?Object} + * @return {!JsonObject} * @private */ buildConfigLoginMap_(configJson) { const loginConfig = configJson['login']; - const loginMap = {}; + const loginMap = dict(); if (!loginConfig) { // Ignore: in some cases login config is not necessary. } else if (typeof loginConfig == 'string') { @@ -295,7 +300,7 @@ export class AccessService { // Check that all URLs are valid. for (const k in loginMap) { - assertHttpsUrl(loginMap[k]); + assertHttpsUrl(loginMap[k], this.accessElement_); } return loginMap; } @@ -375,10 +380,10 @@ export class AccessService { /** @private */ broadcastReauthorize_() { - this.viewer_.broadcast({ + this.viewer_.broadcast(dict({ 'type': 'amp-access-reauthorize', 'origin': this.pubOrigin_, - }); + })); } /** @@ -547,7 +552,7 @@ export class AccessService { } /** - * @param {!JsonObjectDef} response + * @param {!JsonObject} response * @return {!Promise} * @private */ @@ -562,7 +567,7 @@ export class AccessService { /** * @param {!Element} element - * @param {!JsonObjectDef} response + * @param {!JsonObject} response * @return {!Promise} * @private */ @@ -603,8 +608,8 @@ export class AccessService { /** * Discovers and renders templates. * @param {!Element} element - * @param {!JsonObjectDef} response - * @return {!Promise} + * @param {!JsonObject} response + * @return {?Promise} * @private */ renderTemplates_(element, response) { @@ -627,7 +632,7 @@ export class AccessService { /** * @param {!Element} element * @param {!Element} templateOrPrev - * @param {!JsonObjectDef} response + * @param {!JsonObject} response * @return {!Promise} * @private */ @@ -776,7 +781,7 @@ export class AccessService { } /** - * @param {!ActionInvocation} invocation + * @param {!../../../src/service/action-impl.ActionInvocation} invocation * @private */ handleAction_(invocation) { @@ -910,7 +915,7 @@ export class AccessService { } /** - * @return {?Promise} + * @return {?Promise>} * @private */ buildLoginUrls_() { @@ -938,13 +943,13 @@ export class AccessService { * !Promise> * }} */ -let AccessTypeAdapterContextDef; +export let AccessTypeAdapterContextDef; /** * @interface */ -class AccessTypeAdapterDef { +export class AccessTypeAdapterDef { /** * @return {!JsonObject} diff --git a/extensions/amp-access/0.1/jwt.js b/extensions/amp-access/0.1/jwt.js index c7bcf10db61c0..89cf703c139d2 100644 --- a/extensions/amp-access/0.1/jwt.js +++ b/extensions/amp-access/0.1/jwt.js @@ -24,8 +24,8 @@ import {tryParseJson} from '../../../src/json'; /** * @typedef {{ - * header: !JSONObject, - * payload: !JSONObject, + * header: (?JsonObject|undefined), + * payload: (?JsonObject|undefined), * verifiable: string, * sig: string, * }} @@ -48,7 +48,7 @@ export class JwtHelper { /** * Might be `null` if the platform does not support Crypto Subtle. - * @const @private {?SubtleCrypto} + * @const @private {?webCrypto.SubtleCrypto} */ this.subtle_ = win.crypto && (win.crypto.subtle || win.crypto.webkitSubtle) || null; @@ -57,7 +57,7 @@ export class JwtHelper { /** * Decodes JWT token and returns its payload. * @param {string} encodedToken - * @return {!JSONObject} + * @return {?JsonObject|undefined} */ decode(encodedToken) { return this.decodeInternal_(encodedToken).payload; @@ -75,7 +75,7 @@ export class JwtHelper { * Decodes HWT token and verifies its signature. * @param {string} encodedToken * @param {!Promise} pemPromise - * @return {!Promise} + * @return {!Promise} */ decodeAndVerify(encodedToken, pemPromise) { if (!this.subtle_) { @@ -136,7 +136,7 @@ export class JwtHelper { /** * @param {!Promise} pemPromise - * @return {!Promise} + * @return {!Promise} */ importKey_(pemPromise) { return pemPromise.then(pem => { diff --git a/extensions/amp-access/0.1/login-dialog.js b/extensions/amp-access/0.1/login-dialog.js index 83dc551bd2f33..cd33cf7d4d3b2 100644 --- a/extensions/amp-access/0.1/login-dialog.js +++ b/extensions/amp-access/0.1/login-dialog.js @@ -15,12 +15,13 @@ */ import {getMode} from '../../../src/mode'; -import {listen} from '../../../src/event-helper'; +import {getData, listen} from '../../../src/event-helper'; import {dev, user} from '../../../src/log'; import {openWindowDialog} from '../../../src/dom'; import {parseUrl} from '../../../src/url'; import {viewerForDoc} from '../../../src/services'; import {urls} from '../../../src/config'; +import {dict} from '../../../src/utils/object'; /** @const */ const TAG = 'amp-access-login'; @@ -71,11 +72,11 @@ export function getLoginUrl(ampdoc, urlOrPromise) { */ class ViewerLoginDialog { /** - * @param {!Viewer} viewer + * @param {!../../../src/service/viewer-impl.Viewer} viewer * @param {string|!Promise} urlOrPromise */ constructor(viewer, urlOrPromise) { - /** @const {!Viewer} */ + /** @const {!../../../src/service/viewer-impl.Viewer} */ this.viewer = viewer; /** @const {string|!Promise} */ @@ -107,9 +108,9 @@ class ViewerLoginDialog { open() { return this.getLoginUrl().then(loginUrl => { dev().fine(TAG, 'Open viewer dialog: ', loginUrl); - return this.viewer.sendMessageAwaitResponse('openDialog', { + return this.viewer.sendMessageAwaitResponse('openDialog', dict({ 'url': loginUrl, - }); + })); }); } @@ -123,20 +124,20 @@ class ViewerLoginDialog { export class WebLoginDialog { /** * @param {!Window} win - * @param {!Viewer} viewer + * @param {!../../../src/service/viewer-impl.Viewer} viewer * @param {string|!Promise} urlOrPromise */ constructor(win, viewer, urlOrPromise) { /** @const {!Window} */ this.win = win; - /** @const {!Viewer} */ + /** @const {!../../../src/service/viewer-impl.Viewer} */ this.viewer = viewer; /** @const {string|!Promise} */ this.urlOrPromise = urlOrPromise; - /** @private {?function(string)} */ + /** @private {?function(?string)} */ this.resolve_ = null; /** @private {?function(*)} */ @@ -151,7 +152,7 @@ export class WebLoginDialog { /** @private {?number} */ this.heartbeatInterval_ = null; - /** @private {?Unlisten} */ + /** @private {?UnlistenDef} */ this.messageUnlisten_ = null; } @@ -284,18 +285,18 @@ export class WebLoginDialog { if (e.origin != returnOrigin) { return; } - if (!e.data || e.data.sentinel != 'amp') { + if (!getData(e) || getData(e)['sentinel'] != 'amp') { return; } - dev().fine(TAG, 'Received message from dialog: ', e.data); - if (e.data.type == 'result') { + dev().fine(TAG, 'Received message from dialog: ', getData(e)); + if (getData(e)['type'] == 'result') { if (this.dialog_) { - this.dialog_./*OK*/postMessage({ - sentinel: 'amp', - type: 'result-ack', - }, returnOrigin); + this.dialog_./*OK*/postMessage(dict({ + 'sentinel': 'amp', + 'type': 'result-ack', + }), returnOrigin); } - this.loginDone_(e.data.result); + this.loginDone_(getData(e)['result']); } }); } diff --git a/extensions/amp-access/0.1/signin.js b/extensions/amp-access/0.1/signin.js index c015273393790..d094646924307 100644 --- a/extensions/amp-access/0.1/signin.js +++ b/extensions/amp-access/0.1/signin.js @@ -17,6 +17,7 @@ import {isArray} from '../../../src/types'; import {isExperimentOn} from '../../../src/experiments'; import {user} from '../../../src/log'; +import {dict} from '../../../src/utils/object'; /** @const */ @@ -58,15 +59,15 @@ export class SignInProtocol { /** * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc - * @param {!Viewer} viewer + * @param {!../../../src/service/viewer-impl.Viewer} viewer * @param {string} pubOrigin - * @param {!JSONObject} configJson + * @param {!JsonObject} configJson */ constructor(ampdoc, viewer, pubOrigin, configJson) { /** @const */ this.ampdoc = ampdoc; - /** @private @const {!Viewer} */ + /** @private @const {!../../../src/service/viewer-impl.Viewer} */ this.viewer_ = viewer; /** @private @const {string} */ @@ -78,10 +79,11 @@ export class SignInProtocol { this.viewer_.isEmbedded() && this.viewer_.getParam('signin') == '1'; + let acceptAccessToken; + let supportsSignInService; if (this.isEnabled_) { - /** @private @const {boolean} */ - this.acceptAccessToken_ = !!configJson['acceptAccessToken']; + acceptAccessToken = !!configJson['acceptAccessToken']; const viewerSignInService = this.viewer_.getParam('signinService'); const configSignInServices = configJson['signinServices']; @@ -90,16 +92,17 @@ export class SignInProtocol { '"signinServices" must be an array'); } - /** @private @const {boolean} */ - this.supportsSignInService_ = configSignInServices && + supportsSignInService = configSignInServices && configSignInServices.indexOf(viewerSignInService) != -1; } else { - /** @private @const {boolean} */ - this.acceptAccessToken_ = false; - /** @private @const {boolean} */ - this.supportsSignInService_ = false; + acceptAccessToken = false; + supportsSignInService = false; } + /** @private @const {boolean} */ + this.acceptAccessToken_ = acceptAccessToken; + /** @private @const {boolean} */ + this.supportsSignInService_ = supportsSignInService; /** @private {?Promise} */ this.accessTokenPromise_ = null; } @@ -138,9 +141,9 @@ export class SignInProtocol { } if (!this.accessTokenPromise_) { this.accessTokenPromise_ = this.viewer_.sendMessageAwaitResponse( - 'getAccessTokenPassive', { - origin: this.pubOrigin_, - }).then(resp => { + 'getAccessTokenPassive', dict({ + 'origin': this.pubOrigin_, + })).then(resp => { return /** @type {?string} */ (resp); }).catch(reason => { user().error(TAG, 'Failed to retrieve access token: ', reason); @@ -184,10 +187,10 @@ export class SignInProtocol { if (!authorizationCode) { return null; } - return this.viewer_.sendMessageAwaitResponse('storeAccessToken', { - origin: this.pubOrigin_, - authorizationCode, - }).then(resp => { + return this.viewer_.sendMessageAwaitResponse('storeAccessToken', dict({ + 'origin': this.pubOrigin_, + 'authorizationCode': authorizationCode, + })).then(resp => { const accessToken = /** @type {?string} */ (resp); this.updateAccessToken_(accessToken); return accessToken; @@ -217,10 +220,10 @@ export class SignInProtocol { if (!this.supportsSignInService_) { return null; } - return this.viewer_.sendMessageAwaitResponse('requestSignIn', { - origin: this.pubOrigin_, - url, - }).then(resp => { + return this.viewer_.sendMessageAwaitResponse('requestSignIn', dict({ + 'origin': this.pubOrigin_, + 'url': url, + })).then(resp => { const accessToken = /** @type {?string} */ (resp); this.updateAccessToken_(accessToken); // Return empty dialog result. diff --git a/extensions/amp-analytics/0.1/test/test-amp-analytics.js b/extensions/amp-analytics/0.1/test/test-amp-analytics.js index 389b08aea628d..39dc30063421c 100644 --- a/extensions/amp-analytics/0.1/test/test-amp-analytics.js +++ b/extensions/amp-analytics/0.1/test/test-amp-analytics.js @@ -177,8 +177,8 @@ describe('amp-analytics', function() { * inline config and iframePings/optout are not allowed to be used without * AMP team's approval. * - * @param {!JSONObject} config The inline config to update. - * @return {!JSONObject} + * @param {!JsonObject} config The inline config to update. + * @return {!JsonObject} */ function clearVendorOnlyConfig(config) { for (const t in config.triggers) { diff --git a/gulpfile.js b/gulpfile.js index 6fcd099e0ff2d..7d6c06b9656b4 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -47,8 +47,8 @@ var extensionAliasFilePath = {}; // Each extension and version must be listed individually here. declareExtension('amp-3q-player', '0.1', false); -declareExtension('amp-access', '0.1', true, 'NO_TYPE_CHECK'); -declareExtension('amp-access-laterpay', '0.1', true, 'NO_TYPE_CHECK'); +declareExtension('amp-access', '0.1', true); +declareExtension('amp-access-laterpay', '0.1', true); declareExtension('amp-accordion', '0.1', false); declareExtension('amp-ad', '0.1', true); declareExtension('amp-ad-network-adsense-impl', 0.1, false); @@ -144,12 +144,8 @@ declareExtensionVersionAlias( * @param {string} version E.g. 0.1 * @param {boolean|!Object} hasCssOrOptions Whether the extension comes with CSS * or an extension options object. - * @param {string=} opt_noTypeCheck Whether not to check types. - * No new extension must pass this. - * @param {!Array=} opt_extraGlobs */ -function declareExtension(name, version, hasCssOrOptions, opt_noTypeCheck, - opt_extraGlobs) { +function declareExtension(name, version, hasCssOrOptions) { var hasCss = false; var options = {}; if (typeof hasCssOrOptions == 'boolean') { @@ -161,9 +157,6 @@ function declareExtension(name, version, hasCssOrOptions, opt_noTypeCheck, name: name, version: version, hasCss: hasCss, - // Only grandfathered for access - noTypeCheck: (!!opt_noTypeCheck && /access/.test(name)), - extraGlobs: opt_extraGlobs, }, options); } diff --git a/src/service/url-replacements-impl.js b/src/service/url-replacements-impl.js index 7b74b7b51aa1e..ab7ac56d30faf 100644 --- a/src/service/url-replacements-impl.js +++ b/src/service/url-replacements-impl.js @@ -74,7 +74,8 @@ export class GlobalVariableSource extends VariableSource { /** * @private - * @const {function(!./ampdoc-impl.AmpDoc):!Promise} + * @const {function(!./ampdoc-impl.AmpDoc): + * !Promise} */ this.getAccessService_ = accessServiceForDocOrNull; @@ -476,7 +477,8 @@ export class GlobalVariableSource extends VariableSource { /** * Resolves the value via access service. If access service is not configured, * the resulting value is `null`. - * @param {function(!AccessService):(T|!Promise)} getter + * @param {function(!../../extensions/amp-access/0.1/amp-access.AccessService + * ):(T|!Promise)} getter * @param {string} expr * @return {T|null} * @template T diff --git a/src/services.js b/src/services.js index 3718ab563cb05..f227cfb3c19f4 100644 --- a/src/services.js +++ b/src/services.js @@ -33,22 +33,24 @@ import { /** * Returns a promise for the Access service. * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc - * @return {!Promise} + * @return {!Promise} */ export function accessServiceForDoc(nodeOrDoc) { - return /** @type {!Promise} */ ( - getElementServiceForDoc(nodeOrDoc, 'access', 'amp-access')); + return (/** @type {!Promise< + !../extensions/amp-access/0.1/amp-access.AccessService>} */ ( + getElementServiceForDoc(nodeOrDoc, 'access', 'amp-access'))); } /** * Returns a promise for the Access service or a promise for null if the service * is not available on the current page. * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc - * @return {!Promise} + * @return {!Promise} */ export function accessServiceForDocOrNull(nodeOrDoc) { - return /** @type {!Promise} */ ( - getElementServiceIfAvailableForDoc(nodeOrDoc, 'access', 'amp-access')); + return (/** @type { + !Promise} */ ( + getElementServiceIfAvailableForDoc(nodeOrDoc, 'access', 'amp-access'))); } /**