add binext

Signed-off-by: Varun Patil <varunpatil@ucla.edu>

lib/BinExt.php

@@ -0,0 +1,75 @@
+<?php
+
+namespace OCA\Memories;
+
+class BinExt
+{
+    public const EXIFTOOL_VER = '12.58';
+
+    /** Test configured exiftool binary */
+    public static function testExiftool(): bool
+    {
+        $cmd = implode(' ', array_merge(self::getExiftool(), ['-ver']));
+        $out = shell_exec($cmd);
+        if (!$out) {
+            throw new \Exception('failed to run exiftool');
+        }
+
+        $version = trim($out);
+        $target = self::EXIFTOOL_VER;
+        if (!version_compare($version, $target, '=')) {
+            throw new \Exception("version does not match {$version} <==> {$target}");
+        }
+
+        return true;
+    }
+
+    /** Get path to exiftool binary for proc_open */
+    public static function getExiftool(): array
+    {
+        if (Util::getSystemConfig('memories.exiftool_no_local')) {
+            return ['perl', __DIR__.'/../exiftool-bin/exiftool/exiftool'];
+        }
+
+        return [Util::getSystemConfig('memories.exiftool')];
+    }
+
+    /** Detect the exiftool binary to use */
+    public static function detectExiftool(): void
+    {
+        if (!empty($path = Util::getSystemConfig('memories.exiftool'))) {
+            if (file_exists($path) && !is_executable($path)) {
+                chmod($path, 0755);
+            }
+
+            return;
+        }
+
+        if (Util::getSystemConfig('memories.exiftool_no_local')) {
+            return;
+        }
+
+        // Detect architecture
+        $arch = \OCA\Memories\Util::getArch();
+        $libc = \OCA\Memories\Util::getLibc();
+
+        // Get static binary if available
+        if ($arch && $libc) {
+            // get target file path
+            $path = realpath(__DIR__."/../exiftool-bin/exiftool-{$arch}-{$libc}");
+
+            // Set config
+            Util::setSystemConfig('memories.exiftool', $path);
+
+            // make sure it is executable
+            if (file_exists($path)) {
+                if (!is_executable($path)) {
+                    chmod($path, 0755);
+                }
+            } else {
+                error_log("Exiftool binary not found: {$path}");
+                Util::setSystemConfig('memories.exiftool_no_local', true);
+            }
+        }
+    }
+}

lib/Controller/OtherController.php

@@ -24,8 +24,8 @@ declare(strict_types=1);
 namespace OCA\Memories\Controller;
 
 use OCA\Memories\AppInfo\Application;
+use OCA\Memories\BinExt;
 use OCA\Memories\Exceptions;
-use OCA\Memories\Exif;
 use OCA\Memories\Util;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
@@ -87,22 +87,7 @@ class OtherController extends GenericApiController
                 throw Exceptions::Forbidden('Cannot change settings in readonly mode');
             }
 
-            // Make sure the key is valid
+            Util::setSystemConfig($key, $value);
-            $defaults = Util::systemConfigDefaults();
-            if (!\array_key_exists($key, $defaults)) {
-                throw Exceptions::BadRequest('Invalid key');
-            }
-
-            // Make sure the value has the same type as the default value
-            if (\gettype($value) !== \gettype($defaults[$key])) {
-                throw Exceptions::BadRequest('Invalid value type');
-            }
-
-            if ($value === $defaults[$key]) {
-                $this->config->deleteSystemValue($key);
-            } else {
-                $this->config->setSystemValue($key, $value);
-            }
 
             return new JSONResponse([], Http::STATUS_OK);
         });
