This project is archived and is in readonly mode.

#2190 ✓resolved
Aaron Patterson

Add Nokogiri as XmlMini backend

Reported by Aaron Patterson | March 10th, 2009 @ 12:34 AM

I've attached a patch that will add Nokogiri as an XmlMini back end parser. I also added some XmlMini tests.

Comments and changes to this ticket

  • Pratik

    Pratik March 10th, 2009 @ 12:40 AM

    • Assigned user set to “Jeremy Kemper”
  • Pratik

    Pratik March 10th, 2009 @ 12:41 AM

    • Title changed from “[PATCH] Add Nokogiri as XmlMini backend” to “Add Nokogiri as XmlMini backend”
  • Michael Koziarski

    Michael Koziarski March 10th, 2009 @ 01:58 AM

    • Milestone cleared.

    SHould probably get this in ahead of 2.3

  • Jeremy Kemper

    Jeremy Kemper March 10th, 2009 @ 06:59 PM

    • State changed from “new” to “open”

    seems to require nokogiri >= 1.1.1

  • Repository

    Repository March 10th, 2009 @ 07:10 PM

    • State changed from “open” to “committed”

    (from [694998ee4fb8d257ba78424cab630846327a0889]) Nokogiri backend for XmlMini

    [#2190 state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net http://github.com/rails/rails/co...

  • Bart ten Brinke

    Bart ten Brinke March 10th, 2009 @ 08:07 PM

    I'm getting failures if I actually enable nokogiri:

    1) Error: test_array_with_multiple_entries_from_xml(HashToXmlTest): TypeError: can't convert String into Integer

    ./test/../lib/active_support/xml_mini/nokogiri.rb:38:in `[]'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:38:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `call'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `to_hash'
    /Users/bart/.gem/ruby/1.8/gems/nokogiri-1.2.1/lib/nokogiri/xml/node_set.rb:139:in `each'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `call'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `to_hash'
    /Users/bart/.gem/ruby/1.8/gems/nokogiri-1.2.1/lib/nokogiri/xml/node_set.rb:139:in `each'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:55:in `call'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:55:in `to_hash'
    /Users/bart/.gem/ruby/1.8/gems/nokogiri-1.2.1/lib/nokogiri/xml/node_set.rb:139:in `each'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:55:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:20:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:13:in `parse'
    (__DELEGATION__):2:in `__send__'
    (__DELEGATION__):2:in `parse'
    ./test/../lib/active_support/core_ext/hash/conversions.rb:153:in `from_xml'
    ./test/core_ext/hash_ext_test.rb:674:in `test_array_with_multiple_entries_from_xml'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run'
    
    

    2) Error: test_file_from_xml(HashToXmlTest): NoMethodError: undefined method original_filename' for {"content_type"=>"image/png", "name"=>"logo.png", "type"=>"file"}:Hash

    ./test/core_ext/hash_ext_test.rb:689:in `test_file_from_xml'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run'
    
    

    3) Error: test_file_from_xml_with_defaults(HashToXmlTest): NoMethodError: You have a nil object when you didn't expect it! The error occurred while evaluating nil.original_filename

    ./test/core_ext/hash_ext_test.rb:701:in `test_file_from_xml_with_defaults'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run'
    
    

    4) Error: test_multiple_records_from_xml(HashToXmlTest): TypeError: can't convert String into Integer

    ./test/../lib/active_support/xml_mini/nokogiri.rb:44:in `[]'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:44:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `call'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `to_hash'
    /Users/bart/.gem/ruby/1.8/gems/nokogiri-1.2.1/lib/nokogiri/xml/node_set.rb:139:in `each'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:52:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:55:in `call'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:55:in `to_hash'
    /Users/bart/.gem/ruby/1.8/gems/nokogiri-1.2.1/lib/nokogiri/xml/node_set.rb:139:in `each'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:55:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:20:in `to_hash'
    ./test/../lib/active_support/xml_mini/nokogiri.rb:13:in `parse'
    (__DELEGATION__):2:in `__send__'
    (__DELEGATION__):2:in `parse'
    ./test/../lib/active_support/core_ext/hash/conversions.rb:153:in `from_xml'
    ./test/core_ext/hash_ext_test.rb:605:in `test_multiple_records_from_xml'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
    /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run'
    
    

    5) Failure: test_expansion_count_is_limited(QueryTest)

    [./test/core_ext/hash_ext_test.rb:888:in `test_expansion_count_is_limited'
     /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
     /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run']:
    
    

    exception expected but none was thrown.

    6) Failure: test_default_is_rexml(REXMLEngineTest)

    [./test/xml_mini/rexml_engine_test.rb:8:in `test_default_is_rexml'
     /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
     /Library/Ruby/Gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run']:
    
    

    <ActiveSupport::XmlMini_REXML> expected but was <ActiveSupport::XmlMini_Nokogiri>.

    1666 tests, 7289 assertions, 2 failures, 4 errors

  • Bart ten Brinke

    Bart ten Brinke March 10th, 2009 @ 08:11 PM

    If you run the entire rails test suite you'll get even more errors. Looks like the behaviour is different from Rexml. The LibXML implementation does not have this issue.

  • Aaron Patterson

    Aaron Patterson March 10th, 2009 @ 08:32 PM

    What version of nokogiri? How are you executing the tests? They all pass for me.

    Also, do you have tests for the LibXML version? I'm glad to match behavior if it is spec'd.

  • Bart ten Brinke

    Bart ten Brinke March 10th, 2009 @ 08:39 PM

    Well, if you enable the nokogiri gem as default:

    XmlMini.backend = 'Nokogiri'
    
    

    and then run the rails test, you'll see a lot of issues where your nokogiri implementation has a different behaviour as the default REXML and LibXML implementation. This means that enabeling nokogiri will result in a rails which will behave wrong or dangerously ( 5 Failure: test_expansion_count_is_limited(QueryTest )

  • Bart ten Brinke

    Bart ten Brinke March 10th, 2009 @ 08:42 PM

    If you use XmlMini.backend = 'LibXML' you will only get 1 failure: test_default_is_rexml, which is obvious.

  • Aaron Patterson

    Aaron Patterson March 10th, 2009 @ 08:46 PM

    I can fix these. But I must make it clear that it does not behave dangerously. Expansion count is limited, but nokogiri corrects the document and doesn't raise an exception. :-)

  • Bart ten Brinke

    Bart ten Brinke March 10th, 2009 @ 08:52 PM

    Ok, but the behavior is still different :). I didn't look into the Nokogiri implementation. I do agree with you that this test needs to be automated somehow, for future xml implementations.

    BTW: Nice work on the Nokogiri implementation :)

  • Jeremy Kemper

    Jeremy Kemper March 10th, 2009 @ 08:56 PM

    • State changed from “committed” to “open”

    Ok, I won't revert, but reopened pending expanded test coverage. Thanks guys!

  • Bart ten Brinke

    Bart ten Brinke March 10th, 2009 @ 09:09 PM

    Anyone got a nice idea how to implement this in the active support rake file?

    if gem 'libxml-ruby', '=0.9.7'
      XmlMini.backend = 'LibXML'
      run_entire_testsuite
    end
    if gem 'nokogiri'
      XmlMini.backend = 'Nokogiri'
      run_entire_testsuite
    end
    
    

    This is why my initial implementation automatically chose from the available back-ends :)

    Oh yeah, could you put

       require 'nokogiri'
    
    

    at the top of you mixin? That would make it function the same as the LibXML and REXML.

  • Aaron Patterson

    Aaron Patterson March 11th, 2009 @ 03:47 AM

    Hallo! I've attached a patch that fixes the nokogiri back end so that all rails tests pass.

  • Repository

    Repository March 11th, 2009 @ 05:53 AM

    • State changed from “open” to “resolved”

    (from [b9e021df974217b9c6ee273bd6c98b40ebde0cd3]) adding more nokogiri tests and making the main rails tests pass

    [#2190 state:resolved]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net http://github.com/rails/rails/co...

  • Stephen Bannasch

    Stephen Bannasch March 15th, 2009 @ 06:24 AM

    Aaron,

    You might want to check out this benchmark I just put together comparing all three XmlMini backends.

    http://github.com/stepheneb/rail...

    The benchmark runs Hash.from_xml on a 1.7MB XML file.

    Summary: libxml-ruby was 25x faster than REXML while nokogiri was only 1.6 times faster.

    Also: JRuby ran the REXML backend 3 times faster than MRI 1.8.6.

  • Aaron Patterson

    Aaron Patterson March 15th, 2009 @ 09:43 PM

    @Stephen

    You're right. But LibXML seems to produce different results than REXML, where Nokogiri matches the REXML output.

    I would say that because of that, this speed test is not exactly valid.

    http://github.com/tenderlove/rai...

  • Jérôme

    Jérôme April 22nd, 2009 @ 02:11 PM

    Hello

    In which file do you set XmlMini.backend = 'Nokogiri' ?

  • Aaron Patterson

    Aaron Patterson April 22nd, 2009 @ 04:19 PM

    I suppose I would do this in my environment.rb file.

  • Jérôme

    Jérôme April 22nd, 2009 @ 04:27 PM

    Nope :(

    
    /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.3.2/lib/active_support/dependencies.rb:443:in `load_missing_constant': uninitialized constant XmlMini (NameError)
    	from /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.3.2/lib/active_support/dependencies.rb:80:in `const_missing'
    	from /opt/local/lib/ruby/gems/1.8/gems/activesupport-2.3.2/lib/active_support/dependencies.rb:92:in `const_missing'
    	from /Users/jerome/Sites/foo/config/environment.rb:45
    
  • Pratik

    Pratik April 22nd, 2009 @ 04:29 PM

    Jérôme,

    You should really be asking this question the mailing list. LH is not for general support. Also, use ActiveSupport::XmlMini.

    Thanks.

  • Aaron Patterson

    Aaron Patterson April 22nd, 2009 @ 04:29 PM

    Did you try ActiveSupport::XmlMini?

  • Jérôme

    Jérôme April 22nd, 2009 @ 04:33 PM

    As apache says: it works !

    Thanks.

  • Jérôme

    Jérôme April 22nd, 2009 @ 04:34 PM

    Thank you Pratik. However it wasn't documented, not even on the announcement on the official rails blog.

    Anyway it works, end of story. Thank you very much.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Tags

Referenced by

Pages