SQLEncoder Upgrade to GeoAPI Filters - STEMLab/geotools GitHub Wiki

There's a long road to internally using GeoAPI filters rather than Geotools filters. This is just a little piece of the work.

I was itching to get the ArcSDE datastore plugin moved completely off of the "old" filters and completely onto the "new" filters.

Most of the ArcSDE datastore plugin moved quite cleanly over, but there were a few bits of main and jdbc that were being pulled in, and which were using deprecated interfaces. So I set about to:

1. Clean up SQLEncoder.java, moving it from using "old" filters to "new" filters
2. Stop using the SQLUnpacker class and instead use PostPreFilterSplittingVisitor for the SDE internal filter splitting needs

In order to complete this work I had to make two non-trivial decisions that I'd like to run by any interested parties (particularly the main and jdbc module maintainers: jody and cory!) Currently all tests pass and I'm synced to the latest svn revision with no conflicts.

Items need to be completed in descending order.

⚠️ I'm committed to tracking down any code that uses the old SQLUnpacker or SQLEncoder classes and either:

• Bugging the maintainer to change it
• Changing it myself it it's not maintained (in unsupported)

Trying to clean up SQLEncoder.java turned out to be a bad idea. There are many datastore plugins which subclass this class as a way of getting easy filter -> SQL encoding support, plus a few customizations. Changing the underlying methods of SQLEncoder was no good as I don't really have the expertise to fix all the datastores at the same time.

So instead I created a new port of the class (called 'GeoAPIFilterToSQLEncoder') in the org.geotools.data.jdbc package. This new class is really just a port of the old SQLEncoder class, but made to use "new" filters. I then deprecated the old org.geotools.filter.SQLEncoder class (and it's exceptions) and ported its tests/exceptions over to the new GeoAPIFilterToSQLEncoder.

I left a note in SQLEncoder about which class replaces this now-deprecated class.

QUESTION: What should the name of the new SQLEncoder class be? Is the new different name for the class in a different package a good one? Or should it keep the same name, just change package?

QUESTION: What should the package be of the new SQLEncoder class (regardless of its name)? The old SQLEncoder lived in org.geotools.filter. SHould the new one live in org.opengis.filter? Or is living in org.geotools.data.jdbc good enough/better?

After creating and testing this new class, I then ported the SQLEncoderSDE over to use the new class, and everything works great with 100% "new" filters.

Moving from SQLUnpacker to PostPreProcessFilterSplittingVisitor is fairly painless, once you've figured out how PPPFSV works. I ported PPPFSV to use the new filters, and it works just fine now.

FilterCapabilities now represents two things. It represents the capabilites of a filter as expressed with old filters, and the capabilities of a filter as expressed with new filters. This means that the FilterCapabilities class is a bit of a frankenstein, with code handling each individual case/bit in each method and use-case.

QUESTION: What do do with FilterCapabilities?

• Dual nature is accepted as an "ok thing", and the class is re-architected to account for that.

• Dual nature is rejected as a confusing/bad thing, and the class is split into an older, deprecated version and a newer version called "FilterCapabilitiesImpl" which implements the org.opengis.filter.capabilities.FilterCapabilitie interface (or whatever the right interface is in geoapi).

This is not the first time this issue has been raised. It's a part of the 2.5.x timeline and it'll raise its head in a few places. Just pointing out some possible routes here. Personally I vote for the latter route.

Jody's comment:

I would like to move towards the opengis.filter.capabilities - but I imagine we will learn a few things (and may need to revise the interfaces - justin already ran into some limitations around Function?).

deprecation:

 Index: modules/library/jdbc/src/main/java/org/geotools/filter/SQLEncoder.java
 Index: modules/library/jdbc/src/main/java/org/geotools/filter/SQLEncoderException.java

Change to support "new" filters rather than "old" filters

 Index: modules/library/main/src/main/java/org/geotools/filter/visitor/PostPreProcessFilterSplittingVisitor.java

Minor changes/cleanup to support the new PostPreProcessFilterSplittingVisitor:

 Index: modules/library/jdbc/src/main/java/org/geotools/data/jdbc/DefaultSQLBuilder.java
 Index: modules/library/main/src/test/java/org/geotools/filter/visitor/PostPreProcessFilterSplittingVisitorTest.java
 Index: modules/library/main/src/test/java/org/geotools/filter/visitor/AbstractPostPreProcessFilterSplittingVisitorTests.java
 Index: modules/library/main/src/test/java/org/geotools/filter/visitor/PostPreProcessFilterSplittingVisitorSpatialTest.java
 Index: modules/library/main/src/test/java/org/geotools/filter/visitor/PostPreProcessFilterSplitterVisitorFunctionTest.java
 Index: modules/plugin/wfs/src/main/java/org/geotools/data/wfs/WFSDataStore.java

More substantial changes were required to get FilterCapabilities to work as a representation of both new and old FilterCapabilities.

 Index: modules/library/main/src/main/java/org/geotools/filter/FilterCapabilities.java

New files:

 modules/library/jdbc/src/main/java/org/geotools/data/jdbc/GeoAPIFilterToSQLEncoderException.java
 modules/library/jdbc/src/main/java/org/geotools/data/jdbc/GeoAPIFilterToSQLEncoder.java
 modules/library/jdbc/src/test/java/org/geotools/data/jdbc/SQLFilterSuite.java
 modules/library/jdbc/src/test/java/org/geotools/data/jdbc/SQLFilterTestSupport.java
 modules/library/jdbc/src/test/java/org/geotools/data/jdbc/GeoAPIFilterToSQLEncoderTest.java