@@ -118,8 +103,16 @@ class OtherController extends GenericApiController
         return Util::guardEx(function () {
             $status = [];
 
-            // Check exiftool
+            // Check exiftool version
             $status['exiftool'] = $this->getExecutableStatus(Util::getSystemConfig('memories.exiftool'));
+            if ('ok' === $status['exiftool'] || Util::getSystemConfig('memories.exiftool_no_local')) {
+                try {
+                    BinExt::testExiftool();
+                    $status['exiftool'] = 'test_ok';
+                } catch (\Exception $e) {
+                    $status['exiftool'] = 'test_fail:'.$e->getMessage();
+                }
+            }
 
             // Check for system perl
             $status['perl'] = $this->getExecutableStatus(exec('which perl'));

lib/Exif.php

@@ -11,7 +11,6 @@ use OCP\IConfig;
 class Exif
 {
     private const FORBIDDEN_EDIT_MIMES = ['image/bmp', 'image/x-dcraw', 'video/MP2T'];
-    private const EXIFTOOL_VER = '12.58';
     private const EXIFTOOL_TIMEOUT = 30000;
     private const EXIFTOOL_ARGS = ['-api', 'QuickTimeUTC=1', '-n', '-U', '-json', '--b'];
 
@@ -357,66 +356,9 @@ class Exif
         return self::getExifFromLocalPathWithSeparateProc($path, ['-G4']);
     }
 
-    /** Get path to exiftool binary */
+    private static function getExiftool(): array
-    private static function getExiftool()
     {
-        $configKey = 'memories.exiftool';
+        return BinExt::getExiftool();
-        $config = \OC::$server->get(IConfig::class);
-        $configPath = $config->getSystemValue($configKey);
-        $noLocal = $config->getSystemValue($configKey.'_no_local', false);
-
-        // We know already where it is
-        if (!empty($configPath) && file_exists($configPath)) {
-            if (!is_executable($configPath)) {
-                chmod($configPath, 0755);
-            }
-
-            return explode(' ', $configPath);
-        }
-
-        // Detect architecture
-        $arch = $noLocal ? null : \OCA\Memories\Util::getArch();
-        $libc = $noLocal ? null : \OCA\Memories\Util::getLibc();
-
-        // Get static binary if available
-        if ($arch && $libc && !$noLocal) {
-            // get target file path
-            $path = realpath(__DIR__."/../exiftool-bin/exiftool-{$arch}-{$libc}");
-
-            // check if file exists
-            if (file_exists($path)) {
-                // make executable before version check
-                if (!is_executable($path)) {
-                    chmod($path, 0755);
-                }
-
-                // check if the version prints correctly
-                $ver = self::EXIFTOOL_VER;
-                $vero = shell_exec("{$path} -ver");
-                if ($vero && false !== stripos(trim($vero), $ver)) {
-                    $out = trim($vero);
-                    echo "Exiftool binary version check passed {$out} <==> {$ver}\n";
-                    $config->setSystemValue($configKey, $path);
-
-                    return [$path];
-                }
-                error_log("Exiftool version check failed {$vero} <==> {$ver}");
-                $config->setSystemValue($configKey.'_no_local', true);
-            } else {
-                error_log("Exiftool not found: {$path}");
-            }
-        }
-
-        // Fallback to perl script
-        $path = __DIR__.'/../exiftool-bin/exiftool/exiftool';
-        if (file_exists($path)) {
-            return ['perl', $path];
-        }
-
-        error_log("Exiftool not found: {$path}");
-
-        // Fallback to system binary
-        return ['exiftool'];
     }
 
     /** Initialize static exiftool process for local reads */

lib/Migration/Repair.php

@@ -24,6 +24,9 @@ class Repair implements IRepairStep
 
     public function run(IOutput $output): void
     {
+        // detect exiftool binary
+        \OCA\Memories\BinExt::detectExiftool();
+
         // kill any instances of go-transcode and go-vod
         \OCA\Memories\Util::pkill('go-transcode');
         \OCA\Memories\Util::pkill('go-vod');

lib/Util.php

@@ -294,6 +294,38 @@ class Util
         return $config->getSystemValue($key, $default ?? self::systemConfigDefaults()[$key]);
     }
 
+    /**
+     * Set a system config key.
+     *
+     * @param mixed $value
+     *
+     * @throws \InvalidArgumentException
+     */
+    public static function setSystemConfig(string $key, $value): void
+    {
+        $config = \OC::$server->get(\OCP\IConfig::class);
+
+        // Check if the key is valid
+        $defaults = self::systemConfigDefaults();
+        if (!\array_key_exists($key, $defaults)) {
+            throw new \InvalidArgumentException('Invalid system config key');
+        }
+
+        // Check if the value has the correct type
+        if (null !== $value && \gettype($value) !== \gettype($defaults[$key])) {
+            $expected = \gettype($defaults[$key]);
+            $got = \gettype($value);
+
+            throw new \InvalidArgumentException("Invalid type for system config {$key}, expected {$expected}, got {$got}");
+        }
+
+        if ($value === $defaults[$key] || null === $value) {
+            $config->deleteSystemValue($key);
+        } else {
+            $config->setSystemValue($key, $value);
+        }
+    }
+
     /** Get list of defaults for all system config keys. */
     public static function systemConfigDefaults(): array
     {

src/Admin.vue

@@ -430,7 +430,7 @@ export default defineComponent({
       } else if (status.startsWith("test_fail")) {
         return this.t(
           "memories",
-          "{name} binary failed test: {info}",
+          "{name} failed test: {info}",
           {
             name,
             info: status.substring(10),