Reputation: 23690
I want to add 1
to a value within my database within a transaction. I want to ensure that the record is updated properly and hasn't been changed by someone else during that time.
I have the following code that I thought would work, but I can still pause during debugging, change the record in the database to something different and then it becomes inconsistent.
Here's my code:
using (var transaction = this.Context.Database.BeginTransaction())
{
try
{
if (quiz.PasswordRequiredToTakeQuiz())
{
// Check password exists for quiz
bool passwordIsValid = quiz.QuizPasswords.Any(x => x.Password.ToLower() == model.QuizPassword.ToLower() && !x.Deleted);
QuizPassword quizPassword = quiz.QuizPasswords.Where(x => x.Password.ToLower() == model.QuizPassword.ToLower() && !x.Deleted).First();
string passwordError = "Sorry the password you provided has expired or is not valid for this quiz";
if (!passwordIsValid)
{
ViewData.ModelState.AddModelError("QuizPassword", passwordError);
}
else
{
// Password is valid for use with this quiz, but can it be used?
if (quizPassword.RemainingUses < 1 && quizPassword.UnlimitedUses != true)
{
// Password cannot be used
ViewData.ModelState.AddModelError("QuizPassword", passwordError);
}
else
{
// Password CAN be used
if (!quizPassword.UnlimitedUses)
{
quizPassword.RemainingUses--;
}
// Increase use count
quizPassword.UseCount++;
this.Context.EntitySet<QuizPassword>().Attach(quizPassword);
this.Context.Entry(quizPassword).State = EntityState.Modified;
// I can change the record UseCount value in the database at this point
// then when it saves, it becomes inconsistent with other's use of
// the password
this.Context.SaveChanges();
}
}
}
// Commit the changes
transaction.Commit();
}
catch(Exception)
{
transaction.Rollback();
}
finally
{
transaction.Dispose();
}
}
Turn of events:
0
in the databaseSaveChanges()
5
1
.Normally I'd achieve this using SELECT FOR UPDATE
to lock the record temporarily, but I was originally using PHP + MySQL.
I have read that the lock isn't possible, so I'm wondering how this can be achieved.
It's important because I don't want people to be able to use a password more than a set number of times! If it is possible for someone to change the value in the mean time, it doesn't guarantee the correct number of uses.
Upvotes: 0
Views: 1019
Reputation: 23690
I created a stored procedure that returned a SELECT
statement of the value I wanted to return, which is a Success
int
.
DROP PROCEDURE IF EXISTS UsePassword;
DELIMITER //
CREATE PROCEDURE UsePassword (QuizId INT(11), PasswordText VARCHAR(25))
BEGIN
/* Get current state of password */
SELECT RemainingUses, UnlimitedUses, Deleted INTO @RemainingUses, @UnlimitedUses, @Deleted FROM QuizPassword q WHERE q.QuizId = QuizId AND `Password` = PasswordText AND Deleted = 0 LIMIT 0,1 FOR UPDATE;
IF FOUND_ROWS() = 0 OR @Deleted = 1 THEN
/* Valid password not found for quiz */
SET @Success = 0;
ELSEIF @UnlimitedUses = 1 THEN
UPDATE QuizPassword SET UseCount = UseCount + 1 WHERE QuizId = QuizId AND `Password` = PasswordText;
SET @Success = ROW_COUNT();
ELSEIF @RemainingUses > 0 AND @UnlimitedUses = 0 THEN
UPDATE QuizPassword SET UseCount = UseCount + 1, RemainingUses = RemainingUses - 1 WHERE QuizId = QuizId AND `Password` = PasswordText;
SET @Success = ROW_COUNT();
ELSE
SET @Success = 0;
END IF;
/* Return rows changed rows */
SELECT @Success AS Success;
END //
DELIMITER;
I had to create a new object to hold the values, I've only got one field in there, but you could put more.
// Class to hold return values from stored procedure
public class UsePasswordResult
{
public int Success { get; set; }
// could have more fields...
}
I reduced my final code down to this, which calls the stored procedure and assigns the values to the member variables within the object:
using (var transaction = this.Context.Database.BeginTransaction())
{
try
{
if (quiz.PasswordRequiredToTakeQuiz())
{
// Attempt to use password
UsePasswordResult result = this.Context.Database.SqlQuery<UsePasswordResult>("CALL UsePassword({0}, {1})", quiz.Id, model.QuizPassword).FirstOrDefault();
// Check the result of the password use
if (result.Success != 1)
{
// Failed to use the password
ViewData.ModelState.AddModelError("QuizPassword", "Sorry the password you provided has expired or is not valid for this quiz");
}
}
// Is model state still valid after password checks?
if (ModelState.IsValid)
{
// Do stuff
}
transaction.Commit();
}
catch(Exception)
{
transaction.Rollback();
}
finally
{
transaction.Dispose();
}
}
The values you return from the stored procedure must be exactly the same names as the ones in the class it's going to produce as the result.
Because I'm calling the procedure
within my transaction using
statement, the statement locks the record because I've selected it as SELECT... FOR UPDATE
until transaction.Commit()/Rollback()/Dispose() is called within the code... thus preventing anyone from attempting to use the password whilst anyone else.
Upvotes: 0
Reputation: 556
One solution would be to do it with plain sql (ado.net) and pessimistic locking.
BEGIN TRANSACTION
SELECT usecount, unlimiteduses FROM quizpassword WITH (UPDLOCK, HOLDLOCK) WHERE id = x;
// check usecount here
// only do this if unlimitedUses == false
UPDATE quizpassword SET usecount = usecount + 1 WHERE id = x;
UPDATE quizpassword SET remaininguses = remaininguses -1 WHERE id = x;
COMMIT TRANSACTION // (lock is released)
Upvotes: 1