Issue Details (XML | Word | Printable)

Key: NUCSPATIAL-49
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Baris ERGUN
Reporter: Jasper Siepkes
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
DataNucleus Types : Geospatial

OSGi manifest is missing some package imports

Created: 25/Mar/14 11:35 AM   Updated: 13/Apr/14 10:52 AM   Resolved: 12/Apr/14 05:33 PM
Component/s: None
Affects Version/s: 3.2.7
Fix Version/s: 4.0.0.m3

Environment: Java 7 update 45 / Apache Felix 4.2.1


 Description  « Hide
The OSGi manifest of the DataNucleus GeoSpatial plugin is missing some package imports. The easiest way to fix this is to add the following to the DN GeoSpatial POM in the <Import-Package> section:

org.datanucleus.store.types.converters;version="${dn.core.version}",
org.datanucleus.store.rdbms.mapping;version="${dn.rdbms.version}",
org.datanucleus.store.rdbms.adapter;version="${dn.rdbms.version}",

After that the bundle loads. That exposes some other problems (RDBMS plugin can't find the PostGISAdapter) but I'll open a seperate issue for that once I understand what's going on.

Jasper Siepkes added a comment - 25/Mar/14 01:37 PM
I just realized the issue is a bit more complex then my above proposed solution. Hang on while I debug some more.

Jasper Siepkes added a comment - 25/Mar/14 03:05 PM
The problem is that multiple DataNucleus packages export the same namespace. For example 'org.datanucleus.store.rdbms.sql.expression' is exported by DataNucleus RDBMS but also by DataNucleus Geospatial. See: http://wiki.osgi.org/wiki/Split_Packages for more information. Basically split packages are discouraged (see: http://wiki.osgi.org/wiki/Avoid_Split_Packages). This wasn't a problem in previous versions because 'Require-Bundle' was used instead of 'Import-Package'. The obvious solution is to change the namespace, is that an option? For example 'org.datanucleus.geospatial'?

There are other ways around this. For example make the GeoSpatial plugin a fragment by adding "<Fragment-Host>org.datanucleus.store.rdbms</Fragment-Host>" to the bnd options in the pom. But using fragments is not really a nice solution. Fragments don't have a lifecycle, can't have an activator and also are poorly extendable.

Any suggestions Andy?

Baris ERGUN added a comment - 02/Apr/14 02:44 PM
Hi Jasper;

After reading your comments and reading best practice advises and also considering that we are doing major changes in 4.0 I can execute this task. Only that I will try to find the easiest and smartest way of doing this.

Baris ERGUN made changes - 02/Apr/14 02:44 PM
Field Original Value New Value
Assignee Baris ERGUN [ bergun ]
Jasper Siepkes added a comment - 04/Apr/14 10:50 AM
Hi Baris,

Well what I did (because I needed a working solution) is to move everything to the 'org.datanucleus.geospatial' package space. As far as I can tell that is the cleanest and easiest solution. I think this is the best solution (IMHO). I doubt it will cause API breakage for people; I don't think there are people using classes directly from the Geo plugin?

Andy Jefferson added a comment - 04/Apr/14 08:09 PM
I'd have no particular problem with repackaging, but would suggest that it should be
org.datanucleus.store.types.geospatial

since this plugin is adding java type support and nothing else.

We would then have the following (or something similar) under that
query.inmemory.*
converters.*
rdbms.mapping.*
rdbms.sql.expression.*
rdbms.sql.method.*


And then if this is done like that (or we agree on some other naming), raise issues on Jodatime plugin, Guava plugin, java8 plugin to do the same following the same naming

Andy Jefferson added a comment - 06/Apr/14 11:16 AM
Ok, I've now refactored Jodatime, Guava and Java8 plugins to use
org.datanucleus.store.types.{plugin-name} as the root package.

Under that we now have
converters/*
rdbms/*
wrapper/*
query/inmemory/*

Suggest that you follow that for the Geospatial plugin (and make sure all plugin.xml entries are updated accordingly)

Baris ERGUN made changes - 06/Apr/14 02:03 PM
Status Open [ 1 ] In Progress [ 3 ]
Baris ERGUN added a comment - 06/Apr/14 05:32 PM
Andy before any changes applied I have realised that on Mysql only 1 jdo geospatial test is failing. Please verify.

testContains[default](org.datanucleus.tests.JtsGeometrySpatialTest) Time elapsed: 0.079 sec <<< ERROR!
javax.jdo.JDOException: Class "ARRAY" was not found in the CLASSPATH. Please check your specification and your CLASSPATH.
at org.datanucleus.ClassLoaderResolverImpl.classForName(ClassLoaderResolverImpl.java:217)
at org.datanucleus.ClassLoaderResolverImpl.classForName(ClassLoaderResolverImpl.java:381)
at org.datanucleus.store.rdbms.sql.expression.SQLExpressionFactory.invokeMethod(SQLExpressionFactory.java:382)
at org.datanucleus.store.rdbms.sql.expression.GeometryExpression.invoke(GeometryExpression.java:194)
at org.datanucleus.store.rdbms.query.QueryToSQLMapper.processInvokeExpression(QueryToSQLMapper.java:2958)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compilePrimaryExpression(AbstractExpressionEvaluator.java:201)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileUnaryExpression(AbstractExpressionEvaluator.java:170)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileAdditiveMultiplicativeExpression(AbstractExpressionEvaluator.java:149)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileRelationalExpression(AbstractExpressionEvaluator.java:124)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.compileOrAndExpression(AbstractExpressionEvaluator.java:66)
at org.datanucleus.query.evaluator.AbstractExpressionEvaluator.evaluate(AbstractExpressionEvaluator.java:46)
at org.datanucleus.query.expression.Expression.evaluate(Expression.java:337)
at org.datanucleus.query.expression.DyadicExpression.evaluate(DyadicExpression.java:70)
at org.datanucleus.store.rdbms.query.QueryToSQLMapper.compileFilter(QueryToSQLMapper.java:465)
at org.datanucleus.store.rdbms.query.QueryToSQLMapper.compile(QueryToSQLMapper.java:385)
at org.datanucleus.store.rdbms.query.JDOQLQuery.compileQueryFull(JDOQLQuery.java:936)
at org.datanucleus.store.rdbms.query.JDOQLQuery.compileInternal(JDOQLQuery.java:342)
at org.datanucleus.store.query.Query.executeQuery(Query.java:1772)
at org.datanucleus.store.query.Query.executeWithArray(Query.java:1700)
at org.datanucleus.api.jdo.JDOQuery.execute(JDOQuery.java:243)
at org.datanucleus.tests.JtsGeometrySpatialTest.testContains(JtsGeometrySpatialTest.java:801)

Baris ERGUN added a comment - 06/Apr/14 05:58 PM
Well I could not hold back the beast inside me and moved forward and pushed my changes. Anyway if you checkout test sources to e7d015bbb3acfe9d0becdb0e5c1278b63ad25df5' and geospatial sources to ' 477b22f802dfffa80f086a13021178b1a3ef799f' hashes and test Mysql 1 test fails with the above error. Postgresql fails with a known issue which I moved under knownbugs now. Oracle is fine just before and after the commit. When you verify the Mysql failure at your end I would like to open a new issue and investigate it.

Andy Jefferson added a comment - 06/Apr/14 08:15 PM
Is see no need to ask for verification, that's what publically available Jenkins exists for. Also see NUCRDBMS-786 which fixes some problems that were brought up by some changes Renato made to the test suite (which changed the order in which classes would be registered in the plugin mechanism.

I updated your refactored package names to make them match my comments above; all plugins have to be consistent.

Suggest this issue is closed when Jasper checks nightly builds

Baris ERGUN added a comment - 06/Apr/14 09:16 PM - edited
well Renato did a great job actually I was not aware of that the Jenkins integration was complete. So never thought of checking there. And yes that is verified by it already.

According the changes u made

"simple" => "wrapper"
and
"query.evaluator" => "query.inmemory"

When I read ur comments again they are not that clear to me still. Before trying to understand what needs to be changed I have made git diff to couple of ur changes on some of the projects u mentioned but did not also notice those renaming changes....


Baris ERGUN made changes - 06/Apr/14 09:26 PM
Status In Progress [ 3 ] Open [ 1 ]
Jasper Siepkes added a comment - 07/Apr/14 11:09 AM
Ok, great ill test them. I'll use the nightlies from the DN maven Nightly repo for testing and report back.

Jasper Siepkes added a comment - 09/Apr/14 09:00 AM
Looks ok here! Thanks guys!

Andy Jefferson made changes - 12/Apr/14 05:33 PM
Status Open [ 1 ] Resolved [ 5 ]
Fix Version/s 4.0.0.m3 [ 12160 ]
Resolution Fixed [ 1 ]
Andy Jefferson made changes - 13/Apr/14 10:52 AM
Status Resolved [ 5 ] Closed [ 6 ]