Merge pull request #1285 from garak/restore-file-deleting

revert changes to clean files
dustin10 · May 5, 2022 · 044940a · 044940a
2 parents 7593b60 + 4808753
commit 044940a
Show file tree

Hide file tree

Showing 9 changed files with 17 additions and 62 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -35,11 +35,6 @@ parameters:
 			count: 1
 			path: src/Event/Event.php
 
-		-
-			message: "#^Call to an undefined method Doctrine\\\\Common\\\\EventArgs\\:\\:getEntityChangeSet\\(\\)\\.$#"
-			count: 1
-			path: src/EventListener/Doctrine/CleanListener.php
-
 		-
 			message: "#^Method Vich\\\\UploaderBundle\\\\Handler\\\\UploadHandler\\:\\:clean\\(\\) has parameter \\$obj with no type specified\\.$#"
 			count: 1

diff --git a/src/EventListener/Doctrine/BaseListener.php b/src/EventListener/Doctrine/BaseListener.php
@@ -70,20 +70,4 @@ protected function getUploadableFields($object): array
             return $data['propertyName'];
         }, $fields);
     }
-
-    /**
-     * Returns a list of uploadable fields for the given object and mapping.
-     *
-     * @return array<string, string> A map with property name as key and file name as value
-     *
-     * @throws \Vich\UploaderBundle\Exception\MappingNotFoundException
-     */
-    protected function getUploadableFilenameFields(object $object): array
-    {
-        $fields = $this->metadata->getUploadableFields(ClassUtils::getClass($object), $this->mapping);
-
-        return \array_map(static function (array $data): string {
-            return $data['fileNameProperty'];
-        }, $fields);
-    }
 }
diff --git a/src/EventListener/Doctrine/CleanListener.php b/src/EventListener/Doctrine/CleanListener.php
@@ -36,14 +36,8 @@ public function preUpdate(EventArgs $event): void
             return;
         }
 
-        $changeSet = $event->getEntityChangeSet();
-
-        foreach ($this->getUploadableFilenameFields($object) as $field => $fileName) {
-            if (!isset($changeSet[$fileName])) {
-                continue;
-            }
-
-            $this->handler->clean($object, $field, $changeSet[$fileName][0]);
+        foreach ($this->getUploadableFields($object) as $field) {
+            $this->handler->clean($object, $field);
         }
 
         $this->adapter->recomputeChangeSet($event);

diff --git a/src/Handler/UploadHandler.php b/src/Handler/UploadHandler.php
@@ -77,7 +77,7 @@ public function inject($obj, string $fieldName): void
         $this->dispatch(Events::POST_INJECT, new Event($obj, $mapping));
     }
 
-    public function clean($obj, string $fieldName, ?string $forcedFilename = null): void
+    public function clean($obj, string $fieldName): void
     {
         $mapping = $this->getMapping($obj, $fieldName);
 
@@ -86,10 +86,10 @@ public function clean($obj, string $fieldName, ?string $forcedFilename = null):
             return;
         }
 
-        $this->remove($obj, $fieldName, $forcedFilename);
+        $this->remove($obj, $fieldName);
     }
 
-    public function remove($obj, string $fieldName, ?string $forcedFilename = null): void
+    public function remove($obj, string $fieldName): void
     {
         $mapping = $this->getMapping($obj, $fieldName);
         $oldFilename = $mapping->getFileName($obj);
@@ -107,7 +107,7 @@ public function remove($obj, string $fieldName, ?string $forcedFilename = null):
             return;
         }
 
-        $this->storage->remove($obj, $mapping, $forcedFilename);
+        $this->storage->remove($obj, $mapping);
         $mapping->erase($obj);
 
         $this->dispatch(Events::POST_REMOVE, new Event($obj, $mapping));

diff --git a/src/Storage/AbstractStorage.php b/src/Storage/AbstractStorage.php
@@ -56,15 +56,15 @@ public function upload($obj, PropertyMapping $mapping): void
 
     abstract protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name): ?bool;
 
-    public function remove($obj, PropertyMapping $mapping, ?string $forcedFilename = null): ?bool
+    public function remove($obj, PropertyMapping $mapping): ?bool
     {
         $name = $mapping->getFileName($obj);
 
         if (empty($name)) {
             return false;
         }
 
-        return $this->doRemove($mapping, $mapping->getUploadDir($obj), $forcedFilename ?? $name);
+        return $this->doRemove($mapping, $mapping->getUploadDir($obj), $name);
     }
 
     /**

diff --git a/src/Storage/StorageInterface.php b/src/Storage/StorageInterface.php
@@ -24,7 +24,7 @@ public function upload($obj, PropertyMapping $mapping): void;
      * @param object          $obj     The object
      * @param PropertyMapping $mapping The mapping representing the field to remove
      */
-    public function remove($obj, PropertyMapping $mapping, ?string $forcedFilename = null): ?bool;
+    public function remove($obj, PropertyMapping $mapping): ?bool;
 
     /**
      * Resolves the path for a file based on the specified object
@@ -39,7 +39,7 @@ public function remove($obj, PropertyMapping $mapping, ?string $forcedFilename =
      */
     public function resolvePath($obj, ?string $fieldName = null, ?string $className = null, ?bool $relative = false): ?string;
 
-    //TODO: inconsistency - use PropertyMapping instead of fieldName+className
+    // TODO: inconsistency - use PropertyMapping instead of fieldName+className
 
     /**
      * Resolves the uri based on the specified object and mapping name.
@@ -57,7 +57,7 @@ public function resolveUri($obj, ?string $fieldName = null, ?string $className =
      *
      * @param object|array $obj       The object
      * @param string       $fieldName The field to use
-     * @param string       $className The object's class. Mandatory if $obj can't be used to determine it
+     * @param string|null  $className The object's class. Mandatory if $obj can't be used to determine it
      *
      * @return resource|null The resolved resource or null if file not stored
      */

diff --git a/tests/EventListener/Doctrine/CleanListenerTest.php b/tests/EventListener/Doctrine/CleanListenerTest.php
@@ -17,7 +17,6 @@ class CleanListenerTest extends ListenerTestCase
      */
     protected function setUp(): void
     {
-        self::$usePreUpdateEventArgs = true;
         parent::setUp();
 
         $this->listener = new CleanListener(self::MAPPING_NAME, $this->adapter, $this->metadata, $this->handler);
@@ -45,24 +44,17 @@ public function testPreUpdate(): void
             ->willReturn(true);
 
         $this->metadata
+            ->expects(self::once())
             ->method('getUploadableFields')
             ->with(DummyEntity::class, self::MAPPING_NAME)
             ->willReturn([
-                'field_name' => ['propertyName' => 'field_name', 'fileNameProperty' => 'path_name'],
-            ]);
-
-        $this->event
-            ->method('getEntityChangeSet')
-            ->willReturn([
-                'path_name' => [
-                    0 => 'dummy.jpg',
-                ],
+                ['propertyName' => 'field_name'],
             ]);
 
         $this->handler
             ->expects(self::once())
             ->method('clean')
-            ->with($this->object, 'field_name', 'dummy.jpg');
+            ->with($this->object, 'field_name');
 
         $this->adapter
             ->expects(self::once())

diff --git a/tests/EventListener/Doctrine/ListenerTestCase.php b/tests/EventListener/Doctrine/ListenerTestCase.php
@@ -64,7 +64,7 @@ protected function setUp(): void
         $this->metadata = $this->getMetadataReaderMock();
         $this->handler = $this->getHandlerMock();
         $this->object = new DummyEntity();
-        $this->event = self::$usePreUpdateEventArgs ? $this->getPreEventMock() : $this->getEventMock();
+        $this->event = $this->getEventMock();
 
         $that = $this;
 
@@ -114,14 +114,4 @@ protected function getEventMock(): EventArgs
             ->disableOriginalConstructor()
             ->getMock();
     }
-
-    /**
-     * @return PreUpdateEventArgs&\PHPUnit\Framework\MockObject\MockObject
-     */
-    protected function getPreEventMock(): PreUpdateEventArgs
-    {
-        return $this->getMockBuilder(PreUpdateEventArgs::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-    }
 }
diff --git a/tests/Handler/UploadHandlerTest.php b/tests/Handler/UploadHandlerTest.php
@@ -289,12 +289,12 @@ protected function validEvent(): object
 
     protected function expectEvents(array $events): void
     {
-        $arguments = array_map(function (string $event): array {
+        $arguments = \array_map(function (string $event): array {
             return [$this->validEvent(), $event];
         }, $events);
 
         $this->dispatcher
-            ->expects(self::exactly(count($events)))
+            ->expects(self::exactly(\count($events)))
             ->method('dispatch')
             ->withConsecutive(...$arguments)
         ;