Skip to content

Commit dc6ea63

Browse files
author
taylor.smock
committed
Fix #22733: Add UI feedback for url guessing, account for nan responses from servers
git-svn-id: https://josm.openstreetmap.de/svn/trunk@18780 0c6e7542-c601-0410-84e7-c038aed88b3b
1 parent adc02c3 commit dc6ea63

File tree

2 files changed

+109
-39
lines changed

2 files changed

+109
-39
lines changed

‎src/org/openstreetmap/josm/gui/preferences/imagery/AddWMSLayerPanel.java‎

Lines changed: 75 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
import static org.openstreetmap.josm.tools.I18n.tr;
55

6+
import java.awt.GraphicsEnvironment;
7+
import java.awt.event.ActionEvent;
68
import java.awt.event.ActionListener;
79
import java.io.IOException;
810
import java.net.MalformedURLException;
@@ -15,6 +17,7 @@
1517
import javax.swing.JButton;
1618
import javax.swing.JCheckBox;
1719
import javax.swing.JComboBox;
20+
import javax.swing.JComponent;
1821
import javax.swing.JLabel;
1922
import javax.swing.JOptionPane;
2023
import javax.swing.JScrollPane;
@@ -23,7 +26,12 @@
2326
import org.openstreetmap.josm.data.imagery.ImageryInfo;
2427
import org.openstreetmap.josm.data.imagery.ImageryInfo.ImageryType;
2528
import org.openstreetmap.josm.data.imagery.LayerDetails;
29+
import org.openstreetmap.josm.gui.MainApplication;
30+
import org.openstreetmap.josm.gui.PleaseWaitRunnable;
2631
import org.openstreetmap.josm.gui.bbox.SlippyMapBBoxChooser;
32+
import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
33+
import org.openstreetmap.josm.gui.progress.ProgressMonitor;
34+
import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor;
2735
import org.openstreetmap.josm.gui.util.GuiHelper;
2836
import org.openstreetmap.josm.gui.widgets.JosmTextArea;
2937
import org.openstreetmap.josm.io.imagery.WMSImagery;
@@ -81,37 +89,7 @@ public AddWMSLayerPanel() {
8189
add(new JLabel(tr("{0} Enter name for this layer", "7.")), GBC.eol());
8290
add(name, GBC.eop().fill(GBC.HORIZONTAL));
8391

84-
getLayers.addActionListener(e -> {
85-
try {
86-
wms = new WMSImagery(Utils.strip(rawUrl.getText()), getCommonHeaders());
87-
tree.updateTree(wms);
88-
Collection<String> wmsFormats = wms.getFormats();
89-
formats.setModel(new DefaultComboBoxModel<>(wmsFormats.toArray(new String[0])));
90-
formats.setSelectedItem(wms.getPreferredFormat());
91-
} catch (MalformedURLException | InvalidPathException ex1) {
92-
Logging.log(Logging.LEVEL_ERROR, ex1);
93-
JOptionPane.showMessageDialog(getParent(), tr("Invalid service URL."),
94-
tr("WMS Error"), JOptionPane.ERROR_MESSAGE);
95-
} catch (IOException ex2) {
96-
Logging.log(Logging.LEVEL_ERROR, ex2);
97-
JOptionPane.showMessageDialog(getParent(), tr("Could not retrieve WMS layer list."),
98-
tr("WMS Error"), JOptionPane.ERROR_MESSAGE);
99-
} catch (WMSImagery.WMSGetCapabilitiesException ex3) {
100-
String incomingData = ex3.getIncomingData() != null ? ex3.getIncomingData().trim() : "";
101-
String title = tr("WMS Error");
102-
StringBuilder message = new StringBuilder(tr("Could not parse WMS layer list."));
103-
Logging.log(Logging.LEVEL_ERROR, "Could not parse WMS layer list. Incoming data:\n"+incomingData, ex3);
104-
if ((incomingData.startsWith("<html>") || incomingData.startsWith("<HTML>"))
105-
&& (incomingData.endsWith("</html>") || incomingData.endsWith("</HTML>"))) {
106-
GuiHelper.notifyUserHtmlError(this, title, message.toString(), incomingData);
107-
} else {
108-
if (ex3.getMessage() != null) {
109-
message.append('\n').append(ex3.getMessage());
110-
}
111-
JOptionPane.showMessageDialog(getParent(), message.toString(), title, JOptionPane.ERROR_MESSAGE);
112-
}
113-
}
114-
});
92+
getLayers.addActionListener(e -> MainApplication.worker.execute(new GetCapabilitiesRunnable(e)));
11593

11694
ActionListener availabilityManagerAction = a -> {
11795
setDefaultLayers.setEnabled(endpoint.isSelected());
@@ -225,4 +203,70 @@ protected boolean isImageryValid() {
225203
return !getWmsUrl().isEmpty();
226204
}
227205
}
206+
207+
/**
208+
* Perform the get WMS layers network calls in a separate thread, mostly to allow for cancellation
209+
*/
210+
private class GetCapabilitiesRunnable extends PleaseWaitRunnable {
211+
private final ActionEvent actionEvent;
212+
213+
GetCapabilitiesRunnable(ActionEvent actionEvent) {
214+
super(tr("Trying WMS urls"), GraphicsEnvironment.isHeadless() ? NullProgressMonitor.INSTANCE : new PleaseWaitProgressMonitor(),
215+
false);
216+
this.actionEvent = actionEvent;
217+
}
218+
219+
@Override
220+
protected void cancel() {
221+
// We don't really have something we can do -- we use the monitor to pass the cancel state on
222+
}
223+
224+
@Override
225+
protected void realRun() {
226+
if (actionEvent.getSource() instanceof JComponent) {
227+
GuiHelper.runInEDT(() -> ((JComponent) actionEvent.getSource()).setEnabled(false));
228+
}
229+
ProgressMonitor monitor = getProgressMonitor();
230+
try {
231+
wms = new WMSImagery(Utils.strip(rawUrl.getText()), getCommonHeaders(), monitor);
232+
} catch (MalformedURLException | InvalidPathException ex1) {
233+
Logging.log(Logging.LEVEL_ERROR, ex1);
234+
GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(getParent(), tr("Invalid service URL."),
235+
tr("WMS Error"), JOptionPane.ERROR_MESSAGE));
236+
} catch (IOException ex2) {
237+
Logging.log(Logging.LEVEL_ERROR, ex2);
238+
GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(getParent(), tr("Could not retrieve WMS layer list."),
239+
tr("WMS Error"), JOptionPane.ERROR_MESSAGE));
240+
} catch (WMSImagery.WMSGetCapabilitiesException ex3) {
241+
String incomingData = ex3.getIncomingData() != null ? ex3.getIncomingData().trim() : "";
242+
String title = tr("WMS Error");
243+
StringBuilder message = new StringBuilder(tr("Could not parse WMS layer list."));
244+
Logging.log(Logging.LEVEL_ERROR, "Could not parse WMS layer list. Incoming data:\n"+incomingData, ex3);
245+
if ((incomingData.startsWith("<html>") || incomingData.startsWith("<HTML>"))
246+
&& (incomingData.endsWith("</html>") || incomingData.endsWith("</HTML>"))) {
247+
GuiHelper.runInEDT(() -> GuiHelper.notifyUserHtmlError(AddWMSLayerPanel.this, title, message.toString(), incomingData));
248+
} else {
249+
if (ex3.getMessage() != null) {
250+
message.append('\n').append(ex3.getMessage());
251+
}
252+
GuiHelper.runInEDT(() -> JOptionPane.showMessageDialog(getParent(), message.toString(), title, JOptionPane.ERROR_MESSAGE));
253+
}
254+
}
255+
}
256+
257+
@Override
258+
protected void finish() {
259+
if (wms != null) {
260+
GuiHelper.runInEDT(() -> {
261+
tree.updateTree(wms);
262+
Collection<String> wmsFormats = wms.getFormats();
263+
formats.setModel(new DefaultComboBoxModel<>(wmsFormats.toArray(new String[0])));
264+
formats.setSelectedItem(wms.getPreferredFormat());
265+
});
266+
}
267+
if (actionEvent.getSource() instanceof JComponent) {
268+
GuiHelper.runInEDT(() -> ((JComponent) actionEvent.getSource()).setEnabled(true));
269+
}
270+
}
271+
}
228272
}

