Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OClassLoaderHelper uses javax.imageio.spi.ServiceRegistry #3402

Closed
jdillon opened this issue Jan 15, 2015 · 10 comments
Closed

OClassLoaderHelper uses javax.imageio.spi.ServiceRegistry #3402

jdillon opened this issue Jan 15, 2015 · 10 comments
Assignees
Milestone

Comments

@jdillon
Copy link
Contributor

jdillon commented Jan 15, 2015

Seems odd that its depending on javax.imageio libraries here.

Suggest you guys change to use java.util.ServiceLocator instead, which IIUC is essentially the same thing you need here.

@tglman
Copy link
Member

tglman commented Jan 16, 2015

yes, i also wondered a few times why was imageio there, i'll change this with (java.util.ServiceLoader)[https://docs.oracle.com/javase/6/docs/api/java/util/ServiceLoader.html] after we released 2.0 final.

the reason for not do it now is that we don't want to mess up 2 days before release, and this may impact some osgi environment.

@tglman tglman added this to the 2.1 milestone Jan 16, 2015
@jdillon
Copy link
Contributor Author

jdillon commented Jan 16, 2015

FYI, using imageio impl may actually prevent proper usage in osgi:

http://stackoverflow.com/questions/12664287/does-javax-imageio-spi-serviceregistry-work-in-an-osgi-container

FTR we are also having some issues already in Apache Karaf with OrientDB failing to find function factories, unsure if its related, but gut suggests it may be.

@lvca
Copy link
Member

lvca commented Jan 16, 2015

@tglman WDYT?

@jdillon
Copy link
Contributor Author

jdillon commented Jan 16, 2015

FTR I can't say if changing to the java.util.ServiceLoader will actually resolve this yet. I may run some local tests, as it may still be problematic due to how the classloader is selected by consumers of OClassLoaderHelper. Will let you guys know what we find.

@cwilper
Copy link
Contributor

cwilper commented Jan 16, 2015

Update: I just tested switching to java.util.ServiceLoader and by itself, it doesn't change the behavior for the issue we're seeing (karaf still can't find OGraphFunctionFactory, so graph functions show up as missing when you try to use them in select statements).

@jdillon
Copy link
Contributor Author

jdillon commented Jan 16, 2015

thanks @cwilper, we'll have to dig a bit more to see if there is an issue with how we have the CL setup or if its a more fundamental issue of how orient is loading these based on the loading with specific CL orientClassLoader = OSQLEngine.class.getClassLoader();

@lvca
Copy link
Member

lvca commented Jan 16, 2015

Thanks guys, we really appreciate your work on this.

@mcculls
Copy link
Contributor

mcculls commented Jan 17, 2015

FYI, a quick solution to this is to make the graphdb bundle a fragment of the core bundle:

diff --git a/graphdb/pom.xml b/graphdb/pom.xml
index 8f4d342..52b76c6 100644
--- a/graphdb/pom.xml
+++ b/graphdb/pom.xml
@@ -40,6 +40,9 @@
         <javac.src.version>1.6</javac.src.version>
         <javac.target.version>1.6</javac.target.version>
         <jar.manifest.mainclass>com.orientechnologies.orient.server.OServerMain</jar.manifest.mainclass>
+        <osgi.fragment.host>
+            com.orientechnologies.orientdb-core
+        </osgi.fragment.host>
         <osgi.import>
             com.tinkerpop.blueprints;resolution:=optional,
             com.tinkerpop.gremlin.groovy.jsr223;resolution:=optional,

This will merge their contents at runtime in OSGi so that they effectively share the same classpath and should therefore make the graph functions visible.

@lvca
Copy link
Member

lvca commented Jan 17, 2015

I'm not an OSGi expert. Everybody agrees on this change?

@tglman
Copy link
Member

tglman commented Feb 27, 2015

fixed closing

@tglman tglman closed this as completed Feb 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants