Improve search history by attaching change listener (#9794)

Author: dkokkotas

CHANGELOG.md

@@ -68,6 +68,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where it was no longer possible to connect to a shared mysql database due to an exception [#9761](https://github.com/JabRef/jabref/issues/9761)
 - We fixed the citation key generation for (`[authors]`, `[authshort]`, `[authorsAlpha]`, `authIniN`, `authEtAl`, `auth.etal`)[https://docs.jabref.org/setup/citationkeypatterns#special-field-markers] to handle `and others` properly. [koppor#626](https://github.com/koppor/jabref/issues/626)
 - We fixed the Save/save as file type shows BIBTEX_DB instead of "Bibtex library" [#9372](https://github.com/JabRef/jabref/issues/9372)
+- We fixed an issue regarding recording redundant prefixes in search history. [#9685](https://github.com/JabRef/jabref/issues/9685)
 
 ### Removed
 

src/main/java/org/jabref/gui/search/GlobalSearchBar.java

@@ -42,7 +42,6 @@
 
 import org.jabref.gui.ClipBoardManager;
 import org.jabref.gui.DialogService;
-import org.jabref.gui.Globals;
 import org.jabref.gui.JabRefFrame;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.autocompleter.AppendPersonNamesStrategy;
@@ -127,7 +126,7 @@ public GlobalSearchBar(JabRefFrame frame, StateManager stateManager, Preferences
         searchFieldTooltip.setMaxHeight(10);
         updateHintVisibility();
 
-        KeyBindingRepository keyBindingRepository = Globals.getKeyPrefs();
+        KeyBindingRepository keyBindingRepository = preferencesService.getKeyBindingRepository();
         searchField.addEventFilter(KeyEvent.KEY_PRESSED, event -> {
             Optional<KeyBinding> keyBinding = keyBindingRepository.mapToKeyBinding(event);
             if (keyBinding.isPresent()) {
@@ -211,6 +210,18 @@ public GlobalSearchBar(JabRefFrame frame, StateManager stateManager, Preferences
 
         this.stateManager.activeSearchQueryProperty().addListener((obs, oldvalue, newValue) -> newValue.ifPresent(this::updateSearchResultsForQuery));
         this.stateManager.activeDatabaseProperty().addListener((obs, oldValue, newValue) -> stateManager.activeSearchQueryProperty().get().ifPresent(this::updateSearchResultsForQuery));
+        /*
+         * The listener tracks a change on the focus property value.
+         * This happens, from active (user types a query) to inactive / focus
+         * lost (e.g., user selects an entry or triggers the search).
+         * The search history should only be filled, if focus is lost.
+         */
+        searchField.focusedProperty().addListener((obs, oldValue, newValue) -> {
+            // Focus lost can be derived by checking that there is no newValue (or the text is empty)
+            if (oldValue && !(newValue || searchField.getText().isBlank())) {
+                this.stateManager.addSearchHistory(searchField.textProperty().get());
+            }
+        });
     }
 
     private void updateSearchResultsForQuery(SearchQuery query) {
@@ -307,7 +318,6 @@ public void performSearch() {
             informUserAboutInvalidSearchQuery();
             return;
         }
-        this.stateManager.addSearchHistory(searchField.textProperty().get());
         stateManager.setSearchQuery(searchQuery);
     }
 

src/main/java/org/jabref/gui/search/SearchTextField.java

@@ -12,6 +12,7 @@ public static CustomTextField create() {
         CustomTextField textField = (CustomTextField) TextFields.createClearableTextField();
         textField.setPromptText(Localization.lang("Search") + "...");
         textField.setLeft(IconTheme.JabRefIcons.SEARCH.getGraphicNode());
+        textField.setId("searchField");
         return textField;
     }
 }

src/test/java/org/jabref/gui/search/GlobalSearchBarTest.java

@@ -0,0 +1,109 @@
+package org.jabref.gui.search;
+
+import java.util.EnumSet;
+import java.util.List;
+
+import javafx.scene.Scene;
+import javafx.scene.control.TextInputControl;
+import javafx.scene.layout.HBox;
+import javafx.stage.Stage;
+
+import org.jabref.gui.DialogService;
+import org.jabref.gui.JabRefFrame;
+import org.jabref.gui.StateManager;
+import org.jabref.gui.undo.CountingUndoManager;
+import org.jabref.gui.util.DefaultTaskExecutor;
+import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.search.rules.SearchRules;
+import org.jabref.preferences.PreferencesService;
+import org.jabref.preferences.SearchPreferences;
+import org.jabref.testutils.category.GUITest;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Answers;
+import org.testfx.api.FxRobot;
+import org.testfx.framework.junit5.ApplicationExtension;
+import org.testfx.framework.junit5.Start;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@GUITest
+@ExtendWith(ApplicationExtension.class)
+public class GlobalSearchBarTest {
+    private Stage stage;
+    private Scene scene;
+    private HBox hBox;
+
+    private GlobalSearchBar searchBar;
+    private StateManager stateManager;
+
+    @Start
+    public void onStart(Stage stage) {
+        SearchPreferences searchPreferences = mock(SearchPreferences.class);
+        when(searchPreferences.getSearchFlags()).thenReturn(EnumSet.noneOf(SearchRules.SearchFlags.class));
+        PreferencesService prefs = mock(PreferencesService.class, Answers.RETURNS_DEEP_STUBS);
+        when(prefs.getSearchPreferences()).thenReturn(searchPreferences);
+
+        stateManager = new StateManager();
+        // Need for active database, otherwise the searchField will be disabled
+        stateManager.setActiveDatabase(new BibDatabaseContext());
+
+        // Instantiate GlobalSearchBar class, so the change listener is registered
+        searchBar = new GlobalSearchBar(
+                mock(JabRefFrame.class),
+                stateManager,
+                prefs,
+                mock(CountingUndoManager.class),
+                mock(DialogService.class)
+        );
+
+        hBox = new HBox(searchBar);
+
+        scene = new Scene(hBox, 400, 400);
+        this.stage = stage;
+        stage.setScene(scene);
+
+        stage.show();
+    }
+
+    @Test
+    void recordingSearchQueriesOnFocusLostOnly(FxRobot robot) throws InterruptedException {
+        stateManager.clearSearchHistory();
+        String searchQuery = "Smith";
+        // Track the node, that the search query will be typed into
+        TextInputControl searchField = robot.lookup("#searchField").queryTextInputControl();
+
+        // The focus is on searchField node, as we click on the search box
+        var searchFieldRoboto = robot.clickOn(searchField);
+        for (char c : searchQuery.toCharArray()) {
+            searchFieldRoboto.write(String.valueOf(c));
+            Thread.sleep(401);
+            assertTrue(stateManager.getWholeSearchHistory().isEmpty());
+        }
+
+        // Set the focus to another node to trigger the listener and finally record the query.
+        DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> hBox.requestFocus());
+        List<String> lastSearchHistory = stateManager.getWholeSearchHistory().stream().toList();
+
+        assertEquals(List.of("Smith"), lastSearchHistory);
+    }
+
+    @Test
+    void emptyQueryIsNotRecorded(FxRobot robot) {
+        stateManager.clearSearchHistory();
+        String searchQuery = "";
+        TextInputControl searchField = robot.lookup("#searchField").queryTextInputControl();
+
+        var searchFieldRoboto = robot.clickOn(searchField);
+        searchFieldRoboto.write(searchQuery);
+
+        DefaultTaskExecutor.runAndWaitInJavaFXThread(() -> hBox.requestFocus());
+        List<String> lastSearchHistory = stateManager.getWholeSearchHistory().stream().toList();
+
+        assertEquals(List.of(), lastSearchHistory);
+    }
+}