[Alpha patch] Feedback on valid/invalid Overpass Turbo queries

[Don-vip]

In the "Download from Overpass" dialog, the first text field allowing to build Overpass queries does not provide real-time feedback on the validity of the query being typed.

I came up with two solutions, but none of them is good enough:

• the first one calls Overpass Turbo in JavaScript. It's too slow!
• the second one calls our Search syntax validator. It's fast, but wrong!

Does anyone have a better idea?

Here's my patch:

src/org/openstreetmap/josm/actions/OverpassDownloadAction.java

@@ -33 +33 @@
 import javax.swing.JPopupMenu;
 import javax.swing.JScrollPane;
 import javax.swing.plaf.basic.BasicArrowButton;
+import javax.swing.text.JTextComponent;
 
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask;
 import org.openstreetmap.josm.actions.downloadtasks.PostDownloadHandler;
+import org.openstreetmap.josm.actions.search.SearchAction.SearchSetting;
+import org.openstreetmap.josm.actions.search.SearchCompiler;
+import org.openstreetmap.josm.actions.search.SearchCompiler.ParseError;
 import org.openstreetmap.josm.data.Bounds;
 import org.openstreetmap.josm.data.preferences.CollectionProperty;
 import org.openstreetmap.josm.data.preferences.IntegerProperty;
@@ -44 +48 @@
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.download.DownloadDialog;
 import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.gui.widgets.AbstractTextComponentValidator;
 import org.openstreetmap.josm.gui.widgets.HistoryComboBox;
 import org.openstreetmap.josm.gui.widgets.JosmTextArea;
 import org.openstreetmap.josm.io.OverpassDownloadReader;
@@ -153 +158 @@
             final String tooltip = tr("Builds an Overpass query using the Overpass Turbo query wizard");
             overpassWizard = new HistoryComboBox();
             overpassWizard.setToolTipText(tooltip);
-            overpassWizard.getEditor().getEditorComponent().addFocusListener(disableActionsFocusListener);
+            JTextComponent editorComponent = (JTextComponent) overpassWizard.getEditor().getEditorComponent();
+            editorComponent.addFocusListener(disableActionsFocusListener);
+            editorComponent.getDocument().addDocumentListener(new AbstractTextComponentValidator(editorComponent) {
+
+                @Override
+                public void validate() {
+                    if (!isValid()) {
+                        feedbackInvalid(tr("Invalid Overpass Turbo query"));
+                    } else {
+                        feedbackValid(tooltip);
+                    }
+                }
+
+                @Override
+                public boolean isValid() {
+                    // SOLUTION 1: terribly slow
+                    //return OverpassTurboQueryWizard.getInstance().testQuery(overpassWizard.getText());
+
+                    // SOLUTION 2: fast but incorrect
+                    try {
+                        SearchSetting ss = new SearchSetting();
+                        ss.text = overpassWizard.getText();
+                        SearchCompiler.compile(ss);
+                        return true;
+                    } catch (ParseError e) {
+                        return false;
+                    }
+                }
+            });
             final JButton buildQuery = new JButton(tr("Build query"));
             buildQuery.addActionListener(new AbstractAction() {
                 @Override

src/org/openstreetmap/josm/tools/OverpassTurboQueryWizard.java

@@ -27 +27 @@
     /**
      * An exception to indicate a failed parse.
      */
-    public static class ParseException extends RuntimeException {
+    public static class ParseException extends Exception {
+
+        /**
+         * Constructs a new {@code ParseException}.
+         * @param message the error message
+         */
+        public ParseException(String message) {
+            super(message);
+        }
     }
 
     /**
@@ -44 +52 @@
 
     private OverpassTurboQueryWizard() {
         // overpass-turbo is MIT Licensed
-
         try (final Reader reader = new InputStreamReader(
                 getClass().getResourceAsStream("/data/overpass-turbo-ffs.js"), StandardCharsets.UTF_8)) {
             engine.eval("var console = {log: function(){}};");
@@ -55 +62 @@
         }
     }
 
+    private Object doConstructQuery(String search) {
+        try {
+            return ((Invocable) engine).invokeFunction("construct_query", search);
+        } catch (NoSuchMethodException e) {
+            throw new IllegalStateException(e);
+        } catch (ScriptException e) {
+            throw new RuntimeException("Failed to execute OverpassTurboQueryWizard", e);
+        }
+    }
+
     /**
      * Builds an Overpass QL from a {@link org.openstreetmap.josm.actions.search.SearchAction} like query.
      * @param search the {@link org.openstreetmap.josm.actions.search.SearchAction} like query
@@ -62 +79 @@
      * @throws ParseException when the parsing fails
      */
     public String constructQuery(String search) throws ParseException {
-        try {
-            final Object result = ((Invocable) engine).invokeFunction("construct_query", search);
-            if (result == Boolean.FALSE) {
-                throw new ParseException();
-            }
-            String query = (String) result;
-            query = Pattern.compile("^.*\\[out:json\\]", Pattern.DOTALL).matcher(query).replaceFirst("");
-            query = Pattern.compile("^out.*", Pattern.MULTILINE).matcher(query).replaceAll("out meta;");
-            query = query.replace("({{bbox}})", "");
-            return query;
-        } catch (NoSuchMethodException e) {
-            throw new IllegalStateException();
-        } catch (ScriptException e) {
-            throw new RuntimeException("Failed to execute OverpassTurboQueryWizard", e);
+        final Object result = doConstructQuery(search);
+        if (Boolean.FALSE.equals(result)) {
+            throw new ParseException("Cannot parse: " + search);
         }
+        String query = (String) result;
+        query = Pattern.compile("^.*\\[out:json\\]", Pattern.DOTALL).matcher(query).replaceFirst("");
+        query = Pattern.compile("^out.*", Pattern.MULTILINE).matcher(query).replaceAll("out meta;");
+        query = query.replace("({{bbox}})", "");
+        return query;
     }
 
+    /**
+     * Test validity of an Overpass Turbo query.
+     * @param search the {@link org.openstreetmap.josm.actions.search.SearchAction} like query
+     * @return {@code true} if the query is valid
+     */
+    public boolean testQuery(String search) {
+        return !Boolean.FALSE.equals(doConstructQuery(search));
+    }
 }

test/unit/org/openstreetmap/josm/tools/OverpassTurboQueryWizardTest.java

@@ -6 +6 @@
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.openstreetmap.josm.JOSMFixture;
+import org.openstreetmap.josm.tools.OverpassTurboQueryWizard.ParseException;
 
 /**
  * Unit tests of {@link OverpassTurboQueryWizard} class.
@@ -23 +24 @@
 
     /**
      * Test key=value.
+     * @throws ParseException in case of invalid syntax
      */
     @Test
-    public void testKeyValue() {
+    public void testKeyValue() throws ParseException {
         final String query = OverpassTurboQueryWizard.getInstance().constructQuery("amenity=drinking_water");
         assertEquals("" +
                 "[timeout:25];\n" +
@@ -44 +46 @@
 
     /**
      * Test erroneous value.
+     * @throws ParseException in case of invalid syntax
      */
-    @Test(expected = OverpassTurboQueryWizard.ParseException.class)
-    public void testErroneous() {
+    @Test(expected = ParseException.class)
+    public void testErroneous() throws ParseException {
         OverpassTurboQueryWizard.getInstance().constructQuery("foo");
     }
 }

[simon04]

Obsolete due to #18164 / r16262.