Updates expected Adzerk URL format. (ampproject#13106)

* Updated expected Adzerk URL format. * Moving away from using src attr on amp-ad. * Using data-r. * Test update. * Removing unused src attr from test. * Encoding json object. * Encoding quotes. * Test fix * Removing external quotes and added one more test.
gzgogo · Jan 30, 2018 · ac71b31 · ac71b31
1 parent 3febd67
commit ac71b31
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 27 deletions.
diff --git a/examples/adzerk.amp.html b/examples/adzerk.amp.html
@@ -11,13 +11,27 @@
   <script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.1.js"></script>
 </head>
 <body>
+  <!-- Note that the below data-r attributes are not formatted for production.
+       They will only work in local dev mode. --->
+  <amp-ad width=300 height=250
+      type="adzerk"
+      data-r="0">
+    <div placeholder></div>
+    <div fallback></div>
+  </amp-ad>
+
+  <amp-ad width=300 height=250
+      type="adzerk"
+      data-r="101">
+    <div placeholder></div>
+    <div fallback></div>
+  </amp-ad>
 
   <amp-ad width=300 height=250
       type="adzerk"
-      src="https://adzerk.com?id=0">
+      data-r="102">
     <div placeholder></div>
     <div fallback></div>
   </amp-ad>
-
 </body>
 </html>
diff --git a/extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js b/extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js
@@ -93,16 +93,13 @@ export class AmpAdNetworkAdzerkImpl extends AmpA4A {
 
   /** @override */
   getAdUrl() {
-    const src = this.element.getAttribute('src');
-    if (!/^https:\/\/adzerk.com\?id=\d+$/i.test(src)) {
-      return '';
-    }
+    const data = this.element.getAttribute('data-r');
+    dev().assert(data, 'Expected data-r attribte on amp-ad tag');
     if (getMode(this.win).localDev) {
       return `http://ads.localhost:${this.win.location.port}` +
-        '/adzerk/' + /^https:\/\/adzerk.com\?id=(\d+)/.exec(src)[1];
+          '/adzerk/' + data;
     }
-    // TODO(adzerk): specify expected src path.
-    return /^https:\/\/adzerk.com\?id=\d+$/i.test(src) ? src : '';
+    return `https://engine.adzerk.net/amp?r=${encodeURIComponent(data)}`;
   }
 
   /** @override */

diff --git a/extensions/amp-ad-network-adzerk-impl/0.1/test/test-amp-ad-network-adzerk-impl.js b/extensions/amp-ad-network-adzerk-impl/0.1/test/test-amp-ad-network-adzerk-impl.js
@@ -41,7 +41,6 @@ describes.fakeWin('amp-ad-network-adzerk-impl', {amp: true}, env => {
     fetchTextMock = sandbox.stub(Xhr.prototype, 'fetchText');
     element = createElementWithAttributes(doc, 'amp-ad', {
       'type': 'adzerk',
-      'src': 'https://adzerk.com?id=1234',
       'width': '320',
       'height': '50',
     });
@@ -51,25 +50,23 @@ describes.fakeWin('amp-ad-network-adzerk-impl', {amp: true}, env => {
 
   describe('#getAdUrl', () => {
     it('should be valid', () => {
-      ['https://adzerk.com?id=1234',
-        'https://aDzErK.com?id=1234',
-        'https://adzerk.com?id=9'].forEach(src => {
-        element.setAttribute('src', src);
-        expect(impl.isValidElement()).to.be.true;
-        expect(impl.getAdUrl()).to.equal(src);
-      });
+      const r = '{"p":[{"n":1234,"t":[5],"s":677496}]}';
+      element.setAttribute('data-r', r);
+      expect(impl.getAdUrl()).to.equal(
+          `https://engine.adzerk.net/amp?r=${encodeURIComponent(r)}`);
     });
 
-    it('should not be valid', () => {
-      ['http://adzerk.com?id=1234',
-        'https://adzerk.com?id=a',
-        'https://www.adzerk.com?id=1234',
-        'https://adzerk.com?id=1234&a=b',
-        'foohttps://adzer.com?id=1234'].forEach(src => {
-        element.setAttribute('src', src);
-        expect(impl.isValidElement()).to.be.false;
-        expect(impl.getAdUrl()).to.equal('');
-      });
+    it('should be valid #2', () => {
+      element.setAttribute('data-r',
+          '{"p":[{"t":[5],"s":333999,"a":5603000}]}');
+      expect(impl.getAdUrl()).to.equal(
+          'https://engine.adzerk.net' +
+          '/amp?r=%7B%22p%22%3A%5B%7B%22t%22%3A%5B5%5D%2C%22s%22%3A333999' +
+          '%2C%22a%22%3A5603000%7D%5D%7D');
+    });
+
+    it('should be invalid', () => {
+      expect(() => impl.getAdUrl()).to.throw(/Expected data-r attribte/);
     });
   });