zorbash / opus

A framework for pluggable business logic components
MIT License
354 stars 21 forks source link

Halting the pipeline without raising an error #1

Closed rafaels88 closed 6 years ago

rafaels88 commented 6 years ago

Let's suppose we have a pipeline to create a user.

defmodule CreateUserPipeline do
  use Opus.Pipeline

  check :valid_params?
  step :fetch_avatar
  step :encrypt_password
  step :persist_user
  step :notify_clients

  # ...

Now, imagine that I don't want to run over all this pipeline if the user already exists. I can imagine two options to solve that:

1) Creating a call?/1 function in my module to be called before .call/1. This might work, but it forces the client of the module to know that. In my opinion, it can makes sense but sometimes not (sometimes, the pipeline itself should takes care of it as a defense).

2) Should I add a check at the beginning of the pipeline to halt the pipeline before all these steps? It can work, but then Opus will return a {:error, _} tuple. This might make sense most of the times, but sometimes it is not an error. It's just that all the params are correct but the pipeline should skip all the process of creating the user and return something like, {:ok, "User already exists"}.

So, in my opinion, it would be awesome to have a way to add a check at the beginning of the pipeline and add an option to return a custom tuple in case of false, like:

check :user_exists, on_false: {:ok, "User already exists"}

Makes sense?

zorbash commented 6 years ago

Ι can see how the described use-case is meaningful. To make the options you mentioned more clear, I'll provide some examples:

call?/1

# From the side of the caller
if CreateUserPipeline.call?(user_data) do
  case CreateUserPipeline.call(user_data) do
    {:ok, _} -> "yay"
    {:error, _} -> "Do something with the error"
  end
end

This introduces a level of nesting (which can be eliminated if with is used), it is also based on an undocumented convention.

Overloading the check stage to not error when a check fails:

check :user_exists, on_false: {:ok, "User already exists"}

This might make it a bit harder to understand, especially since it's more complex to contain an on_false option to just the first check of a pipeline. If you don't contain it you have a pipeline executed partially which is probably not what you had in mind.

What about:

defmodule CreateUserPipeline do
  use Opus.Pipeline

  skip if: :user_exists? 

  check :valid_params?
  step :fetch_avatar
  step :encrypt_password
  step :persist_user
  step :notify_clients
end

With a skip macro one might declare options on when a pipeline should be skipped as a whole. And by skip we mean that an {:ok, :skipped} value is returned.

What do you think?

rafaels88 commented 6 years ago

Thinking about all the proposed solutions, the skip is the best, love it! Readable, simple and decoupled.

Do you already have an implementation for that or may I send a proposal PR for it?

zorbash commented 6 years ago

Don't have an implementation yet, feel free to send a PR, if you have any questions about the implementation ping me.

zorbash commented 6 years ago

Implemented in #2