Changeset 18007 in josm

Timestamp:

2021-07-12T16:25:33+02:00 (4 years ago)

Author:

Don-vip

Message:

fix #21044 - keep all tags when merging geojson nodes at the same location

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolverModel.java (modified) (2 diffs)
• src/org/openstreetmap/josm/io/GeoJSONReader.java (modified) (8 diffs)
• test/data/regress/21044 (added)
• test/data/regress/21044/test.geojson (added)
• test/unit/org/openstreetmap/josm/io/GeoJSONReaderTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolverModel.java

@@ -10 +10 @@
 import java.util.Map;
 import java.util.Set;
+import java.util.function.BiConsumer;
 
 import javax.swing.table.DefaultTableModel;
@@ -299 +300 @@
         return new HashSet<>(keysWithConflicts);
     }
+
+    /**
+     * Perform an action on all decisions, useful to perform a global decision (keep all, keep none, etc.)
+     * @param action action to perform on decision
+     * @since 18007
+     */
+    public final void actOnDecisions(BiConsumer<String, MultiValueResolutionDecision> action) {
+        decisions.forEach(action::accept);
+    }
 }

trunk/src/org/openstreetmap/josm/io/GeoJSONReader.java

@@ -12 +12 @@
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.TreeMap;
 import java.util.stream.Collectors;
 
@@ -37 +37 @@
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
+import org.openstreetmap.josm.data.osm.Tag;
+import org.openstreetmap.josm.data.osm.TagCollection;
+import org.openstreetmap.josm.data.osm.TagMap;
 import org.openstreetmap.josm.data.osm.UploadPolicy;
 import org.openstreetmap.josm.data.osm.Way;
@@ -43 +46 @@
 import org.openstreetmap.josm.data.validation.TestError;
 import org.openstreetmap.josm.data.validation.tests.DuplicateWay;
+import org.openstreetmap.josm.gui.conflict.tags.TagConflictResolutionUtil;
+import org.openstreetmap.josm.gui.conflict.tags.TagConflictResolverModel;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
@@ -329 +334 @@
 
     /**
-     * Replace all existing tags in primitive by the values given in the GeoJSON feature.
+     * Merge existing tags in primitive (if any) with the values given in the GeoJSON feature.
      * @param feature the GeoJSON feature
      * @param primitive the OSM primitive
@@ -335 +340 @@
     private static void fillTagsFromFeature(final JsonObject feature, final OsmPrimitive primitive) {
         if (feature != null) {
-            primitive.setKeys(getTags(feature));
-        }
+            TagCollection featureTags = getTags(feature);
+            primitive.setKeys(new TagMap(primitive.isTagged() ? mergeAllTagValues(primitive, featureTags) : featureTags));
+        }
+    }
+
+    private static TagCollection mergeAllTagValues(final OsmPrimitive primitive, TagCollection featureTags) {
+        TagCollection tags = TagCollection.from(primitive).union(featureTags);
+        TagConflictResolutionUtil.applyAutomaticTagConflictResolution(tags);
+        TagConflictResolutionUtil.normalizeTagCollectionBeforeEditing(tags, Arrays.asList(primitive));
+        TagConflictResolverModel tagModel = new TagConflictResolverModel();
+        tagModel.populate(new TagCollection(tags), tags.getKeysWithMultipleValues());
+        tagModel.actOnDecisions((k, d) -> d.keepAll());
+        return tagModel.getAllResolutions();
     }
 
@@ -347 +363 @@
     }
 
-    private static Map<String, String> getTags(final JsonObject feature) {
-        final Map<String, String> tags = new TreeMap<>();
+    private static TagCollection getTags(final JsonObject feature) {
+        final TagCollection tags = new TagCollection();
 
         if (feature.containsKey(PROPERTIES) && !feature.isNull(PROPERTIES)) {
@@ -357 +373 @@
 
                     if (value instanceof JsonString) {
-                        tags.put(stringJsonValueEntry.getKey(), ((JsonString) value).getString());
+                        tags.add(new Tag(stringJsonValueEntry.getKey(), ((JsonString) value).getString()));
                     } else if (value instanceof JsonObject) {
                         Logging.warn(
@@ -365 +381 @@
                         );
                     } else if (value.getValueType() != JsonValue.ValueType.NULL) {
-                        tags.put(stringJsonValueEntry.getKey(), value.toString());
+                        tags.add(new Tag(stringJsonValueEntry.getKey(), value.toString()));
                     }
                 }

trunk/test/unit/org/openstreetmap/josm/io/GeoJSONReaderTest.java

@@ -217 +217 @@
 
     /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/21044">Bug #21044</a>.
+     * @throws Exception in case of error
+     */
+    @Test
+    void testTicket21044Duplicates() throws Exception {
+        try (InputStream in = TestUtils.getRegressionDataStream(21044, "test.geojson")) {
+            final List<OsmPrimitive> primitives = new ArrayList<>(
+                    new GeoJSONReader().doParseDataSet(in, null).getPrimitives(it -> true));
+            assertEquals(1, primitives.size());
+            OsmPrimitive primitive = primitives.get(0);
+            assertTrue(primitive instanceof Node);
+            Node n = (Node) primitive;
+            assertNull(n.get("addr:building"));
+            assertEquals("06883", n.get("addr:postcode"));
+            assertEquals("22;26", n.get("addr:housenumber"));
+        }
+    }
+
+    /**
      * Tests error reporting for an invalid FeatureCollection
      * @throws Exception in case of error
@@ -229 +248 @@
         }
     }
-
 }