From 509fa047dcd94932a35988c6ce6501f77aefc784 Mon Sep 17 00:00:00 2001 From: Chris George Date: Mon, 11 May 2026 15:06:32 -0700 Subject: [PATCH] Use SystemPath for DirectoryWatcher. (#1522) - Closes #1520. - Using URL for filesystem paths is bad practice. FilePath is safer and more ergonomic. - Same pattern as #1480 (HostDNSResolver) and #1518 (PacketFilter). --- Sources/APIServer/LocalhostDNSHandler.swift | 15 +- Sources/ContainerOS/DirectoryWatcher.swift | 45 +++--- .../DirectoryWatcherTest.swift | 129 +++++++++--------- 3 files changed, 99 insertions(+), 90 deletions(-) diff --git a/Sources/APIServer/LocalhostDNSHandler.swift b/Sources/APIServer/LocalhostDNSHandler.swift index 670d02cc..7d3c5f8b 100644 --- a/Sources/APIServer/LocalhostDNSHandler.swift +++ b/Sources/APIServer/LocalhostDNSHandler.swift @@ -34,22 +34,27 @@ actor LocalhostDNSHandler: DNSHandler { public init(configPath: FilePath = HostDNSResolver.defaultConfigPath, ttl: UInt32 = 5, log: Logger) { self.ttl = ttl - self.watcher = DirectoryWatcher(directoryURL: URL(filePath: configPath.string), log: log) + self.watcher = DirectoryWatcher(directoryPath: configPath, log: log) self.dns = [DNSName: IPv4Address]() } public func monitorResolvers() async { - await self.watcher.startWatching { [weak self] fileURLs in + await self.watcher.startWatching { [weak self] filePaths in var dns: [DNSName: IPv4Address] = [:] let regex = try Regex(HostDNSResolver.localhostOptionsRegex) - for file in fileURLs.filter({ $0.lastPathComponent.starts(with: HostDNSResolver.containerizationPrefix) }) { - let content = try String(contentsOf: file, encoding: .utf8) + for file in filePaths.filter({ + $0.lastComponent?.string.starts(with: HostDNSResolver.containerizationPrefix) == true + }) { + let content = try String(contentsOfFile: file.string, encoding: .utf8) if let match = content.firstMatch(of: regex), let ipv4 = (match[1].substring.flatMap { try? IPv4Address(String($0)) }) { - let name = String(file.lastPathComponent.dropFirst(HostDNSResolver.containerizationPrefix.count)) + guard let lastName = file.lastComponent?.string else { + continue + } + let name = String(lastName.dropFirst(HostDNSResolver.containerizationPrefix.count)) guard let dnsName = try? DNSName(name) else { continue } diff --git a/Sources/ContainerOS/DirectoryWatcher.swift b/Sources/ContainerOS/DirectoryWatcher.swift index 83890a24..ab00cabb 100644 --- a/Sources/ContainerOS/DirectoryWatcher.swift +++ b/Sources/ContainerOS/DirectoryWatcher.swift @@ -19,6 +19,7 @@ import ContainerizationOS import Foundation import Logging import Synchronization +import SystemPackage /// Watches a directory for changes and invokes a handler when the contents change. /// @@ -28,16 +29,16 @@ import Synchronization /// /// Example usage: /// ```swift -/// let watcher = DirectoryWatcher(directoryURL: myURL, log: logger) -/// try watcher.startWatching { urls in -/// print("Directory contents changed: \(urls)") +/// let watcher = DirectoryWatcher(directoryPath: myPath, log: logger) +/// try watcher.startWatching { paths in +/// print("Directory contents changed: \(paths)") /// } /// ``` public actor DirectoryWatcher { public static let watchPeriod = Duration.seconds(1) - /// The URL of the directory being watched. - public let directoryURL: URL + /// The path of the directory being watched. + public let directoryPath: FilePath private var task: Task? private let monitorQueue: DispatchQueue @@ -45,14 +46,14 @@ public actor DirectoryWatcher { private let log: Logger? - /// Creates a new `DirectoryWatcher` for the given directory URL. + /// Creates a new `DirectoryWatcher` for the given directory path. /// /// - Parameters: - /// - directoryURL: The URL of the directory to watch. + /// - directoryPath: The path of the directory to watch. /// - log: An optional logger for diagnostic messages. - public init(directoryURL: URL, log: Logger?) { - self.directoryURL = directoryURL - self.monitorQueue = DispatchQueue(label: "monitor:\(directoryURL.path)") + public init(directoryPath: FilePath, log: Logger?) { + self.directoryPath = directoryPath + self.monitorQueue = DispatchQueue(label: "monitor:\(directoryPath.string)") self.log = log self.source = Mutex(nil) } @@ -61,14 +62,14 @@ public actor DirectoryWatcher { /// /// - Parameters: /// - handler: handler to run on directory state change. - public func startWatching(handler: @Sendable @escaping ([URL]) throws -> Void) { + public func startWatching(handler: @Sendable @escaping ([FilePath]) throws -> Void) { self.task = Task { var exists: Bool var isDir: ObjCBool = false while true { do { - exists = FileManager.default.fileExists(atPath: self.directoryURL.path, isDirectory: &isDir) + exists = FileManager.default.fileExists(atPath: self.directoryPath.string, isDirectory: &isDir) if exists && isDir.boolValue && self.source.withLock({ $0 }) == nil { try _startWatching(handler: handler) } @@ -82,21 +83,21 @@ public actor DirectoryWatcher { } private func _startWatching( - handler: @escaping ([URL]) throws -> Void + handler: @escaping ([FilePath]) throws -> Void ) throws { - let descriptor = open(directoryURL.path, O_EVTONLY) + let descriptor = open(directoryPath.string, O_EVTONLY) guard descriptor > 0 else { - throw ContainerizationError(.internalError, message: "cannot open \(directoryURL.path), descriptor=\(descriptor)") + throw ContainerizationError(.internalError, message: "cannot open \(directoryPath.string), descriptor=\(descriptor)") } do { - let files = try FileManager.default.contentsOfDirectory(atPath: directoryURL.path) - try handler(files.map { directoryURL.appending(path: $0) }) + let files = try FileManager.default.contentsOfDirectory(atPath: directoryPath.string) + try handler(files.map { directoryPath.appending($0) }) } catch { - throw ContainerizationError(.internalError, message: "failed to run handler for \(directoryURL.path)") + throw ContainerizationError(.internalError, message: "failed to run handler for \(directoryPath.string)") } - log?.info("starting directory watcher", metadata: ["path": "\(directoryURL.path)"]) + log?.info("starting directory watcher", metadata: ["path": "\(directoryPath.string)"]) let dispatchSource = DispatchSource.makeFileSystemObjectSource( fileDescriptor: descriptor, @@ -118,12 +119,12 @@ public actor DirectoryWatcher { } do { - let files = try FileManager.default.contentsOfDirectory(atPath: directoryURL.path) - try handler(files.map { directoryURL.appending(path: $0) }) + let files = try FileManager.default.contentsOfDirectory(atPath: directoryPath.string) + try handler(files.map { directoryPath.appending($0) }) } catch { self.log?.error( "failed to run watch handler", - metadata: ["error": "\(error)", "path": "\(directoryURL.path)"]) + metadata: ["error": "\(error)", "path": "\(directoryPath.string)"]) } } diff --git a/Tests/ContainerOSTests/DirectoryWatcherTest.swift b/Tests/ContainerOSTests/DirectoryWatcherTest.swift index 99fe6b46..b64f1a5c 100644 --- a/Tests/ContainerOSTests/DirectoryWatcherTest.swift +++ b/Tests/ContainerOSTests/DirectoryWatcherTest.swift @@ -18,153 +18,156 @@ import ContainerOS import ContainerizationError import DNSServer import Foundation +import SystemPackage import Testing struct DirectoryWatcherTest { let testUUID = UUID().uuidString - private var testDir: URL! { - let tempDir = URL(fileURLWithPath: FileManager.default.currentDirectoryPath) + private var testDir: FilePath { + let tempURL = URL(fileURLWithPath: FileManager.default.currentDirectoryPath) .appendingPathComponent(".clitests") .appendingPathComponent(testUUID) - try! FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) - return tempDir + try! FileManager.default.createDirectory(at: tempURL, withIntermediateDirectories: true) + return FilePath(tempURL.path) } - private func withTempDir(_ body: (URL) async throws -> T) async throws -> T { - let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + private func withTempDir(_ body: (FilePath) async throws -> T) async throws -> T { + let tempURL = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: tempURL, withIntermediateDirectories: true) + let tempPath = FilePath(tempURL.path) defer { - try? FileManager.default.removeItem(at: tempDir) + try? FileManager.default.removeItem(at: tempURL) } - return try await body(tempDir) + return try await body(tempPath) } - private actor CreatedURLs { - nonisolated(unsafe) public var urls: [URL] + private actor CreatedPaths { + nonisolated(unsafe) public var paths: [FilePath] public init() { - self.urls = [] + self.paths = [] } } @Test func testWatchingExistingDirectory() async throws { - try await withTempDir { tempDir in + try await withTempDir { tempPath in - let watcher = DirectoryWatcher(directoryURL: tempDir, log: nil) - let createdURLs = CreatedURLs() + let watcher = DirectoryWatcher(directoryPath: tempPath, log: nil) + let createdPaths = CreatedPaths() let name = "newFile" - await watcher.startWatching { [createdURLs] urls in - for url in urls where url.lastPathComponent == name { - createdURLs.urls.append(url) + await watcher.startWatching { [createdPaths] paths in + for path in paths where path.lastComponent?.string == name { + createdPaths.paths.append(path) } } try await Task.sleep(for: .milliseconds(100)) - let newFile = tempDir.appendingPathComponent(name) - FileManager.default.createFile(atPath: newFile.path, contents: nil) + let newFile = tempPath.appending(name) + FileManager.default.createFile(atPath: newFile.string, contents: nil) try await Task.sleep(for: .milliseconds(500)) - #expect(!createdURLs.urls.isEmpty, "directory watcher failed to detect new file") - #expect(createdURLs.urls.first!.lastPathComponent == name) + #expect(!createdPaths.paths.isEmpty, "directory watcher failed to detect new file") + #expect(createdPaths.paths.first!.lastComponent?.string == name) } } @Test func testWatchingNonExistingDirectory() async throws { - try await withTempDir { tempDir in + try await withTempDir { tempPath in let uuid = UUID().uuidString - let childDir = tempDir.appendingPathComponent(uuid) + let childPath = tempPath.appending(uuid) - let watcher = DirectoryWatcher(directoryURL: childDir, log: nil) - let createdURLs = CreatedURLs() + let watcher = DirectoryWatcher(directoryPath: childPath, log: nil) + let createdPaths = CreatedPaths() let name = "newFile" - await watcher.startWatching { [createdURLs] urls in - for url in urls where url.lastPathComponent == name { - createdURLs.urls.append(url) + await watcher.startWatching { [createdPaths] paths in + for path in paths where path.lastComponent?.string == name { + createdPaths.paths.append(path) } } try await Task.sleep(for: .milliseconds(100)) - try FileManager.default.createDirectory(at: childDir, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: childPath.string, withIntermediateDirectories: true) try await Task.sleep(for: DirectoryWatcher.watchPeriod) - let newFile = childDir.appendingPathComponent(name) - FileManager.default.createFile(atPath: newFile.path, contents: nil) + let newFile = childPath.appending(name) + FileManager.default.createFile(atPath: newFile.string, contents: nil) try await Task.sleep(for: .milliseconds(500)) - #expect(!createdURLs.urls.isEmpty, "directory watcher failed to detect parent directory") - #expect(createdURLs.urls.first!.lastPathComponent == name) + #expect(!createdPaths.paths.isEmpty, "directory watcher failed to detect parent directory") + #expect(createdPaths.paths.first!.lastComponent?.string == name) } } @Test func testWatchingNonExistingParent() async throws { - try await withTempDir { tempDir in + try await withTempDir { tempPath in let parent = UUID().uuidString let child = UUID().uuidString - let childDir = tempDir.appendingPathComponent(parent).appendingPathComponent(child) + let childPath = tempPath.appending(parent).appending(child) - let watcher = DirectoryWatcher(directoryURL: childDir, log: nil) - let createdURLs = CreatedURLs() + let watcher = DirectoryWatcher(directoryPath: childPath, log: nil) + let createdPaths = CreatedPaths() let name = "newFile" - await watcher.startWatching { urls in - for url in urls where url.lastPathComponent == name { - createdURLs.urls.append(url) + await watcher.startWatching { paths in + for path in paths where path.lastComponent?.string == name { + createdPaths.paths.append(path) } } try await Task.sleep(for: .milliseconds(100)) - try FileManager.default.createDirectory(at: childDir, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: childPath.string, withIntermediateDirectories: true) try await Task.sleep(for: DirectoryWatcher.watchPeriod) - let newFile = childDir.appendingPathComponent(name) - FileManager.default.createFile(atPath: newFile.path, contents: nil) + let newFile = childPath.appending(name) + FileManager.default.createFile(atPath: newFile.string, contents: nil) try await Task.sleep(for: .milliseconds(500)) - #expect(!createdURLs.urls.isEmpty, "directory watcher failed to detect parent directory") - #expect(createdURLs.urls.first!.lastPathComponent == name) + #expect(!createdPaths.paths.isEmpty, "directory watcher failed to detect parent directory") + #expect(createdPaths.paths.first!.lastComponent?.string == name) } } @Test func testWatchingRecreatedDirectory() async throws { - try await withTempDir { tempDir in - let dir = tempDir.appendingPathComponent(UUID().uuidString) - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try await withTempDir { tempPath in + let dirPath = tempPath.appending(UUID().uuidString) + try FileManager.default.createDirectory(atPath: dirPath.string, withIntermediateDirectories: true) - let watcher = DirectoryWatcher(directoryURL: dir, log: nil) - let createdURLs = CreatedURLs() + let watcher = DirectoryWatcher(directoryPath: dirPath, log: nil) + let createdPaths = CreatedPaths() let beforeDelete = "beforeDelete" let afterDelete = "afterDelete" - await watcher.startWatching { [createdURLs] urls in - for url in urls - where url.lastPathComponent == beforeDelete || url.lastPathComponent == afterDelete { - createdURLs.urls.append(url) + await watcher.startWatching { [createdPaths] paths in + for path in paths + where path.lastComponent?.string == beforeDelete || path.lastComponent?.string == afterDelete { + createdPaths.paths.append(path) } } try await Task.sleep(for: .milliseconds(100)) - let file1 = dir.appendingPathComponent(beforeDelete) - FileManager.default.createFile(atPath: file1.path, contents: nil) + let file1 = dirPath.appending(beforeDelete) + FileManager.default.createFile(atPath: file1.string, contents: nil) try await Task.sleep(for: .milliseconds(100)) - try FileManager.default.removeItem(at: dir) + try FileManager.default.removeItem(atPath: dirPath.string) try await Task.sleep(for: .milliseconds(100)) - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: dirPath.string, withIntermediateDirectories: true) try await Task.sleep(for: DirectoryWatcher.watchPeriod) - let file2 = dir.appendingPathComponent(afterDelete) - FileManager.default.createFile(atPath: file2.path, contents: nil) + let file2 = dirPath.appending(afterDelete) + FileManager.default.createFile(atPath: file2.string, contents: nil) try await Task.sleep(for: .milliseconds(500)) - #expect(!createdURLs.urls.isEmpty, "directory watcher failed to detect new file") - #expect(Set(createdURLs.urls.map { $0.lastPathComponent }) == Set([beforeDelete, afterDelete])) + #expect(!createdPaths.paths.isEmpty, "directory watcher failed to detect new file") + #expect( + Set(createdPaths.paths.compactMap { $0.lastComponent?.string }) == Set([beforeDelete, afterDelete])) } }