Index: /trunk/src/org/openstreetmap/josm/gui/download/PlaceSelection.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/download/PlaceSelection.java (revision 16418) +++ /trunk/src/org/openstreetmap/josm/gui/download/PlaceSelection.java (revision 16419) @@ -16,7 +16,11 @@ import java.text.DecimalFormat; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.StringTokenizer; +import java.util.function.BiFunction; +import java.util.function.Consumer; import javax.swing.AbstractAction; @@ -57,5 +61,4 @@ import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.Logging; -import org.openstreetmap.josm.tools.Utils; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; @@ -74,17 +77,17 @@ private DownloadDialog parent; private static final Server[] SERVERS = { - new Server("Nominatim", NameFinder.NOMINATIM_URL, tr("Class Type"), tr("Bounds")) + new Server("Nominatim", NameFinder::buildNominatimURL, tr("Class Type"), tr("Bounds")) }; - private final JosmComboBox server = new JosmComboBox<>(SERVERS); + private final JosmComboBox serverComboBox = new JosmComboBox<>(SERVERS); private static class Server { public final String name; - public final String url; + public final BiFunction, URL> urlFunction; public final String thirdcol; public final String fourthcol; - Server(String n, String u, String t, String f) { + Server(String n, BiFunction, URL> u, String t, String f) { name = n; - url = u; + urlFunction = u; thirdcol = t; fourthcol = f; @@ -102,9 +105,9 @@ lpanel.add(new JLabel(tr("Choose the server for searching:")), GBC.std(0, 0).weight(0, 0).insets(0, 0, 5, 0)); - lpanel.add(server, GBC.std(1, 0).fill(GBC.HORIZONTAL)); + lpanel.add(serverComboBox, GBC.std(1, 0).fill(GBC.HORIZONTAL)); String s = Config.getPref().get("namefinder.server", SERVERS[0].name); for (int i = 0; i < SERVERS.length; ++i) { if (SERVERS[i].name.equals(s)) { - server.setSelectedIndex(i); + serverComboBox.setSelectedIndex(i); } } @@ -171,57 +174,82 @@ } + /** + * Action to perform initial search, and (if query is unchanged) load more results. + */ class SearchAction extends AbstractAction implements DocumentListener { + String lastSearchExpression; + boolean isSearchMore; + SearchAction() { - putValue(NAME, tr("Search...")); new ImageProvider("dialogs", "search").getResource().attachImageIcon(this, true); - putValue(SHORT_DESCRIPTION, tr("Click to start searching for places")); - updateEnabledState(); + updateState(); } @Override public void actionPerformed(ActionEvent e) { - if (!isEnabled() || cbSearchExpression.getText().trim().isEmpty()) + String searchExpression = cbSearchExpression.getText(); + if (!isEnabled() || searchExpression.trim().isEmpty()) return; cbSearchExpression.addCurrentItemToHistory(); Config.getPref().putList(HISTORY_KEY, cbSearchExpression.getHistory()); - NameQueryTask task = new NameQueryTask(cbSearchExpression.getText()); + Server server = (Server) serverComboBox.getSelectedItem(); + URL url = server.urlFunction.apply(searchExpression, isSearchMore ? model.getData() : Collections.emptyList()); + NameQueryTask task = new NameQueryTask(url, data -> { + if (isSearchMore) { + model.addData(data); + } else { + model.setData(data); + } + Config.getPref().put("namefinder.server", server.name); + columnmodel.setHeadlines(server.thirdcol, server.fourthcol); + lastSearchExpression = searchExpression; + updateState(); + }); MainApplication.worker.submit(task); } - protected final void updateEnabledState() { - setEnabled(!cbSearchExpression.getText().trim().isEmpty()); + protected final void updateState() { + String searchExpression = cbSearchExpression.getText(); + setEnabled(!searchExpression.trim().isEmpty()); + isSearchMore = Objects.equals(lastSearchExpression, searchExpression) && !model.getData().isEmpty(); + if (isSearchMore) { + putValue(NAME, tr("Search more...")); + putValue(SHORT_DESCRIPTION, tr("Click to search for more places")); + } else { + putValue(NAME, tr("Search...")); + putValue(SHORT_DESCRIPTION, tr("Click to start searching for places")); + } } @Override public void changedUpdate(DocumentEvent e) { - updateEnabledState(); + updateState(); } @Override public void insertUpdate(DocumentEvent e) { - updateEnabledState(); + updateState(); } @Override public void removeUpdate(DocumentEvent e) { - updateEnabledState(); - } - } - - class NameQueryTask extends PleaseWaitRunnable { - - private final String searchExpression; + updateState(); + } + } + + static class NameQueryTask extends PleaseWaitRunnable { + + private final URL url; + private final Consumer> dataConsumer; private HttpClient connection; private List data; private boolean canceled; - private final Server useserver; private Exception lastException; - NameQueryTask(String searchExpression) { + NameQueryTask(URL url, Consumer> dataConsumer) { super(tr("Querying name server"), false /* don't ignore exceptions */); - this.searchExpression = searchExpression; - useserver = (Server) server.getSelectedItem(); - Config.getPref().put("namefinder.server", useserver.name); + this.url = url; + this.dataConsumer = dataConsumer; } @@ -244,15 +272,11 @@ return; } - columnmodel.setHeadlines(useserver.thirdcol, useserver.fourthcol); - model.setData(this.data); + dataConsumer.accept(data); } @Override protected void realRun() throws SAXException, IOException, OsmTransferException { - String urlString = useserver.url+Utils.encodeUrl(searchExpression); - try { getProgressMonitor().indeterminateSubTask(tr("Querying name server ...")); - URL url = new URL(urlString); synchronized (this) { connection = HttpClient.create(url); @@ -265,5 +289,5 @@ if (!canceled) { // Nominatim sometimes returns garbage, see #5934, #10643 - Logging.log(Logging.LEVEL_WARN, tr("Error occurred with query ''{0}'': ''{1}''", urlString, e.getMessage()), e); + Logging.log(Logging.LEVEL_WARN, tr("Error occurred with query ''{0}'': ''{1}''", url, e.getMessage()), e); GuiHelper.runInEDTAndWait(() -> HelpAwareOptionPane.showOptionDialog( MainApplication.getMainFrame(), @@ -276,5 +300,5 @@ if (!canceled) { OsmTransferException ex = new OsmTransferException(e); - ex.setUrl(urlString); + ex.setUrl(url.toString()); lastException = ex; } @@ -309,4 +333,13 @@ } fireTableDataChanged(); + } + + public void addData(List data) { + this.data.addAll(data); + fireTableDataChanged(); + } + + public List getData() { + return Collections.unmodifiableList(data); } Index: /trunk/src/org/openstreetmap/josm/io/NameFinder.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/NameFinder.java (revision 16418) +++ /trunk/src/org/openstreetmap/josm/io/NameFinder.java (revision 16419) @@ -6,8 +6,13 @@ import java.io.IOException; import java.io.Reader; +import java.io.UncheckedIOException; +import java.net.MalformedURLException; import java.net.URL; +import java.util.Collection; import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; import javax.xml.parsers.ParserConfigurationException; @@ -51,4 +56,34 @@ /** + * Builds the Nominatim URL for performing the given search + * @param searchExpression the Nominatim query + * @return the Nominatim URL + */ + public static URL buildNominatimURL(String searchExpression) { + return buildNominatimURL(searchExpression, Collections.emptyList()); + } + + /** + * Builds the Nominatim URL for performing the given search and excluding the results (of a previous search) + * @param searchExpression the Nominatim query + * @param excludeResults the results to exclude + * @return the Nominatim URL + * @see Result limitation in Nominatim Documentation + */ + public static URL buildNominatimURL(String searchExpression, Collection excludeResults) { + try { + final String excludeString = excludeResults.isEmpty() + ? "" + : excludeResults.stream() + .map(SearchResult::getPlaceId) + .map(String::valueOf) + .collect(Collectors.joining(",", "&exclude_place_ids=", "")); + return new URL(NOMINATIM_URL_PROP.get() + Utils.encodeUrl(searchExpression) + excludeString); + } catch (MalformedURLException ex) { + throw new UncheckedIOException(ex); + } + } + + /** * Performs a Nominatim search. * @param searchExpression Nominatim search expression @@ -57,5 +92,5 @@ */ public static List queryNominatim(final String searchExpression) throws IOException { - return query(new URL(NOMINATIM_URL_PROP.get() + Utils.encodeUrl(searchExpression))); + return query(buildNominatimURL(searchExpression)); } @@ -108,4 +143,5 @@ private Bounds bounds; private PrimitiveId osmId; + private long placeId; /** @@ -179,4 +215,12 @@ public final PrimitiveId getOsmId() { return osmId; + } + + /** + * Returns the Nominatim place id. + * @return the Nominatim place id + */ + public long getPlaceId() { + return placeId; } @@ -249,4 +293,6 @@ currentResult.osmId = new SimplePrimitiveId(Long.parseLong(osmId), OsmPrimitiveType.from(osmType)); } + currentResult.placeId = Optional.ofNullable(atts.getValue("place_id")).filter(s -> !s.isEmpty()) + .map(Long::parseLong).orElse(0L); data.add(currentResult); }