Update to newer Version of GeoTools to support Java 11 about udig-platform

[nprigour]

Since the proposed update seems quite a huge task I would suggest to resolve as many pending issues/PRs as possible before going with this.

[egouge]

What about removing this dependency entirely and updating to the "new-ish" java.time api?

[egouge]

Sure - I started poking around at this today in my own branch from master. The upgrade to gt22 is a bit more work than the upgrade I did to gt21 as some of the deprecated stuff has now been removed entirely, but I didn't see anything major yet.

[jodygarnett]

Happy to answer questions and support where I can :) -- Jody Garnett

…

On Tue, 24 Sep 2019 at 16:32, Emily ***@***.***> wrote: Sure - I started poking around at this today in my own branch from master. The upgrade to gt22 is a bit more work than the upgrade I did to gt21 as some of the deprecated stuff has now been removed entirely, but I didn't see anything major yet. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#354?email_source=notifications&email_token=AAEZXML2EZEPIQK5QQPM2C3QLKPSFA5CNFSM4ITAETD2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7QDMRA#issuecomment-534787652>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEZXMPZFYVO4TDG362GEF3QLKPSFANCNFSM4ITAETDQ> .

[fgdrf]

see GeoTools upgrade guide how to handle Units package change to si.uom and for Geometry library to org.locationtech.jts (from com.vividsolutions.jts)

[fgdrf]

Since the proposed update seems quite a huge task I would suggest to resolve as many pending issues/PRs as possible before going with this.

Fair point! We started to work on this during an Eclipse Hackathon session early this month.

However, maybe we can collaborate on this and test whole applications. Right now its not in a state to submit somthing we can work on but stay tuned ;)

[fgdrf]

org.joda.time dependency is not used by GeoTools, its a dependency from uDig define in pom-libs.xml:

		<dependency>
			<groupId>joda-time</groupId>
			<artifactId>joda-time</artifactId>
			<version>1.6</version>
		</dependency>

as stated on mailinglist few years ago it makes sense to seperated this dependeny and re-bundle it as Eclipse smarthome project did.

Following modules importing org.joda.time classes:

• org.locationtech.udig.catalog.kml
• org.locationtech.udig.jconsole
• org.locationtech.udig.omsbox
• org.locationtech.udig.project (in ViewportModel and RenderPackage API)
• org.locationtech.udig.tools.jgrass

[fgdrf]

Following modules importing org.joda.time classes:

• org.locationtech.udig.catalog.kml
• org.locationtech.udig.jconsole
• org.locationtech.udig.omsbox
• org.locationtech.udig.project (in ViewportModel and RenderPackage API)
• org.locationtech.udig.tools.jgrass

Since all listed modules are (indirectly) dependent from org.locationtech.udig.project it might make sense to bundle joda-time within that module and export packages from there.

Opinions?

[fgdrf]

What about removing this dependency entirely and updating to the "new-ish" java.time api?

Good point, this would required to re-generate classes from Model I guess. Can you help here?

Maybe it makes sense to make it in two steps:

1. seperate joda-time dependency into a new bundle (rather then packging it into uidg.project bundle)

2. move on to new java.time api and re-generate classes from EMF model

[egouge]

I've never regenerated the model, so I don't know if I could help, but I could try!

FYI - I am using a modified branch of uDig that I upgraded to geotools 21.1 and removed the joda time package. I only use a very small number of the udig plugins, and only updated those. I didn't regenerate the emf model - I just updated the data types, which so far it seems to be working for my limited use case.

[fgdrf]

Good to hear uDig runs with GeoTools 21.x. I assume jgrass and jconsole stuff isn't bundled into your application..
However we can handle it.

I just saw a transitive dependency from gt-netcdf (19.4) module in libs-bundle which can be provided using import package in MF file of org.locationtech.udig.libs (and upgrade to newer version)

