Fix error in deleting code when no filepath exists (#614)

andrade-leonardo · web-flow · commit d545ef3f5c61 · 2025-04-15T00:25:39.000+05:30
diff --git a/phpstan.neon b/phpstan.neon
@@ -10,7 +10,7 @@ parameters:
 			identifier: missingType.generics
 
 		-
-			message: "#^Parameter \\#1 \\$field of method Cake\\\\Datasource\\\\EntityInterface\\:\\:get\\(\\) expects string, array\\<int, string\\>\\|string given\\.$#"
+			message: '#^Parameter \#1 \$field of method Cake\\Datasource\\EntityInterface\:\:get\(\) expects string, array\<string\>\|string given\.$#'
 			count: 1
 			path: src/File/Path/DefaultProcessor.php
 
diff --git a/src/File/Path/Basepath/DefaultTrait.php b/src/File/Path/Basepath/DefaultTrait.php
@@ -53,17 +53,17 @@ public function basepath(): string
                 if ($value === null) {
                     throw new LogicException(sprintf(
                         'Field value for substitution is missing: %s',
-                        $field
+                        $field,
                     ));
                 } elseif (!is_scalar($value)) {
                     throw new LogicException(sprintf(
                         'Field value for substitution must be a integer, float, string or boolean: %s',
-                        $field
+                        $field,
                     ));
                 } elseif (strlen((string)$value) < 1) {
                     throw new LogicException(sprintf(
                         'Field value for substitution must be non-zero in length: %s',
-                        $field
+                        $field,
                     ));
                 }
 
@@ -74,7 +74,7 @@ public function basepath(): string
         return str_replace(
             array_keys($replacements),
             array_values($replacements),
-            $path
+            $path,
         );
     }
 }
diff --git a/src/File/Path/DefaultProcessor.php b/src/File/Path/DefaultProcessor.php
@@ -63,7 +63,7 @@ public function __construct(
         EntityInterface $entity,
         UploadedFileInterface|string $data,
         string $field,
-        array $settings
+        array $settings,
     ) {
         $this->table = $table;
         $this->entity = $entity;
diff --git a/src/File/Path/Filename/DefaultTrait.php b/src/File/Path/Filename/DefaultTrait.php
@@ -23,7 +23,7 @@ public function filename(): string
                 $this->entity,
                 $this->data,
                 $this->field,
-                $this->settings
+                $this->settings,
             );
         }
 
diff --git a/src/File/Path/ProcessorInterface.php b/src/File/Path/ProcessorInterface.php
@@ -23,7 +23,7 @@ public function __construct(
         EntityInterface $entity,
         string|UploadedFileInterface $data,
         string $field,
-        array $settings
+        array $settings,
     );
 
     /**
diff --git a/src/File/Transformer/DefaultTransformer.php b/src/File/Transformer/DefaultTransformer.php
@@ -23,7 +23,7 @@ public function __construct(
         protected EntityInterface $entity,
         protected UploadedFileInterface $data,
         protected string $field,
-        protected array $settings
+        protected array $settings,
     ) {
     }
 
diff --git a/src/File/Transformer/TransformerInterface.php b/src/File/Transformer/TransformerInterface.php
@@ -23,7 +23,7 @@ public function __construct(
         EntityInterface $entity,
         UploadedFileInterface $data,
         string $field,
-        array $settings
+        array $settings,
     );
 
     /**
diff --git a/src/File/Writer/DefaultWriter.php b/src/File/Writer/DefaultWriter.php
@@ -31,7 +31,7 @@ public function __construct(
         protected EntityInterface $entity,
         protected ?UploadedFileInterface $data,
         protected string $field,
-        protected array $settings
+        protected array $settings,
     ) {
     }
 
diff --git a/src/File/Writer/WriterInterface.php b/src/File/Writer/WriterInterface.php
@@ -23,7 +23,7 @@ public function __construct(
         EntityInterface $entity,
         ?UploadedFileInterface $data,
         string $field,
-        array $settings
+        array $settings,
     );
 
     /**
diff --git a/src/Model/Behavior/UploadBehavior.php b/src/Model/Behavior/UploadBehavior.php
@@ -163,7 +163,11 @@ public function afterDelete(EventInterface $event, EntityInterface $entity, Arra
         $result = true;
 
         foreach ($this->getConfig(null, []) as $field => $settings) {
-            if (in_array($field, $this->protectedFieldNames) || Hash::get($settings, 'keepFilesOnDelete', true)) {
+            if (
+                in_array($field, $this->protectedFieldNames)
+                || Hash::get($settings, 'keepFilesOnDelete', true)
+                || $entity->get($field) === null
+            ) {
                 continue;
             }
 
@@ -206,7 +210,7 @@ public function getPathProcessor(
         EntityInterface $entity,
         string|UploadedFileInterface $data,
         string $field,
-        array $settings
+        array $settings,
     ): ProcessorInterface {
         /** @var class-string<\Josegonzalez\Upload\File\Path\ProcessorInterface> $processorClass */
         $processorClass = Hash::get($settings, 'pathProcessor', DefaultProcessor::class);
