New Posts New Posts RSS Feed: Entities marked as HasChanged when a property hasn't really changed
  FAQ FAQ  Forum Search   Calendar   Register Register  Login Login

Entities marked as HasChanged when a property hasn't really changed
 Post Reply Post Reply Page  12>
Author
  Share Topic Share Topic  Topic Search Topic Search  Topic Options Topic Options
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Topic: Entities marked as HasChanged when a property hasn't really changed
    Posted: 31-Jul-2009 at 7:17am
When I execute this test: 
[TestMethod]

public void TestEntityChangedProperties()

{

    Deal deal = Deal.Create();

    deal.FlowDateStart = new DateTime(2009, 4, 1);

    IdeaBlade.EntityModel.SaveResult result = deal.EntityAspect.EntityManager.SaveChanges();

    Assert.IsTrue(result.Ok);

    Assert.IsFalse(deal.EntityAspect.HasChanges());



    //now, change the property to itself and see if HasChanges has changed

    deal.FlowDateStart = new DateTime(2009, 4, 1);

    Assert.IsFalse(deal.EntityAspect.HasChanges()); //FAILS

    Assert.AreEqual(IdeaBlade.EntityModel.EntityState.Unchanged, deal.EntityAspect.EntityState);

}


The test fails. Why? If I am setting the date to 4/1 and then after the save, changing it back to 4/1, shouldn't the property setter not bother to set the value and not mark the entity as dirty?


Edited by skingaby - 31-Jul-2009 at 7:18am
Back to Top
kimj View Drop Down
IdeaBlade
IdeaBlade
Avatar

Joined: 09-May-2007
Posts: 1391 		Post Options Post Options   Quote kimj Quote  Post ReplyReply Direct Link To This Post Posted: 31-Jul-2009 at 9:26am
We've had an internal debate about whether property setters on the Entity should first check if the incoming value is equal to the current value.  So far, the winning side says that we do not make this check.  This means that even if the new value is the same as the old, we still mark the entity as dirty.   This behavior may change some day, especially if we get a lot of complaints about it.
 
One workaround, although I haven't tried it, might be to add an entity/property wide BeforeSet property interceptor which makes this check and cancels further setter actions if needed.
 
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 31-Jul-2009 at 10:28am
GROOOANN!!!!
Cast my vote firmly against making an object dirty when a property hasn't really changed!
Yeuch.
And the workaround is going to be an AWFUL lot of tedious code.

Here's our real world problem:
1) Create a Silverlight grid with some date columns bound to Entities.
2) Tie functionality to the "Dirty" state of the row, perhaps enable/disable the row Save button.
3) Make single click edit functionality work in the cells. (I.e. skip the "click to select, then click again to edit" by dropping into edit mode on the cell when it gets focus.
4) Now, run the app. Click on the date field in several rows. Because of the silverlight binding behavior, as you click through the rows, with single-click edit, the field drops into and out of edit mode and the entity property gets read and set. Now, every row you touched is dirty, even though you haven't changed any thing. Inexplicably, all the save buttons light up. 555-1212 >> "Helpdesk? Yes, I didn't change anything but it wants me to save."

Maybe you can build this in and make a toggle so that developers can select to enable/disable the check with a selection in the Object Mapper.

Here's the code that I was generating in our custom datalayer before we switched to IdeaBlade: 
<System.Runtime.Serialization.DataMemberAttribute()>  _

Public Overridable Property DateOfDeal() As Date Implements IDeal.DateOfDeal

     Get

          Return Me._dateOfDeal

     End Get

     Set

          If (Me._dateOfDeal <> value) Then

               Me._dateOfDeal = value

               Me.IsDirty = true

          End If

     End Set

End Property



<System.Runtime.Serialization.DataMemberAttribute()>  _

