Make file writes atomic, and don't decode empty file. (#129)

* Don't decode empty file.

* wip

* wip

* 15.2 fix

* 15.2 fix

* bring back atomic

* atomic working again

* clean upo

* new lines

* Update Sources/Sharing/SharedKeys/FileStorageKey.swift

Co-authored-by: Pyry Jahkola <[email protected]>

* Increase test iterations.

* Get rid of stub bytes.

---------

Co-authored-by: Pyry Jahkola <[email protected]>

Author: mbrandonw

Sources/Sharing/SharedKeys/FileStorageKey.swift

@@ -107,7 +107,7 @@
     public func load(context _: LoadContext<Value>, continuation: LoadContinuation<Value>) {
       guard
         let data = try? storage.load(url),
-        data != stubBytes
+        !data.isEmpty
       else {
         continuation.resumeReturningInitialValue()
         return
@@ -127,40 +127,65 @@
             // NB: Make sure there is a file to create a source for.
             if !storage.fileExists(url) {
               try storage.createDirectory(url.deletingLastPathComponent(), true)
-              try storage.save(stubBytes, url)
+              try storage.save(Data(), url)
             }
-            let writeCancellable = try storage.fileSystemSource(url, [.write]) { [weak self] in
+            let externalCancellable = try storage.fileSystemSource(url, [.write, .rename]) {
+              [weak self] in
               guard let self else { return }
-              state.withValue { state in
-                let modificationDate =
-                  (try? self.storage.attributesOfItemAtPath(self.url.path)[.modificationDate]
-                    as? Date)
-                  ?? Date.distantPast
+              let fileExists = self.storage.fileExists(self.url)
+              defer {
+                if !fileExists {
+                  setUpSources()
+                }
+              }
+              if state.withValue({ $0.workItem == nil }) {
+                if fileExists {
+                  subscriber.yield(with: Result { try self.decode(self.storage.load(self.url)) })
+                } else {
+                  subscriber.yieldReturningInitialValue()
+                }
+              }
+            }
+            let internalCancellable = try storage.fileSystemSource(url, [.delete]) {
+              [weak self] in
+              guard let self else { return }
+              let fileExists = self.storage.fileExists(self.url)
+              defer {
+                if !fileExists {
+                  setUpSources()
+                }
+              }
+              let modificationDate =
+                fileExists
+                ? (try? self.storage.attributesOfItemAtPath(self.url.path)[.modificationDate]
+                  as? Date)
+                : nil
+              let shouldYield = state.withValue { state in
+                guard fileExists
+                else {
+                  state.cancelWorkItem()
+                  return true
+                }
+                let modificationDate = modificationDate ?? .distantPast
                 guard
                   !state.modificationDates.contains(modificationDate)
                 else {
                   state.modificationDates.removeAll(where: { $0 <= modificationDate })
-                  return
+                  return false
                 }
-
-                guard state.workItem == nil
-                else { return }
-
-                subscriber.yield(with: Result { try self.decode(self.storage.load(self.url)) })
+                return state.workItem == nil
               }
-            }
-            let deleteCancellable = try storage.fileSystemSource(url, [.delete, .rename]) {
-              [weak self] in
-              guard let self else { return }
-              state.withValue { state in
-                state.cancelWorkItem()
+              if shouldYield {
+                if fileExists {
+                  subscriber.yield(with: Result { try self.decode(self.storage.load(self.url)) })
+                } else {
+                  subscriber.yieldReturningInitialValue()
+                }
               }
-              subscriber.yield(with: .success(try? decode(storage.load(url))))
-              setUpSources()
             }
             $0 = SharedSubscription {
-              writeCancellable.cancel()
-              deleteCancellable.cancel()
+              externalCancellable.cancel()
+              internalCancellable.cancel()
             }
           } catch {
             subscriber.yield(throwing: error)
@@ -353,8 +378,12 @@
           close(source.handle)
         }
       },
-      load: { try Data(contentsOf: $0) },
-      save: { try $0.write(to: $1) }
+      load: { url in
+        try Data(contentsOf: url)
+      },
+      save: { data, url in
+        try data.write(to: url, options: .atomic)
+      }
     )
 
     /// File storage that emulates a file system without actually writing anything to disk.
@@ -413,6 +442,4 @@
     #endif
     return encoder
   }()
-
-  private let stubBytes = Data("co.pointfree.Sharing.FileStorage.stub".utf8)
 #endif

Tests/SharingTests/FileStorageTests.swift

@@ -17,7 +17,7 @@
         @Shared(.fileStorage(.fileURL)) var users = [User]()
         #expect($users.loadError == nil)
         expectNoDifference(
-          fileSystem.value, [.fileURL: Data("co.pointfree.Sharing.FileStorage.stub".utf8)]
+          fileSystem.value, [.fileURL: Data()]
         )
         $users.withLock { $0.append(.blob) }
         try expectNoDifference(fileSystem.value.users(for: .fileURL), [.blob])
@@ -31,7 +31,7 @@
         @Shared(.utf8String) var string = ""
         #expect($string.loadError == nil)
         expectNoDifference(
-          fileSystem.value, [.utf8StringURL: Data("co.pointfree.Sharing.FileStorage.stub".utf8)]
+          fileSystem.value, [.utf8StringURL: Data()]
         )
         $string.withLock { $0 = "hello" }
         expectNoDifference(
@@ -269,6 +269,24 @@
         }
       }
 
