Skip to content

Commit

Permalink
test that illustrates failure scenario for issue #54 (#55)
Browse files Browse the repository at this point in the history
* test that illustrates failure scenario for issue #54
* establishing a tree of AutomergeEncoderImpl during encoding to be able to later walk and clean up where needed
* tracking keys written, highest index written, and objectIds for each container when found
* removing extraneous elements from tree, test to verify tree integrity
* verifying ObjectId found for each relevant container
* enables post-encoding cleanup using details from captured encode-tree
  • Loading branch information
heckj committed Jul 30, 2023
1 parent d448ddd commit 47c3720
Show file tree
Hide file tree
Showing 8 changed files with 232 additions and 21 deletions.
14 changes: 14 additions & 0 deletions .swiftpm/xcode/xcshareddata/xcschemes/Automerge.xcscheme
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@
ReferencedContainer = "container:">
</BuildableReference>
</BuildActionEntry>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "YES"
buildForProfiling = "YES"
buildForArchiving = "YES"
buildForAnalyzing = "YES">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "AutomergeUniffi"
BuildableName = "AutomergeUniffi"
BlueprintName = "AutomergeUniffi"
ReferencedContainer = "container:">
</BuildableReference>
</BuildActionEntry>
</BuildActionEntries>
</BuildAction>
<TestAction
Expand Down
2 changes: 2 additions & 0 deletions Sources/Automerge/Codable/Encoding/AutomergeEncoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public struct AutomergeEncoder {
cautiousWrite: cautiousWrite
)
try value.encode(to: encoder)
encoder.postencodeCleanup()
}

public func encode<T: Encodable>(_ value: T, at path: [CodingKey]) throws {
Expand All @@ -39,5 +40,6 @@ public struct AutomergeEncoder {
cautiousWrite: cautiousWrite
)
try value.encode(to: encoder)
encoder.postencodeCleanup()
}
}
65 changes: 64 additions & 1 deletion Sources/Automerge/Codable/Encoding/AutomergeEncoderImpl.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/// Convenience marker within AutomergeEncoderImpl to indicate the kind of associated container

/// The internal implementation of AutomergeEncoder.
///
/// Instances of the class capture one of the various kinds of schema value types - single value, array, or object.
Expand All @@ -12,6 +14,16 @@ final class AutomergeEncoderImpl {
// indicator that the singleValue has written a value
var singleValueWritten: Bool = false

// Tracking details of what was written by Codable implementations
// working with the container encode() calls. The details captured
// in these variables allow us to "clean up" anything extraneous
// in the data store that wasn't overwritten by an encode.
var containerType: EncodingContainerType?
var childEncoders: [AutomergeEncoderImpl] = []
var highestUnkeyedIndexWritten: UInt64?
var mapKeysWritten: [String] = []
var objectIdForContainer: ObjId?

init(
userInfo: [CodingUserInfoKey: Any],
codingPath: [CodingKey],
Expand All @@ -25,12 +37,60 @@ final class AutomergeEncoderImpl {
schemaStrategy = strategy
self.cautiousWrite = cautiousWrite
}

func postencodeCleanup() {
precondition(objectIdForContainer != nil)
precondition(containerType != nil)
guard let objectIdForContainer, let containerType else {
return
}
switch containerType {
case .Key:
precondition(!mapKeysWritten.isEmpty)
// Remove keys that exist in this objectId that weren't
// written during encode. (clean up 'dead' keys from maps)
let extraAutomergeKeys = document.keys(obj: objectIdForContainer)
.filter { keyValue in
!mapKeysWritten.contains(keyValue)
}
for extraKey in extraAutomergeKeys {
do {
try document.delete(obj: objectIdForContainer, key: extraKey)
} catch {
fatalError("Unable to delete extra key \(extraKey) during post-encode cleanup: \(error)")
}
}
case .Index:
precondition(highestUnkeyedIndexWritten != nil)
guard let highestUnkeyedIndexWritten else {
return
}
// Remove index elements that exist in this objectId beyond
// the max written during encode. (allow arrays to 'shrink')
var highestAutomergeIndex = document.length(obj: objectIdForContainer) - 1
while highestAutomergeIndex > highestUnkeyedIndexWritten {
do {
try document.delete(obj: objectIdForContainer, index: highestAutomergeIndex)
highestAutomergeIndex -= 1
} catch {
fatalError(
"Unable to delete index position \(highestAutomergeIndex) during post-encode cleanup: \(error)"
)
}
}
case .Value:
return
}
// Recursively walk the encoded tree doing "cleanup".
for child in childEncoders {
child.postencodeCleanup()
}
}
}

// A bit of example code that someone might implement to provide Encodable conformance
// for their own type.
//
//
// extension Coordinate: Encodable {
// func encode(to encoder: Encoder) throws {
// var container = encoder.container(keyedBy: CodingKeys.self)
Expand Down Expand Up @@ -60,6 +120,7 @@ extension AutomergeEncoderImpl: Encoder {
codingPath: codingPath,
doc: document
)
containerType = .Key
return KeyedEncodingContainer(container)
}

Expand All @@ -74,6 +135,7 @@ extension AutomergeEncoderImpl: Encoder {
preconditionFailure()
}

