CslaDataProvider for Silverlight bug

CslaDataProvider for Silverlight bug

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


cds posted on Sunday, May 03, 2009

Hi

I've upgraded to 3.6.2 of CslaLight having previously used 3.6.1. Some code in the CslaDataProvider has changed which has now broken my application.

My specific problem relates to the CslaDataProvider::QueryCompleted method.

In 3.6.1 the ObjectInstance property was set before the Error property, but in 3.6.2 it happens that the Error property is set before the ObjectInstance property is set. Here's the code:

3.6.2:

private void QueryCompleted(object sender, EventArgs e)

{

IDataPortalResult eventArgs = e as IDataPortalResult;

this.Error = eventArgs.Error;

this.ObjectInstance = eventArgs.Object;

RefreshCanOperationsValues();

this.IsBusy = false;

}

 3.6.1:

 

private void QueryCompleted(object sender, EventArgs e)

{

IDataPortalResult eventArgs = e as IDataPortalResult;

if (_manageObjectLifetime && eventArgs.Object != null && eventArgs.Error == null)

{

this.ObjectInstance = eventArgs.Object;

this.Error = eventArgs.Error;

}

else if (eventArgs.Error != null)

{

this.Error = eventArgs.Error;

}

RefreshCanOperationsValues();

this.IsBusy = false;

}

 

The reason this matters is that the Error property also sets IsBusy to false. And the reason that matters is the crux of the issue...

In my system I want to know when the CslaDataProvider has completed an async Refresh operation. There doesn't seem to be any way to detect that. So, back in 3.6.1 I sub-classed the CslaDataProvider and then overrode the OnPropertyChanged method to listen for changes to the IsBusy property and fired my own method when IsBusy went from true to false. Back in 3.6.1 this worked, because the ObjectInstance property had already been set by the time IsBusy was set to false. This all seems like a hack but it was the only way I could get it to work, short of modifying the CslaDataProvider source myself (which I'm loathe to do).

So, what I'm really driving at here is why the various OnXXX methods in the CslaDataProvider aren't virtual. If they were I could just override them and handle the event myself.

So, Rocky - can you please advise whether you consider the order of setting the Error property and the ObjectInstance property is a bug, and whether you can introduce virtual methods into this class?

Alternatively what's the correct way of solving the problem of wanting to be advised when the Refresh has completed? I'm trying to build some generic code for a bunch of maintenance screens and I'd rather be able to subclass the CslaDataProvider rather than have to hook up and detach events.

Thanks,

Craig

(And here's my subclass code so you can see what I'm trying to do)

 

public class MyDataProvider : CslaDataProvider

{

private bool _isBusy;

public bool HasData

{

get { return ObjectInstance != null; }

}

public bool HasNoData

{

get { return ObjectInstance == null; }

}

protected override void OnPropertyChanged(PropertyChangedEventArgs e)

{

switch (e.PropertyName)

{

case "Data":

base.OnPropertyChanged(new PropertyChangedEventArgs("HasData"));

base.OnPropertyChanged(new PropertyChangedEventArgs("HasNoData"));

break;

case "Error":

if (Error != null)

{

Debug.WriteLine(Error);

}

break;

case "IsBusy":

var newBusy = IsBusy;

if (!newBusy && _isBusy)

OnOperationComplete();

_isBusy = newBusy;

break;

}

base.OnPropertyChanged(e);

}

private Action _operationCompleteAction;

 

/// <summary>

/// Registers a one-time action to execute when IsBusy changes back to false - i.e. when an

/// operation completes.

/// </summary>

/// <param name="action">The action.</param>

public void WhenComplete(Action action)

{

_operationCompleteAction = action;

}

private void OnOperationComplete()

{

if (_operationCompleteAction != null)

{

try

{

_operationCompleteAction.Invoke();

}

catch (Exception ex)

{

Debug.WriteLine(ex);

}

_operationCompleteAction = null;

}

}

}

 

 

RockfordLhotka replied on Monday, May 04, 2009

The change to CslaDataProvider was necessary to fix a rather serious bug in the way the Error and Data properties were being set during data load (query or refresh). The problem was that each value, as it was set, would raise a PropertyChanged and DataChanged event, and this would cause the UI to double-bind.

The most notable example of this is when there was an error, your error handling code (such as the new ErrorDialog control) would execute twice - even though there was only one query. It was a mess.

So the 3.6.2 code is much more careful about how this works, so the DataChanged event is raised just one time, even though both the Error and Data properties may have been set.

Ultimately, the way you know that a query operation is complete is by handling the DataChanged event. This is the same in Silverlight as in WPF. I would think you'd be able to handle DataChanged to accomplish your goal?

cds replied on Monday, May 04, 2009

Hi Rocky

Thanks for the reply. I will give this a go but from a quick look at the CslaDataProvider code I don't think this is going to help me - the DataChanged event is raised by calling OnDataChanged(), and this is called both by the Error property being set and by the ObjectInstance property being set.

Won't this cause the DataChanged event to be raised twice, with the first one raised before the ObjectInstance is actually set?

Finally, are there reasons that there are so few virtual methods in this class? As I said in my first post, I wanted to inherit from it but the lack of virtual methods makes this difficult. I guess, if all else fails, I can take the code and modify it to suit my purposes...

Thanks,

Craig

RockfordLhotka replied on Monday, May 04, 2009

The current code (maybe I’m thinking of 3.6.3?) calls OnDataChanged() once per query. That’s the bug fix I referred to in my previous post.

 

There are a couple reasons there are few virtual methods.

 

First, the intent is to emulate the WPF control as much as possible, and it has few virtual methods.

 

Second, as a framework designer, it is always best to not make methods virtual unless there’s a specific scenario that requires such a move. This is because virtual methods and inheritance are an incredibly tight form of coupling, and the more virtual methods in a class, the more fragile the overall system. The more methods I make virtual, the better the odds I’ll break your code as I enhance/fix/change the framework over time.

 

So I do make methods virtual, but not casually, and certainly not by default. If there are specific members that should be virtual to meet a scenario, please call them to my attention – we can discuss the scenario(s) and decide on a good way to solve them.

 

Rocky

 

swegele replied on Saturday, May 16, 2009

Rocky,

Your right...that fix will be in 3.6.3.  I am getting the behavior you are mentioning in 3.6.2 of DataChanged firing many times for one query.

Sean

swegele replied on Saturday, May 16, 2009

I got the latest source for ErrorDialog and CslaDataProvider and recompiled...

still getting the DataChanged event firing 16 times for one query (8 of those after the object is actually returned)

I confirmed that my factory method is only being called once.

Sean

 

swegele replied on Saturday, May 16, 2009

Ugh it helps if one does this:

m_Provider.DataChanged += CslaDataProvider_DataChanged;

instead of this:

m_Provider.PropertyChanged += CslaDataProvider_DataChanged;

Tried to delete previous post but I didn't get it fast enough...sorry.  It works fine now!

Sean

Copyright (c) Marimer LLC