diff options
author | Olivier Goffart <ogoffart@woboq.com> | 2017-09-19 11:53:51 +0300 |
---|---|---|
committer | Olivier Goffart <olivier@woboq.com> | 2017-09-22 10:29:08 +0300 |
commit | 95d23b1914ce0dbe46341c70b9af79fed7e98a96 (patch) | |
tree | adbc6d7148c8268722d2db83510a82f3e1abb876 | |
parent | 0464cdb43fe4a5588472525c6ce54bc281477c59 (diff) |
RemotePermissions: Store in a class rather than in a QByteArray to save memory
Create a specific type that parses the permissions so we can store
it in a short rather than in a QByteArray
Note: in RemotePermissions::toString, we make sure the string is not
empty by adding a space, this was already existing before commit
e8f7adc7cacd4f55e26b2dd14464654e82204307 where it was removed by mistake.
-rw-r--r-- | src/common/common.cmake | 1 | ||||
-rw-r--r-- | src/common/remotepermissions.cpp | 67 | ||||
-rw-r--r-- | src/common/remotepermissions.h | 92 | ||||
-rw-r--r-- | src/common/syncjournaldb.cpp | 8 | ||||
-rw-r--r-- | src/common/syncjournalfilerecord.h | 3 | ||||
-rw-r--r-- | src/csync/csync.h | 4 | ||||
-rw-r--r-- | src/csync/csync_private.h | 4 | ||||
-rw-r--r-- | src/csync/csync_statedb.cpp | 5 | ||||
-rw-r--r-- | src/csync/csync_update.cpp | 4 | ||||
-rw-r--r-- | src/gui/owncloudgui.cpp | 2 | ||||
-rw-r--r-- | src/libsync/discoveryphase.cpp | 37 | ||||
-rw-r--r-- | src/libsync/discoveryphase.h | 8 | ||||
-rw-r--r-- | src/libsync/propagatedownload.cpp | 10 | ||||
-rw-r--r-- | src/libsync/syncengine.cpp | 45 | ||||
-rw-r--r-- | src/libsync/syncengine.h | 2 | ||||
-rw-r--r-- | src/libsync/syncfileitem.h | 2 | ||||
-rw-r--r-- | src/libsync/syncfilestatustracker.cpp | 6 | ||||
-rw-r--r-- | test/testsyncjournaldb.cpp | 8 |
18 files changed, 234 insertions, 74 deletions
diff --git a/src/common/common.cmake b/src/common/common.cmake index e448868c6..9d7898e8a 100644 --- a/src/common/common.cmake +++ b/src/common/common.cmake @@ -8,4 +8,5 @@ set(common_SOURCES ${CMAKE_CURRENT_LIST_DIR}/syncjournaldb.cpp ${CMAKE_CURRENT_LIST_DIR}/syncjournalfilerecord.cpp ${CMAKE_CURRENT_LIST_DIR}/utility.cpp + ${CMAKE_CURRENT_LIST_DIR}/remotepermissions.cpp ) diff --git a/src/common/remotepermissions.cpp b/src/common/remotepermissions.cpp new file mode 100644 index 000000000..ce39460b5 --- /dev/null +++ b/src/common/remotepermissions.cpp @@ -0,0 +1,67 @@ +/* + * Copyright (C) by Olivier Goffart <ogoffart@woboq.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "remotepermissions.h" +#include <cstring> + +namespace OCC { + +static const char letters[] = " WDNVCKRSMm"; + + +template <typename Char> +void RemotePermissions::fromArray(const Char *p) +{ + _value = p ? notNullMask : 0; + if (!p) + return; + while (*p) { + if (auto res = std::strchr(letters, static_cast<char>(*p))) + _value |= (1 << (res - letters)); + ++p; + } +} + +RemotePermissions::RemotePermissions(const char *p) +{ + fromArray(p); +} + +RemotePermissions::RemotePermissions(const QString &s) +{ + fromArray(s.isEmpty() ? nullptr : s.utf16()); +} + +QByteArray RemotePermissions::toString() const +{ + QByteArray result; + if (isNull()) + return result; + result.reserve(PermissionsCount); + for (uint i = 1; i <= PermissionsCount; ++i) { + if (_value & (1 << i)) + result.append(letters[i]); + } + if (result.isEmpty()) { + // Make sure it is not empty so we can differentiate null and empty permissions + result.append(' '); + } + return result; +} + +} // namespace OCC diff --git a/src/common/remotepermissions.h b/src/common/remotepermissions.h new file mode 100644 index 000000000..2b34dcbf0 --- /dev/null +++ b/src/common/remotepermissions.h @@ -0,0 +1,92 @@ +/* + * Copyright (C) by Olivier Goffart <ogoffart@woboq.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#pragma once + +#include <QString> +#include <QMetaType> +#include "ocsynclib.h" + +namespace OCC { + +/** + * Class that store in a memory efficient way the remote permission + */ +class OCSYNC_EXPORT RemotePermissions +{ +private: + // The first bit tells if the value is set or not + // The remaining bits correspond to know if the value is set + quint16 _value = 0; + static constexpr int notNullMask = 0x1; + + template <typename Char> // can be 'char' or 'ushort' if conversion from QString + void fromArray(const Char *p); + +public: + enum Permissions { + CanWrite = 1, // W + CanDelete = 2, // D + CanRename = 3, // N + CanMove = 4, // V + CanAddFile = 5, // C + CanAddSubDirectories = 6, // K + CanReshare = 7, // R + // Note: on the server, this means SharedWithMe, but in discoveryphase.cpp we also set + // this permission when the server reports the any "share-types" + IsShared = 8, // S + IsMounted = 9, // M + IsMountedSub = 10, // m (internal: set if the parent dir has IsMounted) + + // Note: when adding support for more permissions, we need to invalid the cache in the database. + // (by setting forceRemoteDiscovery in SyncJournalDb::checkConnect) + PermissionsCount = IsMountedSub + }; + RemotePermissions() = default; + explicit RemotePermissions(const char *); + explicit RemotePermissions(const QString &); + + QByteArray toString() const; + bool hasPermission(Permissions p) const + { + return _value & (1 << static_cast<int>(p)); + } + void setPermission(Permissions p) + { + _value |= (1 << static_cast<int>(p)) | notNullMask; + } + void unsetPermission(Permissions p) + { + _value &= ~(1 << static_cast<int>(p)); + } + + bool isNull() const { return !(_value & notNullMask); } + friend bool operator==(RemotePermissions a, RemotePermissions b) + { + return a._value == b._value; + } + friend bool operator!=(RemotePermissions a, RemotePermissions b) + { + return !(a == b); + } +}; + + +} // namespace OCC + +Q_DECLARE_METATYPE(OCC::RemotePermissions) diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp index 65254fd72..9e5052b5a 100644 --- a/src/common/syncjournaldb.cpp +++ b/src/common/syncjournaldb.cpp @@ -894,7 +894,7 @@ bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) qCInfo(lcDb) << "Updating file record for path:" << record._path << "inode:" << record._inode << "modtime:" << record._modtime << "type:" << record._type - << "etag:" << record._etag << "fileId:" << record._fileId << "remotePerm:" << record._remotePerm + << "etag:" << record._etag << "fileId:" << record._fileId << "remotePerm:" << record._remotePerm.toString() << "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader; qlonglong phash = getPHash(record._path); @@ -908,9 +908,7 @@ bool SyncJournalDb::setFileRecord(const SyncJournalFileRecord &_record) QString fileId(record._fileId); if (fileId.isEmpty()) fileId = ""; - QString remotePerm(record._remotePerm); - if (remotePerm.isEmpty()) - remotePerm = QString(); // have NULL in DB (vs empty) + QByteArray remotePerm = record._remotePerm.toString(); QByteArray checksumType, checksum; parseChecksumHeader(record._checksumHeader, &checksumType, &checksum); int contentChecksumTypeId = mapChecksumType(checksumType); @@ -1001,7 +999,7 @@ SyncJournalFileRecord SyncJournalDb::getFileRecord(const QString &filename) rec._type = _getFileRecordQuery->intValue(6); rec._etag = _getFileRecordQuery->baValue(7); rec._fileId = _getFileRecordQuery->baValue(8); - rec._remotePerm = _getFileRecordQuery->baValue(9); + rec._remotePerm = RemotePermissions(_getFileRecordQuery->baValue(9).constData()); rec._fileSize = _getFileRecordQuery->int64Value(10); rec._serverHasIgnoredFiles = (_getFileRecordQuery->intValue(11) > 0); rec._checksumHeader = _getFileRecordQuery->baValue(12); diff --git a/src/common/syncjournalfilerecord.h b/src/common/syncjournalfilerecord.h index ba1c7ced0..c6008713e 100644 --- a/src/common/syncjournalfilerecord.h +++ b/src/common/syncjournalfilerecord.h @@ -23,6 +23,7 @@ #include <QDateTime> #include "ocsynclib.h" +#include "remotepermissions.h" namespace OCC { @@ -57,7 +58,7 @@ public: QByteArray _etag; QByteArray _fileId; qint64 _fileSize; - QByteArray _remotePerm; + RemotePermissions _remotePerm; bool _serverHasIgnoredFiles; QByteArray _checksumHeader; }; diff --git a/src/csync/csync.h b/src/csync/csync.h index 9eaba2ab4..0e7cf12c5 100644 --- a/src/csync/csync.h +++ b/src/csync/csync.h @@ -41,6 +41,7 @@ #include <config_csync.h> #include <memory> #include <QByteArray> +#include "common/remotepermissions.h" #if defined(Q_CC_GNU) && !defined(Q_CC_INTEL) && !defined(Q_CC_CLANG) && (__GNUC__ * 100 + __GNUC_MINOR__ < 408) // openSuse 12.3 didn't like enum bitfields. @@ -163,6 +164,8 @@ struct OCSYNC_EXPORT csync_file_stat_s { time_t modtime; int64_t size; uint64_t inode; + + OCC::RemotePermissions remotePerm; enum csync_ftw_type_e type BITFIELD(4); bool child_modified BITFIELD(1); bool has_ignored_files BITFIELD(1); // Specify that a directory, or child directory contains ignored files. @@ -174,7 +177,6 @@ struct OCSYNC_EXPORT csync_file_stat_s { QByteArray file_id; QByteArray directDownloadUrl; QByteArray directDownloadCookies; - QByteArray remotePerm; QByteArray original_path; // only set if locale conversion fails // In the local tree, this can hold a checksum and its type if it is diff --git a/src/csync/csync_private.h b/src/csync/csync_private.h index 53383db89..9167a53a4 100644 --- a/src/csync/csync_private.h +++ b/src/csync/csync_private.h @@ -86,7 +86,7 @@ struct OCSYNC_EXPORT csync_s { /* hooks for checking the white list (uses the update_callback_userdata) */ int (*checkSelectiveSyncBlackListHook)(void*, const QByteArray &) = nullptr; - int (*checkSelectiveSyncNewFolderHook)(void*, const QByteArray &/* path */, const QByteArray &/* remotePerm */) = nullptr; + int (*checkSelectiveSyncNewFolderHook)(void *, const QByteArray & /* path */, OCC::RemotePermissions) = nullptr; csync_vio_opendir_hook remote_opendir_hook = nullptr; @@ -126,7 +126,7 @@ struct OCSYNC_EXPORT csync_s { struct { FileMap files; bool read_from_db = false; - QByteArray root_perms; /* Permission of the root folder. (Since the root folder is not in the db tree, we need to keep a separate entry.) */ + OCC::RemotePermissions root_perms; /* Permission of the root folder. (Since the root folder is not in the db tree, we need to keep a separate entry.) */ } remote; /* replica we are currently walking */ diff --git a/src/csync/csync_statedb.cpp b/src/csync/csync_statedb.cpp index 89ac83fd7..b489b6789 100644 --- a/src/csync/csync_statedb.cpp +++ b/src/csync/csync_statedb.cpp @@ -264,7 +264,10 @@ static int _csync_file_stat_from_metadata_table( std::unique_ptr<csync_file_stat st->type = static_cast<enum csync_ftw_type_e>(sqlite3_column_int(stmt, 3)); st->etag = (char*)sqlite3_column_text(stmt, 4); st->file_id = (char*)sqlite3_column_text(stmt, 5); - st->remotePerm = (char*)sqlite3_column_text(stmt, 6); + const char *permStr = (char *)sqlite3_column_text(stmt, 6); + // If permStr is empty, construct a null RemotePermissions. We make sure that non-null + // permissions are never empty in RemotePermissions.toString() + st->remotePerm = permStr && *permStr ? OCC::RemotePermissions(permStr) : OCC::RemotePermissions(); st->size = sqlite3_column_int64(stmt, 7); st->has_ignored_files = sqlite3_column_int(stmt, 8); st->checksumHeader = (char *)sqlite3_column_text(stmt, 9); diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp index 71be03be8..28bf5cd26 100644 --- a/src/csync/csync_update.cpp +++ b/src/csync/csync_update.cpp @@ -183,10 +183,10 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f /* we have an update! */ CSYNC_LOG(CSYNC_LOG_PRIORITY_INFO, "Database entry found, compare: %" PRId64 " <-> %" PRId64 ", etag: %s <-> %s, inode: %" PRId64 " <-> %" PRId64 - ", size: %" PRId64 " <-> %" PRId64 ", perms: %s <-> %s, ignore: %d", + ", size: %" PRId64 " <-> %" PRId64 ", perms: %x <-> %x, ignore: %d", ((int64_t) fs->modtime), ((int64_t) tmp->modtime), fs->etag.constData(), tmp->etag.constData(), (uint64_t) fs->inode, (uint64_t) tmp->inode, - (uint64_t) fs->size, (uint64_t) tmp->size, fs->remotePerm.constData(), tmp->remotePerm.constData(), tmp->has_ignored_files ); + (uint64_t) fs->size, (uint64_t) tmp->size, *reinterpret_cast<short*>(&fs->remotePerm), *reinterpret_cast<short*>(&tmp->remotePerm), tmp->has_ignored_files ); if (ctx->current == REMOTE_REPLICA && fs->etag != tmp->etag) { fs->instruction = CSYNC_INSTRUCTION_EVAL; diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp index 0d2fcffd1..6c57442ea 100644 --- a/src/gui/owncloudgui.cpp +++ b/src/gui/owncloudgui.cpp @@ -1007,7 +1007,7 @@ void ownCloudGui::slotShowShareDialog(const QString &sharePath, const QString &l bool resharingAllowed = true; // lets assume the good if (fileRecord.isValid()) { // check the permission: Is resharing allowed? - if (!fileRecord._remotePerm.contains('R')) { + if (!fileRecord._remotePerm.isNull() && !fileRecord._remotePerm.hasPermission(RemotePermissions::CanReshare)) { resharingAllowed = false; } } diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp index 23ac664c6..e5283f131 100644 --- a/src/libsync/discoveryphase.cpp +++ b/src/libsync/discoveryphase.cpp @@ -87,10 +87,11 @@ int DiscoveryJob::isInSelectiveSyncBlackListCallback(void *data, const QByteArra return static_cast<DiscoveryJob *>(data)->isInSelectiveSyncBlackList(path); } -bool DiscoveryJob::checkSelectiveSyncNewFolder(const QString &path, const QByteArray &remotePerm) +bool DiscoveryJob::checkSelectiveSyncNewFolder(const QString &path, RemotePermissions remotePerm) { - if (_syncOptions._confirmExternalStorage && remotePerm.contains('M')) { - // 'M' in the permission means external storage. + if (_syncOptions._confirmExternalStorage + && remotePerm.hasPermission(RemotePermissions::IsMounted)) { + // external storage. /* Note: DiscoverySingleDirectoryJob::directoryListingIteratedSlot make sure that only the * root of a mounted storage has 'M', all sub entries have 'm' */ @@ -144,7 +145,7 @@ bool DiscoveryJob::checkSelectiveSyncNewFolder(const QString &path, const QByteA } } -int DiscoveryJob::checkSelectiveSyncNewFolderCallback(void *data, const QByteArray &path, const QByteArray &remotePerm) +int DiscoveryJob::checkSelectiveSyncNewFolderCallback(void *data, const QByteArray &path, RemotePermissions remotePerm) { return static_cast<DiscoveryJob *>(data)->checkSelectiveSyncNewFolder(QString::fromUtf8(path), remotePerm); } @@ -350,20 +351,19 @@ static std::unique_ptr<csync_file_stat_t> propertyMapToFileStat(const QMap<QStri } else if (property == "dDC") { file_stat->directDownloadCookies = value.toUtf8(); } else if (property == "permissions") { - file_stat->remotePerm = value.toUtf8(); + file_stat->remotePerm = RemotePermissions(value); } else if (property == "checksums") { file_stat->checksumHeader = findBestChecksum(value.toUtf8()); } else if (property == "share-types" && !value.isEmpty()) { - // Since QMap is sorted, "share-types" is always "permissions". - if (file_stat->remotePerm.isEmpty()) { + // Since QMap is sorted, "share-types" is always after "permissions". + if (file_stat->remotePerm.isNull()) { qWarning() << "Server returned a share type, but no permissions?"; } else { // S means shared with me. // But for our purpose, we want to know if the file is shared. It does not matter // if we are the owner or not. - // Piggy back on the persmission field 'S' - if (!file_stat->remotePerm.contains('S')) - file_stat->remotePerm.append('S'); + // Piggy back on the persmission field + file_stat->remotePerm.setPermission(RemotePermissions::IsShared); } } } @@ -376,9 +376,9 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con // The first entry is for the folder itself, we should process it differently. _ignoredFirst = true; if (map.contains("permissions")) { - auto perm = map.value("permissions"); + RemotePermissions perm(map.value("permissions")); emit firstDirectoryPermissions(perm); - _isExternalStorage = perm.contains(QLatin1Char('M')); + _isExternalStorage = perm.hasPermission(RemotePermissions::IsMounted); } if (map.contains("data-fingerprint")) { _dataFingerprint = map.value("data-fingerprint").toUtf8(); @@ -401,11 +401,12 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con if (file_stat->etag.isEmpty()) { qCCritical(lcDiscovery) << "etag of" << file_stat->path << "is" << file_stat->etag << "This must not happen."; } - if (_isExternalStorage) { + if (_isExternalStorage && file_stat->remotePerm.hasPermission(RemotePermissions::IsMounted)) { /* All the entries in a external storage have 'M' in their permission. However, for all purposes in the desktop client, we only need to know about the mount points. So replace the 'M' by a 'm' for every sub entries in an external storage */ - file_stat->remotePerm.replace('M', 'm'); + file_stat->remotePerm.unsetPermission(RemotePermissions::IsMounted); + file_stat->remotePerm.setPermission(RemotePermissions::IsMountedSub); } QStringRef fileRef(&file); @@ -557,12 +558,12 @@ void DiscoveryMainThread::singleDirectoryJobFinishedWithErrorSlot(int csyncErrno _discoveryJob->_vioMutex.unlock(); } -void DiscoveryMainThread::singleDirectoryJobFirstDirectoryPermissionsSlot(const QString &p) +void DiscoveryMainThread::singleDirectoryJobFirstDirectoryPermissionsSlot(RemotePermissions p) { // Should be thread safe since the sync thread is blocked - if (_discoveryJob->_csync_ctx->remote.root_perms.isEmpty()) { - qCDebug(lcDiscovery) << "Permissions for root dir:" << p; - _discoveryJob->_csync_ctx->remote.root_perms = p.toUtf8(); + if (_discoveryJob->_csync_ctx->remote.root_perms.isNull()) { + qCDebug(lcDiscovery) << "Permissions for root dir:" << p.toString(); + _discoveryJob->_csync_ctx->remote.root_perms = p; } } diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index c3d44405b..eea33ad52 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -113,7 +113,7 @@ public: // This is not actually a network job, it is just a job signals: - void firstDirectoryPermissions(const QString &); + void firstDirectoryPermissions(RemotePermissions); void etagConcatenation(const QString &); void etag(const QString &); void finishedWithResult(); @@ -178,7 +178,7 @@ public slots: // From Job: void singleDirectoryJobResultSlot(); void singleDirectoryJobFinishedWithErrorSlot(int csyncErrnoCode, const QString &msg); - void singleDirectoryJobFirstDirectoryPermissionsSlot(const QString &); + void singleDirectoryJobFirstDirectoryPermissionsSlot(RemotePermissions); void slotGetSizeFinishedWithError(); void slotGetSizeResult(const QVariantMap &); @@ -212,8 +212,8 @@ class DiscoveryJob : public QObject */ bool isInSelectiveSyncBlackList(const QByteArray &path) const; static int isInSelectiveSyncBlackListCallback(void *, const QByteArray &); - bool checkSelectiveSyncNewFolder(const QString &path, const QByteArray &remotePerm); - static int checkSelectiveSyncNewFolderCallback(void *data, const QByteArray &path, const QByteArray &remotePerm); + bool checkSelectiveSyncNewFolder(const QString &path, RemotePermissions rp); + static int checkSelectiveSyncNewFolderCallback(void *data, const QByteArray &path, RemotePermissions rm); // Just for progress static void update_job_update_callback(bool local, diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 812c01316..4bb4495c9 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -823,13 +823,7 @@ void PropagateDownloadFile::downloadFinished() } // Apply the remote permissions - // Older server versions sometimes provide empty remote permissions - // see #4450 - don't adjust the write permissions there. - const int serverVersionGoodRemotePerm = Account::makeServerVersion(7, 0, 0); - if (propagator()->account()->serverVersionInt() >= serverVersionGoodRemotePerm) { - FileSystem::setFileReadOnlyWeak(_tmpFile.fileName(), - !_item->_remotePerm.contains('W')); - } + FileSystem::setFileReadOnlyWeak(_tmpFile.fileName(), !_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanWrite)); QString error; emit propagator()->touchedFile(fn); @@ -882,7 +876,7 @@ void PropagateDownloadFile::updateMetadata(bool isConflict) done(isConflict ? SyncFileItem::Conflict : SyncFileItem::Success); // handle the special recall file - if (!_item->_remotePerm.contains("S") + if (!_item->_remotePerm.hasPermission(RemotePermissions::IsShared) && (_item->_file == QLatin1String(".sys.admin#recall#") || _item->_file.endsWith("/.sys.admin#recall#"))) { handleRecallFile(fn, propagator()->_localDir, *propagator()->_journal); diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index b4b10d405..22a8068b0 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -411,7 +411,7 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other, if (!file->directDownloadCookies.isEmpty()) { item->_directDownloadCookies = QString::fromUtf8(file->directDownloadCookies); } - if (!file->remotePerm.isEmpty()) { + if (!file->remotePerm.isNull()) { item->_remotePerm = file->remotePerm; } @@ -601,8 +601,8 @@ int SyncEngine::treewalkFile(csync_file_stat_t *file, csync_file_stat_t *other, // If the 'W' remote permission changed, update the local filesystem SyncJournalFileRecord prev = _journal->getFileRecord(item->_file); - if (prev.isValid() && prev._remotePerm.contains('W') != item->_remotePerm.contains('W')) { - const bool isReadOnly = !item->_remotePerm.contains('W'); + if (prev.isValid() && prev._remotePerm.hasPermission(RemotePermissions::CanWrite) != item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) { + const bool isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite); FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); } @@ -946,7 +946,7 @@ void SyncEngine::slotDiscoveryJobFinished(int discoveryResult) qCWarning(lcEngine) << "Error in remote treewalk."; } - qCInfo(lcEngine) << "Permissions of the root folder: " << _csync_ctx->remote.root_perms; + qCInfo(lcEngine) << "Permissions of the root folder: " << _csync_ctx->remote.root_perms.toString(); // The map was used for merging trees, convert it to a list: SyncFileItemVector syncItems = _syncItemMap.values().toVector(); @@ -1251,11 +1251,11 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) case CSYNC_INSTRUCTION_NEW: { int slashPos = (*it)->_file.lastIndexOf('/'); QString parentDir = slashPos <= 0 ? "" : (*it)->_file.mid(0, slashPos); - const QByteArray perms = getPermissions(parentDir); + const auto perms = getPermissions(parentDir); if (perms.isNull()) { // No permissions set break; - } else if ((*it)->isDirectory() && !perms.contains("K")) { + } else if ((*it)->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddSubDirectories)) { qCWarning(lcEngine) << "checkForPermission: ERROR" << (*it)->_file; (*it)->_instruction = CSYNC_INSTRUCTION_ERROR; (*it)->_status = SyncFileItem::NormalError; @@ -1277,7 +1277,7 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) (*it)->_errorString = tr("Not allowed because you don't have permission to add parent folder"); } - } else if (!(*it)->isDirectory() && !perms.contains("C")) { + } else if (!(*it)->isDirectory() && !perms.hasPermission(RemotePermissions::CanAddFile)) { qCWarning(lcEngine) << "checkForPermission: ERROR" << (*it)->_file; (*it)->_instruction = CSYNC_INSTRUCTION_ERROR; (*it)->_status = SyncFileItem::NormalError; @@ -1286,12 +1286,12 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) break; } case CSYNC_INSTRUCTION_SYNC: { - const QByteArray perms = getPermissions((*it)->_file); + const auto perms = getPermissions((*it)->_file); if (perms.isNull()) { // No permissions set break; } - if (!perms.contains("W")) { + if (!perms.hasPermission(RemotePermissions::CanWrite)) { qCWarning(lcEngine) << "checkForPermission: RESTORING" << (*it)->_file; (*it)->_instruction = CSYNC_INSTRUCTION_CONFLICT; (*it)->_direction = SyncFileItem::Down; @@ -1312,12 +1312,12 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) break; } case CSYNC_INSTRUCTION_REMOVE: { - const QByteArray perms = getPermissions((*it)->_file); + const auto perms = getPermissions((*it)->_file); if (perms.isNull()) { // No permissions set break; } - if (!perms.contains("D")) { + if (!perms.hasPermission(RemotePermissions::CanDelete)) { qCWarning(lcEngine) << "checkForPermission: RESTORING" << (*it)->_file; (*it)->_instruction = CSYNC_INSTRUCTION_NEW; (*it)->_direction = SyncFileItem::Down; @@ -1344,7 +1344,8 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) (*it)->_errorString = tr("Not allowed to remove, restoring"); } } - } else if (perms.contains("S") && perms.contains("D")) { + } else if (perms.hasPermission(RemotePermissions::IsShared) + && perms.hasPermission(RemotePermissions::CanDelete)) { // this is a top level shared dir which can be removed to unshare it, // regardless if it is a read only share or not. // To avoid that we try to restore files underneath this dir which have @@ -1369,8 +1370,8 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) case CSYNC_INSTRUCTION_RENAME: { int slashPos = (*it)->_renameTarget.lastIndexOf('/'); const QString parentDir = slashPos <= 0 ? "" : (*it)->_renameTarget.mid(0, slashPos); - const QByteArray destPerms = getPermissions(parentDir); - const QByteArray filePerms = getPermissions((*it)->_file); + const auto destPerms = getPermissions(parentDir); + const auto filePerms = getPermissions((*it)->_file); //true when it is just a rename in the same directory. (not a move) bool isRename = (*it)->_file.startsWith(parentDir) && (*it)->_file.lastIndexOf('/') == slashPos; @@ -1381,21 +1382,21 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) if (isRename || destPerms.isNull()) { // no need to check for the destination dir permission destinationOK = true; - } else if ((*it)->isDirectory() && !destPerms.contains("K")) { + } else if ((*it)->isDirectory() && !destPerms.hasPermission(RemotePermissions::CanAddSubDirectories)) { destinationOK = false; - } else if (!(*it)->isDirectory() && !destPerms.contains("C")) { + } else if (!(*it)->isDirectory() && !destPerms.hasPermission(RemotePermissions::CanAddFile)) { destinationOK = false; } // check if we are allowed to move from the source bool sourceOK = true; if (!filePerms.isNull() - && ((isRename && !filePerms.contains("N")) - || (!isRename && !filePerms.contains("V")))) { + && ((isRename && !filePerms.hasPermission(RemotePermissions::CanRename)) + || (!isRename && !filePerms.hasPermission(RemotePermissions::CanMove)))) { // We are not allowed to move or rename this file sourceOK = false; - if (filePerms.contains("D") && destinationOK) { + if (filePerms.hasPermission(RemotePermissions::CanDelete) && destinationOK) { // but we are allowed to delete it // TODO! simulate delete & upload } @@ -1451,13 +1452,13 @@ void SyncEngine::checkForPermission(SyncFileItemVector &syncItems) } } -QByteArray SyncEngine::getPermissions(const QString &file) const +RemotePermissions SyncEngine::getPermissions(const QString &file) const { static bool isTest = qgetenv("OWNCLOUD_TEST_PERMISSIONS").toInt(); if (isTest) { QRegExp rx("_PERM_([^_]*)_[^/]*$"); if (rx.indexIn(file) != -1) { - return rx.cap(1).toLatin1(); + return RemotePermissions(rx.cap(1)); } } @@ -1471,7 +1472,7 @@ QByteArray SyncEngine::getPermissions(const QString &file) const if (it != _csync_ctx->remote.files.end()) { return it->second->remotePerm; } - return QByteArray(); + return RemotePermissions(); } void SyncEngine::restoreOldFiles(SyncFileItemVector &syncItems) diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 7f96afef9..44c8ca7a2 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -235,7 +235,7 @@ private: * to recover */ void checkForPermission(SyncFileItemVector &syncItems); - QByteArray getPermissions(const QString &file) const; + RemotePermissions getPermissions(const QString &file) const; /** * Instead of downloading files from the server, upload the files to the server diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index ab9526a93..dd364ae43 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -215,6 +215,7 @@ public: Status _status BITFIELD(4); bool _isRestoration BITFIELD(1); // The original operation was forbidden, and this is a restoration quint16 _httpErrorCode; + RemotePermissions _remotePerm; QString _errorString; // Contains a string only in case of error QByteArray _responseTimeStamp; quint32 _affectedItems; // the number of affected items by the operation on this item. @@ -228,7 +229,6 @@ public: quint64 _size; quint64 _inode; QByteArray _fileId; - QByteArray _remotePerm; // This is the value for the 'new' side, matching with _size and _modtime. // diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp index 4db5b72fc..855abff7a 100644 --- a/src/libsync/syncfilestatustracker.cpp +++ b/src/libsync/syncfilestatustracker.cpp @@ -147,7 +147,7 @@ SyncFileStatus SyncFileStatusTracker::fileStatus(const QString &relativePath) // First look it up in the database to know if it's shared SyncJournalFileRecord rec = _syncEngine->journal()->getFileRecord(relativePath); if (rec.isValid()) { - return resolveSyncAndErrorStatus(relativePath, rec._remotePerm.contains("S") ? Shared : NotShared); + return resolveSyncAndErrorStatus(relativePath, rec._remotePerm.hasPermission(RemotePermissions::IsShared) ? Shared : NotShared); } // Must be a new file not yet in the database, check if it's syncing or has an error. @@ -226,7 +226,7 @@ void SyncFileStatusTracker::slotAboutToPropagate(SyncFileItemVector &items) _syncProblems[item->_file] = SyncFileStatus::StatusWarning; } - SharedFlag sharedFlag = item->_remotePerm.contains("S") ? Shared : NotShared; + SharedFlag sharedFlag = item->_remotePerm.hasPermission(RemotePermissions::IsShared) ? Shared : NotShared; if (item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA && item->_instruction != CSYNC_INSTRUCTION_IGNORE @@ -272,7 +272,7 @@ void SyncFileStatusTracker::slotItemCompleted(const SyncFileItemPtr &item) _syncProblems.erase(item->_file); } - SharedFlag sharedFlag = item->_remotePerm.contains("S") ? Shared : NotShared; + SharedFlag sharedFlag = item->_remotePerm.hasPermission(RemotePermissions::IsShared) ? Shared : NotShared; if (item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA && item->_instruction != CSYNC_INSTRUCTION_IGNORE diff --git a/test/testsyncjournaldb.cpp b/test/testsyncjournaldb.cpp index e3ea07cd9..c7d8b85c3 100644 --- a/test/testsyncjournaldb.cpp +++ b/test/testsyncjournaldb.cpp @@ -54,7 +54,7 @@ private slots: record._type = 5; record._etag = "789789"; record._fileId = "abcd"; - record._remotePerm = "744"; + record._remotePerm = RemotePermissions("RW"); record._fileSize = 213089055; record._checksumHeader = "MD5:mychecksum"; QVERIFY(_db.setFileRecord(record)); @@ -74,7 +74,7 @@ private slots: record._type = 7; record._etag = "789FFF"; record._fileId = "efg"; - record._remotePerm = "777"; + record._remotePerm = RemotePermissions("NV"); record._fileSize = 289055; _db.setFileRecordMetadata(record); storedRecord = _db.getFileRecord("foo"); @@ -91,7 +91,7 @@ private slots: { SyncJournalFileRecord record; record._path = "foo-checksum"; - record._remotePerm = "744"; + record._remotePerm = RemotePermissions("RW"); record._checksumHeader = "MD5:mychecksum"; record._modtime = QDateTime::currentDateTimeUtc(); QVERIFY(_db.setFileRecord(record)); @@ -111,7 +111,7 @@ private slots: { SyncJournalFileRecord record; record._path = "foo-nochecksum"; - record._remotePerm = "744"; + record._remotePerm = RemotePermissions("RWN"); record._modtime = QDateTime::currentDateTimeUtc(); QVERIFY(_db.setFileRecord(record)); |