Opened 13 years ago
Closed 13 years ago
#4467 closed defect (fixed)
Don't silently drop locally deleted member primitives from downloaded ways and relation
Reported by: | mjulius | Owned by: | team |
---|---|---|---|
Priority: | critical | Milestone: | |
Component: | Core | Version: | latest |
Keywords: | r-2010-01-blocker | Cc: | nakor.wp@…, skyper |
Description
JOSM is currently dropping locally deleted way nodes or relation members from downloaded ways and relations.
I believe this is dangerous because it alters the downloaded data without a really good way of restoring that and maybe even without the user noticing. Instead, this should raise a conflict or maybe a warning message with the option to undelete those nodes.
Attachments (0)
Change History (25)
comment:1 Changed 13 years ago by
Priority: | normal → critical |
---|
comment:2 Changed 13 years ago by
comment:3 Changed 13 years ago by
Cc: | nakor.wp@… added |
---|
comment:4 follow-up: 5 Changed 13 years ago by
Just copying more explanation from a ticket I opened 2 weeks ago and which was closed as a duplicate of this one. The conflict is actually pointed out but incorrectly silently resolved.
Upload attached file, you will get a conflict that the deleted node still exists in the DB. Choose "Prepare conflict resolution" The resolution window comes up empty Try to upload again (DO NOT actually do it)
JOSM simply deletes the node from the two ways it was in, effectively destroying data without letting the user now.
comment:5 Changed 13 years ago by
Replying to Nakor:
Just copying more explanation from a ticket I opened 2 weeks ago and which was closed as a duplicate of this one. The conflict is actually pointed out but incorrectly silently resolved.
The conflict is pointed out by the API - not by JOSM.
Upload attached file, you will get a conflict that the deleted node still exists in the DB. Choose "Prepare conflict resolution" The resolution window comes up empty Try to upload again (DO NOT actually do it)
JOSM simply deletes the node from the two ways it was in, effectively destroying data without letting the user now.
And this is the issue described in this ticket.
The way it works is this:
- delete a node that is used by a way JOSM doesn't know about
- upload => API reports a conflict => JOSM offers to resolve that
- to do this JOSM loads the way from API which contains the now deleted node
- because your deleted node and the node downloaded from the API have the same version JOSM does not recognize this as a conflict and keeps the deleted node.
- and, because the node is deleted it is removed from the way
In short: JOSM never had a conflict in its data - it only has reported to you that the API has detected a conflict. That's why this is not a matter of conflict resolving but one of conflict detecting.
comment:6 Changed 13 years ago by
Sorry about that. The message in JOSM is confusing then because it says it is going to prepare a conflict resolution.
comment:7 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 Changed 13 years ago by
comment:10 Changed 13 years ago by
mjulius, please check r2939. I'm not sure if it's related to this ticket, but it's reverting your code and I couldn't find your private e-mail.
comment:11 Changed 13 years ago by
Thanks for actually doing that. This was a thinking error on my part.
comment:12 follow-up: 13 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
I tried, but it wasn't fixed:
- delete some nodes that are still used by a way, but this way is not known to JOSM
- upload -> "Upload failed because you tried to delete node ... which is still in use in way ..."
- click "Prepare conflict resolution"
- -> nothing, no conflicts displayed
- Ctrl-a -> error from the measurement plugin (1)
- when trying to upload again, the same api error comes. In earlier versions, JOSM downloaded the conflicting way. And the issue was gone. (For this single node.)
(Maybe connected to the recent API-reimplementation? (2))
(1)
Build-Date: 2010-02-07 18:56:33 Revision: 2948 Is-Local-Build: true Memory Usage: 63 MB / 297 MB (22 MB allocated, but free) Java version: 1.6.0_15, Sun Microsystems Inc., Linux Dataset consistency test: [DELETED REFERENCED] {Way id=49794163 version=1 VT nodes=[{Node id=632652219 version=0 IV }, {Node id=632652215 version=0 IV }, {Node id=632652216 version=1 MVD lat=54.0969967,lon=13.3507683}, {Node id=632652211 version=0 IV }, {Node id=632652212 version=0 IV }]} refers to deleted primitive {Node id=632652216 version=1 MVD lat=54.0969967,lon=13.3507683} Plugins: measurement,osmarender,photo_geotagging,remotecontrol,tageditor,tagging-preset-tester,terracer,validator,wmsplugin Plugin wmsplugin Version: 19626 Plugin tageditor Version: 19587 Plugin remotecontrol Version: 19471 Plugin validator Version: 19688 Plugin osmarender Version: 19419 Plugin tagging-preset-tester Version: 19481 Plugin terracer Version: 19678 Plugin photo_geotagging Version: 19672 Plugin measurement Version: 19681 java.lang.NullPointerException at org.openstreetmap.josm.plugins.measurement.MeasurementLayer.calcDistance(MeasurementLayer.java:151) at org.openstreetmap.josm.plugins.measurement.MeasurementDialog$1.selectionChanged(MeasurementDialog.java:130) at org.openstreetmap.josm.data.osm.DataSet.fireSelectionChanged(DataSet.java:285) at org.openstreetmap.josm.data.osm.DataSet.addSelected(DataSet.java:445) at org.openstreetmap.josm.data.osm.DataSet.setSelected(DataSet.java:390) at org.openstreetmap.josm.data.osm.DataSet.setSelected(DataSet.java:403) at org.openstreetmap.josm.actions.SelectAllAction.actionPerformed(SelectAllAction.java:23) at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1636) at javax.swing.JComponent.processKeyBinding(JComponent.java:2851) at javax.swing.KeyboardManager.fireBinding(KeyboardManager.java:267) at javax.swing.KeyboardManager.fireKeyboardAction(KeyboardManager.java:216) at javax.swing.JComponent.processKeyBindingsForAllComponents(JComponent.java:2928) at javax.swing.JComponent.processKeyBindings(JComponent.java:2920) at javax.swing.JComponent.processKeyEvent(JComponent.java:2814) at java.awt.Component.processEvent(Component.java:6040) at java.awt.Container.processEvent(Container.java:2041) at java.awt.Component.dispatchEventImpl(Component.java:4630) at java.awt.Container.dispatchEventImpl(Container.java:2099) at java.awt.Component.dispatchEvent(Component.java:4460) at java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1848) at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:704) at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:969) at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:841) at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:668) at java.awt.Component.dispatchEventImpl(Component.java:4502) at java.awt.Container.dispatchEventImpl(Container.java:2099) at java.awt.Window.dispatchEventImpl(Window.java:2475) at java.awt.Component.dispatchEvent(Component.java:4460) at java.awt.EventQueue.dispatchEvent(EventQueue.java:599) at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269) at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184) at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174) at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169) at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161) at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)
(2)
http://lists.openstreetmap.org/pipermail/talk-de/2010-February/062703.html
comment:13 Changed 13 years ago by
Replying to bastiK:
- when trying to upload again, the same api error comes. In earlier versions, JOSM downloaded the conflicting way. And the issue was gone. (For this single node.)
That's because earlier it silently deleted the node from the other way as well, when doing that conflict 'resolution' thing. I argued earlier (in #3761) that this is not always the desired behaviour.
As both options seem to be equally valid, but applicable to different circumstances, to me it seems that JOSM should ask
"The node {node} you deleted is still present in way {way} on the server. [Delete from other way as well] [Leave in other way] [Download way and let me handle this]".
Where in the latter case ('let me handle this') it enters a conflict in the conflict list.
comment:14 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
This is a different issue.
The problem here is that when downloading referrers the API only returns the referrers themselves and not the other primitives the referrers refer to. In your example, JOSM downloads the ways that are referring to the deleted node, but not their other nodes. That's why JOSM does not create a conflict and why it cannot display the way. And if you select such a way (by 'Select all' for example) the Measurement plugin throws a NPE when it encounters a way with incomplete nodes.
There need to be separate tickets created for these issues.
comment:15 Changed 13 years ago by
comment:17 follow-up: 19 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
This is still not perfect. When my dataset contains deleted node that is referenced by way in their dataset, the result is merged dataset with way that reference deleted node. This produce incorrect rendering and might cause problems in other parts of JOSM.
This get fixed after conflict resolution, but user is not forced to do conflict resolution immediately, so for some time JOSM works with inconsistent dataset.
Simple solution would be setting all referenced nodes or members with conflict in my dataset to non deleted state.
comment:18 Changed 13 years ago by
Keywords: | r-2010-01-blocker added |
---|
comment:19 Changed 13 years ago by
Replying to jttt:
This is still not perfect. When my dataset contains deleted node that is referenced by way in their dataset, the result is merged dataset with way that reference deleted node. This produce incorrect rendering and might cause problems in other parts of JOSM.
This get fixed after conflict resolution, but user is not forced to do conflict resolution immediately, so for some time JOSM works with inconsistent dataset.
Maybe we should prevent any data modification when there is a conflict in the dataset.
Simple solution would be setting all referenced nodes or members with conflict in my dataset to non deleted state.
and not produce a conflict at all and simply merge the non-deleted primitive from theirs if it is referenced? Otherwise we would need to remember somehow that the primitive was deleted.
We could also introduce another flag into OsmPrimitive to mark primitives with conflicts. The renderer can then display those and they can be made selectable. This would also allow the renderer to easily highlight all primitives with conflicts.
That might be the best way to handle conflicts in deleted state.
comment:20 follow-up: 21 Changed 13 years ago by
We could also introduce another flag into OsmPrimitive to mark primitives with conflicts.
I think we don't have to introduce a flag.
Each OsmLayer maintains a ConflictCollection. We can ask it whether an OsmPrimitive participates in a conflict. Rendering performance could be an issue, though.
comment:21 Changed 13 years ago by
Replying to anonymous:
We could also introduce another flag into OsmPrimitive to mark primitives with conflicts.
I think we don't have to introduce a flag.
Each OsmLayer maintains a ConflictCollection. We can ask it whether an OsmPrimitive participates in a conflict. Rendering performance could be an issue, though.
Exactly. It might be a bet less so if the renderer first builds a HashSet of OsmPrimitives with conflicts. But, looking up a flag is probably still faster.
A performance penalty would be less significant if we only do that for deleted primitives.
Of course, all this only needs to be done when there are any conflicts. Therefore, this would not effect normal operation. It might serve as an incentive to resolve conflicts early. ;-)
comment:22 follow-up: 24 Changed 13 years ago by
Replying to mjulius:
Simple solution would be setting all referenced nodes or members with conflict in my dataset to non deleted state.
and not produce a conflict at all and simply merge the non-deleted primitive from theirs if it is referenced? Otherwise we would need to remember somehow that the primitive was deleted.
No, there will still be a conflict. Special type of conflict, that will contain reference to primitive and information, that primitive is in fact deleted.
The renderer can then display those and they can be made selectable.
This problem isn't only about rendering, also other parts of JOSM shouldn't be forced to test for deleted nodes/members.
On the other hand, special rendering of primitives in conflict will be useful.
comment:23 Changed 13 years ago by
Cc: | skyper added |
---|
comment:24 Changed 13 years ago by
Replying to jttt:
On the other hand, special rendering of primitives in conflict will be useful.
Really cool would be if JOSM somehow rendered both versions of a conflicting primitive when there was a change in location.
comment:25 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Ticket #4403 has been marked as a duplicate of this ticket.