FileStorage: improve the ownership management

src/Entity/Core/File.php

@@ -98,13 +98,6 @@ class File
     #[ORM\Column(type: 'text', length: 255, nullable: true)]
     private ?string $config;
 
-    /**
-     * Dossier contenant le fichier
-     * @var string
-     */
-    #[ORM\Column(length: 24)]
-    private string $folder;
-
     /**
      * Type de document (uploaded, mail, bill...etc)
      * @var string
@@ -386,25 +379,6 @@ class File
         return $this;
     }
 
-    /**
-     * @return string
-     */
-    public function getFolder(): string
-    {
-        return $this->folder;
-    }
-
-    /**
-     * @param string $folder
-     * @return File
-     */
-    public function setFolder(string $folder): self
-    {
-        $this->folder = $folder;
-
-        return $this;
-    }
-
     /**
      * @return string
      */

src/Service/Storage/FileStorage.php

@@ -9,11 +9,13 @@ use App\Entity\Organization\Organization;
 use App\Entity\Person\Person;
 use App\Enum\Core\FileStatusEnum;
 use App\Enum\Core\FileTypeEnum;
+use App\Repository\Access\AccessRepository;
 use App\Service\Utils\Path;
 use DateTime;
 use Doctrine\ORM\EntityManagerInterface;
 use Exception;
 use Gaufrette\Filesystem;
+use JetBrains\PhpStorm\Pure;
 use Knp\Bundle\GaufretteBundle\FilesystemMap;
 use Mimey\MimeTypes;
 use Ramsey\Uuid\Uuid;
@@ -33,7 +35,8 @@ class FileStorage
 
     public function __construct(
         protected FilesystemMap $filesystemMap,
-        protected EntityManagerInterface $entityManager
+        protected EntityManagerInterface $entityManager,
+        protected AccessRepository $accessRepository
     )
     {
         $this->filesystem = $filesystemMap->get(static::FS_KEY);
@@ -41,7 +44,7 @@ class FileStorage
 
     /**
      * Return true if the file exists in the file storage
-     * 
+     *
      * @param File $file
      * @return bool
      */
@@ -111,14 +114,10 @@ class FileStorage
             ->setCreatedBy($createdBy->getId())
             ->setStatus(FileStatusEnum::PENDING()->getValue());
 
-        if ($owner instanceof Access) {
-            $file->setOrganization($owner->getOrganization())
-                ->setPerson($owner->getPerson());
-        } else if ($owner instanceof Organization) {
-            $file->setOrganization($owner);
-        } else if ($owner instanceof Person) {
-            $file->setPerson($owner);
-        }
+        [$organization, $person] = $this->getOrganizationAndPersonFromOwner($owner);
+
+        $file->setOrganization($organization)
+             ->setPerson($person);
 
         $this->entityManager->persist($file);
 
@@ -143,9 +142,16 @@ class FileStorage
             throw new RuntimeException('File has no filename');
         }
 
-        /** @noinspection ProperNullCoalescingOperatorUsageInspection */
+        // Try to get the Access owner from the organization_id and person_id
+        $access = null;
+        if ($file->getOrganization() !== null && $file->getPerson()) {
+            $access = $this->accessRepository->findOneBy(
+                ['organization' => $file->getOrganization(), 'person' => $file->getPerson()]
+            );
+        }
+
         $prefix = $this->getPrefix(
-            $file->getOrganization() ?? $file->getPerson(),
+            $access ?? $file->getOrganization() ?? $file->getPerson(),
             $file->getIsTemporaryFile(),
             $file->getType()
         );
@@ -212,8 +218,8 @@ class FileStorage
             $type,
             $author,
             $isTemporary,
-            $mimeType,
             $visibility,
+            $mimeType,
             false
         );
 
@@ -243,24 +249,32 @@ class FileStorage
     }
 
     /**
-     * If an organization or access owns the file, the prefix will be '(_temp_/)organization/{id}'.
-     * If a person owns it, the prefix will be '{temp}/person/{id}'
+     * If an organization owns the file, the prefix will be '(_temp_/)organization/{id}(/{type})'.
+     * If a person owns it, the prefix will be '(_temp_/)person/{id}(/{type})'
+     * If access owns it, the prefix will be '(_temp_/)organization/{organization_id}/{access_id}(/{type})'
      *
      * With {id} being the id of the organization or of the person.
      *
      * If the file is temporary, '_temp_/' is prepended to the prefix.
+     * If a file type is given, this type is appended to the prefix (low case)
      *
      * @param Organization|Access|Person $owner
      * @param bool $isTemporary
      * @param string|null $type
      * @return string
      */
-    private function getPrefix(Organization | Access | Person $owner, bool $isTemporary, string $type = null): string
+    private function getPrefix(
+        Organization | Access | Person $owner,
+        bool $isTemporary,
+        string $type = null
+    ): string
     {
         if ($owner instanceof Access) {
-            $prefix = Path::join('organization', $owner->getOrganization()?->getId(), $owner->getPerson()?->getId());
+            $prefix = Path::join('organization', $owner->getOrganization()?->getId(), $owner->getId());
+        } else if ($owner instanceof Organization) {
+            $prefix = Path::join('organization', $owner->getId());
         } else {
-            $prefix = Path::join($owner instanceof Person ? 'person' : 'organization', $owner->getId());
+            $prefix = Path::join('person', $owner->getId());
         }
 
         if ($isTemporary) {
@@ -289,4 +303,23 @@ class FileStorage
         }
         return (new MimeTypes)->getMimeType($ext);
     }
+
+    /**
+     * Return an array [$organization, $person] from a given owner
+     *
+     * @param Organization|Access|Person $owner
+     * @return array
+     */
+    #[Pure]
+    private function getOrganizationAndPersonFromOwner(Organization | Access | Person $owner): array {
+        if ($owner instanceof Access) {
+            return [$owner->getOrganization(), $owner->getPerson()];
+        }
+
+        if ($owner instanceof Organization) {
+            return [$owner, null];
+        }
+
+        return [null, $owner];
+    }
 }