From 9b9d85bea1e3fc3c4713ff130fcfae70102b1a72 Mon Sep 17 00:00:00 2001 From: pjechris Date: Mon, 23 Oct 2023 10:21:29 +0200 Subject: [PATCH] fix(alias): Fix alias not enqueued when one object in collection changes (#60) --- .../CohesionKit/Identity/IdentityStore.swift | 15 ++----- .../Observer/ObserverRegistry.swift | 2 +- .../CohesionKit/Storage/AliasContainer.swift | 28 ++++++++++++ .../CohesionKit/Storage/AliasStorage.swift | 4 +- Tests/CohesionKitTests/IdentityMapTests.swift | 44 ++++++++++++++++++- 5 files changed, 77 insertions(+), 16 deletions(-) diff --git a/Sources/CohesionKit/Identity/IdentityStore.swift b/Sources/CohesionKit/Identity/IdentityStore.swift index a1772cf..7147478 100644 --- a/Sources/CohesionKit/Identity/IdentityStore.swift +++ b/Sources/CohesionKit/Identity/IdentityStore.swift @@ -133,7 +133,7 @@ public class IdentityMap { /// - Parameter named: the alias to look for public func find(named: AliasKey) -> EntityObserver { identityQueue.sync { - let node = refAliases[safe: named] + let node = refAliases[safe: named, onChange: registry.enqueueChange(for:)] return EntityObserver(alias: node, registry: registry) } } @@ -142,7 +142,7 @@ public class IdentityMap { /// - Returns: an observer returning the alias value. Note that the value will be an Array public func find(named: AliasKey) -> EntityObserver { identityQueue.sync { - let node = refAliases[safe: named] + let node = refAliases[safe: named, onChange: registry.enqueueChange(for:)] return EntityObserver(alias: node, registry: registry) } } @@ -195,12 +195,11 @@ public class IdentityMap { } private func storeAlias(content: T, key: AliasKey, modifiedAt: Stamp?) { - let aliasNode = refAliases[safe: key] + let aliasNode = refAliases[safe: key, onChange: registry.enqueueChange(for:)] let aliasContainer = AliasContainer(key: key, content: content) _ = nodeStore(in: aliasNode, entity: aliasContainer, modifiedAt: modifiedAt) - registry.enqueueChange(for: aliasNode) logger?.didRegisterAlias(key) } @@ -274,8 +273,6 @@ extension IdentityMap { update(&content) - _ = nodeStore(entity: content, modifiedAt: modifiedAt) - storeAlias(content: content, key: named, modifiedAt: modifiedAt) return true @@ -295,8 +292,6 @@ extension IdentityMap { update(&content) - _ = nodeStore(entity: content, modifiedAt: modifiedAt) - storeAlias(content: content, key: named, modifiedAt: modifiedAt) return true @@ -317,8 +312,6 @@ extension IdentityMap { update(&content) - _ = content.map { nodeStore(entity: $0, modifiedAt: modifiedAt) } - storeAlias(content: content, key: named, modifiedAt: modifiedAt) return true @@ -339,8 +332,6 @@ extension IdentityMap { update(&content) - _ = content.map { nodeStore(entity: $0, modifiedAt: modifiedAt) } - storeAlias(content: content, key: named, modifiedAt: modifiedAt) return true diff --git a/Sources/CohesionKit/Observer/ObserverRegistry.swift b/Sources/CohesionKit/Observer/ObserverRegistry.swift index 02757f2..dcbc53a 100644 --- a/Sources/CohesionKit/Observer/ObserverRegistry.swift +++ b/Sources/CohesionKit/Observer/ObserverRegistry.swift @@ -135,4 +135,4 @@ extension ObserverRegistry { hasher.combine(ObjectIdentifier(self)) } } -} \ No newline at end of file +} diff --git a/Sources/CohesionKit/Storage/AliasContainer.swift b/Sources/CohesionKit/Storage/AliasContainer.swift index 9761503..37e0585 100644 --- a/Sources/CohesionKit/Storage/AliasContainer.swift +++ b/Sources/CohesionKit/Storage/AliasContainer.swift @@ -10,10 +10,18 @@ struct AliasContainer: Identifiable, Aggregate { extension AliasContainer { var nestedEntitiesKeyPaths: [PartialIdentifiableKeyPath] { + if let self = self as? AggregateKeyPathsEraser { + return self.erasedEntitiesKeyPaths as! [PartialIdentifiableKeyPath] + } + if let self = self as? IdentifiableKeyPathsEraser { return self.erasedEntitiesKeyPaths as! [PartialIdentifiableKeyPath] } + if let self = self as? CollectionAggregateKeyPathsEraser { + return self.erasedEntitiesKeyPaths as! [PartialIdentifiableKeyPath] + } + if let self = self as? CollectionIdentifiableKeyPathsEraser { return self.erasedEntitiesKeyPaths as! [PartialIdentifiableKeyPath] } @@ -32,6 +40,26 @@ extension AliasContainer: IdentifiableKeyPathsEraser where T: Identifiable { } } +private protocol AggregateKeyPathsEraser { + var erasedEntitiesKeyPaths: [Any] { get } +} + +extension AliasContainer: AggregateKeyPathsEraser where T: Aggregate { + var erasedEntitiesKeyPaths: [Any] { + [PartialIdentifiableKeyPath(\.content)] + } +} + +private protocol CollectionAggregateKeyPathsEraser { + var erasedEntitiesKeyPaths: [Any] { get } +} + +extension AliasContainer: CollectionAggregateKeyPathsEraser where T: MutableCollection, T.Element: Aggregate, T.Index: Hashable { + var erasedEntitiesKeyPaths: [Any] { + [PartialIdentifiableKeyPath(\.content)] + } +} + private protocol CollectionIdentifiableKeyPathsEraser { var erasedEntitiesKeyPaths: [Any] { get } } diff --git a/Sources/CohesionKit/Storage/AliasStorage.swift b/Sources/CohesionKit/Storage/AliasStorage.swift index 2c26aa6..58cc9ec 100644 --- a/Sources/CohesionKit/Storage/AliasStorage.swift +++ b/Sources/CohesionKit/Storage/AliasStorage.swift @@ -7,9 +7,9 @@ extension AliasStorage { set { self[buildKey(for: T.self, key: aliasKey)] = newValue } } - subscript(safe key: AliasKey) -> EntityNode> { + subscript(safe key: AliasKey, onChange onChange: ((EntityNode>) -> Void)? = nil) -> EntityNode> { mutating get { - self[key: key, default: EntityNode(AliasContainer(key: key), modifiedAt: nil)] + self[key: key, default: EntityNode(AliasContainer(key: key), modifiedAt: nil, onChange: onChange)] } } diff --git a/Tests/CohesionKitTests/IdentityMapTests.swift b/Tests/CohesionKitTests/IdentityMapTests.swift index f8e0a64..fdbb9cc 100644 --- a/Tests/CohesionKitTests/IdentityMapTests.swift +++ b/Tests/CohesionKitTests/IdentityMapTests.swift @@ -346,7 +346,7 @@ extension IdentityMapTests { } } - func test_updateNamed_entityIsCollection_itEnqueuesNestedObjectsInRegistry() { + func test_updateNamed_entityIsAggregate_itEnqueuesNestedObjectsInRegistry() { let registry = ObserverRegistryStub() let identityMap = IdentityMap(registry: registry) let initialValue = RootFixture( @@ -367,6 +367,44 @@ extension IdentityMapTests { XCTAssertTrue(registry.hasPendingChange(for: singleNodeUpdate)) } + + func test_updateNamed_aliasIsAggregate_itEnqueuesAliasInRegistry() { + let registry = ObserverRegistryStub() + let identityMap = IdentityMap(registry: registry) + let initialValue = RootFixture( + id: 1, + primitive: "", + singleNode: .init(id: 1), + listNodes: [] + ) + let singleNodeUpdate = SingleNodeFixture(id: 1, primitive: "update") + + _ = identityMap.store(entity: initialValue, named: .root) + + registry.clearPendingChangesStub() + + identityMap.update(named: .root) { + $0.singleNode = singleNodeUpdate + } + + XCTAssertTrue(registry.hasPendingChange(for: AliasContainer.self)) + } + + func test_update_entityIsIndirectlyUsedByAlias_itEnqueuesAliasInRegistry() { + let aggregate = RootFixture(id: 1, primitive: "", singleNode: SingleNodeFixture(id: 1), listNodes: []) + let registry = ObserverRegistryStub() + let identityMap = IdentityMap(registry: registry) + + _ = identityMap.store(entities: [aggregate], named: .rootList) + + registry.clearPendingChangesStub() + + identityMap.update(SingleNodeFixture.self, id: 1) { + $0.primitive = "updated" + } + + XCTAssertTrue(registry.hasPendingChange(for: AliasContainer<[RootFixture]>.self)) + } } private extension AliasKey where T == SingleNodeFixture { @@ -380,3 +418,7 @@ private extension AliasKey where T == [SingleNodeFixture] { private extension AliasKey where T == RootFixture { static let root = AliasKey(named: "root") } + +private extension AliasKey where T == [RootFixture] { + static let rootList = AliasKey(named: "root") +}