search

qgis / QGIS

QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)
https://qgis.org
GNU General Public License v2.0
10.62k stars 3.01k forks source link

Edits in GeoJson datasources are not saved anymore #28580

Closed qgib closed 5 years ago

qgib commented 5 years ago

Author Name: Ehsan Aliverdi (Ehsan Aliverdi) Original Redmine Issue: 20760 Affected QGIS version: 3.4.4 Redmine category:data_provider

I tested this issue in Qgis 3.4.0, 3.4.1, 3.4.2 in all the same issue in our company we work with Geojson files in Qgis. When editing the attributes of an existing feature in Geojson layer, QGIS creates a temp layer and applies the changes on that new temp layer and never applies the changes on original layer. this problem wasn't there in version 3.2.3

qgib commented 5 years ago

Author Name: Giovanni Manghi (@gioman)

qgib commented 5 years ago

Author Name: Ben Hur Pintor (@benhur07b)

Hi Ehsan,

Can you provide a sample dataset to test this? I just tested editing a GeoJSON layer on 3.4.2 (Ubuntu 18.04) and everything works as expected. I can edit the fields and even add/delete fields. No temp layer is created and the original layer is edited.

Thanks!

Sincerely, Ben Hur

qgib commented 5 years ago

Author Name: Ehsan Aliverdi (Ehsan Aliverdi)

Ben Hur Pintor wrote:

Hi Ehsan,

Can you provide a sample dataset to test this? I just tested editing a GeoJSON layer on 3.4.2 (Ubuntu 18.04) and everything works as expected. I can edit the fields and even add/delete fields. No temp layer is created and the original layer is edited.

Thanks!

Sincerely, Ben Hur

Hi Ben Thanks for reply Please find a sample dataset enclosed. I have tested this in more than 10 different pc with windows. I just did another test with the following qgis(see the image enclosed) step to reproduce the issue: -Put equipment layer on editable mode -use identify tool to id a feature(anyone will work) -use edit feature form to edit an attribute of a feature -save the changes -toggle edit -use identify tool to id the feature. You will see nothing is changed.

Kindly Regards Ehsan

qgib commented 5 years ago

Author Name: Ben Hur Pintor (@benhur07b)

Ehsan Aliverdi wrote:

Hi Ben Thanks for reply Please find a sample dataset enclosed. I have tested this in more than 10 different pc with windows. I just did another test with the following qgis(see the image enclosed) step to reproduce the issue: -Put equipment layer on editable mode -use identify tool to id a feature(anyone will work) -use edit feature form to edit an attribute of a feature -save the changes -toggle edit -use identify tool to id the feature. You will see nothing is changed.

Kindly Regards Ehsan

Hi Ehsan,

For my setup (QGIS 3.4.2 on Ubuntu 18.04), I could edit the attributes of the GeoJSON. Kindly check the GIF attached if I followed your instructions to reproduce the bug. Maybe this is a Windows-specific problem. If possible, could you also send a video or GIF of the bug in action.

Thank you.

Sincerely, Ben Hur

qgib commented 5 years ago

Author Name: Ehsan Aliverdi (Ehsan Aliverdi)

Ben Hur Pintor wrote:

Ehsan Aliverdi wrote:

Hi Ben Thanks for reply Please find a sample dataset enclosed. I have tested this in more than 10 different pc with windows. I just did another test with the following qgis(see the image enclosed) step to reproduce the issue: -Put equipment layer on editable mode -use identify tool to id a feature(anyone will work) -use edit feature form to edit an attribute of a feature -save the changes -toggle edit -use identify tool to id the feature. You will see nothing is changed.

Kindly Regards Ehsan

Hi Ehsan,

For my setup (QGIS 3.4.2 on Ubuntu 18.04), I could edit the attributes of the GeoJSON. Kindly check the GIF attached if I followed your instructions to reproduce the bug. Maybe this is a Windows-specific problem. If possible, could you also send a video or GIF of the bug in action.