Public Overridable Property DealNumber() As System.Nullable(Of Long) Implements IDeal.DealNumber

     Get

          Return Me._dealNumber

     End Get

     Set

          If ((value.HasValue = false)  _

                         AndAlso (Me._dealNumber.HasValue = true)) Then

               Me._dealNumber = Nothing

               Me.IsDirty = true

          Else

               If ((value.HasValue = true)  _

                              AndAlso (Me._dealNumber.HasValue = false)) Then

                    Me._dealNumber = value

                    Me.IsDirty = true

               Else

                    If (((value.HasValue = true)  _

                                   AndAlso (Me._dealNumber.HasValue = true))  _

                                   AndAlso (value.Value <> Me._dealNumber.Value)) Then

                         Me._dealNumber = value

                         Me.IsDirty = true

                    End If

               End If

          End If

     End Set

End Property
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 31-Jul-2009 at 11:33am
So I added a method to the BaseEntity class. We have created this class and used it in the Object Mapper as our Base Entity Class: 
public class BaseEntity : IdeaBlade.EntityModel.Entity


The new method looks like this. It would seem to be rather crude though, and I imagine I will run into some types it can't parse, but I'll cross that bridge... Anyway, does this seem reasonable? 

[BeforeSet(Order=-1.0)]

public void BeforeSettingAnyProperty(IPropertyInterceptorArgs args)

{

    var dataArgs = args as IDataEntityPropertyInterceptorArgs;

    if (dataArgs != null)

    {

        var oldValue = dataArgs.DataEntityProperty.GetValue(dataArgs.Instance, EntityVersion.Current);

        var newValue = dataArgs.Value;

        if (oldValue == null && newValue == null)

            //both are null, nothings changed, cancel the update

            dataArgs.Cancel = true;

        else if (oldValue == null && newValue != null || oldValue != null && newValue == null)

            //one is null, don't cancel

            dataArgs.Cancel = false;

        else if (oldValue.Equals(newValue))

            //they're equal, nothings changed, cancel the update

            dataArgs.Cancel = true;

    }

}


After implementing this, the Unit test above passes now.
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 31-Jul-2009 at 11:41am
P.S. Another reason to NOT set properties when the value hasn't changed is that some property changes are a trigger for other behaviors, for example, recalcs, refreshes, redraws, etc. If the property hasn't really changed, then there is no need to do any of those re-actions, some of which may be time-consuming or chatty.
For example, if you have an Order with 100 Order details. If the RecalcOrderTotals method iterates through all line items and calculates the extended price, then sums the extended price, factors in the the discount, taxes and shipping (applying the shipping price break logic) and then sets the OrderTotal, SalesTaxTotal, and ShippingTotal properties. That seems like an awful lot of overhead because a user just changed a line item quantity from 1 to 1.
Back to Top
WardBell View Drop Down
IdeaBlade
IdeaBlade
Avatar

Joined: 31-Mar-2009
Location: Emeryville, CA,
Posts: 338 		Post Options Post Options   Quote WardBell Quote  Post ReplyReply Direct Link To This Post Posted: 31-Jul-2009 at 1:38pm
I FEEL you. It's driving me crazy too. The Silverlight DataForm automatically resets every bound property to itself when in edit mode ... dirtying every object it sees. I have gotten nowhere arguing with MS against this practice; I follow their reasoning and understand why it was economical for them. They actually expect bound objects to ignore a reset-to-same-value. Today, we do not.
 
Kim is correct about the internal debate ... which continues and has received new life from your stimulating example. No promises from me today. There are some use cases we have to feel good about first; we didn't arrive at the present behavior casually. But that behavior is being re-reviewed and we are all receptive to your argument. Stay tuned.
 
p.s.: yes, you wrote an interceptor that should work 99+% of the time; if you find an exception, it will be rare and you can insert a special purpose interceptor for that one.
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 01-Aug-2009 at 8:20am
Thanks Ward. You are a wise man and I hope to see this new feature in an upcoming release, for now, I will cross my fingers on the interceptor.
Back to Top
kimj View Drop Down
IdeaBlade
IdeaBlade
Avatar

