search

locationtech / udig-platform

uDig parent project containing all core components. More plugins can be found in community repos: http://github.com/uDig-Community
http://udig.refractions.net
190 stars 133 forks source link

In Shapefile layers, uncommitted features are affected by all operations. #348

Open akater320 opened 5 years ago

akater320 commented 5 years ago

When editing a shapefile layer, uncommitted features are affected by (almost) all edit operations. Examples: Select any feature (e.g. polygon) in a shape file layer. That polygon plus all uncommitted polygons will be selected. Attempt to delete any feature in a shapefile layer. That polygon plus all uncommitted polygons will be deleted. Attempt to move/modify the geometry of any feature. All uncommitted features will be assigned a copy of the same geometry. The result is multiple features stacked on top of each other.

This seems to be caused by GeoTools not propagating the filter/query to the DiffFeatureReader. All features in the current transaction will be added to the returned iterator. ContentFeatureSource.java

nprigour commented 5 years ago

This is quite a serious bug. I just check it and indeed the behaviour if you try to edit more than one feature without prior committing the changes is really weird! It seems also that the issue is present for quite some time, maybe from upgrading to geotools 19.x. I wonder how this got unoticed for such a long time (to be honest I work almost solely with jdbc layers where the problem seems not to be present)!! @akater320 are you sure that the issue stems from the ContentFeatureStore? Moreover are you aware whether it has been fixed in subsequent geotools versions?
@fgdrf I think we should urgently ask a question about this to the geotools team and request their counsel.

nprigour commented 5 years ago

Checking a little bit further it seems that we have this issue in udig even in version 2.0.0 that uses geotools 14.1. I had to go 4-5 years back in time that is udig 1.4 and geotools 9.0-M0 to see correct behavior during multiple edits of a shapefile layer!

akater320 commented 5 years ago

Thanks for confirming. The problem probably this line: https://github.com/geotools/geotools/blob/e40158adf59a37d985133184968f251d9a8ff4f8/modules/library/main/src/main/java/org/geotools/data/store/ContentFeatureSource.java#L619

It should be using the constructor with the Filter parameter. https://github.com/geotools/geotools/blob/e40158adf59a37d985133184968f251d9a8ff4f8/modules/library/main/src/main/java/org/geotools/data/DiffFeatureReader.java#L93

The default filter is Filter.INCLUDE, so queryAdded() will return everything rather than only the features matching the user's query. https://github.com/geotools/geotools/blob/e40158adf59a37d985133184968f251d9a8ff4f8/modules/library/main/src/main/java/org/geotools/data/DiffFeatureReader.java#L211

akater320 commented 5 years ago

FYI: You may encounter this issue while testing with ShapeFiles. https://github.com/geotools/geotools/commit/a26e08badb6c4d95de6f11aa025839f4fba35ad4

nprigour commented 5 years ago

But @akater320 I cannot understand if the problem is indeed where you mention, shouldn't all FeatureSources be affected since this method getReader is final and used by all FeatureSources that inherit from the ContentFeatureSource (that is to say all type of sources)? I.e. I work for some years with JDBCFeatureSources (MySQL) in udig and the problem surely not present there!

akater320 commented 5 years ago

Excellent point @nprigour It looks like JDBCFeatureStore delegates read-only operations to JDBCFeatureSource which handles transactions intrinsically. https://github.com/geotools/geotools/blob/e40158adf59a37d985133184968f251d9a8ff4f8/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureStore.java#L211 https://github.com/geotools/geotools/blob/e40158adf59a37d985133184968f251d9a8ff4f8/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureSource.java#L544

JDBCFeatureReader never gets wrapped by a DiffFeatureReader in this block. https://github.com/geotools/geotools/blob/e40158adf59a37d985133184968f251d9a8ff4f8/modules/library/main/src/main/java/org/geotools/data/store/ContentFeatureSource.java#L616

nprigour commented 5 years ago

Hi @akater320, Following your advice I checked by replacing the DiffFeatureReader constructor with the one that contains also the Filter parameter (in the suggested line 619 of ContentFeatureSource) and it seems that the problem is resolved in what concerns multiply editing actions in Shapefiles (JDBC layers also seem unaffected by this change). That is great! @akater320 do you have an account in geotools project? If yes would it be possible to create a jira ticket in geotools (https://osgeo-org.atlassian.net/projects/GEOT/issues) describing the problem and possibly also referencing the present issue to justify the problem? Then you can proceed with providing a PR fix on geotools github. Note: as far s I know, geotools is now in version 21.x which has a lot of changes compared to v19.4 used by udig (also it is probably not under support anymore) but I am sure the problem is still present. Anyway let's propose a fix and we may ask later how we can backport it to v19.x.

akater320 commented 5 years ago

https://osgeo-org.atlassian.net/projects/GEOT/issues/GEOT-6293

nprigour commented 4 years ago

Hi @akater320,

would it be possible to revise the https://osgeo-org.atlassian.net/projects/GEOT/issues/GEOT-6293 jira issue according to what I am proposing in https://github.com/geotools/geotools/pull/2604 (see my latest comment) so that we can proceed with the geotools fix. If you are unable to rename the issue and/or its description, please inform back so that I generate another one and we set the GEOT-6293 as duplicate.

jodygarnett commented 4 years ago

The fix for this is being included in GeoTools 22.1.