fix root element of ad section

sokil · Jun 29, 2018 · 8433125 · 8433125
1 parent 54c8e27
commit 8433125
Show file tree

Hide file tree

Showing 9 changed files with 158 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.4.8 (2018-06-29)
+  * Fixed 'Document::getAdSections()'. Previously root DOM element was set to first child which may be text node. Now it points to `InLine` or `Wrapper` element.
+  * Now may be specified id of `Impression` in `AbstractAdNode::addImpression`
+
 ## 0.4.7 (2018-06-29)
   * Fixed `AbstractNode::getValuesOfArrayNode`. Affected  `AbstractAdNode::getErrors()` and `AbstractAdNode::getImpressions()` if multiple nodes in array found. Array always contain first node instead of values of all nodes.
 
@@ -11,11 +15,15 @@
 ## 0.4.4 (2018-01-25)
   * Allow Ad sequence
 
+## 0.4.2 (2017-11-02)
+  * Returned method `AbstractAdNode::setImpression` and marked deprecated
+
 ## 0.4 (2017-07-26)
   * Move to PSR-4
   * Added `Factory` to create document. Factory methods in `Document` are deprecated
   * Added `Error` tag to `VAST`, `InLine` and `Wrapper`
-  * Added `Impression` to `InLine` and `Wrapper`
+  * Allowed to add multiple `Impression` to `InLine` and `Wrapper`
+  * Removed method `AbstractAdNode::setImpression` 
   * Classes moved. Check your extends
 
 ## 0.3 (2016-03-24)

diff --git a/README.md b/README.md
@@ -40,7 +40,7 @@ $ad1 = $document
     ->setId('ad1')
     ->setAdSystem('Ad Server Name')
     ->setAdTitle('Ad Title')
-    ->addImpression('http://ad.server.com/impression');
+    ->addImpression('http://ad.server.com/impression', 'imp1');
 
 // create creative for ad section
 $linearCreative = $ad1
@@ -88,7 +88,7 @@ This will generate:
         <InLine>
             <AdSystem>Ad Server Name</AdSystem>
             <AdTitle><![CDATA[Ad Title]]></AdTitle>
