Async Csla.Rules.BusinessRule Question

Async Csla.Rules.BusinessRule Question

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


Geoff_Manning posted on Friday, June 18, 2010

This is a two part question: 

First, can anyone explain why it's necessary for the Csla.Rules.RuleContext to set the Target to null? This is throwing a wrench in my plans because I wanted to access the object's BrokenRules collection to only run this possibly time-intensive rule if one of the other rules on this property are not already broken. Specifically, it is safe to assume that if the primary key is not in a valid state already, that it obviously isn't already assigned in the table, so it just seems like a big waste of time to check regardless.

So the second part is, assuming that there is no way for me to get a reference to the context.Target since it is null, does anyone have any ideas how to figure out if any rules on this property are already broken? I've considered the possibility of rewriting all the logic of all the other rules in this rule as well, but that is obviously very unclean.  Any help would be greatly appreciated.

Along the same lines, perhaps rule chaining would be helpful here? I don't understand it too well yet, so I'm not sure if I could make something like this work with rule chaining or not, but I'd assume at the very least that if I do that I wouldn't be able to use the DataAnnotation rules and commonrules on this property, since those are the rules that I assume would have to fire off this rule if they don't fail.

RockfordLhotka replied on Friday, June 18, 2010

The issue is that CSLA, like most of .NET, is not threadsafe. Therefore, allowing multiple threads to interact with one object causes problems. Problems which are virtually impossible to overcome (since they tie into things like data binding, which is also not threadsafe).

To prevent people from making what would be a common and horrible mistake by having a background thread on an async rule interact with the business object, the Target property is always set to null for async rules. The alternative would be chaos. Horrible, horrible chaos.

Instead what happens, is that you get to specify the list of input properties your rule requires, and those values are copied and the copies are provided to your rule. That way the rule can't accidentally alter a property value mid-stream on a background thread, thus triggering all the nasty cross-thread data binding troubles.

Anything your rule needs to change should be put in the output property dictionary, so after control returns to the UI thread, those property values can be safely updated - again without triggering the cross-thread nastiness.

Geoff_Manning replied on Friday, June 18, 2010

Ok, I can accept that, but not everything in an async rule necessarily needs to be in a backroundworker, correct?

Perhaps I can explain my case a bit better: I have an object where the primary key is a modifiable field. Because of this I store an original persisted id that comes from the database (or empty string if new object) and the current id that the consumer of the object can change. The purpose of the async rule is ensure that the current id does not exist in the database, because it must be unique.

For efficiency and functionality, before I start a background worker I want to see if it's even useful to do this check by 1. seeing if there are any rules broken on this property already (which is forcing me to make redundant checks since I can't access the BrokenRules collection) and 2. seeing if this property currently equals the "original" property in the database, upon which it obviously does exist in the database and SHOULD because it is in fact one and the same row.

Now for #2 I can obviously pass in both the current id and original id properties and that takes care of that, but for #1, what I REALLY need is a way to check if any of the rules on the current id are broken, upon which it also wouldn't exist in the db because bad data would never make it to the database (I'm thinking positive here, although most of it is duplicated with db check constraints, so is a pretty good assumption).

Because the only think I can pass in to the async rule is IPropertyInfo, I can't see any other way than using the context.Target, which is null so i can't use it.

So, if I modified the Csla source to NOT set the target to null, and left it up to me to not use the reference to the Target inside of the BackgroundWorker, would that theoretically work?

Marjon1 replied on Friday, June 18, 2010

Geoff,

I'll probably come back and edit this post later this afternoon, but what I think you are trying to do can be done with the priority mechanism that is already there on CSLA rules. You assign a priority number to a rule at the time of adding it and if any of the rules prior to that priority level are broken that rule will not run, therefore your rule does not need to be aware of what else is broken. We use this to great affect for doing a similar thing (i.e. only checking the DB for duplicates if the value has a string, isn't too long and matches a regular expression), only after all those rules at priority 0 are passed are the rules at our higher level (in our case 10) run. You would still want to do option #2 in your rule, but this will take care of issue #1.

This is demonstrated in one of the CSLA videos and I'm sure it is in the book as well (can't recall chapter right now).

I would strongly recommend changing something so fundamental to the CSLA experience, especially when it comes to threading.

Marjon

Geoff_Manning replied on Friday, June 18, 2010

Thank you so much to both of you. 

@ Rocky: Your explanation helped me a lot. 

@ Marjon: That solution worked perfectly for me.

Not a high priority to get answered, and I don't plan to change anything in the CSLA source now that I can get it to do what I need, but I'm still curious about one thing to improve my understanding of how it's affected by threading: Theoretically, if context.Target in the Execute() override had the reference to the object when IsAsync is set to true, would it not cause an issue to use the reference in the actual Execute method outside of any BackgroundWorker delegate?

RockfordLhotka replied on Saturday, June 19, 2010

That's true Goeff, it would be safe to use Target outside the async parts of the rule. But just because you understand that, and I understand that, doesn't stop the hard, cold reality that people would continually use Target inside their async code - especially given lambdas, where it isn't really apparent that one chunk of your Execute method is on a background thread.

I'm open to suggestions on how to loosen the restriction, but in some way that doesn't cause a continual and forever support burden for me. Perhaps some property you set to true like:

IFullyUnderstandTheConsequencesOfUsingTargetInAnAsyncRuleAndWillNotBotherRockyWhenIDoTheWrongThing

Big Smile

Geoff_Manning replied on Wednesday, June 23, 2010

Haha, that's hilarious. I can definitely see your reasoning, but designing a framework to prevent it from being misused is a thin line and in this case it crosses the line since there is a way to use the target without causing an issue.  Regardless, I'm finding that writing async code and understanding it is a battle regardless of this specific problem that could occur if done incorrectly. 

In this case I already have a solution so I'm not especially concerned about using Context when it is marked as Async at the moment, but perhaps a variation of what you just said could be taken seriously:

Maybe instead of calling it context.Target, it could be context.NonAsyncSafeTarget, and you could include notes about the usage of Target in the XML documentation that shows up in the intellisense, and to make it even harder for someone who doesn't know what they are doing to screw up, make then modify a property in the constructor, lets call it AllowNonAsyncSafeTarget that defaults to false unless they specifically change this.

I personally think that just the XML documentation and the AllowNonAsyncSafeTarget property would be enough for anyone, but renaming Target to NonAsyncSafeTarget would make it to where if ANYONE EVER tried to get your support on this due to not understanding why they are getting threading issues, they should be executed immediately. ;)

RockfordLhotka replied on Wednesday, June 23, 2010

Alright, you've convinced me. I'm adding this to IBusinessRule:

    /// <summary>
    /// Gets a value indicating that the Target property should
    /// be set even for an async rule (note that using Target
    /// from a background thread will cause major problems).
    /// </summary>
    bool ProvideTargetWhenAsync { get; }

Geoff_Manning replied on Wednesday, June 23, 2010

Great! Thanks, Rocky! Big Smile

ajj3085 replied on Thursday, June 24, 2010

May I also suggest that Rocky setup an email filter which forwards any emails with "async rule" and "target" to you. :-)

Copyright (c) Marimer LLC