Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveRecord to_json doesn't invoke :include's to_json #576

Closed
lighthouse-import opened this issue May 16, 2011 · 46 comments
Closed

ActiveRecord to_json doesn't invoke :include's to_json #576

lighthouse-import opened this issue May 16, 2011 · 46 comments

Comments

@lighthouse-import
Copy link

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/610
Created by David Burger - 2008-07-13 04:50:26 UTC

When using ActiveRecord's to_json with the parameter :include => associations to include associations, the association's to_json method is not invoked to create the json for that association. This prevents the ability to override the to_json method of a class and have it used appropriately when it is serialized as part of an :include. This patch corrects this so that the to_json method of a class will be used to produce the json for that class when the class is used in an :include =>.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Joshua Peek - 2008-10-28 16:25:14 UTC

Staling out, please reopen if this is still a problem.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by bentlegen - 2009-02-19 04:02:32 UTC

This is still an open issue for our team. Please consider re-opening.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Jörg Battermann - 2009-02-23 00:43:32 UTC

Joshua / or anyone else listening,

this still -is- a problem (in Rails 2.2.2). Basically I have the following:

My 'User' model has an explicit 'def to_json' declaration that hides a couple of the model's attributes and also adds a couple methods to the output. All working fine when e.g. doing a @user_instance.to_json

However, e.g. my photo model belongs_to an user. now when doing a @photo_instance.to_json(:include => [:user]), all my user-class's def to_json get ignored and the full user model is serialized, ignoring my :except and :methods declarations in the user's to_json method.

So basically yep, the to_json method of included ':include =>[:abc, :def]'s still don't get called.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Seth Ladd - 2009-02-27 20:12:38 UTC

This is still an issue. I'd like to re-open, but I don't think normal users can.

Please reopen.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Matt Jones - 2009-02-27 21:46:25 UTC

@seth - reopening at your request. I took a look at this, and it
does appear that the problem still exists.

At a quick glance, the fix is going to require changes to:
(around line 80, active_record/serialization.rb)

if records.is_a?(Enumerable)
  serializable_record[association] = records.collect { |r| self.class.new(r, opts).serializable_record }
else
  serializable_record[association] = self.class.new(records, opts).serializable_record
end

Ideally, it would seem nice to have a hash equivalent to to_json that would return the desired hash without flattening it to a string first.

Alternatively, one could borrow much of the implementation in xml_serializer.rb, but that's really just a sign that the abstract implementation of add_associations needs to be tweaked.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Ken Collins - 2009-07-19 01:13:17 UTC

I just had the same issue. I've got a model tree and I just defined to_json(options={}) in each model that add :only defaults and their own :include options. I was hoping that I could go all the way back up the chain at the end of this and just call #to_json on the parent object and was disappointed to some very very odd behavior.

I also noticed that if I use :only on the top level, it applies it to all levels of :include. I'm searching for other tickets that address this but will try this patch too and let you know.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Ken Collins - 2009-07-20 13:36:50 UTC

OK, I did some poking around in master this weekend and this is what I found. First, this patch seems outdated with the new plugable JSON backends. Second, giving the new use of how #to_json is really calling #as_json as being the best way to remain JSON agnostic, this ticket may be moot for 3.0. Here is some model examples. So using #as_json in rails 3 does what I would hope and expect.

class Foo
  JSON_ATTRS = [:id,:created_at]
  has_many :bars
  def as_json(options=nil)
    attributes.slice(*JSON_ATTRS).merge(:bars => bars)
  end
end

class Bar
  JSON_ATTRS = [:id,:owner_id,:owner_type,:etc]
  has_many :bats
  def as_json(options=nil)
    attributes.slice(*JSON_ATTRS).merge(:bats => bats)
  end
end

class Bat
  JSON_ATTRS = [:not_this,:or_that]
  def as_json(options=nil)
    attributes.except(*JSON_ATTRS)
  end
end


Foo.first.to_json # => Will include all associations defined by model.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Ken Collins - 2009-07-20 13:39:41 UTC

Better attempt at code formatting

