Feature/add cancellation token

[luizfbicalho]

Add cancellation token as an alternative to timeout

closes #3911

[luizfbicalho]

I only checked the first four files (SessionManager, StateManager and ViewModel). It seems all touched files have formatting issues. Because of that I abort the review until you recheck all files. The correct intendation is set by the editorconfig so this should be fixable by auto formatting.

I used the dotnet format csla.test.sln to resolve this issue, but there are too many changes across the solution, more than 300 files need change.

I just updated those files changed in this PR, we can discuss later something to improve this.

[StefanOssendorf]

I only checked the first four files (SessionManager, StateManager and ViewModel). It seems all touched files have formatting issues. Because of that I abort the review until you recheck all files. The correct intendation is set by the editorconfig so this should be fixable by auto formatting.

I used the dotnet format csla.test.sln to resolve this issue, but there are too many changes across the solution, more than 300 files need change.

I just updated those files changed in this PR, we can discuss later something to improve this.

Yeah. Fixing the whole solution is out of scope. I'm only ever inspecting the files you changed. :)

[rockfordlhotka]

Regarding the change of timeout in async rule execution to use a CancelationToken - does this provide value?

Right now it appears to be a different way to achieve the same result, and so the change brings on (probably minor) risk for no gain. On the other hand, if we think it is paving the way for a future where canceling async rule execution could be done via code, then perhaps there's value?

[luizfbicalho]

Regarding the change of timeout in async rule execution to use a CancelationToken - does this provide value?

Right now it appears to be a different way to achieve the same result, and so the change brings on (probably minor) risk for no gain. On the other hand, if we think it is paving the way for a future where canceling async rule execution could be done via code, then perhaps there's value?

Timeout I think that is much more limited, I have an example one project that one of my tasks didn't pass correctly the cancellation token, and this was a problem in te IIS reset, this task couldn't be cancelled in time, and used to break the IIS reset, causing 503

[rockfordlhotka]

Regarding the change of timeout in async rule execution to use a CancelationToken - does this provide value? Right now it appears to be a different way to achieve the same result, and so the change brings on (probably minor) risk for no gain. On the other hand, if we think it is paving the way for a future where canceling async rule execution could be done via code, then perhaps there's value?

Timeout I think that is much more limited, I have an example one project that one of my tasks didn't pass correctly the cancellation token, and this was a problem in te IIS reset, this task couldn't be cancelled in time, and used to break the IIS reset, causing 503

What I am getting at though, is that the code doesn't allow someone to cancel the operation because the cancelation token is created off the timeout and isn't exposed higher in the API.

I think we need to consider what someone would want. If they call CheckRulesAsync and cancel it, I imagine they'd want to cancel all executing rules? Sync and async. Though we don't have a way to cancel sync rules, so it would be async rules only.

Inside an async rule, presumably we pass in the cancelation token (via context?) so a rule author could (if possible) cancel what they are doing.

[StefanOssendorf]

I agree with Rocky here. We should remove the business rules changes here and make a proper design/concept for how we want to be able to use a cancellation token in the context of rules. As rocky said. I as a user would expect to be able to cancel the current running rules and not only to provide a waiting timeout.

[luizfbicalho]

I agree with Rocky here. We should remove the business rules changes here and make a proper design/concept for how we want to be able to use a cancellation token in the context of rules. As rocky said. I as a user would expect to be able to cancel the current running rules and not only to provide a waiting timeout.

OK, which one do you want to rollback?

[StefanOssendorf]

I agree with Rocky here. We should remove the business rules changes here and make a proper design/concept for how we want to be able to use a cancellation token in the context of rules. As rocky said. I as a user would expect to be able to cancel the current running rules and not only to provide a waiting timeout.

OK, which one do you want to rollback?

If possible this whole file Source/Csla/Rules/BusinessRules.cs. But if that's not easy only the public method accepting the ct would be okay for me.

[luizfbicalho]

I agree with Rocky here. We should remove the business rules changes here and make a proper design/concept for how we want to be able to use a cancellation token in the context of rules. As rocky said. I as a user would expect to be able to cancel the current running rules and not only to provide a waiting timeout.

OK, which one do you want to rollback?

If possible this whole file Source/Csla/Rules/BusinessRules.cs. But if that's not easy only the public method accepting the ct would be okay for me.

I changed it back to the original, and already solved the format problems, the tests I'll do in the weekend

[StefanOssendorf]

Resolve the confligt and we are ready to merge :)

[luizfbicalho]

Resolve the confligt and we are ready to merge :)

Are you sure? I'm still making some more tests

[StefanOssendorf]

Resolve the confligt and we are ready to merge :)

Are you sure? I'm still making some more tests

I won't say no to more useful tests :)

[luizfbicalho]

Resolve the confligt and we are ready to merge :)

Are you sure? I'm still making some more tests

I won't say no to more useful tests :)

I added some more. Please check if it's all ok

[StefanOssendorf]

Please fix first the conflicts 😅

[luizfbicalho]

Please fix first the conflicts 😅

I'll fix tonight