Fix converters not being applied to null values

ddeboer · Jun 2, 2014 · 0aeb02d · 0aeb02d
1 parent 56a8ff7
commit 0aeb02d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 6 deletions.
diff --git a/src/Ddeboer/DataImport/Workflow.php b/src/Ddeboer/DataImport/Workflow.php
@@ -305,7 +305,10 @@ protected function convertItem($item)
         }
 
         foreach ($this->valueConverters as $property => $converters) {
-            if (isset($item[$property])) {
+            // isset() returns false when value is null, so we need
+            // array_key_exists() too. Combine both to have best performance,
+            // as isset() is much faster.
+            if (isset($item[$property]) || array_key_exists($property, $item)) {
                 foreach ($converters as $converter) {
                     $item[$property] = $converter->convert($item[$property]);
                 }

diff --git a/tests/Ddeboer/DataImport/Tests/WorkflowTest.php b/tests/Ddeboer/DataImport/Tests/WorkflowTest.php
@@ -189,7 +189,7 @@ public function testFilterPriority()
             ->disableOriginalConstructor()
             ->setMethods(array('filter'))
             ->getMock();
-        $validatorFilter->expects($this->exactly(2))
+        $validatorFilter->expects($this->exactly(3))
             ->method('filter')
             ->will($this->returnValue(false));
 
@@ -205,7 +205,7 @@ public function testFilterPriorityOverride()
             ->disableOriginalConstructor()
             ->setMethods(array('filter'))
             ->getMock();
-        $offsetFilter->expects($this->exactly(2))
+        $offsetFilter->expects($this->exactly(3))
             ->method('filter')
             ->will($this->returnValue(false));
 
@@ -255,9 +255,9 @@ public function testAddFilterAfterConversion()
 
         $workflow->process();
 
-        //there are two rows in reader, so every filter should be called twice
-        $this->assertEquals(2, $filterCalledIncrementor);
-        $this->assertEquals(2, $afterConversionFilterCalledIncrementor);
+        //there are two rows in reader, so every filter should be called thrice
+        $this->assertEquals(3, $filterCalledIncrementor);
+        $this->assertEquals(3, $afterConversionFilterCalledIncrementor);
     }
 
     public function testExceptionInterfaceThrownFromWriterIsCaught()
@@ -287,6 +287,17 @@ public function testExceptionInterfaceThrownFromWriterIsCaught()
         $workflow->process();
     }
 
+    public function testNullValueIsConverted()
+    {
+        $workflow = $this->getWorkflow();
+        $valueConverter = $this->getMockBuilder('Ddeboer\DataImport\ValueConverter\ValueConverterInterface')
+            ->getMock()
+        ;
+        $valueConverter->expects($this->exactly(3))->method('convert');
+        $workflow->addValueConverter('first', $valueConverter);
+        $workflow->process();
+    }
+
     protected function getWorkflow()
     {
         $reader = new ArrayReader(array(
@@ -297,6 +308,10 @@ protected function getWorkflow()
             array(
                 'first' => 'Miss',
                 'last'  => 'Moneypenny'
+            ),
+            array(
+                'first' => null,
+                'last'  => 'Doe'
             )
         ));