CSLA 3.5: Bug in MarkClean method with child collections?

CSLA 3.5: Bug in MarkClean method with child collections?

Old forum URL: forums.lhotka.net/forums/t/5249.aspx

Patrick posted on Friday, August 15, 2008

Hi,

I have a root object with child collections.
Order
OrderItems

And I call MarkClean on Order it doesn't actually mark the child objects as clean.
Or to be more accurate it sets OrderItems._isDirty = false but the actual order items themselves stay untouched and might still be dirty...
This in turn means that after calling Order.MarkClean() Order.IsDirty can still be true.

Thanks,
Patrick

It's implemented like this:

// FieldManager.cs
    internal void MarkClean()
    {
      foreach (var item in _fieldData)
        if (item != null && item.IsDirty)
// It seems like we would need recursion here so that each FieldData inside the FieldData would set IsDirty to false as well.
          item.MarkClean();
    }


// FieldData.cs
    public void MarkClean()
    {
      _isDirty = false;
    }

ajj3085 replied on Friday, August 15, 2008

Don't think that's a bug.  If you're doing the old-school style of unmanaged properties, it was always your responsibility to mark child objects as clean after they saved themselves.

If you're using Managed properties, I would think that the call to DataPortal.UpdateChild will handle marking the child object as clean.   If it's not, that could be a bug, but it seems to be working for me.

Patrick replied on Friday, August 15, 2008

ajj3085:
If you're using Managed properties, I would think that the call to DataPortal.UpdateChild will handle marking the child object as clean.  

Yes I'm using managed properties.

The very fact though that I can call MarkClean() on an object and afterwards IsDirty == true makes it seem to me that:
* either the name is not clear. If that's the case MarkSelfClean() might be more correct
* that there is a bug by not going recursive through the object fields
* or that I'm misunderstanding the methods purpose Smile [:)]

Thanks,
Patrick

RockfordLhotka replied on Friday, August 15, 2008

This seems like a potential bug.

MarkClean() never has, and still wouldn't, affect child objects.

But it has, and should, make the current object's state non-dirty. Which means, as you point out, that it should loop through the field manager and set all non-child fields as unchanged. That would preserve the same behavior as with non-managed fields.

ajj3085 replied on Monday, August 18, 2008

Ok, I see that there could be a problem... I just don't get why a FieldData would contain other FieldData objects. Is that normal?  I haven't looked too much at the code (I did check out this part now)... I guess I'll have to read very carefully the new book when it's out.  Fortunately, I have it on pre-order on Amazon.  Smile [:)]

RockfordLhotka replied on Monday, August 18, 2008

The field manager concept is designed to allow CSLA to manage backing field values for properties. That includes references to child objects, and values like a number or string or date.

 

The default FieldData<T> implementation understands the difference between child objects and any other value, as does the field manager. These two types of value are stored in a single list internally, because that was the best solution overall, but they are really quite different in many ways.

 

And YOU can create your own IFieldData implementation, in which case you might define entirely different behaviors for whether a field is dirty or not, etc. The specific target scenario is where you want to maintain the original value for each field so you can do per-field concurrency checking and more sophisticated dirty checking.

 

The book doesn’t really cover that scenario – I’m having a hard enough time keeping the book at the page number target as it is. I think two other books are needed

 

a.       Using CSLA .NET

b.      Extending CSLA .NET

 

I just need a couple clones to write those :)

 

Rocky

 

ajj3085 replied on Monday, August 18, 2008

Ok, I think I get it.  I see now why I don't think I've hit this issue.

markell replied on Wednesday, April 01, 2009

Hi folks.
Maybe I misunderstand something, but I hit the same issue not with child collections, but with simple parent-child relationship. And I am staring right now at this piece of code inside the FieldData.cs file:

    public virtual bool IsDirty
    {
      get
      {
        ITrackStatus child = _data as ITrackStatus;
        if (child != null)
        {
          return child.IsDirty;

        }
        else
        {
          return _isDirty;
        }
      }
    }

    /// <summary>
    /// Marks the field as unchanged.
    /// </summary>
    public void MarkClean()
    {
      _isDirty = false;
    }

How come MarkClean is not synchronized with IsDirty? Shouldn't we have the following assertion:

if (obj.IsDirty)
{
   obj.MarkClean();
   Assert(!obj.IsDirty);
}

Because right now this assertion fails for the simple parent-child pair, when the child field is changed.
Or am I missing something?
Thanks.

RockfordLhotka replied on Wednesday, April 01, 2009

The IsSelfDirty property is probably what you are looking for.

 

Please remember that CSLA .NET has been around for over 9 years now, and it has evolved (and continues to evolve) over that time. While it evolves, I try as much as possible to preserve backward compatibility by not changing the names or semantic meaning of existing properties and methods.

 

The end result are some inconsistencies. For the most part, I feel the cost of living with a few minor inconsistencies is lower than the cost of breaking every application that uses CSLA by renaming methods, and so I leave the names alone as much as possible.

 

Rocky

markell replied on Thursday, April 02, 2009

Hi Rocky.
Thanks for the prompt reply.
I do not understand why IsSelfDirty?
Shouldn't MarkClean mark the whole object tree as clean?
If not than which method does so?

As far as I can see ITrackStatus is a read-only interface. It allows one to write deep IsDirty, but provides no means for deep MarkClean.

In my copy of csla I changed FieldData<T>.MarkClean to:
    public void MarkClean()
    {
      _isDirty = false;
      BusinessBase bb = _data as BusinessBase;
      if (bb != null)
      {
        bb.MarkClean();
      }
    }

Of course, I had to change the scope of BusinessBase.MarkClean from protected
to protected internal.

I realize that this workaround is problematic because BusinessBase does not 
correspond to ITrackStatus, but it suits my immediate needs. I just do not
want to change the ITrackStatus interface.

I would like very much to get your advise on how to do it correctly.
Thanks.

RockfordLhotka replied on Thursday, April 02, 2009

There is no method to mark the entire graph as clean, or new, or deleted.

 

Generally speaking, marking an individual object as dirty or clean is handled by the data portal, or by n-level undo. CSLA is designed with that approach in mind.

 

Rocky

markell replied on Saturday, April 04, 2009

Your reply made me review my data portal code and I think I have found a bug there, which is a relief, since now I can revert my csla changes and fix it where it should be - in my code.
Thanks.

Copyright (c) Marimer LLC