Joined: 09-May-2007
Posts: 1391 		Post Options Post Options   Quote kimj Quote  Post ReplyReply Direct Link To This Post Posted: 01-Aug-2009 at 8:48am
You'll see this in the September release :)
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 03-Aug-2009 at 8:14am
Yay!!
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 25-Sep-2009 at 12:59pm
Problem: It would seem that this fix has been half implemented. If the change is to a scalar property I.e. Order.Price (a Decimal), then the HasChanges is not set when the property is set to the existing value.
However, if the property is a reference property (i.e. Order.Customer), then changing it to the same value still sets the HasChanges. Our main class has a ton of reference properties that are populated by entities in comboboxes in the UI. This still has the problem that started this thread that non-changes in the grid still register as changes in the entity.
Am I testing this wrong? Is this working and I have it configured wrong? Thanks.

Edited by skingaby - 25-Sep-2009 at 1:00pm
Back to Top
WardBell View Drop Down
IdeaBlade
IdeaBlade
Avatar

Joined: 31-Mar-2009
Location: Emeryville, CA,
Posts: 338 		Post Options Post Options   Quote WardBell Quote  Post ReplyReply Direct Link To This Post Posted: 25-Sep-2009 at 2:00pm
Hmmm. I cannot reproduce with this little "Test" program; all the asserts pass. Maybe you can repro?
 
In the following example, a "Bar" has a parent "Bang"
 
    private void NotModifiedIfChangeToSameValueTests() {
      // Skingaby Forum post 9/25/09 regarding F1183
      Console.WriteLine("NotModifiedIfChangeToSameValueTests");

      var EM1 = new EntityManager();

      var bang = new DomainModel.Bang();
      bang.Id = 42;
      var bangName = "Bang";
      bang.Name = bangName;
      EM1.AttachEntity(bang);
      Assert.IsTrue(bang.EntityAspect.EntityState == IdeaBlade.EntityModel.EntityState.Unchanged);
 
      // setting value to same value should not change entity state
      bang.Name = bang.Name;
      Assert.IsTrue(bang.EntityAspect.EntityState == IdeaBlade.EntityModel.EntityState.Unchanged);
 
      var bar = new DomainModel.Bar();
      bar.Id = 82;
      bar.Bang_fk_Id = bang.Id;
      EM1.AttachEntity(bar);
      Assert.IsTrue(bar.EntityAspect.EntityState == IdeaBlade.EntityModel.EntityState.Unchanged);
      Assert.IsTrue(bar.Bang == bang);
 
      // setting fk id to same value should not change entity state
      bar.Bang_fk_Id = bang.Id;
      Assert.IsTrue(bar.EntityAspect.EntityState == IdeaBlade.EntityModel.EntityState.Unchanged);
 
      // setting parent entity to same entity should not change entity state
      // Skingaby post says that it DOES become modified which would be a bug
      bar.Bang = bang;
      Assert.IsTrue(bar.EntityAspect.EntityState == IdeaBlade.EntityModel.EntityState.Unchanged);
 
      Assert.IsFalse(EM1.HasChanges());
 
      HoldConsoleWindowOpen();
    }
Back to Top
pk55 View Drop Down
Senior Member
Senior Member


Joined: 22-Jul-2009
Location: CA
Posts: 105 		Post Options Post Options   Quote pk55 Quote  Post ReplyReply Direct Link To This Post Posted: 28-Sep-2009 at 10:48am

I'm seeing different behavior, not with tests from code, but with the UI just binding to scalar properties of an entity using a plain textbox (no dataform involved here).  

After loading the entity, I've validated that HasChanges is FALSE.  When I change the value of a scalar field (just a int) and tab off the field (causing a property changed event to fire) the entity is marked as having changes.  If I then change the value back to the original value and again tab off, the entity is still marked as having changes (both the EntityManager.HasChanges and the EntityAspect.HasChanges are TRUE).
 
If I issue a Save after setting the value back to it's original value, I see (using SQL profiler) that an update was issued with no columns from the entity included  in the SET (Entity Framework just sets an inline-declared dummy int variable @p to 0 so the SQL will not fail):
 
