Merge pull request #9 from niels-nijens/fix-doctrine-proxy-upload

Fix call stack when a Doctrine proxy subclass is active

Model/UploadTrait.php

@@ -32,7 +32,8 @@ trait UploadTrait
      */
     public function getFileUpload()
     {
-        $propertyName = $this->getFileUploadPropertyName();
+        $propertyName = $this->getFileUploadPropertyName(__FUNCTION__);
+
         if (isset($this->fileUploads[$propertyName])) {
             return $this->fileUploads[$propertyName];
         }
@@ -55,7 +56,8 @@ public function getFileUploads()
      */
     public function setFileUpload(UploadedFile $file = null)
     {
-        $propertyName = $this->getFileUploadPropertyName();
+        $propertyName = $this->getFileUploadPropertyName(__FUNCTION__);
+
         unset($this->fileUploads[$propertyName]);
         if ($file instanceof UploadedFile) {
             $this->fileUploads[$propertyName] = $file;
@@ -77,8 +79,14 @@ public function setFileUploadPath($directory)
      *
      * @return string
      */
-    private function getFileUploadPropertyName()
+    private function getFileUploadPropertyName($realCallerMethod)
     {
-        return lcfirst(substr(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS)[1]['function'], 3, -6));
+        $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
+        $callerMethodName = $backtrace[1]['function'];
+        if ($callerMethodName === $realCallerMethod) {
+            $callerMethodName = $backtrace[2]['function'];
+        }
+
+        return lcfirst(substr($callerMethodName, 3, -6));
     }
-}
+}

Tests/Model/UploadTraitTest.php

@@ -0,0 +1,100 @@
+<?php
+
+namespace ConnectHolland\FileUploadBundle\Tests\Model;
+
+use ConnectHolland\FileUploadBundle\Tests\UploadEntityMock;
+use ConnectHolland\FileUploadBundle\Tests\UploadEntityProxyMock;
+use PHPUnit_Framework_TestCase;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+/**
+ * UploadTraitTest.
+ *
+ * @author Niels Nijens <niels@connectholland.nl>
+ */
+class UploadTraitTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * Tests if the UploadEntityMock::setImageUpload (alias of UploadTrait::setFileUpload)
+     * sets the expected file uploads property.
+     */
+    public function testSetFileUpload()
+    {
+        $uploadedFileMock = $this->getMockBuilder(UploadedFile::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $entityMock = new UploadEntityMock();
+        $entityMock->setImageUpload($uploadedFileMock);
+
+        $this->assertAttributeSame(
+            array(
+                'image' => $uploadedFileMock,
+            ),
+            'fileUploads',
+            $entityMock
+        );
+    }
+
+    /**
+     * Tests if the UploadEntityProxyMock::setImageUpload (alias of UploadTrait::setFileUpload)
+     * sets the expected file uploads property.
+     *
+     * This tests the scenario of a Doctrine entity being a parent class of
+     * a proxy class with all the method overloaded as this changes the
+     * PHP stack to determine the caller method.
+     */
+    public function testSetFileUploadFromProxy()
+    {
+        $uploadedFileMock = $this->getMockBuilder(UploadedFile::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $uploadEntityProxyMock = new UploadEntityProxyMock();
+        $uploadEntityProxyMock->setImageUpload($uploadedFileMock);
+
+        $this->assertAttributeSame(
+            array(
+                'image' => $uploadedFileMock,
+            ),
+            'fileUploads',
+            $uploadEntityProxyMock
+        );
+    }
+
+    /**
+     * Tests if the UploadEntityMock::getImageUpload (alias of UploadTrait::getFileUpload)
+     * returns the expected file uploads property.
+     */
+    public function testGetFileUpload()
+    {
+        $uploadedFileMock = $this->getMockBuilder(UploadedFile::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $entityMock = new UploadEntityMock();
+        $entityMock->setImageUpload($uploadedFileMock);
+
+        $this->assertSame($uploadedFileMock, $entityMock->getImageUpload($uploadedFileMock));
+    }
+
+    /**
+     * Tests if the UploadEntityProxyMock::getImageUpload (alias of UploadTrait::getFileUpload)
+     * sets the expected file uploads property.
+     *
+     * This tests the scenario of a Doctrine entity being a parent class of
+     * a proxy class with all the method overloaded as this changes the
+     * PHP stack to determine the caller method.
+     */
+    public function testGetFileUploadFromProxy()
+    {
+        $uploadedFileMock = $this->getMockBuilder(UploadedFile::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $uploadEntityProxyMock = new UploadEntityProxyMock();
+        $uploadEntityProxyMock->setImageUpload($uploadedFileMock);
+
+        $this->assertSame($uploadedFileMock, $uploadEntityProxyMock->getImageUpload($uploadedFileMock));
+    }
+}

Tests/UploadEntityMock.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace ConnectHolland\FileUploadBundle\Tests;
+
+use ConnectHolland\FileUploadBundle\Model\UploadTrait;
+
+/**
+ * Mock class for testing the UploadTrait.
+ *
+ * @author Niels Nijens <niels@connectholland.nl>
+ */
+class UploadEntityMock
+{
+    use UploadTrait {
+        getFileUpload as getImageUpload;
+        setFileUpload as setImageUpload;
+    }
+}

Tests/UploadEntityProxyMock.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace ConnectHolland\FileUploadBundle\Tests;
+
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
+/**
+ * Mock class mimicking the behavior of a Doctrine proxy class.
+ *
+ * @author Niels Nijens <niels@connectholland.nl>
+ */
+class UploadEntityProxyMock extends UploadEntityMock
+{
+    /**
+     * Overloaded trait method alias.
+     *
+     * @return UploadedFile
+     */
+    public function getImageUpload()
+    {
+        return parent::getImageUpload();
+    }
+
+    /**
+     * Overloaded trait method alias.
+     *
+     * @param UploadedFile $file
+     */
+    public function setImageUpload(UploadedFile $file = null)
+    {
+        parent::setImageUpload($file);
+    }
+}