Changeset 10737 in josm

Timestamp:

2016-08-05T20:09:55+02:00 (8 years ago)

Author:

Don-vip

Message:

fix #12900 - Conflicts in pasted tags cannot be resolved, cannot be resolved to "none" (patch by michael2402) - gsoc-core

Location:

trunk

Files:

• src/org/openstreetmap/josm/actions/PasteTagsAction.java (modified) (3 diffs)
• src/org/openstreetmap/josm/data/osm/Tag.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/conflict/tags/PasteTagsConflictResolverDialog.java (modified) (3 diffs)
• src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolverModel.java (modified) (4 diffs)
• src/org/openstreetmap/josm/gui/datatransfer/OsmTransferHandler.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/datatransfer/PrimitiveTransferable.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/datatransfer/data/PrimitiveTagTransferData.java (added)
• src/org/openstreetmap/josm/gui/datatransfer/data/TagTransferData.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/datatransfer/importers/AbstractTagPaster.java (modified) (4 diffs)
• src/org/openstreetmap/josm/gui/datatransfer/importers/PrimitiveTagTransferPaster.java (added)
• src/org/openstreetmap/josm/gui/datatransfer/importers/TextTagPaster.java (modified) (4 diffs)
• test/unit/org/openstreetmap/josm/actions/PasteTagsActionTest.java (deleted)
• test/unit/org/openstreetmap/josm/gui/datatransfer/data (added)
• test/unit/org/openstreetmap/josm/gui/datatransfer/data/PrimitiveTagTransferDataTest.java (added)

trunk/src/org/openstreetmap/josm/actions/PasteTagsAction.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
-import static org.openstreetmap.josm.tools.I18n.trn;
 
 import java.awt.event.ActionEvent;
 import java.awt.event.KeyEvent;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.EnumMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
 
-import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.command.ChangePropertyCommand;
-import org.openstreetmap.josm.command.Command;
-import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
-import org.openstreetmap.josm.data.osm.PrimitiveData;
-import org.openstreetmap.josm.data.osm.Tag;
-import org.openstreetmap.josm.data.osm.TagCollection;
-import org.openstreetmap.josm.gui.conflict.tags.PasteTagsConflictResolverDialog;
 import org.openstreetmap.josm.gui.datatransfer.OsmTransferHandler;
