From 982b28f058bd79054d0e7258a06f88186f69fb3c Mon Sep 17 00:00:00 2001 From: Pieter Mees Date: Thu, 16 Mar 2017 18:15:33 -0400 Subject: [PATCH 1/4] Update dependencies --- package.json | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index dead637..6417d10 100644 --- a/package.json +++ b/package.json @@ -28,20 +28,21 @@ "dependencies": { "eventemitter3": "^2.0.0", "iab-vast-model": "^0.4.0", - "iab-vast-parser": "^0.4.0", + "iab-vast-parser": "^0.4.1", "isomorphic-fetch": "^2.2.1" }, "devDependencies": { "babel-plugin-transform-builtin-extend": "^1.1.0", - "babel-plugin-transform-runtime": "^6.22.0", + "babel-plugin-transform-runtime": "^6.23.0", "babel-preset-env": "^1.1.8", - "babel-register": "^6.22.0", - "babel-runtime": "^6.22.0", + "babel-register": "^6.24.0", + "babel-runtime": "^6.23.0", "chai": "^3.5.0", "chai-as-promised": "^6.0.0", "core-js": "^2.4.0", "del": "^2.2.0", - "express": "^4.14.1", + "dirty-chai": "^1.2.2", + "express": "^4.15.2", "fs-promise": "^2.0.0", "gulp": "^3.9.1", "gulp-babel": "^6.1.2", @@ -50,16 +51,16 @@ "gulp-istanbul": "^1.0.0", "gulp-load-plugins": "^1.5.0", "gulp-mocha": "^3.0.0", - "gulp-sourcemaps": "^2.4.0", - "gulp-standard": "^8.0.3", + "gulp-sourcemaps": "^2.4.1", + "gulp-standard": "^9.0.0", "in-publish": "^2.0.0", "isparta": "^4.0.0", "mocha-junit-reporter": "^1.13.0", "run-sequence": "^1.1.5", "sinon": "^1.17.7", "sinon-chai": "^2.8.0", - "standard": "^8.6.0", - "yargs": "^6.6.0" + "standard": "^9.0.1", + "yargs": "^7.0.2" }, "standard": { "globals": [ From 8e2f9582dbc9ff1ea2b378144f1612de69f7fb46 Mon Sep 17 00:00:00 2001 From: Pieter Mees Date: Thu, 16 Mar 2017 18:15:41 -0400 Subject: [PATCH 2/4] User dirty-chai to avoid standard errors --- test/lib/setup.js | 2 ++ test/unit/node.js | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/lib/setup.js b/test/lib/setup.js index 800797b..6d655f1 100644 --- a/test/lib/setup.js +++ b/test/lib/setup.js @@ -3,9 +3,11 @@ import chai from 'chai' import chaiAsPromised from 'chai-as-promised' import sinon from 'sinon' import sinonChai from 'sinon-chai' +import dirtyChai from 'dirty-chai' chai.use(chaiAsPromised) chai.use(sinonChai) +chai.use(dirtyChai) global.expect = chai.expect global.sinon = sinon diff --git a/test/unit/node.js b/test/unit/node.js index 146c8a9..067aabf 100644 --- a/test/unit/node.js +++ b/test/unit/node.js @@ -185,7 +185,7 @@ describe('VASTLoader', function () { const loader = createLoader('tremor-video/vast_inline_linear.xml') loader.on(type, spy) await loader.load() - expect(spy.called).to.be.true + expect(spy.called).to.be.true() }) } @@ -195,7 +195,7 @@ describe('VASTLoader', function () { const loader = createLoader('tremor-video/vast_wrapper_linear_1.xml') loader.on(type, spy) await loader.load() - expect(spy.calledTwice).to.be.true + expect(spy.calledTwice).to.be.true() }) } @@ -206,7 +206,7 @@ describe('VASTLoader', function () { try { await loader.load() } catch (err) {} - expect(spy.calledOnce).to.be.true + expect(spy.calledOnce).to.be.true() }) }) From 9174f626868a6474de1dfb40a23fb3f58091bdf7 Mon Sep 17 00:00:00 2001 From: Pieter Mees Date: Thu, 16 Mar 2017 18:16:27 -0400 Subject: [PATCH 3/4] 0.6.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6417d10..4a5ac41 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "iab-vast-loader", - "version": "0.6.1", + "version": "0.6.2", "description": "Loads and parses IAB VAST tags, resolving wrapped tags along the way.", "main": "index.js", "jsnext:main": "src/index.js", From 2c02b99896cd6c89cb0b88ce790497c4c16fa9c2 Mon Sep 17 00:00:00 2001 From: Pieter Delbeke Date: Wed, 15 Mar 2017 17:58:10 +0100 Subject: [PATCH 4/4] feat: support tree structure (ad buffet) --- README.md | 19 +++++-- package.json | 1 + src/index.js | 28 ++++++---- src/vast-tree-node.js | 22 ++++++++ test/fixtures/ads-wrapper-multi.xml | 13 +++++ test/fixtures/no-ads-alt.xml | 2 + test/unit/vast-loader-error.js | 32 ++++++++++++ test/unit/{node.js => vast-loader.js} | 74 ++++++++++----------------- 8 files changed, 131 insertions(+), 60 deletions(-) create mode 100644 src/vast-tree-node.js create mode 100644 test/fixtures/ads-wrapper-multi.xml create mode 100644 test/fixtures/no-ads-alt.xml create mode 100644 test/unit/vast-loader-error.js rename test/unit/{node.js => vast-loader.js} (83%) diff --git a/README.md b/README.md index 90ca18b..fb27929 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,8 @@ const loader = new VASTLoader(tagUrl) // Load the tag chain and await the resulting Promise loader.load() - .then((chain) => { - console.info('Loaded VAST tags:', chain) + .then((tree) => { + console.info('Loaded VAST tags:', tree) }) .catch((err) => { console.error('Error loading tag:', err) @@ -36,8 +36,19 @@ Creates a VAST loader. loader.load() ``` -Returns a `Promise` for an array of `VAST` instances. The `VAST` class is -provided by [iab-vast-model](https://www.npmjs.com/package/iab-vast-model). +Returns a `Promise` for a `VASTTreeNode` instance. +A `VASTTreeNode` has a property called `vast` which contains the loaded VAST document, parsed into a `VAST` class. +This `VAST` class is provided by [iab-vast-model](https://www.npmjs.com/package/iab-vast-model). +The `VASTTreeNode` also has a property called `childNodes` that contains an array of "sub ads", again instances of `VASTTreeNode`. + +## VASTTreeNode + +```js +node.vast // the VAST document +node.childNodes // array of VASTTreeNode +node.firstChild // first element in the childNodes array +node.hasChildNodes() // function that tells you if any sub ads where present +``` ## Error Handling diff --git a/package.json b/package.json index 4a5ac41..f05d25d 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "globals": [ "describe", "it", + "xit", "before", "after", "beforeEach", diff --git a/src/index.js b/src/index.js index a00fd97..8b6a524 100644 --- a/src/index.js +++ b/src/index.js @@ -2,6 +2,7 @@ import EventEmitter from 'eventemitter3' import fetch from 'isomorphic-fetch' import parse from 'iab-vast-parser' import { Wrapper } from 'iab-vast-model' +import VASTTreeNode from './vast-tree-node' import VASTLoaderError from './error' import atob from './atob' @@ -44,9 +45,13 @@ export default class Loader extends EventEmitter { .then(() => { this._emit('willFetch', { uri }) const match = RE_DATA_URI.exec(uri) - return (match == null) ? this._fetchUri() - : (match[1] != null) ? atob(match[2]) - : decodeURIComponent(match[2]) + if (match == null) { + return this._fetchUri() + } else if (match[1] != null) { + return atob(match[2]) + } else { + return decodeURIComponent(match[2]) + } }) .then((body) => { this._emit('didFetch', { uri, body }) @@ -54,14 +59,20 @@ export default class Loader extends EventEmitter { const vast = parse(body) this._emit('didParse', { uri, body, vast }) if (vast.ads.length > 0) { - const ad = vast.ads.get(0) - if (ad instanceof Wrapper || ad.$type === 'Wrapper') { - return this._loadWrapped(ad.vastAdTagURI, vast) + const adsToLoad = [] + for (var i = 0; i < vast.ads.length; i++) { + var ad = vast.ads.get(i) + if (ad instanceof Wrapper || ad.$type === 'Wrapper') { + adsToLoad.push(this._loadWrapped(ad.vastAdTagURI, vast)) + } } + return Promise.all(adsToLoad).then((ads) => { + return new VASTTreeNode(vast, ads) + }) } else if (this._depth > 1) { throw new VASTLoaderError(303) } - return [vast] + return new VASTTreeNode(vast) }) } @@ -75,9 +86,6 @@ export default class Loader extends EventEmitter { const childLoader = new Loader(vastAdTagURI, null, this) return childLoader.load() }) - .then((children) => { - return [vast, ...children] - }) } _fetchUri () { diff --git a/src/vast-tree-node.js b/src/vast-tree-node.js new file mode 100644 index 0000000..3987707 --- /dev/null +++ b/src/vast-tree-node.js @@ -0,0 +1,22 @@ +export default class VASTTreeNode { + constructor (vast, children = []) { + this._vast = vast + this._children = children + } + + get vast () { + return this._vast + } + + get childNodes () { + return this._children + } + + get firstChild () { + return this._children[0] + } + + hasChildNodes () { + return this._children.length > 0 + } +} diff --git a/test/fixtures/ads-wrapper-multi.xml b/test/fixtures/ads-wrapper-multi.xml new file mode 100644 index 0000000..2f2a9bc --- /dev/null +++ b/test/fixtures/ads-wrapper-multi.xml @@ -0,0 +1,13 @@ + + + + + http://demo.tremormedia.com/proddev/vast/vast_inline_linear.xml + + + + + http://demo.tremormedia.com/proddev/vast/vast_inline_linear.xml + + + diff --git a/test/fixtures/no-ads-alt.xml b/test/fixtures/no-ads-alt.xml new file mode 100644 index 0000000..2134bff --- /dev/null +++ b/test/fixtures/no-ads-alt.xml @@ -0,0 +1,2 @@ + + diff --git a/test/unit/vast-loader-error.js b/test/unit/vast-loader-error.js new file mode 100644 index 0000000..17f0601 --- /dev/null +++ b/test/unit/vast-loader-error.js @@ -0,0 +1,32 @@ +import VASTLoaderError from '../../src/error' + +describe('VASTLoaderError', function () { + describe('#code', function () { + it('gets set from the constructor', function () { + const error = new VASTLoaderError(301) + expect(error.code).to.equal(301) + }) + }) + + describe('#message', function () { + it('resolves from the code', function () { + const error = new VASTLoaderError(301) + expect(error.message).to.equal('Timeout.') + }) + }) + + describe('#cause', function () { + it('gets set from the constructor', function () { + const cause = new Error('Foo') + const error = new VASTLoaderError(301, cause) + expect(error.cause).to.equal(cause) + }) + }) + + describe('#$type', function () { + it('is VASTLoaderError', function () { + const error = new VASTLoaderError(900) + expect(error.$type).to.equal('VASTLoaderError') + }) + }) +}) diff --git a/test/unit/node.js b/test/unit/vast-loader.js similarity index 83% rename from test/unit/node.js rename to test/unit/vast-loader.js index 067aabf..1313d49 100644 --- a/test/unit/node.js +++ b/test/unit/vast-loader.js @@ -2,6 +2,7 @@ import express from 'express' import fsp from 'fs-promise' import path from 'path' import { default as VASTLoader, VASTLoaderError } from '../../src/' +import VASTTreeNode from '../../src/vast-tree-node' const expectLoaderError = (error, code, message, cause) => { expect(error).to.be.an.instanceof(VASTLoaderError) @@ -12,42 +13,12 @@ const expectLoaderError = (error, code, message, cause) => { } } -describe('VASTLoaderError', function () { - describe('#code', function () { - it('gets set from the constructor', function () { - const error = new VASTLoaderError(301) - expect(error.code).to.equal(301) - }) - }) - - describe('#message', function () { - it('resolves from the code', function () { - const error = new VASTLoaderError(301) - expect(error.message).to.equal('Timeout.') - }) - }) - - describe('#cause', function () { - it('gets set from the constructor', function () { - const cause = new Error('Foo') - const error = new VASTLoaderError(301, cause) - expect(error.cause).to.equal(cause) - }) - }) - - describe('#$type', function () { - it('is VASTLoaderError', function () { - const error = new VASTLoaderError(900) - expect(error.$type).to.equal('VASTLoaderError') - }) - }) -}) - describe('VASTLoader', function () { const fixturesPath = path.resolve(__dirname, '../fixtures') const proxyPaths = { 'http://demo.tremormedia.com/proddev/vast/vast_inline_linear.xml': 'tremor-video/vast_inline_linear.xml', 'http://example.com/no-ads.xml': 'no-ads.xml', + 'http://example.com/no-ads-alt.xml': 'no-ads-alt.xml', 'http://example.com/invalid-ads.xml': 'invalid-ads.xml' } @@ -104,16 +75,27 @@ describe('VASTLoader', function () { describe('#load()', function () { it('loads the InLine', async function () { const loader = createLoader('tremor-video/vast_inline_linear.xml') - const chain = await loader.load() - expect(chain).to.be.an.instanceof(Array) - expect(chain.length).to.equal(1) + const tree = await loader.load() + expect(tree).to.be.an.instanceof(VASTTreeNode) + expect(tree.hasChildNodes()).to.be.false }) it('loads the Wrapper', async function () { const loader = createLoader('tremor-video/vast_wrapper_linear_1.xml') - const chain = await loader.load() - expect(chain).to.be.an.instanceof(Array) - expect(chain.length).to.equal(2) + const tree = await loader.load() + expect(tree).to.be.an.instanceof(VASTTreeNode) + expect(tree.hasChildNodes()).to.be.true + expect(tree.childNodes.length).to.equal(1) + }) + + xit('loads alls ads in a Wrapper', async function () { + // iab-vast-parser first needs to support ad buffets + const loader = createLoader('ads-wrapper-multi.xml') + const tree = await loader.load() + expect(tree).to.be.an.instanceof(VASTTreeNode) + expect(tree.hasChildNodes()).to.be.true + expect(tree.childNodes.length).to.equal(2) + expect(tree.childNodes[0]).to.equal(tree.firstChild) }) it('loads the InLine as Base64', async function () { @@ -121,9 +103,9 @@ describe('VASTLoader', function () { const base64 = (await fsp.readFile(file)).toString('base64') const dataUri = 'data:text/xml;base64,' + base64 const loader = new VASTLoader(dataUri) - const chain = await loader.load() - expect(chain).to.be.an.instanceof(Array) - expect(chain.length).to.equal(1) + const tree = await loader.load() + expect(tree).to.be.an.instanceof(VASTTreeNode) + expect(tree.hasChildNodes()).to.be.false }) it('loads the InLine as XML', async function () { @@ -131,16 +113,16 @@ describe('VASTLoader', function () { const xml = (await fsp.readFile(file, 'utf8')).replace(/\r?\n/g, '') const dataUri = 'data:text/xml,' + xml const loader = new VASTLoader(dataUri) - const chain = await loader.load() - expect(chain).to.be.an.instanceof(Array) - expect(chain.length).to.equal(1) + const tree = await loader.load() + expect(tree).to.be.an.instanceof(VASTTreeNode) + expect(tree.hasChildNodes()).to.be.false }) it('loads the empty tag', async function () { const loader = createLoader('no-ads.xml') - const chain = await loader.load() - expect(chain.length).to.equal(1) - expect(chain[0].ads.length).to.equal(0) + const tree = await loader.load() + expect(tree.hasChildNodes()).to.be.false + expect(tree.vast.ads.length).to.equal(0) }) it('throws VAST 303 on empty InLine inside Wrapper', async function () {