Skip to content

Commit

Permalink
extend modal dialog for uploading from server (#894)
Browse files Browse the repository at this point in the history 
* extend modal dialog for uploading from server
* log warning if deletion of original failed
* localizations limited to English
* improved media import
* ImportPhoto::photo: new mandatory argument `import_via_symlink` before optional ones
* sanity check for incompatible settings
* Exec: eliminate `skip_duplicates` (existing `force_skip_duplicates` is enough and takes precedence)
* `resync_metadata` did not update keys with value null (but should have). in particular, this prevented updates to `takedate` if the corresponding entry in the DB was set to zero
* too many false positives were caused by comparing int's to their corresponding string representation.
* cancel import from server
* use session storage to process asynchronous cancel request from GUI during import from server

Co-authored-by: kamil4 <[email protected]>
Co-authored-by: ildyria <[email protected]>
  • Loading branch information
@evoludolab @kamil4 @ildyria
3 people authored Feb 16, 2021
1 parent edc4c21 commit 3274d9a
Show file tree
Hide file tree
Showing 32 changed files with 508 additions and 106 deletions.
66 changes: 52 additions & 14 deletions app/Actions/Import/Exec.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,24 @@
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;
use Exception;
use Illuminate\Support\Facades\Session;
use Illuminate\Support\Facades\Storage;

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;
Expand Down Expand Up @@ -57,10 +62,25 @@ private function status_update(string $status)
}
flush();
} else {
echo $status . PHP_EOL;
echo substr($status, strpos($status, ' ') + 1) . PHP_EOL;
}
}

private function status_progress(string $path, string $msg)
{
$this->status_update('Status: ' . $path . ': ' . $msg);
}

private function status_warning(string $msg)
{
$this->status_update('Warning: ' . $msg);
}

private function status_error(string $path, string $msg)
{
$this->status_update('Problem: ' . $path . ': ' . $msg);
}

private function parsePath(string &$path, string $origPath)
{
if (!isset($path)) {
Expand All @@ -72,7 +92,7 @@ private function parsePath(string &$path, string $origPath)
$path = substr($path, 0, -1);
}
if (is_dir($path) === false) {
$this->status_update('Problem: ' . $origPath . ': Given path is not a directory');
$this->status_error($origPath, 'Given path is not a directory');
Logs::error(__METHOD__, __LINE__, 'Given path is not a directory (' . $origPath . ')');

return false;
Expand All @@ -85,7 +105,7 @@ private function parsePath(string &$path, string $origPath)
realpath($path) === Storage::path('small') ||
realpath($path) === Storage::path('thumb')
) {
$this->status_update('Problem: ' . $origPath . ': Given path is reserved');
$this->status_error($origPath, 'Given path is reserved');
Logs::error(__METHOD__, __LINE__, 'The given path is a reserved path of Lychee (' . $origPath . ')');

return false;
Expand Down Expand Up @@ -131,7 +151,7 @@ private function memWarningCheck()
{
if ($this->memCheck && !$this->memWarningGiven && memory_get_usage() > $this->memLimit) {
// @codeCoverageIgnoreStart
$this->status_update('Warning: Approaching memory limit');
$this->status_warning('Approaching memory limit');
$this->memWarningGiven = true;
// @codeCoverageIgnoreEnd
}
Expand Down Expand Up @@ -171,8 +191,17 @@ public function do(
// Add '%' at end for CLI output
$percent_symbol = ($this->statusCLIFormatting) ? '%' : '';

$this->status_update('Status: ' . $origPath . ': 0' . $percent_symbol);
$this->status_progress($origPath, '0' . $percent_symbol);
foreach ($files as $file) {
// re-read session in case cancelling import was requested
session()->start();
if (Session::has('cancel')) {
Session::forget('cancel');
$this->status_error($origPath, 'Import cancelled');
Logs::warning(__METHOD__, __LINE__, 'Import cancelled');

return;
}
// Reset the execution timeout for every iteration.
set_time_limit(ini_get('max_execution_time'));

Expand All @@ -183,7 +212,7 @@ public function do(
// 100%, which are always generated.
$time = microtime(true);
if ($time - $lastStatus >= 1) {
$this->status_update('Status: ' . $origPath . ': ' . intval($filesCount / $filesTotal * 100) . $percent_symbol);
$this->status_progress($origPath, intval($filesCount / $filesTotal * 100) . $percent_symbol);
$lastStatus = $time;
}

Expand All @@ -203,30 +232,39 @@ public function do(
// It is possible to move a file because of directory permissions but
// the file may still be unreadable by the user
if (!is_readable($file)) {
$this->status_update('Problem: ' . $file . ': Could not read file');
$this->status_error($file, 'Could not read file');
Logs::error(__METHOD__, __LINE__, 'Could not read file or directory (' . $file . ')');
continue;
}
$extension = Helpers::getExtension($file, true);
$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) {
$this->status_update('Problem: ' . $file . ': Could not import file');
try {
if ($this->photo($file, $this->delete_imported, $this->import_via_symlink, $albumID, $this->skip_duplicates, $this->resync_metadata) === false) {
$this->status_error($file, 'Could not import file');
Logs::error(__METHOD__, __LINE__, 'Could not import file (' . $file . ')');
}
} catch (PhotoSkippedException $e) {
$this->status_error($file, 'Skipped duplicate');
} catch (PhotoResyncedException $e) {
$this->status_error($file, 'Skipped duplicate (resynced metadata)');
} catch (Exception $e) {
$this->status_error($file, 'Could not import file');
Logs::error(__METHOD__, __LINE__, 'Could not import file (' . $file . ')');
}
} else {
$this->status_update('Problem: ' . $file . ': Unsupported file type');
$this->status_error($file, 'Unsupported file type');
Logs::error(__METHOD__, __LINE__, 'Unsupported file type (' . $file . ')');
}
}
$this->status_update('Status: ' . $origPath . ': 100' . $percent_symbol);
$this->status_progress($origPath, '100' . $percent_symbol);

// Album creation
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()
Expand All @@ -239,7 +277,7 @@ public function do(
if ($album === false) {
// @codeCoverageIgnoreStart

$this->status_update('Problem: ' . basename($dir) . ': Could not create album');
$this->status_error(basename($dir), ': Could not create album');
Logs::error(__METHOD__, __LINE__, 'Could not create album in Lychee (' . basename($dir) . ')');
continue;
}
Expand Down
26 changes: 15 additions & 11 deletions app/Actions/Import/Extensions/ImportPhoto.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace App\Actions\Import\Extensions;

use App\Actions\Photo\Create;
use Exception;

trait ImportPhoto
{
Expand All @@ -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.
Expand All @@ -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;
}
Expand Down
17 changes: 17 additions & 0 deletions app/Actions/Import/FromServer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down
2 changes: 1 addition & 1 deletion app/Actions/Import/FromUrl.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 . ')');
}
Expand Down
13 changes: 7 additions & 6 deletions app/Actions/Photo/Create.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -101,9 +102,9 @@ public function add(
*/

if (!$duplicate) {
$strategy = new StrategyPhoto();
$strategy = new StrategyPhoto($import_via_symlink);
} else {
$strategy = new StrategyDuplicate($force_skip_duplicates, $resync_metadata, $delete_imported);
$strategy = new StrategyDuplicate($skip_duplicates, $resync_metadata, $delete_imported);
}

$strategy->storeFile($this);
Expand All @@ -129,8 +130,8 @@ public function add(
$res = $this->save($this->photo);
}

if ($delete_imported && !$this->is_uploaded && ($exists || Configs::get_value('import_via_symlink', '0') !== '1')) {
@unlink($this->tmp_name);
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;
Expand Down
23 changes: 13 additions & 10 deletions app/Actions/Photo/Strategies/StrategyDuplicate.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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');
}
}
Loading

0 comments on commit 3274d9a

Please sign in to comment.