Updating exercise file link interactions with other entities and updating exercise file replacements (to correctly preserve links).

Author: krulis-martin

app/V1Module/presenters/ExerciseFilesPresenter.php

@@ -126,6 +126,7 @@ public function actionUploadExerciseFiles(string $id)
 
         $files = $this->uploadedFiles->findAllById($this->getRequest()->getPost("files"));
         $currentFiles = [];
+        $filesToRemove = [];
         $totalFileSize = 0;
 
         /** @var ExerciseFile $file */
@@ -145,7 +146,7 @@ public function actionUploadExerciseFiles(string $id)
             if (array_key_exists($file->getName(), $currentFiles)) {
                 /** @var ExerciseFile $currentFile */
                 $currentFile = $currentFiles[$file->getName()];
-                $exercise->getExerciseFiles()->removeElement($currentFile);
+                $filesToRemove[$file->getName()] = $currentFile;
                 $totalFileSize -= $currentFile->getFileSize();
             } else {
                 $totalFileCount += 1;
@@ -174,6 +175,20 @@ public function actionUploadExerciseFiles(string $id)
         foreach ($files as $file) {
             $hash = $this->fileStorage->storeUploadedExerciseFile($file);
             $exerciseFile = ExerciseFile::fromUploadedFileAndExercise($file, $exercise, $hash);
+            if (array_key_exists($file->getName(), $filesToRemove)) {
+                /** @var ExerciseFile $currentFile */
+                $fileToRemove = $filesToRemove[$file->getName()];
+
+                // move links from old file to the new one
+                $links = $this->fileLinks->findBy(["exerciseFile" => $fileToRemove, "exercise" => $exercise]);
+                foreach ($links as $link) {
+                    $link->setExerciseFile($exerciseFile);
+                    $this->fileLinks->persist($link, false);
+                }
+
+                $exercise->getExerciseFiles()->removeElement($fileToRemove);
+            }
+
             $this->uploadedFiles->persist($exerciseFile, false);
             $this->uploadedFiles->remove($file, false);
         }

app/model/entity/ExerciseFile.php

@@ -63,13 +63,6 @@ function (Assignment $assignment) {
      */
     protected $pipelines;
 
-    /**
-     * @ORM\OneToMany(targetEntity="ExerciseFileLink", mappedBy="exerciseFile")
-     * @var Collection<ExerciseFileLink>
-     */
-    protected $links;
-
-
     /**
      * ExerciseFile constructor.
      * @param string $name
@@ -95,7 +88,6 @@ public function __construct(
         $this->exercises = new ArrayCollection();
         $this->assignments = new ArrayCollection();
         $this->pipelines = new ArrayCollection();
-        $this->links = new ArrayCollection();
 
         if ($exercise) {
             $this->exercises->add($exercise);

app/model/entity/ExerciseFileLink.php

@@ -8,9 +8,10 @@
 
 /**
  * Additional identification for files that are accessible to users via stable URLs.
- * The entity always points to a single ExerciseFile, but also follows the CoW principle used for all
- * exercise-assignment records. I.e., when Assignment is created from Exercise, the links are copied
- * as well, but they still point to the same ExerciseFile entities.
+ * The entity always points to a single ExerciseFile, but also follows the principles used for all
+ * exercise-assignment records. However, links are small, so they are copied eagerly (not by CoW).
+ * I.e., when Assignment is created from Exercise, the links are copied (immediately) as well.
+ * The link of an exercise may be updated, but the link of an assignment is immutable.
  * @ORM\Entity
  * @ORM\Table(uniqueConstraints={@ORM\UniqueConstraint(columns={"key", "exercise_id"})})
  */
@@ -45,19 +46,21 @@ class ExerciseFileLink implements JsonSerializable
      */
     protected $requiredRole;
 
-
     /**
-     * @ORM\ManyToOne(targetEntity="ExerciseFile", inversedBy="links", cascade={"persist", "remove"})
+     * @ORM\ManyToOne(targetEntity="ExerciseFile")
+     * @ORM\JoinColumn(onDelete="CASCADE")
      */
     protected $exerciseFile;
 
     /**
      * @ORM\ManyToOne(targetEntity="Exercise", inversedBy="fileLinks")
+     * @ORM\JoinColumn(onDelete="CASCADE")
      */
     protected $exercise;
 
     /**
      * @ORM\ManyToOne(targetEntity="Assignment", inversedBy="fileLinks")
+     * @ORM\JoinColumn(onDelete="CASCADE")
      */
     protected $assignment;
 
@@ -177,6 +180,16 @@ public function getExerciseFile(): ExerciseFile
         return $this->exerciseFile;
     }
 
+    /**
+     * This is a rare case where we allow changing the linked ExerciseFile.
+     * It is used only when the immutable ExerciseFile is being replaced.
+     * Changing the link to point to a different ExerciseFile should not be allowed otherwise.
+     */
+    public function setExerciseFile(ExerciseFile $exerciseFile): void
+    {
+        $this->exerciseFile = $exerciseFile;
+    }
+
     public function getExercise(): ?Exercise
     {
         return $this->exercise;

app/model/entity/base/ExerciseData.php

@@ -365,7 +365,7 @@ function (AttachmentFile $file) {
     }
 
     /**
-     * @ORM\OneToMany(targetEntity="ExerciseFileLink", mappedBy="exerciseFile", cascade={"persist", "remove"})
+     * @ORM\OneToMany(targetEntity="ExerciseFileLink", mappedBy="exerciseFile", cascade={"persist"})
      * @var Collection<ExerciseFileLink>
      */
     protected $fileLinks;

migrations/Version20251114174621.php

@@ -21,9 +21,9 @@ public function up(Schema $schema): void
     {
         // this up() migration is auto-generated, please modify it to your needs
         $this->addSql('CREATE TABLE exercise_file_link (id CHAR(36) NOT NULL COMMENT \'(DC2Type:uuid)\', exercise_file_id CHAR(36) DEFAULT NULL COMMENT \'(DC2Type:uuid)\', exercise_id CHAR(36) DEFAULT NULL COMMENT \'(DC2Type:uuid)\', assignment_id CHAR(36) DEFAULT NULL COMMENT \'(DC2Type:uuid)\', `key` VARCHAR(16) NOT NULL, save_name VARCHAR(255) DEFAULT NULL, required_role VARCHAR(255) DEFAULT NULL, created_at DATETIME NOT NULL, INDEX IDX_1187F77549DE8E29 (exercise_file_id), INDEX IDX_1187F775E934951A (exercise_id), INDEX IDX_1187F775D19302F8 (assignment_id), UNIQUE INDEX UNIQ_1187F7758A90ABA9E934951A (`key`, exercise_id), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
-        $this->addSql('ALTER TABLE exercise_file_link ADD CONSTRAINT FK_1187F77549DE8E29 FOREIGN KEY (exercise_file_id) REFERENCES `uploaded_file` (id)');
-        $this->addSql('ALTER TABLE exercise_file_link ADD CONSTRAINT FK_1187F775E934951A FOREIGN KEY (exercise_id) REFERENCES exercise (id)');
-        $this->addSql('ALTER TABLE exercise_file_link ADD CONSTRAINT FK_1187F775D19302F8 FOREIGN KEY (assignment_id) REFERENCES assignment (id)');
+        $this->addSql('ALTER TABLE exercise_file_link ADD CONSTRAINT FK_1187F775D19302F8 FOREIGN KEY (assignment_id) REFERENCES assignment (id) ON DELETE CASCADE');
+        $this->addSql('ALTER TABLE exercise_file_link ADD CONSTRAINT FK_1187F775E934951A FOREIGN KEY (exercise_id) REFERENCES exercise (id) ON DELETE CASCADE');
+        $this->addSql('ALTER TABLE exercise_file_link ADD CONSTRAINT FK_1187F77549DE8E29 FOREIGN KEY (exercise_file_id) REFERENCES `uploaded_file` (id) ON DELETE CASCADE');
         $this->addSql('ALTER TABLE uploaded_file DROP is_public');
     }