Rails core - Jun 2009 - PATCH: Record sensitive procs for AR::Serialization.to_xml (feedback?)

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2373-record-sensitive-procs-for-to_xml

This is a patch that I contributed a couple of months ago. In short,
the point is to enable procs to access the record being serialized by
adding support for another argument in proc''s goalposts. With this
patch you can no do something like:

proc = Proc.new { |options, record| options[:builder].tag!(''name-
reverse'', record.name.reverse) }
@records.to_xml :procs => [ proc ]

Which would, of course, add a reversed string of the name to each
serialized record.

to_xml is incredibly convenient when used to serialize AR objects,
because it provides a short-hand for expressing those objects without
having to resort to builder templates, hand-drawn builder instances,
and such.  I think that this patch will make that expression much more
powerful without requiring invasive changes.

Further:
http://stackoverflow.com/questions/260668/building-dynamic-fields-using-activerecordserialization-toxml

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2373-record-sensitive-procs-for-to_xml

Did anyone have a chance to look at this patch?  It''s an noninvasive
patch
that gives procs passed into to the to_xml method via the :procs option
access to the object being serialized.  The simple cases that I''ve used
to
explain it''s usefulness may not have been compelling enough, but
visualize
this:

You need to return an array of Widget models via xml.  The xml generated by
array_of_widgets.to_xml has all the attributes you would normally need, but
you also want to add the URL to view each widget individually in the event
that consumer of this feed want to access that record again without having
to query the whole string.  Without this patch, you would have to either:
build an xml template (a chore just to get one extra attr) or break MVC by
hacking a URL generation method into the Widget model and referencing it via
to_xml''s :method option.  (There are also other, even less ideal
options.)
With this patch however, you can do this:

array_of_widgets.to_xml :procs => [ Proc.new { |options, record|
options[:builder].tag!(''show-url'', widget_url(record)]


The utility of this brief expression seems compelling to me.  Am I
overlooking some way that this is bad or redundant?

Thanks for your time :)

john


On Mon, Jun 1, 2009 at 10:23 AM, John Maxwell <johnmaxwell@gmail.com>
wrote:
>
>
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2373-record-sensitive-procs-for-to_xml
>
> This is a patch that I contributed a couple of months ago. In short,
> the point is to enable procs to access the record being serialized by
> adding support for another argument in proc''s goalposts. With this
> patch you can no do something like:
>
> proc = Proc.new { |options, record| options[:builder].tag!(''name-
> reverse'', record.name.reverse) }
> @records.to_xml :procs => [ proc ]
>
> Which would, of course, add a reversed string of the name to each
> serialized record.
>
> to_xml is incredibly convenient when used to serialize AR objects,
> because it provides a short-hand for expressing those objects without
> having to resort to builder templates, hand-drawn builder instances,
> and such.  I think that this patch will make that expression much more
> powerful without requiring invasive changes.
>
> Further:
>
http://stackoverflow.com/questions/260668/building-dynamic-fields-using-activerecordserialization-toxml
--~--~---------~--~----~------------~-------~--~----~
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 Fri, Jul 10, 2009 at 8:07 PM, John Maxwell<johnmaxwell@gmail.com>
wrote:
>
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2373-record-sensitive-procs-for-to_xml
> You need to return an array of Widget models via xml.  The xml generated by
> array_of_widgets.to_xml has all the attributes you would normally need, but
> you also want to add the URL to view each widget individually in the event
> that consumer of this feed want to access that record again without having
> to query the whole string.  Without this patch, you would have to either:
> build an xml template (a chore just to get one extra attr) or break MVC by
> hacking a URL generation method into the Widget model and referencing it
via
> to_xml''s :method option.  (There are also other, even less ideal
options.)
I have no strong opposition to your proposal, but I feel it can only
ever address a subset of the common problems faced. Let me explain my
position here;

Like many other''s i decided to override the .to_xml method because as
the vanilla xml generation is too inflexible. It''s a rails app with
many similar (data-oriented) model classes for a (quite large) xml
api. Each class will need the same kind of xml generation.

These are the xml generation issues i''ve needed to solve:

1) The ordering of xml elements from rails serialization class is
alphabetical only. The xml i need to generate requires a specific
sequence and structure.
2) Nest multi level nodes / "has_many associations - wheras the
default rails xml serializer produces flat (single level) xml.
3) Supression of not-nil attributes. The rails output produces <tag
nil="true"></tag> basically this isn''t conforming to
the api and will
cause an error.
4) The xml builder isnt particularly effecient, as it doesn''t use
libxml.

The way i solved all of these problems was to create an "acts_as_"
plugin, and override the .to_xml  method in that. I can then just
include a single "acts_as_" line in a model class which will use the
overriden method. This approach isn''t particularly inelegant, i feel.
It also allows me to use libxml, and well, libxml - you can''t really
beat it can you?

...I guess what i''m driving at here is that with this Proc patch -
just seems it wouldn''t solve all of the most common xml generation
problems.

I guess your patch is particularly relevant in the context of generating
subtly different flavours of xml ? Like pass in one procA for one purpose, and
procB for something else?

As a rails contributor myself, don''t get me wrong - I''ve no
opposition
to this patch as it may turn
out to be of good use for other. It would just also be nice to see some
broader discussion about the xml generation topic. Indeed if anything
it would be nice to see more patches on the xml code.