Thank you.

Sincerely, Ben Hur

Thanks alot for your reply. I am using windows. for your information I attached a gif file which shows the problem. we use qgis on 20 PCs in our company and all have the same issue because of this issue we downgraded every qgis to 2.3.2 which works perfect.

Regards Ehsan

qgib commented 5 years ago

Author Name: Paul Blottiere (Paul Blottiere)

FWIW, I didn't succeed in reproducing the issue either.

qgib commented 5 years ago

Author Name: Giovanni Manghi (@gioman)

Paul Blottiere wrote:

FWIW, I didn't succeed in reproducing the issue either.

I can easily replicate on 3.4.4 (following the steps in the attached cast) on Windows (clean install of both QGIS and the OS).

qgib commented 5 years ago

Author Name: Paul Blottiere (Paul Blottiere)

Hi @Giovanni,

I can easily replicate on 3.4.4 (following the steps in the attached cast) on Windows (clean install of both QGIS and the OS).

OK, so according to @Ben Hur comment and my own tests on Debian, it seems that it's pretty specific to Windows.

qgib commented 5 years ago

Author Name: Peter Petrik (@PeterPetrik)

unable to replicate on MacOS too. So definitely windows-only issue.

qgib commented 5 years ago

Author Name: Adam Liddell (@aaliddell)

Also replicated this bug on 3.6.0 with Windows 7. The edits are written to a .geojson.tmp file, but never copied over the top of the .geojson file.

qgib commented 5 years ago

Author Name: Adam Liddell (@aaliddell)

Assuming this is the driver used for GeoJSON, could this be the offending section in GDAL OGR: https://github.com/OSGeo/gdal/blob/ab9e80368ef662712c963191fd86fde2c5b75600/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsondatasource.cpp#L1056-L1098

Here's the steps it's taking:

  1. The new *.geojson.tmp file containing the changes is written
  2. The .geojson is moved to .geojson.bak
  3. The .geojson.tmp file is renamed to .geojson
  4. The *.geojson.bak file is removed

On Windows, this appears to be failing in step 2, as the tmp file is created but the bak file is not. Perhaps this is due to how Windows does file locking, meaning the rename is prohibited as the file appears to be open? Unfortunately I can't seem to get the log output from this section of code, so I don't see 'Cannot create backup copy' anywhere.

qgib commented 5 years ago

Author Name: Amine Aboufirass (Amine Aboufirass)

Has there been any progress on this? I also submitted a similar question on stackexchange... https://gis.stackexchange.com/questions/319450/qgis-values-in-attribute-table-become-null-after-saving

Pehaps there is a workaround?

qgib commented 5 years ago

Author Name: Evgeniy Lazarev (Evgeniy Lazarev)

I asked Oct-31-2018 on stackexcange about this problem: https://gis.stackexchange.com/questions/300860/why-does-qgis-3-4-and-higher-create-tmp-file-instead-of-refresh-editing-layer-a Since then I have to use 3.2.3 QGIS version.

qgib commented 5 years ago

Author Name: Edmond Lai (Edmond Lai)

I am having the same issue with QGIS version 3.6.2-Noosa on Windows 10.

qgib commented 5 years ago

Author Name: louis de clebs (louis de clebs)

I am having the same issue on windows 7 (Unfortunately have to use windows at work). Might also have something to do with line endings? When I commit a geojson file created by QGIS, git put a warning saying LF line ending in the geojson file are replaced by CRLF. Would it be something there https://github.com/qgis/QGIS/blob/master/src/core/qgsvectorfilewriter.cpp#L1060 ?

Only workaround at this stage is to save the file, close qgis, delete old geojson, rename new file by removing ".tmp" extension and re-open qgis... Or is there an easier way?

lmdc45 commented 5 years ago