@@ -227,7 +231,7 @@ public function getWriter(
         EntityInterface $entity,
         ?UploadedFileInterface $data,
         string $field,
-        array $settings
+        array $settings,
     ): WriterInterface {
         /** @var class-string<\Josegonzalez\Upload\File\Writer\WriterInterface> $writerClass */
         $writerClass = Hash::get($settings, 'writer', DefaultWriter::class);
@@ -262,7 +266,7 @@ public function constructFiles(
         UploadedFileInterface $data,
         string $field,
         array $settings,
-        array $pathinfo
+        array $pathinfo,
     ): array {
         $basepath = $pathinfo['basepath'];
         $filename = $pathinfo['filename'];
@@ -284,7 +288,7 @@ public function constructFiles(
         } else {
             throw new UnexpectedValueException(sprintf(
                 "'transformer' not set to instance of TransformerInterface: %s",
-                $transformerClass
+                $transformerClass,
             ));
         }
 
diff --git a/tests/Stub/ChildBehavior.php b/tests/Stub/ChildBehavior.php
@@ -16,7 +16,7 @@ public function constructFiles(
         UploadedFileInterface $data,
         string $field,
         array $settings,
-        array $pathinfo
+        array $pathinfo,
     ): array {
         $files = parent::constructFiles($entity, $data, $field, $settings, $pathinfo);
         $this->constructedFiles = $files;
diff --git a/tests/TestCase/File/Transformer/DefaultTransformerTest.php b/tests/TestCase/File/Transformer/DefaultTransformerTest.php
@@ -29,7 +29,7 @@ public function testTransform()
     {
         $this->assertEquals(
             [$this->uploadedFile->getStream()->getMetadata('uri') => 'foo.txt'],
-            $this->transformer->transform('foo.txt')
+            $this->transformer->transform('foo.txt'),
         );
     }
 }
diff --git a/tests/TestCase/File/Writer/DefaultWriterTest.php b/tests/TestCase/File/Writer/DefaultWriterTest.php
@@ -40,7 +40,7 @@ public function setUp(): void
             $this->entity,
             $this->data,
             $this->field,
-            $this->settings
+            $this->settings,
         );
 
         $this->vfs = Vfs::setup('tmp');
diff --git a/tests/TestCase/Model/Behavior/UploadBehaviorTest.php b/tests/TestCase/Model/Behavior/UploadBehaviorTest.php
@@ -30,7 +30,7 @@ public function setUp(): void
                 1,
                 UPLOAD_ERR_OK,
                 'derp',
-                'text/plain'
+                'text/plain',
             ),
         ];
 