+      @Test func moveFileThenWrite() async throws {
+        try await withMainSerialExecutor {
+          try JSONEncoder().encode([User.blob]).write(to: .fileURL)
+
+          @Shared(.fileStorage(.fileURL)) var users = [User]()
+          await Task.yield()
+          expectNoDifference(users, [.blob])
+
+          try FileManager.default.moveItem(at: .fileURL, to: .anotherFileURL)
+          try await Task.sleep(nanoseconds: 100_000_000)
+          expectNoDifference(users, [])
+
+          try JSONEncoder().encode([User.blobEsq]).write(to: .fileURL)
+          try await Task.sleep(nanoseconds: 1_000_000_000)
+          expectNoDifference(users, [.blobEsq])
+        }
+      }
+
       @Test func testDeleteFileThenWriteToFile() async throws {
         try await withMainSerialExecutor {
           try JSONEncoder().encode([User.blob]).write(to: .fileURL)
@@ -299,7 +317,7 @@
           try await Task.sleep(nanoseconds: 1_200_000_000)
           expectNoDifference(users, [.blob])
           #expect(
-            try Data(contentsOf: .fileURL) == Data("co.pointfree.Sharing.FileStorage.stub".utf8)
+            try Data(contentsOf: .fileURL) == Data()
           )
         }
       }
@@ -356,14 +374,16 @@
       @MainActor
       @Test func multipleMutations() async throws {
         @Shared(.counts) var counts
-        for m in 1...1000 {
-          for n in 1...10 {
+        let iterations = 1_000
+        let buckets = 10
+        for m in 1...iterations {
+          for n in 1...buckets {
             $counts.withLock {
               $0[n, default: 0] += 1
             }
           }
           expectNoDifference(
-            Dictionary((1...10).map { n in (n, m) }, uniquingKeysWith: { $1 }),
+            Dictionary((1...buckets).map { n in (n, m) }, uniquingKeysWith: { $1 }),
             counts
           )
           try await Task.sleep(nanoseconds: 1_000_000)
@@ -386,14 +406,41 @@
 
         #expect(counts[0] == 10_000)
       }
+
+      @Test func emptyData() throws {
+        try? FileManager.default.removeItem(at: .fileURL)
+        try Data().write(to: .fileURL)
+        @Shared(.fileStorage(.fileURL)) var count = 0
+        #expect(count == 0)
+      }
+
+      @Test func corruptData() async throws {
+        try? FileManager.default.removeItem(at: .fileURL)
+        try Data("corrupted".utf8).write(to: .fileURL)
+        @Shared(value: 0) var count: Int
+        withKnownIssue {
+          $count = Shared(wrappedValue: 0, .fileStorage(.fileURL))
+        } matching: {
+          $0.description.hasPrefix("""
+            Caught error: dataCorrupted(Swift.DecodingError.Context(codingPath: [], \
+            debugDescription: "The given data was not valid JSON.", underlyingError: \
+            Optional(Error Domain=NSCocoaErrorDomain Code=3840 "Unexpected character
+            """)
+        }
+        #expect(count == 0)
+        $count.withLock { $0 = 1 }
+        try await Task.sleep(for: .seconds(0.01))
+        #expect(count == 1)
+        #expect(try String(decoding: Data(contentsOf: .fileURL), as: UTF8.self) == "1")
+      }
     }
   }
 
   extension [URL: Data] {
     fileprivate func users(for url: URL) throws -> [User]? {
       guard
         let data = self[url],
-        data != Data("co.pointfree.Sharing.FileStorage.stub".utf8)
+        !data.isEmpty
       else { return nil }
       return try JSONDecoder().decode([User].self, from: data)
     }