Merge branch 'develop' into KDESKTOP-675-Linux-CI-Configuration

Infomaniak · Apr 2, 2024 · 70517fb · 70517fb
2 parents ed2d99d + ef7e054
commit 70517fb
Show file tree

Hide file tree

Showing 14 changed files with 350 additions and 370 deletions.
diff --git a/infomaniak-build-tools/windows/Readme.md b/infomaniak-build-tools/windows/Readme.md
diff --git a/src/libcommonserver/io/iohelper.cpp b/src/libcommonserver/io/iohelper.cpp
@@ -42,10 +42,10 @@ std::function<std::uintmax_t(const SyncPath &path, std::error_code &ec)> IoHelpe
 std::function<SyncPath(std::error_code &ec)> IoHelper::_tempDirectoryPath =
     static_cast<SyncPath (*)(std::error_code &ec)>(&std::filesystem::temp_directory_path);
 #ifdef __APPLE__
-std::function<bool(const SyncPath &path, ItemType &itemType)> IoHelper::_readAlias = [](const SyncPath &path,
-                                                                                        ItemType &itemType) -> bool {
+std::function<bool(const SyncPath &path, SyncPath &targetPath, IoError &ioError)> IoHelper::_readAlias =
+    [](const SyncPath &path, SyncPath &targetPath, IoError &ioError) -> bool {
     std::string data;
-    return readAlias(path, data, itemType.targetPath, itemType.ioError);
+    return IoHelper::readAlias(path, data, targetPath, ioError);
 };
 #endif
 
@@ -148,11 +148,12 @@ bool IoHelper::_isExpectedError(IoError ioError) noexcept {
 bool IoHelper::_setTargetType(ItemType &itemType) noexcept {
     std::error_code ec;
     const bool isDir = _isDirectory(itemType.targetPath, ec);
-    itemType.ioError = stdError2ioError(ec);
+    IoError ioError = stdError2ioError(ec);
 
-    if (itemType.ioError != IoErrorSuccess) {
-        const bool expected = _isExpectedError(itemType.ioError);
+    if (ioError != IoErrorSuccess) {
+        const bool expected = _isExpectedError(ioError);
         if (!expected) {
+            itemType.ioError = ioError;
             LOGW_WARN(Log::instance()->getLogger(), L"Failed to check if the item is a directory: "
                                                         << Utility::formatStdError(itemType.targetPath, ec).c_str());
         }
@@ -214,9 +215,12 @@ bool IoHelper::getItemType(const SyncPath &path, ItemType &itemType) noexcept {
 
     if (isAlias) {
         // !!! isAlias is true for a symlink and for a Finder alias !!!
-        if (!_readAlias(path, itemType)) {
+        IoError aliasReadError = IoErrorSuccess;
+        if (!_readAlias(path, itemType.targetPath, aliasReadError)) {
             LOGW_WARN(Log::instance()->getLogger(), L"Failed to read an item first identified as an alias: "
                                                         << Utility::formatIoError(path, itemType.ioError).c_str());
+            itemType.ioError = aliasReadError;
+
             return false;
         }
 

diff --git a/src/libcommonserver/io/iohelper.h b/src/libcommonserver/io/iohelper.h
@@ -269,7 +269,7 @@ struct IoHelper {
 
 #ifdef __APPLE__
         // Can be modified in tests.
-        static std::function<bool(const SyncPath &path, ItemType &itemType)> _readAlias;
+        static std::function<bool(const SyncPath &path, SyncPath &targetPath, IoError &ioError)> _readAlias;
 #endif
 
     private:

diff --git a/src/libsyncengine/jobs/network/uploadjob.cpp b/src/libsyncengine/jobs/network/uploadjob.cpp
@@ -146,41 +146,40 @@ void UploadJob::setQueryParameters(Poco::URI &uri, bool &canceled) {
     canceled = false;
 }
 
+namespace details {
+inline bool isLink(LinkType linkType) {
+    return linkType == LinkTypeSymlink || linkType == LinkTypeHardlink ||
+           (linkType == LinkTypeFinderAlias && OldUtility::isMac()) || (linkType == LinkTypeJunction && OldUtility::isWindows());
+}
+}  // namespace details
+
 void UploadJob::setData(bool &canceled) {
+    canceled = true;
+
     ItemType itemType;
     if (!IoHelper::getItemType(_filePath, itemType)) {
-        LOGW_WARN(KDC::Log::instance()->getLogger(), L"Error in IoHelper::getItemType - path=" << Path2WStr(_filePath).c_str());
-        canceled = true;
+        LOGW_WARN(_logger, L"Error in IoHelper::getItemType - " << Utility::formatSyncPath(_filePath).c_str());
         return;
     }
 
     if (itemType.ioError == IoErrorNoSuchFileOrDirectory) {
-        LOGW_DEBUG(KDC::Log::instance()->getLogger(), L"Item does not exist anymore - path=" << Path2WStr(_filePath).c_str());
-        canceled = true;
+        LOGW_DEBUG(_logger, L"Item does not exist anymore - " << Utility::formatSyncPath(_filePath).c_str());
         return;
     }
 
     if (itemType.ioError == IoErrorAccessDenied) {
-        LOGW_DEBUG(KDC::Log::instance()->getLogger(), L"Item misses search permission - path=" << Path2WStr(_filePath).c_str());
-        canceled = true;
+        LOGW_DEBUG(_logger, L"Item misses search permission - " << Utility::formatSyncPath(_filePath).c_str());
         return;
     }
 
-    if (_linkType == LinkTypeSymlink || _linkType == LinkTypeHardlink ||
-        (_linkType == LinkTypeFinderAlias && OldUtility::isMac()) || (_linkType == LinkTypeJunction && OldUtility::isWindows())) {
-        // Link
+    _linkType = itemType.linkType;
+
+    if (details::isLink(_linkType)) {
         LOG_DEBUG(_logger, "Read link data - type=" << _linkType);
-        if (!readLink()) {
-            canceled = true;
-            return;
-        }
+        if (!readLink()) return;
     } else {
-        // Normal file
         LOG_DEBUG(_logger, "Read file data");
-        if (!readFile()) {
-            canceled = true;
-            return;
-        }
+        if (!readFile()) return;
     }
 
     _contentHash = Utility::computeXxHash(_data);

diff --git a/src/server/appserver.cpp b/src/server/appserver.cpp
@@ -21,7 +21,6 @@
 #include "common/utility.h"
 #include "updater/kdcupdater.h"
 #include "libcommonserver/vfs.h"
-#include "config.h"
 #include "migration/migrationparams.h"
 #include "updater/kdcupdater.h"
 #include "socketapi.h"

diff --git a/src/server/appserver.h b/src/server/appserver.h
@@ -35,6 +35,7 @@
 #include "libcommon/info/driveinfo.h"
 #include "libcommon/info/syncinfo.h"
 #include "libcommon/info/syncfileiteminfo.h"
+#include "config.h"
 
 #include <QApplication>
 #include <QElapsedTimer>

diff --git a/test/libcommonserver/io/testcheckifisdirectory.cpp b/test/libcommonserver/io/testcheckifisdirectory.cpp
@@ -124,7 +124,7 @@ void TestIo::testCheckIfIsDirectory() {
 
         CPPUNIT_ASSERT(_testObj->checkIfIsDirectory(path, isDirectory, ioError));
         CPPUNIT_ASSERT(!isDirectory);
-        CPPUNIT_ASSERT(ioError == IoErrorNoSuchFileOrDirectory);
+        CPPUNIT_ASSERT(ioError == IoErrorSuccess);  // Although the target path is invalid.
     }
 
     // A regular directory missing all permissions: no error expected
@@ -133,9 +133,7 @@ void TestIo::testCheckIfIsDirectory() {
         const SyncPath path = temporaryDirectory.path / "permission_less_directory";
         std::filesystem::create_directory(path);
 
-        const auto allPermissions =
-            std::filesystem::perms::owner_all | std::filesystem::perms::others_all | std::filesystem::perms::group_all;
-        std::filesystem::permissions(path, allPermissions, std::filesystem::perm_options::remove);
+        std::filesystem::permissions(path, std::filesystem::perms::all, std::filesystem::perm_options::remove);
 
         IoError ioError = IoErrorUnknown;
         bool isDirectory = false;
@@ -144,7 +142,7 @@ void TestIo::testCheckIfIsDirectory() {
         CPPUNIT_ASSERT(isDirectory);
         CPPUNIT_ASSERT(ioError == IoErrorSuccess);
 
-        std::filesystem::permissions(path, allPermissions, std::filesystem::perm_options::add);
+        std::filesystem::permissions(path, std::filesystem::perms::all, std::filesystem::perm_options::add);
     }
 
     // A non-existing file with a very long name
@@ -245,7 +243,7 @@ void TestIo::testCheckIfIsDirectory() {
 
         CPPUNIT_ASSERT(_testObj->checkIfIsDirectory(path, isDirectory, ioError));
         CPPUNIT_ASSERT(!isDirectory);
-        CPPUNIT_ASSERT(ioError == IoErrorNoSuchFileOrDirectory);
+        CPPUNIT_ASSERT(ioError == IoErrorSuccess);
     }
 #endif
 }

diff --git a/test/libcommonserver/io/testcreatealias.cpp b/test/libcommonserver/io/testcreatealias.cpp
@@ -24,7 +24,10 @@ using namespace CppUnit;
 
 namespace KDC {
 
+
 void TestIo::testCreateAlias() {
+    GetItemChecker checker{_testObj};
+
     // A MacOSX Finder alias on a regular file.
     {
         const TemporaryDirectory temporaryDirectory;
@@ -36,12 +39,8 @@ void TestIo::testCreateAlias() {
         CPPUNIT_ASSERT(aliasError == IoErrorSuccess);
         CPPUNIT_ASSERT(std::filesystem::exists(path));
 
-        ItemType itemType;
-        CPPUNIT_ASSERT(IoHelper::getItemType(path, itemType));
-        CPPUNIT_ASSERT(itemType.ioError == IoErrorSuccess);
-        CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
-        CPPUNIT_ASSERT(itemType.linkType == LinkTypeFinderAlias);
-        CPPUNIT_ASSERT(itemType.targetType == NodeTypeFile);
+        const auto result = checker.checkSuccessfulLinkRetrieval(path, targetPath, LinkTypeFinderAlias, NodeTypeFile);
+        CPPUNIT_ASSERT_MESSAGE(result.message, result.success);
     }
 
     // A MacOSX Finder alias on a regular directory.
@@ -55,12 +54,8 @@ void TestIo::testCreateAlias() {
         CPPUNIT_ASSERT(aliasError == IoErrorSuccess);
         CPPUNIT_ASSERT(std::filesystem::exists(path));
 
-        ItemType itemType;
-        CPPUNIT_ASSERT(IoHelper::getItemType(path, itemType));
-        CPPUNIT_ASSERT(itemType.ioError == IoErrorSuccess);
-        CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
-        CPPUNIT_ASSERT(itemType.linkType == LinkTypeFinderAlias);
-        CPPUNIT_ASSERT(itemType.targetType == NodeTypeDirectory);
+        const auto result = checker.checkSuccessfulLinkRetrieval(path, targetPath, LinkTypeFinderAlias, NodeTypeDirectory);
+        CPPUNIT_ASSERT_MESSAGE(result.message, result.success);
     }
 
     // The target file does not exist: failure
@@ -87,12 +82,8 @@ void TestIo::testCreateAlias() {
         CPPUNIT_ASSERT(aliasError == IoErrorSuccess);
         CPPUNIT_ASSERT(std::filesystem::exists(path));
 
-        ItemType itemType;
-        CPPUNIT_ASSERT(IoHelper::getItemType(path, itemType));
-        CPPUNIT_ASSERT(itemType.ioError == IoErrorSuccess);
-        CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
-        CPPUNIT_ASSERT(itemType.linkType == LinkTypeFinderAlias);
-        CPPUNIT_ASSERT(itemType.targetType == NodeTypeFile);
+        const auto result = checker.checkSuccessfulLinkRetrieval(path, targetPath, LinkTypeFinderAlias, NodeTypeFile);
+        CPPUNIT_ASSERT_MESSAGE(result.message, result.success);
     }
 
     // The alias path is the path of an existing directory: failure
@@ -126,12 +117,10 @@ void TestIo::testCreateAlias() {
         CPPUNIT_ASSERT(aliasError == IoErrorSuccess);
         CPPUNIT_ASSERT(std::filesystem::exists(path));
 
-        ItemType itemType;
-        CPPUNIT_ASSERT(IoHelper::getItemType(path, itemType));
-        CPPUNIT_ASSERT(itemType.ioError == IoErrorSuccess);
-        CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
-        CPPUNIT_ASSERT(itemType.linkType == LinkTypeFinderAlias);
-        CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
+        const auto privateTargetPath =
+            SyncPath{"/private" + std::string(targetPath.c_str())};  // Simple concatenation doesn't append the `/private` prefix.
+        const auto result = checker.checkSuccessfulLinkRetrieval(path, privateTargetPath, LinkTypeFinderAlias, NodeTypeFile);
+        CPPUNIT_ASSERT_MESSAGE(result.message, result.success);
     }
 
     // The alias file name is very long: failure
@@ -157,18 +146,15 @@ void TestIo::testCreateAlias() {
         IoError aliasError = IoErrorSuccess;
         CPPUNIT_ASSERT(!IoHelper::createAliasFromPath(targetPath, path, aliasError));
         CPPUNIT_ASSERT(aliasError == IoErrorNoSuchFileOrDirectory);
-        // The test CPPUNIT_ASSERT(!std::filesystem::exists(path)) throws because a filesystem error.
+        // The test CPPUNIT_ASSERT(!std::filesystem::exists(path)) throws because of a filesystem error.
     }
 
     // The alias file path is very long: failure
     {
         const std::string pathSegment(50, 'a');
-        SyncPath path = _localTestDirPath;
-        for (int i = 0; i < 1000; ++i) {
-            path /= pathSegment;  // Eventually exceeds the max allowed path length on every file system of interest.
-        }
-
+        const SyncPath path = makeVeryLonPath(_localTestDirPath);
         const SyncPath targetPath = _localTestDirPath / "test_pictures/picture-1.jpg";
+
         IoError aliasError = IoErrorSuccess;
         CPPUNIT_ASSERT(!IoHelper::createAliasFromPath(targetPath, path, aliasError));
         CPPUNIT_ASSERT(aliasError == IoErrorNoSuchFileOrDirectory);
@@ -178,10 +164,7 @@ void TestIo::testCreateAlias() {
     // The target file path is very long: failure
     {
         const std::string pathSegment(50, 'a');
-        SyncPath targetPath = _localTestDirPath;
-        for (int i = 0; i < 1000; ++i) {
-            targetPath /= pathSegment;  // Eventually exceeds the max allowed path length on every file system of interest.
-        }
+        const SyncPath targetPath = makeVeryLonPath(_localTestDirPath);
 
         const TemporaryDirectory temporaryDirectory;
         const SyncPath path = temporaryDirectory.path / "alias.txt";  // This file doesn't exist.
@@ -204,11 +187,8 @@ void TestIo::testCreateAlias() {
         CPPUNIT_ASSERT(aliasError == IoErrorSuccess);
         CPPUNIT_ASSERT(std::filesystem::exists(path));
 
-        ItemType itemType;
-        CPPUNIT_ASSERT(IoHelper::getItemType(path, itemType));
-        CPPUNIT_ASSERT(itemType.ioError == IoErrorSuccess);
-        CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
-        CPPUNIT_ASSERT(itemType.linkType == LinkTypeFinderAlias);
+        const auto result = checker.checkSuccessfulLinkRetrieval(path, targetPath, LinkTypeFinderAlias, NodeTypeFile);
+        CPPUNIT_ASSERT_MESSAGE(result.message, result.success);
     }
 }
 

diff --git a/test/libcommonserver/io/testcreatesymlink.cpp b/test/libcommonserver/io/testcreatesymlink.cpp
@@ -76,7 +76,7 @@ void TestIo::testCreateSymlink() {
 
         ItemType itemType;
         CPPUNIT_ASSERT(IoHelper::getItemType(path, itemType));
-        CPPUNIT_ASSERT(itemType.ioError == IoErrorNoSuchFileOrDirectory);
+        CPPUNIT_ASSERT(itemType.ioError == IoErrorSuccess);  // Although the target path is invalid.
         CPPUNIT_ASSERT(itemType.nodeType == NodeTypeFile);
         CPPUNIT_ASSERT(itemType.linkType == LinkTypeSymlink);
         CPPUNIT_ASSERT(itemType.targetType == NodeTypeUnknown);

diff --git a/test/libcommonserver/io/testgetfilesize.cpp b/test/libcommonserver/io/testgetfilesize.cpp
@@ -17,7 +17,6 @@
  */
 
 #include "testio.h"
-#include "config.h"
 
 #include <filesystem>
 
@@ -47,15 +46,14 @@ void TestIo::testGetFileSizeSimpleCases() {
             ofs << "Some content.\n";
             ofs.close();
         }
-        const auto allPermissions =
-            std::filesystem::perms::owner_all | std::filesystem::perms::others_all | std::filesystem::perms::group_all;
-        std::filesystem::permissions(path, allPermissions, std::filesystem::perm_options::remove);
+
+        std::filesystem::permissions(path, std::filesystem::perms::all, std::filesystem::perm_options::remove);
 
         uint64_t fileSize = 0u;
         IoError ioError = IoErrorSuccess;
 
         CPPUNIT_ASSERT(_testObj->getFileSize(path, fileSize, ioError));
-        std::filesystem::permissions(path, allPermissions, std::filesystem::perm_options::add);
+        std::filesystem::permissions(path, std::filesystem::perms::all, std::filesystem::perm_options::add);
 
         CPPUNIT_ASSERT(ioError == IoErrorSuccess);
         CPPUNIT_ASSERT(fileSize == std::filesystem::file_size(path));
@@ -117,7 +115,7 @@ void TestIo::testGetFileSizeSimpleCases() {
 
             CPPUNIT_ASSERT(_testObj->getFileSize(path, fileSize, ioError));
             CPPUNIT_ASSERT(fileSize == targetPath.native().size());
-            CPPUNIT_ASSERT(ioError == IoErrorNoSuchFileOrDirectory);
+            CPPUNIT_ASSERT(ioError == IoErrorSuccess);  // Although the target path is invalid.
         }
     }