-import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.Shortcut;
-import org.openstreetmap.josm.tools.TextTagParser;
 
 /**
@@ -55 +38 @@
     }
 
-    /**
-     * Used to update the tags.
-     */
-    public static class TagPaster {
-
-        private final Collection<PrimitiveData> source;
-        private final Collection<OsmPrimitive> target;
-        private final List<Tag> tags = new ArrayList<>();
-
-        /**
-         * Constructs a new {@code TagPaster}.
-         * @param source source primitives
-         * @param target target primitives
-         */
-        public TagPaster(Collection<PrimitiveData> source, Collection<OsmPrimitive> target) {
-            this.source = source;
-            this.target = target;
-        }
-
-        /**
-         * Determines if the source for tag pasting is heterogeneous, i.e. if it doesn't consist of
-         * {@link OsmPrimitive}s of exactly one type
-         * @return true if the source for tag pasting is heterogeneous
-         */
-        protected boolean isHeterogeneousSource() {
-            int count = 0;
-            count = !getSourcePrimitivesByType(OsmPrimitiveType.NODE).isEmpty() ? (count + 1) : count;
-            count = !getSourcePrimitivesByType(OsmPrimitiveType.WAY).isEmpty() ? (count + 1) : count;
-            count = !getSourcePrimitivesByType(OsmPrimitiveType.RELATION).isEmpty() ? (count + 1) : count;
-            return count > 1;
-        }
-
-        /**
-         * Replies all primitives of type <code>type</code> in the current selection.
-         *
-         * @param type  the type
-         * @return all primitives of type <code>type</code> in the current selection.
-         */
-        protected Collection<? extends PrimitiveData> getSourcePrimitivesByType(OsmPrimitiveType type) {
-            return PrimitiveData.getFilteredList(source, type);
-        }
-
-        /**
-         * Replies the collection of tags for all primitives of type <code>type</code> in the current
-         * selection
-         *
-         * @param type  the type
-         * @return the collection of tags for all primitives of type <code>type</code> in the current
-         * selection
-         */
-        protected TagCollection getSourceTagsByType(OsmPrimitiveType type) {
-            return TagCollection.unionOfAllPrimitives(getSourcePrimitivesByType(type));
-        }
-
-        /**
-         * Replies true if there is at least one tag in the current selection for primitives of
-         * type <code>type</code>
-         *
-         * @param type the type
-         * @return true if there is at least one tag in the current selection for primitives of
-         * type <code>type</code>
-         */
-        protected boolean hasSourceTagsByType(OsmPrimitiveType type) {
-            return !getSourceTagsByType(type).isEmpty();
-        }
-
-        protected void buildTags(TagCollection tc) {
-            for (String key : tc.getKeys()) {
-                tags.add(new Tag(key, tc.getValues(key).iterator().next()));
-            }
-        }
-
-        protected Map<OsmPrimitiveType, Integer> getSourceStatistics() {
-            Map<OsmPrimitiveType, Integer> ret = new EnumMap<>(OsmPrimitiveType.class);
-            for (OsmPrimitiveType type: OsmPrimitiveType.dataValues()) {
-                if (!getSourceTagsByType(type).isEmpty()) {
-                    ret.put(type, getSourcePrimitivesByType(type).size());
-                }
-            }
-            return ret;
-        }
-
-        protected Map<OsmPrimitiveType, Integer> getTargetStatistics() {
-            Map<OsmPrimitiveType, Integer> ret = new EnumMap<>(OsmPrimitiveType.class);
-            for (OsmPrimitiveType type: OsmPrimitiveType.dataValues()) {
-                int count = OsmPrimitive.getFilteredList(target, type.getOsmClass()).size();
-                if (count > 0) {
-                    ret.put(type, count);
-                }
-            }
-            return ret;
-        }
-
-        /**
-         * Pastes the tags from a homogeneous source (the selection consisting
-         * of one type of {@link OsmPrimitive}s only).
-         *
-         * Tags from a homogeneous source can be pasted to a heterogeneous target. All target primitives,
-         * regardless of their type, receive the same tags.
-         */
-        protected void pasteFromHomogeneousSource() {
-            TagCollection tc = null;
-            for (OsmPrimitiveType type : OsmPrimitiveType.dataValues()) {
-                TagCollection tc1 = getSourceTagsByType(type);
-                if (!tc1.isEmpty()) {
-                    tc = tc1;
-                }
-            }
-            if (tc == null)
-                // no tags found to paste. Abort.
-                return;
-
-            if (!tc.isApplicableToPrimitive()) {
-                PasteTagsConflictResolverDialog dialog = new PasteTagsConflictResolverDialog(Main.parent);
-                dialog.populate(tc, getSourceStatistics(), getTargetStatistics());
-                dialog.setVisible(true);
-                if (dialog.isCanceled())
-                    return;
-                buildTags(dialog.getResolution());
-            } else {
-                // no conflicts in the source tags to resolve. Just apply the tags to the target primitives
-                buildTags(tc);
-            }
-        }
-
-        /**
-         * Replies true if there is at least one primitive of type <code>type</code>
-         * is in the target collection
-         *
-         * @param type  the type to look for
-         * @return true if there is at least one primitive of type <code>type</code> in the collection
-         * <code>selection</code>
-         */
-        protected boolean hasTargetPrimitives(Class<? extends OsmPrimitive> type) {
-            return !OsmPrimitive.getFilteredList(target, type).isEmpty();
-        }
-
-        /**
-         * Replies true if this a heterogeneous source can be pasted without conflict to targets
-         *
-         * @return true if this a heterogeneous source can be pasted without conflicts to targets
-         */
-        protected boolean canPasteFromHeterogeneousSourceWithoutConflict() {
-            for (OsmPrimitiveType type : OsmPrimitiveType.dataValues()) {
-                if (hasTargetPrimitives(type.getOsmClass())) {
-                    TagCollection tc = TagCollection.unionOfAllPrimitives(getSourcePrimitivesByType(type));
-                    if (!tc.isEmpty() && !tc.isApplicableToPrimitive())
-                        return false;
-                }
-            }
-            return true;
-        }
-
-        /**
-         * Pastes the tags in the current selection of the paste buffer to a set of target primitives.
-         */
-        protected void pasteFromHeterogeneousSource() {
-            if (canPasteFromHeterogeneousSourceWithoutConflict()) {
-                for (OsmPrimitiveType type : OsmPrimitiveType.dataValues()) {
-                    if (hasSourceTagsByType(type) && hasTargetPrimitives(type.getOsmClass())) {
-                        buildTags(getSourceTagsByType(type));
-                    }
-                }
-            } else {
-                PasteTagsConflictResolverDialog dialog = new PasteTagsConflictResolverDialog(Main.parent);
-                dialog.populate(
-                        getSourceTagsByType(OsmPrimitiveType.NODE),
-                        getSourceTagsByType(OsmPrimitiveType.WAY),
-                        getSourceTagsByType(OsmPrimitiveType.RELATION),
-                        getSourceStatistics(),
-                        getTargetStatistics()
-                );
-                dialog.setVisible(true);
-                if (dialog.isCanceled())
-                    return;
-                for (OsmPrimitiveType type : OsmPrimitiveType.dataValues()) {
-                    if (hasSourceTagsByType(type) && hasTargetPrimitives(type.getOsmClass())) {
-                        buildTags(dialog.getResolution(type));
-                    }
-                }
-            }
-        }
-
-        /**
-         * Performs the paste operation.
-         * @return list of tags
-         */
-        public List<Tag> execute() {
-            tags.clear();
-            if (isHeterogeneousSource()) {
-                pasteFromHeterogeneousSource();
-            } else {
-                pasteFromHomogeneousSource();
-            }
-            return tags;
-        }
-
-    }
-
     @Override
     public void actionPerformed(ActionEvent e) {
@@ -262 +46 @@
 
         transferHandler.pasteTags(selection);
-    }
-
-    /**
-     * Paste tags from arbitrary text, not using JOSM buffer
-     * @param selection selected primitives
-     * @param text text containing tags
-     * @return true if action was successful
-     * @see TextTagParser#readTagsFromText
-     */
-    public static boolean pasteTagsFromText(Collection<OsmPrimitive> selection, String text) {
-        Map<String, String> tags = TextTagParser.readTagsFromText(text);
-        if (tags == null || tags.isEmpty()) {
-            TextTagParser.showBadBufferMessage(help);
-            return false;
-        }
-        if (!TextTagParser.validateTags(tags)) return false;
-
-        List<Command> commands = new ArrayList<>(tags.size());
-        for (Entry<String, String> entry: tags.entrySet()) {
-            String v = entry.getValue();
-            commands.add(new ChangePropertyCommand(selection, entry.getKey(), "".equals(v) ? null : v));
-        }
-        commitCommands(selection, commands);
-        return !commands.isEmpty();
-    }
-
-    /**
-     * Create and execute SequenceCommand with descriptive title
-     * @param selection selected primitives
-     * @param commands the commands to perform in a sequential command
-     */
-    private static void commitCommands(Collection<OsmPrimitive> selection, List<Command> commands) {
-        if (!commands.isEmpty()) {
-            String title1 = trn("Pasting {0} tag", "Pasting {0} tags", commands.size(), commands.size());
-            String title2 = trn("to {0} object", "to {0} objects", selection.size(), selection.size());
-            @I18n.QuirkyPluralString
-            final String title = title1 + ' ' + title2;
-            Main.main.undoRedo.add(
-                    new SequenceCommand(
-                            title,
-                            commands
-                    ));
-        }
     }
 

