Ensure bad files get cleared out regardless (#30)

Author: kreeger

Flapjack.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name        = 'Flapjack'
-  s.version     = '0.4.0'
+  s.version     = '0.5.0'
   s.summary     = 'A Swift data persistence API with support for Core Data.'
   s.description = <<-DESC
 Flapjack is an iOS/macOS/tvOS framework with 2 primary goals.

Flapjack/CoreData/CoreDataAccess.swift

@@ -168,7 +168,18 @@ public final class CoreDataAccess: DataAccess {
      - parameter completion: A closure to be called upon completion.
      */
     public func deleteDatabase(rebuild: Bool, completion: @escaping (DataAccessError?) -> Void) {
+        let fileExistsAtPath = storeType.url.map { FileManager.default.fileExists(atPath: $0.path, isDirectory: nil) } ?? false
+
+        // If we've never attached any store, delete any stale file that may be there and add them
+        //   anew with `addDefaultPersistentStores(async:completion:)`.
         guard !persistentStores.isEmpty else {
+            if let storeURL = storeType.url, fileExistsAtPath {
+                do {
+                    try FileManager.default.removeItem(atPath: storeURL.path)
+                } catch let error {
+                    Logger.error("Error destroying persistent store at \(storeURL): \(error)")
+                }
+            }
             if rebuild {
                 addDefaultPersistentStores(async: shouldLoadAsynchronously, completion: completion)
             } else {
@@ -177,8 +188,8 @@ public final class CoreDataAccess: DataAccess {
             return
         }
 
+        // Let listeners know we're about to detach our store from our coordinator, and then do it.
         NotificationCenter.default.post(name: type(of: self).willDestroyMainContextNotification, object: mainContext)
-
         let localPersistentStores = persistentStores
         localPersistentStores.enumerated().forEach { idx, persistentStore in
             do {
@@ -188,12 +199,11 @@ public final class CoreDataAccess: DataAccess {
             }
         }
 
-        if let storeURL = storeType.url {
+        // Now that we've detached the store from the coordinator, 
+        if let storeURL = storeType.url, fileExistsAtPath {
             do {
-                if FileManager.default.fileExists(atPath: storeURL.path, isDirectory: nil) {
-                    try persistentStoreCoordinator.destroyPersistentStore(at: storeURL, ofType: storeType.coreDataType, options: nil)
-                    try FileManager.default.removeItem(atPath: storeURL.path)
-                }
+                try persistentStoreCoordinator.destroyPersistentStore(at: storeURL, ofType: storeType.coreDataType, options: nil)
+                try FileManager.default.removeItem(atPath: storeURL.path)
             } catch let error {
                 Logger.error("Error destroying persistent store at \(storeURL): \(error)")
             }

Tests/CoreData/CoreDataAccessTests.swift

@@ -238,7 +238,8 @@ class CoreDataAccessWithSQLFileTests: XCTestCase {
         XCTAssertTrue(FileManager.default.fileExists(atPath: storeURL.path, isDirectory: nil))
 
         let expect = expectation(description: "completion")
-        dataAccess.deleteDatabase(rebuild: false) { _ in
+        dataAccess.deleteDatabase(rebuild: false) { error in
+            XCTAssertNil(error)
             XCTAssertFalse(FileManager.default.fileExists(atPath: self.storeURL.path, isDirectory: nil))
             expect.fulfill()
         }
@@ -250,7 +251,8 @@ class CoreDataAccessWithSQLFileTests: XCTestCase {
         XCTAssertTrue(FileManager.default.fileExists(atPath: storeURL.path, isDirectory: nil))
 
         let expect = expectation(description: "completion")
-        dataAccess.deleteDatabase(rebuild: true) { _ in
+        dataAccess.deleteDatabase(rebuild: true) { error in
+            XCTAssertNil(error)
             XCTAssertTrue(FileManager.default.fileExists(atPath: self.storeURL.path, isDirectory: nil))
             expect.fulfill()
         }
@@ -259,7 +261,8 @@ class CoreDataAccessWithSQLFileTests: XCTestCase {
 
     func testDeleteDatabaseWithoutPersistentStoresNoRebuild() {
         let expect = expectation(description: "completion")
-        dataAccess.deleteDatabase(rebuild: false) { _ in
+        dataAccess.deleteDatabase(rebuild: false) { error in
+            XCTAssertNil(error)
             XCTAssertFalse(FileManager.default.fileExists(atPath: self.storeURL.path, isDirectory: nil))
             expect.fulfill()
         }
@@ -268,7 +271,34 @@ class CoreDataAccessWithSQLFileTests: XCTestCase {
 
     func testDeleteDatabaseWithoutPersistentStoresWithRebuild() {
         let expect = expectation(description: "completion")
-        dataAccess.deleteDatabase(rebuild: true) { _ in
+        dataAccess.deleteDatabase(rebuild: true) { error in
+            XCTAssertNil(error)
+            XCTAssertTrue(FileManager.default.fileExists(atPath: self.storeURL.path, isDirectory: nil))
+            expect.fulfill()
+        }
+        waitForExpectations(timeout: 1.0) { XCTAssertNil($0) }
+    }
+
+    func testDeleteDatabaseWithoutPersistentStoresJunkSQLNoRebuild() throws {
+        try "junk".write(to: storeURL, atomically: true, encoding: .utf8)
+        XCTAssertTrue(FileManager.default.fileExists(atPath: storeURL.path, isDirectory: nil))
+
+        let expect = expectation(description: "completion")
+        dataAccess.deleteDatabase(rebuild: false) { error in
+            XCTAssertNil(error)
+            XCTAssertFalse(FileManager.default.fileExists(atPath: self.storeURL.path, isDirectory: nil))
+            expect.fulfill()
+        }
+        waitForExpectations(timeout: 1.0) { XCTAssertNil($0) }
+    }
+
+    func testDeleteDatabaseWithoutPersistentStoresJunkSQLWithRebuild() throws {
+        try "junk".write(to: storeURL, atomically: true, encoding: .utf8)
+        XCTAssertTrue(FileManager.default.fileExists(atPath: storeURL.path, isDirectory: nil))
+
+        let expect = expectation(description: "completion")
+        dataAccess.deleteDatabase(rebuild: true) { error in
+            XCTAssertNil(error)
             XCTAssertTrue(FileManager.default.fileExists(atPath: self.storeURL.path, isDirectory: nil))
             expect.fulfill()
         }