edisonywh / gearbox Goto Github PK

Hi,

First of all, thank you for publishing this library.
I was looking for gen_server_less implementation of machinery that does not come with web dashboard part also.

I have read through the code and tests. It's exactly what I wanted.

I have some recommendation/feature requests -

• Instead of using String data type for the state. I think we could let the state be of any types. for example, a tuple to represent a coordinate on x-y plane {0.0, 0.5}, or a list, or any other types.
• Instead of using the :field to set the key to fetch the state from a map or a struct. I think we could use an anonymous function instead of string or atom key as a function to fetch the state out from data of any types.

My idea is that anything can have its own state, it could be a String, a Number, a Struct, a List.
Then we send this into mentioned above anonymous function to get the actual state out.

Let's me know what do you think of it.

Apologies for the slight oversight, but I found a "bug" where Gearbox's transition/3 (and the bang variant) will change the status in memory (returning a new copy, with updated status).

While this is great for normal, pure Elixir usage, this will actually make working with Ecto a bit difficult.

For example:

Let's say I have an item, called Issue, and I want to transition the state from unread to read`.

  def update_issue_status(%Issue{} = issue, status) do
    issue
    |> Gearbox.transition!(IssueMachine, status)
    |> Issue.status_changeset(%{status: status})
    |> Repo.update()
  end

This would fail, because Gearbox.transition/3 will change the issue's status in memory, and when it's passed to changeset (and being casted), it'll be ignored, because there are no "changes". To illustrate the step by step flow of status:

  def update_issue_status(%Issue{} = issue, status) do
    issue # status is now `unread`
    |> Gearbox.transition!(IssueMachine, status) # status is now `read`
    |> Issue.status_changeset(%{status: status}) # Because `status` is updated to `read`, we are now telling Ecto to change from `status: 'read'` to `status: 'read'`, which will generate a changeset with no changes.
    |> Repo.update() # This will continue executing because changeset without changes is technically valid.
 
  end

I'm still thinking about how to deal with this, discussions are very welcomed. One possible way is to have a validate_transition API that just does validation without changing the structure in place, but that feels a little weird as well, because this may change the usage/mission of Gearbox (previously was intended to use it everywhere but your schema, but with the change above, it will now require users to use Gearbox at the schema levels. I'm not sure if this is the correct direction to take.

Temporary Fix:

You can temporarily circumvent this bug by making sure when you feed a structure into changeset, you feed the old version of it, so the Changeset will be able to generate a changeset.

Throwing out ideas here, but there's no real reason why Gearbox needs to be 'used', perhaps we could refactor and achieve the same by just using @behaviors, so all state machines need to adhere to a certain sets of behaviors?

As described in the README, the first argument can be a struct or a non-struct, but the function typespec accepts only a struct, making type check warn a contract break.