Handle SetStreamMetadata idempotently #129
Conversation
|
I'm comfortable in declaring this as non-breaking. |
damianh
changed the title
|
|
||
| var metadata = await store.GetStreamMetadata(streamId); | ||
|
|
||
| metadata.MetadataStreamVersion.ShouldBe(0); |
There was a problem hiding this comment.
The primary test. StreamVersion should not have been incremented and stay at 0.
| <GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles> | ||
| <RootNamespace>SqlStreamStore</RootNamespace> | ||
| <NoWarn>1701;1702;1705;1591</NoWarn> | ||
| <LangVersion>latest</LangVersion> |
There was a problem hiding this comment.
Slipped this in to all projects
| } | ||
|
|
||
| using(var command = new SqlCommand(commandText, connection)) | ||
| using (var command = new SqlCommand(commandText, connection, transaction)) |
There was a problem hiding this comment.
Needed to pass this down because command wouldn't work if in a transaction scope. In other paths, this is null, which is fine.
There was a problem hiding this comment.
I'm not completely following - are you saying that particular overload needs to be called even if transaction is null?
There was a problem hiding this comment.
Because this transcation
was getting associated with the connection, if I didn't pass it to this sql command I was getting sql exception.System.InvalidOperationException: BeginExecuteReader requires the command to have a transaction when the connection assigned to the command is in a pending local transaction. The Transaction property of the command has not been initialized.
at System.Data.SqlClient.SqlCommand.ValidateCommand(Boolean async, String method)
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean asyncWrite, String method)
at System.Data.SqlClient.SqlCommand.BeginExecuteReader(CommandBehavior behavior, AsyncCallback callback, Object stateObject)
at System.Threading.Tasks.TaskFactory`1.FromAsyncImpl[TArg1](Func`4 beginMethod, Func`2 endFunction, Action`1 endAction, TArg1 arg1, Object state, TaskCreationOptions creationOptions)
at System.Threading.Tasks.TaskFactory`1.FromAsync[TArg1](Func`4 beginMethod, Func`2 endMethod, TArg1 arg1, Object state)
at System.Data.SqlClient.SqlCommand.ExecuteReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
|
Would like to slip this into |
| /// <returns> | ||
| /// A deterministically generated GUID. | ||
| /// </returns> | ||
| public Guid Create(IEnumerable<byte> source) |
There was a problem hiding this comment.
Very minor, but there's no reason to step into the enumerable world - sticking with byte arrays keeps the translation down to a minimum.
| } | ||
|
|
||
| using(var command = new SqlCommand(commandText, connection)) | ||
| using (var command = new SqlCommand(commandText, connection, transaction)) |
There was a problem hiding this comment.
I'm not completely following - are you saying that particular overload needs to be called even if transaction is null?
… be used by all storage implementations.
…essage id. The underlying append operation handles the operation idempotently
… setting stream metadata. Need to pass the existing outer transaction to subsequent sql commands.
2 participants
In many cases a developer will lazily set the metadata of a stream. There are scenarios, especially when appending with StreamVersion.Any that they may set the same metadata over and over. This will increase the metadata stream length unnecessarily.
In another case, if users want to handling this idempotently, they have to invoke a read'n'check first. This has subtle race conditions.
This PR:
MetadataMessageIdGeneratorwith a fixed GUID namespace.