ActiveResource 3.2.1 : ActiveResource::Formats::JsonFormat.decode and ActiveResource::Base.include_root_in_json

So ActiveResource::Base.include_root_in_json is no longer supported, and then I encounter the following within Json Formats:

module ActiveResource
module Formats
module JsonFormat
def decode(json)
Formats.remove_root(ActiveSupport::JSON.decode(json))
end
end
end
end

If ActiveResource::Base.include_root_in_json felt hacky enough to remove, didn’t that change above feel even more hacky?

Yes?

Sorry, I don't understand the question. Is there a bug?

Yes, it violates the principal that the function should do only what it claims to do.

Can you show a code example?

Sure, I’ll create a fresh project to juxtapose the problem this causes with GET and POST on the same model.

Cool, thanks!

Took longer than I thought to get a basic use case that fails. Scenario 2 below is the failure:

My Code:

class User < ActiveResource::Base
self.site = “http://localhost:9000
end

class Image < ActiveResource::Base
end

class ImagePage < ActiveResource::Base
self.element_name = “image_page”
self.collection_name = “irrelevant_never_used_in_this_manner”
end

class ApplicationController < ActionController::Base
def index
Dir.glob("#{Rails.root}/app/models/*.rb").sort.each { |file| require_dependency file }
user = User.find(123)
p user
render :text => “hello world”
end
end

Scenario 1 using remove_root in decode, user.json has “id”

def decode(json)
Formats.remove_root(ActiveSupport::JSON.decode(json))
end

GET /users/123.json

“{“id”:“123”,“image_page”:{“images”:[{“id”:123},{“id”:456}],“total”:2000,“count”:2,“start_index”:0}}”

p user = #<User:0x00000103ab6c98 @attributes={“id”=>“123”, “image_page”=>#<ImagePage:0x00000103ab5cd0 @attributes={“images”=>[#<Image:0x00000103aac158 @attributes={“id”=>123}, @prefix_options={}, @persisted=false>, #<Image:0x00000103aabed8 @attributes={“id”=>456}, @prefix_options={}, @persisted=false>], “total”=>2000, “count”=>2, “start_index”=>0}, @prefix_options={}, @persisted=false>}, @prefix_options={}, @persisted=true>

Scenario 2 using remove_root in decode, user.json does not have “id”

def decode(json)
Formats.remove_root(ActiveSupport::JSON.decode(json))
end

GET /users/123.json

“{“image_page”:{“images”:[{“id”:123},{“id”:456}],“total”:2000,“count”:2,“start_index”:0}}”

p user = #<User:0x00000101133cb8 @attributes={“images”=>[#<Image:0x000001011327a0 @attributes={“id”=>123}, @prefix_options={}, @persisted=false>, #<Image:0x00000103a49e40 @attributes={“id”=>456}, @prefix_options={}, @persisted=false>], “total”=>2000, “count”=>2, “start_index”=>0}, @prefix_options={}, @persisted=true>

Scenario 3 not using remove_root in decode, user.json has “id”

def decode(json)
ActiveSupport::JSON.decode(json)
end

GET /users/123.json

“{“id”:“123”,“image_page”:{“images”:[{“id”:123},{“id”:456}],“total”:2000,“count”:2,“start_index”:0}}”

p user = #<User:0x0000010455a168 @attributes={“id”=>“123”, “image_page”=>#<ImagePage:0x00000104559880 @attributes={“images”=>[#<Image:0x00000104557c88 @attributes={“id”=>123}, @prefix_options={}, @persisted=false>, #<Image:0x00000104557a08 @attributes={“id”=>456}, @prefix_options={}, @persisted=false>], “total”=>2000, “count”=>2, “start_index”=>0}, @prefix_options={}, @persisted=false>}, @prefix_options={}, @persisted=true>

Scenario 4 not using remove_root in decode, user.json does not have “id”

def decode(json)
ActiveSupport::JSON.decode(json)
end

GET /users/123.json

“{“image_page”:{“images”:[{“id”:123},{“id”:456}],“total”:2000,“count”:2,“start_index”:0}}”

p user = #<User:0x000001044166d0 @attributes={“image_page”=>#<ImagePage:0x00000104415f28 @attributes={“images”=>[#<Image:0x00000104414920 @attributes={“id”=>123}, @prefix_options={}, @persisted=false>, #<Image:0x000001044146a0 @attributes={“id”=>456}, @prefix_options={}, @persisted=false>], “total”=>2000, “count”=>2, “start_index”=>0}, @prefix_options={}, @persisted=false>}, @prefix_options={}, @persisted=true>

And I should add, though it was difficult to get myself into this fail case, It was amazingly quick to set up 2 codebases and servers to perform this task.

Great work people! :slight_smile:

Though it’s less than likely a resource would return with no “id” (or only 1 attribute), I think that it needs to be considered. It’s rare to find perfect CRUD.

Imagine that User.find(123) was actually CoolViews.find(“aggregateOfAllHotTopics”). I guess that the service could supply an “id” of “aggregateOfAllHotTopics”, but should that be mandatory?

What about searches? Will searches have an “id”? It’s very likely that the structure of the search won’t be perfect CRUD / REST.

And just to point out, lack of “id” isn’t the problem, the problem is that there’s only 1 attribute. Adding “id” fixes the problem, but also adding any attribute fixes the problem.

Given the code below, it seems you're just starting using Rails and you still don't understand Rails basic concepts.

Shouldn't you consider posting in the user's mailing list first? It's more likely that you'll get better advices there than here...

