1

I'm running the below code to update some records based on a bank transaction history file that is sent to us each morning. It's pretty basic stuff but, for some reason, when I hit the end, dbContext.GetChangeSet() reports "0" for all actions.

public void ProcessBatchFile(string fileName)
{
    List<string[]> failed = new List<string[]>();
    int recCount = 0;
    DateTime dtStart = DateTime.Now;
    using (ePermitsDataContext dbContext = new ePermitsDataContext())
    {
        try
        {
            // A transaction must be begun before any data is read.
            dbContext.BeginTransaction();
            dbContext.ObjectTrackingEnabled = true;

            // Load all the records for this batch file.
            var batchRecords = (from b in dbContext.AmegyDailyFiles
                                where b.FileName == fileName
                                && b.BatchProcessed == false
                                && (b.FailReason == null || b.FailReason.Trim().Length < 1)
                                select b);

            // Loop through the loaded records
            int paymentID;
            foreach (var r in batchRecords)
            {
                paymentID = 0;
                try
                {
                    // We have to 'parse' the primary key, since it's stored as a string value with leading zero's.
                    if (!int.TryParse(r.TransAct.TrimStart('0'), out paymentID))
                        throw new Exception("TransAct value is not a valid integer: " + r.TransAct);

                    // Store the parsed, Int32 value in the original record and read the "real" record from the database.
                    r.OrderPaymentID = paymentID;
                    var orderPayment = this.GetOrderPayment(dbContext, paymentID);

                    if (string.IsNullOrWhiteSpace(orderPayment.AuthorizationCode))
                        // If we haven't processed this payment "Payment Received" do it now.
                        this.PaymentReceived(orderPayment, r.AuthorizationNumber);

                    // Update the PaymentTypeDetailID (type of Credit Card--all other types will return NULL).
                    var paymentTypeDetail = dbContext.PaymentTypes.FirstOrDefault(w => w.PaymentType1 == r.PayType);
                    orderPayment.PaymentTypeDetailID = (paymentTypeDetail != null ? (int?)paymentTypeDetail.PaymentTypeID : null);

                    // Match the batch record as processed.
                    r.BatchProcessed = true;
                    r.BatchProcessedDateTime = DateTime.Now;
                    dbContext.SubmitChanges();
                }
                catch (Exception ex)
                {
                    // If there's a problem, just record the error message and add it to the "failed" list for logging and notification.
                    if (paymentID > 0)
                        r.OrderPaymentID = paymentID;
                    r.BatchProcessed = false;
                    r.BatchProcessedDateTime = null;
                    r.FailReason = ex.Message;
                    failed.Add(new string[] { r.TransAct, ex.Message });
                    dbContext.SubmitChanges();
                }
                recCount++;
            }

            dbContext.CommitTransaction();
        }
        // Any transaction will already be commited, if the process completed successfully.  I just want to make
        //   absolutely certain that there's no chance of leaving a transaction open.
        finally { dbContext.RollbackTransaction(); }
    }

    TimeSpan procTime = DateTime.Now.Subtract(dtStart);

    // Send an email notification that the processor completed.
    System.Text.StringBuilder sb = new System.Text.StringBuilder();
    sb.AppendFormat("<p>Processed {0} batch records from batch file '{1}'.</p>", recCount, fileName);
    if (failed.Count > 0)
    {
        sb.AppendFormat("<p>The following {0} records failed:</p>", failed.Count);
        sb.Append("<ul>");
        for (int i = 0; i < failed.Count; i++)
            sb.AppendFormat("<li>{0}: {1}</li>", failed[i][0], failed[i][1]);
        sb.Append("<ul>");
    }
    sb.AppendFormat("<p>Time taken: {0}:{1}:{2}.{3}</p>", procTime.Hours, procTime.Minutes, procTime.Seconds, procTime.Milliseconds);
    EMailHelper.SendAdminEmailNotification("Batch Processing Complete", sb.ToString(), true);
}

The dbContext.BeginTransaction() method is something I added to the DataContext just to make it easy to use explicit transactions. I'm fairly confident that this isn't the problem, since it's used extensively elsewhere in the application. Our database design makes it necessary to use explicit transactions for a few, specific operations, and the call to "PaymentReceived" happens to be one of them.

I have stepped through the code and confirmed that the Rollback() method on the transaction itself is not begin called, and I have also checked the dbContext.GetChangeSet() before the call to CommitTransaction() happens with the same result.

I have included the BeginTransaction(), CommitTransaction() and RollbackTransaction() method bodies below, just for clarity.

