Critical Bug - Rule that sets output value for field with null default fails

Critical Bug - Rule that sets output value for field with null default fails

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


Jaans posted on Monday, January 14, 2013

Hi Rocky / Jonny

I've just discovered a badly breaking bug with new code that was recently introduced and is present in the current release of CSLA 4.5.10.

I've managed to track down specifically where it happens, and what conditions cause it to trip up.

It's a new set of method in the Csla\Core\BusinessBase.cs class:
protected void LoadPropertyMarkDirty<P>(PropertyInfo<P> propertyInfo, P newValue)

The code fails with a null reference exception in the event that any of the following returns null whereupon it is assigned to variable oldValue.

When oldValue is null, the following statement throws the null reference exception:

var valuesDiffer = !(oldValue.Equals(newValue));


This wasn't an problem with the previous release of Csla 4.3.x.

The circumstances where a field's default value might be null is any reference type or even a byte[] type. The above code is only called when there is a mutator rule that calls context.OutValue( property, someValue ) in the rule. When the value of the property in question was null, then we run into the problem.

Also see attached screenshot showing a debug session.

Thanks,
Johann 

 

 

JonnyBee replied on Thursday, January 17, 2013

Hi Jaans, 

I will look into it next week when I am back from holiday.

Jaans replied on Thursday, January 17, 2013

No problem!

Thanks Jonny - I hope you're enjoying your time over here! 

JonnyBee replied on Thursday, January 17, 2013

Updated in trunk. 

http://www.lhotka.net/cslabugs/edit_bug.aspx?id=1134 

decius replied on Thursday, January 24, 2013

I ran into this too. Excellent! Thanks Johnny!

When will this make it to the stable release? Seems like a pretty critical bug IMO. All my business rules operate this way and I never bother to set default values of reference types

ngm replied on Saturday, January 26, 2013

Even little bit cleaner - get rid of PropertyInfo param in ValuesDiffer by checking:

if (typeof (IBusinessObject).IsAssignableFrom(typeof(P)))

instead of:

if (typeof (IBusinessObject).IsAssignableFrom(propertyInfo.Type))

Take care,

- ngm

 

RockfordLhotka replied on Saturday, February 02, 2013

If all goes well I expect to do the next 4.5 release this month.

Jaans replied on Tuesday, February 05, 2013

Excellent!

Thanks all.

PS: Any chance you are looking at including fixes for:

 

Jaans

 

RockfordLhotka replied on Wednesday, February 06, 2013

I believe that the first issue is resolved with the changes I've made to the data portal. Hopefully you can test and confirm once I get the prerelease online.

I am not sure I see the need for a RefreshAsync method, because there's a new/better way to work with async methods when initializing the viewmodel (imo anyway). This is done by overriding DoInitAsync and calling your business object's factory method from there.

I haven't had time to look into the CancelEdit issue. I did file it as a bug so it doesn't get lost, but I doubt I'll have time to get it into this WP8-focused release, because I'm under some time pressure to get the WP8 support finalized.

Jaans replied on Wednesday, February 06, 2013

Thanks Rocky

Re the RefreshAsync Method request - I'm sure there are many ways to skin the proverbial cat, so what ever you feel is appropriate.
We have our own base class that inherits from the CSLA viewmodel so we can just slot our RefreshAsync method there.

Some background on why we are preferring the "RefreshAsync" way might be interesting.
For us, the value of it over the InitAsync method is that it "raising" the OnRefreshing / OnRefreshed methods and it follows the paradigm of "Refresh" that the earlier version of CSLA ViewModels used to have, making our upgrade story to the new versions of CSLA easier.

The new method is a logical way for us to "refresh" the model for a viewmodel that was previously "Init"-ed. The implementation also gives  "strongly-typed" support, especially when calling the factory with multiple parameters (compared to the string based Refresh we had before). Think "Refresh" button.

Though what is nice about the InitAsync is that the actual work that is done to update the model property is encapsulated.

So, maybe it is more of a case of what suits the developers needs more and therefore up to the developer to add the method if it suits them. 

Good luck with the new release!

Copyright (c) Marimer LLC