From 66b4a5ae11b7e70608e7b6c3d31c598e49578cf4 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 28 Feb 2019 22:14:04 -0600 Subject: [PATCH 01/15] partial progress on wrapper support --- src/lib/vast.js | 10 ++++++++-- test/cypress/support/index.js | 3 --- test/vast.spec.js | 1 - 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/lib/vast.js b/src/lib/vast.js index 5440618..cf71dc9 100644 --- a/src/lib/vast.js +++ b/src/lib/vast.js @@ -3,6 +3,7 @@ import Clickthrough from './vast_elements/clickthrough'; import Impression from './vast_elements/impression'; import ErrorImpression from './vast_elements/error_impression'; import TrackingEvents from './vast_elements/tracking_events'; +import WrapperUrl from './vast_elements/wrapper_url'; import StreamChooser from './stream_chooser'; import VideoElementApplication from './applications/video_element'; @@ -24,6 +25,7 @@ export default class Vast { Impression: new Impression(this), ErrorImpression: new ErrorImpression(this), TrackingEvents: new TrackingEvents(this), + WrapperUrl: new WrapperUrl(this), }; if (xml) { @@ -57,6 +59,10 @@ export default class Vast { return this.loadedElements['Clickthrough'].openClickthroughUrl(); } + wrapperUrl() { + return this.loadedElements['WrapperUrl'].wrapperUrl(); + } + impressionUrls() { return this.loadedElements['Impression'].impressionUrls(); } @@ -127,7 +133,7 @@ export default class Vast { if (this.vastDocument.documentElement.nodeName == 'parsererror') { throw new VastXMLParsingError(`Error parsing ${this.vastXml}. Not valid XML`); } - this.processAllElements(); + this.processElements(); } } @@ -164,7 +170,7 @@ export default class Vast { }); } - processAllElements() { + processElements() { Object.values(this.loadedElements).forEach(e => e.process()); } } diff --git a/test/cypress/support/index.js b/test/cypress/support/index.js index 37a498f..3d469a6 100644 --- a/test/cypress/support/index.js +++ b/test/cypress/support/index.js @@ -15,6 +15,3 @@ // Import commands.js using ES2015 syntax: import './commands'; - -// Alternatively you can use CommonJS syntax: -// require('./commands') diff --git a/test/vast.spec.js b/test/vast.spec.js index 53f7699..a45ac6d 100644 --- a/test/vast.spec.js +++ b/test/vast.spec.js @@ -126,7 +126,6 @@ const mockXhr = (eventToFire = 'load', response = '', eventDelay = 100) => { responseText: response, response: response, addEventListener: jest.fn((eventName, func) => { - // console.log('adding fake listener for', eventName, 'stimulating', eventToFire) if (eventName == eventToFire) { setTimeout(() => { func({ message: eventToFire }); From fba15798186e2c38b567b163cbaa938b2633b757 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 28 Feb 2019 22:15:50 -0600 Subject: [PATCH 02/15] more work on wrappers --- src/lib/vast_elements/wrapper_url.js | 25 ++++++++++++++ test/fixtures/vast-progress.xml | 47 ++++++++++++++++++++++++++ test/fixtures/vast-wrapper.xml | 21 ++++++++++++ test/vast_elements/wrapper_url.spec.js | 22 ++++++++++++ 4 files changed, 115 insertions(+) create mode 100644 src/lib/vast_elements/wrapper_url.js create mode 100644 test/fixtures/vast-progress.xml create mode 100644 test/fixtures/vast-wrapper.xml create mode 100644 test/vast_elements/wrapper_url.spec.js diff --git a/src/lib/vast_elements/wrapper_url.js b/src/lib/vast_elements/wrapper_url.js new file mode 100644 index 0000000..b781ed3 --- /dev/null +++ b/src/lib/vast_elements/wrapper_url.js @@ -0,0 +1,25 @@ +import VastElementBase from './vast_element_base'; + +export default class WrapperUrl extends VastElementBase { + setup() { + this.url = null; + } + + static selector() { + return 'Ad Wrapper VASTAdTagURI'; + } + + onVastReady() { + this.url = this.elements.map(el => { + return el.childNodes[0].nodeValue; + })[0]; + } + + hasWrapperUrl() { + return !!this.wrapperUrl(); + } + + wrapperUrl() { + return this.url; + } +} diff --git a/test/fixtures/vast-progress.xml b/test/fixtures/vast-progress.xml new file mode 100644 index 0000000..6484cad --- /dev/null +++ b/test/fixtures/vast-progress.xml @@ -0,0 +1,47 @@ + + + + iabtechlab + iabtechlab video ad + + + + http://example.com/error + http://example.com/track/impression + + + + 00:00:16 + + http://example.com/tracking/start + http://example.com/tracking/firstQuartile + http://example.com/tracking/midpoint + http://example.com/tracking/thirdQuartile + http://example.com/tracking/complete + http://example.com/tracking/progress-10 + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/fixtures/vast-wrapper.xml b/test/fixtures/vast-wrapper.xml new file mode 100644 index 0000000..c8227a4 --- /dev/null +++ b/test/fixtures/vast-wrapper.xml @@ -0,0 +1,21 @@ + + + + + iabtechlab + + http://example.com/error + http://example.com/track/impression + + + + + + + + + + + + + diff --git a/test/vast_elements/wrapper_url.spec.js b/test/vast_elements/wrapper_url.spec.js new file mode 100644 index 0000000..37b71a1 --- /dev/null +++ b/test/vast_elements/wrapper_url.spec.js @@ -0,0 +1,22 @@ +import Vast from '../../src/lib/vast'; +import * as fs from 'fs'; + +describe('AdTagURI extension', () => { + let xmlString; + let vast; + + beforeAll(() => { + xmlString = fs.readFileSync('./test/fixtures/vast-wrapper.xml'); + vast = new Vast({ xml: xmlString }); + }); + + it('should find wrapper urls in vast tags', () => { + expect(typeof vast.wrapperUrl).toBe('function'); + }); + + it('should get the wrapper urls in vast tags', () => { + expect(vast.wrapperUrl()).toBe( + 'https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%203.0%20Samples/Inline_Companion_Tag-test.xml' + ); + }); +}); From 260eb0aedd2803cd7dc82df2bccfac8b53aa0a3d Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 1 Mar 2019 00:06:34 -0600 Subject: [PATCH 03/15] better node value support for white space --- src/lib/node_value.js | 8 ++++++++ src/lib/vast_elements/clickthrough.js | 5 ++--- src/lib/vast_elements/impression.js | 5 ++--- src/lib/vast_elements/media_files.js | 3 ++- src/lib/vast_elements/tracking_events.js | 3 ++- 5 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 src/lib/node_value.js diff --git a/src/lib/node_value.js b/src/lib/node_value.js new file mode 100644 index 0000000..029508d --- /dev/null +++ b/src/lib/node_value.js @@ -0,0 +1,8 @@ +export default class NodeValue { + static fromElement(el) { + const matchedItem = Array.from(el.childNodes).find(n => { + return (n.nodeType == Node.TEXT_NODE || n.nodeType == Node.CDATA_SECTION_NODE) && !!n.nodeValue.trim(); + }); + return matchedItem ? matchedItem.nodeValue : null; + } +} diff --git a/src/lib/vast_elements/clickthrough.js b/src/lib/vast_elements/clickthrough.js index 79f4ab8..9062cee 100644 --- a/src/lib/vast_elements/clickthrough.js +++ b/src/lib/vast_elements/clickthrough.js @@ -1,4 +1,5 @@ import VastElementBase from './vast_element_base'; +import NodeValue from '../node_value'; export default class Clickthrough extends VastElementBase { setup() { @@ -10,9 +11,7 @@ export default class Clickthrough extends VastElementBase { } onVastReady() { - this.clickthrough = this.elements.map(el => { - return el.childNodes[0].nodeValue; - })[0]; + this.clickthrough = this.elements.map(el => NodeValue.fromElement(el))[0]; } clickthroughUrl() { diff --git a/src/lib/vast_elements/impression.js b/src/lib/vast_elements/impression.js index f2b4571..f5ee41b 100644 --- a/src/lib/vast_elements/impression.js +++ b/src/lib/vast_elements/impression.js @@ -1,4 +1,5 @@ import VastElementBase from './vast_element_base'; +import NodeValue from '../node_value'; export default class Impression extends VastElementBase { setup() { @@ -10,9 +11,7 @@ export default class Impression extends VastElementBase { } onVastReady() { - this._impressionUrls = this.elements.map(el => { - return el.childNodes[0].nodeValue; - }); + this._impressionUrls = this.elements.map(el => NodeValue.fromElement(el)); } impressionUrls() { diff --git a/src/lib/vast_elements/media_files.js b/src/lib/vast_elements/media_files.js index 40412cd..f88e586 100644 --- a/src/lib/vast_elements/media_files.js +++ b/src/lib/vast_elements/media_files.js @@ -1,4 +1,5 @@ import VastElementBase from './vast_element_base'; +import NodeValue from '../node_value'; class MediaFile { constructor(mediaElement) { @@ -22,7 +23,7 @@ class MediaFile { } url() { - return this.element.childNodes[0].nodeValue; + return NodeValue.fromElement(this.element); } codec() { diff --git a/src/lib/vast_elements/tracking_events.js b/src/lib/vast_elements/tracking_events.js index f920ed6..c52b431 100644 --- a/src/lib/vast_elements/tracking_events.js +++ b/src/lib/vast_elements/tracking_events.js @@ -1,4 +1,5 @@ import VastElementBase from './vast_element_base'; +import NodeValue from '../node_value'; export default class TrackingEvents extends VastElementBase { setup() { @@ -11,7 +12,7 @@ export default class TrackingEvents extends VastElementBase { onVastReady() { this.trackingUrls = this.elements.map(el => { - return [el.getAttribute('event'), el.childNodes[0].nodeValue]; + return [el.getAttribute('event'), NodeValue.fromElement(el)]; }); } From dcaf39435ab9d1173b8f10774a9873ff064bc31e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 1 Mar 2019 00:07:58 -0600 Subject: [PATCH 04/15] async parsing and recursive loading --- src/lib/vast.js | 20 ++++++++++++++------ src/lib/vast_elements/vast_element_base.js | 2 +- src/lib/vast_elements/wrapper_url.js | 7 ++----- test/fixtures/vast-wrapper.xml | 2 +- test/vast.spec.js | 15 ++++++++++----- test/vast_elements/wrapper_url.spec.js | 16 ++++++++++++++-- 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/lib/vast.js b/src/lib/vast.js index cf71dc9..5bc3a12 100644 --- a/src/lib/vast.js +++ b/src/lib/vast.js @@ -18,6 +18,7 @@ export default class Vast { this.vastUrl = null; this.vastDocument = null; this.bandwidthEstimateInKbs = 0; + this.wrapperFollowsRemaining = 5; this.loadedElements = { MediaFiles: new MediaFiles(this), @@ -33,10 +34,10 @@ export default class Vast { } } - useXmlString(xml) { + async useXmlString(xml) { this.vastXml = xml; this.vastDocument = null; - this.parse(); + await this.parse(); } bandwidth() { @@ -126,14 +127,15 @@ export default class Vast { return chooser.bestVideo(); } - parse() { + async parse() { if (!this.vastDocument) { const parser = new DOMParser(); this.vastDocument = parser.parseFromString(this.vastXml, 'application/xml'); if (this.vastDocument.documentElement.nodeName == 'parsererror') { throw new VastXMLParsingError(`Error parsing ${this.vastXml}. Not valid XML`); + } else { + await this.processElements(); } - this.processElements(); } } @@ -144,12 +146,13 @@ export default class Vast { request.timeout = timeout; let startTime; - request.addEventListener('load', e => { + request.addEventListener('load', async e => { const downloadTime = new Date().getTime() - startTime; const downloadSize = request.responseText.length; this.bandwidthEstimateInKbs = (downloadSize * 8) / (downloadTime / 1000) / 1024; this.useXmlString(request.response); + await this.parse(); resolve(); }); @@ -164,13 +167,18 @@ export default class Vast { request.addEventListener('timeout', e => { reject(new VastNetworkError(`Network Timeout: Request did not complete in ${timeout}ms`)); }); + startTime = new Date().getTime(); request.open('GET', this.vastUrl); request.send(); }); } - processElements() { + async processElements() { Object.values(this.loadedElements).forEach(e => e.process()); + if (this.wrapperUrl() && this.wrapperFollowsRemaining-- >= 0) { + await this.loadRemoteVast(this.wrapperUrl()); + } + // TODO: throw VastNetworkError if wrapperFollowsRemaining is below 0 } } diff --git a/src/lib/vast_elements/vast_element_base.js b/src/lib/vast_elements/vast_element_base.js index 8f283fa..92f1d0b 100644 --- a/src/lib/vast_elements/vast_element_base.js +++ b/src/lib/vast_elements/vast_element_base.js @@ -20,7 +20,7 @@ export default class VastElementBase { const selector = this.constructor.selector(); - this.elements = Array.from(this.vast.vastDocument.querySelectorAll(selector)); + this.elements = this.elements.concat(Array.from(this.vast.vastDocument.querySelectorAll(selector))); this.onVastReady(); } diff --git a/src/lib/vast_elements/wrapper_url.js b/src/lib/vast_elements/wrapper_url.js index b781ed3..53db9e7 100644 --- a/src/lib/vast_elements/wrapper_url.js +++ b/src/lib/vast_elements/wrapper_url.js @@ -1,4 +1,5 @@ import VastElementBase from './vast_element_base'; +import NodeValue from '../node_value'; export default class WrapperUrl extends VastElementBase { setup() { @@ -11,14 +12,10 @@ export default class WrapperUrl extends VastElementBase { onVastReady() { this.url = this.elements.map(el => { - return el.childNodes[0].nodeValue; + return NodeValue.fromElement(el); })[0]; } - hasWrapperUrl() { - return !!this.wrapperUrl(); - } - wrapperUrl() { return this.url; } diff --git a/test/fixtures/vast-wrapper.xml b/test/fixtures/vast-wrapper.xml index c8227a4..0e67bab 100644 --- a/test/fixtures/vast-wrapper.xml +++ b/test/fixtures/vast-wrapper.xml @@ -5,7 +5,7 @@ iabtechlab http://example.com/error - http://example.com/track/impression + http://example.com/track/impression-parent diff --git a/test/vast.spec.js b/test/vast.spec.js index a45ac6d..1e3c024 100644 --- a/test/vast.spec.js +++ b/test/vast.spec.js @@ -22,13 +22,18 @@ describe('Internal Error Handling', () => { vast = new Vast({ xml: fs.readFileSync('./test/fixtures/vast.xml') }); }); - it('should pass an XML parse error to the callback', () => { - expect(() => { - vast.useXmlString('not real xml'); - }).toThrow(VastXMLParsingError); + it('should pass an XML parse error to the callback', async () => { let caughtError; try { - vast.useXmlString('not real xml'); + await vast.useXmlString('not real xml'); + } catch (error) { + caughtError = error; + } + expect(caughtError.constructor).toBe(VastXMLParsingError); + + caughtError = null; + try { + await vast.useXmlString('not real xml'); } catch (error) { caughtError = error; } diff --git a/test/vast_elements/wrapper_url.spec.js b/test/vast_elements/wrapper_url.spec.js index 37b71a1..ad33c2a 100644 --- a/test/vast_elements/wrapper_url.spec.js +++ b/test/vast_elements/wrapper_url.spec.js @@ -5,9 +5,10 @@ describe('AdTagURI extension', () => { let xmlString; let vast; - beforeAll(() => { + beforeAll(async () => { xmlString = fs.readFileSync('./test/fixtures/vast-wrapper.xml'); - vast = new Vast({ xml: xmlString }); + vast = new Vast(); + await vast.useXmlString(xmlString); }); it('should find wrapper urls in vast tags', () => { @@ -19,4 +20,15 @@ describe('AdTagURI extension', () => { 'https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%203.0%20Samples/Inline_Companion_Tag-test.xml' ); }); + + it('should load up an impression url from this first and followed vast tag', () => { + expect(vast.impressionUrls()).toContain('http://example.com/track/impression'); + expect(vast.impressionUrls()).toContain('http://example.com/track/impression-parent'); + }); + + it('should use the creatives from the final location', () => { + expect(vast.videos().map(v => v.url())).toContain( + 'https://iab-publicfiles.s3.amazonaws.com/vast/VAST-4.0-Short-Intro.mp4' + ); + }); }); From c3578349dfa8bbe4d2e7e7672407cd25ff5c383e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 1 Mar 2019 09:04:55 -0600 Subject: [PATCH 05/15] adding in failing test for node value --- test/node_value.spec.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 test/node_value.spec.js diff --git a/test/node_value.spec.js b/test/node_value.spec.js new file mode 100644 index 0000000..4c13cbf --- /dev/null +++ b/test/node_value.spec.js @@ -0,0 +1,23 @@ +import NodeValue from '../src/lib/node_value'; + +describe('Handling white space without breaking a sweat', () => { + it('should get values with no white space', () => { + const xml = '123'; + const parser = new DOMParser(); + const doc = parser.parseFromString(xml, 'application/xml'); + expect(doc.querySelectorAll('important')).toHaveLength(1); + expect(NodeValue.fromElement(doc.querySelector('important'))).toBe('123'); + }); + + it('should handle leading edge white spaces too', () => { + const el = parsedXmlElementWithValue('\n\n12345'); + expect(NodeValue.fromElement(el)).toBe('12345'); + }); +}); + +const parsedXmlElementWithValue = (value = 123) => { + const xml = `${value}`; + const parser = new DOMParser(); + const doc = parser.parseFromString(xml, 'application/xml'); + return doc.querySelector('important'); +}; From 1f2b3c4c0dc18b51743111b5922d4fe42a08100b Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 1 Mar 2019 10:59:56 -0600 Subject: [PATCH 06/15] node value tests are passing --- src/lib/node_value.js | 2 +- test/node_value.spec.js | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/lib/node_value.js b/src/lib/node_value.js index 029508d..c7f6a56 100644 --- a/src/lib/node_value.js +++ b/src/lib/node_value.js @@ -3,6 +3,6 @@ export default class NodeValue { const matchedItem = Array.from(el.childNodes).find(n => { return (n.nodeType == Node.TEXT_NODE || n.nodeType == Node.CDATA_SECTION_NODE) && !!n.nodeValue.trim(); }); - return matchedItem ? matchedItem.nodeValue : null; + return matchedItem ? matchedItem.nodeValue.trim() : null; } } diff --git a/test/node_value.spec.js b/test/node_value.spec.js index 4c13cbf..ae900c6 100644 --- a/test/node_value.spec.js +++ b/test/node_value.spec.js @@ -15,6 +15,18 @@ describe('Handling white space without breaking a sweat', () => { }); }); +describe('Get only the value from the node', () => { + it('should handle cdata attributes just fine', () => { + const el = parsedXmlElementWithValue(''); + expect(NodeValue.fromElement(el)).toBe('hello winston'); + }); + + it('should ignore comment blocks', () => { + const el = parsedXmlElementWithValue('apple sauce'); + expect(NodeValue.fromElement(el)).toBe('apple sauce'); + }); +}); + const parsedXmlElementWithValue = (value = 123) => { const xml = `${value}`; const parser = new DOMParser(); From 7858b39a94fd7518d531a8983b58d21f934f5a04 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 1 Mar 2019 11:07:58 -0600 Subject: [PATCH 07/15] Adds comment description to method --- src/lib/node_value.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lib/node_value.js b/src/lib/node_value.js index c7f6a56..0e83e40 100644 --- a/src/lib/node_value.js +++ b/src/lib/node_value.js @@ -1,4 +1,9 @@ export default class NodeValue { + /** + * Returns the first TEXT or CDATA value from an XML element. + * + * @param {DOM Element} el An elemenet with a single CDATA or TEXT entity + */ static fromElement(el) { const matchedItem = Array.from(el.childNodes).find(n => { return (n.nodeType == Node.TEXT_NODE || n.nodeType == Node.CDATA_SECTION_NODE) && !!n.nodeValue.trim(); From 6fd280ce1f053d1f02961f8113d840222b29a350 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sun, 3 Mar 2019 07:42:14 -0600 Subject: [PATCH 08/15] struggling to understand async/await error handling --- package.json | 3 +- src/lib/vast.js | 25 +++++++--- test/fixtures/vast-wrapper-loop.xml | 21 ++++++++ test/fixtures/vast-wrapper-next.xml | 69 ++++++++++++++++++++++++++ test/vast_elements/wrapper_url.spec.js | 51 ++++++++++++++++++- yarn.lock | 33 ++++++++++++ 6 files changed, 192 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/vast-wrapper-loop.xml create mode 100644 test/fixtures/vast-wrapper-next.xml diff --git a/package.json b/package.json index 51f861a..b8055da 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,8 @@ "pretty-quick": "^1.10.0", "webpack": "^4.29.3", "webpack-cli": "^3.2.3", - "webpack-dev-server": "^3.2.0" + "webpack-dev-server": "^3.2.0", + "xhr-mock": "^2.4.1" }, "husky": { "hooks": { diff --git a/src/lib/vast.js b/src/lib/vast.js index 5bc3a12..3480df2 100644 --- a/src/lib/vast.js +++ b/src/lib/vast.js @@ -13,12 +13,12 @@ export class VastXMLParsingError extends Error {} export class VastNetworkError extends Error {} export default class Vast { - constructor({ xml } = {}) { + constructor({ xml, numberWrapperFollowsAllowed } = { numberWrapperFollowsAllowed: 5 }) { this.vastXml = null; this.vastUrl = null; this.vastDocument = null; this.bandwidthEstimateInKbs = 0; - this.wrapperFollowsRemaining = 5; + this.wrapperFollowsRemaining = numberWrapperFollowsAllowed; this.loadedElements = { MediaFiles: new MediaFiles(this), @@ -37,7 +37,10 @@ export default class Vast { async useXmlString(xml) { this.vastXml = xml; this.vastDocument = null; - await this.parse(); + await this.parse().catch(e => { + console.log('xml rethrow'); + throw e; + }); } bandwidth() { @@ -134,7 +137,10 @@ export default class Vast { if (this.vastDocument.documentElement.nodeName == 'parsererror') { throw new VastXMLParsingError(`Error parsing ${this.vastXml}. Not valid XML`); } else { - await this.processElements(); + await this.processElements().catch(e => { + console.log('catching and re throwing'); + throw e; + }); } } } @@ -176,9 +182,14 @@ export default class Vast { async processElements() { Object.values(this.loadedElements).forEach(e => e.process()); - if (this.wrapperUrl() && this.wrapperFollowsRemaining-- >= 0) { - await this.loadRemoteVast(this.wrapperUrl()); + + if (this.wrapperUrl()) { + if (this.wrapperFollowsRemaining-- > 0) { + await this.loadRemoteVast(this.wrapperUrl()); + } else { + console.log('throwing error -- out of redirects'); + throw new VastNetworkError('Netwwork Error: Too Many Vast Wrapper Follows'); + } } - // TODO: throw VastNetworkError if wrapperFollowsRemaining is below 0 } } diff --git a/test/fixtures/vast-wrapper-loop.xml b/test/fixtures/vast-wrapper-loop.xml new file mode 100644 index 0000000..0e67bab --- /dev/null +++ b/test/fixtures/vast-wrapper-loop.xml @@ -0,0 +1,21 @@ + + + + + iabtechlab + + http://example.com/error + http://example.com/track/impression-parent + + + + + + + + + + + + + diff --git a/test/fixtures/vast-wrapper-next.xml b/test/fixtures/vast-wrapper-next.xml new file mode 100644 index 0000000..04b903f --- /dev/null +++ b/test/fixtures/vast-wrapper-next.xml @@ -0,0 +1,69 @@ + + + + iabtechlab + + + + + + + + + + + + + http://example.com/error + http://example.com/track/impression + + + + + + + + + + + + + + + + + 00:00:16 + + http://example.com/tracking/start + http://example.com/tracking/firstQuartile + http://example.com/tracking/midpoint + http://example.com/tracking/thirdQuartile + http://example.com/tracking/complete + http://example.com/tracking/progress-10 + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/vast_elements/wrapper_url.spec.js b/test/vast_elements/wrapper_url.spec.js index ad33c2a..867f854 100644 --- a/test/vast_elements/wrapper_url.spec.js +++ b/test/vast_elements/wrapper_url.spec.js @@ -1,7 +1,8 @@ -import Vast from '../../src/lib/vast'; +import Vast, { VastNetworkError } from '../../src/lib/vast'; import * as fs from 'fs'; +import mock from 'xhr-mock'; -describe('AdTagURI extension', () => { +describe('Wrapper extension', () => { let xmlString; let vast; @@ -11,6 +12,22 @@ describe('AdTagURI extension', () => { await vast.useXmlString(xmlString); }); + beforeEach(() => { + mock.setup(); + const followingXml = fs.readFileSync('./test/fixtures/vast-wrapper-next.xml'); + mock.get( + 'https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%203.0%20Samples/Inline_Companion_Tag-test.xml', + { + headers: { + 'Content-Length': followingXml.length, + 'Content-Type': 'application/xml', + }, + body: followingXml, + } + ); + }); + afterEach(() => mock.teardown()); + it('should find wrapper urls in vast tags', () => { expect(typeof vast.wrapperUrl).toBe('function'); }); @@ -32,3 +49,33 @@ describe('AdTagURI extension', () => { ); }); }); + +describe('Wrapper related errors', () => { + beforeEach(() => mock.setup()); + afterEach(() => mock.teardown()); + + it('should throw an error if it traverses/issues too many vast requests', async () => { + // No matter what we request always return another wapper version + const followingXml = fs.readFileSync('./test/fixtures/vast-wrapper.xml'); + mock.get( + 'https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%203.0%20Samples/Inline_Companion_Tag-test.xml', + { + headers: { + 'Content-Length': followingXml.length, + 'Content-Type': 'application/xml', + }, + body: followingXml, + } + ); + + const xmlString = fs.readFileSync('./test/fixtures/vast-wrapper.xml'); + try { + const vast = new Vast(); + await vast.useXmlString(xmlString).catch(boom => { + console.log('caught', boom); + }); + } catch (e) { + console.log('caught upper', e); + } + }); +}); diff --git a/yarn.lock b/yarn.lock index 933e6ef..9373a9d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2400,6 +2400,11 @@ dom-serializer@0: domelementtype "~1.1.1" entities "~1.1.1" +dom-walk@^0.1.0: + version "0.1.1" + resolved "https://registry.yarnpkg.com/dom-walk/-/dom-walk-0.1.1.tgz#672226dc74c8f799ad35307df936aba11acd6018" + integrity sha1-ZyIm3HTI95mtNTB9+TaroRrNYBg= + domain-browser@^1.1.1: version "1.2.0" resolved "https://registry.yarnpkg.com/domain-browser/-/domain-browser-1.2.0.tgz#3d31f50191a6749dd1375a7f522e823d42e54eda" @@ -3290,6 +3295,14 @@ global-prefix@^1.0.1: is-windows "^1.0.1" which "^1.2.14" +global@^4.3.0: + version "4.3.2" + resolved "https://registry.yarnpkg.com/global/-/global-4.3.2.tgz#e76989268a6c74c38908b1305b10fc0e394e9d0f" + integrity sha1-52mJJopsdMOJCLEwWxD8DjlOnQ8= + dependencies: + min-document "^2.19.0" + process "~0.5.1" + globals@^11.1.0, globals@^11.7.0: version "11.11.0" resolved "https://registry.yarnpkg.com/globals/-/globals-11.11.0.tgz#dcf93757fa2de5486fbeed7118538adf789e9c2e" @@ -4972,6 +4985,13 @@ mimic-fn@^1.0.0: resolved "https://registry.yarnpkg.com/mimic-fn/-/mimic-fn-1.2.0.tgz#820c86a39334640e99516928bd03fca88057d022" integrity sha512-jf84uxzwiuiIVKiOLpfYk7N46TSy8ubTonmneY9vrpHNAnp0QBt2BxWV9dO3/j+BoVAb+a5G6YDPW3M5HOdMWQ== +min-document@^2.19.0: + version "2.19.0" + resolved "https://registry.yarnpkg.com/min-document/-/min-document-2.19.0.tgz#7bd282e3f5842ed295bb748cdd9f1ffa2c824685" + integrity sha1-e9KC4/WELtKVu3SM3Z8f+iyCRoU= + dependencies: + dom-walk "^0.1.0" + minimalistic-assert@^1.0.0, minimalistic-assert@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/minimalistic-assert/-/minimalistic-assert-1.0.1.tgz#2e194de044626d4a10e7f7fbc00ce73e83e4d5c7" @@ -5858,6 +5878,11 @@ process@^0.11.10: resolved "https://registry.yarnpkg.com/process/-/process-0.11.10.tgz#7332300e840161bda3e69a1d1d91a7d4bc16f182" integrity sha1-czIwDoQBYb2j5podHZGn1LwW8YI= +process@~0.5.1: + version "0.5.2" + resolved "https://registry.yarnpkg.com/process/-/process-0.5.2.tgz#1638d8a8e34c2f440a91db95ab9aeb677fc185cf" + integrity sha1-FjjYqONML0QKkduVq5rrZ3/Bhc8= + progress@^2.0.0: version "2.0.3" resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.3.tgz#7e8cf8d8f5b8f239c1bc68beb4eb78567d572ef8" @@ -7694,6 +7719,14 @@ ws@^5.2.0: dependencies: async-limiter "~1.0.0" +xhr-mock@^2.4.1: + version "2.4.1" + resolved "https://registry.yarnpkg.com/xhr-mock/-/xhr-mock-2.4.1.tgz#cb502e3d50b8b2ec31bd61766ce516bfc1dd072f" + integrity sha1-y1AuPVC4suwxvWF2bOUWv8HdBy8= + dependencies: + global "^4.3.0" + url "^0.11.0" + xml-name-validator@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/xml-name-validator/-/xml-name-validator-3.0.0.tgz#6ae73e06de4d8c6e47f9fb181f78d648ad457c6a" From bc84c206432ac1a50381182235e9fcf73a9bd4c0 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sun, 3 Mar 2019 09:00:46 -0600 Subject: [PATCH 09/15] handles error redirects and adds error pixels --- src/lib/vast.js | 22 ++++---- src/lib/vast_elements/README.md | 4 +- src/lib/vast_elements/vast_element_base.js | 14 ++++- src/lib/vast_elements/wrapper_url.js | 4 ++ test/fixtures/vast-wrapper-loop.xml | 2 +- test/vast_elements/wrapper_url.spec.js | 60 ++++++++++++++-------- 6 files changed, 68 insertions(+), 38 deletions(-) diff --git a/src/lib/vast.js b/src/lib/vast.js index 3480df2..7145619 100644 --- a/src/lib/vast.js +++ b/src/lib/vast.js @@ -37,10 +37,7 @@ export default class Vast { async useXmlString(xml) { this.vastXml = xml; this.vastDocument = null; - await this.parse().catch(e => { - console.log('xml rethrow'); - throw e; - }); + await this.parse(); } bandwidth() { @@ -67,6 +64,10 @@ export default class Vast { return this.loadedElements['WrapperUrl'].wrapperUrl(); } + url() { + return this.vastUrl; + } + impressionUrls() { return this.loadedElements['Impression'].impressionUrls(); } @@ -137,10 +138,7 @@ export default class Vast { if (this.vastDocument.documentElement.nodeName == 'parsererror') { throw new VastXMLParsingError(`Error parsing ${this.vastXml}. Not valid XML`); } else { - await this.processElements().catch(e => { - console.log('catching and re throwing'); - throw e; - }); + await this.processElements(); } } } @@ -156,9 +154,7 @@ export default class Vast { const downloadTime = new Date().getTime() - startTime; const downloadSize = request.responseText.length; this.bandwidthEstimateInKbs = (downloadSize * 8) / (downloadTime / 1000) / 1024; - - this.useXmlString(request.response); - await this.parse(); + await this.useXmlString(request.response).catch(err => reject(err)); resolve(); }); @@ -187,8 +183,8 @@ export default class Vast { if (this.wrapperFollowsRemaining-- > 0) { await this.loadRemoteVast(this.wrapperUrl()); } else { - console.log('throwing error -- out of redirects'); - throw new VastNetworkError('Netwwork Error: Too Many Vast Wrapper Follows'); + this.addErrorImpressionUrls(); + throw new VastNetworkError('Network Error: Too Many Vast Wrapper Follows'); } } } diff --git a/src/lib/vast_elements/README.md b/src/lib/vast_elements/README.md index d42f7d8..1ca7e77 100644 --- a/src/lib/vast_elements/README.md +++ b/src/lib/vast_elements/README.md @@ -23,10 +23,12 @@ The idea is that once the Vast XML is loaded, each VastElement will query the XM ### Implementing a new VastElement -There are three major methods that the extending class must support +There are four major methods that the extending class must support 1. `static selector()` – This should return a selector as a string, and is used to determine which elements your element class will consume +1. `static appendElementsOnFollow()` – Returns a boolean (defaults to `true`) and controls if following Vast's Wrapper VASTAdTagURI url's should append or reload this Vast Elements' knowledge of the DOM. For most elements this should be true (appending), but in a few cases such as finding a new wrapper url to follow would cause redirect loops. + 1. `setup()` – this will be called once your element class is loaded, use it like a constructor. 1. `onVastReady()` – The vast file has been onVastReady and your selector has been run, you can find your elements loaded in `this.elements` diff --git a/src/lib/vast_elements/vast_element_base.js b/src/lib/vast_elements/vast_element_base.js index 92f1d0b..8143b5e 100644 --- a/src/lib/vast_elements/vast_element_base.js +++ b/src/lib/vast_elements/vast_element_base.js @@ -8,6 +8,14 @@ export default class VastElementBase { // Selector to determine applicable vast elements static selector() {} + // When following Vast redirects (wrappers) should the elements + // be added to the existing list or should they replace the existing elements + // True will append + // False will replace + static appendElementsOnFollow() { + return true; + } + // Subclasses have be loaded setup() {} @@ -20,7 +28,11 @@ export default class VastElementBase { const selector = this.constructor.selector(); - this.elements = this.elements.concat(Array.from(this.vast.vastDocument.querySelectorAll(selector))); + if (this.constructor.appendElementsOnFollow()) { + this.elements = this.elements.concat(Array.from(this.vast.vastDocument.querySelectorAll(selector))); + } else { + this.elements = Array.from(this.vast.vastDocument.querySelectorAll(selector)); + } this.onVastReady(); } diff --git a/src/lib/vast_elements/wrapper_url.js b/src/lib/vast_elements/wrapper_url.js index 53db9e7..f373b4f 100644 --- a/src/lib/vast_elements/wrapper_url.js +++ b/src/lib/vast_elements/wrapper_url.js @@ -10,6 +10,10 @@ export default class WrapperUrl extends VastElementBase { return 'Ad Wrapper VASTAdTagURI'; } + static appendElementsOnFollow() { + return false; + } + onVastReady() { this.url = this.elements.map(el => { return NodeValue.fromElement(el); diff --git a/test/fixtures/vast-wrapper-loop.xml b/test/fixtures/vast-wrapper-loop.xml index 0e67bab..6a14283 100644 --- a/test/fixtures/vast-wrapper-loop.xml +++ b/test/fixtures/vast-wrapper-loop.xml @@ -3,7 +3,7 @@ iabtechlab - + http://example.com/error http://example.com/track/impression-parent diff --git a/test/vast_elements/wrapper_url.spec.js b/test/vast_elements/wrapper_url.spec.js index 867f854..bc460ee 100644 --- a/test/vast_elements/wrapper_url.spec.js +++ b/test/vast_elements/wrapper_url.spec.js @@ -26,6 +26,7 @@ describe('Wrapper extension', () => { } ); }); + afterEach(() => mock.teardown()); it('should find wrapper urls in vast tags', () => { @@ -33,7 +34,8 @@ describe('Wrapper extension', () => { }); it('should get the wrapper urls in vast tags', () => { - expect(vast.wrapperUrl()).toBe( + // The wrapperUrl should be followed and set as the url + expect(vast.url()).toBe( 'https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%203.0%20Samples/Inline_Companion_Tag-test.xml' ); }); @@ -51,31 +53,45 @@ describe('Wrapper extension', () => { }); describe('Wrapper related errors', () => { - beforeEach(() => mock.setup()); + let xml; + + beforeEach(() => { + mock.setup(); + + xml = fs.readFileSync('./test/fixtures/vast-wrapper-loop.xml'); + mock.get('https://forever.vast/', { + headers: { + 'Content-Length': xml.length, + 'Content-Type': 'application/xml', + }, + body: xml, + }); + }); + afterEach(() => mock.teardown()); it('should throw an error if it traverses/issues too many vast requests', async () => { - // No matter what we request always return another wapper version - const followingXml = fs.readFileSync('./test/fixtures/vast-wrapper.xml'); - mock.get( - 'https://raw.githubusercontent.com/InteractiveAdvertisingBureau/VAST_Samples/master/VAST%203.0%20Samples/Inline_Companion_Tag-test.xml', - { - headers: { - 'Content-Length': followingXml.length, - 'Content-Type': 'application/xml', - }, - body: followingXml, - } - ); - - const xmlString = fs.readFileSync('./test/fixtures/vast-wrapper.xml'); + const vast = new Vast(); + let caughtError; try { - const vast = new Vast(); - await vast.useXmlString(xmlString).catch(boom => { - console.log('caught', boom); - }); - } catch (e) { - console.log('caught upper', e); + await vast.useXmlString(xml); + } catch (boom) { + caughtError = boom; } + + expect(caughtError).toBeDefined(); + expect(caughtError.constructor).toBe(VastNetworkError); + expect(caughtError.message).toMatch(/Too Many Vast Wrapper Follows/i); + }); + + it('should place an error pixel on the page on redirects too deep', async () => { + const existingPixels = window.document.querySelectorAll('img.vast-pixel').length; + + const vast = new Vast(); + try { + await vast.useXmlString(xml); + } catch (boom) {} + + expect(window.document.querySelectorAll('img.vast-pixel').length).toBeGreaterThan(existingPixels); }); }); From 9df0b889818b71f6a451ff5f013ac7b81da5aeed Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sun, 3 Mar 2019 14:25:51 -0600 Subject: [PATCH 10/15] updates documentation and version number --- CHANGELOG.md | 10 ++++ README.md | 88 +++++++++++++++++++++---------- package.json | 2 +- src/lib/vast_elements/README.md | 2 +- test/assets/video-js-preroll.html | 11 ++-- test/assets/video-js-primary.html | 11 ++-- 6 files changed, 82 insertions(+), 42 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..4af89c0 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +## v1.1 (March 3, 2019) + +- Preparing for open sourcing, adding documentation +- Adds Wrapper/VASTAdTagURI support, `async` in more loading paths +- Moves XML Node Value parsing to it's own library + +## v1.0 + +- Basic Vast support +- Applications for `