Fix VideoMetadata validation rule for new metadata objects

biigle · Dec 18, 2023 · 2437e98 · 2437e98
1 parent 480cec7
commit 2437e98
Show file tree

Hide file tree

Showing 4 changed files with 234 additions and 105 deletions.
diff --git a/app/Rules/ImageMetadata.php b/app/Rules/ImageMetadata.php
@@ -4,6 +4,7 @@
 
 use Illuminate\Contracts\Validation\Rule;
 use Biigle\Services\MetadataParsing\VolumeMetadata;
+use Biigle\Services\MetadataParsing\FileMetadata;
 
 class ImageMetadata implements Rule
 {
@@ -47,86 +48,95 @@ public function passes($attribute, $value): bool
             throw new \Exception('No value of type '.VolumeMetadata::class.' given.');
         }
 
-        $fileMetadata = $value->getFiles();
         // This checks if any information is given at all.
-        if ($fileMetadata->isEmpty()) {
+        if ($value->isEmpty()) {
             $this->message = 'The metadata information is empty.';
 
             return false;
         }
 
-        foreach ($fileMetadata as $file) {
-
-            if (!in_array($file->name, $this->files)) {
-                $this->message = "There is no file with filename {$file->name}.";
+        $fileMetadata = $value->getFiles();
 
+        foreach ($fileMetadata as $file) {
+            if (!$this->fileMetadataPasses($file)) {
                 return false;
             }
+        }
+
+        return true;
+    }
+
+    /**
+     * Get the validation error message.
+     *
+     * @return string
+     */
+    public function message()
+    {
+        return $this->message;
+    }
 
-            if (!is_null($file->lng)) {
-                if (!is_numeric($file->lng) || abs($file->lng) > 180) {
-                    $this->message = "'{$file->lng}' is no valid longitude for file {$file->name}.";
+    protected function fileMetadataPasses(FileMetadata $file)
+    {
+        if (!in_array($file->name, $this->files)) {
+            $this->message = "There is no file with filename {$file->name}.";
 
-                    return false;
-                }
+            return false;
+        }
 
-                if (is_null($file->lat)) {
-                    $this->message = "Missing latitude for file {$file->name}.";
+        if (!is_null($file->lng)) {
+            if (!is_numeric($file->lng) || abs($file->lng) > 180) {
+                $this->message = "'{$file->lng}' is no valid longitude for file {$file->name}.";
 
-                    return false;
-                }
+                return false;
             }
 
-            if (!is_null($file->lat)) {
-                if (!is_numeric($file->lat) || abs($file->lat) > 90) {
-                    $this->message = "'{$file->lat}' is no valid latitude for file {$file->name}.";
+            if (is_null($file->lat)) {
+                $this->message = "Missing latitude for file {$file->name}.";
 
-                    return false;
-                }
+                return false;
+            }
+        }
 
-                if (is_null($file->lng)) {
-                    $this->message = "Missing longitude for file {$file->name}.";
+        if (!is_null($file->lat)) {
+            if (!is_numeric($file->lat) || abs($file->lat) > 90) {
+                $this->message = "'{$file->lat}' is no valid latitude for file {$file->name}.";
 
-                    return false;
-                }
+                return false;
             }
 
-            // Catch both a malformed date (false) and the zero date (negative integer).
-            if (!is_null($file->takenAt)) {
-                if (!(strtotime($file->takenAt) > 0)) {
-                    $this->message = "'{$file->takenAt}' is no valid date for file {$file->name}.";
+            if (is_null($file->lng)) {
+                $this->message = "Missing longitude for file {$file->name}.";
 
-                    return false;
-                }
+                return false;
             }
+        }
 
-            foreach (self::NUMERIC_FIELDS as $key => $text) {
-                if (!is_null($file->$key) && !is_numeric($file->$key)) {
-                    $this->message = "'{$file->$key}' is no valid {$text} for file {$file->name}.";
+        // Catch both a malformed date (false) and the zero date (negative integer).
+        if (!is_null($file->takenAt)) {
+            if (!(strtotime($file->takenAt) > 0)) {
+                $this->message = "'{$file->takenAt}' is no valid date for file {$file->name}.";
 
-                    return false;
-                }
+                return false;
             }
+        }
 
-            if (!is_null($file->yaw)) {
-                if ($file->yaw < 0 || $file->yaw > 360) {
-                    $this->message = "'{$file->yaw}' is no valid yaw for file {$file->name}.";
+        foreach (self::NUMERIC_FIELDS as $key => $text) {
+            if (!is_null($file->$key) && !is_numeric($file->$key)) {
+                $this->message = "'{$file->$key}' is no valid {$text} for file {$file->name}.";
 
-                    return false;
-                }
+                return false;
             }
         }
 
-        return true;
-    }
+        if (!is_null($file->yaw)) {
+            if ($file->yaw < 0 || $file->yaw > 360) {
+                $this->message = "'{$file->yaw}' is no valid yaw for file {$file->name}.";
 
-    /**
-     * Get the validation error message.
-     *
-     * @return string
-     */
-    public function message()
-    {
-        return $this->message;
+                return false;
+            }
+        }
+
+        return true;
     }
 }
diff --git a/app/Rules/VideoMetadata.php b/app/Rules/VideoMetadata.php
@@ -15,27 +15,13 @@ public function passes($attribute, $value): bool
             return false;
         }
 
-        $columns = array_shift($value);
-
-        $filenames = [];
-        foreach ($value as $index => $row) {
-            $combined = array_combine($columns, $row);
-            $combined = array_filter($combined);
-            $filename = $combined['filename'];
-            if (array_key_exists($filename, $filenames)) {
-                // If this exists, it was already checked if it is a valid date by the
-                // parent method.
-                if (!array_key_exists('taken_at', $combined)) {
-                    // +1 since index starts at 0.
-                    // +1 since column description row was removed above.
-                    $line = $index + 2;
-
-                    $this->message = "File {$filename} has multiple entries but no 'taken_at' at line {$line}.";
+        $fileMetadata = $value->getFiles();
 
+        foreach ($fileMetadata as $file) {
+            foreach ($file->getFrames() as $frame) {
+                if (!$this->fileMetadataPasses($frame)) {
                     return false;
                 }
-            } else {
-                $filenames[$filename] = true;
             }
         }
 

diff --git a/tests/php/Rules/ImageMetadataTest.php b/tests/php/Rules/ImageMetadataTest.php
@@ -99,12 +99,15 @@ public function testEmptyFilename()
     {
         $validator = new ImageMetadataRule(['abc.jpg']);
         $metadata = new VolumeMetadata;
-        $metadata->addFile(new ImageMetadata(
-            name: 'abc.jpg'
-        ));
-        $metadata->addFile(new ImageMetadata(
-            name: ''
-        ));
+        $metadata->addFile(new ImageMetadata(name: ''));
+        $this->assertFalse($validator->passes(null, $metadata));
+    }
+
+    public function testEmpty()
+    {
+        $validator = new ImageMetadataRule(['abc.jpg']);
+        $metadata = new VolumeMetadata;
+        $metadata->addFile(new ImageMetadata(name: 'abc.jpg'));
         $this->assertFalse($validator->passes(null, $metadata));
     }
 }