diff --git a/docs/examples.rst b/docs/examples.rst index f6752aa3..04945ad3 100644 --- a/docs/examples.rst +++ b/docs/examples.rst @@ -67,8 +67,14 @@ Basic example Form->input('photo', ['type' => 'file']); ?> 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: Form->create($user, ['type' => 'file']); ?> Form->input('username'); ?> Form->input('photo', ['type' => 'file']); ?> - Form->input('photo_dir', ['type' => 'hidden']); ?> 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 index 698cd549..a086f32c 100644 --- 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 index 390f2dfa..55406832 100644 --- a/tests/TestCase/Model/Behavior/UploadBehaviorTest.php +++ b/tests/TestCase/Model/Behavior/UploadBehaviorTest.php @@ -415,8 +415,9 @@ 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) @@ -424,25 +425,81 @@ public function testAfterDeleteUsesPathProcessorToDetectPathToTheFile() ->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);