diff --git src/org/openstreetmap/josm/gui/colocation/ColocatedNodesResolver.java src/org/openstreetmap/josm/gui/colocation/ColocatedNodesResolver.java new file mode 100644 index 000000000..4887233ee --- /dev/null +++ src/org/openstreetmap/josm/gui/colocation/ColocatedNodesResolver.java @@ -0,0 +1,235 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.colocation; + +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.gui.ExtendedDialog; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.spi.preferences.Config; + +import javax.swing.ButtonGroup; +import javax.swing.ButtonModel; +import javax.swing.JCheckBox; +import javax.swing.JLabel; +import javax.swing.JOptionPane; +import javax.swing.JPanel; +import javax.swing.JRadioButton; +import javax.swing.event.ChangeEvent; +import java.awt.BorderLayout; +import java.awt.FlowLayout; +import java.util.HashMap; +import java.util.Map; + +import static org.openstreetmap.josm.tools.I18n.tr; + +/** + * Co-located node resolver is used to detect nodes that are at the same LatLon point during a geojson import. + * + * @since xxx + */ +public class ColocatedNodesResolver { + // Resolution choices. Strings are used for easy interoperability with + // preference storage + /** + * Keep only one node found at location (recommended behavior) + */ + public static final String RESOLVE_KEEP_ONE = "one"; + + /** + * Keep all nodes found at location + */ + public static final String RESOLVE_KEEP_ALL = "all"; + + // Future application choices + /** + * Prompt user to manually resolve next future incident + */ + public static final int APPLY_PROMPT = 1; + + /** + * Resolve detected colocations in chosen manner for all nodes sharing the + * same location as this incident, but prompting user again for new + * locations + */ + public static final int APPLY_ALL_AT_LOCATION = 2; + + /** + * Resolve detected colocations in chosen manner for all further incidents + * and do not prompt user further + */ + public static final int APPLY_ALL = 3; + + /** + * The current resolution choice for this resolver (see RESOLVE_* constants) + */ + private String currentResolution; + + /** + * The current application choice for this resolver (see APPLY_* constants) + */ + private int currentApplication; + + /** + * Map of locations to resolution decision. An entry exists when the user has chosen to apply a + * resolution to all future detected nodes at that location + */ + private final Map locationDecisions; + + /** + * Constructs a ColocatedNodesResolver with the backward-compatible behavior of + * automatically keeping only one node found at a location + */ + public ColocatedNodesResolver() { + this(RESOLVE_KEEP_ONE, APPLY_ALL); + } + + public ColocatedNodesResolver(final String defaultResolution, final int defaultApplication) { + this.currentResolution = defaultResolution; + this.currentApplication = defaultApplication; + this.locationDecisions = new HashMap<>(); + } + + /** + * Resolves detected colocation at a specified location, either using a + * decision made in past that is to apply to future colocations, or else + * prompting the user to make a decision + * + * @param latlon the location to check for a resolution + * @return the resolution + */ + public String resolveColocatedNodes(final LatLon latlon) { + // First, is there a decision specific to this location? + if (this.locationDecisions.containsKey(latlon)) { + return this.locationDecisions.get(latlon); + } + + // Next, is there a decision to apply to all detected colocations? + if (this.currentApplication == APPLY_ALL) { + return this.currentResolution; + } + + // Is there a previously saved choice stored in preferences? + final String preferenceKey = "import.colocated-nodes.keep"; + final String resolution = Config.getPref().get(preferenceKey, null); + + if (RESOLVE_KEEP_ONE.equals(resolution) || RESOLVE_KEEP_ALL.equals(resolution)) { + return resolution; + } + + // Otherwise ask the user how to resolve + GuiHelper.runInEDTAndWait(() -> { + ResolveDialog dialog = new ResolveDialog(latlon, this.currentApplication); + dialog.showDialog(); + switch (dialog.getValue()) { + case 1: + this.currentResolution = RESOLVE_KEEP_ONE; + break; + case 2: + this.currentResolution = RESOLVE_KEEP_ALL; + break; + } + this.currentApplication = dialog.getApplyToValue(); + + if (this.currentApplication == APPLY_ALL_AT_LOCATION) { + this.locationDecisions.put(latlon, this.currentResolution); + } + + if (dialog.shouldSaveChoice()) { + Config.getPref().put(preferenceKey, this.currentResolution); + } + }); + + return this.currentResolution; + } + + /** + * Dialog that prompts user to decide how to treat detected colocated nodes + */ + private static class ResolveDialog extends ExtendedDialog { + private final ButtonGroup applyOptionsGroup; + private final JCheckBox rememberCheckbox; + + ResolveDialog(final LatLon latlon, final int currentApplication) { + super(MainApplication.getMainFrame(), + tr("Resolve Co-located Nodes"), + tr("Keep One Node (recommended)"), tr("Keep All Nodes")); + + setIcon(JOptionPane.WARNING_MESSAGE); + JPanel dialogPanel = new JPanel(new BorderLayout()); + JPanel rememberChoicePanel = new JPanel(new BorderLayout()); + dialogPanel.add(new JLabel("" + + tr("Import contains multiple nodes positioned at ") + + latlon.toDisplayString() + + ".
" + + tr("How would you like to proceed with these nodes?") + + "

" + + ""), + BorderLayout.NORTH); + + // Options for applying chosen resolution to future colocated nodes + JPanel applyOptionsPanel = new JPanel(new FlowLayout()); + JPanel buttonGroupPanel = new JPanel(new FlowLayout()); + this.applyOptionsGroup = new ButtonGroup(); + + JRadioButton locationOption = new JRadioButton(tr("This location")); + locationOption.setActionCommand("location"); + this.applyOptionsGroup.add(locationOption); + locationOption.setSelected(currentApplication == APPLY_ALL_AT_LOCATION); + buttonGroupPanel.add(locationOption); + + JRadioButton allOption = new JRadioButton(tr("All locations")); + allOption.setActionCommand("all"); + this.applyOptionsGroup.add(allOption); + allOption.setSelected(currentApplication != APPLY_ALL_AT_LOCATION); + + // Listen for changes to the applies-to-all radio button, as we + // only want to offer the option to remember choice if the choice + // is being applied to all nodes rather than a single location, as + // we naturally have to prompt for each new location if user wants + // per-location option + allOption.addChangeListener((ChangeEvent e) -> { + if (allOption.isSelected()) { + if (!dialogPanel.isAncestorOf(rememberChoicePanel)) { + dialogPanel.add(rememberChoicePanel, BorderLayout.SOUTH); + } + } else { + dialogPanel.remove(rememberChoicePanel); + } + + this.revalidate(); + this.pack(); + this.repaint(); + }); + buttonGroupPanel.add(allOption); + + applyOptionsPanel.add(new JLabel(tr("Apply this choice to:"))); + applyOptionsPanel.add(buttonGroupPanel); + dialogPanel.add(applyOptionsPanel, BorderLayout.CENTER); + + // Remember this decision? Only available for all-locations choice, + // as we naturally have to prompt for each new location if user + // wants per-location option + this.rememberCheckbox = new JCheckBox(tr("Don''t ask me again")); + rememberChoicePanel.add(this.rememberCheckbox, BorderLayout.SOUTH); + if (currentApplication != APPLY_ALL_AT_LOCATION) { + dialogPanel.add(rememberChoicePanel, BorderLayout.SOUTH); + } + + setContent(dialogPanel); + } + + public int getApplyToValue() { + final ButtonModel selected = this.applyOptionsGroup.getSelection(); + if (selected != null && "all".equals(selected.getActionCommand())) { + return APPLY_ALL; + } else { + return APPLY_ALL_AT_LOCATION; + } + } + + public boolean shouldSaveChoice() { + // Only allow apply-to-all choices to be saved + return this.getApplyToValue() == APPLY_ALL && this.rememberCheckbox.isSelected(); + } + } +} diff --git src/org/openstreetmap/josm/gui/io/importexport/GeoJSONImporter.java src/org/openstreetmap/josm/gui/io/importexport/GeoJSONImporter.java index c1e59d67e..94800c0b4 100644 --- src/org/openstreetmap/josm/gui/io/importexport/GeoJSONImporter.java +++ src/org/openstreetmap/josm/gui/io/importexport/GeoJSONImporter.java @@ -16,6 +16,7 @@ import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.gui.colocation.ColocatedNodesResolver; import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.io.CachedFile; import org.openstreetmap.josm.io.Compression; @@ -48,7 +49,9 @@ public class GeoJSONImporter extends FileImporter { progressMonitor.setTicksCount(2); Logging.info("Parsing GeoJSON: {0}", file.getAbsolutePath()); try (InputStream fileInputStream = Compression.getUncompressedFileInputStream(file)) { - DataSet data = GeoJSONReader.parseDataSet(fileInputStream, progressMonitor); + ColocatedNodesResolver resolver = new ColocatedNodesResolver( + ColocatedNodesResolver.RESOLVE_KEEP_ONE, ColocatedNodesResolver.APPLY_PROMPT); + DataSet data = GeoJSONReader.parseDataSet(fileInputStream, progressMonitor, resolver); progressMonitor.worked(1); MainApplication.getLayerManager().addLayer(new OsmDataLayer(data, file.getName(), file)); } catch (IOException | IllegalArgumentException | IllegalDataException e) { diff --git src/org/openstreetmap/josm/io/GeoJSONReader.java src/org/openstreetmap/josm/io/GeoJSONReader.java index f389688a7..f8f004364 100644 --- src/org/openstreetmap/josm/io/GeoJSONReader.java +++ src/org/openstreetmap/josm/io/GeoJSONReader.java @@ -44,6 +44,7 @@ import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.tests.DuplicateWay; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.gui.colocation.ColocatedNodesResolver; import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -65,9 +66,15 @@ public class GeoJSONReader extends AbstractReader { /** The record separator is 0x1E per RFC 7464 */ private static final byte RECORD_SEPARATOR_BYTE = 0x1E; private Projection projection = Projections.getProjectionByCode("EPSG:4326"); // WGS 84 + private final ColocatedNodesResolver resolver; GeoJSONReader() { // Restricts visibility + this.resolver = new ColocatedNodesResolver(); + } + + GeoJSONReader(final ColocatedNodesResolver resolver) { + this.resolver = resolver; } private void parse(final JsonParser parser) throws IllegalDataException { @@ -279,8 +286,10 @@ public class GeoJSONReader extends AbstractReader { private Node createNode(final LatLon latlon) { final List existingNodes = getDataSet().searchNodes(new BBox(latlon, latlon)); if (!existingNodes.isEmpty()) { - // reuse existing node, avoid multiple nodes on top of each other - return existingNodes.get(0); + if (ColocatedNodesResolver.RESOLVE_KEEP_ONE.equals(this.resolver.resolveColocatedNodes(latlon))) { + // reuse existing node + return existingNodes.get(0); + } } final Node node = new Node(latlon); getDataSet().addPrimitive(node); @@ -300,7 +309,9 @@ public class GeoJSONReader extends AbstractReader { final boolean doAutoclose; if (size > 1) { if (latlons.get(0).equals(latlons.get(size - 1))) { - doAutoclose = false; // already closed + // Auto-close to avoid creating a dup final node + latlons.remove(size - 1); + doAutoclose = true; } else { doAutoclose = autoClose; } @@ -464,7 +475,8 @@ public class GeoJSONReader extends AbstractReader { } /** - * Parse the given input source and return the dataset. + * Parse the given input source and return the dataset, using default + * node-colocation resolution rules * * @param source the source input stream. Must not be null. * @param progressMonitor the progress monitor. If null, {@link NullProgressMonitor#INSTANCE} is assumed @@ -473,6 +485,21 @@ public class GeoJSONReader extends AbstractReader { * @throws IllegalArgumentException if source is null */ public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException { - return new GeoJSONReader().doParseDataSet(source, progressMonitor); + return GeoJSONReader.parseDataSet(source, progressMonitor, new ColocatedNodesResolver()); + } + + /** + * Parse the given input source and return the dataset. + * + * @param source the source input stream. Must not be null. + * @param progressMonitor the progress monitor. If null, {@link NullProgressMonitor#INSTANCE} is assumed + * @param resolver the resolver for determining outcome of nodes colocated in same location + * @return the dataset with the parsed data + * @throws IllegalDataException if an error was found while parsing the data from the source + * @throws IllegalArgumentException if source is null + */ + public static DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor, ColocatedNodesResolver resolver) + throws IllegalDataException { + return new GeoJSONReader(resolver).doParseDataSet(source, progressMonitor); } }