moiristo / deep_cloneable

This gem gives every ActiveRecord::Base object the possibility to do a deep clone that includes user specified associations.
MIT License
786 stars 88 forks source link

Wrong number of arguments under Ruby 2.0 and Rails 3.2.13 #24

Closed turadg closed 10 years ago

turadg commented 11 years ago

With Ruby 2.0 and Rails 3.2-STABLE, the :include isn't working:

> question.dup :include => :choices
ArgumentError: wrong number of arguments (1 for 0)

Maybe it's not removing the original dup correctly?

Fwiw, I first thought it was a bug in Rails because even regular dup didn't work

> p.dup
NoMethodError: private method `initialize_dup' called for #<Question:0x007fd6fb9d9a98>

But I just switched updated Rails past this bug in Rails 3.2.x with Ruby 2.0 that was fixed in 3.12.13.

moiristo commented 11 years ago

This should be fixed in 1.5.1, which version are you using?

turadg commented 11 years ago

1.5.1. I used gem 'deep_cloneable', '~> 1.5.0' in my Gemfile and 1.5.0 is pulled from rubygems.

Just reported as a heads up. I went and deep cloned manually in my project for now.

On Wed, May 15, 2013 at 3:44 AM, Reinier de Lange notifications@github.comwrote:

This should be fixed in 1.5.1, which version are you using?

— Reply to this email directly or view it on GitHubhttps://github.com/moiristo/deep_cloneable/issues/24#issuecomment-17924350 .

indrekj commented 11 years ago

Same problem.

bundle exec rake #=> wrong number of arguments (1 for 0) bundle show deep_cloneable #=>.rvm/gems/ruby-2.0.0-p0@fff/gems/deep_cloneable-1.5.1`

activerecord 3.2.12 ruby 2.0.0-p0

moiristo commented 11 years ago

I don't understand why this is happening. If you look at the Travis build of master (https://travis-ci.org/moiristo/deep_cloneable), all tests pass for ruby-2.0.0. In which way does your setup differ?

indrekj commented 11 years ago

Minimal example (with some other error):

