Removing previous modification of uploaded file (external links need a separate entity). Marking deprecated entry points.

krulis-martin · krulis-martin · commit 175e2e285c3a · 2025-11-14T00:41:56.000+01:00
diff --git a/app/V1Module/presenters/ExerciseFilesPresenter.php b/app/V1Module/presenters/ExerciseFilesPresenter.php
@@ -263,6 +263,7 @@ public function checkUploadAttachmentFiles(string $id)
     /**
      * Associate attachment exercise files with an exercise
      * @POST
+     * @DEPRECATED attachment files were unified with exercise files
      * @throws ForbiddenRequestException
      */
     #[Post("files", new VMixed(), "Identifiers of attachment files", nullable: true)]
@@ -319,6 +320,7 @@ public function checkGetAttachmentFiles(string $id)
     /**
      * Get a list of all attachment files for an exercise
      * @GET
+     * @DEPRECATED attachment files were unified with exercise files
      * @throws ForbiddenRequestException
      */
     #[Path("id", new VUuid(), "identification of exercise", required: true)]
@@ -345,6 +347,7 @@ public function checkDeleteAttachmentFile(string $id, string $fileId)
     /**
      * Delete attachment exercise file with given id
      * @DELETE
+     * @DEPRECATED attachment files were unified with exercise files
      * @throws ForbiddenRequestException
      * @throws NotFoundException
      */
@@ -408,6 +411,7 @@ public function checkDownloadAttachmentFilesArchive(string $id)
     /**
      * Download archive containing all attachment files for exercise.
      * @GET
+     * @DEPRECATED attachment files were unified with exercise files
      * @throws NotFoundException
      * @throws \Nette\Application\BadRequestException
      * @throws \Nette\Application\AbortException
diff --git a/app/V1Module/presenters/UploadedFilesPresenter.php b/app/V1Module/presenters/UploadedFilesPresenter.php
@@ -353,7 +353,7 @@ public function actionUpload()
             );
         }
 
-        // In theory, this may create race condition (DB record is commited before file is moved).
+        // In theory, this may create race condition (DB record is committed before file is moved).
         // But we need the ID from the database so we can save the file.
         $uploadedFile = new UploadedFile($file->getName(), new DateTime(), $file->getSize(), $user);
         $this->uploadedFiles->persist($uploadedFile);
@@ -528,8 +528,8 @@ public function checkCompletePartial(string $id)
     }
 
     /**
-     * Finalize partial upload and convert the partial file into UploadFile.
-     * All data chunks are extracted from the store, assembled into one file, and is moved back into the store.
+     * Finalize partial upload and convert the partial file into UploadedFile entity.
+     * All data chunks are extracted from the store, assembled into one file, and that is moved back into the store.
      * @POST
      */
     #[Path("id", new VUuid(), "Identifier of the partial file", required: true)]
@@ -594,6 +594,7 @@ public function checkDownloadExerciseFile(string $id)
     /**
      * Download exercise file
      * @GET
+     * @DEPRECATED use generic uploaded-file download endpoint instead
      * @throws ForbiddenRequestException
      * @throws NotFoundException
      * @throws \Nette\Application\AbortException
diff --git a/app/V1Module/router/RouterFactory.php b/app/V1Module/router/RouterFactory.php
@@ -173,7 +173,7 @@ private static function createExercisesRoutes(string $prefix): RouteList
             "ExerciseFiles:downloadExerciseFilesArchive"
         );
 
-        // deprecated routes for supplementary files
+        // deprecated routes for supplementary files (replaced with exercise-files)
         $router[] = new GetRoute("$prefix/<id>/supplementary-files", "ExerciseFiles:getExerciseFiles");
         $router[] = new PostRoute("$prefix/<id>/supplementary-files", "ExerciseFiles:uploadExerciseFiles");
         $router[] = new DeleteRoute(
@@ -185,6 +185,7 @@ private static function createExercisesRoutes(string $prefix): RouteList
             "ExerciseFiles:downloadExerciseFilesArchive"
         );
 
+        // deprecated (will be removed with AttachmentFile entity, unified with exercise-files)
         $router[] = new GetRoute("$prefix/<id>/attachment-files", "ExerciseFiles:getAttachmentFiles");
         $router[] = new PostRoute("$prefix/<id>/attachment-files", "ExerciseFiles:uploadAttachmentFiles");
         $router[] = new DeleteRoute("$prefix/<id>/attachment-files/<fileId>", "ExerciseFiles:deleteAttachmentFile");
@@ -470,11 +471,13 @@ private static function createUploadedFilesRoutes(string $prefix): RouteList
         $router[] = new PostRoute("$prefix/partial/<id>", "UploadedFiles:completePartial");
 
         $router[] = new PostRoute("$prefix", "UploadedFiles:upload");
-        $router[] = new GetRoute("$prefix/supplementary-file/<id>/download", "UploadedFiles:downloadExerciseFile");
         $router[] = new GetRoute("$prefix/<id>", "UploadedFiles:detail");
         $router[] = new GetRoute("$prefix/<id>/download", "UploadedFiles:download");
         $router[] = new GetRoute("$prefix/<id>/content", "UploadedFiles:content");
         $router[] = new GetRoute("$prefix/<id>/digest", "UploadedFiles:digest");
+
+        // deprecated (should be handled by generic download)
+        $router[] = new GetRoute("$prefix/supplementary-file/<id>/download", "UploadedFiles:downloadExerciseFile");
         return $router;
     }
 
diff --git a/app/V1Module/security/Policies/UploadedFilePermissionPolicy.php b/app/V1Module/security/Policies/UploadedFilePermissionPolicy.php
@@ -2,10 +2,10 @@
 
 namespace App\Security\Policies;
 
-use App\Model\Entity\Assignment;
 use App\Model\Entity\AttachmentFile;
 use App\Model\Entity\Exercise;
 use App\Model\Entity\ExerciseFile;
+use App\Model\Entity\Group;
 use App\Model\Entity\UploadedFile;
 use App\Model\Repository\Assignments;
 use App\Model\Repository\UploadedFiles;
@@ -61,6 +61,7 @@ public function isAuthorOfFileExercises(Identity $identity, UploadedFile $file)
         }
 
         foreach ($file->getExercises() as $exercise) {
+            /** @var Exercise $exercise */
             if ($exercise->isAuthor($user)) {
                 return true;
             }
