MicroLite-ORM / MicroLite

MicroLite ORM framework
microliteorm.wordpress.com
Apache License 2.0
85 stars 24 forks source link

Table row versioning (Optimistic concurrency control) #427

Closed pleb closed 8 years ago

pleb commented 8 years ago

A common use case is to only update a db row if the version column matches. For instance, UPDATE .. WHERE Version = 1 AND Id = 1. Such a method can ensures the caller is only updating records which haven't changed since the data was fetched. Usually this is combined with a transaction, and upon error, the transaction is rolled back.

pleb commented 8 years ago

I've added additional support for this https://github.com/CollaboratingPlatypus/MicroLite/tree/versioning

Items left on my list are:

pleb commented 8 years ago

User or Framework - to throw or not to throw.

Breaking it down...

Throw currency exception

From experience, this would be the norm. The OptimisticConcurrencyException is already available in the BCL.

if (rowsAffected == 0 && objectInfo.TableInfo.VersionColumn != null)
{
    throw new DBConcurrencyException(ExceptionMessages.Session_UpdateOptimistiConcurrencyError.FormatWith(objectInfo.TableInfo.Schema, objectInfo.TableInfo.Name, objectInfo.TableInfo.VersionColumn))
}

User to handle

Let's scratch the extension method idea, as it doesn't really suit. :neutral_face:

If the user is using the row version feature, I would at a guess, think that they would want this check handled for them. If it isn't, I could envisage a situation a typical routine of drop in the standard update listener to throw DBConcurrencyException exception.

Result

I guess the question is whether there's a use case for not throwing the DBConcurrencyException.

pleb commented 8 years ago

I think this feature work is nearly complete. I just need to run a few integration tests to confirm it does work as the unit tests state it should.

pleb commented 8 years ago

This feature work is complete. Waiting for the current PRs to go through, so that there's no overlap.

pleb commented 8 years ago

Trevor didn't follow up on the PR, so I'm closing this issue