Rails core - Jan 2007 - changing the default behaviour of to_xml for has-many: associations

I recently submitted a patch to work around an issue I was having &
added a new optional behaviour to the to_xml method but on further
reflection I wonder if the default behaviour should be changed.

The gory detail, documentation and test case is all here...
http://dev.rubyonrails.org/ticket/7004

Here''s a brief example...

 class Person < ActiveRecord::Base
  has_many: children
 end

 class Male < Person; end
 class Female < Person; end

The default to_xml would look like this...

 <person>
  
<males><child>...</child><child>...</child>...</males>
 </person>

i.e. choosing the type name of the first child and pluralizing it as
the relationship name (''males'' if the first child is male). So
to find
the children you''re not sure if you need to look for
''males'' or
''females'' element which would make things like xpath a bit
messier
than it might to find, say, all the children''s names. e.g. I''d
much
rather in XPath do /person/children/*/name

It gets worse if there are 2 relationships to people. e.g.

 class Person < ActiveRecord::Base
  has_many: children
  has_many: siblings
 end

Now we''ve 2 elements inside <person> each of which could be
<males> or
<females> and one of them is the children relationship and one is the
siblings - which makes using the XML pretty hard. Also the nested
elements inside the <males> or <females> loose their type as they
are
all called <child> or <sibling>

So the patch i''ve submitted allows this behaviour to be reversed if
you supply the :polymorphic option to to_xml. If so it  would use the
relationship name for the outer collection element and for the inner
collection element use the type name of the child. This would make the
more natural XML

 <person>
 <children> <male .../> <female .../> </children>
 <siblings> <male .../> <female .../> </siblings>
 </person>

Which is nice and easy to XPath into to find what you need as well as
keeping the polymorphic type information etc. It also works a bit
nicer in XML schema / validation libraries and whatnot. So in XML to
find the male children names its /person/children/male/name or to find
all children names /person/chlldren/*/name etc.

I originally created this patch so that this new behaviour has to be
enabled via the :polymorphic option to avoid breaking things which
rely on the old behaviour. Being my first post here I didn''t wanna
break anything :). However I didn''t see any tests for the old
behaviour and I''m starting to wonder whether this new behaviour should
be the default.

I admit to being a complete newbie Railser, but I admit being quiet
surprised how this worked first time I saw it and I''d have thought
that for has-many: relationships the to_xml would use the relationship
name for the outer element (which is more likely to be unique unlike
the type name) and then the type name for the inner relationship?

I''d be totally happy if my patch was submitted so folks could just
enable this newer behaviour when they wanted it - I just wondered what
others thought on what the default to_xml behaviour should be for
has_many: with the possibility of inheritence?

-- 

James
-------
http://radio.weblogs.com/0112098/

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> I recently submitted a patch to work around an issue I was having &
> added a new optional behaviour to the to_xml method but on further
> reflection I wonder if the default behaviour should be changed.
Hmm, that is certainly annoying behaviour.  I can definitely buy the
case for an option to use the association name rather than
foo.bars.first.whatever.that.code.does.  I''m not necessarily sure that
''polymorphic'' is the right name for it, as that''s
used elsewhere in AR
with a slightly different meaning.

I''m hesitant to break backwards compatibility, but there''s
definitely
a case here for doing something.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Jan 17, 5:32 pm, "Michael Koziarski" <mich...@koziarski.com>
wrote:
> Hmm, that is certainly annoying behaviour.  I can definitely buy the
> case for an option to use the association name rather than
> foo.bars.first.whatever.that.code.does.  I''m not necessarily sure
that
> ''polymorphic'' is the right name for it, as
that''s used elsewhere in AR
> with a slightly different meaning.
I agree. I can certainly understand how the current behavior was
arrived at, but there''s room for improvement.

I don''t think ''polymorphic'' is the right thing to use
here, either. My
feeling is that the association name should be used as the containing
element name by default. If you want to use a different name in the
xml, perhaps a syntax like

  foo.to_xml( :include => [ { :spams => :hams }, :bars ] )

would do the trick?
> I''m hesitant to break backwards compatibility, but
there''s definitely
> a case here for doing something.
Personally, I would not hesitate to break backwards compatibility if
the current behavior is determined to be wrong. There''s already a lot
of house cleaning going on in the march towards 2.0 -- don''t see much
reason why this situation should be any different than any of the
current deprecations (assuming a change is agreed upon).

-- 
Regards,

John Wilger


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

John Wilger wrote:
> 
> 
> On Jan 17, 5:32 pm, "Michael Koziarski"
<mich...@koziarski.com> wrote:
>> Hmm, that is certainly annoying behaviour.  I can definitely buy the
>> case for an option to use the association name rather than
>> foo.bars.first.whatever.that.code.does.  I''m not necessarily
sure that
>> ''polymorphic'' is the right name for it, as
that''s used elsewhere in AR
>> with a slightly different meaning.
> 
> I agree. I can certainly understand how the current behavior was
> arrived at, but there''s room for improvement.
> 
> I don''t think ''polymorphic'' is the right thing
to use here, either. My
> feeling is that the association name should be used as the containing
> element name by default. If you want to use a different name in the
> xml, perhaps a syntax like
> 
>   foo.to_xml( :include => [ { :spams => :hams }, :bars ] )
> 
> would do the trick?
> 
>> I''m hesitant to break backwards compatibility, but
there''s definitely
>> a case here for doing something.
> 
> Personally, I would not hesitate to break backwards compatibility if
> the current behavior is determined to be wrong. There''s already a
lot
> of house cleaning going on in the march towards 2.0 -- don''t see
much
> reason why this situation should be any different than any of the
> current deprecations (assuming a change is agreed upon).
> 
Given that has_many: associations are not even marshalled to XML unless you
specifically ask for them - I wonder how many folks are really using the
current behaviour. So I''d personally prefer the default behaviour
change -
rails should do the right thing by default - and we keep the old behaviour
as an option folks can enable if they really want.

Incidentally I really hate the option name I picked of :polymorphic but
every other option I considered seemed too long. Maybe we need options like
:use_association_name_for_many or :use_first_type_name_for_many (depending
on which is the default and which has to be enabled)?

James

-- 
View this message in context:
http://www.nabble.com/changing-the-default-behaviour-of-to_xml-for-has-many%3A-associations-tf3027005.html#a8428128
Sent from the RubyOnRails Core mailing list archive at Nabble.com.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

> Given that has_many: associations are not even marshalled to XML unless you
> specifically ask for them - I wonder how many folks are really using the
> current behaviour. So I''d personally prefer the default behaviour
change -
> rails should do the right thing by default - and we keep the old behaviour
> as an option folks can enable if they really want.
Ok, I''m sold.  If we change the default behaviour to use the
association name, and provide an option to retain the old behaviour
then people who rely on the current stuff (think @section.to_xml
:include=>:published_posts) can retain it.
> Incidentally I really hate the option name I picked of :polymorphic but
> every other option I considered seemed too long. Maybe we need options like
> :use_association_name_for_many or :use_first_type_name_for_many (depending
> on which is the default and which has to be enabled)?
I''ll have a think about a decent name for this ''use first type
name
for collection'' option, but if anyone else wants to chime in with the
perfect solution, now''s the time :).


-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---