dry-transaction: Can't override operations unless they came from a container

Using dry-transaction 0.10.2 (and Ruby 2.3.4p301, if it matters), I’m able to override operations only if they came from a container.

That is, this works:

require 'dry/transaction'

Container = Dry::Container.new.tap do |c|
  c.register(:container_operation, call: false) do |_|
    Dry::Monads.Right(:success)
  end
end

class Example1
  include Dry::Transaction(container: Container)

  step :operation, with: :container_operation
end

puts Example1.new.call(nil)
# => Right(:success)

puts Example1.new(operation: ->(_) { Dry::Monads.Left(:error) }).call(nil)
# => Left(:error)

But this does not:

require 'dry/transaction'

class Example2
  include Dry::Transaction

  step :operation

  def operation(_)
    Right(:success)
  end
end

puts Example2.new.call(nil)
# => Right(:success)

puts Example2.new(operation: ->(_) { Dry::Monads.Left(:error) }).call(nil)
# => Right(:success)

I expected the Example2 case to behave the same way as the Example1 case. Is this a known limitation or expected behavior of class-based transactions?

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 20 (11 by maintainers)

Most upvoted comments

@htp How you going with this one? Just checking in 😃

timriley on Nov 27, 2017

I swear I’m going to work on this, hah.

Been working toward / prepping for a major release at the office and that’s taken priority- if you’d like to close this for triaging / hygiene, I can reopen when I actually have a change in hand for you.

htp on Dec 12, 2017

(I’m still planning to work on this!)

htp on Nov 3, 2017

Hi @htp, thanks for submitting this, and thanks also @GustavoCaso for talking this through!

It’s an interesting behaviour you came across, @htp. I’d never thought about supporting replacement of instance method-based step operations by injected objects because I considered those instance methods to be “intrinsic” to the class, i.e. not separate enough to be some sort of external concern/external object. i.e. not the kind of thing that should be changeable from the outside. Providing alternative objects for these when testing almost feels as much as a “smell” as e.g. stubbing private methods when testing.

So this is why we didn’t support this behaviour when moving to the class-based transactions. You’re right that this behaviour is different to how we handle container-based operations, but I wouldn’t go so far as to say it’s a regression (I know you weren’t suggesting this, btw) — it’s really the fact that instance method-based step operations are a wholly new thing and come with some slightly different behaviour.

So that’s my feeling when looking at this from one angle. But after considering this further, I feel like there’s perhaps another angle — what if someone was using dry-transaction without any container at all? That person may want to define transactions consisting of steps using injected operation objects, and still have the ability to wrap those in local methods (or not).

For this kind of usage, we really do need to support injected operation objects for steps defined without any associated container identifier (whether the step operation is provided by a local method or otherwise).

So, after thinking all of this through, I’d definitely welcome a PR to add this behaviour. What do you think?

I’m going to at least re-open this issue in the meantime.

timriley on Oct 13, 2017