After you get used to Rails, if you still have questions about how Rails should do things, then your questions will be better discussed here.

Cheers,
Rodrigo.

Rodrigo has made a very important point about which list to use.

How do to X with rails -> rubyonrails-talk
How rails does X -> rails-core

Are you referring to my complete and utter lack of syntactic sugar?
It almost seems as though I deliberately avoid the stuff completely :slight_smile:

This might be the wrong forum, but I’m sure that ActiveResource team are happy that I proofed the issue.

As an expert core member Rodrigo, do you know the alternative to this line?
Dir.glob("#{Rails.root}/app/models/*.rb").sort.each { |file| require_dependency file }

This looks to be a variant of issue #2692:

https://github.com/rails/rails/pull/2692

although the patch in that instance wouldn't fix this bug, as the detection heuristic will guess that image_page is a root element to be removed.

It appears that the problem is that remove_root doesn't have any awareness of the context in which it's operating - really, we should only be removing the appropriately-named root element (if it exists).

--Matt Jones

I’m pointing out something that changed, and would break my project if I upgrade, in ActiveResource which I believe stems from an ActiveResource fix to a problem caused by something that changed in ActiveModel regarding include_root_in_json.

This might be the wrong forum, but this is my fix:

module ActiveResource
class Base
self.include_root_in_json = false
end
module Formats
module JsonFormat
def decode(json)
ActiveSupport::JSON.decode(json)
end
end
end
end

module ActiveModel
module Serializers
module JSON
def as_json(options = nil)
hash = serializable_hash(options)
if include_root_in_json
custom_root = options && options[:root]
hash = { custom_root || self.class.model_name.element => hash }
end
hash
end

def from_json(json)
hash = ActiveSupport::JSON.decode(json)
hash = hash.values.first if include_root_in_json
self.attributes = hash
self
end
end
end
end

I don’t use ActiveRecord, so I don’t know the full ramifications of using that patch above. But it does make ActiveResource work again as I explore my upgrade to 3.2.* .

Thank you Matt, it does appear to be the same issue.

Please don’t remove text from the threads. Not everyone reads them
in Gmail and it makes it really hard to read the messages in an
e-mail client, like Thunderbird.

    Given the

code below, it seems you’re just starting using Rails and you

    still don't understand Rails basic concepts.
      Shouldn't you consider posting in the user's mailing list

first? It’s
more likely that you’ll get better advices there than here…

      After you get used to Rails, if you still have questions

about how Rails
should do things, then your questions will be better discussed
here.

Cheers,
Rodrigo.

    Are you referring to my complete and utter lack of syntactic

sugar?

    It almost seems as though I deliberately avoid the stuff

completely :slight_smile:

    This might be the wrong forum, but I'm sure that

ActiveResource team are happy that I proofed the issue.

    As an expert core member Rodrigo, do you know the alternative to

this line?

    Dir.glob("#{Rails.root}/app/        models/*.rb").sort.each { |file|

require_dependency file }

Please don't remove text from the threads. Not everyone reads them

in Gmail and it makes it really hard to read the messages in an
e-mail client, like Thunderbird.

---

class User < ActiveResource::    Base


  self.site = ["http://localhost:9000"](http://localhost:9000)


end"

---



I don't call this "lack of syntactic sugar". This is completely

wrong Ruby code and makes me feel that you don’t know Ruby enough.
Please, post the full code as it is actually easier to read the code
with all its “syntactic sugar”.

I'm not a Rails core member, nor even an expert in Rails. And I

don’t know why you need this. Shouldn’t Rails be automatically
loading those files? Why should they be loaded in any particular
order?

Cheers,

Rodrigo.

This is as full as I need my User.rb model to be, in practice and for the proof on this thread. Lord praise the fat model, which is encapsulated nicely inside of ActiveResource and ActiveModel.

Common sense advice: When determining how fat your model is, also consider the classes/modules from which your model inherits.

You don’t have to add unnecessary filler code to your model to make it fatter than the controller from which it is invoked. Though I can imagine that there’s some in syntactic sugar land that will pervert the meaning of “fat model / skinny controller” in that way :slight_smile:

And syntactic sugar is not legible to the next person who is looking at your code.

Thank you for the advice about trimming the quoted text.

On Sat, Mar 3, 2012 at 9:44 PM, Rodrigo Rosenfeld Rosas >

---

class User < ActiveResource::Base
self.site = "http://localhost:9000"
end"
---

I don't call this "lack of syntactic sugar". This is completely wrong Ruby
code and makes me feel that you don't know Ruby enough. Please, post the
full code as it is actually easier to read the code with all its "syntactic
sugar".

please have a look at
http://api.rubyonrails.org/classes/ActiveResource/Base.html
and see where this is coming from. but maybe I might just got the
whole thing wrong :wink:

regards, Kristian

But he had this strong sense that I did something wrong. Shouldn’t that be good enough for you?!?!?!?!

He glanced at bitter, unsweetened, functionally sound and readable code. Is it his fault that he just assumed?

:slight_smile:

Yes, it was my fault here as I haven’t used ActiveResouce::Base
before and I was assuming that “self.site” was a random attribute to
be included in JSON.

Sorry for the confusion and bad assumptions. Clearly I wasn't in a

good day after fighting the whole week with Groovy:

http://rosenfeld.herokuapp.com/en/articles/ruby-rails/2012-03-04-how-nokogiri-and-jruby-saved-my-week

Sorry again,

Rodrigo.