-            <Impression><![CDATA[http://ad.server.com/impression]]></Impression>
+            <Impression id="imp1"><![CDATA[http://ad.server.com/impression]]></Impression>
             <Creatives>
                 <Creative>
                     <Linear>

diff --git a/src/Ad/AbstractAdNode.php b/src/Ad/AbstractAdNode.php
@@ -7,6 +7,11 @@
 
 abstract class AbstractAdNode extends AbstractNode
 {
+    /**
+     * @var \DOMElement
+     */
+    private $domElement;
+
     /**
      * @var \DOMElement
      */
@@ -35,6 +40,21 @@ abstract class AbstractAdNode extends AbstractNode
     public function __construct(\DOMElement $adDomElement)
     {
         $this->adDomElement = $adDomElement;
+
+        $this->domElement = $this->adDomElement->getElementsByTagName($this->getType())->item(0);
+    }
+
+    /**
+     * Return type of ad (InLine or Wrapper)
+     *
+     * @return string
+     */
+    public function getType()
+    {
+        $parts = explode('\\', get_class($this));
+        $type = array_pop($parts);
+
+        return $type;
     }
 
     /**
@@ -44,7 +64,7 @@ public function __construct(\DOMElement $adDomElement)
      */
     protected function getDomElement()
     {
-        return $this->adDomElement->firstChild;
+        return $this->domElement;
     }
 
     /**
@@ -96,7 +116,7 @@ public function getSequence()
     }
 
     /**
-     * /Vast/Ad/Inline/AdSystem element
+     * Set /Vast/Ad/Inline/AdSystem element
      *
      * @param string $adSystem
      *
@@ -109,6 +129,18 @@ public function setAdSystem($adSystem)
         return $this;
     }
 
+    /**
+     * Get /Vast/Ad/Inline/AdSystem element
+     *
+     * @return string
+     */
+    public function getAdSystem()
+    {
+        $adSystem = $this->getScalarNodeValue('AdSystem');
+
+        return $adSystem;
+    }
+
     /**
      * Build class name for creative of given type
      *
@@ -169,7 +201,7 @@ protected function buildCreative($type)
      */
     public function addError($url)
     {
-        $this->addCdataToArrayNode('Error', $url);
+        $this->addCdataNode('Error', $url);
 
         return $this;
     }
@@ -188,13 +220,26 @@ public function getErrors()
      * Add Impression tracking url
      * Allowed multiple impressions
      *
-     * @param string $url
+     * @param string $url A URI that directs the video player to a tracking resource file that the video player
+     *        ust use to notify the ad server when the impression occurs.
+     * @param string|null $id An ad server id for the impression. Impression URIs of the same id for an ad should
+     *        be requested at the same time or as close in time as possible to help prevent
+     *        discrepancies.
      *
      * @return $this
      */
-    public function addImpression($url)
+    public function addImpression($url, $id = null)
     {
-        $this->addCdataToArrayNode('Impression', $url);
+        $attributes = array();
+        if ($id !== null) {
+            $attributes['id'] = $id;
+        }
+
+        $this->addCdataNode(
+            'Impression',
+            $url,
+            $attributes
+        );
 
         return $this;
     }

diff --git a/src/Ad/InLine.php b/src/Ad/InLine.php
@@ -2,6 +2,7 @@
 
 namespace Sokil\Vast\Ad;
 
+use Sokil\Vast\Creative\AbstractLinearCreative;
 use Sokil\Vast\Creative\InLine\Linear;
 
 class InLine extends AbstractAdNode
@@ -32,7 +33,7 @@ protected function buildCreativeClassName($type)
      * Create Linear creative
      *
      * @throws \Exception
-     * @return Linear
+     * @return AbstractLinearCreative
      */
     public function createLinearCreative()
     {

diff --git a/src/Document.php b/src/Document.php
@@ -162,7 +162,6 @@ public function getAdSections()
                 }
 
                 $this->vastAdNodeList[] = new $adTypeClassName($adDomElement);
-                break;
             }
         }
 
@@ -179,7 +178,7 @@ public function getAdSections()
      */
     public function addErrors($url)
     {
-        $this->addCdataToArrayNode('Error', $url);
+        $this->addCdataNode('Error', $url);
         return $this;
     }
 

diff --git a/src/Document/AbstractNode.php b/src/Document/AbstractNode.php
@@ -5,12 +5,14 @@
 abstract class AbstractNode
 {
     /**
+     * Root DOM element, represented by this Node class.
+     *
      * @return \DOMElement
      */
     abstract protected function getDomElement();
 
     /**
-     * Set cdata for given child node
+     * Set cdata for given child node or create new child node
      *
      * @param string $name name of node
      * @param string $value value of cdata
@@ -40,12 +42,30 @@ protected function setScalarNodeCdata($name, $value)
     }
 
     /**
+     * @param string $name
+     *
+     * @return string|null null if node not found
+     */
+    protected function getScalarNodeValue($name)
+    {
+        $domElements = $this->getDomElement()->getElementsByTagName($name);
+        if ($domElements->length === 0) {
+            return null;
+        }
+
+        return $domElements->item(0)->nodeValue;
+    }
+
+    /**
+     * Append new child node to node
+     *
      * @param string $nodeName
      * @param string $value
+     * @param array $attributes
      *
      * @return $this
      */
-    protected function addCdataToArrayNode($nodeName, $value)
+    protected function addCdataNode($nodeName, $value, array $attributes = array())
     {
         // create element
         $domElement = $this->getDomElement()->ownerDocument->createElement($nodeName);
@@ -55,6 +75,11 @@ protected function addCdataToArrayNode($nodeName, $value)
         $cdata = $this->getDomElement()->ownerDocument->createCDATASection($value);
         $domElement->appendChild($cdata);
 
+        // add attributes
+        foreach ($attributes as $attributeId => $attributeValue) {
+            $domElement->setAttribute($attributeId, $attributeValue);
+        }
+
         return $this;
     }
 

diff --git a/tests/DocumentTest.php b/tests/DocumentTest.php
@@ -21,7 +21,7 @@ public function testCreateInLineAdSection()
             ->setId('ad1')
             ->setAdSystem('Ad Server Name')
             ->setAdTitle('Ad Title')
-            ->addImpression('http://ad.server.com/impression');
+            ->addImpression('http://ad.server.com/impression', 'imp1');
 
         // create creative for ad section
         $ad1
@@ -40,7 +40,7 @@ public function testCreateInLineAdSection()
                 ->setBitrate(600)
                 ->setUrl('http://server.com/media.mp4');
 
-        $expectedXml = '<?xml version="1.0" encoding="UTF-8"?><VAST version="2.0"><Ad id="ad1"><InLine><AdSystem><![CDATA[Ad Server Name]]></AdSystem><AdTitle><![CDATA[Ad Title]]></AdTitle><Impression><![CDATA[http://ad.server.com/impression]]></Impression><Creatives><Creative><Linear><Duration>00:02:08</Duration><VideoClicks><ClickThrough><![CDATA[http://entertainmentserver.com/landing]]></ClickThrough><ClickTracking><![CDATA[http://ad.server.com/videoclicks/clicktracking]]></ClickTracking><CustomClick><![CDATA[http://ad.server.com/videoclicks/customclick]]></CustomClick></VideoClicks><TrackingEvents><Tracking event="start"><![CDATA[http://ad.server.com/trackingevent/start]]></Tracking><Tracking event="pause"><![CDATA[http://ad.server.com/trackingevent/stop]]></Tracking></TrackingEvents><MediaFiles><MediaFile delivery="progressive" type="video/mp4" height="100" width="100" bitrate="600"><![CDATA[http://server.com/media.mp4]]></MediaFile></MediaFiles></Linear></Creative></Creatives></InLine></Ad></VAST>';
+        $expectedXml = '<?xml version="1.0" encoding="UTF-8"?><VAST version="2.0"><Ad id="ad1"><InLine><AdSystem><![CDATA[Ad Server Name]]></AdSystem><AdTitle><![CDATA[Ad Title]]></AdTitle><Impression id="imp1"><![CDATA[http://ad.server.com/impression]]></Impression><Creatives><Creative><Linear><Duration>00:02:08</Duration><VideoClicks><ClickThrough><![CDATA[http://entertainmentserver.com/landing]]></ClickThrough><ClickTracking><![CDATA[http://ad.server.com/videoclicks/clicktracking]]></ClickTracking><CustomClick><![CDATA[http://ad.server.com/videoclicks/customclick]]></CustomClick></VideoClicks><TrackingEvents><Tracking event="start"><![CDATA[http://ad.server.com/trackingevent/start]]></Tracking><Tracking event="pause"><![CDATA[http://ad.server.com/trackingevent/stop]]></Tracking></TrackingEvents><MediaFiles><MediaFile delivery="progressive" type="video/mp4" height="100" width="100" bitrate="600"><![CDATA[http://server.com/media.mp4]]></MediaFile></MediaFiles></Linear></Creative></Creatives></InLine></Ad></VAST>';
         $this->assertVastXmlEquals($expectedXml, $document);
     }
 
@@ -363,6 +363,8 @@ public function testImpressionInWrapperAd()
         );
     }
 
+
+
     /**
      * test Document to output string
      */
@@ -406,7 +408,10 @@ public function testFromString()
         $factory = new Factory();
         $document = $factory->create('2.0');
 
-        $this->assertInstanceOf('Sokil\Vast\Document', $document::fromString('<?xml version="1.0" encoding="UTF-8"?><VAST version="2.0"/>'));
+        $this->assertInstanceOf(
+            'Sokil\Vast\Document',
+            $document::fromString('<?xml version="1.0" encoding="UTF-8"?><VAST version="2.0"/>')
+        );
     }
 
     /**
@@ -417,7 +422,7 @@ public function testFromFile()
         $factory = new Factory();
         $document = $factory->create('2.0');
 
-        $this->assertInstanceOf('Sokil\Vast\Document', $document::fromFile(__DIR__.'/vast.xml'));
+        $this->assertInstanceOf('Sokil\Vast\Document', $document::fromFile(__DIR__ . '/vast.xml'));
     }
 
 }
diff --git a/tests/FactoryTest.php b/tests/FactoryTest.php
@@ -4,31 +4,41 @@
 
 class FactoryTest extends AbstractTestCase
 {
-    /**
-     * @var string
-     */
-    private $vastXml = '<?xml version="1.0" encoding="UTF-8"?><VAST version="2.0"><Ad id="ad1"><InLine><AdSystem><![CDATA[Ad Server Name]]></AdSystem><AdTitle><![CDATA[Ad Title]]></AdTitle><Impression><![CDATA[http://ad.server.com/impression]]></Impression><Creatives><Creative><Linear><Duration>00:02:08</Duration><VideoClicks><ClickThrough><![CDATA[http://entertainmentserver.com/landing]]></ClickThrough><ClickTracking><![CDATA[http://ad.server.com/videoclicks/clicktracking]]></ClickTracking><CustomClick><![CDATA[http://ad.server.com/videoclicks/customclick]]></CustomClick></VideoClicks><TrackingEvents><Tracking event="start"><![CDATA[http://ad.server.com/trackingevent/start]]></Tracking><Tracking event="pause"><![CDATA[http://ad.server.com/trackingevent/stop]]></Tracking></TrackingEvents><MediaFiles><MediaFile delivery="progressive" type="video/mp4" height="100" width="100"><![CDATA[http://server.com/media.mp4]]></MediaFile></MediaFiles></Linear></Creative></Creatives></InLine></Ad></VAST>';
-
     public function testFromFile()
     {
-        $tempFilename = tempnam(sys_get_temp_dir(), "vast");
-        file_put_contents($tempFilename, $this->vastXml);
-
         $factory = new Factory();
-        $vastDocument = $factory->fromFile($tempFilename);
+        $vastDocument = $factory->fromFile(__DIR__ . '/vast.xml');
 
+        // check if loaded
         $this->assertInstanceOf(
             'Sokil\Vast\Document',
             $vastDocument
         );
 
-        unlink($tempFilename);
+        // get first ad section
+        $adSections = $vastDocument->getAdSections();
+        $adSection = $adSections[0];
+
+        // get scalar node
+        $adSystem = $adSection->getAdSystem();
+        $this->assertSame('Ad Server Name', $adSystem);
+
+        // get multi-nodes
+
+        $this->assertEquals(
+            array(
+                'http://ad.server.com/impression1',
+                'http://ad.server.com/impression2',
+                'http://ad.server.com/impression3',
+            ),
+            $adSection->getImpressions()
+        );
     }
 
     public function testFromString()
     {
         $factory = new Factory();
-        $vastDocument = $factory->fromString($this->vastXml);
+        $vastDocument = $factory->fromString(file_get_contents(__DIR__ . '/vast.xml'));
 
         $this->assertInstanceOf(
             'Sokil\Vast\Document',

diff --git a/tests/vast.xml b/tests/vast.xml
@@ -1 +1,36 @@
-<?xml version="1.0" encoding="UTF-8"?><VAST version="2.0"/>
+<?xml version="1.0" encoding="UTF-8"?>
+<VAST version="4.0">
+    <Ad id="ad1">
+        <InLine>
+            <AdSystem><![CDATA[Ad Server Name]]></AdSystem>
+            <AdTitle><![CDATA[Ad Title]]></AdTitle>
+            <Impression id="imp1"><![CDATA[http://ad.server.com/impression1]]></Impression>
+            <Impression id="imp2"><![CDATA[http://ad.server.com/impression2]]></Impression>
+            <Impression id="imp3"><![CDATA[http://ad.server.com/impression3]]></Impression>
+            <Creatives>
+                <Creative>
+                    <Linear>
+                        <Duration>00:02:08</Duration>
+                        <VideoClicks>
+                            <ClickThrough><![CDATA[http://entertainmentserver.com/landing]]></ClickThrough>
+                            <ClickTracking><![CDATA[http://ad.server.com/videoclicks/clicktracking]]></ClickTracking>
+                            <CustomClick><![CDATA[http://ad.server.com/videoclicks/customclick]]></CustomClick>
+                        </VideoClicks>
+                        <TrackingEvents>
+                            <Tracking event="start"><![CDATA[http://ad.server.com/trackingevent/start]]></Tracking>
+                            <Tracking event="pause"><![CDATA[http://ad.server.com/trackingevent/stop]]></Tracking>
+                        </TrackingEvents>
+                        <MediaFiles>
+                            <MediaFile delivery="progressive" type="video/mp4" height="100" width="100" bitrate="2500">
+                                <![CDATA[http://server.com/media1.mp4]]>
+                            </MediaFile>
+                            <MediaFile delivery="progressive" type="video/mp4" height="200" width="200" bitrate="2500">
+                                <![CDATA[http://server.com/media2.mp4]]>
+                            </MediaFile>
+                        </MediaFiles>
+                    </Linear>
+                </Creative>
+            </Creatives>
+        </InLine>
+    </Ad>
+</VAST>