Hi
I’ve been studying the BusinessListBase.Child_PropertyChanged method and I think there may be a BUG.
The BindingList.RaiseListChangedEvents property is used to stop raising ListChanged events in some circumstances (i.e. BusinessListBase.Removeitem). The Child_PropertyChanged is supposed to substitute the BindingList functionality when a list item raises the PropertyChanged event.
If I’m not wrong, if an inheritor of BusinessListBase uses Remoting then the Child_PropertyChanged method won’t take in account the RaiseListChangedEvents property and so the expected behaviour won’t be like without using Remoting. Am I right?
To solve this issue the Child_PropertyChanged method should take in account the property BindingList.RaiseListChangedEvents, to avoid raising undesired events.
Other difference I see is when a list item raises its event PropertyChanged with the PropertyName argument equal to an empty string (PropertyName = “”). With the CSLA code the BusinessListBase always will raise a ListChanged event with ListChangedType = ItemChanged. Instead, when we are not using Remoting the same object would raise the ListChanged event with ListChangedType = Reset.
To solve this problem the only thing I see is to avoid using the BindingList behaviour and use only the CSLA implementation. To get it, the CSLA would need:
- Set the BindingList.RaiseListChangedEvents property to False. It may do this in the BusinessListBase constructor.
- Provide its own BusinessListBase.RaiseListChangedEvents property to be able to control the raising of events.
- Add handlers to the PropertyChanged event of each child object when they are added to the list. This may be done in the InsertItem method.
- Remove handlers from the PropertyChanged event of child objects when they are removed from the list. This may be done in the RemoveItem method.
I’m evaluating doing these changes to the framework and I would be very grateful if you give me your opinion.
Thanks in advance.
Benjamin Moles
Do you have unit tests showing these problems?
If not, can you create them? You don't need to use remoting btw, you can just use Clone() to get the same effect as remoting.
http://forums.lhotka.net/forums/thread/4078.aspx
This may be related to the older thread above?
Chris
Hi Chris,
Thanks for your comment, but I have already seen those posts. The problem that I'm talking about is related to them but it's a different issue.
Thanks again.
Benjamin
Hi
Here you have a small testing application that can be used to compare the behaviour of an example of business object.
With this tool I’ve confirmed what I suspected. There are differences in the objects behaviour when they are serialized and when they’re not.
I’ve coded some tests that you can check to see it.
To see what happens with each test you can see the member “DoTest” in the following classes:
- ListChanged_Test1
- ListChanged_Test2
- RaiseListChangedEvents_Test1
In that method you’ll find the sequence of steps followed to do the test.
You also can use the manual test to do anything you want to the sample SalesOrder objects (and its child objects also).
Hi Rocky
Have you seen my sample application?
Ben
Yes, but I’m under some tight deadlines and probably won’t
have time to really look at this for a couple weeks.
Rocky
From: ben
[mailto:cslanet@lhotka.net]
Sent: Monday, September 24, 2007 10:09 AM
To: rocky@lhotka.net
Subject: Re: [CSLA .NET] BusinessListBase.Child_PropertyChanged
discussion
Hi Rocky
Have you seen my sample application?
Ben
I’ve got a workarund to resolve both problems:
First problem:
Description:
BindingList (Of T) raises OnListChanged (Reset) after one item raises PropertyChanged event with PropertyName = String.Empty (OnUnknownPropertyChanged).
Solution:
Modify “BindableBase.vb”:
...
<Serializable()> _
Public MustInherit Class BindableBase
...
''' <summary>
''' Allow to get the PropertyChanged handler of any obsever
''' that implement it
''' </summary>
''' <remarks>
''' <para>BindingList (Of T) raises OnListChanged (Reset)
''' after one item raises PropertyChanged event with
''' PropertyName = String.Empty (OnUnknownPropertyChanged).</para>
'''
''' <para>This is a problem because of one change in one item
''' makes a hole collection to be refetched in any grid or similar.</para>
'''
''' <para>All inheritors of BindingList should implemen
''' this interface in order to allow us to replace the
''' default handler of BindingList for other smarter handler.</para>
''' </remarks>
Friend Interface IPropertyChangedObserver
Function PropertyChangedHandler() As PropertyChangedEventHandler
End Interface
...
Public Custom Event PropertyChanged As PropertyChangedEventHandler _
Implements INotifyPropertyChanged.PropertyChanged
AddHandler(ByVal value As PropertyChangedEventHandler)
' Workaround to avoid using BindableList(Of T) implementation.
If value.Method.DeclaringType.Name = "BindingList`1" Then
Dim targetObserver As IPropertyChangedObserver
targetObserver = TryCast(value.Target, IPropertyChangedObserver)
If targetObserver IsNot Nothing Then
value = targetObserver.PropertyChangedHandler
End If
End If
If value.Method.IsPublic AndAlso _
(value.Method.DeclaringType.IsSerializable OrElse _
value.Method.IsStatic) Then
mSerializableHandlers = _
DirectCast(System.Delegate.Combine( _
mSerializableHandlers, value), PropertyChangedEventHandler)
Else
mNonSerializableHandlers = _
DirectCast(System.Delegate.Combine( _
mNonSerializableHandlers, value), PropertyChangedEventHandler)
End If
End AddHandler
...
End Event
...
End Class
...
Modify “BusinessListBase.vb”:
...
<Serializable()> _
Public MustInherit Class BusinessListBase( _
Of T As BusinessListBase(Of T, C), C As {Core.IEditableBusinessObject})
Inherits Core.ExtendedBindingList(Of C)
Implements BindableBase.IPropertyChangedObserver
Implements Core.IEditableCollection
Implements Core.IUndoableObject
Implements ICloneable
Implements ISavable
Implements IParent
...
#Region " Cascade Child events "
Private Function MyPropertyChangedHandler() As PropertyChangedEventHandler _
Implements BindableBase.IPropertyChangedObserver.PropertyChangedHandler
Return AddressOf Me.Child_PropertyChanged
End Function
...
#End Region
...
End Class
...
Modify “EditableRootListBase.vb”:
...
<Serializable()> _
Public MustInherit Class EditableRootListBase (Of T As {Core.IEditableBusinessObject, Core.IUndoableObject, Core.ISavable})
Inherits Core.ExtendedBindingList(Of T)
Implements Csla.Core.BindableBase.IPropertyChangedObserver
Implements Core.IParent
...
#Region " Cascade Child events "
Private Function MyPropertyChangedHandler() As PropertyChangedEventHandler _
Implements Csla.Core.BindableBase.IPropertyChangedObserver.PropertyChangedHandler
Return AddressOf Me.Child_PropertyChanged
End Function
...
#End Region
...
End Class
...
Second problem:
Description:
BusinessListBase.Child_PropertyChanged and EditableRootListBase.Child_PropertyChanged do not take in account property “RaiseListChangedEvents”. To work as BindingList(Of T).Child_PropertyChanged should do it.
Solution:
Modify “BusinessListBase.vb”:
...
<Serializable()> _
Public MustInherit Class BusinessListBase( _
Of T As BusinessListBase(Of T, C), C As {Core.IEditableBusinessObject})
Inherits Core.ExtendedBindingList(Of C)
...
#Region " Cascade Child events "
...
Private Sub Child_PropertyChanged(ByVal sender As Object, _
ByVal e As System.ComponentModel.PropertyChangedEventArgs)
If Me.RaiseListChangedEvents Then ' CSLA modification
For index As Integer = 0 To Count - 1
If ReferenceEquals(Me(index), sender) Then
Dim descriptor As PropertyDescriptor = GetPropertyDescriptor(e.PropertyName)
If descriptor IsNot Nothing Then
OnListChanged(New System.ComponentModel.ListChangedEventArgs( _
ComponentModel.ListChangedType.ItemChanged, index, descriptor))
Else
OnListChanged(New System.ComponentModel.ListChangedEventArgs( _
ComponentModel.ListChangedType.ItemChanged, index))
End If
Exit For
End If
Next
End If
End Sub
...
#End Region
...
End Class
...
Modify “EditableRootListBase.vb”:
...
<Serializable()> _
Public MustInherit Class EditableRootListBase (Of T As {Core.IEditableBusinessObject, Core.IUndoableObject, Core.ISavable})
Inherits Core.ExtendedBindingList(Of T)
Implements Csla.Core.BindableBase.IPropertyChangedObserver
Implements Core.IParent
...
#Region " Cascade Child events "
...
Private Sub Child_PropertyChanged(ByVal sender As Object, _
ByVal e As System.ComponentModel.PropertyChangedEventArgs)
If Me.RaiseListChangedEvents Then ' CSLA modification
For index As Integer = 0 To Count - 1
If ReferenceEquals(Me(index), sender) Then
Dim descriptor As PropertyDescriptor = GetPropertyDescriptor(e.PropertyName)
If descriptor IsNot Nothing Then
OnListChanged(New System.ComponentModel.ListChangedEventArgs( _
ComponentModel.ListChangedType.ItemChanged, index, descriptor))
Else
OnListChanged(New System.ComponentModel.ListChangedEventArgs( _
ComponentModel.ListChangedType.ItemChanged, index))
End If
Exit For
End If
Next
End If
End Sub
...
#End Region
...
End Class
...
The fact that Child_PropertyChanged does not take RaiseListChangedEvents into account does create problems. (Just wasted about 6 hours or so trying to figure this out.) I had validation rules that were triggered by ListChanged events to make sure the child items were appropriate for the list, and since the parent entity had to be taken into account, the validation rule had to go into the parent. Well, all was working fine, until I started using the WCF DataPortal. Sure, enough, tracked it down to MarkOld triggering the list changed event, and I ended up getting database timeout errors due to locks. I found this posting, and was able to replicate the problem by using the Clone method, as Rocky mentioned.
Anyway, I agree that the codebase should be updated to take RaiseListChangedEvents into account. However, another workaround, that seems to be working, is to remove the even handler prior to calling Update() on my child collection, and then adding it immediately after. I'd rather not modify our CSLA codebase, as I'm afraid someone else may replace the CSLA assembly without my knowledge, and they may not have the "fix".
I too vote for the suggested fix to go into the codebase.
William
Copyright (c) Marimer LLC