Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Correct output from ActiveModel::Errors to_json

#4890

ActiveModel::Errors#to_xml correctly outputs valid XML that can be used to display all errors. ActiveModel::Errors#to_json does not output valid JSON because it does not have a custom implementation of to_json to deal with ActiveModel::Errors#each which flattens the attribute errors array.

irb(main):001:0> u = User.create
=> #<User id: nil, username: nil>
irb(main):002:0> u.errors
=> #<OrderedHash {:username=>["is too short (...snip...)", "can't be blank"]}>
irb(main):003:0> puts u.errors.to_xml

<?xml version="1.0" encoding="UTF-8"?>
<errors>
  <error>Username is too short (minimum is 3 characters)</error>
  <error>Username can't be blank</error>
</errors>
=> nil
irb(main):004:0> puts u.errors.to_json
{"username":"is too short (minimum is 3 characters)","username":"can't be blank"}
=> nil

This JSON is invalid and when parsed would only give one error message per attribute.
ActiveModel::Errors#to_json should return the same messages as to_xml.

["Username is too short (minimum is 3 characters)","Username can't be blank"]

Reported by Samuel Kadolph · June 17th, 2010 @ 06:02 PM

State: resolved
Milestone: 3.0.2
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Samuel Kadolph
    Samuel Kadolph

    The solution is to override the ActiveModel::Errors#as_json method to give the full_messages array to be converted to JSON. Patch is attached.

    June 17th, 2010 @ 06:11 PM

  2. Samuel Kadolph
    Samuel Kadolph

    Complete patch is attached. (Sorry, new to the whole patching rails thing.)

    June 17th, 2010 @ 06:45 PM

  3. José Valim
    José Valim
    • Milestone cleared.
    • Assigned user set to José Valim

    The patch looks great, but it need tests to ensure we won't have a regression later.

    June 22nd, 2010 @ 04:39 PM

  4. Neeraj Singh
    Neeraj Singh

    Trying to create a test for this one but running into stack level too deep issue.

    Hopefully I'll have a test patch soon. :-)

    June 23rd, 2010 @ 10:07 PM

  5. Samuel Kadolph
    Samuel Kadolph

    I've been looking into writing a test case for this but I have to get the tests for rails going first which hasn't been easy on Windows and our only other Unix server with ruby 1.8.7 is for production and I won't be installing the oodles of gems that rails dev requires.

    That aside, I did notice there is no test case for ActiveModel::Errors#to_xml. So it would be beneficial to write a case for that as well.

    June 23rd, 2010 @ 10:11 PM

  6. José Valim
    José Valim

    Neeraj, I think it's expected that you run into StackLevelTooDeep before the patch is applied. :) It means your test is correct!

    June 25th, 2010 @ 08:41 AM

  7. Neeraj Singh
    Neeraj Singh
    • Tag changed from active model, active_model, errors, json, to_json to active model, active_model, errors, json, rails3, to_json
    • State changed from new to open

    There are couple of things going on which is making me pause and think.

    • in the test if I do topic.errors.to_json then it blows up saying to_json is not defined which does not make sense since active_model requires active_support which in turn requires json. So all objects should have to_json. I don't fully understand how autoload works so I need to learn that.

    • if I explicitly say require active_support/json to bring to_json then I get "stack level too deep" even with the fix in.

    I am looking into it. expect some delays :-)

    June 25th, 2010 @ 03:48 PM

  8. José Valim
    José Valim

    JSON is autoloaded, but the to_json method is only added after you load it. So you need to explicitly require the json file (in tests).

    About the stack level too deep error, could you please paste the strack trace here (not necessarily the whole backtrace ;))?

    June 25th, 2010 @ 04:05 PM

  9. Neeraj Singh
    Neeraj Singh

    test requires 'cases/helper'. helper in turn requires active_model which in turn requires active_support.

    JSON is eagerly autoloaded in active_support. json.rb in turn requires encoding. So I fail to understand why to_json will be missing. Sorry for being so thick.

    eager_autoload do
  autoload :JSON
end
 
require 'active_support/json/decoding'
require 'active_support/json/encoding'

    June 25th, 2010 @ 04:15 PM

  10. Neeraj Singh
    Neeraj Singh

    I added following line to test/cases/validations/presence_validation_test.rb

    require "active_support/json/encoding"

# added last three tests
  test 'accepts array arguments' do
    Topic.validates_presence_of %w(title content)
    t = Topic.new
    assert t.invalid?
    assert_equal ["can't be blank"], t.errors[:title]
    assert_equal ["can't be blank"], t.errors[:content]
    assert_match %r{<error>Title can't be blank</error>}, t.errors.to_xml
    assert_match %r{<error>Content can't be blank</error>}, t.errors.to_xml
    assert_nothing_raised { t.errors.to_json }
  end

    This is the exception I am getting in its entirety

    $ ruby -Itest test/cases/validations/presence_validation_test.rb -n /test_accepts_array_arguments/
Loaded suite test/cases/validations/presence_validation_test
Started
F
Finished in 0.048406 seconds.

  1) Failure:
test_accepts_array_arguments(PresenceValidationTest)
    [test/cases/validations/presence_validation_test.rb:44:in `test_accepts_array_arguments'
     /Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `__send__'
     /Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `run'
     /Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/callbacks.rb:408:in `_run_setup_callbacks'
     /Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/testing/setup_and_teardown.rb:65:in `run']:
Exception raised:
Class: <SystemStackError>
Message: <"stack level too deep">
---Backtrace---
test/cases/validations/presence_validation_test.rb:44:in `to_json'
test/cases/validations/presence_validation_test.rb:44:in `test_accepts_array_arguments'
test/cases/validations/presence_validation_test.rb:44:in `test_accepts_array_arguments'
/Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `__send__'
/Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `run'
/Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/callbacks.rb:408:in `_run_setup_callbacks'
/Users/nsingh/dev/working/rails_tickets/r_4890/vendor/bundler/gems/rails-16a5e918a06649ffac24fd5873b875daf66212ad-master/activesupport/lib/active_support/testing/setup_and_teardown.rb:65:in `run'
---------------

1 tests, 6 assertions, 1 failures, 0 errors

    June 25th, 2010 @ 04:19 PM

  11. José Valim
    José Valim
    • Importance changed from to Low

    Ok, I went deep down this problem and it turned out to be the JSON gem overwriting the to_json implementation for all core classes so Rails implementation was never used. The gem implementation does not work with hash inheritance, making ActiveSupport::OrderedHash fail when serialized to_json (and consequently ActiveModel::Errors). Patch is coming soon.

    June 26th, 2010 @ 10:51 AM

  12. Repository
    Repository
    • State changed from open to resolved

    (from [7bd85a8fc2d216a5e2b1d0380df572f782a54d1c]) Work around the fact the JSON gem was overwriting to_json implementation for all Ruby core classes.

    This is required because the JSON gem is incompatible with Rails behavior and was not allowing ActiveModel::Errors to be serialized.
    So we need to ensure Rails implementation is the one triggered. [#4890 Correct output from ActiveModel::Errors to_json state:resolved]
    http://github.com/rails/rails/commit/7bd85a8fc2d216a5e2b1d0380df572...

    June 26th, 2010 @ 11:10 AM

  13. Jeremy Kemper
    Jeremy Kemper
    • Milestone set to 3.0.2

    October 15th, 2010 @ 11:01 PM