Subject: [PATCH] Fix #22733: UI blocks while trying different WMS endpoint urls and cannot handle `nan` in responses --- Index: src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java b/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java --- a/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java (revision 18675) +++ b/src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java (date 1677099495750) @@ -3,6 +3,8 @@ import static org.openstreetmap.josm.tools.I18n.tr; +import java.awt.GraphicsEnvironment; +import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.io.IOException; import java.net.MalformedURLException; @@ -15,6 +17,7 @@ import javax.swing.JButton; import javax.swing.JCheckBox; import javax.swing.JComboBox; +import javax.swing.JComponent; import javax.swing.JLabel; import javax.swing.JOptionPane; import javax.swing.JScrollPane; @@ -23,13 +26,20 @@ import org.openstreetmap.josm.data.imagery.ImageryInfo; import org.openstreetmap.josm.data.imagery.ImageryInfo.ImageryType; import org.openstreetmap.josm.data.imagery.LayerDetails; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.PleaseWaitRunnable; import org.openstreetmap.josm.gui.bbox.SlippyMapBBoxChooser; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; +import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor; import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.gui.widgets.JosmTextArea; +import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.io.imagery.WMSImagery; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; +import org.xml.sax.SAXException; /** * An imagery panel used to add WMS imagery sources. @@ -81,37 +91,7 @@ add(new JLabel(tr("{0} Enter name for this layer", "7.")), GBC.eol()); add(name, GBC.eop().fill(GBC.HORIZONTAL)); - getLayers.addActionListener(e -> { - try { - wms = new WMSImagery(Utils.strip(rawUrl.getText()), getCommonHeaders()); - tree.updateTree(wms); - Collection wmsFormats = wms.getFormats(); - formats.setModel(new DefaultComboBoxModel<>(wmsFormats.toArray(new String[0]))); - formats.setSelectedItem(wms.getPreferredFormat()); - } catch (MalformedURLException | InvalidPathException ex1) { - Logging.log(Logging.LEVEL_ERROR, ex1); - JOptionPane.showMessageDialog(getParent(), tr("Invalid service URL."), - tr("WMS Error"), JOptionPane.ERROR_MESSAGE); - } catch (IOException ex2) { - Logging.log(Logging.LEVEL_ERROR, ex2); - JOptionPane.showMessageDialog(getParent(), tr("Could not retrieve WMS layer list."), - tr("WMS Error"), JOptionPane.ERROR_MESSAGE); - } catch (WMSImagery.WMSGetCapabilitiesException ex3) { - String incomingData = ex3.getIncomingData() != null ? ex3.getIncomingData().trim() : ""; - String title = tr("WMS Error"); - StringBuilder message = new StringBuilder(tr("Could not parse WMS layer list.")); - Logging.log(Logging.LEVEL_ERROR, "Could not parse WMS layer list. Incoming data:\n"+incomingData, ex3); - if ((incomingData.startsWith("") || incomingData.startsWith("")) - && (incomingData.endsWith("") || incomingData.endsWith(""))) { - GuiHelper.notifyUserHtmlError(this, title, message.toString(), incomingData); - } else { - if (ex3.getMessage() != null) { - message.append('\n').append(ex3.getMessage()); - } - JOptionPane.showMessageDialog(getParent(), message.toString(), title, JOptionPane.ERROR_MESSAGE); - } - } - }); + getLayers.addActionListener(e -> MainApplication.worker.execute(new GetCapabilitiesRunnable(e))); ActionListener availabilityManagerAction = a -> { setDefaultLayers.setEnabled(endpoint.isSelected()); @@ -151,6 +131,72 @@ registerValidableComponent(setDefaultLayers); } + /** + * Perform the get WMS layers network calls in a separate thread, mostly to allow for cancellation + */ + private class GetCapabilitiesRunnable extends PleaseWaitRunnable { + private final ActionEvent actionEvent; + + protected GetCapabilitiesRunnable(ActionEvent actionEvent) { + super(tr("Trying WMS urls"), GraphicsEnvironment.isHeadless() ? NullProgressMonitor.INSTANCE : new PleaseWaitProgressMonitor(), + false); + this.actionEvent = actionEvent; + } + + @Override + protected void cancel() { + // We don't really have something we can do -- we use the monitor to pass the cancel state on + } + + @Override + protected void realRun() throws SAXException, IOException, OsmTransferException { + if (actionEvent.getSource() instanceof JComponent) { + GuiHelper.runInEDT(() -> ((JComponent) actionEvent.getSource()).setEnabled(false)); + } + ProgressMonitor monitor = getProgressMonitor(); + try { + wms = new WMSImagery(Utils.strip(rawUrl.getText()), getCommonHeaders(), monitor); + } catch (MalformedURLException | InvalidPathException ex1) { + Logging.log(Logging.LEVEL_ERROR, ex1); + GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(getParent(), tr("Invalid service URL."), + tr("WMS Error"), JOptionPane.ERROR_MESSAGE)); + } catch (IOException ex2) { + Logging.log(Logging.LEVEL_ERROR, ex2); + GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(getParent(), tr("Could not retrieve WMS layer list."), + tr("WMS Error"), JOptionPane.ERROR_MESSAGE)); + } catch (WMSImagery.WMSGetCapabilitiesException ex3) { + String incomingData = ex3.getIncomingData() != null ? ex3.getIncomingData().trim() : ""; + String title = tr("WMS Error"); + StringBuilder message = new StringBuilder(tr("Could not parse WMS layer list.")); + Logging.log(Logging.LEVEL_ERROR, "Could not parse WMS layer list. Incoming data:\n"+incomingData, ex3); + if ((incomingData.startsWith("") || incomingData.startsWith("")) + && (incomingData.endsWith("") || incomingData.endsWith(""))) { + GuiHelper.runInEDT(() -> GuiHelper.notifyUserHtmlError(AddWMSLayerPanel.this, title, message.toString(), incomingData)); + } else { + if (ex3.getMessage() != null) { + message.append('\n').append(ex3.getMessage()); + } + GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(getParent(), message.toString(), title, JOptionPane.ERROR_MESSAGE)); + } + } + } + + @Override + protected void finish() { + if (wms != null) { + GuiHelper.runInEDT(() -> { + tree.updateTree(wms); + Collection wmsFormats = wms.getFormats(); + formats.setModel(new DefaultComboBoxModel<>(wmsFormats.toArray(new String[0]))); + formats.setSelectedItem(wms.getPreferredFormat()); + }); + } + if (actionEvent.getSource() instanceof JComponent) { + GuiHelper.runInEDT(() -> ((JComponent) actionEvent.getSource()).setEnabled(true)); + } + } + } + protected final void onLayerSelectionChanged() { if (wms != null && wms.buildRootUrl() != null) { wmsUrl.setText(wms.buildGetMapUrl( Index: src/org/openstreetmap/josm/io/imagery/WMSImagery.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/io/imagery/WMSImagery.java b/src/org/openstreetmap/josm/io/imagery/WMSImagery.java --- a/src/org/openstreetmap/josm/io/imagery/WMSImagery.java (revision 18675) +++ b/src/org/openstreetmap/josm/io/imagery/WMSImagery.java (date 1677163917141) @@ -11,6 +11,7 @@ import java.net.URL; import java.nio.file.InvalidPathException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -35,6 +36,8 @@ import org.openstreetmap.josm.data.imagery.LayerDetails; import org.openstreetmap.josm.data.projection.Projection; import org.openstreetmap.josm.data.projection.Projections; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; +import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.io.CachedFile; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -150,20 +153,43 @@ * @throws InvalidPathException if a Path object cannot be constructed for the capabilities cached file */ public WMSImagery(String url, Map headers) throws IOException, WMSGetCapabilitiesException { + this(url, headers, NullProgressMonitor.INSTANCE); + } + + /** + * Make getCapabilities request towards given URL using headers + * @param url service url + * @param headers HTTP headers to be sent with request + * @param monitor Feedback for which URL we are currently trying, the integer is the total number of urls we are going to try + * @throws IOException when connection error when fetching get capabilities document + * @throws WMSGetCapabilitiesException when there are errors when parsing get capabilities document + * @throws InvalidPathException if a Path object cannot be constructed for the capabilities cached file + */ + public WMSImagery(String url, Map headers, ProgressMonitor monitor) + throws IOException, WMSGetCapabilitiesException { if (headers != null) { this.headers.putAll(headers); } IOException savedExc = null; String workingAddress = null; - url_search: - for (String z: new String[]{ - normalizeUrl(url), - url, - url + CAPABILITIES_QUERY_STRING, - }) { - for (String ver: new String[]{"", "&VERSION=1.3.0", "&VERSION=1.1.1"}) { + final String[] baseAdditions = { + normalizeUrl(url), + url, + url + CAPABILITIES_QUERY_STRING, + }; + final String[] versionAdditions = {"", "&VERSION=1.3.0", "&VERSION=1.1.1"}; + final int totalNumberOfUrlsToTry = baseAdditions.length * versionAdditions.length; + monitor.setTicksCount(totalNumberOfUrlsToTry); + url_search: + for (String z : baseAdditions) { + for (String ver : versionAdditions) { + if (monitor.isCanceled()) { + break url_search; + } try { + monitor.setCustomText(z + ver); + monitor.worked(1); attemptGetCapabilities(z + ver); workingAddress = z; calculateChildren(); @@ -192,7 +218,6 @@ } } } - if (savedExc != null) { throw savedExc; } @@ -615,7 +640,7 @@ } private static Bounds parseBBox(Projection conv, String miny, String minx, String maxy, String maxx) { - if (miny == null || minx == null || maxy == null || maxx == null) { + if (miny == null || minx == null || maxy == null || maxx == null || Arrays.asList(miny, minx, maxy, maxx).contains("nan")) { return null; } if (conv != null) {