This project is archived and is in readonly mode.
ActiveResource does not serialize XML correctly
-
Neeraj Singh
Even Rails beta 3 is working fine. Problem is in the edge version.
I introduced a major change the way to_xml works a week back. Looks like that commit is causing the error. Looking into this issue.
-
Neeraj Singh
On further analysis the commit that is causing this problem is this one.
http://github.com/rails/rails/commit/7cd1d37a51f5f53f8fc1360f886d26...
-
Neeraj Singh
- Assigned user set to Jeremy Kemper
Commit http://github.com/rails/rails/commit/7cd1d37a51f5f53f8fc1360f886d26... deleted following line
self.class.format.encode(attributes, {:root => self.class.element_name}.merge(options))With that line gone there is no easy way to pass self.class.element_name as root.
I would like to submit a patch for it but I do not see any easy way. Please provide how should I proceed.
The way I see it there is need for to_xml method in the class.
I will work on a test patch that will add a failing test.
I know usually one should not directly assign a ticket to someone . However I am assigning this ticket to Jeremy because he signed off on the last commit.
-
Jeremy Kemper
- Milestone cleared.
- State changed from new to open
- Assigned user changed from Jeremy Kemper to Santiago Pastorino
Doh! Needs a regression test + fix.
-
Santiago Pastorino
- Tag changed from rails 3.0 beta, activeresource, serialization, to_xml to rails 3.0 beta, activeresource, patch, serialization, to_xml
- State changed from open to verified
On the patches i've made tests on ActiveResource use the methods to_xml and to_json.
I refactored AMo::Name and allow element and collection attributes to be writable.
Then i make ActiveRecord delegate to AMo the use of element_name and collection_name instead of managing his own copy itself.
Added a few tests also. -
José Valim
Pastorino, your patch is changing the cached values. This means that every time I call human, it's going to execute this line:
ActiveSupport::Inflector.humanize(@element).freeze
And you should change the @human value if the @element changes. Consider this case:
class Post < ActiveResource::Base self.element_name = "api-post" end Post.model_name.human #=> "Post"You should also cache the collection using "@collection ||=".
-
Santiago Pastorino
- Tag changed from rails 3.0 beta, activeresource, patch, serialization, to_xml to rails 3.0 beta, activeresource, patch, serialization, to_json, to_xml
Yes, you're right on the human cache. The other one shouldn't be cached because i want to return @collection only if someone does self.collection_name = "api-posts" if it's not setted we should return the inflection thing i did.
Patch again. -
José Valim
Caching the result would still work for collection when you set it through self.collection_name
-
Santiago Pastorino
Hehehe, yes you're right. Perhaps i need to go bed :).
New patch here. -
Jeremy Kemper
ActiveModel::Nameisn't meant to be a mutable object, though. You could set and cache these on the instance itself, or instantiate and cache a newNamewith updated values. -
Santiago Pastorino
Ok i will redo with this in mind ;)
-
Santiago Pastorino
Take a look now Jeremy, thanks.
-
Santiago Pastorino
patch here
-
Santiago Pastorino
With the new changes
-
Santiago Pastorino
As Jeremy requested ;), thanks Jeremy
-
Jeremy Kemper
Pending tests for
ActiveModel::Namechanges. -
Santiago Pastorino
I hope now the patch is ok
-
Jeremy Kemper
Setting a custom collection name is not preserved:
name.collection = 'foo' name.element = 'bar' assert_equal 'foo', name.collection # but it returns 'bars'Also, this is unneeded
since you defined the writer methods already.- attr_reader :singular, :plural, :element, :collection, :partial_path - alias_method :cache_key, :collection + attr_reader :singular, :plural, :partial_path + attr_accessor :element, :collection + alias_method :cache_key, :collection -
Santiago Pastorino
Case fixed and more tests added
-
Santiago Pastorino
Jeremy, I leave here the another possibility we discussed
-
Santiago Pastorino
Jeremy I've uploaded again the two patches the first i leave as it was and the second one i've fixed a bit to cache partial_path.
Please tell me if this need something more to be done. -
Repository
(from [bea3c26833ad3e1e94f7331e0553a4e2164e7de5]) Make ActiveResource serialize XML correctly when element_name is set.
[#4529 ActiveResource does not serialize XML correctly]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/bea3c26833ad3e1e94f7331e0553a4... -
Santiago Pastorino
Jeremy backport of tests http://github.com/spastorino/rails/commit/6267187f3c2c703cb1f1dcb1b... but nothing broke on 2-3-stable
-
Santiago Pastorino
- State changed from verified to resolved
-
Repository
(from [c4ef7bb2a0a2e2351f5ed2b1f98c9b40c441591e]) to_json and to_xml tests added to ActiveResource
[#4529 ActiveResource does not serialize XML correctly state:resolved]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/c4ef7bb2a0a2e2351f5ed2b1f98c9b... -
Chris Griego
This changeset includes a regression or an intentional change that should be documented.
module Foo class Bar < ActiveResource::Base end endBefore the changeset, this would requests the url /bars
After the changeset, this now requests the url /foo/bars -
José Valim
- State changed from resolved to open
-
Santiago Pastorino
- Milestone cleared.
-
Santiago Pastorino
A better one ;)
-
Santiago Pastorino
- Assigned user changed from Santiago Pastorino to José Valim
-
Repository
- State changed from open to committed
(from [7ffe76046ad142377bf6b286f4477b09513d1e37]) ActiveResource shouldn't consider modules in the path
[#4529 ActiveResource does not serialize XML correctly state:committed]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/7ffe76046ad142377bf6b286f4477b...
