Changeset 11772 in josm for trunk/src/org/openstreetmap/josm/gui/conflict

Timestamp:

2017-03-23T00:01:04+01:00 (7 years ago)

Author:

Don-vip

Message:

fix #14475 - Deadlock while starting unit tests: remove the need to construct a CombinePrimitiveDialog to detect conflicts

Location:

trunk/src/org/openstreetmap/josm/gui/conflict/tags

Files:

• CombinePrimitiveResolver.java (added)
• CombinePrimitiveResolverDialog.java (modified) (25 diffs)
• PasteTagsConflictResolverDialog.java (modified) (9 diffs)
• TagConflictResolver.java (modified) (1 diff)

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

@@ -12 +12 @@
 import java.awt.GraphicsEnvironment;
 import java.awt.event.ActionEvent;
-import java.awt.event.HierarchyBoundsListener;
-import java.awt.event.HierarchyEvent;
 import java.awt.event.WindowAdapter;
 import java.awt.event.WindowEvent;
@@ -35 +33 @@
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.actions.ExpertToggleAction;
-import org.openstreetmap.josm.command.ChangePropertyCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.data.osm.Node;
@@ -47 +44 @@
 import org.openstreetmap.josm.gui.help.HelpUtil;
 import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.gui.widgets.AutoAdjustingSplitPane;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 import org.openstreetmap.josm.tools.ImageProvider;