containerType = .Index
return AutomergeUnkeyedEncodingContainer(
impl: self,
codingPath: codingPath,
Expand All @@ -92,6 +154,7 @@ extension AutomergeEncoderImpl: Encoder {
preconditionFailure()
}

containerType = .Value
return AutomergeSingleValueEncodingContainer(
impl: self,
codingPath: codingPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
) {
case let .success(objId):
objectId = objId
impl.objectIdForContainer = objId
lookupError = nil
case let .failure(capturedError):
objectId = nil
Expand Down Expand Up @@ -98,6 +99,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
throw reportBestError()
}
try document.put(obj: objectId, key: key.stringValue, value: .Null)
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Bool, forKey key: Self.Key) throws {
Expand All @@ -108,6 +110,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .bool)
}
try document.put(obj: objectId, key: key.stringValue, value: .Boolean(value))
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: String, forKey key: Self.Key) throws {
Expand All @@ -118,6 +121,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .string)
}
try document.put(obj: objectId, key: key.stringValue, value: .String(value))
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Double, forKey key: Self.Key) throws {
Expand All @@ -134,6 +138,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .double)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Float, forKey key: Self.Key) throws {
Expand All @@ -150,6 +155,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .double)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Int, forKey key: Self.Key) throws {
Expand All @@ -160,6 +166,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .int)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Int8, forKey key: Self.Key) throws {
Expand All @@ -170,6 +177,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .int)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Int16, forKey key: Self.Key) throws {
Expand All @@ -180,6 +188,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .int)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Int32, forKey key: Self.Key) throws {
Expand All @@ -190,6 +199,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .int)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: Int64, forKey key: Self.Key) throws {
Expand All @@ -200,6 +210,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .int)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: UInt, forKey key: Self.Key) throws {
Expand All @@ -210,6 +221,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .uint)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: UInt8, forKey key: Self.Key) throws {
Expand All @@ -220,6 +232,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .uint)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: UInt16, forKey key: Self.Key) throws {
Expand All @@ -230,6 +243,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .uint)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: UInt32, forKey key: Self.Key) throws {
Expand All @@ -240,6 +254,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .uint)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode(_ value: UInt64, forKey key: Self.Key) throws {
Expand All @@ -250,9 +265,13 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .uint)
}
try document.put(obj: objectId, key: key.stringValue, value: value.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
}

mutating func encode<T: Encodable>(_ value: T, forKey key: Self.Key) throws {
guard let objectId = objectId else {
throw reportBestError()
}
let newPath = impl.codingPath + [key]
// this is where we need to figure out what the encodable type is in order to create
// the correct Automerge objectType underneath the covers.
Expand All @@ -270,17 +289,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
// the Codable method `encode` is called - because that's where a container is created. So while we
// can set this "newPath", we don't have the deets to create (if needed) a new objectId until we
// initialize a specific container type.
guard let objectId = objectId else {
throw reportBestError()
}

let newEncoder = AutomergeEncoderImpl(
userInfo: impl.userInfo,
codingPath: newPath,
doc: document,
strategy: impl.schemaStrategy,
cautiousWrite: impl.cautiousWrite
)
switch T.self {
case is Date.Type:
// Capture and override the default encodable pathing for Date since
Expand All @@ -290,6 +299,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .timestamp)
}
try document.put(obj: objectId, key: key.stringValue, value: downcastDate.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
case is Data.Type:
// Capture and override the default encodable pathing for Data since
// Automerge supports it as a primitive value type.
Expand All @@ -298,6 +308,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .bytes)
}
try document.put(obj: objectId, key: key.stringValue, value: downcastData.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
case is Counter.Type:
// Capture and override the default encodable pathing for Counter since
// Automerge supports it as a primitive value type.
Expand All @@ -306,6 +317,7 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try checkTypeMatch(value: value, objectId: objectId, key: key, type: .counter)
}
try document.put(obj: objectId, key: key.stringValue, value: downcastCounter.toScalarValue())
impl.mapKeysWritten.append(key.stringValue)
case is Text.Type:
// Capture and override the default encodable pathing for Counter since
// Automerge supports it as a primitive value type.
Expand Down Expand Up @@ -336,8 +348,21 @@ struct AutomergeKeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProt
try document.spliceText(obj: textNodeId, start: UInt64(offset), delete: 1)
}
}
impl.mapKeysWritten.append(key.stringValue)
default:
let newEncoder = AutomergeEncoderImpl(
userInfo: impl.userInfo,
codingPath: newPath,
doc: document,
strategy: impl.schemaStrategy,
cautiousWrite: impl.cautiousWrite
)
// Create a link from the current AutomergeEncoderImpl to the child, which
// will be referenced from future containers and updated with status.
impl.childEncoders.append(newEncoder)

try value.encode(to: newEncoder)
impl.mapKeysWritten.append(key.stringValue)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct AutomergeSingleValueEncodingContainer: SingleValueEncodingContainer {
case let .success(objId):
if let lastCodingKey = codingPath.last {
objectId = objId
impl.objectIdForContainer = objId
codingkey = AnyCodingKey(lastCodingKey)
lookupError = nil
} else {
Expand Down
Loading

0 comments on commit 47c3720

Please sign in to comment.