/// <summary>
/// Begins a new explicit transaction on this context.  This is useful if you need to perform a call to SubmitChanges multiple times due to "circular" foreign key linkage, but still want to maintain an atomic write.
/// </summary>
public void BeginTransaction()
{
    if (this.HasOpenTransaction)
        return;

    if (this.Connection.State != System.Data.ConnectionState.Open)
        this.Connection.Open();

    System.Data.Common.DbTransaction trans = this.Connection.BeginTransaction();
    this.Transaction = trans;
    this._openTrans = true;
}
/// <summary>
/// Commits the current transaction (if active) and submits all changes on this context.
/// </summary>
public void CommitTransaction()
{
    this.SubmitChanges();
    if (this.Transaction != null)
        this.Transaction.Commit();
    this._openTrans = false;
    this.RollbackTransaction(); // Since the transaction has already been committed, this just disposes and decouples the transaction object itself.
}
/// <summary>
/// Disposes and removes an existing transaction on the this context.  This is useful if you want to use the context again after an explicit transaction has been used.
/// </summary>
public void RollbackTransaction()
{
    // Kill/Rollback the transaction, as necessary.
    try
    {
        if (this.Transaction != null)
        {
            if (this._openTrans)
                this.Transaction.Rollback();
            this.Transaction.Dispose();
            this.Transaction = null;
        }
        this._openTrans = false;
    }
    catch (ObjectDisposedException) { } // If this gets called after the object is disposed, we don't want to let it throw exceptions.
    catch { throw; }
}
Mike U
  • 709
  • 6
  • 11
  • 1
    `catch (ObjectDisposedException) { }` that's swallowing severe bugs. Fix the bug. Don't hide the error message. – usr Jan 22 '16 at 20:52
  • 1
    This error handling is subtly broken. If you make an entity invalid and it fails when writing, all future writes will fail, too, because the entity is stuck in the context. – usr Jan 22 '16 at 20:55
  • 1
    I advise you to abandon those transaction helper methods. The usual `using (var tran = BeginTran()) { tran.Commit(); }` is all you need. Does the same thing but simpler. – usr Jan 22 '16 at 20:57
  • You say it's simpler to not use the transaction methods, but you're not maintaining a 500k line code base :) – Mike U Jan 23 '16 at 22:36
  • As for errors, I've stepped through the code line-by-line *multiple* times, and no errors are occurring. Not from .NET or SQL. As for swallowing the "ObjeectDIsposedException" that's a preventative measure. I don't care if someone calls "RollbackTransaction" after the Transaction has already been disposed. It's not being swallowed to "hide" a bug, it's being swallowed to make sure that this non-error doesn't break other code. – Mike U Jan 23 '16 at 22:39
  • 1
    I'm maintaining a 100kloc codebase that heavily uses L2S and transactions. The transactions use `TransactionScope`, though, but that should be similar. Regarding hiding bugs, I'd consider it to be a code smell to allow wrong API usage. You also might hide genuine bugs that way.; Run an experiment to confirm that changes can be made at all. Add `batchRecords.First().SomeProperty = 1234; db.SubmitChanges();`. Does this go through? It should. You can then move that line around and test different places. This is vague, but we have little options :) Need to investigate. – usr Jan 23 '16 at 23:42
  • I tried this, and still nothing. I took a screen shot of my debugging session to better illustrate what I'm seeing. Note that the breakpoint is on the "CommitTransaction()" line, so that command has not yet happened. I also removed all references to "SubmitChanges()" just to make certain that that wasn't affecting the result. http://imgur.com/XiTgqaZ – Mike U Jan 25 '16 at 15:36
  • 1
    I guess the question reduces to "when I set an entity property the change is never written". Correct? That means we can ignore pretty much everything in the code posted. – usr Jan 25 '16 at 15:39
  • Yes. Ultimately, the issue is that the DataContext is not properly tracking changes. I haven't seen anything like this before, where there is no error of any kind being thrown, but my ChangeSet comes out "0". The code was posted in case anyone spotted something I had missed. – Mike U Jan 25 '16 at 15:40
  • 1
    Can you post a 3 line piece of repro code that you can run and see the error? Like 1. create context, 2. write, 3. submit. That makes 100% sure that nothing in this big code causes the problem. – usr Jan 25 '16 at 15:41
  • That was a good idea I hadn't tried yet. So, for a sanity check, I commented out everything except the DataContext instantiation, query, batchRecord.First() and replaced the "CommitTransaction" with a standard "SubmitChanges()". I also commented out the "BeginTransaction" line at the top. Same result. This is really, really weird. http://imgur.com/OZH8qPl – Mike U Jan 25 '16 at 15:47
  • 1
    OK, you can find this bug for sure by cloning the solution and then deleting EVERYTHING piece by piece to reduce the repo. In the end there will be the DBML and 1 cs file with 3 lines of code. In the process the bug will disappear I predict and you find it. Or, the DBML is botched somehow. Can you set fields to never write back in the DBML? No idea. – usr Jan 25 '16 at 15:54
  • Found the problem, partially thanks to the direction you pointed me in with the First() thing. Looking at the generated entity code, there were no change tracking events generated. – Mike U Jan 25 '16 at 16:20

1 Answers1

1

I just found the problem: my DBA didn't put a primary key on the table when he created it for me, so LinqToSql did not generate any of the "PropertyChanged" event/handler stuff in the entity class, which is why the DataContext was not aware that changes were being made. Apparently, if your table has no primary key, Linq2Sql won't track any changes to that table, which makes sense, but it would be nice if there were some kind of notification to that effect. I'm sure my DBA didn't think about it, because of this just being a way of "tracking" which of these line items from the text file had been processed and doesn't directly relate to any other tables.

Mike U
  • 709
  • 6
  • 11