@@ -45,7 +45,7 @@ public function setUp(): void
                 fopen('php://temp', 'wb+'),
                 0,
                 UPLOAD_ERR_NO_FILE,
-                'derp'
+                'derp',
             ),
         ];
         $this->configError = [
@@ -284,13 +284,13 @@ public function testBeforeMarshalDataAsArray()
                  ->will($this->returnValue($this->settings));
 
         $data = new ArrayObject(
-            $this->transformUploadedFilesToArray($this->dataOk)
+            $this->transformUploadedFilesToArray($this->dataOk),
         );
         $behavior->beforeMarshal(new Event('fake.event'), $data, new ArrayObject());
         $this->assertEquals(new ArrayObject($this->transformUploadedFilesToArray($this->dataOk)), $data);
 
         $data = new ArrayObject(
-            $this->transformUploadedFilesToArray($this->dataError)
+            $this->transformUploadedFilesToArray($this->dataError),
         );
         $behavior->beforeMarshal(new Event('fake.event'), $data, new ArrayObject());
         $this->assertEquals(new ArrayObject([]), $data);
@@ -524,7 +524,7 @@ public function testAfterDeleteUsesPathProcessorToDetectPathToTheFile()
             ->with('dir')
             ->will($this->returnValue(false));
 
-        $this->entity->expects($this->exactly(2))
+        $this->entity->expects($this->exactly(3))
             ->method('get')
             ->with('field')
             ->will($this->returnValue($field));
@@ -601,7 +601,7 @@ public function testAfterDeleteNoDeleteCallback()
         $this->configOk['field']['deleteCallback'] = null;
 
         $behavior->setConfig($this->configOk);
-        $this->entity->expects($this->exactly(2))
+        $this->entity->expects($this->exactly(3))
             ->method('get')
             ->with('field')
             ->will($this->returnValue($field));
@@ -642,7 +642,7 @@ public function testAfterDeleteUsesDeleteCallback()
         };
 
         $behavior->setConfig($this->configOk);
-        $this->entity->expects($this->exactly(4))
+        $this->entity->expects($this->exactly(5))
             ->method('get')
             ->with('field')
             ->will($this->returnValue($field));
@@ -687,6 +687,26 @@ public function testAfterDeleteWithProtectedFieldName()
         $this->assertTrue($behavior->afterDelete(new Event('fake.event'), $this->entity, new ArrayObject()));
     }
 
+    public function testAfterDeleteWithNullableFileField()
+    {
+        $methods = array_diff($this->behaviorMethods, ['afterDelete', 'config', 'setConfig', 'getConfig']);
+        $behavior = $this->getMockBuilder('Josegonzalez\Upload\Model\Behavior\UploadBehavior')
+            ->onlyMethods($methods)
+            ->setConstructorArgs([$this->table, $this->settings])
+            ->getMock();
+        $behavior->setConfig($this->configOk);
+
+        $this->entity->expects($this->once())
+            ->method('get')
+            ->with('field')
+            ->will($this->returnValue(null));
+
+        $behavior->expects($this->never())
+            ->method('getPathProcessor');
+
+        $behavior->afterDelete(new Event('fake.event'), $this->entity, new ArrayObject());
+    }
+
     public function testGetWriter()
     {
         $processor = $this->behavior->getWriter($this->entity, new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'), 'field', []);
@@ -700,7 +720,7 @@ public function testConstructFiles()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'),
             'field',
             [],
-            ['basepath' => 'path', 'filename' => 'file.txt']
+            ['basepath' => 'path', 'filename' => 'file.txt'],
         );
         $this->assertEquals(['php://temp' => 'path/file.txt'], $files);
 
@@ -709,7 +729,7 @@ public function testConstructFiles()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'),
             'field',
             [],
-            ['basepath' => 'some/path', 'filename' => 'file.txt']
+            ['basepath' => 'some/path', 'filename' => 'file.txt'],
         );
         $this->assertEquals(['php://temp' => 'some/path/file.txt'], $files);
     }
