Currently in dry-transaction if a step is defined with a method, it seems you cannot inject an alternate implementation. The logic to initialize the transaction will always prefer the method over a passed in operation.
It doesn’t feel consistent to me that the way a step is defined should affect the ability to override it, and there don’t appear to be tests describing the situation, so I’m guessing this is just an implementation detail, not desired behavior.
Is this a change people would like to see? I’d be happy to make a PR for it!
Well, my hopes for this being an obvious and nonintrusive change were dashed as soon as I tried to implement it and ran the tests 
I had read the docs about wrapping steps, but didn’t connect the dots. If you inject a step definition into a transaction, and a method of the same name exists, that method will wrap the injected operation, not replace it. If you aren’t calling super in your method, injecting a different implementation does nothing.
This does feel inconsistent, as I mentioned in my original post, but I’m new to this dry-rb ecosystem.
Clearly this is not a simple change now, so any higher level thoughts on the subject? Maybe wrapping should be explicitly added to the API as a step option or something, so it doesn’t step on method-defined steps?
Hi @rhec, thanks for taking the time to explore this and share it here in the forum. You’re actually not the first person to consider this kind of behaviour; it came up via a GitHub issue in the last few weeks, too.
One of my responses to the issue explained the reason why we didn’t allow instance method-based steps to be overridden from the outside:
[When developing the class-based transactions], 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 we don’t have any plans currently to make it so steps provided by instance methods can be replaced from the outside.
That said, I’d still love to hear what you were hoping to do. Maybe with some more understanding of various use cases we might be able to provide something that helps you.
Thanks for the reply and your interest @timriley, reading through that github issue is helpful.
To give you more context on what I’m doing:
I’m working with an existing and actively developed Rails app and I am looking for a way to improve the separation of our business logic from lower-level concerns like persistence and validation - i.e. get away from ‘fat’ models. So everything I read about dry-rb sounds great! Philosophically we are aligned.
The challenging part is getting started, and I was hoping to be able to introduce pieces of dry-rb one at a time in areas where I think they can provide value in isolation. The less I have to re-organize existing code the better, and I’d love to avoid introducing a lot of new concepts at once.
My thinking is transactions are nice, high-level concept to start with. So defining a transaction as a class, with no container, and a set of methods as operations is very appealing to me. Simple to write, obvious to the reader what it’s doing, no decisions to make about how to organize your code, and still very useful! It’s the perfect way to get started, and I’d guess that’s why it’s the first example in the docs.
So given all that context, what I was actually doing is just trying to prove I could inject different operations in my tests, for all the reasons it’s good to be able to inject different operations when they come from a container.
I know I’m repeating this point, so hopefully I’m not belaboring it, but even after your explanation I don’t quite get why an operation should be different based on the details of how it’s implemented. If it’s callable and returns an either monad, it seems like it should behave consistently. I understand it could just be convention, and if conventions are useful, they can certainly be worth some inconsistency and increase in learning curve. But it does feel like you are giving up a lot of simplicity and consistency for this convention. Maybe I just haven’t run into the right use case for wrapping operations, but it doesn’t seem worth the tradeoff to me at this point.
Thanks again if you made it this far - long post!
@rhec You usually wrap operation to correct input and output to match injected operation API.
If you need the more explicit way to work with operations you can inject them to current operation by using dry-auto_inject. For example, we use it to inject some helpers:
include Import[h: 'rails.url_helpers']
If you implement business logic in transaction methods you probably don’t want to test it in isolation. And if you want - just extract it to another transaction.
In tests, you usually worry about some side effects (like persistence). For this, you can extract them into separated generic operation and use it everywhere.
class MakeSomeWork
include Dry::Transaction(container: Container)
step :work
step :persist
def work(model:, **input)
# make some work
end
def persist(model:, **input)
super(model)
end
end
class MakeSomeWork
include Dry::Transaction::Operation
include Import['persist']
def call(model:, **input)
# make some work
persist.call(model)
end
end
Thanks for the response, @rhec!
Yep, right now dry-transaction won’t let you inject operations if they don’t come from a container. This is what we’ll be addressing in response to issue #75 (the one I linked above). Once this is done, any step that you define will be injectable.
However, if you have instance methods matching step names, then those will always be called first, and it’ll be up to the method body to call super if it wants to ensure the step’s injected operation object runs.
Thanks @semenovDL @timriley
I’m making progress!
@timriley on your thread: https://github.com/dry-rb/dry-transaction/issues/75
I began thinking why am I mocking a local method in a dry-transaction? Is it really a code smell (akin to mocking pvt methods), so I asked myself, what are local transaction methods for… they can be used to clearly delineate program control flow, but maybe shouldn’t / don’t need to, since you can do this in an operation. Are they for coordinating high level operations? probably better, however some operations don’t feel big enough to pull out into self contained operations… (a few lines of code) plus they are related somewhat.
What’s the best way to model the code?
If you pull each and every operation out you are left with a very lightweight barebone transaction that only lists steps… I’m not seeing huge value here. Whereas the steps I have are actually related to each other behaviourally, so logically live together. Do you put them simply as operations in the same namespace, or group them inside a transaction local methods?
class ProcessRequest < Transaction
include Import['adapter.x', 'repo.y', 'operations.update_db_status', 'operations.upload_it']
step :submit_request_to_x
tee :store_data
step :upload_to_ext
def submit_request_to_x(request)
// plus some error handling
update_db_status.(id: 'xxx', 'status_a')
data = x.submit(request)
update_db_status.(id: 'xxx', 'status_b')
Success(data)
end
def store_data(data)
// update some statuses
y.persist(data)
// handle status update on persistence error etc
end
def upload_to_ext(data)
upload_it.(data) do |m|
// some custom handling of success / failure state transitions + logging
end
end
end
Now I could model this as a single operation, and handle the control flow myself, with lots of begin / rescue blocks, and private methods reflecting those 3 methods above. But then I lose the clarity that there are 3 distinct behavioural steps. I could pull the methods into separate operations, but then I end up with very thin wrappers around other operations and adapter and repos.
So back to the topic of injecting operations or overriding / mocking steps defined as methods… I have another example where I wanted a cleaner mock of the local method rather than just replace the injected operation. Let me clarify:
class MyTransaction < Transaction
include Import['transactions.complex_stuff']
step :might_exit_early, with: 'transactions.do_stuff'
tee :enqueue_do_stuff
def might_exit_early(payload)
return Failure(:exit_early) if condition_met
super(payload)
end
def enqueue_do_stuff(blah)
// enqueue 'complex_stuff' transaction and handle error conditions, and success/failure of transaction
// update_status accordingly and do logging
enqueue....do
complex_stuff.call() do |m|
// blah
end
end
end
So the enqueueing logic is just wrapper around a transaction, plus a little ‘maintenance’ around that. I want to test the transaction overall behaviour that the wrapped operation “might_exit_early” logic. On the continue use_case, I don’t care the second step to take place.
2 options…
- mock out
enqueue_do_stuff()or - inject a dummy for
transactions.complex_stuff - unit test local method
might_exit_early
#1 isn’t allowed, #2 ends up running more code than I want,
#1 I worked around by using rspec allow_any_instance_of(MyTransaction).to receive(:enqueue_do_stuff)
#3 works equally, but feels wrong as I’m effectively unit testing the internals of the transaction, whereas I’d rather test it from outside, but shortcutting the subsequent processing.
At the moment, I think option #2 gives the balance of testing at the right visibility, but still runs a bit more code than I actually want to hit… I want to bypass the 2nd step completely.