Skip to content

Commit

Permalink
Move real time config manager to service directory
Browse files Browse the repository at this point in the history
  • Loading branch information
powerivq committed Nov 3, 2020
1 parent 4591a44 commit b4c7bdb
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 38 deletions.
2 changes: 1 addition & 1 deletion build-system/externs/amp.extern.js
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ AMP.RealTimeConfigManager;

/**
* Actual filled values for this exists in
* extensions/amp-a4a/0.1/real-time-config-manager.js
* src/service/real-time-config/real-time-config-impl.js
* @enum {string}
*/
const RTC_ERROR_ENUM = {};
Expand Down
2 changes: 1 addition & 1 deletion build-system/tasks/presubmit-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,6 @@ const forbiddenTerms = {
'extensions/amp-a4a/0.1/test/test-a4a-var-source.js',
'extensions/amp-a4a/0.1/test/test-amp-a4a.js',
'extensions/amp-a4a/0.1/test/test-amp-ad-utils.js',
'extensions/amp-a4a/0.1/test/test-callout-vendors.js',
'extensions/amp-a4a/0.1/test/test-refresh.js',
'extensions/amp-access/0.1/test/test-access-expr.js',
'extensions/amp-access/0.1/test/test-amp-login-done-dialog.js',
Expand Down Expand Up @@ -831,6 +830,7 @@ const forbiddenTerms = {
'test/unit/test-amp-inabox.js',
'test/unit/test-animation.js',
'test/unit/test-batched-json.js',
'test/unit/test-callout-vendors.js',
'test/unit/test-chunk.js',
'test/unit/test-cid.js',
'test/unit/test-css.js',
Expand Down
5 changes: 3 additions & 2 deletions build-system/test-configs/dep-check-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ exports.rules = [
'extensions/amp-ad-custom/0.1/amp-ad-custom.js->extensions/amp-a4a/0.1/template-validator.js',
'extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js->extensions/amp-a4a/0.1/amp-ad-template-helper.js',
'extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js->extensions/amp-a4a/0.1/amp-ad-type-defs.js',
'extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js->extensions/amp-a4a/0.1/callout-vendors.js',
'extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js->extensions/amp-a4a/0.1/real-time-config-manager.js',
'extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js->extensions/amp-a4a/0.1/refresh-manager.js',

// AMP access depends on AMP access
Expand Down Expand Up @@ -397,6 +395,9 @@ exports.rules = [
// Experiment moving Fixed Layer to extension
'extensions/amp-viewer-integration/0.1/amp-viewer-integration.js->' +
'src/service/fixed-layer.js',
// Ads remote config manager
'extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js->src/service/real-time-config/callout-vendors.js',
'extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js->src/service/real-time-config/real-time-config-impl.js',
],
},
{
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-a4a/0.1/test/test-amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {FetchMock, networkFailure} from './fetch-mock';
import {FriendlyIframeEmbed} from '../../../../src/friendly-iframe-embed';
import {LayoutPriority} from '../../../../src/layout';
import {MockA4AImpl, TEST_URL} from './utils';
import {RealTimeConfigManager} from '../real-time-config-manager';
import {RealTimeConfigManager} from '../../../../src/service/real-time-config/real-time-config-impl';
import {Services} from '../../../../src/services';
import {Signals} from '../../../../src/utils/signals';
import {cancellation} from '../../../../src/error';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// Most other ad networks will want to put their A4A code entirely in the
// extensions/amp-ad-network-${NETWORK_NAME}-impl directory.

import '../../amp-a4a/0.1/real-time-config-manager';
import '../../../src/service/real-time-config/real-time-config-impl';
import {
AmpA4A,
ConsentTupleDef,
Expand Down Expand Up @@ -56,7 +56,7 @@ import {
} from './flexible-ad-slot-utils';
import {Layout, isLayoutSizeDefined} from '../../../src/layout';
import {Navigation} from '../../../src/service/navigation';
import {RTC_VENDORS} from '../../amp-a4a/0.1/callout-vendors';
import {RTC_VENDORS} from '../../../src/service/real-time-config/callout-vendors';
import {
RefreshManager, // eslint-disable-line no-unused-vars
getRefreshManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@
// AmpAd is not loaded already, so we need to load it separately.
import '../../../amp-ad/0.1/amp-ad';
import {AmpAdNetworkDoubleclickImpl} from '../amp-ad-network-doubleclick-impl';
import {RTC_ERROR_ENUM} from '../../../amp-a4a/0.1/real-time-config-manager';
import {RTC_VENDORS} from '../../../amp-a4a/0.1/callout-vendors';
import {RTC_ERROR_ENUM} from '../../../../src/service/real-time-config/real-time-config-impl';
import {RTC_VENDORS} from '../../../../src/service/real-time-config/callout-vendors';
import {Services} from '../../../../src/services';
import {createElementWithAttributes} from '../../../../src/dom';

describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, (env) => {
let impl;
let element;
let rtcVendors;

beforeEach(() => {
env.win.__AMP_MODE.test = true;
Expand All @@ -47,10 +48,12 @@ describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, (env) => {
});
impl = new AmpAdNetworkDoubleclickImpl(element, env.win.document, env.win);
impl.populateAdUrlState();
rtcVendors = RTC_VENDORS;
});

afterEach(() => {
impl = null;
RTC_VENDORS = rtcVendors;
});

describe('#mergeRtcResponses_', () => {
Expand Down
10 changes: 10 additions & 0 deletions src/service/real-time-config/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// For an explanation of the OWNERS rules and syntax, see:
// https://github.com/ampproject/amp-github-apps/blob/master/owners/OWNERS.example

{
rules: [
{
owners: [{name: 'ampproject/wg-ads-reviewers'}],
},
],
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {getMode} from '../../../src/mode';
import {jsonConfiguration} from '../../../src/json';
import {getMode} from '../../mode';
import {jsonConfiguration} from '../../json';

//////////////////////////////////////////////////////////////////
// //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {CONSENT_POLICY_STATE} from '../../../src/consent-state';
import {CONSENT_POLICY_STATE} from '../../consent-state';
import {RTC_VENDORS} from './callout-vendors';
import {Services} from '../../../src/services';
import {dev, user, userAssert} from '../../../src/log';
import {getMode} from '../../../src/mode';
import {isArray, isObject} from '../../../src/types';
import {isCancellation} from '../../../src/error';
import {tryParseJson} from '../../../src/json';
import {Services} from '../../services';
import {dev, user, userAssert} from '../../log';
import {getMode} from '../../mode';
import {isArray, isObject} from '../../types';
import {isCancellation} from '../../error';
import {tryParseJson} from '../../json';

/** @type {string} */
const TAG = 'real-time-config';
Expand All @@ -31,10 +31,6 @@ const MAX_RTC_CALLOUTS = 5;
/** @type {number} */
const MAX_URL_LENGTH = 16384;

/** @type {boolean} */
const ERROR_REPORTING_ENABLED =
getMode(window).localDev || getMode(window).test || Math.random() < 0.01;

/** @typedef {{
urls: (undefined|Array<string>|
Array<{url:string, errorReportingUrl:string,
Expand Down Expand Up @@ -77,10 +73,10 @@ export const RTC_ERROR_ENUM = {

export class RealTimeConfigManager {
/**
* @param {!./amp-a4a.AmpA4A} a4aElement
* @param {!../../../extensions/amp-a4a/0.1/amp-a4a.AmpA4A} a4aElement
*/
constructor(a4aElement) {
/** @private {!./amp-a4a.AmpA4A} */
/** @private {!../../../extensions/amp-a4a/0.1/amp-a4a.AmpA4A} */
this.a4aElement_ = a4aElement;

/** @private {!Window} */
Expand Down Expand Up @@ -134,7 +130,11 @@ export class RealTimeConfigManager {
* @param {string} errorReportingUrl
*/
sendErrorMessage(errorType, errorReportingUrl) {
if (!ERROR_REPORTING_ENABLED) {
if (
!getMode(this.win_).localDev &&
!getMode(this.win_).test &&
Math.random() >= 0.01
) {
return;
}
const allowlist = {ERROR_TYPE: true, HREF: true};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* limitations under the License.
*/

import {RTC_VENDORS} from '../callout-vendors';
import {isSecureUrlDeprecated} from '../../../../src/url';
import {RTC_VENDORS} from '../../src/service/real-time-config/callout-vendors';
import {isSecureUrlDeprecated} from '../../src/url';

// The keys of RTC_VENDORS are not allowed to have any capital letters.
// This test acts as a presubmit to enforce that.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,20 @@
// Fast Fetch impls are always loaded via an AmpAd tag, which means AmpAd is
// always available for them. However, when we test an impl in isolation,
// AmpAd is not loaded already, so we need to load it separately.
import '../../../amp-ad/0.1/amp-ad';
import {AmpA4A} from '../amp-a4a';
import {CONSENT_POLICY_STATE} from '../../../../src/consent-state';
import '../../extensions/amp-ad/0.1/amp-ad';
import {AmpA4A} from '../../extensions/amp-a4a/0.1/amp-a4a';
import {CONSENT_POLICY_STATE} from '../../src/consent-state';
import {
RTC_ERROR_ENUM,
RealTimeConfigManager,
} from '../real-time-config-manager';
import {Services} from '../../../../src/services';
import {Xhr} from '../../../../src/service/xhr-impl';
import {createElementWithAttributes} from '../../../../src/dom';
import {dev, user} from '../../../../src/log';
import {isFiniteNumber} from '../../../../src/types';

describes.realWin('real-time-config-manager', {amp: true}, (env) => {
} from '../../src/service/real-time-config/real-time-config-impl';
import {Services} from '../../src/services';
import {Xhr} from '../../src/service/xhr-impl';
import {createElementWithAttributes} from '../../src/dom';
import {dev, user} from '../../src/log';
import {isFiniteNumber} from '../../src/types';

describes.realWin('real-time-config service', {amp: true}, (env) => {
let element;
let a4aElement;
let fetchJsonStub;
Expand Down

0 comments on commit b4c7bdb

Please sign in to comment.