extend modal dialog for uploading from server

app/Actions/Import/Exec.php

@@ -6,6 +6,8 @@
 use App\Actions\Import\Extensions\ImportPhoto;
 use App\Actions\Photo\Extensions\Constants;
 use App\Assets\Helpers;
+use App\Exceptions\PhotoResyncedException;
+use App\Exceptions\PhotoSkippedException;
 use App\Models\Album;
 use App\Models\Configs;
 use App\Models\Logs;
@@ -16,9 +18,10 @@ class Exec
 	use ImportPhoto;
 	use Constants;
 
-	public $force_skip_duplicates = false;
+	public $skip_duplicates = false;
 	public $resync_metadata = false;
 	public $delete_imported;
+	public $import_via_symlink;
 
 	public $memCheck = true;
 	public $statusCLIFormatting = false;
@@ -57,7 +60,7 @@ private function status_update(string $status)
 			}
 			flush();
 		} else {
-			echo $status . PHP_EOL;
+			echo substr($status, strpos($status, ' ') + 1) . PHP_EOL;
 		}
 	}
 
@@ -211,7 +214,16 @@ public function do(
 			$is_raw = in_array(strtolower($extension), $this->raw_formats, true);
 			if (@exif_imagetype($file) !== false || in_array(strtolower($extension), $this->validExtensions, true) || $is_raw) {
 				// Photo or Video
-				if ($this->photo($file, $this->delete_imported, $albumID, $this->force_skip_duplicates, $this->resync_metadata) === false) {
+				try {
+					if ($this->photo($file, $this->delete_imported, $this->import_via_symlink, $albumID, $this->skip_duplicates, $this->resync_metadata) === false) {
+						$this->status_update('Problem: ' . $file . ': Could not import file');
+						Logs::error(__METHOD__, __LINE__, 'Could not import file (' . $file . ')');
+					}
+				} catch (PhotoSkippedException $e) {
+					$this->status_update('Problem: ' . $file . ': Skipped duplicate');
+				} catch (PhotoResyncedException $e) {
+					$this->status_update('Problem: ' . $file . ': Skipped duplicate (resynced metadata)');
+				} catch (Exception $e) {
 					$this->status_update('Problem: ' . $file . ': Could not import file');
 					Logs::error(__METHOD__, __LINE__, 'Could not import file (' . $file . ')');
 				}
@@ -226,7 +238,7 @@ public function do(
 		foreach ($dirs as $dir) {
 			// Folder
 			$album = null;
-			if ($this->force_skip_duplicates || Configs::get_value('skip_duplicates', '0') === '1') {
+			if ($this->skip_duplicates) {
 				$album = Album::where('parent_id', '=', $albumID == 0 ? null : $albumID)
 					->where('title', '=', basename($dir))
 					->get()

app/Actions/Import/Extensions/ImportPhoto.php

@@ -3,7 +3,6 @@
 namespace App\Actions\Import\Extensions;
 
 use App\Actions\Photo\Create;
-use Exception;
 
 trait ImportPhoto
 {
@@ -12,12 +11,14 @@ trait ImportPhoto
 	 *
 	 * @param $path
 	 * @param bool $delete_imported
+	 * @param bool $import_via_symlink
 	 * @param int  $albumID
-	 * @param bool $force_skip_duplicates
+	 * @param bool $skip_duplicates
+	 * @param bool $resync_metadata
 	 *
 	 * @return bool returns true when photo import was successful
 	 */
-	public function photo($path, $delete_imported, $albumID = 0, $force_skip_duplicates = false, $resync_metadata = false)
+	public function photo($path, $delete_imported, $import_via_symlink, $albumID = 0, $skip_duplicates = false, $resync_metadata = false)
 	{
 		// No need to validate photo type and extension in this function.
 		// $photo->add will take care of it.
@@ -30,17 +31,20 @@ public function photo($path, $delete_imported, $albumID = 0, $force_skip_duplica
 
 		$create = resolve(Create::class);
 
-		try {
-			if ($create->add($nameFile, $albumID, $delete_imported, $force_skip_duplicates, $resync_metadata) === false) {
-				// @codeCoverageIgnoreStart
-				return false;
-				// @codeCoverageIgnoreEnd
-			}
+		// avoid incompatible settings (delete originals takes precedence over symbolic links)
+		if ($delete_imported) {
+			$import_via_symlink = false;
+		}
+		// (re-syncing metadata makes no sense when importing duplicates)
+		if (!$skip_duplicates) {
+			$resync_metadata = false;
+		}
+
+		if ($create->add($nameFile, $albumID, $delete_imported, $skip_duplicates, $import_via_symlink, $resync_metadata) === false) {
 			// @codeCoverageIgnoreStart
-		} catch (Exception $e) {
 			return false;
+			// @codeCoverageIgnoreEnd
 		}
-		// @codeCoverageIgnoreEnd
 
 		return true;
 	}

app/Actions/Import/FromServer.php

@@ -22,6 +22,23 @@ public function do($validated)
 		} else {
 			$this->exec->delete_imported = (Configs::get_value('delete_imported', '0') === '1');
 		}
+		if (isset($validated['import_via_symlink'])) {
+			$this->exec->import_via_symlink = ($validated['import_via_symlink'] === '1');
+		} else {
+			$this->exec->import_via_symlink = (Configs::get_value('import_via_symlink', '0') === '1');
+		}
+		if (isset($validated['skip_duplicates'])) {
+			$this->exec->skip_duplicates = ($validated['skip_duplicates'] === '1');
+		} else {
+			$this->exec->skip_duplicates = (Configs::get_value('skip_duplicates', '0') === '1');
+		}
+		if (isset($validated['resync_metadata'])) {
+			$this->exec->resync_metadata = ($validated['resync_metadata'] === '1');
+		} else {
+			// do we need a default?
+//			$this->exec->resync_metadata = (Configs::get_value('resync_metadata', '0') === '1');
+			$this->exec->resync_metadata = false;
+		}
 
 		// memory_limit can have a K/M/etc suffix which makes querying it
 		// more complicated...

app/Actions/Import/FromUrl.php

@@ -55,7 +55,7 @@ public function do(array $urls, $albumId): bool
 			}
 
 			// Import photo
-			if (!$this->photo($tmp_name, true, $albumId)) {
+			if (!$this->photo($tmp_name, true, false, $albumId)) {
 				$error = true;
 				Logs::error(__METHOD__, __LINE__, 'Could not import file (' . $tmp_name . ')');
 			}

app/Actions/Photo/Create.php

@@ -13,7 +13,7 @@
 use App\Actions\Photo\Strategies\StrategyPhoto;
 use App\Assets\Helpers;
 use App\Http\Livewire\Album;
-use App\Models\Configs;
+use App\Models\Logs;
 use App\Models\Photo;
 use Illuminate\Support\Facades\Storage;
 
@@ -59,7 +59,8 @@ public function add(
 		array $file,
 		$albumID_in = 0,
 		bool $delete_imported = false,
-		bool $force_skip_duplicates = false,
+		bool $skip_duplicates = false,
+		bool $import_via_symlink = false,
 		bool $resync_metadata = false
 	) {
 		// Check permissions
@@ -100,37 +101,39 @@ public function add(
 		 * ! a video
 		 */
 
-		if (!$duplicate) {
-			$strategy = new StrategyPhoto();
-		} else {
-			$strategy = new StrategyDuplicate($force_skip_duplicates, $resync_metadata, $delete_imported);
-		}
-
-		$strategy->storeFile($this);
-		$strategy->hydrate($this, $duplicate, $file);
+		try {
+			if (!$duplicate) {
+				$strategy = new StrategyPhoto($import_via_symlink);
+			} else {
+				$strategy = new StrategyDuplicate($skip_duplicates, $resync_metadata, $delete_imported);
+			}
 
-		// set $this->info
-		$strategy->loadMetadata($this, $file);
+			$strategy->storeFile($this);
+			$strategy->hydrate($this, $duplicate, $file);
 
-		$strategy->setParentAndOwnership($this);
+			// set $this->info
+			$strategy->loadMetadata($this, $file);
 
-		// set $this->livePhotoPartner
-		$strategy->findLivePartner($this);
+			$strategy->setParentAndOwnership($this);
 
-		$no_error = true;
-		$skip_db_entry_creation = false;
+			// set $this->livePhotoPartner
+			$strategy->findLivePartner($this);
 
-		$strategy->generate_thumbs($this, $skip_db_entry_creation, $no_error);
+			$no_error = true;
+			$skip_db_entry_creation = false;
 
-		// In case it's a live photo and we've uploaded the video
-		if ($skip_db_entry_creation === true) {
-			$res = $this->livePhotoPartner->id;
-		} else {
-			$res = $this->save($this->photo);
-		}
+			$strategy->generate_thumbs($this, $skip_db_entry_creation, $no_error);
 
-		if ($delete_imported && !$this->is_uploaded && ($exists || Configs::get_value('import_via_symlink', '0') !== '1')) {
-			@unlink($this->tmp_name);
+			// In case it's a live photo and we've uploaded the video
+			if ($skip_db_entry_creation === true) {
+				$res = $this->livePhotoPartner->id;
+			} else {
+				$res = $this->save($this->photo);
+			}
+		} finally {
+			if ($delete_imported && !$this->is_uploaded && ($exists || !$import_via_symlink) && !@unlink($this->tmp_name)) {
+				Logs::warning(__METHOD__, __LINE__, 'Failed to delete file (' . $this->tmp_name . ')');
+			}
 		}
 
 		return $res;

app/Actions/Photo/Strategies/StrategyDuplicate.php

@@ -4,24 +4,24 @@
 
 use App\Actions\Photo\Create;
 use App\Actions\Photo\Extensions\Metadata;
-use App\Exceptions\JsonWarning;
-use App\Models\Configs;
+use App\Exceptions\PhotoResyncedException;
+use App\Exceptions\PhotoSkippedException;
 use App\Models\Logs;
 use App\Models\Photo;
 use Storage;
 
 class StrategyDuplicate extends StrategyPhotoBase
 {
-	public $force_skip_duplicates;
+	public $skip_duplicates;
 	public $resync_metadata;
 	public $delete_imported;
 
 	public function __construct(
-		bool $force_skip_duplicate,
+		bool $skip_duplicates,
 		bool $resync_metadata,
 		bool $delete_imported
 	) {
-		$this->force_skip_duplicate = $force_skip_duplicate;
+		$this->skip_duplicates = $skip_duplicates;
 		$this->resync_metadata = $resync_metadata;
 		$this->delete_imported = $delete_imported;
 	}
@@ -49,14 +49,17 @@ public function hydrate(Create &$create, ?Photo &$existing = null, ?array $file
 
 		// Photo already exists
 		// Check if the user wants to skip duplicates
-		if ($this->force_skip_duplicates || Configs::get_value('skip_duplicates', '0') === '1') {
+		if ($this->skip_duplicates) {
 			$metadataChanged = false;
 
 			// Before we skip entirely, check if there is a sidecar file and if the metadata needs to be updated (from a sidecar)
 			if ($this->resync_metadata === true) {
 				$info = $this->getMetadata($file, $create->path, $create->kind, $create->extension);
+				$attr = $existing->attributesToArray();
 				foreach ($info as $key => $value) {
-					if ($existing->$key !== null && $value !== $existing->$key) {
+					if (array_key_exists($key, $attr)	// check if key exists, even if null
+						&& (($existing->$key !== null && $value !== $existing->$key) || ($existing->$key === null && $value !== null && $value !== ''))
+						&& $value != $existing->$key) {	// avoid false positives when comparing variables of different types (e.g string vs int)
 						$metadataChanged = true;
 						$existing->$key = $value;
 					}
@@ -67,11 +70,11 @@ public function hydrate(Create &$create, ?Photo &$existing = null, ?array $file
 				Logs::notice(__METHOD__, __LINE__, 'Updating metdata of existing photo.');
 				$existing->save();
 
-				$res = new JsonWarning('This photo has been skipped because it\'s already in your library, but its metadata has been updated.');
+				$res = new PhotoResyncedException('This photo has been skipped because it\'s already in your library, but its metadata has been updated.');
 			} else {
 				Logs::notice(__METHOD__, __LINE__, 'Skipped upload of existing photo because skipDuplicates is activated');
 
-				$res = new JsonWarning('This photo has been skipped because it\'s already in your library.');
+				$res = new PhotoSkippedException('This photo has been skipped because it\'s already in your library.');
 			}
 
 			if ($this->delete_imported && !$create->is_uploaded) {
@@ -85,6 +88,6 @@ public function hydrate(Create &$create, ?Photo &$existing = null, ?array $file
 
 	public function generate_thumbs(Create &$create, bool &$skip_db_entry_creation, bool &$no_error)
 	{
-		Logs::notice(__FILE__, __LINE__, 'Nothing to store, image is a duplicate');
+		Logs::notice(__FILE__, __LINE__, 'Nothing to generate, image is a duplicate');
 	}
 }

app/Actions/Photo/Strategies/StrategyPhoto.php

@@ -8,18 +8,20 @@
 use App\Exceptions\JsonError;
 use App\Image\ImageHandlerInterface;
 use App\Metadata\Extractor;
-use App\Models\Configs;
 use App\Models\Logs;
 use App\Models\Photo;
 use Exception;
 
 class StrategyPhoto extends StrategyPhotoBase
 {
 	public $imageHandler;
+	public $import_via_symlink;
 
-	public function __construct()
-	{
+	public function __construct(
+		bool $import_via_symlink
+	) {
 		$this->imageHandler = app(ImageHandlerInterface::class);
+		$this->import_via_symlink = $import_via_symlink;
 	}
 
 	use ImageEditing;
@@ -31,7 +33,7 @@ public function storeFile(Create $create)
 		if (!$create->is_uploaded) {
 			// TODO: use the storage facade here
 			// Check if the user wants to create symlinks instead of copying the photo
-			if (Configs::get_value('import_via_symlink', '0') === '1') {
+			if ($this->import_via_symlink) {
 				if (!symlink($create->tmp_name, $create->path)) {
 					// @codeCoverageIgnoreStart
 					Logs::error(__METHOD__, __LINE__, 'Could not create symlink');
@@ -76,7 +78,7 @@ public function generate_thumbs(Create &$create, bool &$skip_db_entry_creation,
 			) {
 				// If we are importing via symlink, we don't actually overwrite
 				// the source but we still need to fix the dimensions.
-				$pretend = (!$create->is_uploaded && Configs::get_value('import_via_symlink', '0') === '1');
+				$pretend = (!$create->is_uploaded && $this->import_via_symlink);
 				$rotation = $this->imageHandler->autoRotate($create->path, $create->info, $pretend);
 
 				if ($rotation !== [false, false]) {

app/Console/Commands/Sync.php

@@ -4,6 +4,7 @@
 
 use AccessControl;
 use App\Actions\Import\Exec;
+use App\Models\Configs;
 use Exception;
 use Illuminate\Console\Command;
 
@@ -39,7 +40,8 @@ public function handle(Exec $exec)
 		$exec->statusCLIFormatting = true;
 		$exec->memCheck = false;
 		$exec->delete_imported = false; // we want to sync -> do not delete imported files
-		$exec->force_skip_duplicates = true;
+		$exec->import_via_symlink = (Configs::get_value('import_via_symlink', '0') === '1');
+		$exec->skip_duplicates = true;
 		$exec->resync_metadata = $this->option('resync_metadata');
 
 		AccessControl::log_as_id($owner_id);

app/Exceptions/PhotoResyncedException.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Exceptions;
+
+use Throwable;
+
+class PhotoResyncedException extends JsonWarning
+{
+	public function __construct(
+		$message,
+		$code = 0,
+		Throwable $previous = null
+	) {
+		parent::__construct($message, $code, $previous);
+	}
+}

app/Exceptions/PhotoSkippedException.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Exceptions;
+
+use Throwable;
+
+class PhotoSkippedException extends JsonWarning
+{
+	public function __construct(
+		$message,
+		$code = 0,
+		Throwable $previous = null
+	) {
+		parent::__construct($message, $code, $previous);
+	}
+}