@@ -62 +60 @@
  * Prior to {@link #launchIfNecessary}, the following usage sequence was needed:
  *
- * There is a singleton instance of this dialog which can be retrieved using
- * {@link #getInstance()}.
- *
  * The dialog uses two models: one  for resolving tag conflicts, the other
  * for resolving conflicts in relation memberships. For both models there are accessors,
@@ -71 +66 @@
  * Models have to be <strong>populated</strong> before the dialog is launched. Example:
  * <pre>
- *    CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
+ *    CombinePrimitiveResolverDialog dialog = new CombinePrimitiveResolverDialog(Main.parent);
  *    dialog.getTagConflictResolverModel().populate(aTagCollection);
  *    dialog.getRelationMemberConflictResolverModel().populate(aRelationLinkCollection);
@@ -87 +82 @@
 public class CombinePrimitiveResolverDialog extends JDialog {
 
-    /** the unique instance of the dialog */
-    private static CombinePrimitiveResolverDialog instance;
-
-    /**
-     * Replies the unique instance of the dialog
-     *
-     * @return the unique instance of the dialog
-     * @deprecated use {@link #launchIfNecessary} instead.
-     */
-    @Deprecated
-    public static synchronized CombinePrimitiveResolverDialog getInstance() {
-        if (instance == null) {
-            GuiHelper.runInEDTAndWait(() -> instance = new CombinePrimitiveResolverDialog(Main.parent));
-        }
-        return instance;
-    }
-
     private AutoAdjustingSplitPane spTagConflictTypes;
-    private TagConflictResolver pnlTagConflictResolver;
+    private final TagConflictResolverModel modelTagConflictResolver;
+    protected TagConflictResolver pnlTagConflictResolver;
+    private final RelationMemberConflictResolverModel modelRelConflictResolver;
     protected RelationMemberConflictResolver pnlRelationMemberConflictResolver;
+    private final CombinePrimitiveResolver primitiveResolver;
     private boolean applied;
     private JPanel pnlButtons;
@@ -117 +98 @@
 
     /**
-     * Replies the target primitive the collection of primitives is merged
-     * or combined to.
+     * Constructs a new {@code CombinePrimitiveResolverDialog}.
+     * @param parent The parent component in which this dialog will be displayed.
+     */
+    public CombinePrimitiveResolverDialog(Component parent) {
+        this(parent, new TagConflictResolverModel(), new RelationMemberConflictResolverModel());
+    }
+
+    /**
+     * Constructs a new {@code CombinePrimitiveResolverDialog}.
+     * @param parent The parent component in which this dialog will be displayed.
+     * @param tagModel tag conflict resolver model
+     * @param relModel relation member conflict resolver model
+     * @since 11772
+     */
+    public CombinePrimitiveResolverDialog(Component parent,
+            TagConflictResolverModel tagModel, RelationMemberConflictResolverModel relModel) {
+        super(GuiHelper.getFrameForComponent(parent), ModalityType.DOCUMENT_MODAL);
+        this.modelTagConflictResolver = tagModel;
+        this.modelRelConflictResolver = relModel;
+        this.primitiveResolver = new CombinePrimitiveResolver(tagModel, relModel);
+        build();
+    }
+
+    /**
+     * Replies the target primitive the collection of primitives is merged or combined to.
      *
      * @return the target primitive
-     */
-    public OsmPrimitive getTargetPrimitmive() {
+     * @since 11772 (naming)
+     */
+    public OsmPrimitive getTargetPrimitive() {
         return targetPrimitive;
     }
@@ -149 +154 @@
     }
 
+    /**
+     * Updates the dialog title.
+     */
     protected void updateTitle() {
         if (targetPrimitive == null) {
@@ -169 +177 @@
     }
 
+    /**
+     * Builds the components.
+     */
     protected final void build() {
         getContentPane().setLayout(new BorderLayout());
@@ -182 +193 @@
     }
 
+    /**
+     * Builds the tag conflict resolver panel.
+     * @return the tag conflict resolver panel
+     */
     protected JPanel buildTagConflictResolverPanel() {
-        pnlTagConflictResolver = new TagConflictResolver();
+        pnlTagConflictResolver = new TagConflictResolver(modelTagConflictResolver);
         return pnlTagConflictResolver;
     }
 
+    /**
+     * Builds the relation member conflict resolver panel.
+     * @return the relation member conflict resolver panel
+     */
     protected JPanel buildRelationMemberConflictResolverPanel() {
-        pnlRelationMemberConflictResolver = new RelationMemberConflictResolver(new RelationMemberConflictResolverModel());
+        pnlRelationMemberConflictResolver = new RelationMemberConflictResolver(modelRelConflictResolver);
         return pnlRelationMemberConflictResolver;
     }
 
+    /**
+     * Builds the "Apply" action.
+     * @return the "Apply" action
+     */
     protected ApplyAction buildApplyAction() {
         return new ApplyAction();
     }
 
+    /**
+     * Builds the button panel.
+     * @return the button panel
+     */
     protected JPanel buildButtonPanel() {
         JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER));
@@ -201 +228 @@
         // -- apply button
         ApplyAction applyAction = buildApplyAction();
-        pnlTagConflictResolver.getModel().addPropertyChangeListener(applyAction);
-        pnlRelationMemberConflictResolver.getModel().addPropertyChangeListener(applyAction);
+        modelTagConflictResolver.addPropertyChangeListener(applyAction);
+        modelRelConflictResolver.addPropertyChangeListener(applyAction);
         btnApply = new JButton(applyAction);
         btnApply.setFocusable(true);
@@ -219 +246 @@
 
     /**
-     * Constructs a new {@code CombinePrimitiveResolverDialog}.
-     * @param parent The parent component in which this dialog will be displayed.
-     */
-    public CombinePrimitiveResolverDialog(Component parent) {
-        super(GuiHelper.getFrameForComponent(parent), ModalityType.DOCUMENT_MODAL);
-        build();
-    }
-
-    /**
      * Replies the tag conflict resolver model.
      * @return The tag conflict resolver model.
      */
     public TagConflictResolverModel getTagConflictResolverModel() {
-        return pnlTagConflictResolver.getModel();
+        return modelTagConflictResolver;
     }
 
@@ -240 +258 @@
      */
     public RelationMemberConflictResolverModel getRelationMemberConflictResolverModel() {
-        return pnlRelationMemberConflictResolver.getModel();
+        return modelRelConflictResolver;
     }
 
@@ -249 +267 @@
      */
     public boolean isResolvedCompletely() {
-        return getTagConflictResolverModel().isResolvedCompletely()
-                && getRelationMemberConflictResolverModel().isResolvedCompletely();
-    }
-
+        return modelTagConflictResolver.isResolvedCompletely()
+            && modelRelConflictResolver.isResolvedCompletely();
+    }
+
+    /**
+     * Builds the list of tag change commands.
+     * @param primitive target primitive
+     * @param tc all resolutions
+     * @return the list of tag change commands
+     */
     protected List<Command> buildTagChangeCommand(OsmPrimitive primitive, TagCollection tc) {
-        List<Command> cmds = new LinkedList<>();
-        for (String key : tc.getKeys()) {
-            if (tc.hasUniqueEmptyValue(key)) {
-                if (primitive.get(key) != null) {
-                    cmds.add(new ChangePropertyCommand(primitive, key, null));
-                }
-            } else {
-                String value = tc.getJoinedValues(key);
-                if (!value.equals(primitive.get(key))) {
-                    cmds.add(new ChangePropertyCommand(primitive, key, value));
-                }
-            }
-        }
-        return cmds;
+        return primitiveResolver.buildTagChangeCommand(primitive, tc);
     }
 
@@ -275 +286 @@
      */
     public List<Command> buildResolutionCommands() {
-        List<Command> cmds = new LinkedList<>();
-
-        TagCollection allResolutions = getTagConflictResolverModel().getAllResolutions();
-        if (!allResolutions.isEmpty()) {
-            cmds.addAll(buildTagChangeCommand(targetPrimitive, allResolutions));
-        }
-        for (String p : OsmPrimitive.getDiscardableKeys()) {
-            if (targetPrimitive.get(p) != null) {
-                cmds.add(new ChangePropertyCommand(targetPrimitive, p, null));
-            }
-        }
-
-        if (getRelationMemberConflictResolverModel().getNumDecisions() > 0) {
-            cmds.addAll(getRelationMemberConflictResolverModel().buildResolutionCommands(targetPrimitive));
-        }
-
-        Command cmd = pnlRelationMemberConflictResolver.buildTagApplyCommands(getRelationMemberConflictResolverModel()
+        List<Command> cmds = primitiveResolver.buildResolutionCommands(targetPrimitive);
+        Command cmd = pnlRelationMemberConflictResolver.buildTagApplyCommands(modelRelConflictResolver
                 .getModifiedRelations(targetPrimitive));
         if (cmd != null) {
@@ -312 +308 @@
      */
     private void prepareDefaultDecisions(boolean fireEvent) {
-        getTagConflictResolverModel().prepareDefaultTagDecisions(fireEvent);
-        getRelationMemberConflictResolverModel().prepareDefaultRelationDecisions(fireEvent);
-    }
-
+        modelTagConflictResolver.prepareDefaultTagDecisions(fireEvent);
+        modelRelConflictResolver.prepareDefaultRelationDecisions(fireEvent);
+    }
+
+    /**
+     * Builds empty conflicts panel.
+     * @return empty conflicts panel
+     */
     protected JPanel buildEmptyConflictsPanel() {
         JPanel pnl = new JPanel(new BorderLayout());
@@ -322 +322 @@
     }
 
+    /**
+     * Prepares GUI before conflict resolution starts.
+     */
     protected void prepareGUIBeforeConflictResolutionStarts() {
-        RelationMemberConflictResolverModel relModel = getRelationMemberConflictResolverModel();
-        TagConflictResolverModel tagModel = getTagConflictResolverModel();
         getContentPane().removeAll();
 
-        if (relModel.getNumDecisions() > 0 && tagModel.getNumDecisions() > 0) {
+        if (modelRelConflictResolver.getNumDecisions() > 0 && modelTagConflictResolver.getNumDecisions() > 0) {
             // display both, the dialog for resolving relation conflicts and for resolving tag conflicts
             spTagConflictTypes.setTopComponent(pnlTagConflictResolver);
             spTagConflictTypes.setBottomComponent(pnlRelationMemberConflictResolver);
             getContentPane().add(spTagConflictTypes, BorderLayout.CENTER);
-        } else if (relModel.getNumDecisions() > 0) {
+        } else if (modelRelConflictResolver.getNumDecisions() > 0) {
             // relation conflicts only
             getContentPane().add(pnlRelationMemberConflictResolver, BorderLayout.CENTER);
-        } else if (tagModel.getNumDecisions() > 0) {
+        } else if (modelTagConflictResolver.getNumDecisions() > 0) {
             // tag conflicts only
             getContentPane().add(pnlTagConflictResolver, BorderLayout.CENTER);
@@ -348 +349 @@
     }
 
+    /**
+     * Sets whether this dialog has been closed with "Apply".
+     * @param applied {@code true} if this dialog has been closed with "Apply"
+     */
     protected void setApplied(boolean applied) {
         this.applied = applied;
@@ -375 +380 @@
     }
 
-    class CancelAction extends AbstractAction {
-
-        CancelAction() {
+    /**
+     * Cancel action.
+     */
+    protected class CancelAction extends AbstractAction {
+
+        /**
+         * Constructs a new {@code CancelAction}.
+         */
+        public CancelAction() {
             putValue(Action.SHORT_DESCRIPTION, tr("Cancel conflict resolution"));
             putValue(Action.NAME, tr("Cancel"));
@@ -390 +401 @@
     }
 
+    /**
+     * Apply action.
+     */
     protected class ApplyAction extends AbstractAction implements PropertyChangeListener {
 
+        /**
+         * Constructs a new {@code ApplyAction}.
+         */
         public ApplyAction() {
             putValue(Action.SHORT_DESCRIPTION, tr("Apply resolved conflicts"));
@@ -406 +423 @@
         }
 
+        /**
+         * Updates enabled state.
+         */
         protected final void updateEnabledState() {
-            setEnabled(pnlTagConflictResolver.getModel().getNumConflicts() == 0
-                    && pnlRelationMemberConflictResolver.getModel().getNumConflicts() == 0);
+            setEnabled(modelTagConflictResolver.isResolvedCompletely()
+                    && modelRelConflictResolver.isResolvedCompletely());
         }
 
@@ -423 +443 @@
 
     private void adjustDividerLocation() {
-        int numTagDecisions = getTagConflictResolverModel().getNumDecisions();
-        int numRelationDecisions = getRelationMemberConflictResolverModel().getNumDecisions();
+        int numTagDecisions = modelTagConflictResolver.getNumDecisions();
+        int numRelationDecisions = modelRelConflictResolver.getNumDecisions();
         if (numTagDecisions > 0 && numRelationDecisions > 0) {
             double nTop = 1.0 + numTagDecisions;
@@ -436 +456 @@
         public void windowOpened(WindowEvent e) {
             adjustDividerLocation();
-        }
-    }
-
-    static class AutoAdjustingSplitPane extends JSplitPane implements PropertyChangeListener, HierarchyBoundsListener {
-        private double dividerLocation;
-
-        AutoAdjustingSplitPane(int newOrientation) {
-            super(newOrientation);
-            addPropertyChangeListener(JSplitPane.DIVIDER_LOCATION_PROPERTY, this);
-            addHierarchyBoundsListener(this);
-        }
-
-        @Override
-        public void ancestorResized(HierarchyEvent e) {
-            setDividerLocation((int) (dividerLocation * getHeight()));
-        }
-
-        @Override
-        public void ancestorMoved(HierarchyEvent e) {
-            // do nothing
-        }
-
-        @Override
-        public void propertyChange(PropertyChangeEvent evt) {
-            if (JSplitPane.DIVIDER_LOCATION_PROPERTY.equals(evt.getPropertyName())) {
-                int newVal = (Integer) evt.getNewValue();
-                if (getHeight() != 0) {
-                    dividerLocation = (double) newVal / (double) getHeight();
-                }
-            }
         }
     }
@@ -512 +502 @@
         }
 
-        List<Command> cmds = new LinkedList<>();
-
-        if (!GraphicsEnvironment.isHeadless()) {
+        final List<Command> cmds = new LinkedList<>();
+
+        final TagConflictResolverModel tagModel = new TagConflictResolverModel();
+        final RelationMemberConflictResolverModel relModel = new RelationMemberConflictResolverModel();
+
+        tagModel.populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues(), false);
+        relModel.populate(parentRelations, primitives, false);
+        tagModel.prepareDefaultTagDecisions(false);
+        relModel.prepareDefaultRelationDecisions(false);
+
+        if (tagModel.isResolvedCompletely() && relModel.isResolvedCompletely()) {
+            // Build commands without need of dialog
+            CombinePrimitiveResolver resolver = new CombinePrimitiveResolver(tagModel, relModel);
+            for (OsmPrimitive i : targetPrimitives) {
+                cmds.addAll(resolver.buildResolutionCommands(i));
+            }
+        } else if (!GraphicsEnvironment.isHeadless()) {
             // Build conflict resolution dialog
-            final CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
-
-            dialog.getTagConflictResolverModel().populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues(), false);
-            dialog.getRelationMemberConflictResolverModel().populate(parentRelations, primitives, false);
-            dialog.prepareDefaultDecisions(false);
+            final CombinePrimitiveResolverDialog dialog = new CombinePrimitiveResolverDialog(Main.parent, tagModel, relModel);
 
             // Ensure a proper title is displayed instead of a previous target (fix #7925)
@@ -529 +529 @@
             }
 
-            // Resolve tag conflicts if necessary
-            if (!dialog.isResolvedCompletely()) {
-                GuiHelper.runInEDTAndWait(() -> {
-                    dialog.getTagConflictResolverModel().fireTableDataChanged();
-                    dialog.getRelationMemberConflictResolverModel().fireTableDataChanged();
-                    dialog.updateTitle();
-                });
-                dialog.setVisible(true);
-                if (!dialog.isApplied()) {
-                    throw new UserCancelException();
-                }
-            }
+            // Resolve tag conflicts
+            GuiHelper.runInEDTAndWait(() -> {
+                tagModel.fireTableDataChanged();
+                relModel.fireTableDataChanged();
+                dialog.updateTitle();
+            });
+            dialog.setVisible(true);
+            if (!dialog.isApplied()) {
+                throw new UserCancelException();
+            }
+
+            // Build commands
             for (OsmPrimitive i : targetPrimitives) {
                 dialog.setTargetPrimitive(i, false);

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

@@ -58 +58 @@
     }
 
-    private final TagConflictResolver allPrimitivesResolver = new TagConflictResolver();
+    private final TagConflictResolverModel model = new TagConflictResolverModel();
     private final transient Map<OsmPrimitiveType, TagConflictResolver> resolvers = new EnumMap<>(OsmPrimitiveType.class);
     private final JTabbedPane tpResolvers = new JTabbedPane();
@@ -81 +81 @@
         setTitle(tr("Conflicts in pasted tags"));
         for (OsmPrimitiveType type: OsmPrimitiveType.dataValues()) {
-            resolvers.put(type, new TagConflictResolver());
-            resolvers.get(type).getModel().addPropertyChangeListener(this);
+            TagConflictResolverModel tagModel = new TagConflictResolverModel();
+            resolvers.put(type, new TagConflictResolver(tagModel));
+            tagModel.addPropertyChangeListener(this);
         }
         getContentPane().setLayout(new GridBagLayout());
@@ -106 +107 @@
         getContentPane().add(buildButtonPanel(), gc);
         InputMapUtils.addEscapeAction(getRootPane(), new CancelAction());
-
     }
 
@@ -114 +114 @@
         // -- apply button
         ApplyAction applyAction = new ApplyAction();
-        allPrimitivesResolver.getModel().addPropertyChangeListener(applyAction);
+        model.addPropertyChangeListener(applyAction);
         for (TagConflictResolver r : resolvers.values()) {
             r.getModel().addPropertyChangeListener(applyAction);
@@ -141 +141 @@
      */
     protected void initResolver(OsmPrimitiveType type, TagCollection tc, Map<OsmPrimitiveType, Integer> targetStatistics) {
-        resolvers.get(type).getModel().populate(tc, tc.getKeysWithMultipleValues());
-        resolvers.get(type).getModel().prepareDefaultTagDecisions();
+        TagConflictResolver resolver = resolvers.get(type);
+        resolver.getModel().populate(tc, tc.getKeysWithMultipleValues());
+        resolver.getModel().prepareDefaultTagDecisions();
         if (!tc.isEmpty() && targetStatistics.get(type) != null && targetStatistics.get(type) > 0) {
-            tpResolvers.add(PANE_TITLES.get(type), resolvers.get(type));
+            tpResolvers.add(PANE_TITLES.get(type), resolver);
         }
     }
@@ -164 +165 @@
         // init the resolver
         //
-        allPrimitivesResolver.getModel().populate(tagsForAllPrimitives, tagsForAllPrimitives.getKeysWithMultipleValues());
-        allPrimitivesResolver.getModel().prepareDefaultTagDecisions();
+        model.populate(tagsForAllPrimitives, tagsForAllPrimitives.getKeysWithMultipleValues());
+        model.prepareDefaultTagDecisions();
 
         // prepare the dialog with one tag resolver
         pnlTagResolver.removeAll();
-        pnlTagResolver.add(allPrimitivesResolver, BorderLayout.CENTER);
+        pnlTagResolver.add(new TagConflictResolver(model), BorderLayout.CENTER);
 
         statisticsModel.reset();
@@ -297 +298 @@
                 setEnabled(false);
             } else if (mode.equals(Mode.RESOLVING_ONE_TAGCOLLECTION_ONLY)) {
-                setEnabled(allPrimitivesResolver.getModel().isResolvedCompletely());
+                setEnabled(model.isResolvedCompletely());
             } else {
                 setEnabled(resolvers.values().stream().allMatch(val -> val.getModel().isResolvedCompletely()));
@@ -329 +330 @@
      */
     public TagCollection getResolution() {
-        return allPrimitivesResolver.getModel().getResolution();
+        return model.getResolution();
     }
 
@@ -340 +341 @@
     public void propertyChange(PropertyChangeEvent evt) {
         if (evt.getPropertyName().equals(TagConflictResolverModel.NUM_CONFLICTS_PROP)) {
-            TagConflictResolverModel model = (TagConflictResolverModel) evt.getSource();
+            TagConflictResolverModel tagModel = (TagConflictResolverModel) evt.getSource();
             for (int i = 0; i < tpResolvers.getTabCount(); i++) {
                 TagConflictResolver resolver = (TagConflictResolver) tpResolvers.getComponentAt(i);
-                if (model == resolver.getModel()) {
+                if (tagModel == resolver.getModel()) {
                     tpResolvers.setIconAt(i,
                             (Integer) evt.getNewValue() == 0 ? iconResolved : iconUnresolved

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

@@ -31 +31 @@
     /**
      * Constructs a new {@code TagConflictResolver}.
+     * @param model tag conflict resolver model
+     * @since 11772
      */
-    public TagConflictResolver() {
-        this.model = new TagConflictResolverModel();
+    public TagConflictResolver(TagConflictResolverModel model) {
+        this.model = model;
         build();
     }