automerge / automerge-swift

Swift language bindings presenting Automerge
https://automerge.org/automerge-swift/documentation/automerge/
MIT License
217 stars 11 forks source link

Added public initializer to ActorId and tests to validate setting `actor` property #132

Closed bgomberg closed 3 months ago

bgomberg commented 3 months ago

Regarding the use-case for this, we set the actorId by concatenating the user ID with a globally-unique device ID. We leverage this heavily in our internal tooling to browse document history for debugging various issues. Here's an example where can see who made each change by looking up the user ID. This is very helpful both for development to figure out issues with our sync protocol, as well as customer support in figuring out what made what change to a document.

Screenshot_2024-03-15_at_4 46 26_PM
heckj commented 3 months ago

Thanks @bgomberg - the ActorId struct in the core library is an explicit 16 bytes. Swift doesn't have quite as easy a path to assert that a type is a specific length of bytes, and I'm not sure what happens if you attempt to initialize it with something significantly larger, and if it supports inter-operation with documents with different actorId sizes. It may be that with the current implementation, we're constrained to only 16 bytes, and that if you do implement it, you're responsible for verifying you're not making concurrent updates with the same actorId.

If we're going to expose a public initializer, then I think we'd want at least constrain it to 16 bytes. I'm doing the same in DocumentId with the Automerge-repo work (implementation external from here) - the gist of which is a default initializer that loads from a UUID, and any other initializers that verify the content is 16 bytes or kicks out an error.