@@ -82,7 +83,9 @@ public function isExerciseFileInGroupUserSupervises(Identity $identity, Uploaded
         }
 
         foreach ($file->getExercises() as $exercise) {
+            /** @var Exercise $exercise */
             foreach ($exercise->getGroups() as $group) {
+                /** @var Group $group */
                 if ($group->isAdminOrSupervisorOfSubgroup($user)) {
                     return true;  // The user can assign one of the corresponding exercises in hir group.
                 }
@@ -103,7 +106,7 @@ public function isOwner(Identity $identity, UploadedFile $file)
         return $file->getUser() && $file->getUser()->getId() === $user->getId();
     }
 
-    public function isReferenceSolutionInSupervisedOrObserverdSubGroup(Identity $identity, UploadedFile $file)
+    public function isReferenceSolutionInSupervisedOrObservedSubGroup(Identity $identity, UploadedFile $file)
     {
         $user = $identity->getUserData();
         if ($user === null) {
@@ -140,6 +143,7 @@ public function isRelatedToAssignment(Identity $identity, UploadedFile $file)
 
         if ($file instanceof AttachmentFile) {
             foreach ($file->getExercises() as $exercise) {
+                /** @var Exercise $exercise */
                 foreach ($user->getGroups() as $group) {
                     if ($this->assignments->isAssignedToGroup($exercise, $group)) {
                         return true;
diff --git a/app/config/permissions.neon b/app/config/permissions.neon
@@ -43,7 +43,7 @@ permissions:
       actions:
         - takeOver
 
-    - allow: false  # A safety mechanism that prevents us from accidentaly allowing account takeover to non-admins (e.g. by wildcard rules)
+    - allow: false  # A safety mechanism that prevents us from accidentally allowing account takeover to non-admins (e.g. by wildcard rules)
       actions:
         - takeOver
 
@@ -743,7 +743,7 @@ permissions:
       actions:
           - viewThread
           - addComment
-      conditions:  # TODO - make sure only apropriate group members/authors can do this
+      conditions:  # TODO - make sure only appropriate group members/authors can do this
           - userIsNotGroupLocked
 
     - allow: true
@@ -1030,15 +1030,15 @@ permissions:
       actions:
           - download
           - viewDetail
-          - downloadExerciseFile
+          - downloadExerciseFile  # deprecated (replaced with generic download)
 
     - allow: true
       role: scope-ref-solutions
       resource: uploadedFile
       actions:
           - download
           - viewDetail
-          - downloadExerciseFile
+          - downloadExerciseFile  # deprecated (replaced with generic download)
           - upload
 
     - allow: true
@@ -1075,11 +1075,14 @@ permissions:
       conditions:
           or:
             - file.isSolutionInSupervisedOrObservedGroup
-            - file.isReferenceSolutionInSupervisedOrObserverdSubGroup
+            - file.isReferenceSolutionInSupervisedOrObservedSubGroup
             - file.isPublic
             - file.isOwner
-            - file.isRelatedToAssignment
+            - file.isRelatedToAssignment  # deprecated (will be removed with Attachment files)
+            - file.isAuthorOfFileExercises
+            - file.isExerciseFileInGroupUserSupervises
 
+    # deprecated (will be removed with Attachment files)
     - allow: true
       role: supervisor-student
       resource: uploadedFile
@@ -1094,8 +1097,7 @@ permissions:
       role: supervisor-student
       resource: uploadedFile
       actions:
-          - viewDetail
-          - downloadExerciseFile
+          - downloadExerciseFile  # deprecated (replaced with generic download)
       conditions:
           or:
             - file.isAuthorOfFileExercises
diff --git a/app/model/entity/UploadedFile.php b/app/model/entity/UploadedFile.php
@@ -52,14 +52,6 @@ public function getFileExtension(): string
      */
     protected $isPublic;
 
-    /**
-     * External identification (used in URL) that allows for accessing the file without authentication.
-     * If empty, the file is not accessible this way.
-     * The identifier should be a generated random string.
-     * @ORM\Column(type="string", length=16, nullable=true, unique=true)
-     */
-    protected $external = null;
-
     /**
      * @ORM\Column(type="integer")
      */
@@ -128,26 +120,6 @@ public function setPublic(bool $isPublic): void
         $this->isPublic = $isPublic;
     }
 
-    public function getExternal(): ?string
-    {
-        return $this->external;
-    }
-
-    public function setExternal(?string $external): void
-    {
-        $this->external = $external;
-    }
-
-    /**
-     * Generate a random external ID for this file which uses only URL-safe characters.
-     */
-    public function generateRandomExternalId(): void
-    {
-        // the + and / characters are replaced to make the string URL-safe without the need for encoding
-        // (base64 encoding uses A-Z, a-z, 0-9, +, /)
-        $this->external = str_replace(['+', '/'], ['-', '$'], base64_encode(random_bytes(12)));
-    }
-
     /**
      * Retrieve a corresponding file object from file storage.
      * @param FileStorageManager $manager the storage that retrieves the file
diff --git a/migrations/Version20251113182809.php b/migrations/Version20251113182809.php
