Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#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?

  1. open attached note.osm
  2. select node with note-tag and delete this tag
  3. download an area which includes the above node
  4. 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)

note.osm (62.1 KB) - added by mdk 4 years ago.

Download all attachments as: .zip

Change History (14)

Changed 4 years ago by mdk

Attachment: note.osm added

comment:1 Changed 4 years ago by mdk

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 4 years ago by Don-vip

Milestone: 16.03

comment:3 Changed 4 years ago by Don-vip

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"

comment:4 Changed 4 years ago by Don-vip

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 4 years ago by Don-vip

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 4 years ago by Don-vip

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 4 years ago by Don-vip

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 4 years ago by simon04

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 
    9494                if (!target.isNew() || target.isDeleted()) {
    9595                    continue;
    9696                }
    97                 if (target.hasEqualSemanticAttributes(source)) {
     97                if (target.hasEqualSemanticAttributes(source) && target.getKeys().equals(source.getKeys())) {
    9898                    mergedMap.put(source.getPrimitiveId(), target.getPrimitiveId());
    9999                    // copy the technical attributes from other version
    100100                    target.setVisible(source.isVisible());
    else if (target.isDeleted() && !source.isDeleted() && target.getVersion() == sou 
    356356        } else if (target.isModified() && !source.isModified() && target.getVersion() == source.getVersion()) {
    357357            // target is same as source but target is modified
    358358            // => 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())) {
    360360                target.setModified(false);
    361361            }
    362362        } else if (source.isDeleted() != target.isDeleted()) {

comment:9 Changed 4 years ago by simon04

Resolution: fixed
Status: newclosed

In 9961/josm:

fix #12599 - Added/deleted "uninteresting" tags are "reverted" by downloading area again (unit test by Don-vip)

comment:10 Changed 4 years ago by Don-vip

Thanks! :)

comment:11 Changed 4 years ago by Don-vip

In 9979/josm:

see #12599, fix #12616 - Random repositioning of nodes (incomplete fix from r9961)

comment:12 Changed 4 years ago by simon04

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:13 Changed 4 years ago by Don-vip

Milestone: 16.0316.04

Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.