Csla 3.8 alpha - Possible bug in CslaDataProvider

paupdb posted on Tuesday, August 25, 2009

Updated to 3.8 and found that suddenly the ObjectInstance wasn't behaving the same as it did in 3.7 - particularly in the QueryCompleted callback:

    private void QueryCompleted(object sender, EventArgs e)
    {
      IDataPortalResult eventArgs = e as IDataPortalResult;
      SetError(eventArgs.Error);
      SetObjectInstance(eventArgs.Object);
      try
      {
        OnDataChanged();
      }
      catch (Exception ex)
      {
        // Silverlight seems to throw a meaningless null ref exception
        // and during page load there are possible timing issues
        // where these events may cause non-useful exceptions
        // and this is a workaround to ignore the issues
        var o = ex;
      }
      RefreshCanOperationsValues();
      this.IsBusy = false;
    }

Basically the SetObjectInstance code no longer actually assigns a value to the ObjectInstance property - this it seems is by design because the ObjectInstance has become a dependency property.  The issue is that I have code which examines the ObjectInstance during a DataChanged event handler, and now the ObjectInstance is never set on query.

Would suggest the code in the Query_Changed be refactored as follows:
    private void QueryCompleted(object sender, EventArgs e)
    {
      IDataPortalResult eventArgs = e as IDataPortalResult;
      SetError(eventArgs.Error);
      this.ObjectInstance = eventArgs.Object;

      RefreshCanOperationsValues();
      this.IsBusy = false;
    }

The callback for the ObjectInstance dependency property will invoke SetObjectInstance, which in turn will call OnDataChanged, so no need to have that code in the refactored Query_Changed.

Rocky,
Let me know if I've got the wrong end of the stick here.

RockfordLhotka replied on Tuesday, August 25, 2009

I think you are on the right track. The only possible issue (not having looked at the code) is that I think setting ObjectInstance might clear Error, which we obviously don't want to do in this case.

RockfordLhotka replied on Wednesday, August 26, 2009

I ended up doing some pretty major surgery on CslaDataProvider - not just to address this, but to try and simplify the whole interaction process. I'm still testing my changes, but I think things not only work, but should be a lot easier to understand and maintain now.

paupdb replied on Wednesday, August 26, 2009

Sounds good Rocky - I agree that some parts of the existing CslaDataProvider are fairly convoluted (at least for my purposes).

Hopefully you were able to address some of the methods that were less friendly towards subclasses and there'll be some virtual On methods :)

RockfordLhotka replied on Thursday, August 27, 2009

This task didn't include any subclass-oriented enhancements.

I'm not entirely sure how much time I'm going to spend on that, since the ViewModel<T> class will probably be the primary focus in terms of supporting the creation of a ViewModel - which is basically what you are probably trying to do by subclassing CslaDataProvider.