Empact / roxml

We should probably simply be more open to redefinition.

More comments from gorism:

1. Disabling the offending code (in xml_attr()) didn't work. I didn't think it would, but what the heck. That resulted in duplicate data getting returned, because each time the class re-loads it keeps adding the same element definitions to the @roxml_attr array.

2. So, next I tried running mongrel in production mode, where classes don't get reloaded between requests. Magically fixed!

3. So the issue lies in the fact that the @roxml_attr array is hanging around between class loads. The issue is that @roxml_attr is a class instance variable. It's getting manipulated at class load time of my class(es) that have mixed in ROXML. Each time the classes get loaded (which is every request in development mode), that class instance variable is being referred to without having been re-initialized.

4. So it seems like there are a couple of possibilities here. The first is to re-initialize this value on class load. Seems the most obvious and straightforward to me. After all, if the class is reloading, you probably want the definition of elements/attributes to be reset. The second is to freeze the definition after initial load. That one doesn't feel right, because you would end up with weird behavior in development mode, where developers expert their changes to be immediately visible.

5. For now I'm pursuing the "initialize on class load" thing. I can solve it simply (but in a brute-force way) by simply setting "@roxml_attr = []" at the class level of my Ruby classes that have included ROXML. It's obviously more desirable to do this as part of ROXML. Perhaps an explicit method call that is done prior to any "xml_accessor" or "xml_reader" calls? That's sort of fragile, and I'd prefer for it to be something done behind the scenes. But I experimented for a while and couldn't get such a method to execute in the context of my class, rather than the ROXML module. Initializing @roxml_attrs in the context of the ROXML module does no good, because of scoping.
   

More comments from gorism:

OK, different idea.

How about ignoring the duplicate element/attribute declarations? You can add duplicate accessors all you want to a Ruby class, and the interpreter will use the last one defined.

So, change xml_attr to:

 def xml_attr(sym, type_and_or_opts = nil, opts = nil, &block)
   returning Definition.new(sym, *[type_and_or_opts, opts].compact, &block) do |attr|
     if !roxml_attrs.map(&:accessor).include? attr.accessor
       @roxml_attrs << attr
     end
   end
 end

Or something like that.

I don't believe I've tested this case. If you'd write a failing test case and push it out, I'd find it very helfull, but in the mean time, have you taken a look at the :from option? When you explicitly specify what node you'd like to query from, roxml should always honor that. In short: xml_reader :books, :as => SomeIdAndNameObject, :from => 'books', :in => 'some_node'

I can't give any guarantees, but that's a likely fix.

Hi, thanks for your answer. I've tried what you suggest and it doesn't work.

Here is the exact code I ran :

require 'rubygems'
require 'roxml'
require 'pp'

xml = %(
<myxml>
  <store>
    <books><id>1</id><title>first book</title></books>
    <books><id>2</id><title>second book</title></books>
    <books><id>3</id><title>third book</title></books>
  </store>
</myxml>
)

class Book
  include ROXML
  xml_reader :id, :as => Integer
  xml_reader :title
end

class Store
  include ROXML
  xml_reader :books, :from => 'books', :in => 'store', :as => [Book]
end

class MyXml
  include ROXML
  xml_reader :store, :as => Store
end


pp MyXml.from_xml(xml).store.books

About the failing test, I'm still in the learning process of writing tests and I'm not familiar with the way you organized your specs. I you tell me which spec file I should write my test in, I'd try to make one and send you a pull request.

Cheers