dry-rb / dry-auto_inject

Container-agnostic constructor injection mixin
https://dry-rb.org/gems/dry-auto_inject/
MIT License
176 stars 29 forks source link

Injecting in ActiveJob no longer works #50

Open paul opened 5 years ago

paul commented 5 years ago

I think #48 broke the ability to use AutoInject in ActiveJob classes.

We have a Job that looks like this:

class MessageAutoDeliverJob < ApplicationJob
  include Channels::Import[:account_channel]

  def perform(message)
    account_channel.new(account: message.conversation.account).message(message: message)
  end
end

The Channels container has registered :account_channel as simply AccountChannel.

When I upgraded the AutoInject gem from 0.4.6 to 0.6.0, I get an error in my specs:

     ActiveJob::SerializationError:
       Unsupported argument type: Class

I tossed a debugger in the enqueue process, and in 0.4.8 it attempts to serialize the @arguments, which looks like this:

=> [#<Message:0x000055dd19eddd88
  id: 946066008,
  body: "Chuck Norris can instantiate an abstract class.">]

In 0.6.0, however, now @arguments includes the injected AccountChannel:

[#<Message:0x0000564c8adda108
  id: 946066008,
  body: "When Chuck Norris gives a method an argument, the method loses.">,
 {:account_channel=>AccountChannel}]

If I'm understanding it right, I seems #48 changed the initializer from removing the keys it knows about from the kwargs passed to the inheriting class, to instead just passing everything through if the class's #initialize function splats its args?

One of my prime use-cases for dry-container/auto_inject was to get around the brain-dead way ActiveJob handles initialization, because it can't differentiate between kwargs passed to the initializer for DI, and params passed to the #perform method to do the work. It was really nice that Dry::AutoInject allowed us to do that without having to jump through a bunch of hoops.

48 mentioned possible breakage in Rails Controllers, could something like that be adapted to work with ActiveJob?

sergio-fry commented 2 years ago

šŸ‘

Is there is some workaround? The best solution I've is

class GreatJob < ApplicationJob
  def perform(payload)
    repo.save payload
  end

  def repo
    ApplicationContainer.resolve(:repo)
  end
end
solnic commented 2 years ago

We should properly document that using auto-inject in classes that you don't own is not recommended as there can be incompatibility with constructors. auto-inject does its best to determine super constructor's behavior and act accordingly but sometimes it's just not possible.

One solution would be to add another strategy that uses attribute readers that do exactly what repo method does in your example, the only trick is that those should be memoized.

@timriley WDYT about that? ā˜šŸ»

flash-gordon commented 2 years ago

or use effects... Though, for effects you'll also need a handler somewhere so setup a bit more involved