dreamcat4
dreamcat4@gmail.com

Thanks for your response and support, Dreamcat :)  I really appreciate your
perspective here, especially because your uses for to_xml are more demanding
than my own.

Like you, I feel like there''s more work that could be done on
ActiveRecord::Serialization.  In particular, I was surprised to find that
to_json doesn''t support the same variety of options that to_xml does:
there''s no :procs option and no optional block supported in to_json.
Extending these options to to_json would be especially pertinent to me now,
since in the last few months, I''ve (IMHO) upgraded my data interchange
format from XML to JSON.  On that note, future data interchange formats are
sure to appear, and support is sure to be sought after; and wouldn''t it
be
nice if ActiveRecord::Serialization was prepared to offer a consistent
interface (within reason) for serializing Records into NFIF (new-fangled
interchange format)?  I''m thinking of something that would pair well
with
respond_to.  (Is anyone working on this?  Is there interest in me taking a
swing at it?  If there is, I''ll try to schedule some time.)  As it is
now, I
just convert my ActiveRecord instances into a custom hash and to_json
that...  Lame! :)

But yes, this patch definitely doesn''t try to do all that.  What it
does do
is take advantage of what''s already been built out for to_xml and
enable
more lazy evaluation.  As it is now, the :procs option is essentially
rendered impotent when serializing arrays of Records, because it can''t
easily address the Record being serializing.  But this was probably just an
oversight.  When implementing the :procs option, I imagine the originator
simply didn''t consider the possibility that arrays would be serialized
--
that is, they only considered the scenario in which a single Record was
being serialized -- and a single Record would be easily captured and
addressed from a proc''s closure.

So, yes, this patch is VERY slight.  It simply increases the supported arity
of procs passed into to_xml''s :procs option from 1 to 2, the second of
which
becomes the ActiveRecord instance being serialized.  But despite the
patch''s
simplicity, this improvement makes the :procs option sooo much more
powerful... and lazier :)  So taking to_xml for granted, this seems to be a
great starting point for improvements.

Here, let me "lazy-evaluate" the patch for everybody :P

@@ -215,7 +226,7 @@ module ActiveRecord #:nodoc:
     def add_procs
       if procs = options.delete(:procs)
         [ *procs ].each do |proc|
-          proc.call(options)
+          proc.call(*(proc.arity > 1 ? [options, @record] : [options]))
         end
       end
     end


Thanks for your time :)


john


On Sat, Jul 11, 2009 at 8:08 AM, dreamcat four <dreamcat4@gmail.com>
wrote:
>
> On Fri, Jul 10, 2009 at 8:07 PM, John Maxwell<johnmaxwell@gmail.com>
> wrote:
> >
>
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2373-record-sensitive-procs-for-to_xml
> > You need to return an array of Widget models via xml.  The xml
generated
> by
> > array_of_widgets.to_xml has all the attributes you would normally
need,
> but
> > you also want to add the URL to view each widget individually in the
> event
> > that consumer of this feed want to access that record again without
> having
> > to query the whole string.  Without this patch, you would have to
either:
> > build an xml template (a chore just to get one extra attr) or break
MVC
> by
> > hacking a URL generation method into the Widget model and referencing
it
> via
> > to_xml''s :method option.  (There are also other, even less
ideal
> options.)
>
> I have no strong opposition to your proposal, but I feel it can only
> ever address a subset of the common problems faced. Let me explain my
> position here;
>
> Like many other''s i decided to override the .to_xml method because
as
> the vanilla xml generation is too inflexible. It''s a rails app
with
> many similar (data-oriented) model classes for a (quite large) xml
> api. Each class will need the same kind of xml generation.
>
> These are the xml generation issues i''ve needed to solve:
>
> 1) The ordering of xml elements from rails serialization class is
> alphabetical only. The xml i need to generate requires a specific
> sequence and structure.
> 2) Nest multi level nodes / "has_many associations - wheras the
> default rails xml serializer produces flat (single level) xml.
> 3) Supression of not-nil attributes. The rails output produces <tag
> nil="true"></tag> basically this isn''t
conforming to the api and will
> cause an error.
> 4) The xml builder isnt particularly effecient, as it doesn''t use
libxml.
>
> The way i solved all of these problems was to create an
"acts_as_"
> plugin, and override the .to_xml  method in that. I can then just
> include a single "acts_as_" line in a model class which will use
the
> overriden method. This approach isn''t particularly inelegant, i
feel.
> It also allows me to use libxml, and well, libxml - you can''t
really
> beat it can you?
>
> ...I guess what i''m driving at here is that with this Proc patch -
> just seems it wouldn''t solve all of the most common xml generation
> problems.
>
> I guess your patch is particularly relevant in the context of generating
> subtly different flavours of xml ? Like pass in one procA for one purpose,
> and
> procB for something else?
>
> As a rails contributor myself, don''t get me wrong - I''ve
no opposition
> to this patch as it may turn
> out to be of good use for other. It would just also be nice to see some
> broader discussion about the xml generation topic. Indeed if anything
> it would be nice to see more patches on the xml code.
>
>
> dreamcat4
> dreamcat4@gmail.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
-~----------~----~----~----~------~----~------~--~---