exec sp_executesql N'declare @p int
update [dbo].[myTable]
set @p = 0
where (([myKey] = @0) and ([timestamp] = @1))
select [timestamp]
from [dbo].[myTable]
where @@ROWCOUNT > 0 and [myKey] = @0',N'@0 int,@1 binary(8)',@0=123,@1=0x0000000002066FBA
 
If I change the field but then change it back before causing a property change to fire (in my case, tabbing off the field), then the entity is NOT marked as having changes.
 
In essence, "HasChanges" is really "HasBeenEditedAndAPropertyChangedEventHasBeenFired".  Is that the intented behavior?
Back to Top
WardBell View Drop Down
IdeaBlade
IdeaBlade
Avatar

Joined: 31-Mar-2009
Location: Emeryville, CA,
Posts: 338 		Post Options Post Options   Quote WardBell Quote  Post ReplyReply Direct Link To This Post Posted: 28-Sep-2009 at 11:45am

Thank you for clarifying the scenario. Let me restate it for other readers with an example.

string oldBar = foo.Bar
foo.Bar = oldBar ; // no change; foo remains in "unchanged" EntityState
foo.Bar = oldBar + "something"; // foo enters "modified" EntityState
foo.Bar = oldBar; // restored original value ... but foo is still in "modified" EntityState
And, yes, that is the intended behavior; I believe this is also the behavior of most (all?) other technologies in the same space, e.g., RIA Services and CSLA ... but I haven't confirmed.
 
Interesting name choice. I can think of others that are shorter but you've made your point :-)
 
If this really bothers you, you can filter the entities in the client-side SavingHandler; for each entity, compare each property's current value to its corresponding original value (which every DevForce entity knows); if there are no differences, you undo changes on that entity and exclude it from the list of entities to save.
 
It would take maybe 5 lines of code in total if you didn't get fancy about the field comparison / exclusion strategies. It's up to you if you think this is worthwhile.


Edited by WardBell - 28-Sep-2009 at 11:49am
Back to Top
pk55 View Drop Down
Senior Member
Senior Member


Joined: 22-Jul-2009
Location: CA
Posts: 105 		Post Options Post Options   Quote pk55 Quote  Post ReplyReply Direct Link To This Post Posted: 28-Sep-2009 at 12:15pm
Then what was the reason for the change in the Sept release?  The example you've given to restate the problem seems to look a lot like the original posters problem minus the unit tests assertion.
Back to Top
pk55 View Drop Down
Senior Member
Senior Member


Joined: 22-Jul-2009
Location: CA
Posts: 105 		Post Options Post Options   Quote pk55 Quote  Post ReplyReply Direct Link To This Post Posted: 28-Sep-2009 at 12:22pm
Ignore my last post.  I see that what you're really saying is the "current" value has changed; not the "original" value.  It becomes a user training issue. 
Back to Top
skingaby View Drop Down
DevForce MVP
DevForce MVP
Avatar

Joined: 23-Apr-2008
Location: United States
Posts: 146 		Post Options Post Options   Quote skingaby Quote  Post ReplyReply Direct Link To This Post Posted: 29-Sep-2009 at 1:08pm
Sorry Ward, you are right. My unit test was setting a Navigation property on an Entity. Unbeknownst to me, another developer had added an AfterSet interceptor to the entity that was catching that property change and changing other properties. My mistake.

However, and there's always a however isn't there, why are the AfterSet interceptors being called if the property is not actually changing?
I was handling the check in 5.2.1 in a method with this signature:
[BeforeSet(Order = -999999)] public void BeforeSettingAnyPropertyCheckToNotDirtyForNoReason(IPropertyInterceptorArgs args).
In that interceptor, I was setting args.Cancel = true if the entity property was being set to its existing value, which would abort running any AfterSet interceptors.
Now it seems like I have to put a guard in the other AfterSet's to make sure that they don't make unnecessary changes as they fire. Is that the case?
Back to Top
 Post Reply Post Reply Page  12>

Forum Jump Forum Permissions View Drop Down