representable icon indicating copy to clipboard operation
representable copied to clipboard

Unable to parse XML collection

Open mhuggins opened this issue 11 years ago • 12 comments

Hi there,

I'm trying to use your gem to parse from XML from an existing API. I've created a gist demonstrating what I'm trying to do here.

I'm able to convert objects to XML properly, e.g.:

stores = [
  SupermarketAPI::Models::Store.new(id: 1, name: 'Store 1'),
  SupermarketAPI::Models::Store.new(id: 2, name: 'Store 2'),
]
puts SupermarketAPI::Serializers::Stores.new(stores).to_xml

<ArrayOfStore>
  <Store>
    <StoreId>1</StoreId>
    <Storename>Store 1</Storename>
  </Store>
  <Store>
    <StoreId>2</StoreId>
    <Storename>Store 2</Storename>
  </Store>
</ArrayOfStore>

However, in the sample code I provided, I'm not having success when using the from_xml method against the stores.xml file in the linked gist. I'm seeing that the correct number of objects are parsed out under the root XML (6 stores). However, all of the store data is nil, and I'm not sure why. Am I missing a :wrap or :as option, for example? Am I using representation_wrap incorrectly? Anything else I haven't considered?

Thanks!

mhuggins avatar Oct 25 '14 05:10 mhuggins

Hey Matt, have you tried using the new Representable::Debug to see what's happening? https://github.com/apotonick/representable#debugging

I'm not too sure about XML::Collection maybe we could add a test to assure it does what you want? https://github.com/apotonick/representable/blob/master/test/xml_test.rb#L392

Best avatar ever..! :beers:

apotonick avatar Oct 27 '14 02:10 apotonick

Ahh, I missed that the debug module was added, I'll give that a shot and report my findings. Thanks! :)

mhuggins avatar Oct 27 '14 03:10 mhuggins

I started working on it recently and it probably needs to override more methods, so feel free to PR/comment here what you need to add in the Debug module in order to understand XML parsing.

We can also introduce Debug::XML that logs more specifically for XML if that helps you!

apotonick avatar Oct 27 '14 03:10 apotonick

Interesting find...it looks like this is working as expected:

xml = %Q{
  <?xml version="1.0" encoding="utf-8"?>
  <ArrayOfStore>
    <Store>
      <StoreId>1</StoreId>
      <Storename>Foo</Storename>
    </Store>
  </ArrayOfStore>
}
[].extend(SupermarketApi::StoresSerializer).from_xml(xml)
# => [#<SupermarketApi::Store id="1" name="Foo">] 

while this is not working as expected (note the namespaces in the root node):

xml = %Q{
  <?xml version="1.0" encoding="utf-8"?>
  <ArrayOfStore xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns="http://www.SupermarketAPI.com">
    <Store>
      <StoreId>1</StoreId>
      <Storename>Foo</Storename>
    </Store>
  </ArrayOfStore>
}
[].extend(SupermarketApi::StoresSerializer).from_xml(xml)
# => [#<SupermarketApi::Store id=nil name=nil>] 

I did see that you offer a remove_namespaces! method, but that is only available for Representable::XML modules, not Representable::XML::Collection, which makes me unable to use it. Any thoughts or existing solutions I might have missed?

mhuggins avatar Oct 27 '14 05:10 mhuggins

That really looks like a namespace problem - I didn't even think of that. Check out the #remove_namespace! method, can you copy that into the Collection to see if that would help you?

apotonick avatar Oct 27 '14 09:10 apotonick

Yep, that did the trick. My working XML collection representer ended up looking like this:

module SupermarketApi
  module StoresSerializer
    include Representable::XML::Collection

    self.representation_wrap = :ArrayOfStore

    def self.remove_namespaces!
      representable_attrs.options[:remove_namespaces] = true
    end

    remove_namespaces!

    items class: Store, extend: StoreSerializer
  end
end

mhuggins avatar Oct 27 '14 13:10 mhuggins

One more thing worth noting that I discovered earlier in fixing this is that my representer collection is able to be a class inheriting from Representable::Decorator, but the representer used for the individual items must be a module instead of a Representable::Decorator class. This is because while parsing the elements, it tries to call the extend method against the module, which raises a TypeError. I've definitely done this with JSON in the past using representable, so it seems to be a missing feature on the XML side.

As such, my code in a nutshell looks like this:

class CollectionRepresenter < Representable::Decorator
  include Representable::XML::Collection
  items class: Item, decorate: ItemRepresenter
  # ... snip ...
end

module ItemRepresenter
  include Representable::XML
  # ... snip ...
end

mhuggins avatar Oct 27 '14 13:10 mhuggins

Oh, ok, thanks Matt. That module/class things sounds weird as this is completely generic code in Representable. Could you give me a breaking test? Just add a new test file, I'll merge and put it in the right place. Thanks!

apotonick avatar Oct 27 '14 19:10 apotonick

Huh, I updated the tests to use the decorator class for the model itself, and it seems to work. My code keeps giving me a TypeError that a module is expected instead of a class though, for some reason. I'll keep playing around, and I'll try running against master instead of the latest release too (though I don't see any changes that should impact this).

mhuggins avatar Oct 28 '14 01:10 mhuggins

Matt- this sounds like you're not requiring all files necessary to run this properly. I'll check out your test, thanks so much.

apotonick avatar Oct 28 '14 02:10 apotonick

Thanks Nick. I thought I was requiring the right files, but perhaps not. Perhaps I can create a new branch that includes the decorator approach and point you to the files that are causing me trouble, if that's not too much to ask. :)

mhuggins avatar Oct 28 '14 02:10 mhuggins

Ideally you set up a small app that only does this one thing. We can then pull that into a test once we found out how to provoke it.

Also, make sure to https://twitter.com/apotonick/status/526842565891338241

apotonick avatar Oct 28 '14 02:10 apotonick