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 :slight_smile:

john

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 :slight_smile: 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! :slight_smile:

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 :slight_smile: 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 :stuck_out_tongue:

@@ -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 :slight_smile:

john