id summary reporter owner description type status priority milestone component version resolution keywords cc 12037 [Alpha patch] Feedback on valid/invalid Overpass Turbo queries Don-vip team "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 Overbass 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: {{{ #!diff Index: src/org/openstreetmap/josm/actions/OverpassDownloadAction.java =================================================================== --- src/org/openstreetmap/josm/actions/OverpassDownloadAction.java (revision 8966) +++ src/org/openstreetmap/josm/actions/OverpassDownloadAction.java (working copy) @@ -33,10 +33,14 @@ 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,6 +48,7 @@ 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,7 +158,35 @@ 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 Index: src/org/openstreetmap/josm/tools/OverpassTurboQueryWizard.java =================================================================== --- src/org/openstreetmap/josm/tools/OverpassTurboQueryWizard.java (revision 8966) +++ src/org/openstreetmap/josm/tools/OverpassTurboQueryWizard.java (working copy) @@ -27,7 +27,15 @@ /** * 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,7 +52,6 @@ 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,6 +62,16 @@ } } + 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,21 +79,23 @@ * @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)); + } } Index: test/unit/org/openstreetmap/josm/tools/OverpassTurboQueryWizardTest.java =================================================================== --- test/unit/org/openstreetmap/josm/tools/OverpassTurboQueryWizardTest.java (revision 8966) +++ test/unit/org/openstreetmap/josm/tools/OverpassTurboQueryWizardTest.java (working copy) @@ -6,6 +6,7 @@ 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,9 +24,10 @@ /** * 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,9 +46,10 @@ /** * 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""); } } }}}" enhancement new normal Core overpass turbo feedback simon04