Misleading documentation for ActiveModel::Serializers::JSON#as_json #6578

Closed
frodsan opened this Issue Jun 1, 2012 · 14 comments

Projects

None yet

3 participants

@frodsan

Hi guys, I was checking api documentation and i found that ActiveModel::Serializers::JSON#as_json doesn't work
like the examples listed in the documentation:

# The option <tt>include_root_in_json</tt> controls the top-level behavior
# of +as_json+. If true (the default) +as_json+ will emit a single root
# node named after the object's type. For example:
#
#   user = User.find(1)
#   user.as_json
#   # => { "user": {"id": 1, "name": "Konata Izumi", "age": 16,
#                   "created_at": "2006/08/01", "awesome": true} }
#
#   ActiveRecord::Base.include_root_in_json = false
#   user.as_json
#   # => {"id": 1, "name": "Konata Izumi", "age": 16,
#         "created_at": "2006/08/01", "awesome": true}

I'm having this when i tried it in console:

irb(main):002:0> user.as_json
=> {"id"=>1, "name"=>"Konata Izumi", "age"=>16, "awesome"=>true} # where is the root?

irb(main):003:0> ActiveRecord::Base.include_root_in_json # according to docs, this must be true?
=> false

irb(main):004:0> ActiveRecord::Base.include_root_in_json = true
=> true
user.as_json

=> {"user"=>{"id"=>1, "name"=>"Konata Izumi", "age"=>16, "awesome"=>true}}

But if i include the module in a class, i'm getting the desired result:

class Person
  include ActiveModel::Model
  include ActiveModel::AttributeMethods
  include ActiveModel::Serializers::JSON

  attr_accessor :name, :age

  def attributes
    instance_values
  end
end
irb(main):025:0> person = Person.new(name: "Francesco", age: 22)
=> #<Person:0x007fdc73f02d08 @name="Francesco", @age=22>
irb(main):026:0> person.as_json
=> {"person"=>{"name"=>"Francesco", "age"=>22}}
irb(main):027:0> person.include_root_in_json
=> true
irb(main):029:0> Person.include_root_in_json = false
=> false
irb(main):030:0> person.as_json
=> {"name"=>"Francesco", "age"=>22}

Probably, because this code in ActiveModel::Serializers::JSON:

included do
  extend ActiveModel::Naming
  extend ActiveModel::Configuration

  config_attribute :include_root_in_json
  self.include_root_in_json = true
end

I'm not sure if this attribute must be false by default or
just update the documentation. I will appreciate your help.

Thanks :)

@rafaelfranca
Ruby on Rails member

I tink that the documentation and code is correct. The options is true by default if you are including the ActiveModel::Serializers::JSON module.

Maybe I'm missing something.

@frodsan

If you try the documentation example in rails console, you get another results like i said in the description:

#   user = User.find(1)
#   user.as_json
#   # => { "user": {"id": 1, "name": "Konata Izumi", "age": 16,
#                   "created_at": "2006/08/01", "awesome": true} }
#
#   ActiveRecord::Base.include_root_in_json = false
#   user.as_json
#   # => {"id": 1, "name": "Konata Izumi", "age": 16,
#         "created_at": "2006/08/01", "awesome": true}
irb(main):025:0> person = Person.new(name: "Francesco", age: 22)
=> #<Person:0x007fdc73f02d08 @name="Francesco", @age=22>
irb(main):026:0> person.as_json
=> {"person"=>{"name"=>"Francesco", "age"=>22}}
irb(main):027:0> person.include_root_in_json
=> true
irb(main):029:0> Person.include_root_in_json = false
=> false
irb(main):030:0> person.as_json
=> {"name"=>"Francesco", "age"=>22}
@frodsan frodsan closed this Jun 1, 2012
@frodsan frodsan reopened this Jun 1, 2012
@rafaelfranca
Ruby on Rails member

But this is not the correct behavior?

@frodsan

@rafaelfranca Sorry, i paste the correct result. Actually, i wanted to show you this:

# The option <tt>include_root_in_json</tt> controls the top-level behavior
# of +as_json+. If true (the default) +as_json+ will emit a single root
# node named after the object's type. For example:
#
#   user = User.find(1)
#   user.as_json
#   # => { "user": {"id": 1, "name": "Konata Izumi", "age": 16,
#                   "created_at": "2006/08/01", "awesome": true} }
#
#   ActiveRecord::Base.include_root_in_json = false
#   user.as_json
#   # => {"id": 1, "name": "Konata Izumi", "age": 16,
#         "created_at": "2006/08/01", "awesome": true}
irb(main):002:0> user.as_json
=> {"id"=>1, "name"=>"Konata Izumi", "age"=>16, "awesome"=>true} # where is the root?

