fix: Rules variable are incorrectly updated after cancelling edit on …

…iOS (#509)

The iOS app has modal rule editing which takes a copy of the rule models
on edit, only updating the rule set on save. Unfortunately, we weren't
performing a full deep copy of the rule model, meaning that changes were
being applied and written to disk, even on cancel.

core/Sources/FileawayCore/Configuration/VariableModel.swift

@@ -47,7 +47,7 @@ public class VariableModel: ObservableObject, Identifiable, Codable, Hashable {
         return lhs.id == rhs.id
     }
 
-    public var id = UUID()
+    public let id: UUID
 
     @Published public var name: String
     @Published public var type: VariableType
@@ -56,17 +56,26 @@ public class VariableModel: ObservableObject, Identifiable, Codable, Hashable {
         return HashRainbow.colorForString(name, colors: HashRainbow.NeonColors)
     }
 
-    public init(name: String, type: VariableType) {
+    public init(id: UUID = UUID(), name: String, type: VariableType) {
+        self.id = id
         self.name = name
         self.type = type
     }
 
+    public convenience init(_ variableModel: VariableModel) {
+        self.init(id: variableModel.id,
+                  name: variableModel.name,
+                  type: variableModel.type)
+    }
+
     required public init(from decoder: Decoder) throws {
         let container = try decoder.container(keyedBy: CodingKeys.self)
 
         let name = try container.decode(String.self, forKey: .name)
         let type = try container.decode(RawType.self, forKey: .type)
 
+        id = UUID()
+
         switch type {
         case .string:
             self.name = name

core/Sources/FileawayCore/Model/RuleModel.swift

@@ -63,9 +63,9 @@ public class RuleModel: ObservableObject, Identifiable, CustomStringConvertible,
         }
     }
 
-    public init(id: UUID, rootUrl: URL, name: String, variables: [VariableModel], destination: [ComponentModel]) {
+    public init(id: UUID, archiveURL: URL, name: String, variables: [VariableModel], destination: [ComponentModel]) {
         self.id = id
-        self.archiveURL = rootUrl
+        self.archiveURL = archiveURL
         self.name = name
         self.variables = variables
         self.destination = destination
@@ -81,23 +81,23 @@ public class RuleModel: ObservableObject, Identifiable, CustomStringConvertible,
 
     public convenience init(_ ruleModel: RuleModel) {
         self.init(id: ruleModel.id,
-                  rootUrl: ruleModel.archiveURL,
+                  archiveURL: ruleModel.archiveURL,
                   name: String(ruleModel.name),
-                  variables: ruleModel.variables,
+                  variables: ruleModel.variables.map { VariableModel($0) },
                   destination: ruleModel.destination.map { ComponentModel($0, variable: nil) })
     }
 
     public convenience init(id: UUID, ruleModel: RuleModel) {
         self.init(id: id,
-                  rootUrl: ruleModel.archiveURL,
+                  archiveURL: ruleModel.archiveURL,
                   name: String(ruleModel.name),
                   variables: ruleModel.variables,
                   destination: ruleModel.destination.map { ComponentModel($0, variable: nil) })
     }
 
     public convenience init(rootUrl: URL, rule: Rule) {
         self.init(id: rule.id,
-                  rootUrl: rootUrl,
+                  archiveURL: rootUrl,
                   name: rule.name,
                   variables: rule.variables,
                   destination: rule.destination.map { component in
@@ -187,15 +187,17 @@ public class RuleModel: ObservableObject, Identifiable, CustomStringConvertible,
         return names.count == variables.count && !name.isEmpty
     }
 
-    public func createVariable() {
+    public func createVariable() -> VariableModel {
         let names = Set(variables.map { $0.name })
         var index = 1
         var name = ""
         repeat {
             name = "Variable \(index)"
             index = index + 1
         } while names.contains(name)
-        self.variables.append(VariableModel(name: name, type: .string))
+        let variable = VariableModel(name: name, type: .string)
+        self.variables.append(variable)
+        return variable
     }
 
     public static func == (lhs: RuleModel, rhs: RuleModel) -> Bool {

core/Sources/FileawayCore/Model/RulesModel.swift

@@ -94,7 +94,7 @@ public class RulesModel: ObservableObject {
     public func new() throws -> RuleModel {
         let name = uniqueRuleName(preferredName: "Rule")
         let rule = RuleModel(id: UUID(),
-                             rootUrl: archiveURL,
+                             archiveURL: archiveURL,
                              name: name,
                              variables: [VariableModel(name: "Date", type: .date(hasDay: true))],
                              destination: [

core/Tests/FileawayCoreTests/Extensions/XCTestCase.swift

@@ -0,0 +1,48 @@
+// Copyright (c) 2018-2022 InSeven Limited
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+import XCTest
+
+extension XCTestCase {
+
+    func createTemporaryDirectory() throws -> URL {
+
+        let fileManager = FileManager.default
+        let directoryURL = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
+        try fileManager.createDirectory(at: directoryURL, withIntermediateDirectories: true)
+
+        var isDirectory: ObjCBool = false
+        XCTAssertTrue(fileManager.fileExists(atPath: directoryURL.path, isDirectory: &isDirectory))
+        XCTAssert(isDirectory.boolValue)
+
+        addTeardownBlock {
+            do {
+                let fileManager = FileManager.default
+                try fileManager.removeItem(at: directoryURL)
+                XCTAssertFalse(fileManager.fileExists(atPath: directoryURL.path))
+            } catch {
+                XCTFail("Failed to delete temporary directory with error \(error).")
+            }
+        }
+
+        return directoryURL
+    }
+
+}

core/Tests/FileawayCoreTests/RuleModelTests.swift

@@ -0,0 +1,71 @@
+// Copyright (c) 2018-2022 InSeven Limited
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+import XCTest
+
+@testable import FileawayCore
+
+class RuleModelTests: XCTestCase {
+
+    func testDeepCopy() throws {
+
+        let archiveURL = try createTemporaryDirectory()
+        let rule1 = RuleModel(id: UUID(),
+                              archiveURL: archiveURL,
+                              name: "Rule",
+                              variables: [],
+                              destination: [])
+        _ = rule1.createVariable()
+        XCTAssertEqual(rule1.variables.count, 1)
+        XCTAssertEqual(rule1.variables[0].name, "Variable 1")
+        XCTAssertEqual(rule1.variables[0].type, .string)
+
+        // Ensure that we deep copy a rule so that modifications to the new model do not affect any component
+        // of the original.
+
+        // Variables.
+
+        let rule2 = RuleModel(rule1)
+
+        // Duplicate variables should have the same identifiers and should be identical on first copy.
+        XCTAssertEqual(rule1.variables[0].id, rule2.variables[0].id)
+        XCTAssertEqual(rule1.variables, rule2.variables)
+
+        // Create a new variable and ensure the variable lists now differ.
+        _ = rule2.createVariable()
+        XCTAssertEqual(rule1.variables.count, 1)
+        XCTAssertEqual(rule2.variables.count, 2)
+
+        rule2.variables[0].name = "Description"
+        XCTAssertEqual(rule1.variables[0].name, "Variable 1")
+        XCTAssertNotEqual(rule1.variables[0].name, rule2.variables[0].name)
+
+        rule2.variables[0].type = .date(hasDay: true)
+        XCTAssertEqual(rule1.variables[0].type, .string)
+        XCTAssertNotEqual(rule1.variables[0].type, rule2.variables[0].type)
+
+        // Destination.
+
+        // TODO: Test RuleModel.destination deep copy behaviour #510
+        //       https://github.com/inseven/fileaway/issues/510
+
+    }
+
+}

core/Tests/FileawayCoreTests/RulesModelTests.swift

@@ -26,37 +26,14 @@ import XCTest
 //       https://github.com/inseven/fileaway/issues/500
 class RulesModelTests: XCTestCase {
 
-    func temporaryDirectoryURL() throws -> URL {
-
-        let fileManager = FileManager.default
-        let directoryURL = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
-        try fileManager.createDirectory(at: directoryURL, withIntermediateDirectories: true)
-
-        var isDirectory: ObjCBool = false
-        XCTAssertTrue(fileManager.fileExists(atPath: directoryURL.path, isDirectory: &isDirectory))
-        XCTAssert(isDirectory.boolValue)
-
-        addTeardownBlock {
-            do {
-                let fileManager = FileManager.default
-                try fileManager.removeItem(at: directoryURL)
-                XCTAssertFalse(fileManager.fileExists(atPath: directoryURL.path))
-            } catch {
-                XCTFail("Failed to delete temporary directory with error \(error).")
-            }
-        }
-
-        return directoryURL
-    }
-
     func emptyModel() throws -> RulesModel {
-        let archiveURL = try temporaryDirectoryURL()
+        let archiveURL = try createTemporaryDirectory()
         let rules = RulesModel(archiveURL: archiveURL)
         return rules
     }
 
     func testNewRule() throws {
-        let archiveURL = try temporaryDirectoryURL()
+        let archiveURL = try createTemporaryDirectory()
 
         let rules = RulesModel(archiveURL: archiveURL)
         XCTAssert(rules.ruleModels.isEmpty)

ios/Fileaway/Views/Editor/RuleView.swift

@@ -65,7 +65,7 @@ struct RuleView: View {
                     if self.editMode == .active {
                         Button {
                             withAnimation {
-                                self.editingRuleModel.createVariable()
+                                _ = self.editingRuleModel.createVariable()
                             }
                         } label: {
                             Text("New Variable...")

macos/Fileaway/Views/Settings/VariablesTable.swift

@@ -52,7 +52,7 @@ struct VariablesTable: View {
                 VStack {
                     VStack {
                         Button {
-                            ruleModel.createVariable()
+                            _ = ruleModel.createVariable()
                         } label: {
                             Text("Add")
                                 .frame(width: 80)