Changeset 16419 in josm for trunk/src/org

Timestamp:

2020-05-16T00:08:03+02:00 (4 years ago)

Author:

simon04

Message:

fix #19099 - PlaceSelection: search for more results (Nominatim)

After the initial search, the button changes to "Search more..."

Location:

trunk/src/org/openstreetmap/josm

Files:

• gui/download/PlaceSelection.java (modified) (9 diffs)
• io/NameFinder.java (modified) (6 diffs)

trunk/src/org/openstreetmap/josm/gui/download/PlaceSelection.java

@@ -16 +16 @@
 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 +61 @@
 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 +77 @@
     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> server = new JosmComboBox<>(SERVERS);
+    private final JosmComboBox<Server> serverComboBox = new JosmComboBox<>(SERVERS);
 
     private static class Server {
         public final String name;
-        public final String url;
+        public final BiFunction<String, Collection<SearchResult>, URL> urlFunction;
         public final String thirdcol;
         public final String fourthcol;
 
-        Server(String n, String u, String t, String f) {
+        Server(String n, BiFunction<String, Collection<SearchResult>, URL> u, String t, String f) {
             name = n;
-            url = u;
+            urlFunction = u;
             thirdcol = t;
             fourthcol = f;
@@ -102 +105 @@
 
         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 +174 @@
     }
 
+    /**
+     * 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<List<SearchResult>> dataConsumer;
         private HttpClient connection;
         private List<SearchResult> data;
         private boolean canceled;
-        private final Server useserver;
         private Exception lastException;
 
-        NameQueryTask(String searchExpression) {
+        NameQueryTask(URL url, Consumer<List<SearchResult>> 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 +272 @@
                 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 +289 @@
                 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 +300 @@
                 if (!canceled) {
                     OsmTransferException ex = new OsmTransferException(e);
-                    ex.setUrl(urlString);
+                    ex.setUrl(url.toString());
                     lastException = ex;
                 }
@@ -309 +333 @@
             }
             fireTableDataChanged();
+        }
+
+        public void addData(List<SearchResult> data) {
+            this.data.addAll(data);
+            fireTableDataChanged();
+        }
+
+        public List<SearchResult> getData() {
+            return Collections.unmodifiableList(data);
         }
 

trunk/src/org/openstreetmap/josm/io/NameFinder.java

@@ -6 +6 @@
 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 +56 @@
 
     /**
+     * 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 <a href="https://nominatim.org/release-docs/develop/api/Search/#result-limitation">Result limitation in Nominatim Documentation</a>
+     */
+    public static URL buildNominatimURL(String searchExpression, Collection<SearchResult> 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 +92 @@
      */
     public static List<SearchResult> queryNominatim(final String searchExpression) throws IOException {
-        return query(new URL(NOMINATIM_URL_PROP.get() + Utils.encodeUrl(searchExpression)));
+        return query(buildNominatimURL(searchExpression));
     }
 
@@ -108 +143 @@
         private Bounds bounds;
         private PrimitiveId osmId;
+        private long placeId;
 
         /**
@@ -179 +215 @@
         public final PrimitiveId getOsmId() {
             return osmId;
+        }
+
+        /**
+         * Returns the Nominatim place id.
+         * @return the Nominatim place id
+         */
+        public long getPlaceId() {
+            return placeId;
         }
 
@@ -249 +293 @@
                         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);
                 }