‎src/org/openstreetmap/josm/io/imagery/WMSImagery.java‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.net.URL;
1212
import java.nio.file.InvalidPathException;
1313
import java.util.ArrayList;
14+
import java.util.Arrays;
1415
import java.util.Collection;
1516
import java.util.Collections;
1617
import java.util.HashSet;
@@ -35,6 +36,8 @@
3536
import org.openstreetmap.josm.data.imagery.LayerDetails;
3637
import org.openstreetmap.josm.data.projection.Projection;
3738
import org.openstreetmap.josm.data.projection.Projections;
39+
import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
40+
import org.openstreetmap.josm.gui.progress.ProgressMonitor;
3841
import org.openstreetmap.josm.io.CachedFile;
3942
import org.openstreetmap.josm.tools.Logging;
4043
import org.openstreetmap.josm.tools.Utils;
@@ -150,20 +153,44 @@ public WMSImagery(String url) throws IOException, WMSGetCapabilitiesException {
150153
* @throws InvalidPathException if a Path object cannot be constructed for the capabilities cached file
151154
*/
152155
public WMSImagery(String url, Map<String, String> headers) throws IOException, WMSGetCapabilitiesException {
156+
this(url, headers, NullProgressMonitor.INSTANCE);
157+
}
158+
159+
/**
160+
* Make getCapabilities request towards given URL using headers
161+
* @param url service url
162+
* @param headers HTTP headers to be sent with request
163+
* @param monitor Feedback for which URL we are currently trying, the integer is the <i>total number of urls</i> we are going to try
164+
* @throws IOException when connection error when fetching get capabilities document
165+
* @throws WMSGetCapabilitiesException when there are errors when parsing get capabilities document
166+
* @throws InvalidPathException if a Path object cannot be constructed for the capabilities cached file
167+
* @since xxx
168+
*/
169+
public WMSImagery(String url, Map<String, String> headers, ProgressMonitor monitor)
170+
throws IOException, WMSGetCapabilitiesException {
153171
if (headers != null) {
154172
this.headers.putAll(headers);
155173
}
156174

157175
IOException savedExc = null;
158176
String workingAddress = null;
177+
final String[] baseAdditions = {
178+
normalizeUrl(url),
179+
url,
180+
url + CAPABILITIES_QUERY_STRING,
181+
};
182+
final String[] versionAdditions = {"", "&VERSION=1.3.0", "&VERSION=1.1.1"};
183+
final int totalNumberOfUrlsToTry = baseAdditions.length * versionAdditions.length;
184+
monitor.setTicksCount(totalNumberOfUrlsToTry);
159185
url_search:
160-
for (String z: new String[]{
161-
normalizeUrl(url),
162-
url,
163-
url + CAPABILITIES_QUERY_STRING,
164-
}) {
165-
for (String ver: new String[]{"", "&VERSION=1.3.0", "&VERSION=1.1.1"}) {
186+
for (String z : baseAdditions) {
187+
for (String ver : versionAdditions) {
188+
if (monitor.isCanceled()) {
189+
break url_search;
190+
}
166191
try {
192+
monitor.setCustomText(z + ver);
193+
monitor.worked(1);
167194
attemptGetCapabilities(z + ver);
168195
workingAddress = z;
169196
calculateChildren();
@@ -192,7 +219,6 @@ public WMSImagery(String url, Map<String, String> headers) throws IOException, W
192219
}
193220
}
194221
}
195-
196222
if (savedExc != null) {
197223
throw savedExc;
198224
}
@@ -615,7 +641,7 @@ private Bounds parseBoundingBox(XMLStreamReader reader, Projection conv) {
615641
}
616642

617643
private static Bounds parseBBox(Projection conv, String miny, String minx, String maxy, String maxx) {
618-
if (miny == null || minx == null || maxy == null || maxx == null) {
644+
if (miny == null || minx == null || maxy == null || maxx == null || Arrays.asList(miny, minx, maxy, maxx).contains("nan")) {
619645
return null;
620646
}
621647
if (conv != null) {

0 commit comments

Comments
 (0)