#4890 Correct output from ActiveModel::Errors to_json - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4890 ✓resolved

Correct output from ActiveModel::Errors to_json

Reported by Samuel Kadolph | June 17th, 2010 @ 06:02 PM | in 3.0.2

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"]

Comments and changes to this ticket

You flagged this item as spam.
Samuel Kadolph June 17th, 2010 @ 06:11 PM
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.
- correct_output_from_errors_to_json.diff 628 Bytes
You flagged this item as spam.
Samuel Kadolph June 17th, 2010 @ 06:45 PM
Complete patch is attached. (Sorry, new to the whole patching rails thing.)
- 0001-fixed-activemodel-errors-to_json-output.patch 1.4 KB
José Valim June 22nd, 2010 @ 04:39 PM
- 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.
Neeraj Singh June 23rd, 2010 @ 10:07 PM
Trying to create a test for this one but running into stack level too deep issue.

Hopefully I'll have a test patch soon. :-)
You flagged this item as spam.
Samuel Kadolph June 23rd, 2010 @ 10:11 PM
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.
José Valim June 25th, 2010 @ 08:41 AM
Neeraj, I think it's expected that you run into StackLevelTooDeep before the patch is applied. :) It means your test is correct!
Neeraj Singh June 25th, 2010 @ 03:48 PM
- 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 :-)
José Valim June 25th, 2010 @ 04:05 PM
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 ;))?
Neeraj Singh June 25th, 2010 @ 04:15 PM
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'
```

Neeraj Singh June 25th, 2010 @ 04:19 PM

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

José Valim June 26th, 2010 @ 10:51 AM
- 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.
Repository June 26th, 2010 @ 11:10 AM
- 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 state:resolved]
http://github.com/rails/rails/commit/7bd85a8fc2d216a5e2b1d0380df572...
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Shared Ticket Bins (Sort)

↓↑ drag 455 Open Bugs
↓↑ drag 0 Recent Open Tickets
↓↑ drag 87 Open Patches
↓↑ drag 0 Open Doc Patches
↓↑ drag 542 Stale Tickets
↓↑ drag 3 Verified Patches
↓↑ drag 87 Stale Patches
↓↑ drag 0 Yesterday's Tickets
↓↑ drag 0 Two Days Ago
↓↑ drag 0 This Week
↓↑ drag 0 Last Week
↓↑ drag 7 Bugmash
↓↑ drag 0 Bugmash review queue
↓↑ drag 5 Rails 3 High Priority Tickets

People watching this ticket

Attachments

correct_output_from_errors_... 628 Bytes
0001-fixed-activemodel-erro... 1.4 KB

Referenced by

4890 Correct output from ActiveModel::Errors to_json This is required because the JSON gem is incompatible wit...

Rails Ruby on Rails → 3.0.2

Correct output from ActiveModel::Errors to_json

Comments and changes to this ticket

Samuel Kadolph June 17th, 2010 @ 06:11 PM

Samuel Kadolph June 17th, 2010 @ 06:45 PM

José Valim June 22nd, 2010 @ 04:39 PM

Neeraj Singh June 23rd, 2010 @ 10:07 PM

Samuel Kadolph June 23rd, 2010 @ 10:11 PM

José Valim June 25th, 2010 @ 08:41 AM

Neeraj Singh June 25th, 2010 @ 03:48 PM

José Valim June 25th, 2010 @ 04:05 PM

Neeraj Singh June 25th, 2010 @ 04:15 PM

Neeraj Singh June 25th, 2010 @ 04:19 PM

José Valim June 26th, 2010 @ 10:51 AM

Repository June 26th, 2010 @ 11:10 AM

Jeremy Kemper October 15th, 2010 @ 11:01 PM

Create your profile

Shared Ticket Bins (Sort)

People watching this ticket

Attachments

Tags

Referenced by

Pages

Rails Ruby on Rails → 3.0.2

Keyword searching

Correct output from ActiveModel::Errors to_json

Comments and changes to this ticket

Create your profile

Shared Ticket Bins (Sort)

People watching this ticket

Attachments

Tags

Referenced by

Pages