nitram509 / lib-bpmn-engine

A BPMN engine, meant to be embedded in Go applications with minimal hurdles, and a pleasant developer experience using it. This approach can increase transparency for non-developers.
https://nitram509.github.io/lib-bpmn-engine/
MIT License
287 stars 74 forks source link

Even more flexible task handlers #161

Open jonathanbp opened 1 year ago

jonathanbp commented 1 year ago

I have a use case where the logic that determines how to execute a given task depends on how that task is configured, i.e. which attributes it has. Would you consider adding a method on the newTaskHandlerCommand type which takes a predicate func to determine if a given handler can be invoked. It would look something like:

engine.NewTaskHandler().Matching(func(t *BPMN20.TaskElement) bool { <some logic goes here> }).Handler(myHandler);

I realise there was some work done in #57 and #58 to make the handler assignment more dynamic (with task types) but for my case there wouldn't be a specific type to distinguish between tasks rather a couple of other extension properties.

jinjaghost commented 1 year ago

What if no logic match a handler ? Do you imagine a default handler of last-resort ?

nitram509 commented 1 year ago

@jonathanbp thanks for proposing that feature ... just another detail question, on that:

nitram509 commented 1 year ago

@jinjaghost thanks for chiming in ... IMHO, the "default" handler is a concept, not present yet - and also would help for the other handlers as well. I would suggest not to couple this issue with the general problem , but rather consider them separate feature. What do you think?

nitram509 commented 1 year ago

@jonathanbp May I ask, is this just a feature request, or do you consider participating the Hacktoberfest and prepare a PR as well?

jinjaghost commented 1 year ago

@jinjaghost thanks for chiming in ... IMHO, the "default" handler is a concept, not present yet - and also would help for the other handlers as well. I would suggest not to couple this issue with the general problem , but rather consider them separate feature. What do you think?

This issue is about adding some logic to task handler selection, i think that the "default" handler concept is part of it because in any case, it will be necessary to ultimately select a handler.

As in #149, jsonlogic would be a perfect fit to represent that logic.

jonathanbp commented 1 year ago

In my use-case the task element would probably have enough attributes to select the handler, but having a predicate would mean the decision as to which handler is selected can be controlled in the handler itself at runtime. I'd be happy to do a PR - I just wanted to check whether the idea meshed well with the other approaches or if there were other reasons not to do this or other and better approaches.

jinjaghost commented 1 year ago

If i am not missing something, why don't you run the decision logic from the main handler and call the right handler from there ?

jonathanbp commented 1 year ago

That is what I'm doing at the moment, but I thought this would a more appropriate approach and a nice addition to the library.

nitram509 commented 1 year ago

@jinjaghost thanks for reminding me about #149 (and sorry for my late reply).

Since this issue and #149 do address the same need, of providing more freedom/flexibility in implementing handlers, I do consider this one more simple. A generic handler function can implement anything. Adding jsonlogic on top I don't have seen so often and don't consider that much of a benefit.

About the default handler ... I do consider this feature would already address this and helps us to get default handlers implemented. @jonathanbp would you add this logic as well, please (looks like a quick win to me)?

Happy to review any PR @jonathanbp and also adding "hacktoberfest-accepted" label with pleasure, in case you're participating.

Some implementation hints...