From 976b3c53ce3bd07dedb6327be286436bb4c63bbc Mon Sep 17 00:00:00 2001 From: Thomas Mellenthin Date: Mon, 22 Feb 2021 18:53:39 +0100 Subject: [PATCH 1/4] Custom diff: allow to return String? in case there is no diff --- Sources/LoggerMiddleware/LoggerMiddleware.swift | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Sources/LoggerMiddleware/LoggerMiddleware.swift b/Sources/LoggerMiddleware/LoggerMiddleware.swift index 7c56bbd..78eb003 100644 --- a/Sources/LoggerMiddleware/LoggerMiddleware.swift +++ b/Sources/LoggerMiddleware/LoggerMiddleware.swift @@ -67,7 +67,9 @@ public final class LoggerMiddleware: Middleware where M.StateType self.queue.async { let actionMessage = self.actionTransform.transform(action: action, source: dispatcher) self.actionPrinter.log(action: actionMessage) - self.stateDiffPrinter.log(state: self.stateDiffTransform.transform(oldState: stateBefore, newState: stateAfter)) + if let diffString = self.stateDiffTransform.transform(oldState: stateBefore, newState: stateAfter) { + self.stateDiffPrinter.log(state: diffString) + } } } } @@ -123,9 +125,9 @@ extension LoggerMiddleware { public enum StateDiffTransform { case diff(linesOfContext: Int = 2, prefixLines: String = "🏛 ") case newStateOnly - case custom((StateType?, StateType) -> String) + case custom((StateType?, StateType) -> String?) - func transform(oldState: StateType?, newState: StateType) -> String { + func transform(oldState: StateType?, newState: StateType) -> String? { switch self { case let .diff(linesOfContext, prefixLines): let stateBefore = dumpToString(oldState) From abe3c717437da21e869aee8768c8a73fb9626b01 Mon Sep 17 00:00:00 2001 From: Thomas Mellenthin Date: Mon, 22 Feb 2021 18:55:52 +0100 Subject: [PATCH 2/4] Recursive diff: attempt to recursively diff any State object --- .../LoggerMiddleware/LoggerMiddleware.swift | 76 +++++++++++++++++++ .../LoggerMiddlewareTests.swift | 54 ++++++++++++- 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/Sources/LoggerMiddleware/LoggerMiddleware.swift b/Sources/LoggerMiddleware/LoggerMiddleware.swift index 78eb003..3b17f7d 100644 --- a/Sources/LoggerMiddleware/LoggerMiddleware.swift +++ b/Sources/LoggerMiddleware/LoggerMiddleware.swift @@ -125,6 +125,7 @@ extension LoggerMiddleware { public enum StateDiffTransform { case diff(linesOfContext: Int = 2, prefixLines: String = "🏛 ") case newStateOnly + case recursive(prefixLines: String = "🏛 ", stateName: String) case custom((StateType?, StateType) -> String?) func transform(oldState: StateType?, newState: StateType) -> String? { @@ -136,11 +137,86 @@ extension LoggerMiddleware { ?? "\(prefixLines) No state mutation" case .newStateOnly: return dumpToString(newState) + case let .recursive(prefixLines, stateName): + return recursiveDiff(prefixLines: prefixLines, stateName: stateName, before: oldState, after: newState) case let .custom(closure): return closure(oldState, newState) } } } + + public static func recursiveDiff(prefixLines: String, stateName: String, before: StateType?, after: StateType) -> String? { + // cuts the redundant newline character from the output + diff(prefix: prefixLines, name: stateName, lhs: before, rhs: after)?.trimmingCharacters(in: .whitespacesAndNewlines) + } + + private static func diff(prefix: String, name: String, level: Int = 0, lhs: A, rhs: A) -> String? { + let leftMirror = Mirror(reflecting: lhs) + let rightMirror = Mirror(reflecting: rhs) + + // special handling for Dictionaries + if let left = lhs as? Dictionary, let right = rhs as? Dictionary { + + let leftSorted = left.sorted { a, b in "\(a.key)" < "\(b.key)" } + let rightSorted = right.sorted { a, b in "\(a.key)" < "\(b.key)" } + + let leftPrintable = leftSorted.map { key, value in "\(key): \(value)" }.joined(separator: ", ") + let rightPrintable = rightSorted.map { key, value in "\(key): \(value)" }.joined(separator: ", ") + + // .difference(from:) gives unpleasant results + if leftPrintable == rightPrintable { + return nil + } + + return "\(prefix).\(name): 📦 [\(leftPrintable)] → [\(rightPrintable)]" + } + + // special handling for sets as well: order the contents, compare as strings + if let left = lhs as? Set, let right = rhs as? Set { + let leftSorted = left.map { "\($0)" }.sorted { a, b in a < b } + let rightSorted = right.map { "\($0)" }.sorted { a, b in a < b } + + let leftPrintable = leftSorted.joined(separator: ", ") + let rightPrintable = rightSorted.joined(separator: ", ") + + // .difference(from:) gives unpleasant results + if leftPrintable == rightPrintable { + return nil + } + return "\(prefix).\(name): 📦 <\(leftPrintable)> → <\(rightPrintable)>" + } + + // if there are no children, compare lhs and rhs directly + if 0 == leftMirror.children.count { + if "\(lhs)" == "\(rhs)" { + return nil + } else { + return "\(prefix).\(name): \(lhs) → \(rhs)" + } + } + + // there are children -> diff the object graph recursively + let strings: [String] = leftMirror.children.map({ leftChild in + guard let rightChild = rightMirror.children.first(where: { $0.label == leftChild.label }) else { + return nil + } + + let leftValue = leftChild.value + let rightValue = rightChild.value + + let dot = (level > 0) ? "." : " " + return Self.diff(prefix: "\(prefix)\(dot)\(name)", + name: leftChild.label ?? "", + level: level + 1, + lhs: leftValue, + rhs: rightValue) + }).compactMap { $0 } + + if strings.count > 0 { + return strings.joined(separator: "\n") + } + return nil + } } // MARK: - Action diff --git a/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift b/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift index 6993d6e..e18e8cb 100644 --- a/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift +++ b/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift @@ -1,11 +1,61 @@ import XCTest +import SwiftRex @testable import LoggerMiddleware +struct TestState: Equatable { + public let a: Substate + public let b: [Int] + public let c: String +} + +struct Substate: Equatable { + public let x: Set + public let y: [String: Int] + public let z: Bool +} + +struct TestMiddleware: Middleware { + func receiveContext(getState: @escaping GetState, output: AnyActionHandler) { + } + + func handle(action: Int, from dispatcher: ActionSource, afterReducer: inout AfterReducer) { + } + + typealias InputActionType = Int + typealias OutputActionType = Int + typealias StateType = TestState +} + final class LoggerMiddlewareTests: XCTestCase { - func testExample() { + + func testStateDiff() { + // given + let beforeState: LoggerMiddleware.StateType = TestState(a: Substate(x: ["SetB", "SetA"], + y: ["one": 1, "eleven": 11], + z: true), + b: [0, 1], + c: "Foo") + let afterState: LoggerMiddleware.StateType = TestState(a: Substate(x: ["SetB", "SetC"], + y: ["one": 1, "twelve": 12], + z: false), + b: [0], + c: "Bar") + + // when + let result: String? = LoggerMiddleware.recursiveDiff(prefixLines: "🏛", stateName: "TestState", before: beforeState, after: afterState) + + // then + let expected = """ + 🏛 TestState.some.a.x: 📦 → + 🏛 TestState.some.a.y: 📦 [eleven: 11, one: 1] → [one: 1, twelve: 12] + 🏛 TestState.some.a.z: true → false + 🏛 TestState.some.b: 📦 [0, 1] → [0] + 🏛 TestState.some.c: Foo → Bar + """ + XCTAssertEqual(result, expected) } static var allTests = [ - ("testExample", testExample), + ("testStateDiff", testStateDiff), ] } From 29009dca3495bd4eb4cac47d953c58d508b2efcc Mon Sep 17 00:00:00 2001 From: Thomas Mellenthin Date: Mon, 22 Feb 2021 22:06:13 +0100 Subject: [PATCH 3/4] RecursiveDiff: better nil-Handling, deal with components of collections (#) --- .../LoggerMiddleware/LoggerMiddleware.swift | 41 +++++++++++-------- .../LoggerMiddlewareTests.swift | 32 ++++++++++----- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/Sources/LoggerMiddleware/LoggerMiddleware.swift b/Sources/LoggerMiddleware/LoggerMiddleware.swift index 3b17f7d..1424757 100644 --- a/Sources/LoggerMiddleware/LoggerMiddleware.swift +++ b/Sources/LoggerMiddleware/LoggerMiddleware.swift @@ -150,9 +150,23 @@ extension LoggerMiddleware { diff(prefix: prefixLines, name: stateName, lhs: before, rhs: after)?.trimmingCharacters(in: .whitespacesAndNewlines) } - private static func diff(prefix: String, name: String, level: Int = 0, lhs: A, rhs: A) -> String? { - let leftMirror = Mirror(reflecting: lhs) - let rightMirror = Mirror(reflecting: rhs) + private static func diff(prefix: String, name: String, level: Int = 0, lhs: A?, rhs: A?) -> String? { + + guard let rightHandSide = rhs, let leftHandSide = lhs else { + if let rightHandSide = rhs { + return "\(prefix).\(name): nil → \(rightHandSide)" + } + + if let leftHandSide = lhs { + return "\(prefix).\(name): \(leftHandSide) → nil" + } + + // nil == lhs == rhs + return nil + } + + let leftMirror = Mirror(reflecting: leftHandSide) + let rightMirror = Mirror(reflecting: rightHandSide) // special handling for Dictionaries if let left = lhs as? Dictionary, let right = rhs as? Dictionary { @@ -188,28 +202,21 @@ extension LoggerMiddleware { // if there are no children, compare lhs and rhs directly if 0 == leftMirror.children.count { - if "\(lhs)" == "\(rhs)" { + if "\(leftHandSide)" == "\(rightHandSide)" { return nil } else { - return "\(prefix).\(name): \(lhs) → \(rhs)" + return "\(prefix).\(name): \(leftHandSide) → \(rightHandSide)" } } // there are children -> diff the object graph recursively let strings: [String] = leftMirror.children.map({ leftChild in - guard let rightChild = rightMirror.children.first(where: { $0.label == leftChild.label }) else { - return nil - } - - let leftValue = leftChild.value - let rightValue = rightChild.value - - let dot = (level > 0) ? "." : " " - return Self.diff(prefix: "\(prefix)\(dot)\(name)", - name: leftChild.label ?? "", + let toDotOrNotToDot = (level > 0) ? "." : " " + return Self.diff(prefix: "\(prefix)\(toDotOrNotToDot)\(name)", + name: leftChild.label ?? "#", // label might be missing for items in collections level: level + 1, - lhs: leftValue, - rhs: rightValue) + lhs: leftChild.value, + rhs: rightMirror.children.first(where: { $0.label == leftChild.label })?.value) }).compactMap { $0 } if strings.count > 0 { diff --git a/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift b/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift index e18e8cb..64b3016 100644 --- a/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift +++ b/Tests/LoggerMiddlewareTests/LoggerMiddlewareTests.swift @@ -6,11 +6,14 @@ struct TestState: Equatable { public let a: Substate public let b: [Int] public let c: String + public let d: String? + public let e: String? } struct Substate: Equatable { public let x: Set - public let y: [String: Int] + public let y1: [String: Int] + public let y2: [String: Int?] public let z: Bool } @@ -31,26 +34,35 @@ final class LoggerMiddlewareTests: XCTestCase { func testStateDiff() { // given let beforeState: LoggerMiddleware.StateType = TestState(a: Substate(x: ["SetB", "SetA"], - y: ["one": 1, "eleven": 11], + y1: ["one": 1, "eleven": 11], + y2: ["one": 1, "eleven": 11, "zapp": 42], z: true), b: [0, 1], - c: "Foo") + c: "Foo", + d: "✨", + e: nil) let afterState: LoggerMiddleware.StateType = TestState(a: Substate(x: ["SetB", "SetC"], - y: ["one": 1, "twelve": 12], + y1: ["one": 1, "twelve": 12], + y2: ["one": 1, "twelve": 12, "zapp": nil], z: false), b: [0], - c: "Bar") + c: "Bar", + d: nil, + e: "🥚") // when let result: String? = LoggerMiddleware.recursiveDiff(prefixLines: "🏛", stateName: "TestState", before: beforeState, after: afterState) // then let expected = """ - 🏛 TestState.some.a.x: 📦 → - 🏛 TestState.some.a.y: 📦 [eleven: 11, one: 1] → [one: 1, twelve: 12] - 🏛 TestState.some.a.z: true → false - 🏛 TestState.some.b: 📦 [0, 1] → [0] - 🏛 TestState.some.c: Foo → Bar + 🏛 TestState.a.x: 📦 → + 🏛 TestState.a.y1: 📦 [eleven: 11, one: 1] → [one: 1, twelve: 12] + 🏛 TestState.a.y2: 📦 [eleven: Optional(11), one: Optional(1), zapp: Optional(42)] → [one: Optional(1), twelve: Optional(12), zapp: nil] + 🏛 TestState.a.z: true → false + 🏛 TestState.b.#: 1 → 0 + 🏛 TestState.c: Foo → Bar + 🏛 TestState.d.some: ✨ → nil + 🏛 TestState.e: nil → Optional("🥚") """ XCTAssertEqual(result, expected) } From 21e88a0d8b947c7d70837cf0aca26669ca23f81f Mon Sep 17 00:00:00 2001 From: Thomas Mellenthin Date: Tue, 23 Feb 2021 09:26:16 +0100 Subject: [PATCH 4/4] Recursive diff: cleanup comments, renaming, reordering --- Sources/LoggerMiddleware/LoggerMiddleware.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/LoggerMiddleware/LoggerMiddleware.swift b/Sources/LoggerMiddleware/LoggerMiddleware.swift index 1424757..c289b9d 100644 --- a/Sources/LoggerMiddleware/LoggerMiddleware.swift +++ b/Sources/LoggerMiddleware/LoggerMiddleware.swift @@ -165,11 +165,8 @@ extension LoggerMiddleware { return nil } - let leftMirror = Mirror(reflecting: leftHandSide) - let rightMirror = Mirror(reflecting: rightHandSide) - - // special handling for Dictionaries - if let left = lhs as? Dictionary, let right = rhs as? Dictionary { + // special handling for Dictionaries: stringify and order the keys before comparing + if let left = leftHandSide as? Dictionary, let right = rightHandSide as? Dictionary { let leftSorted = left.sorted { a, b in "\(a.key)" < "\(b.key)" } let rightSorted = right.sorted { a, b in "\(a.key)" < "\(b.key)" } @@ -186,7 +183,7 @@ extension LoggerMiddleware { } // special handling for sets as well: order the contents, compare as strings - if let left = lhs as? Set, let right = rhs as? Set { + if let left = leftHandSide as? Set, let right = rightHandSide as? Set { let leftSorted = left.map { "\($0)" }.sorted { a, b in a < b } let rightSorted = right.map { "\($0)" }.sorted { a, b in a < b } @@ -200,7 +197,10 @@ extension LoggerMiddleware { return "\(prefix).\(name): 📦 <\(leftPrintable)> → <\(rightPrintable)>" } - // if there are no children, compare lhs and rhs directly + let leftMirror = Mirror(reflecting: leftHandSide) + let rightMirror = Mirror(reflecting: rightHandSide) + + // if there are no children, compare leftHandSide and rightHandSide directly if 0 == leftMirror.children.count { if "\(leftHandSide)" == "\(rightHandSide)" { return nil @@ -213,7 +213,7 @@ extension LoggerMiddleware { let strings: [String] = leftMirror.children.map({ leftChild in let toDotOrNotToDot = (level > 0) ? "." : " " return Self.diff(prefix: "\(prefix)\(toDotOrNotToDot)\(name)", - name: leftChild.label ?? "#", // label might be missing for items in collections + name: leftChild.label ?? "#", // label might be missing for items in collections, # represents a collection element level: level + 1, lhs: leftChild.value, rhs: rightMirror.children.first(where: { $0.label == leftChild.label })?.value)