killbill / killbill-plugin-framework-ruby

Framework to write Kill Bill plugins in Ruby
http://killbill.io
8 stars 11 forks source link

unable to start plugins (bundle) due nokogiri #20

Closed kares closed 9 years ago

kares commented 9 years ago

(LoadError) load error: nokogiri/nokogiri -- java.lang.NoClassDefFoundError: org/xml/sax/SAXParseException

... as it seems to be accessing classes outside what it is allowed to ?!?

pretty much all the plugins that use XML (require nokogiri) can not load ;(

was unable to setup litle plugin due this, tried going down to 1.6.1 as a work-around but seems it's not the same cause.

this only reproduces with KB ... details at: https://gist.github.com/kares/29fb09165e6014792da2

p.s. might have slipped in due the JRuby 1.7.16 upgrade (not sure really) - would try newer JRuby (there sure might have been some OSGi class-loading updates) if I get some hints how to build an osgi jruby.jar ...

kares commented 9 years ago

never-mind my ps, I'm able to reproduce locally as well using the jruby.jar based on 1.7.12

pierre commented 9 years ago

That's odd, XML should definitively be supported.

It is probably an import-export problem. We tell KB to export some default classes to OSGI plugins, see the property org.killbill.osgi.system.bundle.export.packages: https://github.com/killbill/killbill-platform/blob/master/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java#L49 (it looks like org.xml.sax is correctly exported).

On the jruby.jar side of things, the list of imports is here: https://github.com/killbill/killbill-platform/blob/master/osgi-bundles/bundles/jruby/pom.xml#L152 (you can also check the MANIFEST.MF in the jruby.jar). I don't see org.xml.sax being listed, maybe that's the culprit? Although the plugin is currently running in production by some users, so I'm not sure what changed (sun vs openjdk maybe?).

FYI, to build the jruby.jar, build the following submodule: https://github.com/killbill/killbill-platform/tree/master/osgi-bundles/bundles/jruby

kares commented 9 years ago

thanks, found the bundle and it seems to help ... I needed to add these for require 'nokogiri' :

                            <!-- nokogiri -->
                            javax.xml;
                            javax.xml.namespace;
                            javax.xml.parsers;
                            javax.xml.validation;
                            javax.xml.stream;
                            javax.xml.stream.events;
                            javax.xml.stream.util;
                            javax.xml.transform;
                            javax.xml.transform.dom;
                            javax.xml.transform.sax;
                            javax.xml.transform.stax;
                            javax.xml.transform.stream;
                            javax.xml.xpath;
                            org.w3c.dom;
                            org.w3c.dom.events;
                            org.w3c.dom.ls;
                            org.xml.sax;
                            org.xml.sax.ext;
                            org.xml.sax.helpers;
                            <!-- recent nokogiri also uses these :
                            com.sun.org.apache.xml.internal;
                            com.sun.org.apache.xpath.internal; -->

crazy part is it's also using com.sun.org.apache OpenJDK internals but Felix does not like exporting those (not sure why - it all in the same JDK rt.jar) ... https://github.com/sparklemotion/nokogiri/blob/dc8dde99ea405df46eafc655c137970f0a6e3295/ext/java/nokogiri/XmlXpathContext.java ... whenever that class gets loaded it will look for some of the "internal" classes as they are not just behind curtains.

but at least I'm fine running the older nokogiri 1.6.1 (mentioned on the issue linked above)

... let me know if you want me to fire up a PR with the change (not sure how to test it)

Although the plugin is currently running in production by some users, so I'm not sure what changed (sun vs openjdk maybe?)

hard to tell ... have not been around long enough, if I would to suspect than maybe Felix got upgraded ...

pierre commented 9 years ago

... let me know if you want me to fire up a PR with the change (not sure how to test it)

That'd be appreciated. For testing, it's fine - manual testing is all we have for that for now...

hard to tell ... have not been around long enough, if I would to suspect than maybe Felix got upgraded ...

Good point! We did upgrade it recently (https://github.com/killbill/killbill-oss-parent/commit/5e74dda19647f6e35eb4ad2d370b3de3b9cccee2), but I suspect they are on an older version.

kares commented 9 years ago

OK, will do but for now it seems "useless" as it's not enough ;( ... loads but does not work :

https://gist.github.com/kares/29fb09165e6014792da2#file-little-nokogiri-add_payment_method-log

... unfortunately it seems the real issue gets swallowed and likely null is returned

pierre commented 9 years ago

The following did the trick for me:

--- a/osgi-bundles/bundles/jruby/pom.xml
+++ b/osgi-bundles/bundles/jruby/pom.xml
@@ -195,7 +195,34 @@
                             javax.security.auth;
                             javax.security.auth.*;
                             javax.security.auth.x500;
-                            javax.security.auth.x500.*;resolution:=optional
+                            javax.security.auth.x500.*;
+                            <!-- nokogiri -->
+                            javax.xml;
+                            javax.xml.namespace;
+                            javax.xml.parsers;
+                            javax.xml.validation;
+                            javax.xml.stream;
+                            javax.xml.stream.events;
+                            javax.xml.stream.util;
+                            javax.xml.transform;
+                            javax.xml.transform.dom;
+                            javax.xml.transform.sax;
+                            javax.xml.transform.stax;
+                            javax.xml.transform.stream;
+                            javax.xml.xpath;
+                            org.w3c.dom;
+                            org.w3c.dom.events;
+                            org.w3c.dom.ls;
+                            org.xml.sax;
+                            org.xml.sax.ext;
+                            org.xml.sax.helpers;
+                            com.sun.org;
+                            com.sun.org.apache;
+                            com.sun.org.apache.xml;
+                            com.sun.org.apache.xml.internal;
+                            com.sun.org.apache.xml.internal.utils;
+                            com.sun.org.apache.xpath;
+                            com.sun.org.apache.xpath.internal;
+                            com.sun.org.apache.xpath.internal.jaxp;resolution:=optional,
                         </Import-Package>
                     </instructions>
                 </configuration>

and

--- a/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java
+++ b/osgi/src/main/java/org/killbill/billing/osgi/config/OSGIConfig.java
@@ -165,8 +165,34 @@ public interface OSGIConfig extends KillbillPlatformConfig {
              "com.sun.xml.internal.ws.wsdl.writer.document.xsd," +

              // sax parser
+             "javax.xml," +
+             "javax.xml.namespace," +
+             "javax.xml.parsers," +
+             "javax.xml.validation," +
+             "javax.xml.stream," +
+             "javax.xml.stream.events," +
+             "javax.xml.stream.util," +
+             "javax.xml.transform," +
+             "javax.xml.transform.dom," +
+             "javax.xml.transform.sax," +
+             "javax.xml.transform.stax," +
+             "javax.xml.transform.stream," +
+             "javax.xml.xpath," +
              "javax.annotation," +
              "javax.jws.soap," +
+             "com.sun.org," +
+             "com.sun.org.apache," +
+             "com.sun.org.apache.xml," +
+             "com.sun.org.apache.xml.internal," +
+             "com.sun.org.apache.xml.internal.utils," +
+             "com.sun.org.apache.xpath," +
+             "com.sun.org.apache.xpath.internal," +
+             "com.sun.org.apache.xpath.internal.jaxp," +
+             "org.w3c.dom," +
+             "org.w3c.dom.events," +
+             "org.w3c.dom.ls," +
+             "org.xml.sax," +
+             "org.xml.sax.ext," +
+             "org.xml.sax.helpers," +
              "org.xml.sax.ext;org.xml.sax.helpers;org.xml.sax," +

              // javax.servlet and javax.servlet.http are not exported by default - we

(import and export packages need to be kept in sync).

It's really unfortunate all of these core classes are not just visible by default...

kares commented 9 years ago

yeah the core ones (present in Java API) are a bit unfortunate, but still nokogiri's use of internal ones is also very unfortunate (one basically can only possibly load nokogiri on Open/Oracle JDK currently) ...

I still needed to export org.w3c.tranversal (and thus I rather included the full Common DOM API) you can see my changes at: https://github.com/kares/killbill-platform/compare/nokogiri ... thanks for the help.

pierre commented 9 years ago

Fixed in Kill Bill 0.14.0.