trunk/src/org/openstreetmap/josm/data/osm/Tag.java

@@ -2 +2 @@
 package org.openstreetmap.josm.data.osm;
 
+import java.io.Serializable;
 import java.util.Collection;
 import java.util.Collections;
@@ -17 +18 @@
  * the modifying methods throw an {@link UnsupportedOperationException}.
  */
-public class Tag implements Tagged, Entry<String, String> {
+public class Tag implements Tagged, Entry<String, String>, Serializable {
+
+    private static final long serialVersionUID = 1;
 
     private final String key;

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

@@ -290 +290 @@
         }
 
-        protected void updateEnabledState() {
+        void updateEnabledState() {
             if (mode == null) {
                 setEnabled(false);
@@ -296 +296 @@
                 setEnabled(allPrimitivesResolver.getModel().isResolvedCompletely());
             } else {
-                boolean enabled = true;
-                for (TagConflictResolver val: resolvers.values()) {
-                    enabled &= val.getModel().isResolvedCompletely();
-                }
-                setEnabled(enabled);
+                setEnabled(resolvers.values().stream().allMatch(val -> val.getModel().isResolvedCompletely()));
             }
         }
@@ -346 +342 @@
                 if (model == resolver.getModel()) {
                     tpResolvers.setIconAt(i,
-                            (Boolean) evt.getNewValue() ? iconResolved : iconUnresolved
-
+                            (Integer) evt.getNewValue() == 0 ? iconResolved : iconUnresolved
                     );
                 }

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

@@ -22 +22 @@
     private transient TagCollection tags;
     private List<String> displayedKeys;
-    private Set<String> keysWithConflicts;
+    private Set<String> keysWithConflicts = new HashSet<>();
     private transient Map<String, MultiValueResolutionDecision> decisions;
     private int numConflicts;
@@ -54 +54 @@
 
     protected void refreshNumConflicts() {
-        int count = 0;
-        for (MultiValueResolutionDecision d : decisions.values()) {
-            if (!d.isDecided()) {
-                count++;
-            }
-        }
-        setNumConflicts(count);
+        setNumConflicts((int) decisions.values().stream().filter(d -> !d.isDecided()).count());
     }
 
@@ -122 +116 @@
         this.tags = tags;
         displayedKeys = new ArrayList<>();
-        this.keysWithConflicts = keysWithConflicts == null ? new HashSet<>() : keysWithConflicts;
+        if (keysWithConflicts != null) {
+            this.keysWithConflicts.addAll(keysWithConflicts);
+        }
         decisions = new HashMap<>();
         rebuild();
@@ -183 +179 @@
      */
     public boolean isResolvedCompletely() {
-        return numConflicts == 0 && keysWithConflicts != null && keysWithConflicts.isEmpty();
-    }
-
+        return numConflicts == 0;
+    }
+
+    /**
+     * Gets the number of reamining conflicts.
+     * @return The number
+     */
     public int getNumConflicts() {
         return numConflicts;
     }
 
+    /**
+     * Gets the number of decisions the user can take
+     * @return The number of decisions
+     */
     public int getNumDecisions() {
         return decisions == null ? 0 : decisions.size();

trunk/src/org/openstreetmap/josm/gui/datatransfer/OsmTransferHandler.java

@@ -17 +17 @@
 import org.openstreetmap.josm.gui.datatransfer.importers.FilePaster;
 import org.openstreetmap.josm.gui.datatransfer.importers.PrimitiveDataPaster;
+import org.openstreetmap.josm.gui.datatransfer.importers.PrimitiveTagTransferPaster;
 import org.openstreetmap.josm.gui.datatransfer.importers.TagTransferPaster;
 import org.openstreetmap.josm.gui.datatransfer.importers.TextTagPaster;
@@ -30 +31 @@
     private static final Collection<AbstractOsmDataPaster> SUPPORTED = Arrays.asList(
             new FilePaster(), new PrimitiveDataPaster(),
+            new PrimitiveTagTransferPaster(),
             new TagTransferPaster(), new TextTagPaster());
 

trunk/src/org/openstreetmap/josm/gui/datatransfer/PrimitiveTransferable.java

@@ -12 +12 @@
 import org.openstreetmap.josm.data.osm.PrimitiveData;
 import org.openstreetmap.josm.gui.datatransfer.data.OsmLayerTransferData;
+import org.openstreetmap.josm.gui.datatransfer.data.PrimitiveTagTransferData;
 import org.openstreetmap.josm.gui.datatransfer.data.PrimitiveTransferData;
 import org.openstreetmap.josm.gui.datatransfer.data.TagTransferData;
@@ -75 +76 @@
         } else if (PrimitiveTransferData.DATA_FLAVOR.equals(flavor)) {
             return primitives;
+        } else if (PrimitiveTagTransferData.FLAVOR.equals(flavor)) {
+            return new PrimitiveTagTransferData(primitives);
         } else if (TagTransferData.FLAVOR.equals(flavor)) {
             return new TagTransferData(primitives.getDirectlyAdded());

trunk/src/org/openstreetmap/josm/gui/datatransfer/data/TagTransferData.java

@@ -14 +14 @@
  * This is a special transfer type that only transfers tag data.
  * <p>
- * It currently contains all tags contained in the selection that was copied.
+ * It contains all tags contained in the selection that was copied. For conflicting tags, any of the values may be used.
  * @author Michael Zangl
  * @since 10604

trunk/src/org/openstreetmap/josm/gui/datatransfer/importers/AbstractTagPaster.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.datatransfer.importers;
+
+import static org.openstreetmap.josm.tools.I18n.trn;
 
 import java.awt.datatransfer.DataFlavor;
@@ -6 +8 @@
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
@@ -12 +16 @@
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.command.ChangePropertyCommand;
+import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.tools.I18n;
 
 /**
@@ -42 +49 @@
             throws UnsupportedFlavorException, IOException {
         ChangePropertyCommand command = new ChangePropertyCommand(selection, getTags(support));
-        Main.main.undoRedo.add(command);
+        commitCommands(selection, Collections.singletonList(command));
         return true;
+    }
+
+    /**
+     * Create and execute SequenceCommand with descriptive title
+     * @param selection selected primitives
+     * @param commands the commands to perform in a sequential command
+     * @since 10737
+     */
+    protected static void commitCommands(Collection<? extends OsmPrimitive> selection, List<Command> commands) {
+        if (!commands.isEmpty()) {
+            String title1 = trn("Pasting {0} tag", "Pasting {0} tags", commands.size(), commands.size());
+            String title2 = trn("to {0} object", "to {0} objects", selection.size(), selection.size());
+            @I18n.QuirkyPluralString
+            final String title = title1 + ' ' + title2;
+            Main.main.undoRedo.add(new SequenceCommand(title, commands));
+        }
     }
 

trunk/src/org/openstreetmap/josm/gui/datatransfer/importers/TextTagPaster.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.datatransfer.importers;
+
+import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 
 import java.awt.datatransfer.DataFlavor;
@@ -18 +20 @@
  */
 public final class TextTagPaster extends AbstractTagPaster {
+    private static final String help = ht("/Action/PasteTags");
 
     /**
@@ -29 +32 @@
     public boolean supports(TransferSupport support) {
         try {
-            return super.supports(support) && getTags(support) != null;
+            return super.supports(support) && containsValidTags(support);
         } catch (UnsupportedFlavorException | IOException e) {
             Main.warn(e);
@@ -36 +39 @@
     }
 
+    private boolean containsValidTags(TransferSupport support) throws UnsupportedFlavorException, IOException {
+        Map<String, String> tags = getTagsImpl(support);
+        return tags != null && !tags.isEmpty();
+    }
+
     @Override
     protected Map<String, String> getTags(TransferSupport support) throws UnsupportedFlavorException, IOException {
+        Map<String, String> tags = getTagsImpl(support);
+        if (tags == null || tags.isEmpty()) {
+            TextTagParser.showBadBufferMessage(help);
+            throw new IOException("Invalid tags to paste.");
+        }
+        if (!TextTagParser.validateTags(tags)) {
+            throw new IOException("Tags to paste are not valid.");
+        }
+        return tags;
+    }
+
+    private Map<String, String> getTagsImpl(TransferSupport support) throws UnsupportedFlavorException, IOException {
         return TextTagParser.readTagsFromText((String) support.getTransferable().getTransferData(df));
     }