Rules called multiple times after 4.5 upgrade

Rules called multiple times after 4.5 upgrade

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


mparsin posted on Wednesday, February 13, 2013

 

Hi!

Our Silverlight 5 application's BOL has a couple of business objects with pretty deep hierarchy.

Such as

ProductEdit => SectionList => SectionEdit => FieldList => StepList => StepEdit =>SecuritySettingsList=>etc.

Some validation rules defined on child level but other like “NoDuplicateFieldName” defined on ProductEdit (EditableRoot) level.

Currently we call rule re-check of these rules from OnChildChanged override.

Is that the right strategy? 
Our problem is that after upgrading to CSLA v4.5 we found that our ViewModel’s ModelPropertyChanged method is getting called hundreds of times when say one of the stepEdit objects changed.  I guess that caused by changes in BusinessBase OnChildChanged (MetaPropertyHasChanged)

Our current solution to the problem is to override MetaPropertyHasChanged in our custom BusinesBase class:

        protected override void MetaPropertyHasChanged(string name)

        {

            // TODO: find a better way to avoid multiple business rule checks.

        }

 

Thanks in advance,

Maxim

 

skagen00 replied on Wednesday, February 13, 2013

OnChildChanged gets called many times for some of the IsDirty/etc properties of the child.  Used to, it didn't.

We had to introduce logic to ignore some of those property changes in OnChildChanged (had the same issue as you I believe).

 

rfcdejong replied on Friday, February 15, 2013

Your hierarchy is not that deep, compared to ours. There is a performance hit when you reach a certain dept in the hierarchy. Because of that our businessobject abstraction layer doesn't notify the parent of any metadata property change. It duplicates every dept and hundreds of times became a MILION!! times in our hierarchy (using ANTS performance profiler looking at ChildChanged when typing in a value in the lowest object in the chain)

We did something like this.

        private List<string> _metaProperties = new List<string> { "IsNew", "IsDeleted", "IsBusy", "IsSelfBusy", "IsDirty", "IsSelfDirty", "IsSavable", "IsValid", "IsSelfValid" };

        protected virtual bool IsMetaProperty(string propertyName)
        {
            return _metaProperties.Contains(propertyName);
        }

        protected override void OnChildChanged(ChildChangedEventArgs e)
        {
            if (e.PropertyChangedArgs == null || !IsMetaProperty(e.PropertyChangedArgs.PropertyName))
            {
                base.OnChildChanged(e);
            }
        }

mparsin replied on Saturday, February 16, 2013

skagen00, rfcdejong 

Thank you very much for your answers. We're doing something very similar with our code. 

Just wanted to make sure we're on the right track. 

 

Thank you,

Maxim

RockfordLhotka replied on Saturday, February 16, 2013

Do you guys think this is a change that should be made to the core framework? That we should collectively come up with a set of rules regarding which child change events should and shouldn't propagate up through the object graph?

ngm replied on Sunday, February 17, 2013

That suppressing code up there will not notify about meta property changes.

From my personal experience, there are usually two scenarios going on here.

One where you've got pretty reasonable depth of graph and where you want to bind onto parent object's meta properties such as IsDirty or IsSavable. Here it's more than expected to have notifications behind those properties synced with the children.

However, with a lot of levels in the graph this synchronization can easily become perf issue, if there are slow handlers on the way. The sole invocation through the graph should be extremely fast. That being said, I would always recommend reconsidering every model with huge graph, even though I designed those as well on multiple occasions.

It would be probably wise to start looking at some opt-in/out subscription model or more intelligent notifications such as reducing redundant ones and so.

This is not an easy answer and it comes to the fact that CSLA strives to be very rich framework for modern interactive interfaces.

- ngm

 

rfcdejong replied on Tuesday, March 12, 2013

In a small graph it isn't really a problem.

Anyway, most of the meta properties only have to be notified being changed just once right after the originating property self has changed.

To make the IsBusy work again while eating up all meta property changes we 
have a workaround for IsBusy:
Whenever there is a OnPropertyChanging a custom class called "PropertyManager" then knows the property is busy and on the OnPropertyChanged it marks the property as not busy. The root BO can ask his own PropertyManager if any property is busy. 

