Merge pull request #7435 from nextcloud/bugfix/live-photos-delete

Fix sync errors when trying to delete video component of live photos

src/common/syncjournaldb.cpp

@@ -49,7 +49,7 @@ Q_LOGGING_CATEGORY(lcDb, "nextcloud.sync.database", QtInfoMsg)
 #define GET_FILE_RECORD_QUERY \
         "SELECT path, inode, modtime, type, md5, fileid, remotePerm, filesize," \
         "  ignoredChildrenRemote, contentchecksumtype.name || ':' || contentChecksum, e2eMangledName, isE2eEncrypted, " \
-        "  lock, lockOwnerDisplayName, lockOwnerId, lockType, lockOwnerEditor, lockTime, lockTimeout, lockToken, isShared, lastShareStateFetchedTimestmap, sharedByMe" \
+        "  lock, lockOwnerDisplayName, lockOwnerId, lockType, lockOwnerEditor, lockTime, lockTimeout, lockToken, isShared, lastShareStateFetchedTimestmap, sharedByMe, isLivePhoto, livePhotoFile" \
         " FROM metadata" \
         "  LEFT JOIN checksumtype as contentchecksumtype ON metadata.contentChecksumTypeId == contentchecksumtype.id"
 
@@ -78,6 +78,8 @@ static void fillFileRecordFromGetQuery(SyncJournalFileRecord &rec, SqlQuery &que
     rec._isShared = query.intValue(20) > 0;
     rec._lastShareStateFetchedTimestamp = query.int64Value(21);
     rec._sharedByMe = query.intValue(22) > 0;
+    rec._isLivePhoto = query.intValue(23) > 0;
+    rec._livePhotoFile = query.stringValue(24);
 }
 
 static QByteArray defaultJournalMode(const QString &dbPath)
@@ -837,6 +839,9 @@ bool SyncJournalDb::updateMetadataTableStructure()
     }
     commitInternal(QStringLiteral("update database structure: add basePath index"));
 
+    addColumn(QStringLiteral("isLivePhoto"), QStringLiteral("INTEGER"));
+    addColumn(QStringLiteral("livePhotoFile"), QStringLiteral("TEXT"));
+
     return re;
 }
 
@@ -963,7 +968,9 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
                  << "lock editor:" << record._lockstate._lockEditorApp
                  << "sharedByMe:" << record._sharedByMe
                  << "isShared:" << record._isShared
