From 7eba65ac233cd501fc5c2f51d5610195aa65b163 Mon Sep 17 00:00:00 2001 From: Flo-129 Date: Wed, 17 Dec 2025 21:39:03 +0100 Subject: [PATCH] feat(macOS): add trash sync toggle for FileProvider domains Add user-facing option to control NSFileProviderDomain.supportsSyncingTrash property (macOS 13.0+). When enabled, deleted files appear in macOS Trash and server trash contents are visible in Finder. When disabled, Finder shows "Delete Immediately" option. Changes: - Add "Integrate macOS Trash" checkbox in FileProvider settings - Store per-account trash sync preference in NSUserDefaults - Implement domain reconfiguration (remove/add cycle) when setting changes - Add thread-safety with QMutex for domain registry access - Clean up unused socket-based communication code (replaced by XPC) Signed-off-by: Flo-129 --- .../FileProviderDomainDefaults.swift | 29 -- .../FileProviderExtension.swift | 7 - .../Services/ClientCommunicationProtocol.h | 2 - .../Services/ClientCommunicationService.swift | 11 - src/gui/macOS/fileprovider.h | 1 + src/gui/macOS/fileprovider_mac.mm | 14 + src/gui/macOS/fileproviderdomainmanager.h | 2 + .../macOS/fileproviderdomainmanager_mac.mm | 362 +++++++++++++++--- .../macOS/fileprovidereditlocallyjob_mac.mm | 5 +- .../macOS/fileprovidersettingscontroller.h | 9 +- .../fileprovidersettingscontroller_mac.mm | 68 ++-- src/gui/macOS/fileproviderxpc.h | 3 - src/gui/macOS/fileproviderxpc_mac.mm | 43 +-- src/gui/macOS/ui/FileProviderSettings.qml | 24 +- 14 files changed, 392 insertions(+), 188 deletions(-) diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderDomainDefaults.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderDomainDefaults.swift index b9bee1677898f..d97c7738f8b75 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderDomainDefaults.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderDomainDefaults.swift @@ -14,7 +14,6 @@ struct FileProviderDomainDefaults { /// > Warning: Do not change the raw values of these keys, as they are used in UserDefaults. Any change would make the already stored value inaccessible and be like a reset. /// private enum ConfigKey: String { - case trashDeletionEnabled case user case userId case serverUrl @@ -84,29 +83,6 @@ struct FileProviderDomainDefaults { } } - /// - /// Whether trash deletion is enabled or not. - /// - var trashDeletionEnabled: Bool { - get { - let identifier = self.identifier.rawValue - - if let value = internalConfig[ConfigKey.trashDeletionEnabled.rawValue] as? Bool { - logger.debug("Returning existing value \"\(value)\" for \"trashDeletionEnabled\" for file provider domain \"\(identifier)\".") - return value - } else { - return false - } - } - - set { - let identifier = self.identifier.rawValue - - logger.error("Setting value \"\(newValue)\" for \"trashDeletionEnabled\" for file provider domain \"\(identifier)\".") - internalConfig[ConfigKey.trashDeletionEnabled.rawValue] = newValue - } - } - /// /// The user name associated with the domain. /// @@ -166,9 +142,4 @@ struct FileProviderDomainDefaults { } } } - - /// - /// Whether a value for `trashDeletionEnabled` has been explicitly set. - /// - lazy var trashDeletionSet = internalConfig[ConfigKey.trashDeletionEnabled.rawValue] != nil } diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift index 0fbba0422083c..6e1add011af83 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/FileProviderExtension.swift @@ -452,13 +452,6 @@ import OSLog logger.debug("Found item for identifier.", [.item: identifier, .name: item.filename]) - guard config.trashDeletionEnabled || item.parentItemIdentifier != .trashContainer else { - logger.info("System requested deletion of item in trash, but deleting trash items is disabled. item: \(item.filename)") - removeSyncAction(actionId) - completionHandler(NSError.fileProviderErrorForRejectedDeletion(of: item)) - return - } - let error = await item.delete(domain: domain, ignoredFiles: ignoredFiles, dbManager: dbManager) if error != nil { diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h index 458798c2ebea0..1aa93a7d0f0da 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationProtocol.h @@ -21,8 +21,6 @@ password:(NSString *)password userAgent:(NSString *)userAgent; - (void)removeAccountConfig; -- (void)getTrashDeletionEnabledStateWithCompletionHandler:(void(^)(BOOL enabled, BOOL set))completionHandler; -- (void)setTrashDeletionEnabled:(BOOL)enabled; - (void)setIgnoreList:(NSArray *)ignoreList; @end diff --git a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift index 36a3b2596ef4c..b9fb2ed273a2c 100644 --- a/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift +++ b/shell_integration/MacOSX/NextcloudIntegration/FileProviderExt/Services/ClientCommunicationService.swift @@ -63,17 +63,6 @@ class ClientCommunicationService: NSObject, NSFileProviderServiceSource, NSXPCLi self.fpExtension.removeAccountConfig() } - func getTrashDeletionEnabledState(completionHandler: @escaping (Bool, Bool) -> Void) { - let enabled = fpExtension.config.trashDeletionEnabled - let set = fpExtension.config.trashDeletionSet - completionHandler(enabled, set) - } - - func setTrashDeletionEnabled(_ enabled: Bool) { - fpExtension.config.trashDeletionEnabled = enabled - logger.info("Trash deletion setting changed to: \(enabled)") - } - func setIgnoreList(_ ignoreList: [String]) { self.fpExtension.ignoredFiles = IgnoredFilesMatcher(ignoreList: ignoreList) logger.info("Ignore list updated.") diff --git a/src/gui/macOS/fileprovider.h b/src/gui/macOS/fileprovider.h index ce5c2fd5fa02c..d5145b082d11d 100644 --- a/src/gui/macOS/fileprovider.h +++ b/src/gui/macOS/fileprovider.h @@ -36,6 +36,7 @@ class FileProvider : public QObject private slots: void configureXPC(); + void onDomainReconfigurationFailed(const QString &accountId, const QString &errorMessage); private: std::unique_ptr _domainManager; diff --git a/src/gui/macOS/fileprovider_mac.mm b/src/gui/macOS/fileprovider_mac.mm index 17cf2fe7edd3b..24828cf7b8e55 100644 --- a/src/gui/macOS/fileprovider_mac.mm +++ b/src/gui/macOS/fileprovider_mac.mm @@ -6,6 +6,7 @@ #include "fileprovider.h" #include +#include #include "libsync/configfile.h" #include "gui/macOS/fileproviderxpc.h" @@ -36,6 +37,8 @@ if (_domainManager) { connect(_domainManager.get(), &FileProviderDomainManager::domainSetupComplete, this, &FileProvider::configureXPC); + connect(_domainManager.get(), &FileProviderDomainManager::domainReconfigurationFailed, + this, &FileProvider::onDomainReconfigurationFailed); _domainManager->start(); qCDebug(lcMacFileProvider()) << "Initialized file provider domain manager"; } @@ -102,5 +105,16 @@ return _socketServer.get(); } +void FileProvider::onDomainReconfigurationFailed(const QString &accountId, const QString &errorMessage) +{ + qCWarning(lcMacFileProvider) << "Domain reconfiguration failed for account:" << accountId << "Error:" << errorMessage; + QMessageBox::warning(nullptr, + tr("Virtual files configuration error"), + tr("Could not update the virtual files settings for account %1.\n\n" + "Error: %2\n\n" + "Please try again or restart the application.") + .arg(accountId, errorMessage)); +} + } // namespace Mac } // namespace OCC diff --git a/src/gui/macOS/fileproviderdomainmanager.h b/src/gui/macOS/fileproviderdomainmanager.h index fcd1177ed1e99..3c47c20ecb809 100644 --- a/src/gui/macOS/fileproviderdomainmanager.h +++ b/src/gui/macOS/fileproviderdomainmanager.h @@ -31,6 +31,7 @@ class FileProviderDomainManager : public QObject signals: void domainSetupComplete(); + void domainReconfigurationFailed(const QString &accountId, const QString &errorMessage); public slots: void addFileProviderDomainForAccount(const OCC::AccountState * const accountState); @@ -42,6 +43,7 @@ private slots: void removeFileProviderDomainForAccount(const OCC::AccountState * const accountState); void disconnectFileProviderDomainForAccount(const OCC::AccountState * const accountState, const QString &reason); void reconnectFileProviderDomainForAccount(const OCC::AccountState * const accountState); + void reconfigureFileProviderDomainForAccount(const QString &accountId); void signalEnumeratorChanged(const OCC::Account * const account); void slotAccountStateChanged(const OCC::AccountState * const accountState); diff --git a/src/gui/macOS/fileproviderdomainmanager_mac.mm b/src/gui/macOS/fileproviderdomainmanager_mac.mm index 17baba9f308a0..758b7acf25202 100644 --- a/src/gui/macOS/fileproviderdomainmanager_mac.mm +++ b/src/gui/macOS/fileproviderdomainmanager_mac.mm @@ -10,6 +10,9 @@ #include #include #include +#include +#include +#include #include #include "config.h" @@ -185,7 +188,7 @@ inline QString accountIdFromDomain(NSFileProviderDomain * const domain) class API_AVAILABLE(macos(11.0)) FileProviderDomainManager::MacImplementation { public: - MacImplementation() = default; + explicit MacImplementation(FileProviderDomainManager *manager) : _manager(manager) {} ~MacImplementation() = default; void findExistingFileProviderDomains() @@ -247,10 +250,13 @@ void findExistingFileProviderDomains() [domain release]; }]; - return; + continue; // Continue loop instead of return to avoid deadlock (dispatch_group_leave must be called) } - _registeredDomains.insert(accountId, domain); + { + QMutexLocker locker(&_registeredDomainsMutex); + _registeredDomains.insert(accountId, domain); + } NSFileProviderManager * const fpManager = [NSFileProviderManager managerForDomain:domain]; [fpManager reconnectWithCompletionHandler:^(NSError * const error) { @@ -301,8 +307,11 @@ void findExistingFileProviderDomains() Q_ASSERT(account); const auto accountId = account->userIdAtHostWithPort(); + QMutexLocker locker(&_registeredDomainsMutex); if (_registeredDomains.contains(accountId)) { - return _registeredDomains[accountId]; + NSFileProviderDomain * const domain = _registeredDomains[accountId]; + [domain retain]; // Caller must release + return domain; } } @@ -312,6 +321,7 @@ void findExistingFileProviderDomains() QString fileProviderDomainIdentifierForAccountId(const QString &accountId) { if (@available(macOS 11.0, *)) { + QMutexLocker locker(&_registeredDomainsMutex); if (_registeredDomains.contains(accountId)) { const auto fileProviderDomain = _registeredDomains[accountId]; return QString::fromNSString([fileProviderDomain identifier]); @@ -338,23 +348,42 @@ void addFileProviderDomain(const AccountState * const accountState) << accountId; // Check if we already have a domain for this account (by account ID, not domain ID) - if (_registeredDomains.contains(accountId) && _registeredDomains.value(accountId) != nil) { - qCDebug(lcMacFileProviderDomainManager) << "File provider domain already exists for account: " - << accountId; - return; + { + QMutexLocker locker(&_registeredDomainsMutex); + if (_registeredDomains.contains(accountId) && _registeredDomains.value(accountId) != nil) { + qCDebug(lcMacFileProviderDomainManager) << "File provider domain already exists for account: " + << accountId; + return; + } } NSFileProviderDomain * const fileProviderDomain = [[NSFileProviderDomain alloc] initWithIdentifier:domainId.toNSString() displayName:domainDisplayName.toNSString()]; + // Set supportsSyncingTrash based on user setting (macOS 13.0+) + if (@available(macOS 13.0, *)) { + const bool trashSyncEnabled = + FileProviderSettingsController::instance()->trashSyncEnabledForAccount(accountId); + fileProviderDomain.supportsSyncingTrash = trashSyncEnabled; + qCInfo(lcMacFileProviderDomainManager) << "Setting supportsSyncingTrash to" + << trashSyncEnabled + << "for domain" + << domainId; + } + [NSFileProviderManager addDomain:fileProviderDomain completionHandler:^(NSError * const error) { if(error) { qCWarning(lcMacFileProviderDomainManager) << "Error adding file provider domain: " << error.code << error.localizedDescription; + [fileProviderDomain release]; + return; } - _registeredDomains.insert(accountId, fileProviderDomain); // Store by account ID for easier lookup + { + QMutexLocker locker(&_registeredDomainsMutex); + _registeredDomains.insert(accountId, fileProviderDomain); // Store by account ID for easier lookup + } }]; } } @@ -402,14 +431,18 @@ void removeFileProviderDomain(const AccountState * const accountState) qCInfo(lcMacFileProviderDomainManager) << "Removing file provider domain for account: " << accountId; - if (!_registeredDomains.contains(accountId)) { - qCWarning(lcMacFileProviderDomainManager) << "File provider domain not found for account: " - << accountId; - return; + NSFileProviderDomain *fileProviderDomain = nil; + { + QMutexLocker locker(&_registeredDomainsMutex); + if (!_registeredDomains.contains(accountId)) { + qCWarning(lcMacFileProviderDomainManager) << "File provider domain not found for account: " + << accountId; + return; + } + fileProviderDomain = _registeredDomains[accountId]; + [fileProviderDomain retain]; // Keep alive during async operation } - NSFileProviderDomain * const fileProviderDomain = _registeredDomains[accountId]; - [NSFileProviderManager removeDomain:fileProviderDomain completionHandler:^(NSError *error) { if (error) { qCWarning(lcMacFileProviderDomainManager) << "Error removing file provider domain: " @@ -418,8 +451,15 @@ void removeFileProviderDomain(const AccountState * const accountState) } removeFileProviderDomainData(fileProviderDomain.identifier); - NSFileProviderDomain * const domain = _registeredDomains.take(accountId); - [domain release]; + NSFileProviderDomain *domain = nil; + { + QMutexLocker locker(&_registeredDomainsMutex); + domain = _registeredDomains.take(accountId); + } + if (domain) { + [domain release]; // Release the map's ownership (if we took it) + } + [fileProviderDomain release]; // Balance our retain // Clean up the UUID mapping when removing the domain OCC::ConfigFile cfg; @@ -428,6 +468,155 @@ void removeFileProviderDomain(const AccountState * const accountState) } } + void reconfigureFileProviderDomain(const QString &accountId) + { + if (@available(macOS 13.0, *)) { + qCInfo(lcMacFileProviderDomainManager) << "Reconfiguring file provider domain for account:" + << accountId; + + NSFileProviderDomain *oldDomain = nil; + QString domainId; + QString displayName; + bool oldTrashSyncEnabled = false; + { + QMutexLocker locker(&_registeredDomainsMutex); + if (!_registeredDomains.contains(accountId)) { + qCWarning(lcMacFileProviderDomainManager) << "Cannot reconfigure non-existent domain for account:" + << accountId; + return; + } + oldDomain = _registeredDomains[accountId]; + [oldDomain retain]; // Keep alive during async operations + domainId = QString::fromNSString(oldDomain.identifier); + displayName = QString::fromNSString(oldDomain.displayName); + oldTrashSyncEnabled = oldDomain.supportsSyncingTrash; + } + + const bool newTrashSyncEnabled = + FileProviderSettingsController::instance()->trashSyncEnabledForAccount(accountId); + + // Check if reconfiguration is actually needed + if (oldTrashSyncEnabled == newTrashSyncEnabled) { + qCDebug(lcMacFileProviderDomainManager) << "No reconfiguration needed for account" + << accountId + << "- supportsSyncingTrash already" + << newTrashSyncEnabled; + [oldDomain release]; // Balance our retain + return; + } + + qCInfo(lcMacFileProviderDomainManager) << "Reconfiguring domain" + << domainId + << "with supportsSyncingTrash =" + << newTrashSyncEnabled; + + // Remove from map before async removal to prevent concurrent access to stale pointer + { + QMutexLocker locker(&_registeredDomainsMutex); + _registeredDomains.remove(accountId); + } + + // Remove old domain and re-add with new configuration + // Note: We don't remove domain data as we're just reconfiguring, not fully removing + [NSFileProviderManager removeDomain:oldDomain completionHandler:^(NSError * const removeError) { + if (removeError) { + qCWarning(lcMacFileProviderDomainManager) << "Error removing domain for reconfiguration:" + << removeError.code + << removeError.localizedDescription; + { + QMutexLocker locker(&_registeredDomainsMutex); + _registeredDomains.insert(accountId, oldDomain); + } + const QString errorMsg = QString::fromNSString(removeError.localizedDescription); + QPointer safeManager = _manager; + if (!safeManager) { + return; + } + QMetaObject::invokeMethod(safeManager.data(), [safeManager, accountId, errorMsg]() { + if (safeManager) { + emit safeManager->domainReconfigurationFailed(accountId, errorMsg); + } + }, Qt::QueuedConnection); + return; + } + + [oldDomain release]; + + NSFileProviderDomain * const newDomain = + [[NSFileProviderDomain alloc] initWithIdentifier:domainId.toNSString() + displayName:displayName.toNSString()]; + + newDomain.supportsSyncingTrash = newTrashSyncEnabled; + + [NSFileProviderManager addDomain:newDomain completionHandler:^(NSError * const addError) { + if (addError) { + qCWarning(lcMacFileProviderDomainManager) << "Error re-adding domain after reconfiguration:" + << addError.code + << addError.localizedDescription; + const QString addErrorMsg = QString::fromNSString(addError.localizedDescription); + NSFileProviderDomain * const restoredDomain = + [[NSFileProviderDomain alloc] initWithIdentifier:domainId.toNSString() + displayName:displayName.toNSString()]; + restoredDomain.supportsSyncingTrash = oldTrashSyncEnabled; + + [NSFileProviderManager addDomain:restoredDomain completionHandler:^(NSError * const restoreError) { + const bool restoreFailed = (restoreError != nil); + if (restoreFailed) { + qCWarning(lcMacFileProviderDomainManager) << "Failed to restore domain:" + << restoreError.code; + [restoredDomain release]; + } else { + qCInfo(lcMacFileProviderDomainManager) << "Successfully restored domain after failed reconfiguration"; + { + QMutexLocker locker(&_registeredDomainsMutex); + _registeredDomains.insert(accountId, restoredDomain); + } + } + + QPointer safeManager = _manager; + if (!safeManager) { + return; + } + QMetaObject::invokeMethod(safeManager.data(), [safeManager, accountId, addErrorMsg, oldTrashSyncEnabled, restoreFailed]() { + if (!safeManager) { + return; + } + if (!restoreFailed) { + FileProviderSettingsController::instance()->setTrashSyncEnabledForAccount(accountId, oldTrashSyncEnabled); + } + emit safeManager->domainReconfigurationFailed(accountId, addErrorMsg); + }, Qt::QueuedConnection); + }]; + [newDomain release]; + return; + } + + qCInfo(lcMacFileProviderDomainManager) << "Successfully reconfigured domain with supportsSyncingTrash:" + << newTrashSyncEnabled; + + { + QMutexLocker locker(&_registeredDomainsMutex); + _registeredDomains.insert(accountId, newDomain); + } + + // Reconnect the newly configured domain + NSFileProviderManager * const fpManager = [NSFileProviderManager managerForDomain:newDomain]; + [fpManager reconnectWithCompletionHandler:^(NSError * const reconnectError) { + if (reconnectError) { + qCWarning(lcMacFileProviderDomainManager) << "Error reconnecting reconfigured domain:" + << reconnectError.code + << reconnectError.localizedDescription; + } else { + qCInfo(lcMacFileProviderDomainManager) << "Successfully reconnected reconfigured domain"; + } + }]; + }]; + }]; + } else { + qCWarning(lcMacFileProviderDomainManager) << "supportsSyncingTrash requires macOS 13.0+"; + } + } + void removeAllFileProviderDomains() { if (@available(macOS 11.0, *)) { @@ -441,8 +630,12 @@ void removeAllFileProviderDomains() return; } - const auto registeredDomainPtrs = _registeredDomains.values(); - const auto accountIds = _registeredDomains.keys(); + QList registeredDomainPtrs; + { + QMutexLocker locker(&_registeredDomainsMutex); + registeredDomainPtrs = _registeredDomains.values(); + _registeredDomains.clear(); + } for (NSFileProviderDomain * const domain : registeredDomainPtrs) { removeFileProviderDomainData(domain.identifier); @@ -451,8 +644,6 @@ void removeAllFileProviderDomains() [domain release]; } } - - _registeredDomains.clear(); }]; } } @@ -485,10 +676,14 @@ void wipeAllFileProviderDomains() removeFileProviderDomainData(domain.identifier); const QString accountId = accountIdFromDomainId(domain.identifier); - NSFileProviderDomain * const registeredDomainPtr = _registeredDomains.take(accountId); + NSFileProviderDomain *registeredDomainPtr = nil; + { + QMutexLocker locker(&_registeredDomainsMutex); + registeredDomainPtr = _registeredDomains.take(accountId); + } if (registeredDomainPtr != nil) { - [domain release]; + [registeredDomainPtr release]; // Release OUR retained copy, not the getDomains array element } }]; } @@ -510,13 +705,17 @@ void disconnectFileProviderDomainForAccount(const AccountState * const accountSt qCInfo(lcMacFileProviderDomainManager) << "Disconnecting file provider domain for account: " << accountId; - if(!_registeredDomains.contains(accountId)) { - qCInfo(lcMacFileProviderDomainManager) << "File provider domain not found for account: " - << accountId; - return; + NSFileProviderDomain *fileProviderDomain = nil; + { + QMutexLocker locker(&_registeredDomainsMutex); + if(!_registeredDomains.contains(accountId)) { + qCInfo(lcMacFileProviderDomainManager) << "File provider domain not found for account: " + << accountId; + return; + } + fileProviderDomain = _registeredDomains[accountId]; + [fileProviderDomain retain]; // Keep alive during async operation } - - NSFileProviderDomain * const fileProviderDomain = _registeredDomains[accountId]; Q_ASSERT(fileProviderDomain != nil); NSFileProviderManager * const fpManager = [NSFileProviderManager managerForDomain:fileProviderDomain]; @@ -528,11 +727,11 @@ void disconnectFileProviderDomainForAccount(const AccountState * const accountSt << fileProviderDomain.displayName << error.code << error.localizedDescription; - return; + } else { + qCInfo(lcMacFileProviderDomainManager) << "Successfully disconnected file provider domain: " + << fileProviderDomain.displayName; } - - qCInfo(lcMacFileProviderDomainManager) << "Successfully disconnected file provider domain: " - << fileProviderDomain.displayName; + [fileProviderDomain release]; // Balance the retain }]; } } @@ -548,13 +747,17 @@ void reconnectFileProviderDomainForAccount(const AccountState * const accountSta qCInfo(lcMacFileProviderDomainManager) << "Reconnecting file provider domain for account: " << accountId; - if(!_registeredDomains.contains(accountId)) { - qCInfo(lcMacFileProviderDomainManager) << "File provider domain not found for account: " - << accountId; - return; + NSFileProviderDomain *fileProviderDomain = nil; + { + QMutexLocker locker(&_registeredDomainsMutex); + if(!_registeredDomains.contains(accountId)) { + qCInfo(lcMacFileProviderDomainManager) << "File provider domain not found for account: " + << accountId; + return; + } + fileProviderDomain = _registeredDomains[accountId]; + [fileProviderDomain retain]; // Keep alive during async operation } - - NSFileProviderDomain * const fileProviderDomain = _registeredDomains[accountId]; Q_ASSERT(fileProviderDomain != nil); NSFileProviderManager * const fpManager = [NSFileProviderManager managerForDomain:fileProviderDomain]; @@ -565,13 +768,13 @@ void reconnectFileProviderDomainForAccount(const AccountState * const accountSta << fileProviderDomain.displayName << error.code << error.localizedDescription; - return; - } - - qCInfo(lcMacFileProviderDomainManager) << "Successfully reconnected file provider domain: " - << fileProviderDomain.displayName; + } else { + qCInfo(lcMacFileProviderDomainManager) << "Successfully reconnected file provider domain: " + << fileProviderDomain.displayName; - signalEnumeratorChanged(account.get()); + signalEnumeratorChanged(account.get()); + } + [fileProviderDomain release]; // Balance the retain }]; } } @@ -585,13 +788,17 @@ void signalEnumeratorChanged(const Account * const account) qCInfo(lcMacFileProviderDomainManager) << "Signalling enumerator changed in file provider domain for account: " << accountId; - if(!_registeredDomains.contains(accountId)) { - qCInfo(lcMacFileProviderDomainManager) << "File provider domain not found for account: " - << accountId; - return; + NSFileProviderDomain *fileProviderDomain = nil; + { + QMutexLocker locker(&_registeredDomainsMutex); + if(!_registeredDomains.contains(accountId)) { + qCInfo(lcMacFileProviderDomainManager) << "File provider domain not found for account: " + << accountId; + return; + } + fileProviderDomain = _registeredDomains[accountId]; + [fileProviderDomain retain]; // Keep alive during async operation } - - NSFileProviderDomain * const fileProviderDomain = _registeredDomains[accountId]; Q_ASSERT(fileProviderDomain != nil); NSFileProviderManager * const fpManager = [NSFileProviderManager managerForDomain:fileProviderDomain]; @@ -600,25 +807,63 @@ void signalEnumeratorChanged(const Account * const account) qCWarning(lcMacFileProviderDomainManager) << "Error signalling enumerator changed for working set:" << error.localizedDescription; } + [fileProviderDomain release]; // Balance the retain }]; } } QStringList getAccountIdsOfFoundFileProviderDomains() const { + QMutexLocker locker(&_registeredDomainsMutex); return _registeredDomains.keys(); } + void reconcileTrashSyncSettings() + { + if (@available(macOS 13.0, *)) { + qCDebug(lcMacFileProviderDomainManager) << "Reconciling trash sync settings with existing domains"; + + QList> accountsToCheck; + { + QMutexLocker locker(&_registeredDomainsMutex); + for (auto it = _registeredDomains.constBegin(); it != _registeredDomains.constEnd(); ++it) { + NSFileProviderDomain * const domain = it.value(); + if (domain != nil) { + accountsToCheck.append({it.key(), domain.supportsSyncingTrash}); + } + } + } + + for (const auto &[accountId, domainTrashSyncEnabled] : accountsToCheck) { + const bool prefTrashSyncEnabled = + FileProviderSettingsController::instance()->trashSyncEnabledForAccount(accountId); + + if (prefTrashSyncEnabled != domainTrashSyncEnabled) { + qCInfo(lcMacFileProviderDomainManager) << "Trash sync mismatch for account" << accountId + << "- pref:" << prefTrashSyncEnabled + << "domain:" << domainTrashSyncEnabled + << "- triggering reconfiguration"; + reconfigureFileProviderDomain(accountId); + } else { + qCDebug(lcMacFileProviderDomainManager) << "Trash sync setting matches for account" << accountId + << "- value:" << prefTrashSyncEnabled; + } + } + } + } + private: + FileProviderDomainManager *_manager = nullptr; //! keys are accountId, i.e. userIdAtHostWithPort QHash _registeredDomains; + mutable QMutex _registeredDomainsMutex; }; FileProviderDomainManager::FileProviderDomainManager(QObject * const parent) : QObject(parent) { if (@available(macOS 11.0, *)) { - d.reset(new FileProviderDomainManager::MacImplementation()); + d.reset(new FileProviderDomainManager::MacImplementation(this)); } else { qCWarning(lcMacFileProviderDomainManager()) << "Trying to run File Provider on system that does not support it."; } @@ -646,6 +891,9 @@ QStringList getAccountIdsOfFoundFileProviderDomains() const connect(FileProviderSettingsController::instance(), &FileProviderSettingsController::vfsEnabledAccountsChanged, this, &FileProviderDomainManager::updateFileProviderDomains); + + connect(FileProviderSettingsController::instance(), &FileProviderSettingsController::trashSyncEnabledForAccountChanged, + this, &FileProviderDomainManager::reconfigureFileProviderDomainForAccount); } void FileProviderDomainManager::setupFileProviderDomains() @@ -656,6 +904,7 @@ QStringList getAccountIdsOfFoundFileProviderDomains() const d->findExistingFileProviderDomains(); updateFileProviderDomains(); + d->reconcileTrashSyncSettings(); } void FileProviderDomainManager::updateFileProviderDomains() @@ -768,6 +1017,15 @@ QStringList getAccountIdsOfFoundFileProviderDomains() const d->removeFileProviderDomain(accountState); } +void FileProviderDomainManager::reconfigureFileProviderDomainForAccount(const QString &accountId) +{ + if (!d) { + return; + } + + d->reconfigureFileProviderDomain(accountId); +} + void FileProviderDomainManager::disconnectFileProviderDomainForAccount(const AccountState * const accountState, const QString &reason) { if (!d) { diff --git a/src/gui/macOS/fileprovidereditlocallyjob_mac.mm b/src/gui/macOS/fileprovidereditlocallyjob_mac.mm index 01552297a17aa..9f26cee6d7b73 100644 --- a/src/gui/macOS/fileprovidereditlocallyjob_mac.mm +++ b/src/gui/macOS/fileprovidereditlocallyjob_mac.mm @@ -32,18 +32,21 @@ const auto userId = _accountState->account()->userIdAtHostWithPort(); const auto ncDomainManager = FileProvider::instance()->domainManager(); const auto voidDomain = ncDomainManager->domainForAccount(_accountState.data()); - + NSFileProviderDomain *const domain = (NSFileProviderDomain *)voidDomain; if (domain == nil) { qCWarning(lcFileProviderEditLocallyMacJob) << "Could not get domain for account:" << userId; emit notAvailable(); + return; } NSFileProviderManager *const manager = [NSFileProviderManager managerForDomain:domain]; + [domain release]; // Balance retain from domainForAccount if (manager == nil) { qCWarning(lcFileProviderEditLocallyMacJob) << "Could not get file provider manager" "for domain of account:" << userId;; emit notAvailable(); + return; } [manager retain]; diff --git a/src/gui/macOS/fileprovidersettingscontroller.h b/src/gui/macOS/fileprovidersettingscontroller.h index 081a28c784646..c2f3a0c17da49 100644 --- a/src/gui/macOS/fileprovidersettingscontroller.h +++ b/src/gui/macOS/fileprovidersettingscontroller.h @@ -29,17 +29,16 @@ class FileProviderSettingsController : public QObject [[nodiscard]] QStringList vfsEnabledAccounts() const; [[nodiscard]] Q_INVOKABLE bool vfsEnabledForAccount(const QString &userIdAtHost) const; - [[nodiscard]] Q_INVOKABLE bool trashDeletionEnabledForAccount(const QString &userIdAtHost) const; - [[nodiscard]] Q_INVOKABLE bool trashDeletionSetForAccount(const QString &userIdAtHost) const; + [[nodiscard]] Q_INVOKABLE bool trashSyncEnabledForAccount(const QString &userIdAtHost) const; + [[nodiscard]] Q_INVOKABLE bool trashSyncSupported() const; public slots: void setVfsEnabledForAccount(const QString &userIdAtHost, const bool setEnabled, const bool showInformationDialog = true); - void setTrashDeletionEnabledForAccount(const QString &userIdAtHost, const bool setEnabled); + void setTrashSyncEnabledForAccount(const QString &userIdAtHost, const bool setEnabled); signals: void vfsEnabledAccountsChanged(); - void trashDeletionEnabledForAccountChanged(const QString &userIdAtHost); - void trashDeletionSetForAccountChanged(const QString &userIdAtHost); + void trashSyncEnabledForAccountChanged(const QString &userIdAtHost); private: explicit FileProviderSettingsController(QObject *parent = nullptr); diff --git a/src/gui/macOS/fileprovidersettingscontroller_mac.mm b/src/gui/macOS/fileprovidersettingscontroller_mac.mm index 611cc9f34e151..b77dc78c06be4 100644 --- a/src/gui/macOS/fileprovidersettingscontroller_mac.mm +++ b/src/gui/macOS/fileprovidersettingscontroller_mac.mm @@ -26,6 +26,7 @@ // NSUserDefaults entries constexpr auto enabledAccountsSettingsKey = "enabledAccounts"; +constexpr auto trashSyncEnabledAccountsKey = "trashSyncEnabledAccounts"; } // namespace @@ -220,61 +221,52 @@ void initialCheck() } } -bool FileProviderSettingsController::trashDeletionEnabledForAccount(const QString &userIdAtHost) const +bool FileProviderSettingsController::trashSyncEnabledForAccount(const QString &userIdAtHost) const { if (userIdAtHost.isEmpty()) { - return false; + return true; // Default to enabled for backward compatibility } - const auto xpc = FileProvider::instance()->xpc(); + NSUserDefaults *const defaults = NSUserDefaults.standardUserDefaults; + NSString *const settingsKey = [NSString stringWithUTF8String:trashSyncEnabledAccountsKey]; + NSDictionary *const settings = [defaults objectForKey:settingsKey]; - if (!xpc) { - return true; - } - - const auto domainId = FileProviderUtils::domainIdentifierForAccountIdentifier(userIdAtHost); - - if (const auto trashDeletionState = xpc->trashDeletionEnabledStateForFileProviderDomain(domainId)) { - return trashDeletionState->first; + if (settings == nil) { + return true; // Default to enabled for backward compatibility } - return true; + NSNumber *const value = settings[userIdAtHost.toNSString()]; + return value == nil ? true : value.boolValue; } -bool FileProviderSettingsController::trashDeletionSetForAccount(const QString &userIdAtHost) const +void FileProviderSettingsController::setTrashSyncEnabledForAccount(const QString &userIdAtHost, const bool setEnabled) { - const auto xpc = FileProvider::instance()->xpc(); - - if (!xpc) { - return false; + qCInfo(lcFileProviderSettingsController) << "Setting trash sync enabled for account" + << userIdAtHost + << "to" + << setEnabled; + + NSUserDefaults *const defaults = NSUserDefaults.standardUserDefaults; + NSString *const settingsKey = [NSString stringWithUTF8String:trashSyncEnabledAccountsKey]; + NSMutableDictionary *settings = + [[defaults objectForKey:settingsKey] mutableCopy]; + + if (settings == nil) { + settings = [NSMutableDictionary dictionary]; } - const auto domainId = FileProviderUtils::domainIdentifierForAccountIdentifier(userIdAtHost); - - if (const auto state = xpc->trashDeletionEnabledStateForFileProviderDomain(domainId)) { - return state->second; - } + settings[userIdAtHost.toNSString()] = @(setEnabled); + [defaults setObject:settings forKey:settingsKey]; - return false; + emit trashSyncEnabledForAccountChanged(userIdAtHost); } -void FileProviderSettingsController::setTrashDeletionEnabledForAccount(const QString &userIdAtHost, const bool setEnabled) +bool FileProviderSettingsController::trashSyncSupported() const { - const auto xpc = FileProvider::instance()->xpc(); - - if (!xpc) { - // Reset state of UI elements - emit trashDeletionEnabledForAccountChanged(userIdAtHost); - emit trashDeletionSetForAccountChanged(userIdAtHost); - return; + if (@available(macOS 13.0, *)) { + return true; } - - const auto domainId = FileProviderUtils::domainIdentifierForAccountIdentifier(userIdAtHost); - - xpc->setTrashDeletionEnabledForFileProviderDomain(domainId, setEnabled); - - emit trashDeletionEnabledForAccountChanged(userIdAtHost); - emit trashDeletionSetForAccountChanged(userIdAtHost); + return false; } } // namespace Mac diff --git a/src/gui/macOS/fileproviderxpc.h b/src/gui/macOS/fileproviderxpc.h index 038d930c7695e..cd3a4920f60c1 100644 --- a/src/gui/macOS/fileproviderxpc.h +++ b/src/gui/macOS/fileproviderxpc.h @@ -28,8 +28,6 @@ class FileProviderXPC : public QObject [[nodiscard]] bool fileProviderDomainReachable(const QString &fileProviderDomainIdentifier, bool retry = true, bool reconfigureOnFail = true); - [[nodiscard]] std::optional> trashDeletionEnabledStateForFileProviderDomain(const QString &fileProviderDomainIdentifier) const; - public slots: void connectToFileProviderDomains(); void authenticateFileProviderDomains(); @@ -37,7 +35,6 @@ public slots: void unauthenticateFileProviderDomain(const QString &fileProviderDomainIdentifier) const; void setIgnoreList() const; - void setTrashDeletionEnabledForFileProviderDomain(const QString &fileProviderDomainIdentifier, bool enabled) const; private slots: void slotAccountStateChanged(AccountState::State state) const; diff --git a/src/gui/macOS/fileproviderxpc_mac.mm b/src/gui/macOS/fileproviderxpc_mac.mm index 03cb0e7755097..ae242ee825300 100644 --- a/src/gui/macOS/fileproviderxpc_mac.mm +++ b/src/gui/macOS/fileproviderxpc_mac.mm @@ -161,8 +161,14 @@ << "going to attempt reconfiguring interface"; const auto ncDomainManager = FileProvider::instance()->domainManager(); const auto accountState = ncDomainManager->accountStateFromFileProviderDomainIdentifier(fileProviderDomainIdentifier); - const auto domain = (NSFileProviderDomain *)(ncDomainManager->domainForAccount(accountState.get())); + NSFileProviderDomain *const domain = (NSFileProviderDomain *)(ncDomainManager->domainForAccount(accountState.get())); + if (domain == nil) { + qCWarning(lcFileProviderXPC) << "Cannot reconfigure unreachable domain, domain pointer is nil"; + return false; + } const auto manager = [NSFileProviderManager managerForDomain:domain]; + [domain release]; // Balance retain in domainForAccount + const auto fpServices = FileProviderXPCUtils::getFileProviderServices(@[manager]); const auto connections = FileProviderXPCUtils::connectToFileProviderServices(fpServices); const auto services = FileProviderXPCUtils::processClientCommunicationConnections(connections); @@ -179,41 +185,6 @@ return response; } -std::optional> FileProviderXPC::trashDeletionEnabledStateForFileProviderDomain(const QString &fileProviderDomainIdentifier) const -{ - qCInfo(lcFileProviderXPC) << "Checking if fast enumeration is enabled for file provider domain" << fileProviderDomainIdentifier; - const auto service = (NSObject *)_clientCommServices.value(fileProviderDomainIdentifier); - - if (service == nil) { - qCWarning(lcFileProviderXPC) << "Could not get service for file provider domain" << fileProviderDomainIdentifier; - return std::nullopt; - } - - __block BOOL receivedTrashDeletionEnabled = YES; // What is the value of the setting being used by the extension? - __block BOOL receivedTrashDeletionEnabledSet = NO; // Has the setting been set by the user? - __block BOOL receivedResponse = NO; - dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); - [service getTrashDeletionEnabledStateWithCompletionHandler:^(BOOL enabled, BOOL set) { - receivedTrashDeletionEnabled = enabled; - receivedTrashDeletionEnabledSet = set; - receivedResponse = YES; - dispatch_semaphore_signal(semaphore); - }]; - dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, semaphoreWaitDelta)); - if (!receivedResponse) { - qCWarning(lcFileProviderXPC) << "Did not receive response for fast enumeration state"; - return std::nullopt; - } - return std::optional>{{receivedTrashDeletionEnabled, receivedTrashDeletionEnabledSet}}; -} - -void FileProviderXPC::setTrashDeletionEnabledForFileProviderDomain(const QString &fileProviderDomainIdentifier, bool enabled) const -{ - qCInfo(lcFileProviderXPC) << "Setting trash deletion enabled for file provider domain" << fileProviderDomainIdentifier << "to" << enabled; - const auto service = (NSObject *)_clientCommServices.value(fileProviderDomainIdentifier); - [service setTrashDeletionEnabled:enabled]; -} - void FileProviderXPC::setIgnoreList() const { ExcludedFiles ignoreList; diff --git a/src/gui/macOS/ui/FileProviderSettings.qml b/src/gui/macOS/ui/FileProviderSettings.qml index e15b3ec63e2f5..66b80c6c855b2 100644 --- a/src/gui/macOS/ui/FileProviderSettings.qml +++ b/src/gui/macOS/ui/FileProviderSettings.qml @@ -53,11 +53,27 @@ Page { checked: root.controller.vfsEnabledForAccount(root.accountUserIdAtHost) onClicked: root.controller.setVfsEnabledForAccount(root.accountUserIdAtHost, checked) } - + CheckBox { - text: qsTr("Allow deletion of items in Trash") - checked: root.controller.trashDeletionEnabledForAccount(root.accountUserIdAtHost) - onClicked: root.controller.setTrashDeletionEnabledForAccount(root.accountUserIdAtHost, checked) + id: trashSyncCheckBox + visible: root.controller.trashSyncSupported() + text: qsTr("Integrate macOS Trash") + checked: root.controller.trashSyncEnabledForAccount(root.accountUserIdAtHost) + onClicked: root.controller.setTrashSyncEnabledForAccount(root.accountUserIdAtHost, checked) + + ToolTip.visible: hovered + ToolTip.text: qsTr("When enabled, deleted files go to macOS Trash and server trash is visible.\nWhen disabled, Finder shows 'Delete Immediately' (files still recoverable from server trash).") + ToolTip.delay: 500 + + Connections { + target: root.controller + function onTrashSyncEnabledForAccountChanged(accountUserIdAtHost) { + if (root.accountUserIdAtHost !== accountUserIdAtHost) { + return; + } + trashSyncCheckBox.checked = root.controller.trashSyncEnabledForAccount(root.accountUserIdAtHost); + } + } } } }