Merge pull request #410 from ndm2/issue-409

Deleting files should use the stored base path if available.
FriendsOfCake · Jun 30, 2017 · 97eb2ed · 97eb2ed
2 parents 245dadd + 041ce76
commit 97eb2ed
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 14 deletions.
diff --git a/docs/examples.rst b/docs/examples.rst
@@ -67,8 +67,14 @@ Basic example
         <?php echo $this->Form->input('photo', ['type' => 'file']); ?>
     <?php echo $this->Form->end(); ?>
 
-Using the above setup, uploaded files cannot be deleted. To do so, a
-field must be added to store the directory of the file as follows:
+Deleting files
+--------------
+
+Using the setup from the previous example, uploaded files can only be deleted as long as the path is configured to use
+static tokens. As soon as dynamic tokens are incorporated, like for example ``{day}``, the generated path will change
+over time, and files cannot be deleted anymore at a later point.
+
+In order to prevent such situations, a field must be added to store the directory of the file as follows:
 
 .. code:: sql
 
@@ -132,9 +138,11 @@ field must be added to store the directory of the file as follows:
     <?php echo $this->Form->create($user, ['type' => 'file']); ?>
         <?php echo $this->Form->input('username'); ?>
         <?php echo $this->Form->input('photo', ['type' => 'file']); ?>
-        <?php echo $this->Form->input('photo_dir', ['type' => 'hidden']); ?>
     <?php echo $this->Form->end(); ?>
 
+Using such a setup, the behavior will use the stored path value instead of generating the path dynamically when deleting
+files.
+
 Advanced example
 ----------------
 

diff --git a/src/Model/Behavior/UploadBehavior.php b/src/Model/Behavior/UploadBehavior.php
@@ -124,13 +124,18 @@ public function afterDelete(Event $event, Entity $entity, ArrayObject $options)
                 continue;
             }
 
-            $path = $this->getPathProcessor($entity, $entity->{$field}, $field, $settings)->basepath();
+            $dirField = Hash::get($settings, 'fields.dir', 'dir');
+            if ($entity->has($dirField)) {
+                $path = $entity->get($dirField);
+            } else {
+                $path = $this->getPathProcessor($entity, $entity->get($field), $field, $settings)->basepath();
+            }
 
             $callback = Hash::get($settings, 'deleteCallback', null);
             if ($callback && is_callable($callback)) {
                 $files = $callback($path, $entity, $field, $settings);
             } else {
-                $files = [$path . $entity->{$field}];
+                $files = [$path . $entity->get($field)];
             }
 
             $writer = $this->getWriter($entity, [], $field, $settings);

diff --git a/tests/TestCase/Model/Behavior/UploadBehaviorTest.php b/tests/TestCase/Model/Behavior/UploadBehaviorTest.php
@@ -415,34 +415,91 @@ public function testAfterDeleteSkip()
 
     public function testAfterDeleteUsesPathProcessorToDetectPathToTheFile()
     {
-        $this->entity->field = rand(1000, 9999);
-        $path = rand(1000, 9999) . DIRECTORY_SEPARATOR;
+        $dir = '/some/path/';
+        $field = 'file.txt';
+
         $methods = array_diff($this->behaviorMethods, ['afterDelete', 'config', 'setConfig', 'getConfig']);
         $behavior = $this->getMockBuilder('Josegonzalez\Upload\Model\Behavior\UploadBehavior')
             ->setMethods($methods)
             ->setConstructorArgs([$this->table, $this->dataOk])
             ->getMock();
         $behavior->config($this->dataOk);
 
+        $this->entity->expects($this->at(0))
+            ->method('has')
+            ->with('dir')
+            ->will($this->returnValue(false));
+        $this->entity->expects($this->at(1))
+            ->method('get')
+            ->with('field')
+            ->will($this->returnValue($field));
+        $this->entity->expects($this->at(2))
+            ->method('get')
+            ->with('field')
+            ->will($this->returnValue($field));
+
         // expecting getPathProcessor to be called with right arguments for dataOk
-        $behavior->expects($this->once())->method('getPathProcessor')
-            ->with($this->entity, $this->entity->field, 'field', $this->dataOk['field'])
+        $behavior->expects($this->once())
+            ->method('getPathProcessor')
+            ->with($this->entity, $field, 'field', $this->dataOk['field'])
             ->willReturn($this->processor);
         // basepath of processor should return our fake path
-        $this->processor->expects($this->once())->method('basepath')
-            ->willReturn($path);
+        $this->processor->expects($this->once())
+            ->method('basepath')
+            ->willReturn($dir);
         // expecting getWriter to be called with right arguments for dataOk
-        $behavior->expects($this->once())->method('getWriter')
+        $behavior->expects($this->once())
+            ->method('getWriter')
             ->with($this->entity, [], 'field', $this->dataOk['field'])
             ->willReturn($this->writer);
         // and here we check that file with right path will be deleted
-        $this->writer->expects($this->once())->method('delete')
-            ->with([$path . $this->entity->field])
+        $this->writer->expects($this->once())
+            ->method('delete')
+            ->with([$dir . $field])
             ->willReturn([true]);
 
         $behavior->afterDelete(new Event('fake.event'), $this->entity, new ArrayObject);
     }
 
+    public function testAfterDeletePrefersStoredPathOverPathProcessor()
+    {
+        $dir = '/some/path/';
+        $field = 'file.txt';
+
+        $methods = array_diff($this->behaviorMethods, ['afterDelete', 'config', 'setConfig', 'getConfig']);
+        $behavior = $this->getMockBuilder('Josegonzalez\Upload\Model\Behavior\UploadBehavior')
+            ->setMethods($methods)
+            ->setConstructorArgs([$this->table, $this->dataOk])
+            ->getMock();
+        $behavior->config($this->dataOk);
+
+        $this->entity->expects($this->at(0))
+            ->method('has')
+            ->with('dir')
+            ->will($this->returnValue(true));
+        $this->entity->expects($this->at(1))
+            ->method('get')
+            ->with('dir')
+            ->will($this->returnValue($dir));
+        $this->entity->expects($this->at(2))
+            ->method('get')
+            ->with('field')
+            ->will($this->returnValue($field));
+
+        $behavior->expects($this->never())
+            ->method('getPathProcessor');
+        $behavior->expects($this->once())
+            ->method('getWriter')
+            ->will($this->returnValue($this->writer));
+
+        $this->writer->expects($this->once())
+            ->method('delete')
+            ->with([$dir . $field])
+            ->will($this->returnValue([true]));
+
+        $this->assertNull($behavior->afterDelete(new Event('fake.event'), $this->entity, new ArrayObject));
+    }
+
     public function testAfterDeleteNoDeleteCallback()
     {
         $this->entity->field = rand(1000, 9999);