From 0cf792cc640bc4c1e9834588ad723074e8df70f8 Mon Sep 17 00:00:00 2001
From: Gregor Longariva <gregor.longariva@fau.de>
Date: Mon, 24 Mar 2025 16:33:31 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20a=20few=20improvements=20to=20mo?=
=?UTF-8?q?unter.swift=20and=20tst=20szenarios=20for=20mounter=20class?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
Network Share Mounter/model/Mounter.swift | 69 +++-
.../ManagerTests/MounterTests.swift | 344 ++++++++++++++++++
2 files changed, 400 insertions(+), 13 deletions(-)
create mode 100644 Network Share MounterTests/ManagerTests/MounterTests.swift
diff --git a/Network Share Mounter/model/Mounter.swift b/Network Share Mounter/model/Mounter.swift
index d8dd8c3..1814ba8 100644
--- a/Network Share Mounter/model/Mounter.swift
+++ b/Network Share Mounter/model/Mounter.swift
@@ -25,10 +25,29 @@ class Mounter: ObservableObject {
// static let mounter = Mounter.init()
/// define locks to protect `shares`-array from race conditions
private let lock = NSLock()
+ private let taskLock = NSLock()
+
+ /// Set to store active mount tasks
+ private var mountTasks = Set<Task<Void, Never>>()
+
/// get home direcotry for the user running the app
let userHomeDirectory: String = FileManager.default.homeDirectoryForCurrentUser.path
- var errorStatus: MounterError = .noError
+ private let errorStatusLock = NSLock()
+ private var _errorStatus: MounterError = .noError
+
+ var errorStatus: MounterError {
+ get {
+ errorStatusLock.lock()
+ defer { errorStatusLock.unlock() }
+ return _errorStatus
+ }
+ set {
+ errorStatusLock.lock()
+ _errorStatus = newValue
+ errorStatusLock.unlock()
+ }
+ }
private var localizedFolder = Defaults.translation["en"]!
var defaultMountPath: String = Defaults.defaultMountPath
@@ -133,6 +152,9 @@ class Mounter: ObservableObject {
/// - Parameter mountStatus: new MountStatus
/// - Parameter for: share to be updated
func updateShare(mountStatus: MountStatus, for share: Share) async {
+ lock.lock()
+ defer { lock.unlock() }
+
if let index = await shareManager.allShares.firstIndex(where: { $0.networkShare == share.networkShare }) {
do {
try await shareManager.updateMountStatus(at: index, to: mountStatus)
@@ -151,6 +173,9 @@ class Mounter: ObservableObject {
/// - Parameter actualMountPoint: an optional `String` definig where the share is mounted (or not, if not defined)
/// - Parameter for: share to be updated
func updateShare(actualMountPoint: String?, for share: Share) async {
+ lock.lock()
+ defer { lock.unlock() }
+
if let index = await shareManager.allShares.firstIndex(where: { $0.networkShare == share.networkShare }) {
do {
try await shareManager.updateActualMountPoint(at: index, to: actualMountPoint)
@@ -191,6 +216,15 @@ class Mounter: ObservableObject {
task.launch()
}
+ /// Safely escapes a path for use in shell commands
+ /// - Parameter path: The path to escape
+ /// - Returns: A properly escaped path string safe for use in shell commands
+ private func escapePath(_ path: String) -> String {
+ // Use single quotes which handle most special characters
+ // But escape single quotes within the path by replacing ' with '\''
+ return "'\(path.replacingOccurrences(of: "'", with: "'\\''"))'"
+ }
+
/// function to delete a directory via system shell `rmdir`
/// - Paramater atPath: full path of the directory
func removeDirectory(atPath: String) {
@@ -200,9 +234,8 @@ class Mounter: ObservableObject {
} else {
let task = Process()
task.launchPath = "/bin/rmdir"
- // Escape spaces in the path for shell command
- let escapedPath = atPath.replacingOccurrences(of: " ", with: "\\ ")
- task.arguments = ["\(escapedPath)"]
+ // Use the safe path escaping function
+ task.arguments = [atPath] // Process handles argument escaping
let pipe = Pipe()
task.standardOutput = pipe
//
@@ -443,6 +476,13 @@ class Mounter: ObservableObject {
/// - shareID: Optional ID of specific share to mount. If nil, mounts all configured shares
/// - Note: Requires active network connection to perform mount operations
func mountGivenShares(userTriggered: Bool = false, forShare shareID: String? = nil) async {
+ // Safely manage task collection
+ taskLock.lock()
+ // Cancel any existing mount tasks
+ mountTasks.forEach { $0.cancel() }
+ mountTasks.removeAll()
+ taskLock.unlock()
+
// Verify network connectivity before attempting mount operations
let netConnection = Monitor.shared
@@ -477,7 +517,7 @@ class Mounter: ObservableObject {
}
// Create concurrent mount tasks for each share
- var mountTasks: [Task<Void, Never>] = []
+ var localMountTasks: [Task<Void, Never>] = []
for share in sharesToMount {
let mountTask = Task {
do {
@@ -518,12 +558,17 @@ class Mounter: ObservableObject {
}
}
}
- mountTasks.append(mountTask)
+ localMountTasks.append(mountTask)
}
+ // Safely store the tasks for lifecycle management
+ taskLock.lock()
+ mountTasks = Set(localMountTasks)
+ taskLock.unlock()
+
// Wait for all mount tasks to complete
await withTaskGroup(of: Void.self) { group in
- for task in mountTasks {
+ for task in localMountTasks {
group.addTask {
await task.value
}
@@ -641,9 +686,8 @@ class Mounter: ObservableObject {
// (or if failed, the directory will be removed later)
// apparently there is no way t oset the `hidden` attribute via FileManager `setAttributes`
// https://developer.apple.com/documentation/foundation/filemanager/1413667-setattributes
- // Escape path for shell command
- let escapedPath = mountDirectory.replacingOccurrences(of: " ", with: "\\ ")
- try? await cliTask("/usr/bin/chflags hidden \(escapedPath)")
+ // Use the safe path escaping function
+ try? await cliTask("/usr/bin/chflags hidden \(escapePath(mountDirectory))")
}
if share.authType == .guest {
openOptions = Defaults.openOptionsGuest
@@ -661,9 +705,8 @@ class Mounter: ObservableObject {
case 0:
Logger.mounter.info("✅ \(url, privacy: .public): successfully mounted on \(mountDirectory, privacy: .public)")
// unhide the directory for the fresh mounted share
- // Escape path for shell command
- let escapedPath = mountDirectory.replacingOccurrences(of: " ", with: "\\ ")
- try? await cliTask("/usr/bin/chflags nohidden \(escapedPath)")
+ // Use the safe path escaping function
+ try? await cliTask("/usr/bin/chflags nohidden \(escapePath(mountDirectory))")
return mountDirectory
case 2:
Logger.mounter.info("❌ \(url, privacy: .public): does not exist")
diff --git a/Network Share MounterTests/ManagerTests/MounterTests.swift b/Network Share MounterTests/ManagerTests/MounterTests.swift
new file mode 100644
index 0000000..61f7a26
--- /dev/null
+++ b/Network Share MounterTests/ManagerTests/MounterTests.swift
@@ -0,0 +1,344 @@
+import XCTest
+@testable import Network_Share_Mounter
+
+/**
+ Tests für die Mounter-Klasse
+
+ Diese Tests verifizieren die Kernfunktionalität der Mounter-Klasse, die für das
+ Mounten und Unmounten von Netzwerk-Shares verantwortlich ist.
+
+ Hinweise:
+ - Dies sind hauptsächlich Integrationstests, die mit der tatsächlichen Mounter-Klasse arbeiten
+ - Alle Testmethoden sind als async markiert, da Mounter mit async/await arbeitet
+ - Wir fokussieren uns auf testbare Funktionalitäten ohne die Originalklasse zu ändern
+ */
+final class MounterTests: XCTestCase {
+
+ // MARK: - Properties
+
+ /// Das zu testende System
+ var sut: Mounter!
+
+ // Testkonstanten
+ let testShare1URL = "smb://testserver.example.com/testshare1"
+ let testShare2URL = "smb://testserver.example.com/testshare2"
+ let testUsername = "testuser"
+ let testPassword = "testpassword"
+ let testMountPoint = "/Users/testuser/Network"
+
+ // MARK: - Setup & Teardown
+
+ override func setUpWithError() throws {
+ // Mounter initialisieren
+ sut = Mounter()
+
+ // FakeURLProtocol für Netzwerk-Mocking (falls benötigt)
+ URLProtocol.registerClass(FakeURLProtocol.self)
+ }
+
+ override func tearDownWithError() throws {
+ // Aufräumen
+ sut = nil
+ URLProtocol.unregisterClass(FakeURLProtocol.self)
+ }
+
+ // MARK: - Hilfsmethoden
+
+ /// Erstellt einen Test-Share zum Testen
+ private func createTestShare(
+ networkShare: String = "smb://testserver.example.com/testshare",
+ authType: AuthType = .krb,
+ username: String? = "testuser",
+ password: String? = nil,
+ mountStatus: MountStatus = .unmounted,
+ mountPoint: String? = nil,
+ managed: Bool = false
+ ) -> Share {
+ return Share.createShare(
+ networkShare: networkShare,
+ authType: authType,
+ mountStatus: mountStatus,
+ username: username,
+ password: password,
+ mountPoint: mountPoint,
+ managed: managed
+ )
+ }
+
+ /// Fügt dem ShareManager einen Testshare hinzu
+ private func addTestShareToManager() async -> Share {
+ let testShare = createTestShare()
+ await sut.shareManager.addShare(testShare)
+ return testShare
+ }
+
+ // MARK: - Tests: Grundfunktionen
+
+ /// Test: Initialisierung des Mounter
+ func testInit() async throws {
+ // Given/When: Der Mounter wurde im Setup erstellt
+
+ // Then: Der Mounter sollte existieren und ShareManager enthalten
+ XCTAssertNotNil(sut, "Mounter sollte initialisiert werden")
+ XCTAssertNotNil(sut.shareManager, "Mounter sollte einen ShareManager haben")
+ XCTAssertEqual(sut.defaultMountPath, Defaults.defaultMountPath, "Standardpfad sollte gesetzt sein")
+ }
+
+ /// Test: AsyncInit-Methode
+ func testAsyncInit() async throws {
+ // Given
+ let prefs = PreferenceManager()
+ let useLocalized = false
+ let useNewLocation = true
+
+ // Setzen der Präferenzen im UserDefaults
+ let defaults = UserDefaults.standard
+ defaults.set(useLocalized, forKey: PreferenceKeys.useLocalizedMountDirectories.rawValue)
+ defaults.set(useNewLocation, forKey: PreferenceKeys.useNewDefaultLocation.rawValue)
+
+ // When
+ await sut.asyncInit()
+
+ // Then
+ // Standardpfad sollte bei useNewDefaultLocation = true auf /Volumes gesetzt sein
+ XCTAssertEqual(sut.defaultMountPath, Defaults.defaultMountPath, "defaultMountPath sollte auf Defaults.defaultMountPath gesetzt sein")
+
+ // Aufräumen
+ defaults.removeObject(forKey: PreferenceKeys.useLocalizedMountDirectories.rawValue)
+ defaults.removeObject(forKey: PreferenceKeys.useNewDefaultLocation.rawValue)
+ }
+
+ /// Test: AsyncInit mit benutzerdefinierten lokalen Ordnern
+ func testAsyncInitWithLocalizedFolders() async throws {
+ // Given
+ let useLocalized = true
+ let useNewLocation = false
+
+ // Setzen der Präferenzen im UserDefaults
+ let defaults = UserDefaults.standard
+ defaults.set(useLocalized, forKey: PreferenceKeys.useLocalizedMountDirectories.rawValue)
+ defaults.set(useNewLocation, forKey: PreferenceKeys.useNewDefaultLocation.rawValue)
+
+ // Speichere den ursprünglichen Sprachcode, um ihn später wiederherzustellen
+ let originalLanguageCode = Locale.current.languageCode
+
+ // When
+ await sut.asyncInit()
+
+ // Then
+ if useNewLocation {
+ XCTAssertEqual(sut.defaultMountPath, Defaults.defaultMountPath, "defaultMountPath sollte auf Defaults.defaultMountPath gesetzt sein")
+ } else {
+ let expectedLocalizedFolder = Defaults.translation[Locale.current.languageCode ?? "en"] ?? Defaults.translation["en"]!
+ let expectedPath = NSString(string: "~/\(expectedLocalizedFolder)").expandingTildeInPath
+ XCTAssertEqual(sut.defaultMountPath, expectedPath, "defaultMountPath sollte auf den lokalisierten Pfad gesetzt sein")
+ }
+
+ // Aufräumen
+ defaults.removeObject(forKey: PreferenceKeys.useLocalizedMountDirectories.rawValue)
+ defaults.removeObject(forKey: PreferenceKeys.useNewDefaultLocation.rawValue)
+ }
+
+ /// Test: Hinzufügen eines Shares
+ func testAddShare() async throws {
+ // Given
+ let testShare = createTestShare()
+
+ // When
+ await sut.addShare(testShare)
+
+ // Then
+ let shares = await sut.shareManager.allShares
+ XCTAssertEqual(shares.count, 1, "Ein Share sollte hinzugefügt worden sein")
+ XCTAssertEqual(shares[0].networkShare, testShare.networkShare, "Der hinzugefügte Share sollte im ShareManager sein")
+ }
+
+ /// Test: Entfernen eines Shares
+ func testRemoveShare() async throws {
+ // Given
+ let testShare = await addTestShareToManager()
+ let shares = await sut.shareManager.allShares
+ XCTAssertEqual(shares.count, 1, "Setup: Ein Share sollte hinzugefügt worden sein")
+
+ // When
+ await sut.removeShare(for: testShare)
+
+ // Then
+ let updatedShares = await sut.shareManager.allShares
+ XCTAssertEqual(updatedShares.count, 0, "Der Share sollte entfernt worden sein")
+ }
+
+ /// Test: Aktualisieren des Mount-Status eines Shares
+ func testUpdateShareMountStatus() async throws {
+ // Given
+ let testShare = await addTestShareToManager()
+
+ // When
+ await sut.updateShare(mountStatus: .mounted, for: testShare)
+
+ // Then
+ let shares = await sut.shareManager.allShares
+ if let index = shares.firstIndex(where: { $0.id == testShare.id }) {
+ XCTAssertEqual(shares[index].mountStatus, .mounted, "Der Mount-Status sollte auf 'mounted' gesetzt sein")
+ } else {
+ XCTFail("Der Share sollte im ShareManager sein")
+ }
+ }
+
+ /// Test: Aktualisieren des Mount-Punkts eines Shares
+ func testUpdateShareMountPoint() async throws {
+ // Given
+ let testShare = await addTestShareToManager()
+ let newMountPoint = "/Volumes/TestShare"
+
+ // When
+ await sut.updateShare(actualMountPoint: newMountPoint, for: testShare)
+
+ // Then
+ let shares = await sut.shareManager.allShares
+ if let index = shares.firstIndex(where: { $0.id == testShare.id }) {
+ XCTAssertEqual(shares[index].actualMountPoint, newMountPoint, "Der Mount-Punkt sollte aktualisiert werden")
+ } else {
+ XCTFail("Der Share sollte im ShareManager sein")
+ }
+ }
+
+ /// Test: Erstellen eines Verzeichnisses für Shares
+ func testCreateMountFolder() throws {
+ // Given
+ let tempDirectoryURL = FileManager.default.temporaryDirectory
+ let testDirURL = tempDirectoryURL.appendingPathComponent(UUID().uuidString)
+ let testDirPath = testDirURL.path
+
+ // Sicherstellen, dass das Verzeichnis nicht existiert
+ try? FileManager.default.removeItem(at: testDirURL)
+
+ // When
+ sut.createMountFolder(atPath: testDirPath)
+
+ // Then
+ XCTAssertTrue(FileManager.default.fileExists(atPath: testDirPath), "Verzeichnis sollte erstellt worden sein")
+
+ // Aufräumen
+ try? FileManager.default.removeItem(at: testDirURL)
+ }
+
+ /// Test: Abrufen eines Shares anhand der Netzwerk-URL
+ func testGetShareForNetworkShare() async throws {
+ // Given
+ let testShare = await addTestShareToManager()
+
+ // When
+ let foundShare = await sut.getShare(forNetworkShare: testShare.networkShare)
+
+ // Then
+ XCTAssertNotNil(foundShare, "Ein Share sollte gefunden werden")
+ XCTAssertEqual(foundShare?.networkShare, testShare.networkShare, "Der gefundene Share sollte korrekt sein")
+ }
+
+ /// Test: Setzen des Mount-Status für alle Shares
+ func testSetAllMountStatus() async throws {
+ // Given
+ let testShare1 = createTestShare(networkShare: testShare1URL)
+ let testShare2 = createTestShare(networkShare: testShare2URL)
+ await sut.shareManager.addShare(testShare1)
+ await sut.shareManager.addShare(testShare2)
+
+ // When
+ await sut.setAllMountStatus(to: .mounted)
+
+ // Then
+ let shares = await sut.shareManager.allShares
+ for share in shares {
+ XCTAssertEqual(share.mountStatus, .mounted, "Alle Shares sollten den Status 'mounted' haben")
+ }
+ }
+
+ /// Test: Aktualisieren eines Shares
+ func testUpdateShare() async throws {
+ // Given
+ let originalShare = createTestShare(networkShare: testShare1URL)
+ await sut.shareManager.addShare(originalShare)
+
+ let updatedShare = createTestShare(
+ networkShare: testShare1URL,
+ authType: .pwd,
+ username: "newuser",
+ password: "newpassword"
+ )
+
+ // When
+ await sut.updateShare(for: updatedShare)
+
+ // Then
+ let shares = await sut.shareManager.allShares
+ XCTAssertEqual(shares[0].authType, .pwd, "Die Authentifizierungsart sollte aktualisiert sein")
+ XCTAssertEqual(shares[0].username, "newuser", "Der Benutzername sollte aktualisiert sein")
+ }
+
+ /// Test: Path-Escaping-Funktion (indirekt)
+ func testEscapePath() async throws {
+ // Da die Methode private ist, testen wir sie indirekt durch eine andere Methode
+ // Wir erstellen temporär ein Verzeichnis, das Sonderzeichen enthält
+
+ // Given
+ let tempDirectoryURL = FileManager.default.temporaryDirectory
+ let specialDirName = "test'dir with spaces"
+ let testDirURL = tempDirectoryURL.appendingPathComponent(specialDirName)
+ let testDirPath = testDirURL.path
+
+ // Sicherstellen, dass das Verzeichnis existiert
+ try? FileManager.default.createDirectory(at: testDirURL, withIntermediateDirectories: true, attributes: nil)
+
+ // Zum Testen: Wir können nicht direkt testen, aber wir können überprüfen, ob ein Verzeichnis
+ // mit Sonderzeichen ohne Fehler entfernt werden kann
+ XCTAssertNoThrow(sut.removeDirectory(atPath: testDirPath), "Das Verzeichnis mit Sonderzeichen sollte ohne Fehler entfernt werden können")
+
+ // Aufräumen - für den Fall, dass etwas schiefgeht
+ try? FileManager.default.removeItem(at: testDirURL)
+ }
+
+ /// Test: Entfernen eines Verzeichnisses in /Volumes
+ func testRemoveDirectoryInVolumes() async throws {
+ // Given
+ let testDirPath = "/Volumes/TestDir" // Diese sollte nicht entfernt werden
+
+ // When/Then - die rmdir-Operation sollte keine Auswirkungen haben
+ // Da wir nicht tatsächlich ein Verzeichnis in /Volumes erstellen können, testen wir nur,
+ // dass die Funktion keine Exception wirft
+ XCTAssertNoThrow(sut.removeDirectory(atPath: testDirPath))
+ }
+}
+
+// MARK: - Hilfsklassen
+
+/// Fake URLProtocol für Netzwerktests
+class FakeURLProtocol: URLProtocol {
+ static var requestHandler: ((URLRequest) throws -> (HTTPURLResponse, Data))?
+
+ override class func canInit(with request: URLRequest) -> Bool {
+ return true
+ }
+
+ override class func canonicalRequest(for request: URLRequest) -> URLRequest {
+ return request
+ }
+
+ override func startLoading() {
+ guard let handler = FakeURLProtocol.requestHandler else {
+ client?.urlProtocolDidFinishLoading(self)
+ return
+ }
+
+ do {
+ let (response, data) = try handler(request)
+ client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
+ client?.urlProtocol(self, didLoad: data)
+ client?.urlProtocolDidFinishLoading(self)
+ } catch {
+ client?.urlProtocol(self, didFailWithError: error)
+ }
+ }
+
+ override func stopLoading() {}
+}
\ No newline at end of file
--
GitLab