irb(main):003:0> ActiveRecord::Base.include_root_in_json # according to docs, this must be true
=> false

irb(main):004:0> ActiveRecord::Base.include_root_in_json = true
=> true
user.as_json

=> {"user"=>{"id"=>1, "name"=>"Konata Izumi", "age"=>16, "awesome"=>true}}
@rafaelfranca
Ruby on Rails member

Ahh. Got it. I think we changed this value, by default in the applicaiton, but the default value is still true

railties/lib/rails/generators/rails/app/templates/config/initializers/wrap_parameters.rb.tt
14:  self.include_root_in_json = false
@frodsan

If ActiveRecord::Base.include_root_in_json is false, why don't we make false too in ActiveModel::Serializers::JSON? It's confusing, and i think the documentation is wrong too. I wanted to open to discussion before fix the docs.

@frodsan

If we keep the two values, i will have to split the documentation in two parts: One for objects that inherit from ActiveRecord::Base and another for objects that include ActiveModel::Serializers::JSON.

@rafaelfranca
Ruby on Rails member

It is still true by default. So if you comment that like it will be true.

I don't know the reason to not change ActiveModel::Serializers::JSON, but I think that we can. cc/ @josevalim

@frodsan

It is true by default for ActiveModel::Serializers::JSON, but for ActiveRecord is false:

ActiveSupport.on_load(:active_record) do
  self.include_root_in_json = false
end
@carlosantoniodasilva
Ruby on Rails member

Hey, seems like I'm late to the party :)

IIRC, AR#include_root_in_json is false since wrap parameters take care of wrapping them for input values, so generating / receiving attribute hashes is always unwrapped by default.

But using only AM serializers and having different functionality from AR, considering there could be both in the same application, might be confusing I guess?

The problem is that include_root_in_json + wrap parameters doesn't seem to play well with each other.

@frodsan

@carlosantoniodasilva what do you suggest?

@carlosantoniodasilva
Ruby on Rails member

I don't know the solution yet, but apparently having these different results is inconsistent and should be avoided. @josevalim thoughts?

@frodsan

I sent a proposal, maybe you can review it :)

@rafaelfranca rafaelfranca pushed a commit that closed this issue Jun 7, 2012
Francesco Rodriguez change AMS::JSON.include_root_in_json default value to false
Changes:

* Update `include_root_in_json` default value to false for default value
  to false for `ActiveModel::Serializers::JSON`.
* Remove unnecessary change to include_root_in_json option in
  wrap_parameters template.
* Update `as_json` documentation.
* Fix JSONSerialization tests.

Problem:

It's confusing that AM serializers behave differently from AR,
even when AR objects include AM serializers module.

    class User < ActiveRecord::Base; end

    class Person
      include ActiveModel::Model
      include ActiveModel::AttributeMethods
      include ActiveModel::Serializers::JSON

      attr_accessor :name, :age

      def attributes
        instance_values
      end
    end

    user.as_json
    => {"id"=>1, "name"=>"Konata Izumi", "age"=>16, "awesome"=>true}
    # root is not included

    person.as_json
    => {"person"=>{"name"=>"Francesco", "age"=>22}}
    # root is included

    ActiveRecord::Base.include_root_in_json
    => false

    Person.include_root_in_json
    => true

    # different default values for include_root_in_json

Proposal:

Change the default value of AM serializers to false, update
the misleading documentation and remove unnecessary change
to false of include_root_in_json option with AR objects.

    class User < ActiveRecord::Base; end

    class Person
      include ActiveModel::Model
      include ActiveModel::AttributeMethods
      include ActiveModel::Serializers::JSON

      attr_accessor :name, :age

      def attributes
        instance_values
      end
    end

    user.as_json
    => {"id"=>1, "name"=>"Konata Izumi", "age"=>16, "awesome"=>true}
    # root is not included

    person.as_json
    => {"name"=>"Francesco", "age"=>22}
    # root is not included

    ActiveRecord::Base.include_root_in_json
    => false

    Person.include_root_in_json
    => false

    # same behaviour, more consistent

Fixes #6578.
ab11a27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment