spring-projects / spring-statemachine

Spring Statemachine is a framework for application developers to use state machine concepts with Spring.
1.53k stars 599 forks source link

Added support for strictly-typed states and events when loading state machine from UML. #1003

Closed colinwebber closed 2 years ago

colinwebber commented 2 years ago

Currently UmlStateMachineModelFactory only supports creating StateMachine<String, String>. This change allows the user to specify their own types, e.g. State and Event enums, resulting in StateMachine<State, Event>.

pivotal-cla commented 2 years ago

@colinwebber Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-cla commented 2 years ago

@colinwebber Thank you for signing the Contributor License Agreement!

mehmetsalgar commented 2 years ago

Thx for this PR, this was one my biggest critic for UML Integration with spring-statemachine, I am generating enumerations for States, Events on the run with XText from the UML Model in Papyrus but it was not possible until now to integrate these Enums to State Machine Factory.

If I understand your plan correctly, to use concrete Enumerations, you are planning, instead of using UmlStateMachineModelFactory a Sub class of GenericUmlStateMachineModelFactory with concrete enumerations for States, Events...

colinwebber commented 2 years ago

You're welcome, it's a privilege to contribute.

Sure, one could subclass GenericUmlStateMachineModelFactory as UmlStateMachineModelFactory now does, or simply: new GenericUmlStateMachineModelFactory<StateConcreteType, EventConcreteType>("classpath:model.uml")

I'm also working on the possibility of providing a customized parser allowing one to override or enhance, say, one aspect of the parsing. In my case I have a machine driving an embedded system with a JavaFX UI. The UI is also driven by state changes and it's quite tedious navigating in Papyrus to add entry actions etc. not to mention creating nearly identical Java classes for those screen-load actions. A custom parser, coupled with a supplied StateMachineComponentResolver, could parse a model with convention-based state names to auto-generate entry actions in order load the corresponding screen.

colinwebber commented 2 years ago

Ah... I see your point about sub-classing for enums. My concrete states/events are not enums and I made a silly assumption that it would just work. :-)

I could add a class with the following signature and built-in type converters. public class EnumUmlStateMachineModelFactory<S extends Enum<S>, E extends Enum<E>> extends GenericUmlStateMachineModelFactory<S, E>

... Also a good test or two for Enum and non-Enum types.

colinwebber commented 2 years ago

Sorry for the delay. I've made the additions as described but the tests are taking quite a while. I hope to amend the PR within the next few days.

colinwebber commented 2 years ago

Hi @mehmetsalgar

Please see the second commit: "Implemented EnumUmlStateMachineModelFactory and EnumTypeConverter to handle the common case of using custom Enumerations to represent states and events..."

All of the original test cases pass for both String and Enumeration types.

colinwebber commented 2 years ago

Hi @mehmetsalgar

What can I do to increase the chance of this PR being accepted. Would you like it broken down into a series of smaller commits? It will take a while, but I can certainly appreciate that it would reduce the risk.

mehmetsalgar commented 2 years ago

Oh, sry mate, I was not aware that you are expecting me t o approve the PR. I am not sure I have permissions to do that.

@jvalkeal can I approve this PR?

Sent from my iPhone

On 1. Oct 2021, at 08:04, colinwebber @.***> wrote:

 Hi @mehmetsalgar

What can I do to increase the chance of this PR being accepted. Would you like it broken down into a series of smaller commits? It will take a while, but I can certainly appreciate that it would reduce the risk.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

colinwebber commented 2 years ago

Hi @jvalkeal

I have been using this fork in a production system since October without any problems. Is there any chance that this will make it back into main?

Regards, Colin