ApiDoc of ReprojectingFeatureCollection not in line with implementation

Description

It is said in the API doc that the ReprojectingFeatureCollection only reprojects the default geometry. But actually it reprojects all geometries. This leads to unwanted results.
In the ReprojectingFeatureIterator class all attributes of a feature are itterated and the geometry is reprojected if it is an instance of Geometry.
https://github.com/geotools/geotools/blob/0e6fc022f395194e7670b63d2de785123a178cdf/modules/library/main/src/main/java/org/geotools/data/store/ReprojectingFeatureIterator.java#L115

Environment

None

Activity

Show:
Andrea Aime
June 3, 2018, 8:37 AM
Marco Peters
June 3, 2018, 11:55 AM

I don't know what the correct/intended behaviour is. So I can't open a PR.
Maybe the intention is to reproject all geometries. Maybe it is already used like this.
If I would know I could do a PR.
The simplest fix is to update the API and let it mention that all geometries are reprojected.
Or introduce a ReprojectingFeatureCollection.features(boolean allGeometries) method, deprecate the existing ReprojectingFeatureCollection.features() and let it call the new one with features(true). This would allow to support both ways.
But this would not help if the parent class DecoratingSimpleFeatureCollection is used generically.

How should this be handled?

Andrea Aime
June 3, 2018, 12:35 PM

The current behavior has been the same for years, and changing it would break downstream applications (e.g., GeoServer WFS) so yes, fixing the doc seem to be the sane/cautious behavior.

Adding a property to control which geometries are reprojected, off by default (if not used, the behavior is same as today) is also a good option, but I'd open a different ticket for it (improvement type) and add tests to cover the new code paths.

Marco Peters
July 4, 2018, 6:27 AM

I've placed a PR:
https://github.com/geotools/geotools/pull/1956

For some reason, it says the branch has conflicts. But should be possible to merge it.

Andrea Aime
July 4, 2018, 6:31 AM

I've tried to manually fix the conflict in the GitHub UI... let's see if the build passes (we have now formatting checks)

Assignee

Unassigned

Reporter

Marco Peters

Triage

None

Components

Fix versions

Affects versions

Priority

Medium
Configure