class Foo
  JSON_ATTRS = [:id,:created_at]
  has_many :bars
  def as_json(options=nil)
    attributes.slice(*JSON_ATTRS).merge(:bars => bars)
  end
end

class Bar
  JSON_ATTRS = [:id,:owner_id,:owner_type,:etc]
  has_many :bats
  def as_json(options=nil)
    attributes.slice(*JSON_ATTRS).merge(:bats => bats)
  end
end

class Bat
  JSON_ATTRS = [:not_this,:or_that]
  def as_json(options=nil)
    attributes.except(*JSON_ATTRS)
  end
end


Foo.first.to_json # => Will include all associations defined by model.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by will bailey - 2009-08-07 20:10:06 UTC

It appears that in Rails 2.3.3 the as_json method is not called on ActiveRecord Objects when to_json is called. Is the code example in the previous post only planned to work in Rails 3? I'm wondering what the reasoning is behind not having to_json invoke as_json in active_record/serializers/json_serializer.rb

I would expect it to work like this:

    def to_json(options = {})
      return ActiveSupport::JSON.encode(as_json) if respond_to?(:as_json)
      hash = Serializer.new(self, options).serializable_record
      hash = { self.class.model_name.element => hash } if include_root_in_json
      ActiveSupport::JSON.encode(hash)
    end

    # Remove this implement in subclasses as desired
    # def as_json(options = nil) self end #:nodoc:

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Bart Zonneveld - 2009-10-07 09:35:56 UTC

+1, although the original patch is apparently outdated.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Jarred Nicholls - 2009-10-07 22:05:25 UTC

While this is clearly something that wasn't thought hard about (before 2.3.3 and after 2.3.3, irregardless of the new as_json functionality), I came up a simple solution to achieve the same effect until there is a better (more integrated) solution. In fact, this "magic method" solution I came up with could easily be an ActiveRecord::Base class method (much like "serialize" or "attr_protected", etc.) to define a list of attributes that are deemed serializable.

First I overrode the "serializable_attribute_names" method in ActiveRecord::Serialization::Serializer class:

module ActiveRecord
  module Serialization
    class Serializer
      def serializable_attribute_names
        attribute_names = @record.respond_to?(:serializable_attributes) ? @record.serializable_attributes : @record.attribute_names

        if options[:only]
          options.delete(:except)
          attribute_names = attribute_names & Array(options[:only]).collect { |n| n.to_s }
        else
          options[:except] = Array(options[:except]) | Array(@record.class.inheritance_column)
          attribute_names = attribute_names - options[:except].collect { |n| n.to_s }
        end

        attribute_names
      end
    end # class Serializer
  end # module Serialization
end # module ActiveRecord

Notice the change in the first line of the method, where I check to see if an ActiveRecord responds to "serializable_attributes". If it does, I take the resulting array of attributes from that rather than the "attribute_names" method. This can be the method we implement in our models:

class User < ActiveRecord::Base
  def serializable_attributes
    # return array of attributes names we deem are safe to serialize
    attribute_names - ["secret_key"]
  end
end

And Viola!, the net effect we wanted from overriding as_json...only now we are also compatible with XML serialization.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Seth Ladd - 2009-10-13 05:24:17 UTC

FWIW this is still a problem in Rails 2.3.4. I've created a sample app, with tests, that illustrates this problem.

http://github.com/sethladd/to_json_busted

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Jarred Nicholls - 2009-10-13 14:22:42 UTC

Yep it is. My code above will make it work as expected if the "expected method" is used, as opposed to overriding as_json. The net effect is the same as what was expected from overriding as_json, as well as working for XML serialization at the same time. I looked into the serialization code and it's not as easy to fix as_json as one would think, without adding JSON serialization logic directly into the Serializer class, which is suppose to be agnostic to the serialization format.

Hope that helps.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by wdlindmeier - 2009-10-22 18:44:22 UTC

I made the following patch to Rails 2.2.2 which fixed the problem for JSON serialization.

class ActiveRecord::Base

  def to_json_options(options={})
    options.symbolize_keys!
  end        

end

class ActiveRecord::Serialization::JsonSerializer

  def initialize(record, options = {})
    super
    @options = @record.to_json_options(@options)
  end

end

