Project

General

Profile

Actions

Defect #43230

open

[Rails 8] ArgumentError: wrong number of arguments (given 0, expected 1) in ApplicationController#find_model_object

Added by Marius BĂLTEANU 3 days ago. Updated 1 day ago.

Status:
New
Priority:
Normal
Category:
Rails support
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Affected version:

Description

Using Rails 8, multiple tests fail with the same error:

Error:
DocumentsControllerTest#test_add_attachment:
ArgumentError: wrong number of arguments (given 0, expected 1)
    app/controllers/application_controller.rb:392:in 'model_object'
    app/controllers/application_controller.rb:383:in 'ApplicationController#find_model_object'
    lib/redmine/sudo_mode.rb:78:in 'Redmine::SudoMode::Controller#sudo_mode'

I cannot find the exact issue from Rails that changed this behaviour and triggers this issue.


Files


Related issues

Blocks Redmine - Feature #43205: Update to Rails 8New

Actions
Actions #1

Updated by Marius BĂLTEANU 3 days ago

  • Subject changed from ArgumentError: wrong number of arguments (given 0, expected 1) in ApplicationController#find_model_object to [Rails 8] ArgumentError: wrong number of arguments (given 0, expected 1) in ApplicationController#find_model_object
Actions #2

Updated by Marius BĂLTEANU 3 days ago

The simplest way to fix this is by renaming the self.model_object method to self.set_model_object to avoid collision (attached the patch).

Another way is to drop this common method added in r3597 and use standard find_* methods.

Actions #3

Updated by Marius BĂLTEANU 3 days ago

Actions #4

Updated by Marius BĂLTEANU 3 days ago

Actions #5

Updated by Marius BĂLTEANU 3 days ago

Actions #6

Updated by Katsuya HIDAKA 2 days ago

I cannot find the exact issue from Rails that changed this behaviour and triggers this issue.

This is probably caused by an internal change to class_attribute in Rails 8.0.1:
https://github.com/rails/rails/pull/52722/files

It is a bit hard to explain, but the cause seems to be as follows.

Before PR#52722, when the model_object= method (defined by class_attribute) was called, the self.model_object method was redefined.
https://github.com/rails/rails/pull/52722/files#diff-2cb0dc1e23cf76912807c4b255a74e23f274a8fe824a2b681014582ef9599578L108

class_methods << <<~RUBY
  silence_redefinition_of_method def #{name}=(value)
    redefine_method(:#{name}) { value } if singleton_class?
    redefine_singleton_method(:#{name}) { value } # <== !!!! HERE !!!!
    value
  end
RUBY
...

Because of this, calling self.model_object without arguments did not raise an error.
Below is a test in the Rails 7.2.2.2 console. Calling Foo.value before Foo.value = 1 raises an error, but calling it after Foo.value = 1 does not.

Loading development environment (Rails 7.2.2.2)
redmine-app(dev)* class Foo
redmine-app(dev)*   class_attribute :value
redmine-app(dev)*   def self.value(x)
redmine-app(dev)*    puts x
redmine-app(dev)*   end
redmine-app(dev)> end
=> :value
redmine-app(dev)> Foo.value
/bundle/ruby/3.4.0/gems/irb-1.15.2/lib/irb.rb:406:in 'full_message': wrong number of arguments (given 0, expected 1) (ArgumentError)
redmine-app(dev)> Foo.value = 1
=> 1
redmine-app(dev)> Foo.value
=> 1

After PR#52722 (Rails 8.0.2+), this behaviour no longer exists.
So when self.model_object is called, it instead calls the later-defined self.model_object(model) method, which causes the ArgumentError.

Actions #7

Updated by Katsuya HIDAKA 2 days ago

In summary, currently (before PR#52722), the following line is executed first:

https://github.com/redmine/redmine/blob/9135d9653616377dd23b8c94d788421069e46349/app/controllers/documents_controller.rb#L22

class DocumentsController < ApplicationController
  default_search_scope :documents
  model_object Document # <== HERE!
  ...

At this point, self.model_object= is called, and the method def self.model_object(object) is redefined by redefine_singleton_method(:#{name}) { value }. As a result, calling self.model_object after that does not invoke self.model_object(object), so no error occurs.

Actions #8

Updated by Marius BĂLTEANU 2 days ago

  • Assignee set to Marius BĂLTEANU

Thanks Katsuya HIDAKA for clarifications!

Regarding the fix, which option do you think is cleaner?

Actions #9

Updated by Katsuya HIDAKA 1 day ago

I think the attached 0001 patch is the better way to resolve this issue.

Personally, I prefer an explicit implementation without using the common method added in r3597. However, the change could affect a wide range of code and potentially impact existing plugins, so it might need to be considered carefully.

Right now this issue breaks some functionality on Rails 8. So I think resolving it first with the minimal changes in the 0001 patch is important to proceed with the Rails 8 upgrade.

After that, if needed, we could also consider dropping find_model_object as a code improvement.

Actions #10

Updated by Katsuya HIDAKA 1 day ago

As a minor variant of the 0001 patch, it could also be an option to simply use the setter method self.model_object= defined by class_attribute :model_object instead of the self.model_object(model) method, like in the code below.

code to simply use self.model_object=

Actions

Also available in: Atom PDF