Updated workaround : If the file has been created from QGIS, use notepad++ or unix2dos to update EOL to Windows style. When EOL is windows style, you can delete a feature. Before saving the file, copy any random feature from the layer, cut and paste it (just looks like you had a feature). Then saving is working. Points is that last action on the layer must be adding a feature.

timlinux commented 5 years ago

I tested the equipment.geojson file on macOS and it edits without issues.

aaliddell commented 5 years ago

Given this is marked as 'high priority', is a data-loss bug and has been idle for >7 months, is there a plan for addressing this?

Tagging this with 'Crash/Data Corruption' would also probably be suitable

gioman commented 5 years ago

@aaliddell Hi agree is high priority as any regression, but it would not be right to tag it as data corruption, as in fact there is not (the original data isn't lost or corrupted, isn't it?). You may want to chime in in this thread too: https://lists.osgeo.org/pipermail/qgis-developer/2019-July/057978.html

rouault commented 5 years ago

This sounds pretty much as similar issues with shapefile compaction, and it appears indeed that "something" on Windows keeps a lock on the file that prevents to rename it, but overwriting it should be possible (that's how this was fixed for shapefiles). The strange thing is that that part of the code is tested by GDAL autotests, and they are run on Windows through AppVeyor CI... What I suspect is that it might be related to a process like an antivirus running on normal people machines, but it might be disabled on AppVeyor. Could people try to temporarily disable their antivirus to confirm ? Also, it would be interested if they could try outside of QGIS, both with antivirus enbled or disabled, the following python script (from the OSGeo4W console):

from osgeo import ogr
ds = ogr.Open('test.geojson', update = 1)
lyr = ds.GetLayer(0)
f = lyr.GetNextFeature()
lyr.SetFeature(f)
ds = None

with any 'test.geojson' file and see if error messages are emitted on the console

aaliddell commented 5 years ago

Hi agree is high priority as any regression, but it would not be right to tag it as data corruption, as in fact there is not (the original data isn't lost or corrupted, isn't it?)

Depends what's considered data. The original data is indeed not touched, but there's also the new data you want to add/edit/delete, which vanishes (to the tmp file first but that'll be lost if you save more than once). Before finding this issue I made a set of edits, saved the file and then moved on to other tasks assuming my edits were persisted; the file then not having the expected content appears to the end user as corruption despite the file still being valid GeoJSON, which is little consolation to having to redo work, but this is mostly semantics over a tag and I'm glad to see some movement...

Is there also the possibility that another part of QGIS is hanging onto an open file handle in recent versions? I ask because it's curious that old versions of QGIS still save GeoJSON fine, which I wouldn't expect to see if it's AV related. Perhaps there's a lsof equivalent on Windows.

I'll have a go with that script and also see if there's an AV log.

aaliddell commented 5 years ago

Here's a summary of a test that demonstrates this behaviour:

There are no records of blocked actions in the AV log. So deletes and edits don't seem to get persisted, but additions do :man_shrugging:. Doing the same steps but picking shapefile when making the layer permanent works fine.

rouault commented 5 years ago

Here's a summary of a test that demonstrates this behaviour:

Is it systematically reproducible, at the same steps ? There should be no difference really regarding the type of action, so I suspect something random. What if you disable the AV ?

There are no records of blocked actions in the AV log

I wouldn't expect anything to be recorder in the AV log. But more an annoying interaction between it and QGIS/GDAL. That is that when new files are created / renamed, the AV would notice those file system changes and open the new file to analyze it, thus creating a temporary lock that prevents immediate further renaming.

aaliddell commented 5 years ago

Is it systematically reproducible, at the same steps ? There should be no difference really regarding the type of action, so I suspect something random. What if you disable the AV ?

Yes, I've gone four times through those steps and I'm yet to see a different behaviour. I'm on a work machine currently, so can't disable AV presently :unamused: but will try on a different machine later. However, Giovanni Manghi (@gioman) up above had this comment:

I can easily replicate on 3.4.4 (following the steps in the attached cast) on Windows (clean install of both QGIS and the OS).

Using process explorer (lsof equivalent) to examine open files, QGIS has two open file handles for the GeoJSON file whilst idle in edit mode. Once exiting edit mode, one of these file handles is closed but one remains (or possibly both closed and one opened, I cannot see the handle IDs). This may be the handle that is 'locking' the file from being changed. Do we know what part of QGIS is holding these handles and if there is a difference in behaviour with how it closes them when editing vs adding features?

rouault commented 5 years ago

OK, I think I know what is happening. Probably not antivirus related after all. The other file handle opened on the file comes from the renderer "threads", and they are cached for one minute even after they have completed. Probably that adding explicit QgsOgrConnPool::instance()->invalidateConnections() calls like done in a number of places in https://github.com/qgis/QGIS/blob/master/src/core/providers/ogr/qgsogrprovider.cpp would help, by forcing them to be closed. Reviewing closer, what you observe can be explained: adding a new feature at the end of a GeoJSON file doesn't require it to be entirely rewritten, so the rename game doesn't happen here. Regarding adding new fields, there's a special processing at https://github.com/qgis/QGIS/blob/master/src/core/providers/ogr/qgsogrprovider.cpp#L4570 that takes care of force closing everything. So the fix would be two folds:

aaliddell commented 5 years ago

Awesome :+1:

rouault commented 5 years ago

@aaliddell I guess you could "workaround" the issue by waiting for one minute, and without doing any scroll or pan action or anything on the canvas, after each edit action and before saving the edits (that would be more a way to confirm my above hypothesis than a real workaround of course). That should be sufficient for the cached connections to be automatically closed.

aaliddell commented 5 years ago

Yep, can confirm that waiting a minute after making an edit before saving works

radumas commented 5 years ago

Yep, can confirm that waiting a minute after making an edit before saving works

I thought this did not work for me on Windows 7 and QGIS 3.6.3, but it was because I had multiple projects open in order to test with a "Clean" user profile. Can confirm waiting works for me as well.

lennoo commented 5 years ago

waiting works for me on windows 7 and QGIS 3,6. I wonder what version would fix the bug

Mike-Honey commented 5 years ago

Waiting a minute isn't working for me, I'm reduced to trying to snatch copies of the .tmp file, renaming them after closing QGIS. It seems when the QGIS project is saved or even just closed it reverts the .geojson files to their original state? Windows 10 and QGIS 3.8.1, also tried QGIS 3.4.10. I hope this can be fixed soon as it's dramatically slowing my workflow.

I would categorise it as "data loss" - we enter data using the app, it appears as permanent, yet later we find our changes have been lost.

FireByTrial commented 5 years ago

I'm still getting this on 3.8.3-Zanzibar (on Windows 10). open a .json/.geojson file and added a new column. edited column values and saved edits. these will be lost if a new attribute table windows is opened (after saving edits and stopping edits). Additionally if the layer is duplicated the new column that was added will not exist in the duplicated layer

gioman commented 5 years ago

I'm still getting this on 3.8.3-Zanzibar. open a .json/.geojson file and added a new column. edited column values and saved edits. these will be lost if a new attribute table windows is opened (after saving edits and stopping edits). Additionally if the layer is duplicated the new column that was added will not exist in the duplicated layer

@FireByTrial probably the fix was not backported, so you must wait for the next release. You can try qgis master in the meantime.

rouault commented 5 years ago

probably the fix was not backported, so you must wait for the next release

The fix is in GDAL. No release have been yet made with it (unless QGIS master on Windows uses GDAL master, but I'm not sure?). Next GDAL maintainance releases (source tarball) are for next week

FireByTrial commented 5 years ago

I ended up exporting to geopackage and doing my edits with that format for now. It seems to work fine and I will just export back to geojson when done.

👍 Thanks for the quick replies.

lmdc45 commented 4 years ago

seems to work just fine with latest 3.10 release.

Thanks