[INFO] +- org.geotools:gt-netcdf:jar:19.4:compile
[INFO] |  +- edu.ucar:cdm:jar:4.6.6:compile
[INFO] |  |  +- edu.ucar:udunits:jar:4.6.6:runtime
[INFO] |  |  +- edu.ucar:httpservices:jar:4.6.6:runtime
[INFO] |  |  |  +- org.apache.httpcomponents:httpclient:jar:4.5.1:runtime
[INFO] |  |  |  \- org.apache.httpcomponents:httpmime:jar:4.5.1:runtime
[INFO] |  |  +- org.apache.httpcomponents:httpcore:jar:4.4.4:runtime
[INFO] |  |  +- joda-time:joda-time:jar:2.8.1:runtime

I've to check if netcdf 21.x still has this dependency ..

[fgdrf]

GeoTools 22.0 has been released which includes an update of EPSG database. Thanks to @jodygarnett for this info since it might fix #242

[nprigour]

Hi @fgdrf ,

I see that you have included this isssue to 2.1.0 release milestone. However compared to the previous release (2.0.0) we have already carried out a significant update from geotools 14.x to 19.x. Therefore IMHO and given the size of the expected refactoring effort, it would be more prudent to have a stable (or at least and interim) release with the 19.4 version of geotools and then delve into the "refactoring hell" that support of geotools 22.x will require.

[fgdrf]

@nprigour Totally agree! Maybe we can discuss with @egouge how to contribute commits from SMART project branch https://github.com/egouge/udig-platform/tree/smart7 into uDig where a lot of GeoTools changes were made already (including joda-time removal if I read changes correctly)

Therfore I've created 2.2.0 Milestone and added this issue

[egouge]

I can help with this, although it might be easier to start from scratch again rather than pull the stuff from SMART. Should I start a new branch for this and start poking away at it and see how far I get?

[fgdrf]

I can help with this, although it might be easier to start from scratch again rather than pull the stuff from SMART. Should I start a new branch for this and start poking away at it and see how far I get?

Great! Looking forward to collaborate on this. Maybe we should increase version to 2.2.0-SNAPSHOT on master and start from there on a new branch.

I'd join to get rid of joda.time and will try to re-generate code from model. if it works out as expected we can close #357

[egouge]

I cannot find a definition for SI.KILOMETER or SI.CENTIMETER units. This is used by the DistanceTool in uDig. Does anyone know what I can use for these?

[nprigour]

Hi @egouge ,

Check https://docs.geotools.org/latest/userguide/welcome/upgrade.html. There is a subsection specific to Migrate to JSR-363 Units under GeoTools 20.x section. At the bottom of this subsection there is a paragraph about Using Units with an example demonstrating the usage of Quantity classes

[egouge]

I saw that page, but missed the "Using Units" section. Thanks for pointing it out.

[egouge]

Any thoughts on what to do with the use of "org.geotools.data.CachingFeatureSource;" That class no longer exists and the comments on older version of geotools says "@deprecated This class is not tested enough to be considered production ready".

From what I can tell this is only used by the CacheInterceptor.

[egouge]

I've managed to get just about everything to compile, with the exception of the CacheInterceptor as stated above. The repo is here:
https://github.com/egouge/udig-platform/tree/geotools22

I have not tested, run, or otherwise validated that any of the changes work.

There are at least two outstanding issues:

• CacheInterceptor
• PIXEL units in UnitList. I couldn't find a value for PIXEL, so for now I commented it out

[jodygarnett]

@egouge I made a static constant for Units.PIXEL because it was not readily accessible in any of the provided SI or NonSI definitions. The definition 1/72 Inch is in keeping with prior use...

[jodygarnett]

Reading CacheInterceptor it looks to be caching features in memory. While this was added to the core library for uDig it was not being maintained tested and has been dropped.

You can see a replacement by comparing:
Quickstart Things to Try (master)

        // CachingFeatureSource is deprecated as experimental (not yet production ready)
        CachingFeatureSource cache = new CachingFeatureSource(featureSource);

