Add query validation for web search

src/main/java/org/jabref/gui/Base.css

@@ -1236,3 +1236,7 @@ TextFlow * {
 .fontsizeSpinner{
     -fx-pref-width: 5em;
 }
+
+.text-field:invalid {
+    -fx-background-color: rgba(240, 128, 128, 0.5);
+}

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -4,7 +4,6 @@
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -1040,11 +1039,10 @@ public void addTab(LibraryTab libraryTab, boolean raisePanel) {
     }
 
     private void trackOpenNewDatabase(LibraryTab libraryTab) {
-        Map<String, String> properties = new HashMap<>();
-        Map<String, Double> measurements = new HashMap<>();
-        measurements.put("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount());
-
-        Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("OpenNewDatabase", properties, measurements));
+        Globals.getTelemetryClient().ifPresent(client -> client.trackEvent(
+                "OpenNewDatabase",
+                Map.of(),
+                Map.of("NumberOfEntries", (double) libraryTab.getBibDatabaseContext().getDatabase().getEntryCount())));
     }
 
     public LibraryTab addTab(BibDatabaseContext databaseContext, boolean raisePanel) {

src/main/java/org/jabref/gui/actions/JabRefAction.java

@@ -1,6 +1,5 @@
 package org.jabref.gui.actions;
 
-import java.util.HashMap;
 import java.util.Map;
 
 import javafx.beans.binding.Bindings;
@@ -77,10 +76,9 @@ private void trackExecute(String actionName) {
     }
 
     private void trackUserActionSource(String actionName, Sources source) {
-        Map<String, String> properties = new HashMap<>();
-        Map<String, Double> measurements = new HashMap<>();
-        properties.put("Source", source.toString());
-
-        Globals.getTelemetryClient().ifPresent(telemetryClient -> telemetryClient.trackEvent(actionName, properties, measurements));
+        Globals.getTelemetryClient().ifPresent(telemetryClient -> telemetryClient.trackEvent(
+                actionName,
+                Map.of("Source", source.toString()),
+                Map.of()));
     }
 }

src/main/java/org/jabref/gui/importer/fetcher/WebSearchPane.java

@@ -1,10 +1,12 @@
 package org.jabref.gui.importer.fetcher;
 
+import javafx.css.PseudoClass;
 import javafx.geometry.Pos;
 import javafx.scene.Node;
 import javafx.scene.control.Button;
 import javafx.scene.control.ComboBox;
 import javafx.scene.control.TextField;
+import javafx.scene.control.Tooltip;
 import javafx.scene.input.KeyCode;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.Priority;
@@ -31,6 +33,8 @@
 
 public class WebSearchPane extends SidePaneComponent {
 
+    private static final PseudoClass QUERY_INVALID = PseudoClass.getPseudoClass("invalid");
+
     private final PreferencesService preferences;
     private final WebSearchPaneViewModel viewModel;
 
@@ -75,6 +79,16 @@ protected Node createContentPane() {
         query.getStyleClass().add("searchBar");
 
         viewModel.queryProperty().bind(query.textProperty());
+        EasyBind.subscribe(viewModel.queryValidationStatus().validProperty(),
+                valid -> {
+                    if (!valid && viewModel.queryValidationStatus().getHighestMessage().isPresent()) {
+                        query.setTooltip(new Tooltip(viewModel.queryValidationStatus().getHighestMessage().get().getMessage()));
+                        query.pseudoClassStateChanged(QUERY_INVALID, true);
+                    } else {
+                        query.setTooltip(null);
+                        query.pseudoClassStateChanged(QUERY_INVALID, false);
+                    }
+                });
 
         // Allows to trigger search on pressing enter
         query.setOnKeyPressed(event -> {

src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java

@@ -1,5 +1,6 @@
 package org.jabref.gui.importer.fetcher;
 
+import java.util.Map;
 import java.util.SortedSet;
 
 import javafx.beans.property.ListProperty;
@@ -12,6 +13,7 @@
 import javafx.collections.ObservableList;
 
 import org.jabref.gui.DialogService;
+import org.jabref.gui.Globals;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.importer.ImportEntriesDialog;
 import org.jabref.gui.util.BackgroundTask;
@@ -23,6 +25,16 @@
 import org.jabref.preferences.PreferencesService;
 
 import com.tobiasdiez.easybind.EasyBind;
+import de.saxsys.mvvmfx.utils.validation.FunctionBasedValidator;
+import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
+import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
+import de.saxsys.mvvmfx.utils.validation.Validator;
+import org.apache.lucene.queryparser.flexible.core.QueryNodeParseException;
+import org.apache.lucene.queryparser.flexible.core.parser.SyntaxParser;
+import org.apache.lucene.queryparser.flexible.standard.parser.ParseException;
+import org.apache.lucene.queryparser.flexible.standard.parser.StandardSyntaxParser;
+
+import static org.jabref.logic.importer.fetcher.transformers.AbstractQueryTransformer.NO_EXPLICIT_FIELD;
 
 public class WebSearchPaneViewModel {
 
@@ -32,6 +44,9 @@ public class WebSearchPaneViewModel {
     private final DialogService dialogService;
     private final StateManager stateManager;
 
+    private final Validator searchQueryValidator;
+    private final SyntaxParser parser = new StandardSyntaxParser();
+
     public WebSearchPaneViewModel(PreferencesService preferencesService, DialogService dialogService, StateManager stateManager) {
         this.dialogService = dialogService;
         this.stateManager = stateManager;
@@ -50,6 +65,29 @@ public WebSearchPaneViewModel(PreferencesService preferencesService, DialogServi
             int newIndex = fetchers.indexOf(newFetcher);
             preferencesService.storeSidePanePreferences(preferencesService.getSidePanePreferences().withWebSearchFetcherSelected(newIndex));
         });
+
+        searchQueryValidator = new FunctionBasedValidator<>(
+                query,
+                queryText -> {
+                    if (StringUtil.isBlank(queryText)) {
+                        // in case user did not enter something, it is treated as valid (to avoid UI WTFs)
+                        return null;
+                    }
+                    try {
+                        parser.parse(queryText, NO_EXPLICIT_FIELD);
+                        return null;
+                    } catch (ParseException e) {
+                        String element = e.currentToken.image;
+                        int position = e.currentToken.beginColumn;
+                        if (element == null) {
+                            return ValidationMessage.error(Localization.lang("Invalid query. Check position %0.", position));
+                        } else {
+                            return ValidationMessage.error(Localization.lang("Invalid query element '%0' at position %1", element, position));
+                        }
+                    } catch (QueryNodeParseException e) {
+                        return ValidationMessage.error("");
+                    }
+                });
     }
 
     public ObservableList<SearchBasedFetcher> getFetchers() {
@@ -77,25 +115,30 @@ public StringProperty queryProperty() {
     }
 
     public void search() {
-        if (StringUtil.isBlank(getQuery())) {
+        String query = getQuery().trim();
+        if (StringUtil.isBlank(query)) {
             dialogService.notify(Localization.lang("Please enter a search string"));
             return;
         }
-
         if (stateManager.getActiveDatabase().isEmpty()) {
             dialogService.notify(Localization.lang("Please open or start a new library before searching"));
             return;
         }
 
         SearchBasedFetcher activeFetcher = getSelectedFetcher();
+        Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("search", Map.of("fetcher", activeFetcher.getName()), Map.of()));
 
         BackgroundTask<ParserResult> task;
-        task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(getQuery().trim())))
-                             .withInitialMessage(Localization.lang("Processing %0", getQuery().trim()));
+        task = BackgroundTask.wrap(() -> new ParserResult(activeFetcher.performSearch(query)))
+                             .withInitialMessage(Localization.lang("Processing %0", query));
         task.onFailure(dialogService::showErrorDialogAndWait);
 
         ImportEntriesDialog dialog = new ImportEntriesDialog(stateManager.getActiveDatabase().get(), task);
         dialog.setTitle(activeFetcher.getName());
         dialogService.showCustomDialogAndWait(dialog);
     }
+
+    public ValidationStatus queryValidationStatus() {
+        return searchQueryValidator.getValidationStatus();
+    }
 }

src/main/java/org/jabref/logic/importer/SearchBasedFetcher.java

@@ -36,12 +36,15 @@ default List<BibEntry> performSearch(String searchQuery) throws FetcherException
         if (searchQuery.isBlank()) {
             return Collections.emptyList();
         }
-        SyntaxParser parser = new StandardSyntaxParser();
 
+        SyntaxParser parser = new StandardSyntaxParser();
+        QueryNode queryNode;
         try {
-            return this.performSearch(parser.parse(searchQuery, NO_EXPLICIT_FIELD));
+            queryNode = parser.parse(searchQuery, NO_EXPLICIT_FIELD);
         } catch (QueryNodeParseException e) {
             throw new FetcherException("An error occurred when parsing the query");
         }
+
+        return this.performSearch(queryNode);
     }
 }

src/main/resources/l10n/JabRef_en.properties

@@ -911,6 +911,8 @@ You\ must\ restart\ JabRef\ for\ this\ to\ come\ into\ effect.=You must restart
 The\ following\ fetchers\ are\ available\:=The following fetchers are available:
 Could\ not\ find\ fetcher\ '%0'=Could not find fetcher '%0'
 Running\ query\ '%0'\ with\ fetcher\ '%1'.=Running query '%0' with fetcher '%1'.
+Invalid\ query.\ Check\ position\ %0.=Invalid query. Check position %0.
+Invalid\ query\ element\ '%0'\ at\ position\ %1=Invalid query element '%0' at position %1
 
 Move\ file=Move file
 Rename\ file=Rename file

src/test/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModelTest.java

@@ -0,0 +1,60 @@
+package org.jabref.gui.importer.fetcher;
+
+import org.jabref.gui.DialogService;
+import org.jabref.gui.StateManager;
+import org.jabref.preferences.PreferencesService;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+
+class WebSearchPaneViewModelTest {
+
+    private PreferencesService preferencesService;
+    private DialogService dialogService;
+    private StateManager stateManager;
+    private WebSearchPaneViewModel viewModel;
+
+    @BeforeEach
+    void setUp() {
+        preferencesService = Mockito.mock(PreferencesService.class, RETURNS_DEEP_STUBS);
+        dialogService = Mockito.mock(DialogService.class);
+        stateManager = Mockito.mock(StateManager.class);
+
+        viewModel = new WebSearchPaneViewModel(preferencesService, dialogService, stateManager);
+    }
+
+    @Test
+    void queryConsistingOfASingleAndIsNotValid() {
+        viewModel.queryProperty().setValue("AND");
+        assertFalse(viewModel.queryValidationStatus().validProperty().getValue());
+    }
+
+    @Test
+    void falseQueryValidationStatus() {
+        viewModel.queryProperty().setValue("Miami !Beach AND OR Blue");
+        assertFalse(viewModel.queryValidationStatus().validProperty().getValue());
+    }
+
+     @Test
+     void correctQueryValidationStatus() {
+        viewModel.queryProperty().setValue("Miami AND Beach OR Houston AND Texas");
+        assertTrue(viewModel.queryValidationStatus().validProperty().getValue());
+    }
+
+    @Test
+    void notFalseQueryValidationStatus() {
+        viewModel.queryProperty().setValue("Miami !Beach AND OR Blue");
+        assertTrue(viewModel.queryValidationStatus().validProperty().not().getValue());
+    }
+
+    @Test
+    void notCorrectQueryValidationStatus() {
+        viewModel.queryProperty().setValue("Miami AND Beach OR Houston AND Texas");
+        assertFalse(viewModel.queryValidationStatus().validProperty().not().getValue());
+    }
+}