@@ -721,7 +741,7 @@ public function testConstructFilesWithBasePathEndingDS()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'),
             'field',
             [],
-            ['basepath' => 'path/', 'filename' => 'file.txt']
+            ['basepath' => 'path/', 'filename' => 'file.txt'],
         );
         $this->assertEquals(['php://temp' => 'path/file.txt'], $files);
 
@@ -730,7 +750,7 @@ public function testConstructFilesWithBasePathEndingDS()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'),
             'field',
             [],
-            ['basepath' => 'some/path/', 'filename' => 'file.txt']
+            ['basepath' => 'some/path/', 'filename' => 'file.txt'],
         );
         $this->assertEquals(['php://temp' => 'some/path/file.txt'], $files);
     }
@@ -745,7 +765,7 @@ public function testConstructFilesWithCallable()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'),
             'field',
             ['transformer' => $callable],
-            ['basepath' => 'some/path', 'filename' => 'file.txt']
+            ['basepath' => 'some/path', 'filename' => 'file.txt'],
         );
         $this->assertEquals(['php://temp' => 'some/path/file.text'], $files);
     }
@@ -760,7 +780,7 @@ public function testConstructFilesWithCallableAndBasePathEndingDS()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK),
             'field',
             ['transformer' => $callable],
-            ['basepath' => 'some/path', 'filename' => 'file.txt']
+            ['basepath' => 'some/path', 'filename' => 'file.txt'],
         );
         $this->assertEquals(['php://temp' => 'some/path/file.text'], $files);
     }
@@ -773,7 +793,7 @@ public function testConstructFilesException()
             new UploadedFile(fopen('php://temp', 'rw+'), 1, UPLOAD_ERR_OK, 'file.txt'),
             'field',
             ['transformer' => 'UnexpectedValueException'],
-            ['basepath' => 'path', 'filename' => 'file.txt']
+            ['basepath' => 'path', 'filename' => 'file.txt'],
         );
     }
 
@@ -821,7 +841,7 @@ function (UploadedFileInterface $file) {
                     'size' => $file->getSize(),
                 ];
             },
-            $data
+            $data,
         );
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ parameters:`
`10`	`10`	`identifier: missingType.generics`
`11`	`11`
`12`	`12`	`-`
`13`		`- message: "#^Parameter \\#1 \\$field of method Cake\\\\Datasource\\\\EntityInterface\\:\\:get\\(\\) expects string, array\\<int, string\\>\\\|string given\\.$#"`
	`13`	`+ message: '#^Parameter \#1 \$field of method Cake\\Datasource\\EntityInterface\:\:get\(\) expects string, array\<string\>\\|string given\.$#'`
`14`	`14`	`count: 1`
`15`	`15`	`path: src/File/Path/DefaultProcessor.php`
`16`	`16`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public function filename(): string`
`23`	`23`	`$this->entity,`
`24`	`24`	`$this->data,`
`25`	`25`	`$this->field,`
`26`		`- $this->settings`
	`26`	`+ $this->settings,`
`27`	`27`	`);`
`28`	`28`	`}`
`29`	`29`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public function __construct(`
`23`	`23`	`protected EntityInterface $entity,`
`24`	`24`	`protected UploadedFileInterface $data,`
`25`	`25`	`protected string $field,`
`26`		`- protected array $settings`
	`26`	`+ protected array $settings,`
`27`	`27`	`) {`
`28`	`28`	`}`
`29`	`29`
Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ public function __construct(`
`31`	`31`	`protected EntityInterface $entity,`
`32`	`32`	`protected ?UploadedFileInterface $data,`
`33`	`33`	`protected string $field,`
`34`		`- protected array $settings`
	`34`	`+ protected array $settings,`
`35`	`35`	`) {`
`36`	`36`	`}`
`37`	`37`