For more detail, lazy loaded properties are implemented this way:

                if (!IsPropertyLoading(SoortPandKeuzesProperty) && !(IsPropertyLoaded(SoortPandKeuzesProperty)))
                {
                    MarkPropertyLoading(SoortPandKeuzesProperty);
                    LookupList<AandSoortPand?>.GetList(LookupType.AandSoortPand, true, null, (s1, e1) =>
                    {
                        if (e1.Error != null)
                            e1.Error.ReThrow();

                        LoadProperty(SoortPandKeuzesProperty, e1.Object);
                        MarkPropertyLoaded(SoortPandKeuzesProperty);
                        OnPropertyChanged(SoortPandKeuzesProperty.Name);
                    });
                }
                return GetProperty(SoortPandKeuzesProperty);

Instead of using FieldManager directly we have those IsPropertyLoading and IsPropertyLoaded methods. The MarkPropertyLoading and MarkPropertyLoaded are two custom methods for marking the property being loaded.
We also have a method to unload a property.

There has to be some sort of notification mechanism, you cannot simply eat up MetaDataProperty notifications in the OnChildChange. The BO's higher in the hierarchi won't notify a OnPropertyChanged of meta data properties.

ngm replied on Tuesday, March 12, 2013

rfcdejong

There has to be some sort of notification mechanism, you cannot simply eat up MetaDataProperty notifications in the OnChildChange. The BO's higher in the hierarchi won't notify a OnPropertyChanged of meta data properties.

I agree. OnChildChange bubbling is very useful when syncing objects within a graph. CSLA does exactly this when maintaining compound properties such as IsDirty, IsSavable or IsValid. 

rfcdejong

Anyway, most of the meta properties only have to be notified being changed just once right after the originating property self has changed.

This is correct. If you take a loot at IsDirty, the moment first property changes it's going to be dirty and stay like that for any subsequent property changes (not taking into account n-level undo).

I don't have CSLA source in front of me right now, but if I remember correctly, IsDirty property change is raised only if the previous state wasn't dirty, preventing wasteful notifications. However, any time child changes any of its properties, it will raise parent's dirty change (indirectly through OnChildChanged) no matter if parent's IsDirty was already set. This is where huge waste occurs. It keeps multiplying if this parent is child as well, it will do the same thing up to the root with addition of notifying parent of both its IsDirty property change and plus ChildChanged because of its child that triggered all this.

- ngm

 

Turntwo replied on Monday, August 12, 2013

I'm experiencing this too in one part of my application that can have decent sized child lists, with small lists of their own:

Root/ChildList/ChildList2

ChildList can have hundreds, maybe a couple thousand records although that is rare.  The case I'm encountering the most issues in the 200-500 range is relatively common for some clients.  ChildList2 has 2-10 entries per ChildList entry. 

Binding a Save button (WPF) to the Root.IsSavable (or IsValid) is causing issues as IsValid is getting called several hundred times per property change, and that number seems to grow as more properties are changed (haven't tracked down the cause of that yet, as I'm more focused on avoiding the scenario entirely). 

I think the real performance hit comes because each of those IsValid calls dynamically runs through the hierarchy checking for validity. 

Rather than eat the propertychanged events and ignore them, it might be better to manage the IsValid, etc properties that propagate calls to all the child objects to somehow cache the last value, and use the notifications to check only the state of the object that changed - e.g. if IsValid for a child went from true to false, update the root IsValid to false, and if it went from false to true - check the graph until finding a single false value, or update to true if everything is valid.  Then subsequent calls to the IsValid getter could use the cached value.  Currently with a list of about 500 entries one property change triggered 48 million calls to FieldData.IsValid. 

Haven't yet looked into how tricky that would be to manage - but given the notifications are going out and spreading through the object graph, it seems like the information needed might already be in place - just need to implement the logic to cache the values, instead of checking every field in the FieldManager for the entire graph every time. 

My short term solution is just to bypass the binding and provide a slightly less user friendly experience for this particular view (which I really want to re-write to not be grid/list based anyway - as when we originally designed it the expectation was for lists of 10-50 children, and the ChildList2 didn't exist...  but no time for that now). 

Copyright (c) Marimer LLC