class Submission < ActiveRecord::Base

  def to_json_options(options={})
    returning(super) do |opts|
      opts[:methods] ||= []
      opts[:methods] = opts[:methods] | ['type', 'photo_urls']
    end
  end

end

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Aaron Gibralter - 2009-12-11 06:58:04 UTC

Does anyone know if the next Rails release will address this? It seems like really unexpected behavior for includes to just dump all of the children's attributes willy nilly.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Jarred Nicholls - 2009-12-11 13:37:00 UTC

Well I have 2.3.5 and this was not addressed. See my solution above - I just overwrote the function that determines what attributes are serializable in ActiveSupport::Serialization::Serializer, and you can add a public method to your models that can return an array of (String) attribute names that can be serialized. This is a fix for both JSON and XML serialization. Of course, this solution doesn't allow for ad hoc decisions on what attributes to serialize when using "include", but it at least makes sure "private" attributes aren't ever included in serialization.

My suggestion (until "include" will accept attributes to include/exclude for child attributes) is to include a method rather than your ActiveRecord relationship that returns particular attributes of your relationship, and can in turn nest deep into the AR relationship tree. It's not a good solution, it's just a workable solution. The moment you have 3 or more "special methods" because you want different attributes in 3 different situations, you will start to taste vomit in your mouth (I know I would). Until there is an elegant/incorporated solution (which we can certainly roll ourselves and submit as a patch), I've given the above solution to at least be able to exclude secret or unnecessary attributes from our models during serialization (at any level of the "include" hierarchy), and have just learned to live with possibly having too much information in my serialized data.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Aaron Gibralter - 2009-12-11 19:09:29 UTC

Ah -- true that works -- I also found this plugin, http://github.com/vigetlabs/serialize_with_options, that seems to preserve the options of included associations; that is, if you use serialize_with_options { } for all your models in the includes.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Jeremy Kemper - 2010-05-04 17:48:33 UTC

[bulk edit]

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by kendall (at kendagriff) - 2010-11-22 17:38:47 UTC

Also having this problem in Rails 3.0.3.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Alex Neth - 2010-12-16 07:31:01 UTC

Building a json api is a bit of a headache because of this bug.

I tried to override as_json in each object class, but this only works when as_json/to_json is called directly on an object, not when it is included in another.

@posts.as_json(:include => :author)

includes default author properties instead of using the as_json method.

I ended up creating a static property on my objects and using that wherever necessary, which is in some ways a better solution anyway, since not every serialization will require the same attributes.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by rails - 2011-03-17 00:00:12 UTC

This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Betelgeuse - 2011-03-17 08:00:12 UTC

Tested with rails 3.0.5 and still broken [state:open].

in to_json model:

  def as_json(options = {})
    options = {
      :include => [:a, :b, :c],
      :except => :a_id
    }
    super options
  end

a: belongs_to
b: has_many :through
c: has_many :through

for a, b, c:

  JSON_OPTIONS = {:only =>  [:id, :a] }

  def as_json(options = {})
    super JSON_OPTIONS
  end

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Betelgeuse - 2011-03-17 08:00:53 UTC

[state: open]

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Daniel - 2011-03-18 13:25:09 UTC

Still broken in 2.3.11 as well.

@lighthouse-import
Copy link
Author

Imported from Lighthouse.
Comment by Daniel - 2011-03-18 13:28:13 UTC

[state:open]

@lighthouse-import
Copy link
Author

Attachments saved to Gist: http://gist.github.com/969907

@unamashana
Copy link

This seems to fix it for me - https://gist.github.com/437968d6a5f9cbc93a8d

Is this an acceptable solution? I can add some specs and send a pull request in case this is what is needed.

@sandstrom
Copy link
Contributor

@prateekdayal Nice patch, though it would be better to modify the serializable_hash method in ActiveModel (instead of active record).

@loudin
Copy link

loudin commented Jul 22, 2011

Has this been fixed? I'm still having the same exact problem in Rails 3.0.8.

@unamashana
Copy link

I have just sent a pull request #2200

@runemadsen
Copy link

Still a problem in Rails 3.0.9. Thanks @prateekdayal for fixing this, would love to see it merged

