Repository: doctrine-extensions/DoctrineExtensions (language: php)

Issue:
Possibly wrong typehint on UploadedFileInfo
The other side of issue https://github.com/antishov/StofDoctrineExtensionsBundle/issues/32

Here https://github.com/doctrine-extensions/DoctrineExtensions/commit/907cd6e11f933ef03093220b93bb48e1b0c25484 FileInfoInterface got some new typehints with the latest release, and I think that the getSize method should return int (or ?int) instead of string|null.

In the docs the interface is used with $_FILES global (https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/uploadable.md#usage) which has a data type of int at the size key.

```php
array(1) {
     ["file"]=> array(5) {
         ["name"]=> string(10) "138728.jpg"
         ["type"]=> string(10) "image/jpeg"
         ["tmp_name"]=> string(14) "/tmp/php1FHOKM"
         ["error"]=> int(0)
         ["size"]=> int(504984)
     }
 } 
```



Discussion / hints:
Hi @mmarton, thanks for reporting!

This is interesting, I added `string|null` following:

https://github.com/doctrine-extensions/DoctrineExtensions/blob/a9fd83ad6e908e7180a719f174ded0b27aec2556/tests/Gedmo/Uploadable/UploadableEntityTest.php#L155

because when using 

```php
    #[ORM\Column(name: 'size', type: Types::DECIMAL)]
    #[Gedmo\UploadableFileSize]
    private $size;
```

`$size` has `decimal` type which means that [doctrine maps it as a `string|null`](https://www.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/types.html#decimal).

Given that, it's true that `FileInfoInterface::getSize()` shouldn't be coupled with the mapping, so I guess the `UploadableListener` should be in charge of parsing the result of `FileInfoInterface::getSize()` into the proper mapping type.

I'm not sure if I can take a look at this today, feel free to open a PR!

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4784f2149..bfc7bd949 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,8 @@ a release.
 ---
 
 ## [Unreleased]
+### Fixed
+- Uploadable: `FileInfoInterface::getSize()` return type declaration (#2413).
 
 ## [3.5.0] - 2022-01-10
 ### Added
diff --git a/src/Uploadable/FileInfo/FileInfoInterface.php b/src/Uploadable/FileInfo/FileInfoInterface.php
index daf1209c7..a05123547 100644
--- a/src/Uploadable/FileInfo/FileInfoInterface.php
+++ b/src/Uploadable/FileInfo/FileInfoInterface.php
@@ -28,7 +28,7 @@ public function getTmpName();
     public function getName();
 
     /**
-     * @return string|null
+     * @return int|null
      */
     public function getSize();
 
diff --git a/src/Uploadable/UploadableListener.php b/src/Uploadable/UploadableListener.php
index 5a21cd7aa..73d07e982 100644
--- a/src/Uploadable/UploadableListener.php
+++ b/src/Uploadable/UploadableListener.php
@@ -10,6 +10,7 @@
 namespace Gedmo\Uploadable;
 
 use Doctrine\Common\EventArgs;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\Persistence\Event\LoadClassMetadataEventArgs;
 use Doctrine\Persistence\Mapping\ClassMetadata;
 use Doctrine\Persistence\NotifyPropertyChanged;
@@ -325,7 +326,12 @@ public function processFile(AdapterInterface $ea, $object, $action)
         }
 
         if ($config['fileSizeField']) {
-            $this->updateField($object, $uow, $ea, $meta, $config['fileSizeField'], $info['fileSize']);
+            $typeOfSizeField = Type::getType($meta->getTypeOfField($config['fileSizeField']));
+            $value = $typeOfSizeField->convertToPHPValue(
+                $info['fileSize'],
+                $om->getConnection()->getDatabasePlatform()
+            );
+            $this->updateField($object, $uow, $ea, $meta, $config['fileSizeField'], $value);
         }
 
         $ea->recomputeSingleObjectChangeSet($uow, $meta, $object);


Test patch:
diff --git a/tests/Gedmo/Uploadable/Fixture/Entity/FileWithMaxSize.php b/tests/Gedmo/Uploadable/Fixture/Entity/FileWithMaxSize.php
index 203cfada8..b2d87be42 100644
--- a/tests/Gedmo/Uploadable/Fixture/Entity/FileWithMaxSize.php
+++ b/tests/Gedmo/Uploadable/Fixture/Entity/FileWithMaxSize.php
@@ -17,10 +17,10 @@
 
 /**
  * @ORM\Entity
- * @Gedmo\Uploadable(allowOverwrite=true, pathMethod="getPath", callback="callbackMethod", maxSize="1")
+ * @Gedmo\Uploadable(allowOverwrite=true, pathMethod="getPath", callback="callbackMethod", maxSize="2")
  */
 #[ORM\Entity]
-#[Gedmo\Uploadable(allowOverwrite: true, pathMethod: 'getPath', callback: 'callbackMethod', maxSize: '1')]
+#[Gedmo\Uploadable(allowOverwrite: true, pathMethod: 'getPath', callback: 'callbackMethod', maxSize: '2')]
 class FileWithMaxSize
 {
     /**
diff --git a/tests/Gedmo/Uploadable/Fixture/Entity/ImageWithTypedProperties.php b/tests/Gedmo/Uploadable/Fixture/Entity/ImageWithTypedProperties.php
new file mode 100644
index 000000000..6919de607
--- /dev/null
+++ b/tests/Gedmo/Uploadable/Fixture/Entity/ImageWithTypedProperties.php
@@ -0,0 +1,126 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Doctrine Behavioral Extensions package.
+ * (c) Gediminas Morkevicius <gediminas.morkevicius@gmail.com> http://www.gediminasm.org
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Gedmo\Tests\Uploadable\Fixture\Entity;
+
+use Doctrine\DBAL\Types\Types;
+use Doctrine\ORM\Mapping as ORM;
+use Gedmo\Mapping\Annotation as Gedmo;
+
+/**
+ * @ORM\Entity
+ * @Gedmo\Uploadable(pathMethod="getPath")
+ */
+#[ORM\Entity]
+#[Gedmo\Uploadable(pathMethod: 'getPath')]
+class ImageWithTypedProperties
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue(strategy="IDENTITY")
+     * @ORM\Column(type="integer")
+     */
+    #[ORM\Id]
+    #[ORM\GeneratedValue(strategy: 'IDENTITY')]
+    #[ORM\Column(type: Types::INTEGER)]
+    private ?int $id = null;
+
+    /**
+     * @ORM\Column(name="title", type="string")
+     */
+    #[ORM\Column(name: 'title', type: Types::STRING)]
+    private ?string $title = null;
+
+    /**
+     * @ORM\Column(name="path", type="string", nullable=true)
+     * @Gedmo\UploadableFilePath
+     */
+    #[ORM\Column(name: 'path', type: Types::STRING, nullable: true)]
+    #[Gedmo\UploadableFilePath]
+    private ?string $filePath = null;
+
+    /**
+     * @ORM\Column(name="size", type="decimal", nullable=true)
+     * @Gedmo\UploadableFileSize
+     */
+    #[ORM\Column(name: 'size', type: Types::DECIMAL, nullable: true)]
+    #[Gedmo\UploadableFileSize]
+    private ?string $size = null;
+
+    /**
+     * @ORM\Column(name="mime_type", type="string", nullable=true)
+     * @Gedmo\UploadableFileMimeType
+     */
+    #[ORM\Column(name: 'mime_type', type: Types::STRING, nullable: true)]
+    #[Gedmo\UploadableFileMimeType]
+    private ?string $mime = null;
+
+    private bool $useBasePath = false;
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+
+    public function setTitle(?string $title): void
+    {
+        $this->title = $title;
+    }
+
+    public function getTitle(): ?string
+    {
+        return $this->title;
+    }
+
+    public function setFilePath(?string $filePath): void
+    {
+        $this->filePath = $filePath;
+    }
+
+    public function getFilePath(): ?string
+    {
+        return $this->filePath;
+    }
+
+    public function getPath(?string $basePath = null): string
+    {
+        if ($this->useBasePath) {
+            return $basePath.'/abc/def';
+        }
+
+        return TESTS_TEMP_DIR.'/uploadable';
+    }
+
+    public function setMime(?string $mime): void
+    {
+        $this->mime = $mime;
+    }
+
+    public function getMime(): ?string
+    {
+        return $this->mime;
+    }
+
+    public function setSize(?string $size): void
+    {
+        $this->size = $size;
+    }
+
+    public function getSize(): ?string
+    {
+        return $this->size;
+    }
+
+    public function setUseBasePath(bool $useBasePath): void
+    {
+        $this->useBasePath = $useBasePath;
+    }
+}
diff --git a/tests/Gedmo/Uploadable/UploadableEntitySizeTypeTest.php b/tests/Gedmo/Uploadable/UploadableEntitySizeTypeTest.php
new file mode 100644
index 000000000..de26d66f6
--- /dev/null
+++ b/tests/Gedmo/Uploadable/UploadableEntitySizeTypeTest.php
@@ -0,0 +1,126 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Doctrine Behavioral Extensions package.
+ * (c) Gediminas Morkevicius <gediminas.morkevicius@gmail.com> http://www.gediminasm.org
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Gedmo\Tests\Uploadable;
+
+use Doctrine\Common\EventManager;
+use Gedmo\Tests\Tool\BaseTestCaseORM;
+use Gedmo\Tests\Uploadable\Fixture\Entity\ImageWithTypedProperties;
+use Gedmo\Tests\Uploadable\Stub\MimeTypeGuesserStub;
+use Gedmo\Tests\Uploadable\Stub\UploadableListenerStub;
+
+/**
+ * This test is for Uploadable behavior with typed properties
+ *
+ * @requires PHP >= 7.4
+ */
+final class UploadableEntitySizeTypeTest extends BaseTestCaseORM
+{
+    public const IMAGE_WITH_TYPED_PROPERTIES_CLASS = ImageWithTypedProperties::class;
+
+    /**
+     * @var UploadableListenerStub
+     */
+    private $listener;
+
+    /**
+     * @var string
+     */
+    private $destinationTestDir;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $evm = new EventManager();
+        $this->listener = new UploadableListenerStub();
+        $this->listener->setMimeTypeGuesser(new MimeTypeGuesserStub('text/plain'));
+
+        $evm->addEventSubscriber($this->listener);
+        $config = $this->getDefaultConfiguration();
+        $this->em = $this->getDefaultMockSqliteEntityManager($evm, $config);
+
+        $this->destinationTestDir = TESTS_TEMP_DIR.'/uploadable';
+
+        $this->clearFilesAndDirectories();
+
+        if (!is_dir($this->destinationTestDir)) {
+            mkdir($this->destinationTestDir);
+        }
+    }
+
+    protected function tearDown(): void
+    {
+        $this->clearFilesAndDirectories();
+    }
+
+    public function testUploadableEntity(): void
+    {
+        $testFile = TESTS_PATH.'/data/test_for_typed_properties.txt';
+        $testFilename = substr($testFile, strrpos($testFile, '/') + 1);
+        $testFileSize = 4;
+        $testFileMimeType = 'text/plain';
+
+        $fileInfo = [
+            'tmp_name' => $testFile,
+            'name' => $testFilename,
+            'size' => $testFileSize,
+            'type' => $testFileMimeType,
+            'error' => 0,
+        ];
+
+        $image = new ImageWithTypedProperties();
+        $image->setTitle('456');
+        $this->listener->addEntityFileInfo($image, $fileInfo);
+
+        $this->em->persist($image);
+        $this->em->flush();
+
+        $this->em->refresh($image);
+
+        $file = $image->getFilePath();
+
+        $this->assertPathEquals($image->getPath().'/'.$testFilename, $image->getFilePath());
+        static::assertTrue(is_file($file));
+        static::assertSame((string) $testFileSize, $image->getSize());
+        static::assertSame($testFileMimeType, $image->getMime());
+
+        $this->em->remove($image);
+        $this->em->flush();
+
+        static::assertFalse(is_file($file));
+    }
+
+    protected function getUsedEntityFixtures(): array
+    {
+        return [
+            self::IMAGE_WITH_TYPED_PROPERTIES_CLASS,
+        ];
+    }
+
+    protected function assertPathEquals(string $expected, string $path, string $message = ''): void
+    {
+        static::assertSame($expected, $path, $message);
+    }
+
+    private function clearFilesAndDirectories(): void
+    {
+        if (is_dir($this->destinationTestDir)) {
+            $iter = new \DirectoryIterator($this->destinationTestDir);
+
+            foreach ($iter as $fileInfo) {
+                if (!$fileInfo->isDot()) {
+                    @unlink($fileInfo->getPathname());
+                }
+            }
+        }
+    }
+}
diff --git a/tests/Gedmo/Uploadable/UploadableEntityTest.php b/tests/Gedmo/Uploadable/UploadableEntityTest.php
index 9a982cf9b..62d20ef84 100644
--- a/tests/Gedmo/Uploadable/UploadableEntityTest.php
+++ b/tests/Gedmo/Uploadable/UploadableEntityTest.php
@@ -119,7 +119,7 @@ final class UploadableEntityTest extends BaseTestCaseORM
     /** @var false|string */
     private $testFilenameWithSpaces;
 
-    /** @var string */
+    /** @var int */
     private $testFileSize;
 
     /** @var string */
@@ -152,7 +152,7 @@ protected function setUp(): void
         $this->testFilename3 = substr($this->testFile3, strrpos($this->testFile3, '/') + 1);
         $this->testFilenameWithoutExt = substr($this->testFileWithoutExt, strrpos($this->testFileWithoutExt, '/') + 1);
         $this->testFilenameWithSpaces = substr($this->testFileWithSpaces, strrpos($this->testFileWithSpaces, '/') + 1);
-        $this->testFileSize = '4';
+        $this->testFileSize = 4;
         $this->testFileMimeType = 'text/plain';
 
         $this->clearFilesAndDirectories();
@@ -197,7 +197,7 @@ public function testUploadableEntity(): void
 
         $this->assertPathEquals($image2->getPath().'/'.$fileInfo['name'], $image2->getFilePath());
         static::assertTrue(is_file($firstFile));
-        static::assertSame($fileInfo['size'], $image2->getSize());
+        static::assertSame((string) $fileInfo['size'], $image2->getSize());
         static::assertSame($fileInfo['type'], $image2->getMime());
 
         // UPDATE of an Uploadable Entity
@@ -251,7 +251,7 @@ public function testUploadableEntityWithCompositePath(): void
 
         $this->assertPathEquals($image2->getPath($this->destinationTestDir).'/'.$fileInfo['name'], $image2->getFilePath());
         static::assertTrue(is_file($firstFile));
-        static::assertSame($fileInfo['size'], $image2->getSize());
+        static::assertSame((string) $fileInfo['size'], $image2->getSize());
         static::assertSame($fileInfo['type'], $image2->getMime());
 
         // UPDATE of an Uploadable Entity
@@ -596,7 +596,7 @@ public function testFileNotExceedingMaximumAllowedSizeDoesntThrowException(): vo
         $this->listener->setDefaultPath($this->destinationTestDir);
 
         $file = new FileWithMaxSize();
-        $size = '0.0001';
+        $size = 1;
         $fileInfo = $this->generateUploadedFile(false, false, ['size' => $size]);
 
         $this->listener->addEntityFileInfo($file, $fileInfo);
@@ -606,7 +606,7 @@ public function testFileNotExceedingMaximumAllowedSizeDoesntThrowException(): vo
 
         $this->em->refresh($file);
 
-        static::assertSame($size, $file->getFileSize());
+        static::assertSame((string) $size, $file->getFileSize());
     }
 
     public function testIfMimeTypeGuesserCantResolveTypeThrowException(): void
diff --git a/tests/data/test_for_typed_properties.txt b/tests/data/test_for_typed_properties.txt
new file mode 100644
index 000000000..bcfb9f8cb
--- /dev/null
+++ b/tests/data/test_for_typed_properties.txt
@@ -0,0 +1,1 @@
+test for typed properties