Skip to content
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.

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

♻️ Move real time config manager to service directory #30966

Merged
merged 1 commit into from
Nov 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,8 +20,8 @@
// 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';

Expand Down Expand Up @@ -152,6 +152,7 @@ describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, (env) => {
expectedParams,
expectedJsonTargeting
);
delete RTC_VENDORS['TEMP_VENDOR'];
});

it('should properly merge into existing json', () => {
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
calebcordry marked this conversation as resolved.
Show resolved Hide resolved
) {
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