Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class-based transactions #52

Closed
timriley opened this issue Feb 27, 2017 · 6 comments
Closed

Class-based transactions #52

timriley opened this issue Feb 27, 2017 · 6 comments

Comments

@timriley
Copy link
Member

After working with dry-transaction for a while, and across a number of apps, I see a few issues in its design:

  • The Dry.Transaction constructor creates objects with an ancestry that app authors have no control over, so there’s no way to add behaviour to all an app’s transactions (this is important in our apps at Icelab, because we use a little #to_queue/#call_from_queue protocol for running operations in background jobs.
  • Also because of this, there’s no place to put transaction-specific logic for adjusting values in-between a transaction’s steps, which can be useful for tying together steps with slightly differing input/output expectations.
  • The constructor returning anonymous objects means we need some sort of harness for building and capturing transactions (i.e. registering in our container) in each of our apps.
  • Having to build and register transactions like this means that their definition ends up sitting far apart from other parts of the codebase that they relate to, and their registration ends up complicating the app’s boot process.

I think we can solve all of these by moving to class-based transactions.

After thinking about it for a while, our transactions are really just a specialised case for auto-injection: a transaction collects a series of operation objects (mapped to container identifiers), and calls them in sequence.

So instead of building a transaction like this:

my_trans = Dry.Transaction(MyContainer) do
  step :one, "operations.one"
  step :two, "operations.two"
end

We could build one like this:

class MyTransaction
  # Similar mixin to Dry::AutoInject
  include Dry::Transaction(MyContainer)
  
  step :one, "operations.one"
  step :two, "operations.two"
end

my_trans = MyTransaction.new

This would allow us to inject replacement dependencies to make testing easier:

# Provide an explicit operation for step one
MyTransaction.new(one: different_operation)

For larger apps, this’d let us provide a per-container transaction mixin for easy reuse, too:

# In container setup
module Main
  Transaction = Dry::Transaction(Main::Container)
end

# Elsewhere
module Main
  class AnotherTransaction
    include Main::Transaction
    
    step :one
    # …
  end
end

Or a base class:

module Main
  class Transaction
    # mix Dry::Transaction module into subclasses

    # Can provide custom call logic if needed
    # def call
    # end
    
    # And shared API to transaction objects
    def to_queue(input)
      [input.id]
    end
    
    def call_from_queue(input_id)
      input = find_from_id(input_id)
      call(input)
    end
  end
end

Another benefit from class-based transactions is that the application author can add extra, local, steps if they needed to:

class MyTransaction
  include Main::Transaction
  
  step :one
  step :prepare_two
  step :two
  
  # Can mix local methods with injected step operations
  def prepare_two(input)
    # do something with output of one
  end
end

Or even wrap the existing steps:

class MyTransaction
  include Main::Transaction
  
  step :one
  step :two
  
  def one(input)
    changed_input = do_something_with(input)
    super(changed_input)
  end
end

And all the other stuff that we get with Ruby’s standard class behaviour.

I think this approach would make dry-transaction easier and more flexible to work with, would make it fit more naturally into larger apps, and handle a variety of different real-world scenarios, many of which we couldn’t even predict right now.

What do you think? (/cc @solnic, @flash-gordon, @AMHOL) Any objections to me working on this approach?

@solnic
Copy link
Member

solnic commented Feb 27, 2017

Yeah I love this idea. This will be more idiomatic and more flexible. I would also say that with this API we could consider releasing 1.0.0 (assuming we find it work well for us of course).

@timriley
Copy link
Member Author

@solnic Great :) I'll get started on it. Agree that this feels like the kind of API we can make it to 1.0.0 with.

@AMHOL
Copy link
Member

AMHOL commented Feb 27, 2017

This sounds like a really good direction, especially the part about transaction specific logic to alter values between steps, when I was experimenting a while ago I ended up doing something like this:

module Blog
  module Transactions
    module Posts
      Create = ::Blog::Transaction.new do
        step :validate, with: ::Blog::Operations::Validator::Validate[
          'blog.validators.posts.create'
        ]
        step :persist, with: ::Blog::Operations::Repository::Create[
          'blog.repositories.posts'
        ]
      end
    end
  end
end

Where operations were just wrappers that would wrap certain command results in a monad for transactions.

@flash-gordon
Copy link
Member

This looks awesome to me 👍

@mihairadulescu
Copy link
Contributor

SICK! 💯

@mihairadulescu
Copy link
Contributor

🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants