rails 7.1 kwargs strategy error

[zgid123]

Describe the bug

After upgrade from rails 7.0.8 to 7.1, the Inject will raise error ArgumentError (wrong number of arguments (given 1, expected 0)).

class TestController < ActionController::API
  include Inject[
    create_service: 'admin_services_create_service',
    update_service: 'admin_services_update_service'
  ]

  def create
    create_service.call(**params)
  end
end

After debugging, at this code block

        def define_initialize(klass)
          super_parameters = MethodParameters.of(klass, :initialize).each do |ps|
            # Look upwards past `def foo(*)` and `def foo(...)` methods
            # until we get an explicit list of parameters
            break ps unless ps.pass_through?
          end

          if super_parameters.splat? || super_parameters.sequential_arguments?
            define_initialize_with_splat(super_parameters)
          else
            define_initialize_with_keywords(super_parameters)
          end

          self
        end

For rails 7.0.8's controller, the super_parameters is not splat or sequential_arguments, but rails 7.1 will be.

I am still not understand what the problem is. I will try to debug and find out problem.

To Reproduce

Use the code above using rails 7.1

Expected behavior

Can inject the service

My environment

• Affects my production application: YES
• Ruby version: 3.2.2
• OS: macos 13.4
• Rails version: 7.1

[zgid123]

It seems the parameters of controller's constructor before 7.1 is different with the 7.1 one.

When using before 7.1, the one klass.instance_method(:initialize).parameters will return [[:keyrest, :kwargs], [:block, :block]], but 7.1 will return [[:rest, :args], [:keyrest, :kwargs], [:block, :block]].

Because of this one, this @splat = parameters.any? { |type, _| type == :rest } will return true, and then the problem above occurs.

This is new for me, so I don't think I can help to fix this one 😂

[zgid123]

I still cannot find out how to make this case work, don't you think we should create a strategy and force it to use define_initialize_with_keywords(super_parameters) instead?

[flash-gordon]

Can you show the stacktrace of the error?
In general, we cannot predict how a base class will handle its arguments, we do our best but it cannot work in all cases.
I think you can trick dry-auto_inject into using the required path by something like

module Dummy
  def initialize(**kw)
    super(**kw)
  end
end

class TestController < ActionController::API
  include Dummy
  include Inject[...]
end

I don't have a ready rails project on hands to test it

[zgid123]

I create this test project, so you can have a look at the issue

https://github.com/zgid123/test-auto-inject-gem.

Full trace

ArgumentError (wrong number of arguments (given 1, expected 0)):
  
actionpack (7.1.2) lib/action_controller/metal.rb:185:in `initialize'
actionpack (7.1.2) lib/action_dispatch/routing/url_for.rb:110:in `initialize'
actionpack (7.1.2) lib/action_controller/metal/url_for.rb:31:in `initialize'
actionpack (7.1.2) lib/action_controller/metal/instrumentation.rb:22:in `initialize'
activerecord (7.1.2) lib/active_record/railties/controller_runtime.rb:20:in `initialize'
dry-auto_inject (1.0.1) lib/dry/auto_inject/strategies/kwargs.rb:68:in `block (2 levels) in define_initialize_with_splat'
dry-auto_inject (1.0.1) lib/dry/auto_inject/strategies/kwargs.rb:19:in `new'
dry-auto_inject (1.0.1) lib/dry/auto_inject/strategies/kwargs.rb:19:in `block (2 levels) in define_new'
actionpack (7.1.2) lib/action_controller/metal.rb:309:in `dispatch'
actionpack (7.1.2) lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
actionpack (7.1.2) lib/action_dispatch/routing/route_set.rb:32:in `serve'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:51:in `block in serve'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:131:in `block in find_routes'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:124:in `each'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:124:in `find_routes'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:32:in `serve'
actionpack (7.1.2) lib/action_dispatch/routing/route_set.rb:882:in `call'
rack (3.0.8) lib/rack/etag.rb:29:in `call'
rack (3.0.8) lib/rack/conditional_get.rb:31:in `call'
rack (3.0.8) lib/rack/head.rb:15:in `call'
activerecord (7.1.2) lib/active_record/migration.rb:654:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
activesupport (7.1.2) lib/active_support/callbacks.rb:101:in `run_callbacks'
actionpack (7.1.2) lib/action_dispatch/middleware/callbacks.rb:28:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/actionable_exceptions.rb:16:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
railties (7.1.2) lib/rails/rack/logger.rb:37:in `call_app'
railties (7.1.2) lib/rails/rack/logger.rb:24:in `block in call'
activesupport (7.1.2) lib/active_support/tagged_logging.rb:135:in `block in tagged'
activesupport (7.1.2) lib/active_support/tagged_logging.rb:39:in `tagged'
activesupport (7.1.2) lib/active_support/tagged_logging.rb:135:in `tagged'
activesupport (7.1.2) lib/active_support/broadcast_logger.rb:240:in `method_missing'
railties (7.1.2) lib/rails/rack/logger.rb:24:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/remote_ip.rb:92:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/request_id.rb:28:in `call'
rack (3.0.8) lib/rack/runtime.rb:24:in `call'
activesupport (7.1.2) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/server_timing.rb:59:in `block in call'
actionpack (7.1.2) lib/action_dispatch/middleware/server_timing.rb:24:in `collect_events'
actionpack (7.1.2) lib/action_dispatch/middleware/server_timing.rb:58:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/static.rb:25:in `call'
rack (3.0.8) lib/rack/sendfile.rb:114:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/host_authorization.rb:141:in `call'
railties (7.1.2) lib/rails/engine.rb:529:in `call'
puma (6.4.0) lib/puma/configuration.rb:272:in `call'
puma (6.4.0) lib/puma/request.rb:100:in `block in handle_request'
puma (6.4.0) lib/puma/thread_pool.rb:378:in `with_force_shutdown'
puma (6.4.0) lib/puma/request.rb:99:in `handle_request'
puma (6.4.0) lib/puma/server.rb:443:in `process_client'
puma (6.4.0) lib/puma/server.rb:241:in `block in run'
puma (6.4.0) lib/puma/thread_pool.rb:155:in `block in spawn_thread'

[flash-gordon]

Thanks, so, I had a look and now I don't think it's a bug. Rails now uses def initialize(...) in its countless mixins, dry-auto_inject gets confused about it. Currently, the kwargs strategy expects if there's (...) then it's some kind of delegation so pass all dependencies to this constructor. We even have tests for it:

dry-auto_inject/spec/integration/kwargs/inheritance/parent_class_injections_spec.rb

Lines 70 to 98 in 49516a5

	describe "support for argument delegation with new syntax" do
	let(:parent_class) do
	Class.new do
	def initialize(...)
	delegated(...)
	end
	def delegated(one:)
	@one = one
	end
	end
	end
	let(:child_class) do
	Class.new(parent_class) do
	include Test::AutoInject[:one]
	def initialize(*)
	super
	end
	ruby2_keywords(:initialize) if respond_to?(:ruby2_keywords, true)
	end
	end
	it "passes deps to the base constructor" do
	child = child_class.new
	expect(child).to have_attributes(one: "dep 1")
	end
	end

Rails, on the other hand, rails doesn't use constructor parameters at all, the base class doesn't accept any arguments. There's no reliable way to distinguish between these cases.

I suggest having a base class as a workaround:

class BaseController < ApplicationController
  def initialize = super
end

class TestsController < ApplicationController
  include Inject[
    test_service: 'test_service'
  ]

  def index
    test_service.call(test: 'test')
    p 'test'

    render json: {
      message: 'ok'
    }
  end
end

or simply this:

class ApplicationController < ActionController::API
  def initialize = super
end

[zgid123]

you are correct. When I first upgraded to ruby 3, I changed the parameters to .... Your fix above is working, but I don't understand why. Hope we can fix this one soon in the next version.

Thank you so much. You saved my day

[flash-gordon]

Actually, I don't think we can/should fix it, it's not a bug. Instead, we could describe how to set up ApplicationController in docs. I'm going to close this one since I don't see what else can be done right now.

[tiev]

Hi @flash-gordon , I recently upgraded a Rails application to Rails 7 and encountered the same issue. The fix above works.
I also troubleshoot and found the issue of Rails changing the method signature in multiple Modules included in ActionController (commit), which causes this issue.
I see the PR #92 is needed going forward. Because the parameters information of ruby 3 (...) is still not recognized.

Instead of detecting all pass_through patterns, how about detecting required/optional args/kwargs? Combining this with method arity, I hope we can easily solve the problem of passing on parameters to super_method.

If you think that's a good idea, I can make a PR and test it with my application.

[flash-gordon]

@tiev since 3.1 can be declared as the minimal version now, I think it's possible to move forward with #92. I'll take a look