Quickstart Things to Try (maintenance)

        FileDataStore store = FileDataStoreFinder.getDataStore(file);
        SimpleFeatureSource featureSource = store.getFeatureSource();
        SimpleFeatureSource cachedSource =
                DataUtilities.source(
                        new SpatialIndexFeatureCollection(featureSource.getFeatures()));

It is not quite the same (no events keeping the cache in sync), but it does offer an in memory copy of the data with a spatial index for performance.

[jodygarnett]

Andrea has the details, you are welcome to pick up the older CachingFeatureSource implementation (either directly into the uDig codebase, or as a new geotools community module).

[nprigour]

Hi @egouge ,

I added a PR related to your branch where I provide some further modifications (see PR comments for details) required for compilation and succesful test execution by running: mvn clean install -Pproduct,test
Note that I did not apply the changes suggested by @jodygarnett regarding CachedInterceptor and Units.PIXEL since especially for the former we must take a decision on how to proceed (I have done it only locally to my IDE in order to verify compilation).

[egouge]

Great thanks! I merged @nprigour PR and fixed up the Unit issue.

Does anyone have any thoughts on what we should do with the CachedInterceptor? I would be inclined to bring an old copy of the CachingFeatureSource class into the uDig code base.

[jodygarnett]

Reading the java docs for SpatialIndexFeatureSource it appears to be a port of CachingFeatureSource.

I think the FeatureCollection events being used by CachingFeatureSource were removed. We needed a new implementation that listens to FeatureListener changes instead.

I would recommend adding the listener logic to SpatialIndexFeatureSource and use that (or extend the class in uDig to listen to events until you have a solution that works).

[egouge]

I looked at this for a while, and ended up copying over the CachingFeatureSource and making it work with the newest geotools. I was able to run uDig and get it to load and view a bunch of shapefiles (yeh!)
But then I tried to get all the test to run using the command:
mvn clean install -Pproduct,test

But it fails on the catalog.wfs.tests for me: :(

testNormal(org.locationtech.udig.catalog.tests.ui.WFSCatalogImportTest)  Time elapsed: 12.917 s  <<< ERROR!
java.lang.IllegalStateException: condition is false!  Condition that failed is:org.locationtech.udig.catalog.tests.ui.CatalogImportTest$1@234bfc8c

Thoughts?

[nprigour]

Hi @egouge ,

In my local machine I do not receive this exception when running maven from command prompt (I am using mvn version 3.6).
However there seems to be an exception thrown in the specified test suite related to the absence of a class from org.apache.commons.codec. Moreover the specified test fails when launched from within eclipse IDE.
I have added another PR related to your branch where I fix the missing dependancy to org.apache.commons.codec. Please try it and see if the mentioned failure is inhibited and report back.

[egouge]

That seems to have resolve the issue I was having - thanks!

I was able to build, run the build and load a shapefile. I was unable to load my single-banded tiff image though so that will require some further investigation.

[nprigour]

Hi @egouge, if you are willing to provide the example tiff I may also have a look at it.

[egouge]

This is a very simple raster with two cells. I suspect I might be missing a library or have something else not configured correctly. I haven't had a chance to investigate further.
SampleRaster.zip

[nprigour]

It seems the problem was related to imageio-ext jars not properly being updated to the newer version required by geotools. I upgraded to v1.3.2 and the problem seems to be resolved. I submitted a PR to your local geotools22 branch.

Maybe we should also check what is going on in general with newer libraries version and clear the pom-libs.xml a bit

[egouge]

Works for me now. Thanks.

[fgdrf]

Please correct me if I'm wrong, is this issue solved since GeoTool 22.1 is on master already?

If so we can close this issue.

However @moovida verified that with Eclipse-2019-03 under he hood uDig runs on MacOSX successful too

[nprigour]

Yes this issue was related to upgrade to geotools 22.x and has been resolved.