bundle exec irb
irb(main):001:0> require "active_record"
irb(main):002:0> ActiveRecord::Base.establish_connection(adapter: "postgresql", ...)
irb(main):003:0> require "deep_cloneable"
irb(main):004:0> class Person < ActiveRecord::Base; end
irb(main):005:0> Person.first.dup
NoMethodError: private method `initialize_dup' called for #<Person:0x007f836f179928>
    from /Users/indrek/.rvm/gems/ruby-2.0.0-p0@f/gems/activemodel-3.2.12/lib/active_model/attribute_methods.rb:404:in `method_missing'
    from /Users/indrek/.rvm/gems/ruby-2.0.0-p0@f/gems/activerecord-3.2.12/lib/active_record/attribute_methods.rb:149:in `method_missing'
    from /Users/indrek/.rvm/gems/ruby-2.0.0-p0@f/gems/activemodel-3.2.12/lib/active_model/validations.rb:179:in `dup'
    from /Users/indrek/.rvm/gems/ruby-2.0.0-p0@f/gems/deep_cloneable-1.5.1/lib/deep_cloneable.rb:67:in `block in <module:DeepCloneable>'
    from /Users/indrek/.rvm/gems/ruby-2.0.0-p0@f/gems/activerecord-3.2.12/lib/active_record/base.rb:562:in `dup'
    from (irb):13
    from /Users/indrek/.rvm/rubies/ruby-2.0.0-p0/bin/irb:12:in `<main>'
indrekj commented 11 years ago

If I change deep_cloneable to this:

class ActiveRecord::Base
  module DeepCloneable
    #if !Object.respond_to? :initialize_dup, true # I commented out this line
      ActiveRecord::Base.class_eval do
        protected :initialize_dup                         # I added this line
        module Dup
          def dup
            copy = super
            copy.initialize_dup(self)
            copy
          end
        end
        remove_possible_method :dup
        include Dup
      end
    #end
    ...

then it starts to work. Not sure if correctly, but it doesn't throw an error any more.

moiristo commented 11 years ago

Maybe it's best to send to initialize_dup (copy.send :initialize_dup,self)). I'll first try to reproduce this issue tonight-ish.

indrekj commented 11 years ago

I just updated activerecord 3.2.12 to 3.2.13 and now everything is okay.

javiervidal commented 11 years ago

Same error for me with ruby-1.9.3-p392 and activerecord 3.2.13 and deep_cloneable 1.5.1.

moiristo commented 11 years ago

I have changed the initialize_dup in master call to use a send instead. However, as I could still not reproduce the error, it's really a shot in the dark. I've tried to reproduce the error by using the gem in a new rails 3.2.13 app, but everything was working properly for me. It could be that a certain other gem dependency is the cause of the error.

Could you please check if the code in the master branch works for you?

moiristo commented 11 years ago

Today I found out that the test suite was not working properly because the version dependencies were not specified right. The error mentioned did come up after some refactoring. I believe it is fixed now in version 1.5.4. The fix uses the Dup module as specified above.

USvER commented 10 years ago

Same problem here:

ruby 2.1.1p76
rails (4.0.3)
deep_cloneable (1.6.1)
moiristo commented 10 years ago

No clue what's wrong here, might be another gem dependency. Please let me know if you find out what's causing this.

unixmonkey commented 10 years ago

Looks like Friendly_id defines .dup also:

[1] pry(#<Post>)> show-method published.dup
From: /Users/djones/.gem/ruby/2.1.2/gems/friendly_id-5.0.3/lib/friendly_id/base.rb @ line 255:
Owner: FriendlyId::Model
Visibility: public
Number of lines: 3

def dup
  super.tap { |duplicate| duplicate.slug = nil if duplicate.respond_to?('slug=') }
end
moiristo commented 10 years ago

Ah, thanks! That might be the problem. I'll try to reproduce the error and come up with a solution. Might need to rename the whole method I guess..

unixmonkey commented 10 years ago

:+1:

Issues in this thread aside, I would still like to be able to call clone = my_object.deep_clone(include: [:things]).

Even if it calls .dup on the inside, I would like knowing which are normal dups, and which are for deep_clonable. The less magic, the better (IMO).

I think with change like that, if following Semver, should introduce a new minor version with both ways of calling it working equally with a deprecation warning on the old way, then a new major version with the old way introduced, as well as some documentation on migrating.

If you would like to go down this road, I'd be glad to help.

moiristo commented 10 years ago

I agree :)

seanculver commented 10 years ago

Any updates on this, I'm also having this issue. Any stable workarounds exist in the meantime?

Rails 4.1 / Ruby 2.0.0

seanculver commented 10 years ago

@unixmonkey I agree with you and I've made the changes that you've suggested and made a pull request.

moiristo commented 10 years ago

I have released v1.7.0 which deprecates the use of 'dup'. The code has been moved to the v1 branch. The new major version, which has the dup method removed, is now in master.

seanculver commented 10 years ago

Thank you! Awesome!

On Thursday, June 26, 2014, Reinier de Lange notifications@github.com wrote:

Closed #24 https://github.com/moiristo/deep_cloneable/issues/24.

Reply to this email directly or view it on GitHub https://github.com/moiristo/deep_cloneable/issues/24#event-135354599.

ghost commented 9 years ago

I've having this issue with Rails 4.2.3 and deep_cloneable 2.1.1.

The strange thing is, it works when I run service / controller tests from RSpec, but when I run the rails app and send a real request, I get the 'Wrong number of arguments' error.

moiristo commented 9 years ago

Can you provide a stacktrace?

ghost commented 9 years ago

Thanks for the reply. I've pasted the call and the full backtrace below.

Seems that it's not actually the environment causing the issue. It works under rspec, but not using rails server or rails console regardless of the environment set.

I think I understand what's happening now. The deep_clone method is being overridden by a monkey patch from the sexp_processor gem:

Attachment.first.method(:deep_clone).source_location
=> ["/Users/b-sutherland/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/sexp_processor-4.5.0/lib/sexp_processor.rb", 648]

https://github.com/seattlerb/sexp_processor/blob/master/lib/sexp_processor.rb#L657

This happens on model classes which use the hair_trigger gem:

gem dependency sexp_processor --reverse-dependencies
Gem sexp_processor-4.6.0
  Used by
    ruby_parser-3.7.0 (sexp_processor (~> 4.1))

gem dependency ruby_parser --reverse-dependencies
Gem ruby_parser-3.6.6
  Used by
    hairtrigger-0.2.14 (ruby_parser (~> 3.5))

For now I've fixed it by adding this method to my model class:

def deep_cloneable_clone(kwargs)
  deep_clone(kwargs)
end

Code:

  215  next_version = kv.deep_clone(
  216         include: ASSOCIATIONS_FOR_DEEP_CLONE,
  217         except: [:updated_by, :customer_id],
  218         validate: false
  219       )
Full backtrace
--------------

 - app/services/meta_service.rb:215:in `update_meta'
 - app/controllers/public_api/meta_controller.rb:114:in `update_meta'
 - actionpack (4.2.3) lib/action_controller/metal/implicit_render.rb:4:in `send_action'
 - actionpack (4.2.3) lib/abstract_controller/base.rb:198:in `process_action'
 - actionpack (4.2.3) lib/action_controller/metal/rendering.rb:10:in `process_action'
 - actionpack (4.2.3) lib/abstract_controller/callbacks.rb:20:in `block in process_action'
 - activesupport (4.2.3) lib/active_support/callbacks.rb:115:in `call'
 - activesupport (4.2.3) lib/active_support/callbacks.rb:553:in `block (2 levels) in compile'
 - activesupport (4.2.3) lib/active_support/callbacks.rb:503:in `call'
 - activesupport (4.2.3) lib/active_support/callbacks.rb:88:in `run_callbacks'
 - actionpack (4.2.3) lib/abstract_controller/callbacks.rb:19:in `process_action'
 - actionpack (4.2.3) lib/action_controller/metal/rescue.rb:29:in `process_action'
 - actionpack (4.2.3) lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
 - activesupport (4.2.3) lib/active_support/notifications.rb:164:in `block in instrument'
 - activesupport (4.2.3) lib/active_support/notifications/instrumenter.rb:20:in `instrument'
 - activesupport (4.2.3) lib/active_support/notifications.rb:164:in `instrument'
 - actionpack (4.2.3) lib/action_controller/metal/instrumentation.rb:30:in `process_action'
 - actionpack (4.2.3) lib/action_controller/metal/params_wrapper.rb:250:in `process_action'
 - activerecord (4.2.3) lib/active_record/railties/controller_runtime.rb:18:in `process_action'
 - actionpack (4.2.3) lib/abstract_controller/base.rb:137:in `process'
 - actionview (4.2.3) lib/action_view/rendering.rb:30:in `process'
 - actionpack (4.2.3) lib/action_controller/metal.rb:196:in `dispatch'
 - actionpack (4.2.3) lib/action_controller/metal/rack_delegation.rb:13:in `dispatch'
 - actionpack (4.2.3) lib/action_controller/metal.rb:237:in `block in action'
 - actionpack (4.2.3) lib/action_dispatch/routing/route_set.rb:76:in `dispatch'
 - actionpack (4.2.3) lib/action_dispatch/routing/route_set.rb:45:in `serve'
 - actionpack (4.2.3) lib/action_dispatch/journey/router.rb:43:in `block in serve'
 - actionpack (4.2.3) lib/action_dispatch/journey/router.rb:30:in `serve'
 - actionpack (4.2.3) lib/action_dispatch/routing/route_set.rb:821:in `call'
 - bullet (4.14.4) lib/bullet/rack.rb:10:in `call'
 - rack (1.6.4) lib/rack/etag.rb:24:in `call'
 - rack (1.6.4) lib/rack/conditionalget.rb:38:in `call'
 - rack (1.6.4) lib/rack/head.rb:13:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/params_parser.rb:27:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/flash.rb:260:in `call'
 - rack (1.6.4) lib/rack/session/abstract/id.rb:225:in `context'
 - rack (1.6.4) lib/rack/session/abstract/id.rb:220:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/cookies.rb:560:in `call'
 - activerecord (4.2.3) lib/active_record/query_cache.rb:36:in `call'
 - activerecord-refresh_connection (0.0.5) lib/activerecord-refresh_connection/active_record/connection_adapters/refresh_connection_management.rb:17:in `call'
 - activerecord (4.2.3) lib/active_record/migration.rb:377:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
 - activesupport (4.2.3) lib/active_support/callbacks.rb:84:in `run_callbacks'
 - actionpack (4.2.3) lib/action_dispatch/middleware/callbacks.rb:27:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/reloader.rb:73:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/remote_ip.rb:78:in `call'
 - better_errors (2.1.1) lib/better_errors/middleware.rb:84:in `protected_app_call'
 - better_errors (2.1.1) lib/better_errors/middleware.rb:79:in `better_errors_call'
 - better_errors (2.1.1) lib/better_errors/middleware.rb:57:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
 - railties (4.2.3) lib/rails/rack/logger.rb:38:in `call_app'
 - railties (4.2.3) lib/rails/rack/logger.rb:20:in `block in call'
 - activesupport (4.2.3) lib/active_support/tagged_logging.rb:68:in `block in tagged'
 - activesupport (4.2.3) lib/active_support/tagged_logging.rb:26:in `tagged'
 - activesupport (4.2.3) lib/active_support/tagged_logging.rb:68:in `tagged'
 - railties (4.2.3) lib/rails/rack/logger.rb:20:in `call'
 - request_store (1.1.0) lib/request_store/middleware.rb:8:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/request_id.rb:21:in `call'
 - rack (1.6.4) lib/rack/methodoverride.rb:22:in `call'
 - rack (1.6.4) lib/rack/runtime.rb:18:in `call'
 - activesupport (4.2.3) lib/active_support/cache/strategy/local_cache_middleware.rb:28:in `call'
 - rack (1.6.4) lib/rack/lock.rb:17:in `call'
 - actionpack (4.2.3) lib/action_dispatch/middleware/static.rb:116:in `call'
 - rack (1.6.4) lib/rack/sendfile.rb:113:in `call'
 - railties (4.2.3) lib/rails/engine.rb:518:in `call'
 - railties (4.2.3) lib/rails/application.rb:165:in `call'
 - rack (1.6.4) lib/rack/content_length.rb:15:in `call'
 - thin (1.6.3) lib/thin/connection.rb:86:in `block in pre_process'
 - thin (1.6.3) lib/thin/connection.rb:84:in `pre_process'
 - thin (1.6.3) lib/thin/connection.rb:53:in `process'
 - thin (1.6.3) lib/thin/connection.rb:39:in `receive_data'
 - eventmachine (1.0.7) lib/eventmachine.rb:187:in `run'
 - thin (1.6.3) lib/thin/backends/base.rb:73:in `start'
 - thin (1.6.3) lib/thin/server.rb:162:in `start'
 - rack (1.6.4) lib/rack/handler/thin.rb:19:in `run'
 - rack (1.6.4) lib/rack/server.rb:286:in `start'
 - railties (4.2.3) lib/rails/commands/server.rb:80:in `start'
 - railties (4.2.3) lib/rails/commands/commands_tasks.rb:80:in `block in server'
 - railties (4.2.3) lib/rails/commands/commands_tasks.rb:75:in `server'
 - railties (4.2.3) lib/rails/commands/commands_tasks.rb:39:in `run_command!'
 - railties (4.2.3) lib/rails/commands.rb:17:in `<top (required)>'
 - bin/rails:4:in `<top (required)>'
 - spring (1.3.6) lib/spring/client/rails.rb:28:in `call'
 - spring (1.3.6) lib/spring/client/command.rb:7:in `call'
 - spring (1.3.6) lib/spring/client.rb:26:in `run'
 - spring (1.3.6) bin/spring:48:in `<top (required)>'
 - /Users/b-sutherland/.rbenv/versions/2.2.0/bin/spring:23:in `<main>'
moiristo commented 9 years ago

Thanks, I'll let the developer of sexp_processor know of this conflict.