-                 << "lastShareStateFetchedTimestamp:" << record._lastShareStateFetchedTimestamp;
+                 << "lastShareStateFetchedTimestamp:" << record._lastShareStateFetchedTimestamp
+                 << "isLivePhoto" << record._isLivePhoto
+                 << "livePhotoFile" << record._livePhotoFile;
 
     const qint64 phash = getPHash(record._path);
     if (!checkConnect()) {
@@ -989,8 +996,8 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
     const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordQuery, QByteArrayLiteral("INSERT OR REPLACE INTO metadata "
                                                                                                         "(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, "
                                                                                                         "contentChecksum, contentChecksumTypeId, e2eMangledName, isE2eEncrypted, lock, lockType, lockOwnerDisplayName, lockOwnerId, "
-                                                                                                        "lockOwnerEditor, lockTime, lockTimeout, lockToken, isShared, lastShareStateFetchedTimestmap, sharedByMe) "
+                                                                                                        "lockOwnerEditor, lockTime, lockTimeout, lockToken, isShared, lastShareStateFetchedTimestmap, sharedByMe, isLivePhoto, livePhotoFile) "
-                                                                                                        "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7,  ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23, ?24, ?25, ?26, ?27, ?28, ?29);"),
+                                                                                                        "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7,  ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21, ?22, ?23, ?24, ?25, ?26, ?27, ?28, ?29, ?30, ?31);"),
         _db);
     if (!query) {
         qCDebug(lcDb) << "database error:" << query->error();
@@ -1026,6 +1033,8 @@ Result<void, QString> SyncJournalDb::setFileRecord(const SyncJournalFileRecord &
     query->bindValue(27, record._isShared);
     query->bindValue(28, record._lastShareStateFetchedTimestamp);
     query->bindValue(29, record._sharedByMe);
+    query->bindValue(30, record._isLivePhoto);
+    query->bindValue(31, record._livePhotoFile);
 
     if (!query->exec()) {
         qCDebug(lcDb) << "database error:" << query->error();

src/common/syncjournalfilerecord.h

@@ -88,6 +88,8 @@ public:
     bool _isShared = false;
     qint64 _lastShareStateFetchedTimestamp = 0;
     bool _sharedByMe = false;
+    bool _isLivePhoto = false;
+    QString _livePhotoFile;
 };
 
 QDebug& operator<<(QDebug &stream, const SyncJournalFileRecord::EncryptionStatus status);

src/libsync/discovery.cpp

@@ -547,6 +547,7 @@ void ProcessDirectoryJob::processFile(PathTuple path,
                            << " | e2eeMangledName: " << dbEntry.e2eMangledName() << "/" << serverEntry.e2eMangledName
                            << " | file lock: " << localFileIsLocked << "//" << serverFileIsLocked
                            << " | file lock type: " << localFileLockType << "//" << serverFileLockType
+                           << " | live photo: " << dbEntry._isLivePhoto << "//" << serverEntry.isLivePhoto
                            << " | metadata missing: /" << localEntry.isMetadataMissing << '/';
 
     qCInfo(lcDisco).nospace() << processingLog;
@@ -718,6 +719,9 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(const SyncFileItemPtr &it
     item->_lockTimeout = serverEntry.lockTimeout;
     item->_lockToken = serverEntry.lockToken;
 
+    item->_isLivePhoto = serverEntry.isLivePhoto;
+    item->_livePhotoFile = serverEntry.livePhotoFile;
+
     // Check for missing server data
     {
         QStringList missingData;
@@ -1119,6 +1123,12 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                 qCWarning(lcDisco) << "Failed to delete a file record from the local DB" << path._original;
             }
             return;
+        } else if (dbEntry._isLivePhoto && QMimeDatabase().mimeTypeForFile(item->_file).inherits(QStringLiteral("video/quicktime"))) {
+            // This is a live photo's video file; the server won't allow deletion of this file
+            // so we need to *not* propagate the .mov deletion to the server and redownload the file
+            qCInfo(lcDisco) << "Live photo video file deletion detected, redownloading" << item->_file;
+            item->_direction = SyncFileItem::Down;
+            item->_instruction = CSYNC_INSTRUCTION_SYNC;
         } else if (!serverModified) {
             // Removed locally: also remove on the server.
             if (!dbEntry._serverHasIgnoredFiles) {

src/libsync/discoveryphase.cpp

@@ -400,7 +400,8 @@ void DiscoverySingleDirectoryJob::start()
           << "http://owncloud.org/ns:dDC"
           << "http://owncloud.org/ns:permissions"
           << "http://owncloud.org/ns:checksums"
-          << "http://nextcloud.org/ns:is-encrypted";
+          << "http://nextcloud.org/ns:is-encrypted"
+          << "http://nextcloud.org/ns:metadata-files-live-photo";
 
     if (_isRootPath)
         props << "http://owncloud.org/ns:data-fingerprint";
@@ -550,6 +551,10 @@ static void propertyMapToRemoteInfo(const QMap<QString, QString> &map, RemotePer
         if (property == "lock-token") {
             result.lockToken = value;
         }
+        if (property == "metadata-files-live-photo") {
+            result.livePhotoFile = value;
+            result.isLivePhoto = true;
+        }
     }
 
     if (result.isDirectory && map.contains("size")) {

src/libsync/discoveryphase.h

@@ -87,6 +87,9 @@ struct RemoteInfo
     qint64 lockTime = 0;
     qint64 lockTimeout = 0;
     QString lockToken;
+
+    bool isLivePhoto = false;
+    QString livePhotoFile;
 };
 
 struct LocalInfo

src/libsync/syncfileitem.cpp

@@ -126,6 +126,8 @@ SyncJournalFileRecord SyncFileItem::toSyncJournalFileRecordWithInode(const QStri
     rec._lockstate._lockTime = _lockTime;
     rec._lockstate._lockTimeout = _lockTimeout;
     rec._lockstate._lockToken = _lockToken;
+    rec._isLivePhoto = _isLivePhoto;
+    rec._livePhotoFile = _livePhotoFile;
 
     // Update the inode if possible
     rec._inode = _inode;
@@ -167,6 +169,8 @@ SyncFileItemPtr SyncFileItem::fromSyncJournalFileRecord(const SyncJournalFileRec
     item->_sharedByMe = rec._sharedByMe;
     item->_isShared = rec._isShared;
     item->_lastShareStateFetchedTimestamp = rec._lastShareStateFetchedTimestamp;
+    item->_isLivePhoto = rec._isLivePhoto;
+    item->_livePhotoFile = rec._livePhotoFile;
     return item;
 }
 
@@ -237,6 +241,11 @@ SyncFileItemPtr SyncFileItem::fromProperties(const QString &filePath, const QMap
         item->_checksumHeader = findBestChecksum(properties.value("checksums").toUtf8());
     }
 
+    if (properties.contains(QStringLiteral("metadata-files-live-photo"))) {
+        item->_isLivePhoto = true;
+        item->_livePhotoFile = properties.value(QStringLiteral("metadata-files-live-photo"));
+    }
+
     // direction and instruction are decided later
     item->_direction = SyncFileItem::None;
     item->_instruction = CSYNC_INSTRUCTION_NONE;

src/libsync/syncfileitem.h

@@ -340,6 +340,9 @@ public:
     bool _isAnyInvalidCharChild = false;
     bool _isAnyCaseClashChild = false;
 
+    bool _isLivePhoto = false;
+    QString _livePhotoFile;
+
     QString _discoveryResult;
 };
 

test/syncenginetestutils.cpp

@@ -223,6 +223,13 @@ void FileInfo::setModTimeKeepEtag(const QString &relativePath, const QDateTime &
     file->lastModified = modTime;
 }
 
+void FileInfo::setIsLivePhoto(const QString &relativePath, const bool isLivePhoto)
+{
+    const auto file = find(relativePath);
+    Q_ASSERT(file);
+    file->isLivePhoto = isLivePhoto;
+}
+
 void FileInfo::modifyLockState(const QString &relativePath, LockState lockState, int lockType, const QString &lockOwner, const QString &lockOwnerId, const QString &lockEditorId, quint64 lockTime, quint64 lockTimeout)
 {
     FileInfo *file = findInvalidatingEtags(relativePath);
@@ -411,6 +418,7 @@ FakePropfindReply::FakePropfindReply(FileInfo &remoteRootFileInfo, QNetworkAcces
         xml.writeTextElement(ncUri, QStringLiteral("lock-time"), QString::number(fileInfo.lockTime));
         xml.writeTextElement(ncUri, QStringLiteral("lock-timeout"), QString::number(fileInfo.lockTimeout));
         xml.writeTextElement(ncUri, QStringLiteral("is-encrypted"), fileInfo.isEncrypted ? QString::number(1) : QString::number(0));
+        xml.writeTextElement(ncUri, QStringLiteral("metadata-files-live-photo"), fileInfo.isLivePhoto ? QString::number(1) : QString::number(0));
         buffer.write(fileInfo.extraDavProperties);
         xml.writeEndElement(); // prop
         xml.writeTextElement(davUri, QStringLiteral("status"), QStringLiteral("HTTP/1.1 200 OK"));

test/syncenginetestutils.h

@@ -142,6 +142,8 @@ public:
 
     void setModTimeKeepEtag(const QString &relativePath, const QDateTime &modTime);
 
+    void setIsLivePhoto(const QString &relativePath, bool isLivePhoto);
+
     void modifyLockState(const QString &relativePath, LockState lockState, int lockType, const QString &lockOwner, const QString &lockOwnerId, const QString &lockEditorId, quint64 lockTime, quint64 lockTimeout) override;
 
     void setE2EE(const QString &relativepath, const bool enabled) override;
@@ -188,6 +190,7 @@ public:
     quint64 lockTime = 0;
     quint64 lockTimeout = 0;
     bool isEncrypted = false;
+    bool isLivePhoto = false;
 
     // Sorted by name to be able to compare trees
     QMap<QString, FileInfo> children;

test/testlocaldiscovery.cpp

@@ -333,6 +333,41 @@ private slots:
         QVERIFY(!fakeFolder.currentRemoteState().find("C/filename.ext"));
     }
 
+    void testRedownloadDeletedLivePhotoMov()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        const auto livePhotoImg = QStringLiteral("IMG_0001.heic");
+        const auto livePhotoMov = QStringLiteral("IMG_0001.mov");
+        fakeFolder.localModifier().insert(livePhotoImg);
+        fakeFolder.localModifier().insert(livePhotoMov);
+
+        ItemCompletedSpy completeSpy(fakeFolder);
+        QVERIFY(fakeFolder.syncOnce());
+
+        QCOMPARE(completeSpy.findItem(livePhotoImg)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(livePhotoMov)->_status, SyncFileItem::Status::Success);
+
+        fakeFolder.remoteModifier().setIsLivePhoto(livePhotoImg, true);
+        fakeFolder.remoteModifier().setIsLivePhoto(livePhotoMov, true);
+        QVERIFY(fakeFolder.syncOnce());
+
+        SyncJournalFileRecord imgRecord;
+        QVERIFY(fakeFolder.syncJournal().getFileRecord(livePhotoImg, &imgRecord));
+        QVERIFY(imgRecord._isLivePhoto);
+
+        SyncJournalFileRecord movRecord;
+        QVERIFY(fakeFolder.syncJournal().getFileRecord(livePhotoMov, &movRecord));
+        QVERIFY(movRecord._isLivePhoto);
+
+        completeSpy.clear();
+        fakeFolder.localModifier().remove(livePhotoMov);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(completeSpy.findItem(livePhotoMov)->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(livePhotoMov)->_instruction, CSYNC_INSTRUCTION_SYNC);
+        QCOMPARE(completeSpy.findItem(livePhotoMov)->_direction, SyncFileItem::Direction::Down);
+    }
+
     void testCreateFileWithTrailingSpaces_localAndRemoteTrimmedDoNotExist_renameAndUploadFile()
     {
         FakeFolder fakeFolder{FileInfo{}};