#12599 closed defect (fixed)
Added/deleted "uninteresting" tags are "reverted" by downloading area again (twice)
Reported by: | mdk | Owned by: | team |
---|---|---|---|
Priority: | major | Milestone: | 16.04 |
Component: | Core | Version: | latest |
Keywords: | template_report download uninteresting | Cc: | simon04 |
Description
What steps will reproduce the problem?
- open attached note.osm
- select node with note-tag and delete this tag
- download an area which includes the above node
- download a second area which also includes the above node (can be the same as in 3)
What is the expected result?
The node stays unchanged (note tag stays deleted).
What happens instead?
After the second download, the node get the deleted note tag back!
Please provide any additional information below. Attach a screenshot if possible.
Here are some hopefully helpful observations:
- If you move the node before downloading, the tag is not "restored".
- If you delete tags like natural=tree or aminity=toilett from a single node, the tag is not "restored".
- If you delete a highway=turning_cycle tag from (the end of) a way, the tag is not "restored".
So far I observe this only on note=* tags in the middle of waterway=stream ways. But I'm not shure if there are other scenarios where this happens.
It's independebt of the size of the downloaded areas used for step 3 and 4. Only the node must be inside the bounding box in both downloads.
URL:http://josm.openstreetmap.de/svn/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2016-03-04 21:52:59 +0100 (Fri, 04 Mar 2016) Build-Date:2016-03-04 22:51:10 Revision:9924 Relative:URL: ^/trunk Identification: JOSM/1.5 (9924 en) Linux Ubuntu 15.10 Memory Usage: 599 MB / 876 MB (189 MB allocated, but free) Java version: 1.8.0_66-internal-b17, Oracle Corporation, OpenJDK Server VM VM arguments: [-Djosm.restart=true, -Djosm.home=/home/michael/.josm-latest, -Djava.net.useSystemProxies=true] Plugins: - ColumbusCSV (31772) - FastDraw (31895) - HouseNumberTaggingTool (31772) - Mapillary (32040) - OpeningHoursEditor (31772) - RoadSigns (31895) - SimplifyArea (31895) - apache-commons (31895) - apache-http (31895) - buildings_tools (31895) - contourmerge (1014) - imagery-xml-bounds (31772) - imagery_offset_db (32046) - pbf (31772) - poly (31772) - public_transport (31895) - reltoolbox (31895) - reverter (32005) - terracer (31895) - turnrestrictions (31895) - utilsplugin2 (32097) Last errors/warnings: - W: java.net.SocketTimeoutException: connect timed out - E: java.net.SocketTimeoutException: connect timed out - W: java.net.SocketTimeoutException: connect timed out - E: java.net.SocketTimeoutException: connect timed out - E: org.openstreetmap.josm.io.OsmTransferException: Could not connect to the OSM server. Please check your internet connection.. Cause: java.net.SocketTimeoutException: connect timed out
Attachments (1)
Change History (15)
Changed 7 years ago by
comment:1 Changed 7 years ago by
Summary: | Deleted (note?) tags are "reverted" by by downloading area again (twice) → Deleted (note?) tags are "reverted" by downloading area again (twice) |
---|
comment:2 Changed 7 years ago by
Milestone: | → 16.03 |
---|
comment:3 Changed 7 years ago by
comment:4 Changed 7 years ago by
Keywords: | download uninteresting added |
---|---|
Summary: | Deleted (note?) tags are "reverted" by downloading area again (twice) → Added/deleted "uninteresting" tags are "reverted" by downloading area again (twice) |
comment:5 Changed 7 years ago by
Looks like DataSetMerger
fails to merge properly primitives having only uninteresting tags. Probably a bug in mergeById
or mergePrimitive
function (the two methods calling hasEqualSemanticAttributes
method which itself calls hasSameInterestingTags
)
comment:6 Changed 7 years ago by
This test reproduces the bug:
/** * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/12599">Bug #12599</a>. */ @Test public void testTicket12599() { // Server node: no modifications Node n1 = new Node(1, 1); n1.setCoor(LatLon.ZERO); assertFalse(n1.isModified()); their.addPrimitive(n1); // Local node: one modification: addition of an uninteresting tag Node n1b = new Node(n1); n1b.setModified(true); n1b.put("note", "something"); assertTrue(n1b.isModified()); assertEquals(0, n1b.getInterestingTags().size()); my.addPrimitive(n1b); // Merge DataSetMerger visitor = new DataSetMerger(my, their); visitor.merge(); // Check that modification is still here Node n = (Node) my.getPrimitiveById(1, OsmPrimitiveType.NODE); assertNotNull(n); assertEquals("something", n.get("note")); assertTrue(n.isModified()); // Merge again visitor = new DataSetMerger(my, their); visitor.merge(); // Check that modification is still here n = (Node) my.getPrimitiveById(1, OsmPrimitiveType.NODE); assertNotNull(n); assertEquals("something", n.get("note")); assertTrue(n.isModified()); }
comment:7 Changed 7 years ago by
Cc: | simon04 added |
---|
Problem comes from:
} else if (target.isModified() && !source.isModified() && target.getVersion() == source.getVersion()) { // target is same as source but target is modified // => keep target and reset modified flag if target and source are semantically equal if (target.hasEqualSemanticAttributes(source)) { target.setModified(false); } }
This comes from r2634 in order to fix #4088. The fix was correct because at that time, hasEqualSemanticAttributes
called hasSameTags
. Now it calls hasSameInterestingTags
since r6629, I think the bug comes from here.
comment:8 Changed 7 years ago by
What about…?
-
src/org/openstreetmap/josm/data/osm/DataSetMerger.java
diff --git a/src/org/openstreetmap/josm/data/osm/DataSetMerger.java b/src/org/openstreetmap/josm/data/osm/DataSetMerger.java index f958cf1..b8fb466 100644
a b protected void mergePrimitive(OsmPrimitive source, Collection<? extends OsmPrimi 94 94 if (!target.isNew() || target.isDeleted()) { 95 95 continue; 96 96 } 97 if (target.hasEqualSemanticAttributes(source) ) {97 if (target.hasEqualSemanticAttributes(source) && target.getKeys().equals(source.getKeys())) { 98 98 mergedMap.put(source.getPrimitiveId(), target.getPrimitiveId()); 99 99 // copy the technical attributes from other version 100 100 target.setVisible(source.isVisible()); … … else if (target.isDeleted() && !source.isDeleted() && target.getVersion() == sou 356 356 } else if (target.isModified() && !source.isModified() && target.getVersion() == source.getVersion()) { 357 357 // target is same as source but target is modified 358 358 // => keep target and reset modified flag if target and source are semantically equal 359 if (target.hasEqualSemanticAttributes(source) ) {359 if (target.hasEqualSemanticAttributes(source) && target.getKeys().equals(source.getKeys())) { 360 360 target.setModified(false); 361 361 } 362 362 } else if (source.isDeleted() != target.isDeleted()) {
comment:12 Changed 7 years ago by
In the last minute I moved from the suggested change from comment:8 for a version w/ better performance w/o the Map
creation hasSameInterestingTags
, but overlooked that subclasses override the method. Sorry.
comment:14 Changed 2 years ago by
The changes in r6629 caused a lot more similar problems. See #20313. Also the reverter plugin expects that method hasEqualSemanticAttributes()
compares all tags (and the javadoc says it does)
I don't understand why it was needed to change this logic in hasEqualSemanticAttributes()
for the implementation of the :sametags. To me this looks like an unintended change.
I can reproduce with
description
. Looks like a problem concerning all "uninteresting" keys (see OsmPrimitive.getUninterestingKeys):"source", "source_ref", "source:", "comment", "converted_by", "watch", "watch:", "description", "attribution", "note", "fixme", "FIXME"