geotools/geotools — pull request #434 by jodygarnett (2014-04-28 01:34:29+00:00)
Title: Filter clean up (GEOT-4785)

Motivation is org.geotools.filter.Filter cast exceptions and org.geotools.filter.Expression cast exceptions.

1) Deprecated Filter methods cleanup (which force client code into an unchecked cast to GeoTools Filter or Expression):
[x] remove Filter.contains( feature)
[x] remove Filter.and( filter )
[x] remove Filter.or( filter )
[x] remove Filter.not()
[x] Filter.getFilterType() - replaced with use of Filters.getFilterType( filter )
[x] Filter.accept( FilterVisitor ) - Last serious implementation is SQLEncoder which is no longer used.

2) Adding an expressionCast method prevents casual failure in client code:

[x] expressionCast CompareFilterImpl.getLeftValue()
[x] expressionCast CompareFilterImpl.getRightValue()
[x] expressionCast IsBetweenImpl.getMiddleValue()
[x] expressionCast IsNullImpl.getNullCheckValue()
[x] expressionCast BetweenFilterImpl.getMiddleValue()
[x] expressionCast GeometryFilterImpl.getLeftGeometry()
[x] expressionCast GeometryFilterImpl.getRightGeometry()
[x] expressionCast LikeFilterImpl.getValue()

The first batch of class cast exceptions - addressed by using expressionCast method that will return null if expression is not a GeoTools Expression. This approach trades Class Cast Exception for NPE, but will allow for code such as equals() that protects against null to function, and may help generic client code.

3) Cleanup use of CompareFilter.getRightValue() and CompareFilter.getLeftValue():
[x] cleanup CompareFilterImpl.equals
[x] cleanup CompareFilterImpl.hashcode
[x] cleanup CompareFilterImpl.compare
[x] cleanup And the rest of them ...

Second batch of class cast exceptions - strategy was to replace with getExpression1() or getExpression2() as appropriate. This approach fixes the problem and allows filter implementations to work for normal use.

4) Remove deprecated Filter classes FilterVisitor and FilterVisitor2

[x] Delete FilterVisitor
[x] Delete FilterVistior2
[x] Remove AbstractFilterVisitor
[x] Replaced unused classes such as SQLEncoder, WFSBBoxFilterVisitor, FilterVisitorFilterWrapper, etc...
[x] Migrate last remaining visitor implementation FilterEncodingFilterVisitor (by stealing bridge code from FilterVisitorFilterWrapper)
[x] Remove Filters.accepts( filter, visitor )

This removes implementations that are expressed in terms of GeoTools Filter and Expression. These implementations (by definition) force the codebase into class cast exceptions

Cleanup was remarkably smooth, the removal of FilterVisitor was the only change that broke GeoServer. This will lead to a pull request against GeoServer WFS GetFeature.java shortly ...

5. Nice to Have

[ ] Remaining use of CompareFilter.getLeftValue() and getRightValue()
[ ] Use of CompareFilter addLeftValue() and addRightValue() - used a lot in test cases, replace with filter factory
[ ] Remaining use of GeometryFilter getLeftGeometry() and getRightValue()
[ ] Use of GeometryFilter addLeftGeometry() and addRightGeometry() - used a lot in test cases, replace with filter factory

Priority would be given to codebase, rather than test cases.