@arbales
Copy link

arbales commented Jul 28, 2011

+1

1 similar comment
@zapnap
Copy link
Contributor

zapnap commented Aug 8, 2011

+1

@runemadsen
Copy link

I just noticed that if you use "methods" instead of "include" it will call the as_json method on the association.

# will not call the as_json method on MyModel instances
@parent.to_json(:include => [ :mymodels ])

# will call the as_json method on MyModel instances
@parent.to_json(:methods => [:mymodels])

So that's a quick way around the bug

@loudin
Copy link

loudin commented Aug 17, 2011

I tried your workaround, runemadsen, but I couldn't get it to work. However,
I did find another work-around that involves manually constructing the JSON
representation of each object in the as_json method.

User Model:
def as_json(options={})
{
:id => self.id,
:first_name => self.first_name,
:last_name => self.last_name,
:address => self.address
}
end

Order Model:
def as_json(options={})
{
:id => self.id
:date => self.date,
:user => self.user
}
end

The Order model will render the as_json method of the User model. It might
be a little tedious, but it does work nicely.

Let's also say you need multiple JSON views within each object, so you want
to hide the address for the User. Simple enough:

User Model:
def as_json(options={})
if options[:view] && options[:view] == "address"
{
:id => self.id,
:first_name => self.first_name,
:last_name => self.last_name,
:address => self.address
}
else
{
:id => self.id,
:first_name => self.first_name,
:last_name => self.last_name
}
end

And in the Orders model:
def as_json(options={})
{
:id => self.id
:date => self.date,
:user => self.user.as_json(:view => "address")
}
end

I'm sure there's a more elegant way to accomplish this (and one that doesn't
break the MVC structure), but I just discovered this workaround and it's
suiting my needs.

On Mon, Aug 15, 2011 at 5:04 PM, runemadsen <
reply@reply.github.com>wrote:

I just noticed that if you use "methods" instead of "include" it will call
the as_json method on the association.

@parent.to_json(:include => [ :mymodels ])

# will call the as_json method on MyModel instances

@parent.to_json(:methods => [:mymodels])

So that's a quick way around the bug

## 

Reply to this email directly or view it on GitHub:
https://github.com/rails/rails/issues/576#issuecomment-1811067

@runemadsen
Copy link

@loudin Strange....

Anyway, often I just end up with doing my own JSON implementation like you. But instead of overriding as_json, I create a to_hash method and then call to_json on the hash. This works really well with associations:

class Parent
  def to_hash
    {
      :name => name,
      :children => children.map { |c| c.to_hash }
    }
  end
end

class Child
  def to_hash
    {
      :name => name
    }
  end
end

# to output json
@parent.to_hash.to_json

@jmacdonald
Copy link

+1

@tamoyal
Copy link

tamoyal commented Dec 6, 2011

As of Rails 3.1, I still see serialization issues and had to do the custom hash method proposed above. I'm assuming this just hasn't been looked at in a while but I'm happy to dive into details and/or help fix if any contributor wants to work through it

@rxaviers
Copy link

I have found out we have an open pull request to fix that issue: #2200

@davidw
Copy link
Contributor

davidw commented Dec 4, 2012

This is still a problem as of Rails 3.2.8.

@freemanoid
Copy link

I can confirm it on 3.2.11

@fess89
Copy link

fess89 commented Feb 19, 2013

Still open in Rails 3.2.12

@filso
Copy link

filso commented May 8, 2013

same in 3.2.13

@sergiocampama
Copy link

still happens in rails-4.0.0-rc1

@dannytip
Copy link

dannytip commented Jul 3, 2013

Will this ever be fixed?

@sergiocampama
Copy link

Actually, reading the source, I don't think this is a bug, it is simply not supposed to work that way. I think that the model is just that, and overriding the as_json method to only show selected attributes is placing view logic (as the most common use of json representations is to "view" the model elsewhere) on the model (which MVC is against). The correct way of doing this, from what I understood from the source, is:

object.to_json({
  only: [:first_name, :last_name],
  include: [
    children: {only: [:age, :count]}
  ]
})

which, IMHO, is controller/view logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests