Avoid data loss when importing pictures
Summary: Fix porting regressions, which left users of Gwenview Importer with: - failed import (import destination still empty) - additionally (when choosing "Delete" instead of "Keep" after import): pictures also removed from import source, with no way to recover Correct additional problems remaining after fixing the import failure: - hang on duplicate filenames - identically named files with different content are never imported - error dialog when deleting pictures from import source BUG: 379615 In detail: 1st problem (introduced in 017b4fe5): Initially, pictures are copied to a temporary folder (e.g. "foo/.gwenview_importer-IcQqvo/"), before being moved/renamed to the final destination (e.g. "foo/"), which is determined by calling "cd .." on the temporary folder. However, mistakenly this path contains a superfluous '/' (e.g. "foo/.gwenview_importer-IcQqvo//"), resulting in the final destination reading "foo/.gwenview_importer-IcQqvo/" instead of "foo/". On cleanup, the temporary folder is removed, simultaneously deleting all new pictures. Fix: Omit '/' where appropriate. 2nd problem (broken by a3262e9b): When trying to find a unique filename, the while loop "stat"ing the file repeats forever. Fix: Call "KIO::stat" and "KJobWidgets::setWindow" also inside the loop. 3rd problem (introduced in 017b4fe5): If the destination directory already contains an identically named file, we try to find a different name by appending a number. For this, we need to strip the old filename from the full path. Unfortunately, only calling "path()" is incorrect, giving "foo/pict0001.jpg/pict0001_1.jpg", rather than "foo/pict0001_1.jpg". Fix: Use "adjusted(QUrl::RemoveFilename)". 4th problem (broken by a3262e9b): Although deletion works, we show a warning dialog stating failure. Fix: Invert the logic of "job->exec()", as the error handling should be skipped by "break"ing out of the while loop. Test Plan: Steps to reproduce (see https://bugs.kde.org/show_bug.cgi?id=379615) no longer result in data loss. Autotests for importer (separate review request in D5750) pass. Hopefully, this helps catching any future porting regressions. Reviewers: ltoscano, sandsmark, gateau Reviewed By: ltoscano Differential Revision: https://phabricator.kde.org/D5749
parent
05e03cfd
Please register or sign in to comment