From 37c8d2c1d693d23996e85992c60c2a820fafea01 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Tue, 8 Apr 2025 23:01:50 +0800 Subject: [PATCH 01/41] feat: add toggle functionality for journal abbreviation lists This feature allows users to enable/disable specific journal abbreviation lists, including both the built-in list and external CSV files, without removing them from configuration. - Added toggle controls in UI with visual indicators for enabled/disabled states - Implemented filtering of abbreviations based on source enabled state - Ensured toggle states persist between application sessions - Optimized performance with efficient repository loading - Added comprehensive test coverage for new functionality Closes JabRef/jabref#12468 --- CHANGELOG.md | 4 +- .../java/org/jabref/gui/frame/MainMenu.java | 8 +- .../jabref/gui/journals/AbbreviateAction.java | 42 ++- .../gui/journals/UndoableUnabbreviator.java | 11 +- .../journals/AbbreviationsFileViewModel.java | 172 +++++++---- .../journals/JournalAbbreviationsTab.java | 139 ++++++++- .../JournalAbbreviationsTabViewModel.java | 48 +++- .../journals/JournalAbbreviationLoader.java | 45 ++- .../JournalAbbreviationPreferences.java | 101 ++++++- .../JournalAbbreviationRepository.java | 160 +++++++++-- .../preferences/JabRefCliPreferences.java | 31 +- .../gui/journals/AbbreviateActionTest.java | 98 +++++++ .../JournalAbbreviationsViewModelTabTest.java | 120 +++++++- .../JournalAbbreviationRepositoryTest.java | 268 ++++++++++++++++-- 14 files changed, 1107 insertions(+), 140 deletions(-) create mode 100644 src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cd61cdab29..342beb448ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added a new functionality where author names having multiple spaces in-between will be considered as separate user block as it does for " and ". [#12701](https://github.com/JabRef/jabref/issues/12701) - We enhanced support for parsing XMP metadata from PDF files. [#12829](https://github.com/JabRef/jabref/issues/12829) - We added a "Preview" header in the JStyles tab in the "Select style" dialog, to make it consistent with the CSL styles tab. [#12838](https://github.com/JabRef/jabref/pull/12838) -- We added path validation to file directories in library properties dialog. [#11840](https://github.com/JabRef/jabref/issues/11840) +- We added ability to toggle journal abbreviation lists (including built-in and external CSV files) on/off in preferences. [#{Issue Number}](https://github.com/JabRef/jabref/pull/{Issue Number}) ### Changed @@ -62,11 +62,9 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - A tooltip now appears after 300ms (instead of 2s). [#12649](https://github.com/JabRef/jabref/issues/12649) - We improved search in preferences and keybindings. [#12647](https://github.com/JabRef/jabref/issues/12647) - We improved the performance of the LibreOffice integration when inserting CSL citations/bibliography. [#12851](https://github.com/JabRef/jabref/pull/12851) -- 'Affected fields' and 'Do not wrap when saving' are now displayed as tags. [#12550](https://github.com/JabRef/jabref/issues/12550) ### Fixed -- We fixed an issue where warning signs were improperly positioned next to text fields containing capital letters. [#12884](https://github.com/JabRef/jabref/issues/12884) - We fixed an issue where the drag'n'drop functionality in entryeditor did not work [#12561](https://github.com/JabRef/jabref/issues/12561) - We fixed an issue where the F4 shortcut key did not work without opening the right-click context menu. [#6101](https://github.com/JabRef/jabref/pull/6101) - We fixed an issue where the file renaming dialog was not resizable and its size was too small for long file names. [#12518](https://github.com/JabRef/jabref/pull/12518) diff --git a/src/main/java/org/jabref/gui/frame/MainMenu.java b/src/main/java/org/jabref/gui/frame/MainMenu.java index e119bf449fd..335ed3a7856 100644 --- a/src/main/java/org/jabref/gui/frame/MainMenu.java +++ b/src/main/java/org/jabref/gui/frame/MainMenu.java @@ -263,11 +263,11 @@ private void createMenu() { new SeparatorMenuItem(), factory.createSubMenu(StandardActions.ABBREVIATE, - factory.createMenuItem(StandardActions.ABBREVIATE_DEFAULT, new AbbreviateAction(StandardActions.ABBREVIATE_DEFAULT, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager)), - factory.createMenuItem(StandardActions.ABBREVIATE_DOTLESS, new AbbreviateAction(StandardActions.ABBREVIATE_DOTLESS, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager)), - factory.createMenuItem(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, new AbbreviateAction(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager))), + factory.createMenuItem(StandardActions.ABBREVIATE_DEFAULT, new AbbreviateAction(StandardActions.ABBREVIATE_DEFAULT, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager)), + factory.createMenuItem(StandardActions.ABBREVIATE_DOTLESS, new AbbreviateAction(StandardActions.ABBREVIATE_DOTLESS, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager)), + factory.createMenuItem(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, new AbbreviateAction(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager))), - factory.createMenuItem(StandardActions.UNABBREVIATE, new AbbreviateAction(StandardActions.UNABBREVIATE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager)) + factory.createMenuItem(StandardActions.UNABBREVIATE, new AbbreviateAction(StandardActions.UNABBREVIATE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager)) ); Menu lookupIdentifiers = factory.createSubMenu(StandardActions.LOOKUP_DOC_IDENTIFIER); diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 0e84e9cfcba..299330cce9b 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -2,6 +2,7 @@ import java.util.List; import java.util.function.Supplier; +import java.nio.file.Path; import javax.swing.undo.UndoManager; @@ -14,6 +15,7 @@ import org.jabref.gui.undo.NamedCompound; import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; +import org.jabref.logic.journals.JournalAbbreviationLoader; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.BackgroundTask; import org.jabref.logic.util.TaskExecutor; @@ -36,7 +38,7 @@ public class AbbreviateAction extends SimpleCommand { private final DialogService dialogService; private final StateManager stateManager; private final JournalAbbreviationPreferences journalAbbreviationPreferences; - private final JournalAbbreviationRepository abbreviationRepository; + private JournalAbbreviationRepository abbreviationRepository; private final TaskExecutor taskExecutor; private final UndoManager undoManager; @@ -47,7 +49,6 @@ public AbbreviateAction(StandardActions action, DialogService dialogService, StateManager stateManager, JournalAbbreviationPreferences abbreviationPreferences, - JournalAbbreviationRepository abbreviationRepository, TaskExecutor taskExecutor, UndoManager undoManager) { this.action = action; @@ -55,7 +56,6 @@ public AbbreviateAction(StandardActions action, this.dialogService = dialogService; this.stateManager = stateManager; this.journalAbbreviationPreferences = abbreviationPreferences; - this.abbreviationRepository = abbreviationRepository; this.taskExecutor = taskExecutor; this.undoManager = undoManager; @@ -80,6 +80,11 @@ public void execute() { .onSuccess(dialogService::notify) .executeWith(taskExecutor)); } else if (action == StandardActions.UNABBREVIATE) { + if (!areAnyJournalSourcesEnabled()) { + dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled")); + return; + } + dialogService.notify(Localization.lang("Unabbreviating...")); stateManager.getActiveDatabase().ifPresent(_ -> BackgroundTask.wrap(() -> unabbreviate(stateManager.getActiveDatabase().get(), stateManager.getSelectedEntries())) @@ -91,6 +96,9 @@ public void execute() { } private String abbreviate(BibDatabaseContext databaseContext, List entries) { + // Reload repository to ensure latest preferences are used + abbreviationRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); + UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator( abbreviationRepository, abbreviationType, @@ -113,6 +121,9 @@ private String abbreviate(BibDatabaseContext databaseContext, List ent } private String unabbreviate(BibDatabaseContext databaseContext, List entries) { + // Reload repository to ensure latest preferences are used + abbreviationRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); + UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(abbreviationRepository); NamedCompound ce = new NamedCompound(Localization.lang("Unabbreviate journal names")); @@ -128,4 +139,29 @@ private String unabbreviate(BibDatabaseContext databaseContext, List e tabSupplier.get().markBaseChanged(); return Localization.lang("Unabbreviated %0 journal names.", String.valueOf(count)); } + + /** + * Checks if any journal abbreviation source is enabled in the preferences. + * This includes both the built-in list and any external journal lists. + * + * @return true if at least one source is enabled, false if all sources are disabled + */ + private boolean areAnyJournalSourcesEnabled() { + boolean anySourceEnabled = journalAbbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID); + + if (!anySourceEnabled) { + for (String listPath : journalAbbreviationPreferences.getExternalJournalLists()) { + if (listPath != null && !listPath.isBlank()) { + // Just check the filename since that's what's used as the source key + String fileName = Path.of(listPath).getFileName().toString(); + if (journalAbbreviationPreferences.isSourceEnabled(fileName)) { + anySourceEnabled = true; + break; + } + } + } + } + + return anySourceEnabled; + } } diff --git a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java index acb101aef58..efea27fb8ee 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java @@ -21,6 +21,8 @@ public UndoableUnabbreviator(JournalAbbreviationRepository journalAbbreviationRe /** * Unabbreviate the journal name of the given entry. + * This method respects the enabled/disabled state of journal abbreviation sources. + * If an abbreviation comes from a disabled source, it will not be unabbreviated. * * @param entry The entry to be treated. * @param field The field @@ -42,15 +44,16 @@ public boolean unabbreviate(BibDatabase database, BibEntry entry, Field field, C text = database.resolveForStrings(text); } - if (!journalAbbreviationRepository.isKnownName(text)) { - return false; // Cannot do anything if it is not known. + var abbreviationOpt = journalAbbreviationRepository.get(text); + if (abbreviationOpt.isEmpty()) { + return false; } if (!journalAbbreviationRepository.isAbbreviatedName(text)) { - return false; // Cannot unabbreviate unabbreviated name. + return false; } - Abbreviation abbreviation = journalAbbreviationRepository.get(text).get(); + Abbreviation abbreviation = abbreviationOpt.get(); String newText = abbreviation.getName(); entry.setField(field, newText); ce.addEdit(new UndoableFieldChange(entry, field, origText, newText)); diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index 88ecb96d240..b9ec18267fa 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -1,110 +1,176 @@ package org.jabref.gui.preferences.journals; -import java.io.FileNotFoundException; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; +import java.util.ArrayList; +import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyBooleanProperty; +import javafx.beans.property.ReadOnlyBooleanWrapper; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleListProperty; +import javafx.beans.property.SimpleStringProperty; import javafx.collections.FXCollections; +import javafx.collections.ObservableList; import org.jabref.logic.journals.Abbreviation; import org.jabref.logic.journals.AbbreviationWriter; import org.jabref.logic.journals.JournalAbbreviationLoader; +import org.jabref.logic.journals.JournalAbbreviationRepository; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class provides a model for abbreviation files. It actually doesn't save the files as objects but rather saves * their paths. This also allows to specify pseudo files as placeholder objects. */ public class AbbreviationsFileViewModel { + private static final Logger LOGGER = LoggerFactory.getLogger(AbbreviationsFileViewModel.class); - private final SimpleListProperty abbreviations = new SimpleListProperty<>( - FXCollections.observableArrayList()); - private final ReadOnlyBooleanProperty isBuiltInList; - private final String name; - private final Optional path; - - public AbbreviationsFileViewModel(Path filePath) { - this.path = Optional.ofNullable(filePath); - this.name = path.get().toAbsolutePath().toString(); - this.isBuiltInList = new SimpleBooleanProperty(false); - } + private final SimpleStringProperty name = new SimpleStringProperty(); + private final ReadOnlyBooleanWrapper isBuiltInList = new ReadOnlyBooleanWrapper(); + private final SimpleListProperty abbreviations = new SimpleListProperty<>(FXCollections.observableArrayList()); + private final Path filePath; + private final SimpleBooleanProperty enabled = new SimpleBooleanProperty(true); /** - * This constructor should only be called to create a pseudo abbreviation file for built in lists. This means it is - * a placeholder and its path will be null meaning it has no place on the filesystem. Its isPseudoFile property - * will therefore be set to true. + * This creates a built in list containing the abbreviations from the given list + * + * @param name The name of the built in list */ public AbbreviationsFileViewModel(List abbreviations, String name) { this.abbreviations.addAll(abbreviations); - this.name = name; - this.path = Optional.empty(); - this.isBuiltInList = new SimpleBooleanProperty(true); + this.name.setValue(name); + this.isBuiltInList.setValue(true); + this.filePath = null; } - public void readAbbreviations() throws IOException { - if (path.isPresent()) { - Collection abbreviationList = JournalAbbreviationLoader.readAbbreviationsFromCsvFile(path.get()); - abbreviationList.forEach(abbreviation -> abbreviations.addAll(new AbbreviationViewModel(abbreviation))); - } else { - throw new FileNotFoundException(); - } + public AbbreviationsFileViewModel(Path filePath) { + this.name.setValue(filePath.getFileName().toString()); + this.filePath = filePath; + this.isBuiltInList.setValue(false); } - /** - * This method will write all abbreviations of this abbreviation file to the file on the file system. - * It essentially will check if the current file is a builtin list and if not it will call - * {@link AbbreviationWriter#writeOrCreate}. - */ - public void writeOrCreate() throws IOException { - if (!isBuiltInList.get()) { - List actualAbbreviations = - abbreviations.stream().filter(abb -> !abb.isPseudoAbbreviation()) - .map(AbbreviationViewModel::getAbbreviationObject) - .collect(Collectors.toList()); - AbbreviationWriter.writeOrCreate(path.get(), actualAbbreviations); - } + public boolean exists() { + return isBuiltInList.get() || Files.exists(filePath); + } + + public SimpleStringProperty nameProperty() { + return name; + } + + public ReadOnlyBooleanProperty isBuiltInListProperty() { + return isBuiltInList.getReadOnlyProperty(); } public SimpleListProperty abbreviationsProperty() { return abbreviations; } - public boolean exists() { - return path.isPresent() && Files.exists(path.get()); + public void readAbbreviations() throws IOException { + if (isBuiltInList.get()) { + return; + } + try { + Collection abbreviationsFromFile = JournalAbbreviationLoader.readAbbreviationsFromCsvFile(filePath); + + List viewModels = abbreviationsFromFile.stream() + .map(AbbreviationViewModel::new) + .collect(Collectors.toCollection(ArrayList::new)); + abbreviations.setAll(viewModels); + } catch (NoSuchFileException e) { + LOGGER.debug("Journal abbreviation list {} does not exist", filePath); + } } - public Optional getAbsolutePath() { - return path; + public void writeOrCreate() throws IOException { + if (isBuiltInList.get()) { + return; + } + + List abbreviationList = abbreviationsProperty().stream() + .map(AbbreviationViewModel::getAbbreviationObject) + .collect(Collectors.toList()); + AbbreviationWriter.writeOrCreate(filePath, abbreviationList); } - public ReadOnlyBooleanProperty isBuiltInListProperty() { - return isBuiltInList; + /** + * Gets the absolute path of this abbreviation file + * + * @return The optional absolute path of the file, or empty if it's a built-in list + */ + public Optional getAbsolutePath() { + if (isBuiltInList.get()) { + return Optional.empty(); + } + + try { + Path normalizedPath = filePath.toAbsolutePath().normalize(); + return Optional.of(normalizedPath); + } catch (Exception e) { + return Optional.of(filePath); + } + } + + /** + * Checks if this source is enabled + * + * @return true if the source is enabled + */ + public boolean isEnabled() { + return enabled.get(); + } + + /** + * Sets the enabled state of this source + * + * @param enabled true to enable the source, false to disable it + */ + public void setEnabled(boolean enabled) { + this.enabled.set(enabled); + } + + /** + * Gets the enabled property for binding + * + * @return the enabled property + */ + public BooleanProperty enabledProperty() { + return enabled; } @Override - public String toString() { - return name; + public boolean equals(Object o) { + if (this == o) { + return true; + } + if ((o == null) || (getClass() != o.getClass())) { + return false; + } + AbbreviationsFileViewModel viewModel = (AbbreviationsFileViewModel) o; + if (isBuiltInList.get() && viewModel.isBuiltInList.get()) { + return name.get().equals(viewModel.name.get()); + } + return !isBuiltInList.get() && !viewModel.isBuiltInList.get() && + Objects.equals(filePath, viewModel.filePath); } @Override public int hashCode() { - return Objects.hash(name); + return Objects.hash(isBuiltInList, filePath); } @Override - public boolean equals(Object obj) { - if (obj instanceof AbbreviationsFileViewModel model) { - return Objects.equals(this.name, model.name); - } else { - return false; - } + public String toString() { + return name.get(); } } + diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index 5bfa504edf5..2fd7a156682 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -15,10 +15,12 @@ import javafx.scene.control.CheckBox; import javafx.scene.control.ComboBox; import javafx.scene.control.Label; +import javafx.scene.control.ListCell; import javafx.scene.control.ProgressIndicator; import javafx.scene.control.TableColumn; import javafx.scene.control.TableView; import javafx.scene.control.cell.TextFieldTableCell; +import javafx.scene.layout.HBox; import javafx.scene.paint.Color; import javafx.util.Duration; @@ -35,6 +37,8 @@ import com.tobiasdiez.easybind.EasyBind; import jakarta.inject.Inject; import org.controlsfx.control.textfield.CustomTextField; +import javafx.application.Platform; +import com.airhacks.afterburner.injection.Injector; /** * This class controls the user interface of the journal abbreviations dialog. The UI elements and their layout are @@ -82,6 +86,7 @@ private void initialize() { filteredAbbreviations = new FilteredList<>(viewModel.abbreviationsProperty()); setUpTable(); + setUpToggleButton(); setBindings(); setAnimations(); @@ -110,6 +115,32 @@ private void setUpTable() { .install(actionsColumn); } + private void setUpToggleButton() { + Button toggleButton = new Button(Localization.lang("Toggle")); + toggleButton.setOnAction(e -> toggleEnableList()); + toggleButton.setTooltip(new javafx.scene.control.Tooltip(Localization.lang("Toggle selected list on/off"))); + toggleButton.getStyleClass().add("icon-button"); + + for (javafx.scene.Node node : getChildren()) { + if (node instanceof HBox) { + HBox hbox = (HBox) node; + boolean containsComboBox = false; + for (javafx.scene.Node child : hbox.getChildren()) { + if (child == journalFilesBox) { + containsComboBox = true; + break; + } + } + + if (containsComboBox) { + int comboBoxIndex = hbox.getChildren().indexOf(journalFilesBox); + hbox.getChildren().add(comboBoxIndex + 1, toggleButton); + break; + } + } + } + } + private void setBindings() { journalAbbreviationsTable.setItems(filteredAbbreviations); @@ -125,6 +156,20 @@ private void setBindings() { removeAbbreviationListButton.disableProperty().bind(viewModel.isFileRemovableProperty().not()); journalFilesBox.itemsProperty().bindBidirectional(viewModel.journalFilesProperty()); journalFilesBox.valueProperty().bindBidirectional(viewModel.currentFileProperty()); + + journalFilesBox.setCellFactory(listView -> new JournalFileListCell()); + journalFilesBox.setButtonCell(new JournalFileListCell()); + + viewModel.journalFilesProperty().addListener((observable, oldValue, newValue) -> { + if (newValue == null) { + return; + } + for (AbbreviationsFileViewModel fileViewModel : newValue) { + fileViewModel.enabledProperty().addListener((obs, oldVal, newVal) -> { + refreshComboBoxDisplay(); + }); + } + }); addAbbreviationButton.disableProperty().bind(viewModel.isEditableAndRemovableProperty().not()); @@ -137,20 +182,48 @@ private void setBindings() { useFJournal.selectedProperty().bindBidirectional(viewModel.useFJournalProperty()); } - private void setAnimations() { - ObjectProperty flashingColor = new SimpleObjectProperty<>(Color.TRANSPARENT); - StringProperty flashingColorStringProperty = ColorUtil.createFlashingColorStringProperty(flashingColor); - - searchBox.styleProperty().bind( - new SimpleStringProperty("-fx-control-inner-background: ").concat(flashingColorStringProperty).concat(";") - ); - invalidateSearch = new Timeline( - new KeyFrame(Duration.seconds(0), new KeyValue(flashingColor, Color.TRANSPARENT, Interpolator.LINEAR)), - new KeyFrame(Duration.seconds(0.25), new KeyValue(flashingColor, Color.RED, Interpolator.LINEAR)), - new KeyFrame(Duration.seconds(0.25), new KeyValue(searchBox.textProperty(), "", Interpolator.DISCRETE)), - new KeyFrame(Duration.seconds(0.25), (ActionEvent event) -> addAbbreviationActions()), - new KeyFrame(Duration.seconds(0.5), new KeyValue(flashingColor, Color.TRANSPARENT, Interpolator.LINEAR)) - ); + /** + * Custom ListCell to display the journal file items with checkboxes. + * This simply shows the checkbox status without trying to handle + * direct checkbox interactions, to avoid conflicts with ComboBox selection. + */ + private static class JournalFileListCell extends ListCell { + @Override + protected void updateItem(AbbreviationsFileViewModel item, boolean empty) { + super.updateItem(item, empty); + + if (empty || item == null) { + setText(null); + setGraphic(null); + } else { + String prefix = item.isEnabled() ? "✓ " : "○ "; + setText(prefix + item.toString()); + + item.enabledProperty().addListener((obs, oldVal, newVal) -> { + if (newVal != null) { + setText((newVal ? "✓ " : "○ ") + item.toString()); + } + }); + } + } + } + + /** + * Force the ComboBox to refresh its display + */ + private void refreshComboBoxDisplay() { + Platform.runLater(() -> { + AbbreviationsFileViewModel currentSelection = journalFilesBox.getValue(); + + journalFilesBox.setButtonCell(new JournalFileListCell()); + + journalFilesBox.setValue(null); + journalFilesBox.setValue(currentSelection); + + journalFilesBox.setCellFactory(listView -> new JournalFileListCell()); + + journalFilesBox.requestLayout(); + }); } @FXML @@ -201,4 +274,42 @@ private void selectNewAbbreviation() { public String getTabName() { return Localization.lang("Journal abbreviations"); } + + @FXML + private void toggleEnableList() { + AbbreviationsFileViewModel selected = journalFilesBox.getValue(); + if (selected != null) { + boolean newEnabledState = !selected.isEnabled(); + selected.setEnabled(newEnabledState); + + // Update repository immediately to reflect changes in the UI + JournalAbbreviationRepository repository = Injector.instantiateModelOrService(JournalAbbreviationRepository.class); + if (selected.isBuiltInListProperty().get()) { + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState); + } else if (selected.getAbsolutePath().isPresent()) { + String fileName = selected.getAbsolutePath().get().getFileName().toString(); + repository.setSourceEnabled(fileName, newEnabledState); + } + + refreshComboBoxDisplay(); + + viewModel.markAsDirty(); + } + } + + private void setAnimations() { + ObjectProperty flashingColor = new SimpleObjectProperty<>(Color.TRANSPARENT); + StringProperty flashingColorStringProperty = ColorUtil.createFlashingColorStringProperty(flashingColor); + + searchBox.styleProperty().bind( + new SimpleStringProperty("-fx-control-inner-background: ").concat(flashingColorStringProperty).concat(";") + ); + invalidateSearch = new Timeline( + new KeyFrame(Duration.seconds(0), new KeyValue(flashingColor, Color.TRANSPARENT, Interpolator.LINEAR)), + new KeyFrame(Duration.seconds(0.25), new KeyValue(flashingColor, Color.RED, Interpolator.LINEAR)), + new KeyFrame(Duration.seconds(0.25), new KeyValue(searchBox.textProperty(), "", Interpolator.DISCRETE)), + new KeyFrame(Duration.seconds(0.25), (ActionEvent event) -> addAbbreviationActions()), + new KeyFrame(Duration.seconds(0.5), new KeyValue(flashingColor, Color.TRANSPARENT, Interpolator.LINEAR)) + ); + } } diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java index 7b95ac28244..83174859b0c 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -1,8 +1,15 @@ package org.jabref.gui.preferences.journals; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -145,7 +152,12 @@ public void addBuiltInList() { List builtInViewModels = result.stream() .map(AbbreviationViewModel::new) .collect(Collectors.toList()); - journalFiles.add(new AbbreviationsFileViewModel(builtInViewModels, Localization.lang("JabRef built in list"))); + AbbreviationsFileViewModel builtInListModel = new AbbreviationsFileViewModel(builtInViewModels, Localization.lang("JabRef built in list")); + + boolean isEnabled = abbreviationsPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID); + builtInListModel.setEnabled(isEnabled); + + journalFiles.add(builtInListModel); selectLastJournalFile(); }) .onFailure(dialogService::showErrorDialogAndWait) @@ -181,6 +193,10 @@ private void openFile(Path filePath) { if (abbreviationsFile.exists()) { try { abbreviationsFile.readAbbreviations(); + + String fileName = filePath.getFileName().toString(); + boolean isEnabled = abbreviationsPreferences.isSourceEnabled(fileName); + abbreviationsFile.setEnabled(isEnabled); } catch (IOException e) { LOGGER.debug("Could not read abbreviations file", e); } @@ -335,19 +351,39 @@ public void storeSettings() { abbreviationsPreferences.setExternalJournalLists(journalStringList); abbreviationsPreferences.setUseFJournalField(useFJournal.get()); + + for (AbbreviationsFileViewModel fileViewModel : journalFiles) { + if (fileViewModel.isBuiltInListProperty().get()) { + abbreviationsPreferences.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, fileViewModel.isEnabled()); + } else if (fileViewModel.getAbsolutePath().isPresent()) { + String fileName = fileViewModel.getAbsolutePath().get().getFileName().toString(); + abbreviationsPreferences.setSourceEnabled(fileName, fileViewModel.isEnabled()); + } + } if (shouldWriteLists) { saveJournalAbbreviationFiles(); shouldWriteLists = false; } }) - .onSuccess(success -> Injector.setModelOrService( - JournalAbbreviationRepository.class, - JournalAbbreviationLoader.loadRepository(abbreviationsPreferences))) - .onFailure(exception -> LOGGER.error("Failed to store journal preferences.", exception)) + .onSuccess(result -> { + JournalAbbreviationRepository newRepository = + JournalAbbreviationLoader.loadRepository(abbreviationsPreferences); + + Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository); + }) + .onFailure(exception -> LOGGER.error("Failed to store journal preferences: {}", exception.getMessage(), exception)) .executeWith(taskExecutor); } + /** + * Marks the abbreviation lists as needing to be saved + * This only tracks the dirty state but doesn't write to preferences + */ + public void markAsDirty() { + shouldWriteLists = true; + } + public SimpleBooleanProperty isLoadingProperty() { return isLoading; } @@ -387,4 +423,4 @@ public SimpleBooleanProperty isFileRemovableProperty() { public SimpleBooleanProperty useFJournalProperty() { return useFJournal; } -} +} \ No newline at end of file diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java index 264a727ff13..c2f9d1f544c 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java @@ -8,6 +8,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,19 +33,30 @@ public static Collection readAbbreviationsFromCsvFile(Path file) t return parser.getAbbreviations(); } + /** + * Loads a journal abbreviation repository based on the given preferences. + * Takes into account enabled/disabled state of journal abbreviation sources. + * + * @param journalAbbreviationPreferences The preferences containing journal list paths and enabled states + * @return A repository with loaded abbreviations + */ public static JournalAbbreviationRepository loadRepository(JournalAbbreviationPreferences journalAbbreviationPreferences) { JournalAbbreviationRepository repository; + boolean builtInEnabled = journalAbbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID); + // Initialize with built-in list try (InputStream resourceAsStream = JournalAbbreviationRepository.class.getResourceAsStream("/journals/journal-list.mv")) { if (resourceAsStream == null) { LOGGER.warn("There is no journal-list.mv. We use a default journal list"); repository = new JournalAbbreviationRepository(); + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, builtInEnabled); } else { Path tempDir = Files.createTempDirectory("jabref-journal"); Path tempJournalList = tempDir.resolve("journal-list.mv"); Files.copy(resourceAsStream, tempJournalList); repository = new JournalAbbreviationRepository(tempJournalList); + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, builtInEnabled); tempDir.toFile().deleteOnExit(); tempJournalList.toFile().deleteOnExit(); } @@ -61,7 +73,38 @@ public static JournalAbbreviationRepository loadRepository(JournalAbbreviationPr Collections.reverse(lists); for (String filename : lists) { try { - repository.addCustomAbbreviations(readAbbreviationsFromCsvFile(Path.of(filename))); + Path filePath = Path.of(filename); + + String simpleFilename = filePath.getFileName().toString(); + String prefixedKey = "JABREF_JOURNAL_LIST:" + simpleFilename; + String absolutePath = filePath.toAbsolutePath().toString(); + + boolean enabled = false; + + if (journalAbbreviationPreferences.hasExplicitEnabledSetting(prefixedKey)) { + enabled = journalAbbreviationPreferences.isSourceEnabled(prefixedKey); + LOGGER.debug("Loading external abbreviations file {} (found by prefixed key): enabled = {}", + simpleFilename, enabled); + } else if (journalAbbreviationPreferences.hasExplicitEnabledSetting(simpleFilename)) { + enabled = journalAbbreviationPreferences.isSourceEnabled(simpleFilename); + LOGGER.debug("Loading external abbreviations file {} (found by filename): enabled = {}", + simpleFilename, enabled); + } else if (journalAbbreviationPreferences.hasExplicitEnabledSetting(absolutePath)) { + enabled = journalAbbreviationPreferences.isSourceEnabled(absolutePath); + LOGGER.debug("Loading external abbreviations file {} (found by path): enabled = {}", + simpleFilename, enabled); + } else { + enabled = true; + LOGGER.debug("Loading external abbreviations file {} (no settings found): enabled = {}", + simpleFilename, enabled); + } + + Collection abbreviations = readAbbreviationsFromCsvFile(filePath); + + repository.addCustomAbbreviations(abbreviations, simpleFilename, enabled); + + repository.addCustomAbbreviations(Collections.emptyList(), prefixedKey, enabled); + } catch (IOException | InvalidPathException e) { // invalid path might come from unix/windows mixup of prefs LOGGER.error("Cannot read external journal list file {}", filename, e); diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java index fb256bbe8ab..7f1f32e29c8 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java @@ -1,23 +1,53 @@ package org.jabref.logic.journals; +import java.util.HashMap; import java.util.List; +import java.util.Map; import javafx.beans.property.BooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; +/** + * Class for storing and managing journal abbreviation preferences + */ public class JournalAbbreviationPreferences { private final ObservableList externalJournalLists; private final BooleanProperty useFJournalField; - + private final Map enabledExternalLists = new HashMap<>(); + private final BooleanProperty enabledListsChanged = new SimpleBooleanProperty(false); + + /** + * Constructs a new JournalAbbreviationPreferences with the given external journal lists and FJournal field preference + * + * @param externalJournalLists List of paths to external journal abbreviation files + * @param useFJournalField Whether to use the FJournal field + */ public JournalAbbreviationPreferences(List externalJournalLists, boolean useFJournalField) { this.externalJournalLists = FXCollections.observableArrayList(externalJournalLists); this.useFJournalField = new SimpleBooleanProperty(useFJournalField); } + /** + * Constructs a new JournalAbbreviationPreferences with the given external journal lists, FJournal field preference, + * and enabled states for journal abbreviation sources + * + * @param externalJournalLists List of paths to external journal abbreviation files + * @param useFJournalField Whether to use the FJournal field + * @param enabledExternalLists Map of source paths to their enabled states + */ + public JournalAbbreviationPreferences(List externalJournalLists, + boolean useFJournalField, + Map enabledExternalLists) { + this(externalJournalLists, useFJournalField); + if (enabledExternalLists != null) { + this.enabledExternalLists.putAll(enabledExternalLists); + } + } + public ObservableList getExternalJournalLists() { return externalJournalLists; } @@ -38,4 +68,73 @@ public BooleanProperty useFJournalFieldProperty() { public void setUseFJournalField(boolean useFJournalField) { this.useFJournalField.set(useFJournalField); } + + /** + * Checks if a journal abbreviation source is enabled + * + * @param sourcePath Path to the abbreviation source + * @return true if the source is enabled or has no explicit state (default is enabled) + */ + public boolean isSourceEnabled(String sourcePath) { + if (sourcePath == null) { + return true; + } + return enabledExternalLists.getOrDefault(sourcePath, true); + } + + /** + * Sets the enabled state for a journal abbreviation source + * + * @param sourcePath Path to the abbreviation source + * @param enabled Whether the source should be enabled + */ + public void setSourceEnabled(String sourcePath, boolean enabled) { + if (sourcePath == null) { + return; + } + enabledExternalLists.put(sourcePath, enabled); + enabledListsChanged.set(!enabledListsChanged.get()); + } + + /** + * Gets all enabled/disabled states for journal abbreviation sources + * + * @return Map of source paths to their enabled states + */ + public Map getEnabledExternalLists() { + return new HashMap<>(enabledExternalLists); + } + + /** + * Sets all enabled/disabled states for journal abbreviation sources + * + * @param enabledLists Map of source paths to their enabled states + */ + public void setEnabledExternalLists(Map enabledLists) { + this.enabledExternalLists.clear(); + if (enabledLists != null) { + this.enabledExternalLists.putAll(enabledLists); + } + enabledListsChanged.set(!enabledListsChanged.get()); + } + + /** + * Property that changes whenever the enabled states map changes + * Used for binding/listening to changes + * + * @return A boolean property that toggles when enabled states change + */ + public BooleanProperty enabledListsChangedProperty() { + return enabledListsChanged; + } + + /** + * Checks if a specific source has an explicit enabled/disabled setting + * + * @param sourcePath Path to check + * @return True if there is an explicit setting for this source + */ + public boolean hasExplicitEnabledSetting(String sourcePath) { + return enabledExternalLists.containsKey(sourcePath); + } } diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 4191be45857..40e641710a3 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -4,6 +4,7 @@ import java.util.Collection; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -12,17 +13,25 @@ import java.util.TreeSet; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.jabref.logic.util.strings.StringSimilarity; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A repository for all journal abbreviations, including add and find methods. */ public class JournalAbbreviationRepository { static final Pattern QUESTION_MARK = Pattern.compile("\\?"); + + /** + * Identifier for the built-in abbreviation list + */ + public static final String BUILTIN_LIST_ID = "BUILTIN_LIST"; private final Map fullToAbbreviationObject = new HashMap<>(); private final Map abbreviationToAbbreviationObject = new HashMap<>(); @@ -30,6 +39,9 @@ public class JournalAbbreviationRepository { private final Map shortestUniqueToAbbreviationObject = new HashMap<>(); private final TreeSet customAbbreviations = new TreeSet<>(); private final StringSimilarity similarity = new StringSimilarity(); + + private final Map enabledSources = new HashMap<>(); + private final Map abbreviationSources = new HashMap<>(); /** * Initializes the internal data based on the abbreviations found in the given MV file @@ -50,8 +62,12 @@ public JournalAbbreviationRepository(Path journalList) { abbreviationToAbbreviationObject.put(abbrevationString, newAbbreviation); dotlessToAbbreviationObject.put(newAbbreviation.getDotlessAbbreviation(), newAbbreviation); shortestUniqueToAbbreviationObject.put(shortestUniqueAbbreviation, newAbbreviation); + + abbreviationSources.put(newAbbreviation, BUILTIN_LIST_ID); }); } + + enabledSources.put(BUILTIN_LIST_ID, true); } /** @@ -67,6 +83,9 @@ public JournalAbbreviationRepository() { abbreviationToAbbreviationObject.put("Demo", newAbbreviation); dotlessToAbbreviationObject.put("Demo", newAbbreviation); shortestUniqueToAbbreviationObject.put("Dem", newAbbreviation); + + abbreviationSources.put(newAbbreviation, BUILTIN_LIST_ID); + enabledSources.put(BUILTIN_LIST_ID, true); } private static boolean isMatched(String name, Abbreviation abbreviation) { @@ -87,30 +106,42 @@ private static boolean isMatchedAbbreviated(String name, Abbreviation abbreviati } /** - * Returns true if the given journal name is contained in the list either in its full form - * (e.g., Physical Review Letters) or its abbreviated form (e.g., Phys. Rev. Lett.). - * If the exact match is not found, attempts a fuzzy match to recognize minor input errors. + * Returns true if the given journal name is in its abbreviated form (e.g. Phys. Rev. Lett.). The test is strict, + * i.e., journals whose abbreviation is the same as the full name are not considered. + * Respects the enabled/disabled state of abbreviation sources. */ - public boolean isKnownName(String journalName) { + public boolean isAbbreviatedName(String journalName) { if (QUESTION_MARK.matcher(journalName).find()) { return false; } - return get(journalName).isPresent(); + String journal = journalName.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&"); + + boolean isCustomAbbreviated = customAbbreviations.stream() + .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) + .anyMatch(abbreviation -> isMatchedAbbreviated(journal, abbreviation)); + + if (!isSourceEnabled(BUILTIN_LIST_ID)) { + return isCustomAbbreviated; + } + + return isCustomAbbreviated || + abbreviationToAbbreviationObject.containsKey(journal) || + dotlessToAbbreviationObject.containsKey(journal) || + shortestUniqueToAbbreviationObject.containsKey(journal); } /** - * Returns true if the given journal name is in its abbreviated form (e.g. Phys. Rev. Lett.). The test is strict, - * i.e., journals whose abbreviation is the same as the full name are not considered + * Returns true if the given journal name is contained in the list either in its full form + * (e.g., Physical Review Letters) or its abbreviated form (e.g., Phys. Rev. Lett.). + * If the exact match is not found, attempts a fuzzy match to recognize minor input errors. + * Respects the enabled/disabled state of abbreviation sources. */ - public boolean isAbbreviatedName(String journalName) { + public boolean isKnownName(String journalName) { if (QUESTION_MARK.matcher(journalName).find()) { return false; } - String journal = journalName.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&"); - return customAbbreviations.stream().anyMatch(abbreviation -> isMatchedAbbreviated(journal, abbreviation)) - || abbreviationToAbbreviationObject.containsKey(journal) - || dotlessToAbbreviationObject.containsKey(journal) - || shortestUniqueToAbbreviationObject.containsKey(journal); + + return get(journalName).isPresent(); } /** @@ -124,30 +155,48 @@ public Optional get(String input) { String journal = input.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&"); Optional customAbbreviation = customAbbreviations.stream() - .filter(abbreviation -> isMatched(journal, abbreviation)) - .findFirst(); + .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) + .filter(abbreviation -> isMatched(journal, abbreviation)) + .findFirst(); + if (customAbbreviation.isPresent()) { return customAbbreviation; } - Optional abbreviation = Optional.ofNullable(fullToAbbreviationObject.get(journal)) - .or(() -> Optional.ofNullable(abbreviationToAbbreviationObject.get(journal))) - .or(() -> Optional.ofNullable(dotlessToAbbreviationObject.get(journal))) - .or(() -> Optional.ofNullable(shortestUniqueToAbbreviationObject.get(journal))); + if (!isSourceEnabled(BUILTIN_LIST_ID)) { + return Optional.empty(); + } + + Optional builtInAbbreviation = Optional.empty(); + + if (isSourceEnabled(BUILTIN_LIST_ID)) { + builtInAbbreviation = Optional.ofNullable(fullToAbbreviationObject.get(journal)) + .or(() -> Optional.ofNullable(abbreviationToAbbreviationObject.get(journal))) + .or(() -> Optional.ofNullable(dotlessToAbbreviationObject.get(journal))) + .or(() -> Optional.ofNullable(shortestUniqueToAbbreviationObject.get(journal))); + } - if (abbreviation.isEmpty()) { - abbreviation = findAbbreviationFuzzyMatched(journal); + if (builtInAbbreviation.isPresent()) { + return builtInAbbreviation; } - return abbreviation; + return findAbbreviationFuzzyMatched(journal); } private Optional findAbbreviationFuzzyMatched(String input) { - Optional customMatch = findBestFuzzyMatched(customAbbreviations, input); + Collection enabledCustomAbbreviations = customAbbreviations.stream() + .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) + .collect(Collectors.toList()); + + Optional customMatch = findBestFuzzyMatched(enabledCustomAbbreviations, input); if (customMatch.isPresent()) { return customMatch; } + if (!isSourceEnabled(BUILTIN_LIST_ID)) { + return Optional.empty(); + } + return findBestFuzzyMatched(fullToAbbreviationObject.values(), input); } @@ -177,6 +226,11 @@ private Optional findBestFuzzyMatched(Collection abb return Optional.of(candidates.getFirst()); } + /** + * Adds a custom abbreviation to the repository + * + * @param abbreviation The abbreviation to add + */ public void addCustomAbbreviation(Abbreviation abbreviation) { Objects.requireNonNull(abbreviation); @@ -184,15 +238,75 @@ public void addCustomAbbreviation(Abbreviation abbreviation) { // The set automatically "removes" duplicates // What is a duplicate? An abbreviation is NOT the same if any field is NOT equal (e.g., if the shortest unique differs, the abbreviation is NOT the same) customAbbreviations.add(abbreviation); + + abbreviationSources.put(abbreviation, BUILTIN_LIST_ID); + } + + /** + * Adds a custom abbreviation to the repository with source tracking + * + * @param abbreviation The abbreviation to add + * @param sourcePath The path or identifier of the source + * @param enabled Whether the source is enabled + */ + public void addCustomAbbreviation(Abbreviation abbreviation, String sourcePath, boolean enabled) { + Objects.requireNonNull(abbreviation); + Objects.requireNonNull(sourcePath); + + customAbbreviations.add(abbreviation); + abbreviationSources.put(abbreviation, sourcePath); + + enabledSources.put(sourcePath, enabled); } public Collection getCustomAbbreviations() { return customAbbreviations; } + /** + * Adds multiple custom abbreviations to the repository + * + * @param abbreviationsToAdd The abbreviations to add + */ public void addCustomAbbreviations(Collection abbreviationsToAdd) { abbreviationsToAdd.forEach(this::addCustomAbbreviation); } + + /** + * Adds abbreviations with a specific source key and enabled state + * + * @param abbreviationsToAdd Collection of abbreviations to add + * @param sourceKey The key identifying the source of these abbreviations + * @param enabled Whether the source should be enabled initially + */ + public void addCustomAbbreviations(Collection abbreviationsToAdd, String sourceKey, boolean enabled) { + enabledSources.put(sourceKey, enabled); + + for (Abbreviation abbreviation : abbreviationsToAdd) { + customAbbreviations.add(abbreviation); + abbreviationSources.put(abbreviation, sourceKey); + } + } + + /** + * Checks if a journal abbreviation source is enabled + * + * @param sourceKey The key identifying the source + * @return true if the source is enabled or has no explicit state (default is enabled) + */ + public boolean isSourceEnabled(String sourceKey) { + return enabledSources.getOrDefault(sourceKey, true); + } + + /** + * Sets the enabled state for a journal abbreviation source + * + * @param sourceKey The key identifying the source + * @param enabled Whether the source should be enabled + */ + public void setSourceEnabled(String sourceKey, boolean enabled) { + enabledSources.put(sourceKey, enabled); + } public Optional getNextAbbreviation(String text) { return get(text).map(abbreviation -> abbreviation.getNext(text)); diff --git a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java index 3bb2bc84066..f279f305bb4 100644 --- a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java +++ b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java @@ -424,6 +424,8 @@ public class JabRefCliPreferences implements CliPreferences { private AiPreferences aiPreferences; private LastFilesOpenedPreferences lastFilesOpenedPreferences; + private static final String ENABLED_EXTERNAL_JOURNAL_LISTS = "enabledExternalJournalLists"; + /** * @implNote The constructor is made protected to enforce this as a singleton class: */ @@ -1014,14 +1016,41 @@ public JournalAbbreviationPreferences getJournalAbbreviationPreferences() { return journalAbbreviationPreferences; } + Map enabledExternalLists = new HashMap<>(); + + enabledExternalLists.put(JournalAbbreviationRepository.BUILTIN_LIST_ID, + getBoolean(ENABLED_EXTERNAL_JOURNAL_LISTS + ":" + JournalAbbreviationRepository.BUILTIN_LIST_ID, true)); + + for (String path : getStringList(EXTERNAL_JOURNAL_LISTS)) { + try { + String absolutePath = Path.of(path).toAbsolutePath().toString(); + enabledExternalLists.put(absolutePath, getBoolean(ENABLED_EXTERNAL_JOURNAL_LISTS + ":" + absolutePath, true)); + } catch (Exception e) { + enabledExternalLists.put(path, getBoolean(ENABLED_EXTERNAL_JOURNAL_LISTS + ":" + path, true)); + LOGGER.warn("Could not resolve absolute path for {}", path, e); + } + } + journalAbbreviationPreferences = new JournalAbbreviationPreferences( getStringList(EXTERNAL_JOURNAL_LISTS), - getBoolean(USE_AMS_FJOURNAL)); + getBoolean(USE_AMS_FJOURNAL), + enabledExternalLists); journalAbbreviationPreferences.getExternalJournalLists().addListener((InvalidationListener) change -> putStringList(EXTERNAL_JOURNAL_LISTS, journalAbbreviationPreferences.getExternalJournalLists())); EasyBind.listen(journalAbbreviationPreferences.useFJournalFieldProperty(), (obs, oldValue, newValue) -> putBoolean(USE_AMS_FJOURNAL, newValue)); + + journalAbbreviationPreferences.getEnabledExternalLists().forEach((path, enabled) -> { + LOGGER.debug("Setting preference value for journal list: {} = {}", path, enabled); + putBoolean(ENABLED_EXTERNAL_JOURNAL_LISTS + ":" + path, enabled); + }); + + journalAbbreviationPreferences.enabledListsChangedProperty().addListener(observable -> { + journalAbbreviationPreferences.getEnabledExternalLists().forEach((path, enabled) -> { + putBoolean(ENABLED_EXTERNAL_JOURNAL_LISTS + ":" + path, enabled); + }); + }); return journalAbbreviationPreferences; } diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java new file mode 100644 index 00000000000..0f4f3416a19 --- /dev/null +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -0,0 +1,98 @@ +package org.jabref.gui.journals; + +import java.util.Optional; + +import javax.swing.undo.UndoManager; + +import javafx.collections.FXCollections; + +import org.jabref.gui.DialogService; +import org.jabref.gui.LibraryTab; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.StandardActions; +import org.jabref.logic.journals.JournalAbbreviationPreferences; +import org.jabref.logic.journals.JournalAbbreviationRepository; +import org.jabref.logic.util.TaskExecutor; +import org.jabref.model.database.BibDatabaseContext; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class AbbreviateActionTest { + + @Mock + private DialogService dialogService; + + @Mock + private StateManager stateManager; + + @Mock + private LibraryTab libraryTab; + + @Mock + private JournalAbbreviationPreferences abbreviationPreferences; + + @Mock + private TaskExecutor taskExecutor; + + @Mock + private UndoManager undoManager; + + @BeforeEach + public void setUp() { + MockitoAnnotations.openMocks(this); + + when(stateManager.getSelectedEntries()).thenReturn(FXCollections.observableArrayList()); + when(stateManager.getActiveDatabase()).thenReturn(Optional.empty()); + } + + @Test + public void unabbreviateWithAllSourcesDisabledShowsNotification() { + when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); + when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(false); + + AbbreviateAction action = new AbbreviateAction( + StandardActions.UNABBREVIATE, + () -> libraryTab, + dialogService, + stateManager, + abbreviationPreferences, + taskExecutor, + undoManager); + + action.execute(); + + verify(dialogService).notify(eq("Cannot unabbreviate: all journal lists are disabled")); + } + + @Test + public void unabbreviateWithOneSourceEnabledExecutesTask() { + when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); + when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(true); + + BibDatabaseContext databaseContext = mock(BibDatabaseContext.class); + when(stateManager.getActiveDatabase()).thenReturn(Optional.of(databaseContext)); + + AbbreviateAction action = new AbbreviateAction( + StandardActions.UNABBREVIATE, + () -> libraryTab, + dialogService, + stateManager, + abbreviationPreferences, + taskExecutor, + undoManager); + + action.execute(); + + verify(dialogService, never()).notify(eq("Cannot unabbreviate: all journal lists are disabled")); + } +} \ No newline at end of file diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 52702a86957..5294681027e 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -12,6 +12,7 @@ import javafx.collections.FXCollections; import javafx.collections.ObservableList; +import javafx.beans.property.SimpleBooleanProperty; import org.jabref.gui.DialogService; import org.jabref.logic.journals.Abbreviation; @@ -32,7 +33,9 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -63,6 +66,7 @@ class JournalAbbreviationsViewModelTabTest { private Path tempFolder; private final JournalAbbreviationRepository repository = JournalAbbreviationLoader.loadBuiltInRepository(); private DialogService dialogService; + private JournalAbbreviationPreferences abbreviationPreferences; static class TestAbbreviation extends Abbreviation { @@ -176,7 +180,12 @@ public static Stream provideTestFiles() { @BeforeEach void setUpViewModel(@TempDir Path tempFolder) throws Exception { - JournalAbbreviationPreferences abbreviationPreferences = mock(JournalAbbreviationPreferences.class); + abbreviationPreferences = mock(JournalAbbreviationPreferences.class); + + + when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); + when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(true); + when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); dialogService = mock(DialogService.class); this.tempFolder = tempFolder; @@ -532,4 +541,113 @@ private Path createTestFile(CsvFileNameAndContent testFile) throws IOException { Files.writeString(file, testFile.content); return file; } + + @Test + void toggleEnabledListChangesEnabledState() { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); + viewModel.addNewFile(); + viewModel.selectLastJournalFile(); + + AbbreviationsFileViewModel fileViewModel = viewModel.currentFileProperty().get(); + + assertTrue(fileViewModel.isEnabled()); + + fileViewModel.setEnabled(false); + + assertFalse(fileViewModel.isEnabled()); + + fileViewModel.setEnabled(true); + assertTrue(fileViewModel.isEnabled()); + + viewModel.markAsDirty(); + } + + @Test + void storeSettingsSavesEnabledState() throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); + viewModel.addNewFile(); + viewModel.selectLastJournalFile(); + + AbbreviationsFileViewModel fileViewModel = viewModel.currentFileProperty().get(); + fileViewModel.setEnabled(false); + + viewModel.storeSettings(); + + Optional path = fileViewModel.getAbsolutePath(); + if (path.isPresent()) { + String fileName = path.get().getFileName().toString(); + verify(abbreviationPreferences).setSourceEnabled(fileName, false); + } + } + + @Test + void addBuiltInListInitializesWithCorrectEnabledState() { + when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(false); + + JournalAbbreviationsTabViewModel testViewModel = new JournalAbbreviationsTabViewModel( + abbreviationPreferences, dialogService, new CurrentThreadTaskExecutor(), repository); + + testViewModel.addBuiltInList(); + + Optional builtInViewModel = testViewModel.journalFilesProperty().stream() + .filter(vm -> vm.isBuiltInListProperty().get()) + .findFirst(); + + assertTrue(builtInViewModel.isPresent()); + assertFalse(builtInViewModel.get().isEnabled()); + } + + @Test + void enabledExternalListFiltersAbbreviationsWhenDisabled() throws IOException { + AbbreviationsFileViewModel fileViewModel = mock(AbbreviationsFileViewModel.class); + JournalAbbreviationRepository testRepository = mock(JournalAbbreviationRepository.class); + + when(fileViewModel.getAbsolutePath()).thenReturn(Optional.of(Path.of("test.csv"))); + when(fileViewModel.isBuiltInListProperty()).thenReturn(new SimpleBooleanProperty(false)); + when(fileViewModel.isEnabled()).thenReturn(true); + + JournalAbbreviationPreferences testPreferences = mock(JournalAbbreviationPreferences.class); + when(testPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList("test.csv")); + + TaskExecutor taskExecutor = new CurrentThreadTaskExecutor(); + JournalAbbreviationsTabViewModel testViewModel = new JournalAbbreviationsTabViewModel( + testPreferences, dialogService, taskExecutor, testRepository); + + testViewModel.journalFilesProperty().add(fileViewModel); + + testViewModel.storeSettings(); + + verify(testPreferences).setSourceEnabled(eq("test.csv"), anyBoolean()); + } + + @Test + void disabledSourceAffectsAbbreviationFiltering() throws IOException { + CsvFileNameAndContent testFile = new CsvFileNameAndContent("unique-journal.csv", + new TestAbbreviation("Unique Journal Title", "Unique J. Title")); + Path testFilePath = createTestFile(testFile); + + JournalAbbreviationPreferences mockPrefs = mock(JournalAbbreviationPreferences.class); + when(mockPrefs.isSourceEnabled(anyString())).thenReturn(true); + when(mockPrefs.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); + + JournalAbbreviationsTabViewModel testViewModel = new JournalAbbreviationsTabViewModel( + mockPrefs, dialogService, new CurrentThreadTaskExecutor(), repository); + + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(testFilePath)); + testViewModel.addNewFile(); + testViewModel.selectLastJournalFile(); + + AbbreviationsFileViewModel fileViewModel = testViewModel.currentFileProperty().get(); + + assertTrue(fileViewModel.isEnabled()); + + fileViewModel.setEnabled(false); + assertFalse(fileViewModel.isEnabled()); + + String filename = testFilePath.getFileName().toString(); + + testViewModel.storeSettings(); + + verify(mockPrefs).setSourceEnabled(eq(filename), eq(false)); + } } diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 5407b885d1f..a8c58df806c 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -1,11 +1,17 @@ package org.jabref.logic.journals; +import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; import javax.swing.undo.CompoundEdit; +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; + import org.jabref.architecture.AllowedToUseSwing; import org.jabref.gui.journals.AbbreviationType; import org.jabref.gui.journals.UndoableAbbreviator; @@ -25,18 +31,48 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @AllowedToUseSwing("UndoableUnabbreviator and UndoableAbbreviator requires Swing Compound Edit in order test the abbreviation and unabbreviation of journal titles") class JournalAbbreviationRepositoryTest { private JournalAbbreviationRepository repository; - + private JournalAbbreviationPreferences abbreviationPreferences; + private final BibDatabase bibDatabase = new BibDatabase(); private UndoableUnabbreviator undoableUnabbreviator; + + private static final Abbreviation AMERICAN_JOURNAL = new Abbreviation("American Journal of Public Health", "Am. J. Public Health"); + private static final Abbreviation ACS_MATERIALS = new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"); + private static final Abbreviation ANTIOXIDANTS = new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"); + private static final Abbreviation PHYSICAL_REVIEW = new Abbreviation("Physical Review B", "Phys. Rev. B"); + + /** + * Creates a test repository with pre-defined abbreviations and all sources enabled + */ + private JournalAbbreviationRepository createTestRepository() { + JournalAbbreviationRepository testRepo = new JournalAbbreviationRepository(); + + testRepo.addCustomAbbreviations(List.of( + AMERICAN_JOURNAL, + ACS_MATERIALS, + ANTIOXIDANTS, + PHYSICAL_REVIEW + ), JournalAbbreviationRepository.BUILTIN_LIST_ID, true); + + return testRepo; + } @BeforeEach void setUp() { - repository = JournalAbbreviationLoader.loadBuiltInRepository(); + repository = new JournalAbbreviationRepository(); + + abbreviationPreferences = mock(JournalAbbreviationPreferences.class); + when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); + when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); + undoableUnabbreviator = new UndoableUnabbreviator(repository); } @@ -161,28 +197,55 @@ void duplicateKeysWithShortestUniqueAbbreviation() { @Test void getFromFullName() { - assertEquals(new Abbreviation("American Journal of Public Health", "Am. J. Public Health"), repository.get("American Journal of Public Health").get()); + repository = createTestRepository(); + + Optional result = repository.get("American Journal of Public Health"); + assertTrue(result.isPresent(), "Repository should contain American Journal of Public Health"); + assertEquals(AMERICAN_JOURNAL, result.get()); } @Test void getFromAbbreviatedName() { - assertEquals(new Abbreviation("American Journal of Public Health", "Am. J. Public Health"), repository.get("Am. J. Public Health").get()); + repository = createTestRepository(); + + Optional result = repository.get("Am. J. Public Health"); + assertTrue(result.isPresent(), "Repository should find the abbreviation"); + assertEquals(AMERICAN_JOURNAL, result.get()); } @Test void abbreviationsWithEscapedAmpersand() { - assertEquals(new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"), repository.get("ACS Applied Materials & Interfaces").get()); - assertEquals(new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"), repository.get("ACS Applied Materials \\& Interfaces").get()); - assertEquals(new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"), repository.get("Antioxidants & Redox Signaling").get()); - assertEquals(new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"), repository.get("Antioxidants \\& Redox Signaling").get()); + repository = createTestRepository(); + + Optional result1 = repository.get("ACS Applied Materials & Interfaces"); + assertTrue(result1.isPresent()); + assertEquals(ACS_MATERIALS, result1.get()); + + Optional result2 = repository.get("ACS Applied Materials \\& Interfaces"); + assertTrue(result2.isPresent()); + assertEquals(ACS_MATERIALS, result2.get()); + + Optional result3 = repository.get("Antioxidants & Redox Signaling"); + assertTrue(result3.isPresent()); + assertEquals(ANTIOXIDANTS, result3.get()); + + Optional result4 = repository.get("Antioxidants \\& Redox Signaling"); + assertTrue(result4.isPresent()); + assertEquals(ANTIOXIDANTS, result4.get()); repository.addCustomAbbreviation(new Abbreviation("Long & Name", "L. N.", "LN")); - assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), repository.get("Long & Name").get()); - assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), repository.get("Long \\& Name").get()); + Optional result5 = repository.get("Long & Name"); + assertTrue(result5.isPresent()); + assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), result5.get()); + + Optional result6 = repository.get("Long \\& Name"); + assertTrue(result6.isPresent()); + assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), result6.get()); } @Test void journalAbbreviationWithEscapedAmpersand() { + repository = createTestRepository(); UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(repository, AbbreviationType.DEFAULT, false); BibEntry entryWithEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article); @@ -196,17 +259,21 @@ void journalAbbreviationWithEscapedAmpersand() { @Test void journalUnabbreviate() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article); abbreviatedJournalEntry.setField(StandardField.JOURNAL, "ACS Appl. Mater. Interfaces"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } @Test void journalAbbreviateWithoutEscapedAmpersand() { + repository = createTestRepository(); UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(repository, AbbreviationType.DEFAULT, false); BibEntry entryWithoutEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article) @@ -220,6 +287,7 @@ void journalAbbreviateWithoutEscapedAmpersand() { @Test void journalAbbreviateWithEmptyFJournal() { + repository = createTestRepository(); UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(repository, AbbreviationType.DEFAULT, true); BibEntry entryWithoutEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article) @@ -235,47 +303,59 @@ void journalAbbreviateWithEmptyFJournal() { @Test void unabbreviateWithJournalExistsAndFJournalNot() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Appl. Mater. Interfaces"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } @Test void unabbreviateWithJournalExistsAndFJournalExists() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Appl. Mater. Interfaces") .withField(AMSField.FJOURNAL, "ACS Applied Materials & Interfaces"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } @Test void journalDotlessAbbreviation() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Appl Mater Interfaces"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } @Test void journalDotlessAbbreviationWithCurlyBraces() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "{ACS Appl Mater Interfaces}"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } /** @@ -283,14 +363,17 @@ void journalDotlessAbbreviationWithCurlyBraces() { */ @Test void titleEmbeddedWithCurlyBracesHavingNoChangesKeepsBraces() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.InCollection) .withField(StandardField.JOURNAL, "{The Visualization Handbook}"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.InCollection) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.InCollection) .withField(StandardField.JOURNAL, "{The Visualization Handbook}"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } /** @@ -298,26 +381,32 @@ void titleEmbeddedWithCurlyBracesHavingNoChangesKeepsBraces() { */ @Test void titleWithNestedCurlyBracesHavingNoChangesKeepsBraces() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.InProceedings) .withField(StandardField.BOOKTITLE, "2015 {IEEE} International Conference on Digital Signal Processing, {DSP} 2015, Singapore, July 21-24, 2015"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.InProceedings) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.InProceedings) .withField(StandardField.BOOKTITLE, "2015 {IEEE} International Conference on Digital Signal Processing, {DSP} 2015, Singapore, July 21-24, 2015"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } @Test void dotlessForPhysRevB() { + repository = createTestRepository(); + undoableUnabbreviator = new UndoableUnabbreviator(repository); + BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "Phys Rev B"); undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit()); - BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) + BibEntry expectedUnabbreviatedJournalEntry = new BibEntry(StandardEntryType.Article) .withField(StandardField.JOURNAL, "Physical Review B"); - assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry); + assertEquals(expectedUnabbreviatedJournalEntry, abbreviatedJournalEntry); } @ParameterizedTest @@ -373,4 +462,131 @@ static Stream provideAbbreviationTestCases() { ) ); } + + @Test + void addCustomAbbreviationsWithEnabledState() { + String sourceKey = "test-source"; + + repository.addCustomAbbreviations(List.of( + new Abbreviation("Journal One", "J. One"), + new Abbreviation("Journal Two", "J. Two") + ), sourceKey, true); + + assertEquals("J. One", repository.getDefaultAbbreviation("Journal One").orElse("WRONG")); + assertEquals("J. Two", repository.getDefaultAbbreviation("Journal Two").orElse("WRONG")); + + assertTrue(repository.isSourceEnabled(sourceKey)); + } + + @Test + void disablingSourcePreventsAccessToAbbreviations() { + String sourceKey = "test-source"; + + repository.addCustomAbbreviations(List.of( + new Abbreviation("Unique Journal", "U. J.") + ), sourceKey, true); + + assertEquals("U. J.", repository.getDefaultAbbreviation("Unique Journal").orElse("WRONG")); + + repository.setSourceEnabled(sourceKey, false); + + assertFalse(repository.isSourceEnabled(sourceKey)); + + assertEquals("WRONG", repository.getDefaultAbbreviation("Unique Journal").orElse("WRONG")); + } + + @Test + void reenablingSourceRestoresAccessToAbbreviations() { + String sourceKey = "test-source"; + + repository.addCustomAbbreviations(List.of( + new Abbreviation("Disabled Journal", "D. J.") + ), sourceKey, true); + + repository.setSourceEnabled(sourceKey, false); + + assertEquals("WRONG", repository.getDefaultAbbreviation("Disabled Journal").orElse("WRONG")); + + repository.setSourceEnabled(sourceKey, true); + + assertTrue(repository.isSourceEnabled(sourceKey)); + + assertEquals("D. J.", repository.getDefaultAbbreviation("Disabled Journal").orElse("WRONG")); + } + + @Test + void builtInListCanBeToggled() { + repository = createTestRepository(); + + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + + String journalName = "American Journal of Public Health"; + String abbreviation = "Am. J. Public Health"; + + assertEquals(abbreviation, repository.getDefaultAbbreviation(journalName).orElse("WRONG")); + + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); + + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + + assertEquals("WRONG", repository.getDefaultAbbreviation(journalName).orElse("WRONG")); + + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, true); + + assertEquals(abbreviation, repository.getDefaultAbbreviation(journalName).orElse("WRONG")); + } + + /** + * This test specifically verifies that disabling sources directly affects how abbreviations are accessed. + * We explicitly create a new test repository and check enabled state at each step. + */ + @Test + void multipleSourcesCanBeToggled() { + JournalAbbreviationRepository testRepo = new JournalAbbreviationRepository(); + + String sourceKey1 = "source-1-special"; + String sourceKey2 = "source-2-special"; + + testRepo.addCustomAbbreviations(List.of( + new Abbreviation("Unique Journal Source One XYZ", "UniqueJS1") + ), sourceKey1, true); + + testRepo.addCustomAbbreviations(List.of( + new Abbreviation("Unique Journal Source Two ABC", "UniqueJS2") + ), sourceKey2, true); + + assertTrue(testRepo.isSourceEnabled(sourceKey1), "Source 1 should be enabled initially"); + assertTrue(testRepo.isSourceEnabled(sourceKey2), "Source 2 should be enabled initially"); + + // Verify both abbreviations are accessible + assertEquals("UniqueJS1", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); + assertEquals("UniqueJS2", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); + + testRepo.setSourceEnabled(sourceKey1, false); + + assertFalse(testRepo.isSourceEnabled(sourceKey1), "Source 1 should be disabled"); + assertTrue(testRepo.isSourceEnabled(sourceKey2), "Source 2 should remain enabled"); + + assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); + assertEquals("UniqueJS2", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); + + testRepo.setSourceEnabled(sourceKey2, false); + + assertFalse(testRepo.isSourceEnabled(sourceKey1), "Source 1 should remain disabled"); + assertFalse(testRepo.isSourceEnabled(sourceKey2), "Source 2 should be disabled"); + + assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); + assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); + } + + @Test + void noEnabledSourcesReturnsEmptyAbbreviation() { + JournalAbbreviationRepository testRepo = createTestRepository(); + + assertTrue(testRepo.get("American Journal of Public Health").isPresent()); + + testRepo.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); + + assertFalse(testRepo.get("American Journal of Public Health").isPresent()); + } } From 1a5edb3b328d325f3176c2ff6f3160362128956e Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 17:46:14 +0800 Subject: [PATCH 02/41] fix: prevent unabbreviation from disabled sources Journal abbreviations coming from sources that the user has disabled should neither be generated nor reversed.\n\nThis patch:\n 1. Makes UndoableUnabbreviator skip entries whose source is disabled\n2. Introduces AbbreviationWithSource to persist source metadata\n3. Filters candidates early in AbbreviateAction\n4. Refreshes repository state when a source is toggled in the UI --- .../jabref/gui/journals/AbbreviateAction.java | 96 ++++++++++++++- .../gui/journals/UndoableUnabbreviator.java | 12 +- .../journals/JournalAbbreviationsTab.java | 19 ++- .../JournalAbbreviationRepository.java | 114 +++++++++++++++--- 4 files changed, 210 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 299330cce9b..194edcb8cee 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -1,6 +1,10 @@ package org.jabref.gui.journals; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.Supplier; import java.nio.file.Path; @@ -13,6 +17,7 @@ import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.actions.StandardActions; import org.jabref.gui.undo.NamedCompound; +import org.jabref.logic.journals.Abbreviation; import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; import org.jabref.logic.journals.JournalAbbreviationLoader; @@ -21,6 +26,7 @@ import org.jabref.logic.util.TaskExecutor; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; import org.slf4j.Logger; @@ -79,7 +85,7 @@ public void execute() { BackgroundTask.wrap(() -> abbreviate(stateManager.getActiveDatabase().get(), stateManager.getSelectedEntries())) .onSuccess(dialogService::notify) .executeWith(taskExecutor)); - } else if (action == StandardActions.UNABBREVIATE) { + } else if (action == StandardActions.UNABBREVIATE) { if (!areAnyJournalSourcesEnabled()) { dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled")); return; @@ -120,16 +126,77 @@ private String abbreviate(BibDatabaseContext databaseContext, List ent return Localization.lang("Abbreviated %0 journal names.", String.valueOf(count)); } + /** + * Unabbreviate journal names in entries, respecting the enabled/disabled state of sources. + * Only unabbreviates entries from enabled sources. + */ private String unabbreviate(BibDatabaseContext databaseContext, List entries) { - // Reload repository to ensure latest preferences are used - abbreviationRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); + List filteredEntries = new ArrayList<>(); + + JournalAbbreviationRepository freshRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); + + Map sourceEnabledStates = new HashMap<>(); + String builtInId = JournalAbbreviationRepository.BUILTIN_LIST_ID; + sourceEnabledStates.put(builtInId, journalAbbreviationPreferences.isSourceEnabled(builtInId)); + + for (String listPath : journalAbbreviationPreferences.getExternalJournalLists()) { + if (listPath != null && !listPath.isBlank()) { + String fileName = Path.of(listPath).getFileName().toString(); + sourceEnabledStates.put(fileName, journalAbbreviationPreferences.isSourceEnabled(fileName)); + } + } + + var allAbbreviationsWithSources = freshRepository.getAllAbbreviationsWithSources(); + Map> textToSourceMap = new HashMap<>(); + + for (var abbrWithSource : allAbbreviationsWithSources) { + Abbreviation abbr = abbrWithSource.getAbbreviation(); + addToMap(textToSourceMap, abbr.getName(), abbrWithSource); + addToMap(textToSourceMap, abbr.getAbbreviation(), abbrWithSource); + addToMap(textToSourceMap, abbr.getDotlessAbbreviation(), abbrWithSource); + addToMap(textToSourceMap, abbr.getShortestUniqueAbbreviation(), abbrWithSource); + } + + for (BibEntry entry : entries) { + boolean includeEntry = true; - UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(abbreviationRepository); - + for (Field journalField : FieldFactory.getJournalNameFields()) { + if (!entry.hasField(journalField)) { + continue; + } + + String text = entry.getFieldLatexFree(journalField).orElse(""); + List possibleSources = + textToSourceMap.getOrDefault(text, Collections.emptyList()); + + if (!possibleSources.isEmpty()) { + boolean allSourcesDisabled = true; + for (var abbrWithSource : possibleSources) { + String source = abbrWithSource.getSource(); + if (sourceEnabledStates.getOrDefault(source, true)) { + allSourcesDisabled = false; + break; + } + } + + if (allSourcesDisabled) { + includeEntry = false; + break; + } + } + } + + if (includeEntry) { + filteredEntries.add(entry); + } + } + + UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(freshRepository); NamedCompound ce = new NamedCompound(Localization.lang("Unabbreviate journal names")); - int count = entries.stream().mapToInt(entry -> + int count = filteredEntries.stream().mapToInt(entry -> (int) FieldFactory.getJournalNameFields().stream().filter(journalField -> undoableAbbreviator.unabbreviate(databaseContext.getDatabase(), entry, journalField, ce)).count()).sum(); + if (count == 0) { return Localization.lang("No journal names could be unabbreviated."); } @@ -140,6 +207,23 @@ private String unabbreviate(BibDatabaseContext databaseContext, List e return Localization.lang("Unabbreviated %0 journal names.", String.valueOf(count)); } + /** + * Helper method to add an abbreviation to the text-to-source map + * + * @param map The map to add to + * @param text The text to use as key + * @param abbrWithSource The abbreviation with source to add + */ + private void addToMap(Map> map, + String text, + JournalAbbreviationRepository.AbbreviationWithSource abbrWithSource) { + if (text == null || text.isBlank()) { + return; + } + + map.computeIfAbsent(text, k -> new ArrayList<>()).add(abbrWithSource); + } + /** * Checks if any journal abbreviation source is enabled in the preferences. * This includes both the built-in list and any external journal lists. diff --git a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java index efea27fb8ee..17bd14f9666 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java @@ -43,13 +43,13 @@ public boolean unabbreviate(BibDatabase database, BibEntry entry, Field field, C if (database != null) { text = database.resolveForStrings(text); } - - var abbreviationOpt = journalAbbreviationRepository.get(text); - if (abbreviationOpt.isEmpty()) { + + if (!journalAbbreviationRepository.isAbbreviatedName(text)) { return false; } - - if (!journalAbbreviationRepository.isAbbreviatedName(text)) { + + var abbreviationOpt = journalAbbreviationRepository.get(text); + if (abbreviationOpt.isEmpty()) { return false; } @@ -76,4 +76,4 @@ public boolean restoreFromFJournal(BibEntry entry, Field field, CompoundEdit ce) return true; } -} +} \ No newline at end of file diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index 2fd7a156682..ba2137b6b2f 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -29,6 +29,8 @@ import org.jabref.gui.preferences.PreferencesTab; import org.jabref.gui.util.ColorUtil; import org.jabref.gui.util.ValueTableCellFactory; +import org.jabref.logic.journals.JournalAbbreviationLoader; +import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.TaskExecutor; @@ -282,16 +284,23 @@ private void toggleEnableList() { boolean newEnabledState = !selected.isEnabled(); selected.setEnabled(newEnabledState); - // Update repository immediately to reflect changes in the UI - JournalAbbreviationRepository repository = Injector.instantiateModelOrService(JournalAbbreviationRepository.class); + refreshComboBoxDisplay(); + + JournalAbbreviationPreferences abbreviationPreferences = preferences.getJournalAbbreviationPreferences(); + if (selected.isBuiltInListProperty().get()) { - repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState); + abbreviationPreferences.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState); } else if (selected.getAbsolutePath().isPresent()) { String fileName = selected.getAbsolutePath().get().getFileName().toString(); - repository.setSourceEnabled(fileName, newEnabledState); + abbreviationPreferences.setSourceEnabled(fileName, newEnabledState); } - refreshComboBoxDisplay(); + JournalAbbreviationRepository newRepository = + JournalAbbreviationLoader.loadRepository(abbreviationPreferences); + + Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository); + + abbreviationRepository = newRepository; viewModel.markAsDirty(); } diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 40e641710a3..315e2d2eef8 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -1,6 +1,7 @@ package org.jabref.logic.journals; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collection; import java.util.Comparator; import java.util.HashMap; @@ -120,14 +121,18 @@ public boolean isAbbreviatedName(String journalName) { .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) .anyMatch(abbreviation -> isMatchedAbbreviated(journal, abbreviation)); - if (!isSourceEnabled(BUILTIN_LIST_ID)) { + boolean builtInEnabled = isSourceEnabled(BUILTIN_LIST_ID); + if (!builtInEnabled) { return isCustomAbbreviated; } - return isCustomAbbreviated || - abbreviationToAbbreviationObject.containsKey(journal) || - dotlessToAbbreviationObject.containsKey(journal) || - shortestUniqueToAbbreviationObject.containsKey(journal); + boolean inAbbreviationMap = abbreviationToAbbreviationObject.containsKey(journal); + boolean inDotlessMap = dotlessToAbbreviationObject.containsKey(journal); + boolean inShortestMap = shortestUniqueToAbbreviationObject.containsKey(journal); + + boolean isAbbreviated = isCustomAbbreviated || inAbbreviationMap || inDotlessMap || inShortestMap; + + return isAbbreviated; } /** @@ -147,13 +152,14 @@ public boolean isKnownName(String journalName) { /** * Attempts to get the abbreviation of the journal given. * if no exact match is found, attempts a fuzzy match on full journal names. + * This method respects the enabled/disabled state of journal abbreviation sources. * * @param input The journal name (either full name or abbreviated name). */ public Optional get(String input) { // Clean up input: trim and unescape ampersand String journal = input.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&"); - + Optional customAbbreviation = customAbbreviations.stream() .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) .filter(abbreviation -> isMatched(journal, abbreviation)) @@ -167,14 +173,10 @@ public Optional get(String input) { return Optional.empty(); } - Optional builtInAbbreviation = Optional.empty(); - - if (isSourceEnabled(BUILTIN_LIST_ID)) { - builtInAbbreviation = Optional.ofNullable(fullToAbbreviationObject.get(journal)) - .or(() -> Optional.ofNullable(abbreviationToAbbreviationObject.get(journal))) - .or(() -> Optional.ofNullable(dotlessToAbbreviationObject.get(journal))) - .or(() -> Optional.ofNullable(shortestUniqueToAbbreviationObject.get(journal))); - } + Optional builtInAbbreviation = Optional.ofNullable(fullToAbbreviationObject.get(journal)) + .or(() -> Optional.ofNullable(abbreviationToAbbreviationObject.get(journal))) + .or(() -> Optional.ofNullable(dotlessToAbbreviationObject.get(journal))) + .or(() -> Optional.ofNullable(shortestUniqueToAbbreviationObject.get(journal))); if (builtInAbbreviation.isPresent()) { return builtInAbbreviation; @@ -182,6 +184,28 @@ public Optional get(String input) { return findAbbreviationFuzzyMatched(journal); } + + /** + * Specialized method for unabbreviation that first checks if a journal name is abbreviated + * from an enabled source before attempting to find its full form. + * + * @param input The journal name to check and unabbreviate. + * @return Optional containing the abbreviation object if the input is an abbreviation + * from an enabled source, empty otherwise. + */ + public Optional getForUnabbreviation(String input) { + boolean builtInEnabled = isSourceEnabled(BUILTIN_LIST_ID); + + boolean isAbbreviated = isAbbreviatedName(input); + + if (!isAbbreviated) { + return Optional.empty(); + } + + Optional result = get(input); + + return result; + } private Optional findAbbreviationFuzzyMatched(String input) { Collection enabledCustomAbbreviations = customAbbreviations.stream() @@ -331,4 +355,66 @@ public Set getFullNames() { public Collection getAllLoaded() { return fullToAbbreviationObject.values(); } + + /** + * Class that pairs an abbreviation with its source for tracking purposes + */ + public static class AbbreviationWithSource { + private final Abbreviation abbreviation; + private final String source; + + public AbbreviationWithSource(Abbreviation abbreviation, String source) { + this.abbreviation = abbreviation; + this.source = source; + } + + public Abbreviation getAbbreviation() { + return abbreviation; + } + + public String getSource() { + return source; + } + } + + /** + * Gets all abbreviations from both custom and built-in sources with their sources + * + * @return List of all abbreviations with their sources + */ + public List getAllAbbreviationsWithSources() { + List result = new ArrayList<>(); + + for (Abbreviation abbr : customAbbreviations) { + String source = abbreviationSources.getOrDefault(abbr, "UNKNOWN"); + result.add(new AbbreviationWithSource(abbr, source)); + } + + for (Abbreviation abbr : fullToAbbreviationObject.values()) { + result.add(new AbbreviationWithSource(abbr, BUILTIN_LIST_ID)); + } + + return result; + } + + /** + * Gets all abbreviations from both custom and built-in sources + * + * @return List of all abbreviations + */ + public List getAllAbbreviations() { + return getAllAbbreviationsWithSources().stream() + .map(AbbreviationWithSource::getAbbreviation) + .collect(Collectors.toList()); + } + + /** + * Gets the source identifier for a given abbreviation + * + * @param abbreviation The abbreviation to look up + * @return The source key, or BUILTIN_LIST_ID if not found + */ + public String getSourceForAbbreviation(Abbreviation abbreviation) { + return abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID); + } } From ed985ecea6bd93b09b2398c61e005d8f4563ab0e Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 18:17:07 +0800 Subject: [PATCH 03/41] test: add coverage for unabbreviation operations --- .../journals/UndoableUnabbreviatorTest.java | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java new file mode 100644 index 00000000000..92ccd1cc678 --- /dev/null +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -0,0 +1,153 @@ +package org.jabref.gui.journals; + +import java.util.List; + +import javax.swing.undo.CompoundEdit; + +import org.jabref.architecture.AllowedToUseSwing; +import org.jabref.logic.journals.Abbreviation; +import org.jabref.logic.journals.JournalAbbreviationRepository; +import org.jabref.model.database.BibDatabase; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.types.StandardEntryType; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@AllowedToUseSwing("UndoableUnabbreviator requires Swing Compound Edit") +class UndoableUnabbreviatorTest { + + private static final Abbreviation BUILT_IN_1 = new Abbreviation("Journal of Built-in Testing", "J. Built-in Test."); + private static final Abbreviation BUILT_IN_2 = new Abbreviation("Archives of Built-in Science", "Arch. Built-in Sci."); + + private static final Abbreviation CUSTOM_1 = new Abbreviation("Journal of Custom Testing", "J. Custom Test."); + private static final Abbreviation CUSTOM_2 = new Abbreviation("Archives of Custom Science", "Arch. Custom Sci."); + + private static final String CUSTOM_SOURCE = "custom-source"; + private JournalAbbreviationRepository repository; + private UndoableUnabbreviator unabbreviator; + private BibDatabase database; + private CompoundEdit compoundEdit; + + @BeforeEach + void setUp() { + repository = new JournalAbbreviationRepository(); + repository.addCustomAbbreviations(List.of(BUILT_IN_1, BUILT_IN_2), + JournalAbbreviationRepository.BUILTIN_LIST_ID, + true); + + repository.addCustomAbbreviations(List.of(CUSTOM_1, CUSTOM_2), + CUSTOM_SOURCE, + true); + + unabbreviator = new UndoableUnabbreviator(repository); + database = new BibDatabase(); + compoundEdit = new CompoundEdit(); + } + + private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { + BibEntry entry = new BibEntry(StandardEntryType.Article); + entry.setField(StandardField.JOURNAL, abbreviatedJournal); + return entry; + } + + @Test + void unabbreviateWithBothSourcesEnabled() { + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), + "Built-in source should be enabled"); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE), + "Custom source should be enabled"); + + BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); + boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); + + assertTrue(builtInResult, "Unabbreviation of built-in abbreviation should succeed"); + assertEquals(BUILT_IN_1.getName(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal name should be replaced with full name"); + + BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); + boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); + + assertTrue(customResult, "Unabbreviation of custom abbreviation should succeed"); + assertEquals(CUSTOM_1.getName(), customEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal name should be replaced with full name"); + } + + @Test + void unabbreviateWithOnlyBuiltInSourceEnabled() { + repository.setSourceEnabled(CUSTOM_SOURCE, false); + + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), + "Built-in source should be enabled"); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE), + "Custom source should be disabled"); + + BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); + boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); + + assertTrue(builtInResult, "Unabbreviation of built-in abbreviation should succeed"); + assertEquals(BUILT_IN_1.getName(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal name should be replaced with full name"); + + BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); + boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); + + assertFalse(customResult, "Unabbreviation of custom abbreviation should fail when source is disabled"); + assertEquals(CUSTOM_1.getAbbreviation(), customEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal abbreviation should remain unchanged"); + } + + @Test + void unabbreviateWithOnlyCustomSourceEnabled() { + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); + + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), + "Built-in source should be disabled"); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE), + "Custom source should be enabled"); + + BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); + boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); + + assertFalse(builtInResult, "Unabbreviation of built-in abbreviation should fail when built-in source is disabled"); + assertEquals(BUILT_IN_1.getAbbreviation(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal abbreviation should remain unchanged"); + + BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); + boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); + + assertTrue(customResult, "Unabbreviation of custom abbreviation should succeed"); + assertEquals(CUSTOM_1.getName(), customEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal name should be replaced with full name"); + } + + @Test + void unabbreviateWithBothSourcesDisabled() { + repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); + repository.setSourceEnabled(CUSTOM_SOURCE, false); + + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), + "Built-in source should be disabled"); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE), + "Custom source should be disabled"); + + BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); + boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); + + assertFalse(builtInResult, "Unabbreviation should fail when all sources are disabled"); + assertEquals(BUILT_IN_1.getAbbreviation(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal abbreviation should remain unchanged"); + + BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); + boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); + + assertFalse(customResult, "Unabbreviation should fail when all sources are disabled"); + assertEquals(CUSTOM_1.getAbbreviation(), customEntry.getField(StandardField.JOURNAL).orElse(""), + "Journal abbreviation should remain unchanged"); + } +} \ No newline at end of file From 5828a7ebf106f2d0a08e84a94eac48b9960164c2 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 18:17:25 +0800 Subject: [PATCH 04/41] refactor: address issues raised by the bot for existing testing code --- .../gui/journals/AbbreviateActionTest.java | 38 ++++++-- .../JournalAbbreviationRepositoryTest.java | 97 ++++++++++++++++++- 2 files changed, 127 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 0f4f3416a19..3ab60beaa96 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -6,12 +6,14 @@ import javafx.collections.FXCollections; +import org.jabref.architecture.AllowedToUseSwing; import org.jabref.gui.DialogService; import org.jabref.gui.LibraryTab; import org.jabref.gui.StateManager; import org.jabref.gui.actions.StandardActions; import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; +import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.TaskExecutor; import org.jabref.model.database.BibDatabaseContext; @@ -20,14 +22,14 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -public class AbbreviateActionTest { +@AllowedToUseSwing("Uses UndoManager from javax.swing") +class AbbreviateActionTest { @Mock private DialogService dialogService; @@ -48,7 +50,7 @@ public class AbbreviateActionTest { private UndoManager undoManager; @BeforeEach - public void setUp() { + void setUp() { MockitoAnnotations.openMocks(this); when(stateManager.getSelectedEntries()).thenReturn(FXCollections.observableArrayList()); @@ -56,7 +58,7 @@ public void setUp() { } @Test - public void unabbreviateWithAllSourcesDisabledShowsNotification() { + void unabbreviateWithAllSourcesDisabledShowsNotification() { when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(false); @@ -71,11 +73,11 @@ public void unabbreviateWithAllSourcesDisabledShowsNotification() { action.execute(); - verify(dialogService).notify(eq("Cannot unabbreviate: all journal lists are disabled")); + verify(dialogService).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled"))); } @Test - public void unabbreviateWithOneSourceEnabledExecutesTask() { + void unabbreviateWithOneSourceEnabledExecutesTask() { when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(true); @@ -93,6 +95,28 @@ public void unabbreviateWithOneSourceEnabledExecutesTask() { action.execute(); - verify(dialogService, never()).notify(eq("Cannot unabbreviate: all journal lists are disabled")); + verify(dialogService, never()).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled"))); + } + + @Test + void checksIfAnyJournalSourcesAreEnabled() { + when(abbreviationPreferences.getExternalJournalLists()).thenReturn( + FXCollections.observableArrayList("source1.csv", "source2.csv")); + when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(false); + when(abbreviationPreferences.isSourceEnabled("source1.csv")).thenReturn(false); + when(abbreviationPreferences.isSourceEnabled("source2.csv")).thenReturn(true); + + AbbreviateAction action = new AbbreviateAction( + StandardActions.UNABBREVIATE, + () -> libraryTab, + dialogService, + stateManager, + abbreviationPreferences, + taskExecutor, + undoManager); + + action.execute(); + + verify(dialogService, never()).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled"))); } } \ No newline at end of file diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index a8c58df806c..30694211ecf 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -555,26 +555,33 @@ void multipleSourcesCanBeToggled() { new Abbreviation("Unique Journal Source Two ABC", "UniqueJS2") ), sourceKey2, true); + // Verify initial state assertTrue(testRepo.isSourceEnabled(sourceKey1), "Source 1 should be enabled initially"); assertTrue(testRepo.isSourceEnabled(sourceKey2), "Source 2 should be enabled initially"); - // Verify both abbreviations are accessible + // Verify both abbreviations are accessible when sources are enabled assertEquals("UniqueJS1", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); assertEquals("UniqueJS2", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); + // Disable first source testRepo.setSourceEnabled(sourceKey1, false); + // Verify first source is disabled, second still enabled assertFalse(testRepo.isSourceEnabled(sourceKey1), "Source 1 should be disabled"); assertTrue(testRepo.isSourceEnabled(sourceKey2), "Source 2 should remain enabled"); + // Verify first abbreviation is no longer accessible, second still is assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); assertEquals("UniqueJS2", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); + // Disable second source testRepo.setSourceEnabled(sourceKey2, false); + // Verify both sources are disabled assertFalse(testRepo.isSourceEnabled(sourceKey1), "Source 1 should remain disabled"); assertFalse(testRepo.isSourceEnabled(sourceKey2), "Source 2 should be disabled"); + // Verify both abbreviations are no longer accessible assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); } @@ -589,4 +596,92 @@ void noEnabledSourcesReturnsEmptyAbbreviation() { assertFalse(testRepo.get("American Journal of Public Health").isPresent()); } + + @Test + void getForUnabbreviationRespectsEnabledSources() { + JournalAbbreviationRepository testRepo = createTestRepository(); + + // Abbreviation should be initially present for unabbreviation + String abbreviation = "Am. J. Public Health"; + assertTrue(testRepo.isAbbreviatedName(abbreviation), "Should recognize as abbreviation"); + + Optional result = testRepo.getForUnabbreviation(abbreviation); + assertTrue(result.isPresent(), "Should find abbreviation when source is enabled"); + assertEquals("American Journal of Public Health", result.get().getName()); + + // After disabling the source, it should not be available + testRepo.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); + + // Should not return for unabbreviation since source is disabled + Optional resultAfterDisabling = testRepo.getForUnabbreviation(abbreviation); + assertFalse(resultAfterDisabling.isPresent(), "Should not find abbreviation when source is disabled"); + } + + @Test + void isAbbreviatedNameRespectsEnabledSources() { + JournalAbbreviationRepository testRepo = createTestRepository(); + + String abbreviation = "Am. J. Public Health"; + assertTrue(testRepo.isAbbreviatedName(abbreviation), "Should recognize as abbreviation when source is enabled"); + + testRepo.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); + + assertFalse(testRepo.isAbbreviatedName(abbreviation), "Should not recognize as abbreviation when source is disabled"); + } + + @Test + void getAllAbbreviationsWithSourcesReturnsCorrectSources() { + // Create a custom repository with known abbreviations + JournalAbbreviationRepository testRepo = new JournalAbbreviationRepository(); + + // Clear any existing abbreviations (clear the maps) + testRepo.getCustomAbbreviations().clear(); + + // Add exactly 4 abbreviations with built-in source + testRepo.addCustomAbbreviations(List.of( + AMERICAN_JOURNAL, + ACS_MATERIALS, + ANTIOXIDANTS, + PHYSICAL_REVIEW + ), JournalAbbreviationRepository.BUILTIN_LIST_ID, true); + + // Add 1 abbreviation with custom source + String customSource = "test-custom"; + testRepo.addCustomAbbreviations(List.of( + new Abbreviation("Custom Journal", "Cust. J.") + ), customSource, true); + + List allWithSources = testRepo.getAllAbbreviationsWithSources(); + + // Verify we have at least the number of abbreviations we added + assertTrue(allWithSources.size() >= 5, + "Should have at least 5 abbreviations (got " + allWithSources.size() + ")"); + + // Verify custom abbreviation has correct source + long customCount = allWithSources.stream() + .filter(aws -> customSource.equals(aws.getSource())) + .count(); + assertEquals(1, customCount, "Should have 1 custom source abbreviation"); + + // Verify built-in abbreviations have correct source + long builtInCount = allWithSources.stream() + .filter(aws -> JournalAbbreviationRepository.BUILTIN_LIST_ID.equals(aws.getSource())) + .count(); + assertTrue(builtInCount >= 4, "Should have at least 4 built-in abbreviations"); + + // Verify we can find the specific custom abbreviation + Optional customAbbr = allWithSources.stream() + .filter(aws -> customSource.equals(aws.getSource())) + .findFirst(); + assertTrue(customAbbr.isPresent(), "Should find custom abbreviation with source"); + assertEquals("Custom Journal", customAbbr.get().getAbbreviation().getName()); + + // Verify our specific built-in abbreviations have the correct source + for (Abbreviation abbr : List.of(AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, PHYSICAL_REVIEW)) { + boolean found = allWithSources.stream() + .anyMatch(aws -> aws.getSource().equals(JournalAbbreviationRepository.BUILTIN_LIST_ID) && + aws.getAbbreviation().getName().equals(abbr.getName())); + assertTrue(found, "Should find " + abbr.getName() + " with built-in source"); + } + } } From 2eee01955815c3f6ebc361ff9ffbd410667cdf70 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 18:27:04 +0800 Subject: [PATCH 05/41] fix: ensure journal abbreviation tests work with toggle feature This commit fixes test failures that occurred after implementing the journal abbreviation toggle feature. The changes include: 1. Ensuring the built-in list is always enabled by default in the JournalAbbreviationLoader for backward compatibility 2. Updating journal abbreviation tests to use controlled test data rather than depending on the built-in repository contents 3. Adding a more comprehensive test case for the abbreviation cycle --- .../journals/JournalAbbreviationLoader.java | 5 +++- .../logic/journals/AbbreviationsTest.java | 30 +++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java index c2f9d1f544c..c0f0daad856 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java @@ -115,6 +115,9 @@ public static JournalAbbreviationRepository loadRepository(JournalAbbreviationPr } public static JournalAbbreviationRepository loadBuiltInRepository() { - return loadRepository(new JournalAbbreviationPreferences(Collections.emptyList(), true)); + JournalAbbreviationPreferences prefs = new JournalAbbreviationPreferences(Collections.emptyList(), true); + + prefs.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, true); + return loadRepository(prefs); } } diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index 075148c1f53..2094610fc5d 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -1,26 +1,50 @@ package org.jabref.logic.journals; +import java.util.List; +import java.util.Optional; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class AbbreviationsTest { private JournalAbbreviationRepository repository; + + private static final Abbreviation TEST_JOURNAL = new Abbreviation("Test Journal", "Test J."); + private static final Abbreviation DOTTED_JOURNAL = new Abbreviation("Dotted Journal", "Dotted J."); @BeforeEach void setUp() { - repository = JournalAbbreviationLoader.loadBuiltInRepository(); + repository = new JournalAbbreviationRepository(); + + repository.addCustomAbbreviations(List.of(TEST_JOURNAL, DOTTED_JOURNAL), + JournalAbbreviationRepository.BUILTIN_LIST_ID, + true); } @Test void getNextAbbreviationAbbreviatesJournalTitle() { - assertEquals("2D Mater.", repository.getNextAbbreviation("2D Materials").get()); + Optional abbreviation = repository.getNextAbbreviation("Test Journal"); + assertTrue(abbreviation.isPresent(), "Should find an abbreviation for 'Test Journal'"); + assertEquals("Test J.", abbreviation.get()); } @Test void getNextAbbreviationConvertsAbbreviationToDotlessAbbreviation() { - assertEquals("2D Mater", repository.getNextAbbreviation("2D Mater.").get()); + Optional abbreviation = repository.getNextAbbreviation("Test J."); + assertTrue(abbreviation.isPresent(), "Should find dotless abbreviation for 'Test J.'"); + assertEquals("Test J", abbreviation.get()); + } + + @Test + void getNextAbbreviationWrapsBackToFullName() { + assertEquals("Test J.", repository.getNextAbbreviation("Test Journal").get()); + + assertEquals("Test J", repository.getNextAbbreviation("Test J.").get()); + + assertEquals("Test Journal", repository.getNextAbbreviation("Test J").get()); } } From 623ab210f94a371623c00c81450ce6a0aaae097c Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 18:47:21 +0800 Subject: [PATCH 06/41] fix: resolve issues raised by the bot regarding testing code --- .../JournalAbbreviationsViewModelTabTest.java | 8 +- .../logic/journals/AbbreviationsTest.java | 16 ++-- .../JournalAbbreviationRepositoryTest.java | 77 +++++++------------ 3 files changed, 39 insertions(+), 62 deletions(-) diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 5294681027e..8a6c8190988 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -574,10 +574,12 @@ void storeSettingsSavesEnabledState() throws IOException { viewModel.storeSettings(); Optional path = fileViewModel.getAbsolutePath(); - if (path.isPresent()) { - String fileName = path.get().getFileName().toString(); - verify(abbreviationPreferences).setSourceEnabled(fileName, false); + if (path.isEmpty()) { + return; } + + String fileName = path.get().getFileName().toString(); + verify(abbreviationPreferences).setSourceEnabled(fileName, false); } @Test diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index 2094610fc5d..8171e6c1868 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -27,24 +27,22 @@ void setUp() { @Test void getNextAbbreviationAbbreviatesJournalTitle() { - Optional abbreviation = repository.getNextAbbreviation("Test Journal"); - assertTrue(abbreviation.isPresent(), "Should find an abbreviation for 'Test Journal'"); - assertEquals("Test J.", abbreviation.get()); + assertEquals("Test J.", repository.getNextAbbreviation("Test Journal").orElseThrow(() -> + new AssertionError("Should find an abbreviation for 'Test Journal'"))); } @Test void getNextAbbreviationConvertsAbbreviationToDotlessAbbreviation() { - Optional abbreviation = repository.getNextAbbreviation("Test J."); - assertTrue(abbreviation.isPresent(), "Should find dotless abbreviation for 'Test J.'"); - assertEquals("Test J", abbreviation.get()); + assertEquals("Test J", repository.getNextAbbreviation("Test J.").orElseThrow(() -> + new AssertionError("Should find dotless abbreviation for 'Test J.'"))); } @Test void getNextAbbreviationWrapsBackToFullName() { - assertEquals("Test J.", repository.getNextAbbreviation("Test Journal").get()); + assertEquals("Test J.", repository.getNextAbbreviation("Test Journal").orElseThrow()); - assertEquals("Test J", repository.getNextAbbreviation("Test J.").get()); + assertEquals("Test J", repository.getNextAbbreviation("Test J.").orElseThrow()); - assertEquals("Test Journal", repository.getNextAbbreviation("Test J").get()); + assertEquals("Test Journal", repository.getNextAbbreviation("Test J").orElseThrow()); } } diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 30694211ecf..c7d3bbdca41 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -199,48 +199,43 @@ void duplicateKeysWithShortestUniqueAbbreviation() { void getFromFullName() { repository = createTestRepository(); - Optional result = repository.get("American Journal of Public Health"); - assertTrue(result.isPresent(), "Repository should contain American Journal of Public Health"); - assertEquals(AMERICAN_JOURNAL, result.get()); + assertEquals(AMERICAN_JOURNAL, repository.get("American Journal of Public Health") + .orElseThrow(() -> new AssertionError("Repository should contain American Journal of Public Health"))); } @Test void getFromAbbreviatedName() { repository = createTestRepository(); - Optional result = repository.get("Am. J. Public Health"); - assertTrue(result.isPresent(), "Repository should find the abbreviation"); - assertEquals(AMERICAN_JOURNAL, result.get()); + assertEquals(AMERICAN_JOURNAL, repository.get("Am. J. Public Health") + .orElseThrow(() -> new AssertionError("Repository should find the abbreviation"))); } @Test void abbreviationsWithEscapedAmpersand() { repository = createTestRepository(); - Optional result1 = repository.get("ACS Applied Materials & Interfaces"); - assertTrue(result1.isPresent()); - assertEquals(ACS_MATERIALS, result1.get()); + // Standard ampersand + assertEquals(ACS_MATERIALS, repository.get("ACS Applied Materials & Interfaces").orElseThrow()); - Optional result2 = repository.get("ACS Applied Materials \\& Interfaces"); - assertTrue(result2.isPresent()); - assertEquals(ACS_MATERIALS, result2.get()); + // Escaped ampersand + assertEquals(ACS_MATERIALS, repository.get("ACS Applied Materials \\& Interfaces").orElseThrow()); - Optional result3 = repository.get("Antioxidants & Redox Signaling"); - assertTrue(result3.isPresent()); - assertEquals(ANTIOXIDANTS, result3.get()); + // Another journal with standard ampersand + assertEquals(ANTIOXIDANTS, repository.get("Antioxidants & Redox Signaling").orElseThrow()); - Optional result4 = repository.get("Antioxidants \\& Redox Signaling"); - assertTrue(result4.isPresent()); - assertEquals(ANTIOXIDANTS, result4.get()); + // Another journal with escaped ampersand + assertEquals(ANTIOXIDANTS, repository.get("Antioxidants \\& Redox Signaling").orElseThrow()); - repository.addCustomAbbreviation(new Abbreviation("Long & Name", "L. N.", "LN")); - Optional result5 = repository.get("Long & Name"); - assertTrue(result5.isPresent()); - assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), result5.get()); + // Add custom abbreviation with ampersand + Abbreviation longAndName = new Abbreviation("Long & Name", "L. N.", "LN"); + repository.addCustomAbbreviation(longAndName); - Optional result6 = repository.get("Long \\& Name"); - assertTrue(result6.isPresent()); - assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), result6.get()); + // Test standard ampersand lookup + assertEquals(longAndName, repository.get("Long & Name").orElseThrow()); + + // Test escaped ampersand lookup + assertEquals(longAndName, repository.get("Long \\& Name").orElseThrow()); } @Test @@ -555,33 +550,27 @@ void multipleSourcesCanBeToggled() { new Abbreviation("Unique Journal Source Two ABC", "UniqueJS2") ), sourceKey2, true); - // Verify initial state assertTrue(testRepo.isSourceEnabled(sourceKey1), "Source 1 should be enabled initially"); assertTrue(testRepo.isSourceEnabled(sourceKey2), "Source 2 should be enabled initially"); - // Verify both abbreviations are accessible when sources are enabled assertEquals("UniqueJS1", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); assertEquals("UniqueJS2", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); // Disable first source testRepo.setSourceEnabled(sourceKey1, false); - // Verify first source is disabled, second still enabled assertFalse(testRepo.isSourceEnabled(sourceKey1), "Source 1 should be disabled"); assertTrue(testRepo.isSourceEnabled(sourceKey2), "Source 2 should remain enabled"); - // Verify first abbreviation is no longer accessible, second still is assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); assertEquals("UniqueJS2", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); // Disable second source testRepo.setSourceEnabled(sourceKey2, false); - // Verify both sources are disabled assertFalse(testRepo.isSourceEnabled(sourceKey1), "Source 1 should remain disabled"); assertFalse(testRepo.isSourceEnabled(sourceKey2), "Source 2 should be disabled"); - // Verify both abbreviations are no longer accessible assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source One XYZ").orElse("WRONG")); assertEquals("WRONG", testRepo.getDefaultAbbreviation("Unique Journal Source Two ABC").orElse("WRONG")); } @@ -590,31 +579,28 @@ void multipleSourcesCanBeToggled() { void noEnabledSourcesReturnsEmptyAbbreviation() { JournalAbbreviationRepository testRepo = createTestRepository(); - assertTrue(testRepo.get("American Journal of Public Health").isPresent()); + Optional result = testRepo.get("American Journal of Public Health"); + assertEquals(AMERICAN_JOURNAL, result.orElse(null)); testRepo.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - - assertFalse(testRepo.get("American Journal of Public Health").isPresent()); + assertEquals(Optional.empty(), testRepo.get("American Journal of Public Health")); } @Test void getForUnabbreviationRespectsEnabledSources() { JournalAbbreviationRepository testRepo = createTestRepository(); - // Abbreviation should be initially present for unabbreviation String abbreviation = "Am. J. Public Health"; - assertTrue(testRepo.isAbbreviatedName(abbreviation), "Should recognize as abbreviation"); + + assertEquals(true, testRepo.isAbbreviatedName(abbreviation)); Optional result = testRepo.getForUnabbreviation(abbreviation); - assertTrue(result.isPresent(), "Should find abbreviation when source is enabled"); - assertEquals("American Journal of Public Health", result.get().getName()); + assertEquals("American Journal of Public Health", result.map(Abbreviation::getName).orElse(null)); - // After disabling the source, it should not be available testRepo.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - // Should not return for unabbreviation since source is disabled Optional resultAfterDisabling = testRepo.getForUnabbreviation(abbreviation); - assertFalse(resultAfterDisabling.isPresent(), "Should not find abbreviation when source is disabled"); + assertEquals(Optional.empty(), resultAfterDisabling); } @Test @@ -631,13 +617,10 @@ void isAbbreviatedNameRespectsEnabledSources() { @Test void getAllAbbreviationsWithSourcesReturnsCorrectSources() { - // Create a custom repository with known abbreviations JournalAbbreviationRepository testRepo = new JournalAbbreviationRepository(); - // Clear any existing abbreviations (clear the maps) testRepo.getCustomAbbreviations().clear(); - // Add exactly 4 abbreviations with built-in source testRepo.addCustomAbbreviations(List.of( AMERICAN_JOURNAL, ACS_MATERIALS, @@ -645,7 +628,6 @@ void getAllAbbreviationsWithSourcesReturnsCorrectSources() { PHYSICAL_REVIEW ), JournalAbbreviationRepository.BUILTIN_LIST_ID, true); - // Add 1 abbreviation with custom source String customSource = "test-custom"; testRepo.addCustomAbbreviations(List.of( new Abbreviation("Custom Journal", "Cust. J.") @@ -653,30 +635,25 @@ void getAllAbbreviationsWithSourcesReturnsCorrectSources() { List allWithSources = testRepo.getAllAbbreviationsWithSources(); - // Verify we have at least the number of abbreviations we added assertTrue(allWithSources.size() >= 5, "Should have at least 5 abbreviations (got " + allWithSources.size() + ")"); - // Verify custom abbreviation has correct source long customCount = allWithSources.stream() .filter(aws -> customSource.equals(aws.getSource())) .count(); assertEquals(1, customCount, "Should have 1 custom source abbreviation"); - // Verify built-in abbreviations have correct source long builtInCount = allWithSources.stream() .filter(aws -> JournalAbbreviationRepository.BUILTIN_LIST_ID.equals(aws.getSource())) .count(); assertTrue(builtInCount >= 4, "Should have at least 4 built-in abbreviations"); - // Verify we can find the specific custom abbreviation Optional customAbbr = allWithSources.stream() .filter(aws -> customSource.equals(aws.getSource())) .findFirst(); assertTrue(customAbbr.isPresent(), "Should find custom abbreviation with source"); assertEquals("Custom Journal", customAbbr.get().getAbbreviation().getName()); - // Verify our specific built-in abbreviations have the correct source for (Abbreviation abbr : List.of(AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, PHYSICAL_REVIEW)) { boolean found = allWithSources.stream() .anyMatch(aws -> aws.getSource().equals(JournalAbbreviationRepository.BUILTIN_LIST_ID) && From 0589d14757effe134093ac861e296cdd74c9ec26 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 19:10:27 +0800 Subject: [PATCH 07/41] fix: resolve more issues raised by the bot regarding testing code --- .../jabref/gui/journals/AbbreviateAction.java | 2 +- .../gui/journals/AbbreviateActionTest.java | 6 ++--- .../journals/UndoableUnabbreviatorTest.java | 5 ++--- .../JournalAbbreviationsViewModelTabTest.java | 11 ++++------ .../logic/journals/AbbreviationsTest.java | 22 +++++++++++++------ 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 194edcb8cee..97f64787e9e 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -87,7 +87,7 @@ public void execute() { .executeWith(taskExecutor)); } else if (action == StandardActions.UNABBREVIATE) { if (!areAnyJournalSourcesEnabled()) { - dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled")); + dialogService.notify(Localization.lang("%Cannot unabbreviate: all journal lists are disabled")); return; } diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 3ab60beaa96..338fa4e570c 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -73,7 +73,7 @@ void unabbreviateWithAllSourcesDisabledShowsNotification() { action.execute(); - verify(dialogService).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService).notify(eq(Localization.lang("%Cannot unabbreviate: all journal lists are disabled"))); } @Test @@ -95,7 +95,7 @@ void unabbreviateWithOneSourceEnabledExecutesTask() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService, never()).notify(eq(Localization.lang("%Cannot unabbreviate: all journal lists are disabled"))); } @Test @@ -117,6 +117,6 @@ void checksIfAnyJournalSourcesAreEnabled() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService, never()).notify(eq(Localization.lang("%Cannot unabbreviate: all journal lists are disabled"))); } } \ No newline at end of file diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index 92ccd1cc678..6359785d461 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -51,9 +51,8 @@ void setUp() { } private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { - BibEntry entry = new BibEntry(StandardEntryType.Article); - entry.setField(StandardField.JOURNAL, abbreviatedJournal); - return entry; + return new BibEntry(StandardEntryType.Article) + .withField(StandardField.JOURNAL, abbreviatedJournal); } @Test diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 8a6c8190988..5e5dcfd9d1d 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -573,13 +573,10 @@ void storeSettingsSavesEnabledState() throws IOException { viewModel.storeSettings(); - Optional path = fileViewModel.getAbsolutePath(); - if (path.isEmpty()) { - return; - } - - String fileName = path.get().getFileName().toString(); - verify(abbreviationPreferences).setSourceEnabled(fileName, false); + fileViewModel.getAbsolutePath().ifPresent(path -> { + String fileName = path.getFileName().toString(); + verify(abbreviationPreferences).setSourceEnabled(fileName, false); + }); } @Test diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index 8171e6c1868..f11539a8364 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -27,22 +27,30 @@ void setUp() { @Test void getNextAbbreviationAbbreviatesJournalTitle() { - assertEquals("Test J.", repository.getNextAbbreviation("Test Journal").orElseThrow(() -> - new AssertionError("Should find an abbreviation for 'Test Journal'"))); + Optional abbreviation = repository.getNextAbbreviation("Test Journal"); + assertTrue(abbreviation.isPresent()); + assertEquals("Test J.", abbreviation.get()); } @Test void getNextAbbreviationConvertsAbbreviationToDotlessAbbreviation() { - assertEquals("Test J", repository.getNextAbbreviation("Test J.").orElseThrow(() -> - new AssertionError("Should find dotless abbreviation for 'Test J.'"))); + Optional abbreviation = repository.getNextAbbreviation("Test J."); + assertTrue(abbreviation.isPresent()); + assertEquals("Test J", abbreviation.get()); } @Test void getNextAbbreviationWrapsBackToFullName() { - assertEquals("Test J.", repository.getNextAbbreviation("Test Journal").orElseThrow()); + Optional abbreviation1 = repository.getNextAbbreviation("Test Journal"); + assertTrue(abbreviation1.isPresent()); + assertEquals("Test J.", abbreviation1.get()); - assertEquals("Test J", repository.getNextAbbreviation("Test J.").orElseThrow()); + Optional abbreviation2 = repository.getNextAbbreviation("Test J."); + assertTrue(abbreviation2.isPresent()); + assertEquals("Test J", abbreviation2.get()); - assertEquals("Test Journal", repository.getNextAbbreviation("Test J").orElseThrow()); + Optional abbreviation3 = repository.getNextAbbreviation("Test J"); + assertTrue(abbreviation3.isPresent()); + assertEquals("Test Journal", abbreviation3.get()); } } From 62ccc81c0def7840851d97f3d1cc6bc4969c86fe Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 19:28:59 +0800 Subject: [PATCH 08/41] fix: resolve remaining issues raised by the bot regarding testing code --- .../gui/journals/AbbreviateActionTest.java | 1 - .../journals/UndoableUnabbreviatorTest.java | 24 ++++++---------- .../JournalAbbreviationsViewModelTabTest.java | 28 +++++++++---------- .../logic/journals/AbbreviationsTest.java | 15 ++++------ 4 files changed, 26 insertions(+), 42 deletions(-) diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 338fa4e570c..42b676feafa 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -52,7 +52,6 @@ class AbbreviateActionTest { @BeforeEach void setUp() { MockitoAnnotations.openMocks(this); - when(stateManager.getSelectedEntries()).thenReturn(FXCollections.observableArrayList()); when(stateManager.getActiveDatabase()).thenReturn(Optional.empty()); } diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index 6359785d461..eabc447ce90 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -57,10 +57,8 @@ private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { @Test void unabbreviateWithBothSourcesEnabled() { - assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), - "Built-in source should be enabled"); - assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE), - "Custom source should be enabled"); + assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -81,10 +79,8 @@ void unabbreviateWithBothSourcesEnabled() { void unabbreviateWithOnlyBuiltInSourceEnabled() { repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), - "Built-in source should be enabled"); - assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE), - "Custom source should be disabled"); + assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -105,10 +101,8 @@ void unabbreviateWithOnlyBuiltInSourceEnabled() { void unabbreviateWithOnlyCustomSourceEnabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), - "Built-in source should be disabled"); - assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE), - "Custom source should be enabled"); + assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -130,10 +124,8 @@ void unabbreviateWithBothSourcesDisabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID), - "Built-in source should be disabled"); - assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE), - "Custom source should be disabled"); + assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 5e5dcfd9d1d..182d4beca92 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -181,8 +181,6 @@ public static Stream provideTestFiles() { @BeforeEach void setUpViewModel(@TempDir Path tempFolder) throws Exception { abbreviationPreferences = mock(JournalAbbreviationPreferences.class); - - when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(true); when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); @@ -270,7 +268,7 @@ void openValidFileContainsTheSpecificEntryAndEnoughAbbreviations(TestData testDa assertEquals(1, viewModel.journalFilesProperty().size()); assertEquals(4, viewModel.abbreviationsProperty().size()); - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); + assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); } @ParameterizedTest @@ -303,7 +301,7 @@ void mixedFileUsage(TestData testData) throws IOException { assertEquals(4, viewModel.abbreviationsProperty().size()); // check "arbitrary" abbreviation - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); + assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); // simulate add new file button when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); @@ -328,7 +326,7 @@ void mixedFileUsage(TestData testData) throws IOException { assertEquals(5, viewModel.abbreviationsProperty().size()); // check "arbitrary" abbreviation - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); + assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); } @Test @@ -436,7 +434,7 @@ void editAbbreviationIncludesNewAbbreviationInAbbreviationsList(TestData testDat // addAbbreviation(testAbbreviation); assertEquals(0, viewModel.abbreviationsProperty().size()); - assertFalse(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); + assertEquals(false, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); } @ParameterizedTest @@ -501,14 +499,14 @@ void saveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(TestData te assertEquals(5, viewModel.abbreviationsProperty().size()); viewModel.selectLastJournalFile(); - assertTrue(viewModel.currentFileProperty().get().getAbsolutePath().get().getFileName().toString().endsWith("3.csv")); + assertEquals(true, viewModel.currentFileProperty().get().getAbsolutePath().get().getFileName().toString().endsWith("3.csv")); selectLastAbbreviation(); viewModel.deleteAbbreviation(); viewModel.addAbbreviation(ABBREVIATION_6); // deletion and addition of an entry leads to same size assertEquals(5, viewModel.abbreviationsProperty().size()); - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(ABBREVIATION_6))); + assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(ABBREVIATION_6))); // overwrite all files viewModel.saveJournalAbbreviationFiles(); @@ -550,14 +548,14 @@ void toggleEnabledListChangesEnabledState() { AbbreviationsFileViewModel fileViewModel = viewModel.currentFileProperty().get(); - assertTrue(fileViewModel.isEnabled()); + assertEquals(true, fileViewModel.isEnabled()); fileViewModel.setEnabled(false); - assertFalse(fileViewModel.isEnabled()); + assertEquals(false, fileViewModel.isEnabled()); fileViewModel.setEnabled(true); - assertTrue(fileViewModel.isEnabled()); + assertEquals(true, fileViewModel.isEnabled()); viewModel.markAsDirty(); } @@ -592,8 +590,8 @@ void addBuiltInListInitializesWithCorrectEnabledState() { .filter(vm -> vm.isBuiltInListProperty().get()) .findFirst(); - assertTrue(builtInViewModel.isPresent()); - assertFalse(builtInViewModel.get().isEnabled()); + assertEquals(true, builtInViewModel.isPresent()); + assertEquals(false, builtInViewModel.get().isEnabled()); } @Test @@ -638,10 +636,10 @@ void disabledSourceAffectsAbbreviationFiltering() throws IOException { AbbreviationsFileViewModel fileViewModel = testViewModel.currentFileProperty().get(); - assertTrue(fileViewModel.isEnabled()); + assertEquals(true, fileViewModel.isEnabled()); fileViewModel.setEnabled(false); - assertFalse(fileViewModel.isEnabled()); + assertEquals(false, fileViewModel.isEnabled()); String filename = testFilePath.getFileName().toString(); diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index f11539a8364..8c28b5b29ea 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -28,29 +28,24 @@ void setUp() { @Test void getNextAbbreviationAbbreviatesJournalTitle() { Optional abbreviation = repository.getNextAbbreviation("Test Journal"); - assertTrue(abbreviation.isPresent()); - assertEquals("Test J.", abbreviation.get()); + assertEquals("Test J.", abbreviation.orElse("WRONG")); } @Test void getNextAbbreviationConvertsAbbreviationToDotlessAbbreviation() { Optional abbreviation = repository.getNextAbbreviation("Test J."); - assertTrue(abbreviation.isPresent()); - assertEquals("Test J", abbreviation.get()); + assertEquals("Test J", abbreviation.orElse("WRONG")); } @Test void getNextAbbreviationWrapsBackToFullName() { Optional abbreviation1 = repository.getNextAbbreviation("Test Journal"); - assertTrue(abbreviation1.isPresent()); - assertEquals("Test J.", abbreviation1.get()); + assertEquals("Test J.", abbreviation1.orElse("WRONG")); Optional abbreviation2 = repository.getNextAbbreviation("Test J."); - assertTrue(abbreviation2.isPresent()); - assertEquals("Test J", abbreviation2.get()); + assertEquals("Test J", abbreviation2.orElse("WRONG")); Optional abbreviation3 = repository.getNextAbbreviation("Test J"); - assertTrue(abbreviation3.isPresent()); - assertEquals("Test Journal", abbreviation3.get()); + assertEquals("Test Journal", abbreviation3.orElse("WRONG")); } } From b6a92b8bb55908fb846d987218f3b5ee054dddd9 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 19:46:56 +0800 Subject: [PATCH 09/41] fix: resolve additional issues raised by the bot regarding testing code --- .../jabref/gui/journals/AbbreviateAction.java | 2 +- .../gui/journals/AbbreviateActionTest.java | 6 +-- .../journals/UndoableUnabbreviatorTest.java | 16 ++++---- .../JournalAbbreviationsViewModelTabTest.java | 41 +++++++++++++------ .../JournalAbbreviationRepositoryTest.java | 14 ++++++- 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 97f64787e9e..c5cf3258b11 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -87,7 +87,7 @@ public void execute() { .executeWith(taskExecutor)); } else if (action == StandardActions.UNABBREVIATE) { if (!areAnyJournalSourcesEnabled()) { - dialogService.notify(Localization.lang("%Cannot unabbreviate: all journal lists are disabled")); + dialogService.notify(Localization.lang("%cannot unabbreviate: all journal lists are disabled")); return; } diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 42b676feafa..3ceaaf6b834 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -72,7 +72,7 @@ void unabbreviateWithAllSourcesDisabledShowsNotification() { action.execute(); - verify(dialogService).notify(eq(Localization.lang("%Cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled"))); } @Test @@ -94,7 +94,7 @@ void unabbreviateWithOneSourceEnabledExecutesTask() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("%Cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled"))); } @Test @@ -116,6 +116,6 @@ void checksIfAnyJournalSourcesAreEnabled() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("%Cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled"))); } } \ No newline at end of file diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index eabc447ce90..14aa4603eb0 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -57,8 +57,8 @@ private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { @Test void unabbreviateWithBothSourcesEnabled() { - assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -79,8 +79,8 @@ void unabbreviateWithBothSourcesEnabled() { void unabbreviateWithOnlyBuiltInSourceEnabled() { repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -101,8 +101,8 @@ void unabbreviateWithOnlyBuiltInSourceEnabled() { void unabbreviateWithOnlyCustomSourceEnabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -124,8 +124,8 @@ void unabbreviateWithBothSourcesDisabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 182d4beca92..7acf2a069e6 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -181,17 +181,34 @@ public static Stream provideTestFiles() { @BeforeEach void setUpViewModel(@TempDir Path tempFolder) throws Exception { abbreviationPreferences = mock(JournalAbbreviationPreferences.class); + if (abbreviationPreferences == null) { + throw new IllegalStateException("Failed to initialize abbreviationPreferences"); + } + when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(true); when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); dialogService = mock(DialogService.class); + if (dialogService == null) { + throw new IllegalStateException("Failed to initialize dialogService"); + } + this.tempFolder = tempFolder; + if (this.tempFolder == null) { + throw new IllegalStateException("Temp folder is null"); + } TaskExecutor taskExecutor = new CurrentThreadTaskExecutor(); viewModel = new JournalAbbreviationsTabViewModel(abbreviationPreferences, dialogService, taskExecutor, repository); + if (viewModel == null) { + throw new IllegalStateException("Failed to initialize viewModel"); + } emptyTestFile = createTestFile(new CsvFileNameAndContent("emptyTestFile.csv", "")); + if (emptyTestFile == null) { + throw new IllegalStateException("Failed to create empty test file"); + } } @Test @@ -268,7 +285,7 @@ void openValidFileContainsTheSpecificEntryAndEnoughAbbreviations(TestData testDa assertEquals(1, viewModel.journalFilesProperty().size()); assertEquals(4, viewModel.abbreviationsProperty().size()); - assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); } @ParameterizedTest @@ -301,7 +318,7 @@ void mixedFileUsage(TestData testData) throws IOException { assertEquals(4, viewModel.abbreviationsProperty().size()); // check "arbitrary" abbreviation - assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); // simulate add new file button when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); @@ -326,7 +343,7 @@ void mixedFileUsage(TestData testData) throws IOException { assertEquals(5, viewModel.abbreviationsProperty().size()); // check "arbitrary" abbreviation - assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); } @Test @@ -434,7 +451,7 @@ void editAbbreviationIncludesNewAbbreviationInAbbreviationsList(TestData testDat // addAbbreviation(testAbbreviation); assertEquals(0, viewModel.abbreviationsProperty().size()); - assertEquals(false, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); + assertFalse(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); } @ParameterizedTest @@ -499,7 +516,7 @@ void saveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(TestData te assertEquals(5, viewModel.abbreviationsProperty().size()); viewModel.selectLastJournalFile(); - assertEquals(true, viewModel.currentFileProperty().get().getAbsolutePath().get().getFileName().toString().endsWith("3.csv")); + assertTrue(viewModel.currentFileProperty().get().getAbsolutePath().get().getFileName().toString().endsWith("3.csv")); selectLastAbbreviation(); viewModel.deleteAbbreviation(); viewModel.addAbbreviation(ABBREVIATION_6); @@ -548,14 +565,14 @@ void toggleEnabledListChangesEnabledState() { AbbreviationsFileViewModel fileViewModel = viewModel.currentFileProperty().get(); - assertEquals(true, fileViewModel.isEnabled()); + assertTrue(fileViewModel.isEnabled()); fileViewModel.setEnabled(false); - assertEquals(false, fileViewModel.isEnabled()); + assertFalse(fileViewModel.isEnabled()); fileViewModel.setEnabled(true); - assertEquals(true, fileViewModel.isEnabled()); + assertTrue(fileViewModel.isEnabled()); viewModel.markAsDirty(); } @@ -590,8 +607,8 @@ void addBuiltInListInitializesWithCorrectEnabledState() { .filter(vm -> vm.isBuiltInListProperty().get()) .findFirst(); - assertEquals(true, builtInViewModel.isPresent()); - assertEquals(false, builtInViewModel.get().isEnabled()); + assertTrue(builtInViewModel.isPresent()); + assertFalse(builtInViewModel.get().isEnabled()); } @Test @@ -636,10 +653,10 @@ void disabledSourceAffectsAbbreviationFiltering() throws IOException { AbbreviationsFileViewModel fileViewModel = testViewModel.currentFileProperty().get(); - assertEquals(true, fileViewModel.isEnabled()); + assertTrue(fileViewModel.isEnabled()); fileViewModel.setEnabled(false); - assertEquals(false, fileViewModel.isEnabled()); + assertFalse(fileViewModel.isEnabled()); String filename = testFilePath.getFileName().toString(); diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index c7d3bbdca41..93a0ead84d8 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -67,13 +67,23 @@ private JournalAbbreviationRepository createTestRepository() { @BeforeEach void setUp() { - repository = new JournalAbbreviationRepository(); - abbreviationPreferences = mock(JournalAbbreviationPreferences.class); + if (abbreviationPreferences == null) { + throw new IllegalStateException("Failed to initialize abbreviationPreferences"); + } + when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); + repository = new JournalAbbreviationRepository(); + if (repository == null) { + throw new IllegalStateException("Failed to initialize repository"); + } + undoableUnabbreviator = new UndoableUnabbreviator(repository); + if (undoableUnabbreviator == null) { + throw new IllegalStateException("Failed to initialize undoableUnabbreviator"); + } } @Test From 774f5fbd9585e101ce122177ed061211c40c41b7 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 13 Apr 2025 20:10:32 +0800 Subject: [PATCH 10/41] fix: try to resolve all issues raised by the bot regarding testing code --- .../jabref/gui/journals/AbbreviateAction.java | 2 +- .../gui/journals/AbbreviateActionTest.java | 7 +++--- .../journals/UndoableUnabbreviatorTest.java | 24 +++++++------------ .../JournalAbbreviationsViewModelTabTest.java | 15 ------------ .../JournalAbbreviationRepositoryTest.java | 9 ------- 5 files changed, 13 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index c5cf3258b11..cc1e8f7d538 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -87,7 +87,7 @@ public void execute() { .executeWith(taskExecutor)); } else if (action == StandardActions.UNABBREVIATE) { if (!areAnyJournalSourcesEnabled()) { - dialogService.notify(Localization.lang("%cannot unabbreviate: all journal lists are disabled")); + dialogService.notify(Localization.lang("%cannot unabbreviate: all journal lists are disabled.")); return; } diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 3ceaaf6b834..3da2d0d834e 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -72,7 +72,7 @@ void unabbreviateWithAllSourcesDisabledShowsNotification() { action.execute(); - verify(dialogService).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); } @Test @@ -94,7 +94,8 @@ void unabbreviateWithOneSourceEnabledExecutesTask() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); + verify(dialogService).notify(eq(Localization.lang("Unabbreviating..."))); } @Test @@ -116,6 +117,6 @@ void checksIfAnyJournalSourcesAreEnabled() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled"))); + verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); } } \ No newline at end of file diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index 14aa4603eb0..16d337c12e7 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -57,20 +57,18 @@ private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { @Test void unabbreviateWithBothSourcesEnabled() { - assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); - assertTrue(builtInResult, "Unabbreviation of built-in abbreviation should succeed"); assertEquals(BUILT_IN_1.getName(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), "Journal name should be replaced with full name"); BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); - assertTrue(customResult, "Unabbreviation of custom abbreviation should succeed"); assertEquals(CUSTOM_1.getName(), customEntry.getField(StandardField.JOURNAL).orElse(""), "Journal name should be replaced with full name"); } @@ -79,20 +77,18 @@ void unabbreviateWithBothSourcesEnabled() { void unabbreviateWithOnlyBuiltInSourceEnabled() { repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); - assertTrue(builtInResult, "Unabbreviation of built-in abbreviation should succeed"); assertEquals(BUILT_IN_1.getName(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), "Journal name should be replaced with full name"); BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); - assertFalse(customResult, "Unabbreviation of custom abbreviation should fail when source is disabled"); assertEquals(CUSTOM_1.getAbbreviation(), customEntry.getField(StandardField.JOURNAL).orElse(""), "Journal abbreviation should remain unchanged"); } @@ -101,20 +97,18 @@ void unabbreviateWithOnlyBuiltInSourceEnabled() { void unabbreviateWithOnlyCustomSourceEnabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); - assertFalse(builtInResult, "Unabbreviation of built-in abbreviation should fail when built-in source is disabled"); assertEquals(BUILT_IN_1.getAbbreviation(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), "Journal abbreviation should remain unchanged"); BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); - assertTrue(customResult, "Unabbreviation of custom abbreviation should succeed"); assertEquals(CUSTOM_1.getName(), customEntry.getField(StandardField.JOURNAL).orElse(""), "Journal name should be replaced with full name"); } @@ -124,20 +118,18 @@ void unabbreviateWithBothSourcesDisabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); - assertFalse(builtInResult, "Unabbreviation should fail when all sources are disabled"); assertEquals(BUILT_IN_1.getAbbreviation(), builtInEntry.getField(StandardField.JOURNAL).orElse(""), "Journal abbreviation should remain unchanged"); BibEntry customEntry = createEntryWithAbbreviatedJournal(CUSTOM_1.getAbbreviation()); boolean customResult = unabbreviator.unabbreviate(database, customEntry, StandardField.JOURNAL, compoundEdit); - assertFalse(customResult, "Unabbreviation should fail when all sources are disabled"); assertEquals(CUSTOM_1.getAbbreviation(), customEntry.getField(StandardField.JOURNAL).orElse(""), "Journal abbreviation should remain unchanged"); } diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 7acf2a069e6..56d26f7626b 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -181,34 +181,19 @@ public static Stream provideTestFiles() { @BeforeEach void setUpViewModel(@TempDir Path tempFolder) throws Exception { abbreviationPreferences = mock(JournalAbbreviationPreferences.class); - if (abbreviationPreferences == null) { - throw new IllegalStateException("Failed to initialize abbreviationPreferences"); - } when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); when(abbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)).thenReturn(true); when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); dialogService = mock(DialogService.class); - if (dialogService == null) { - throw new IllegalStateException("Failed to initialize dialogService"); - } this.tempFolder = tempFolder; - if (this.tempFolder == null) { - throw new IllegalStateException("Temp folder is null"); - } TaskExecutor taskExecutor = new CurrentThreadTaskExecutor(); viewModel = new JournalAbbreviationsTabViewModel(abbreviationPreferences, dialogService, taskExecutor, repository); - if (viewModel == null) { - throw new IllegalStateException("Failed to initialize viewModel"); - } emptyTestFile = createTestFile(new CsvFileNameAndContent("emptyTestFile.csv", "")); - if (emptyTestFile == null) { - throw new IllegalStateException("Failed to create empty test file"); - } } @Test diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 93a0ead84d8..bd6217f2df5 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -68,22 +68,13 @@ private JournalAbbreviationRepository createTestRepository() { @BeforeEach void setUp() { abbreviationPreferences = mock(JournalAbbreviationPreferences.class); - if (abbreviationPreferences == null) { - throw new IllegalStateException("Failed to initialize abbreviationPreferences"); - } when(abbreviationPreferences.isSourceEnabled(anyString())).thenReturn(true); when(abbreviationPreferences.getExternalJournalLists()).thenReturn(FXCollections.observableArrayList()); repository = new JournalAbbreviationRepository(); - if (repository == null) { - throw new IllegalStateException("Failed to initialize repository"); - } undoableUnabbreviator = new UndoableUnabbreviator(repository); - if (undoableUnabbreviator == null) { - throw new IllegalStateException("Failed to initialize undoableUnabbreviator"); - } } @Test From 00b8d242a4041dd7ee333db4ff30077bf1a785a3 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sat, 19 Apr 2025 03:09:54 +0800 Subject: [PATCH 11/41] fix: resolve merge conflicts in CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b41a2f620..08cf56474af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added automatic PubMed URL insertion when importing from PubMed if no URL is present. [#12832](https://github.com/JabRef/jabref/issues/12832/) - We added a "LTWA" abbreviation feature in the "Quality > Abbreviate journal names > LTWA" menu [#12273](https://github.com/JabRef/jabref/issues/12273/) - We added path validation to file directories in library properties dialog. [#11840](https://github.com/JabRef/jabref/issues/11840) +- We now support usage of custom CSL styles in the Open/LibreOffice integration. [#12337](https://github.com/JabRef/jabref/issues/12337) ### Changed @@ -70,6 +71,9 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - A tooltip now appears after 300ms (instead of 2s). [#12649](https://github.com/JabRef/jabref/issues/12649) - We improved search in preferences and keybindings. [#12647](https://github.com/JabRef/jabref/issues/12647) - We improved the performance of the LibreOffice integration when inserting CSL citations/bibliography. [#12851](https://github.com/JabRef/jabref/pull/12851) +- 'Affected fields' and 'Do not wrap when saving' are now displayed as tags. [#12550](https://github.com/JabRef/jabref/issues/12550) +- We revamped the UI of the Select Style dialog (in the LibreOffice panel) for CSL styles. [#12951](https://github.com/JabRef/jabref/pull/12951) +- We reduced the delay in populating the list of CSL styles in the Select Style dialog of the LibreOffice panel. [#12951](https://github.com/JabRef/jabref/pull/12951) ### Fixed From e3b62b704f3e04a89ba995bc0593b7a83c3b67d1 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sat, 19 Apr 2025 03:12:24 +0800 Subject: [PATCH 12/41] doc: update issue number for this PR --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08cf56474af..4ba013d45af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,11 +32,11 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We added an option to choose whether to open the file explorer in the files directory or in the last opened directory when attaching files. [#12554](https://github.com/JabRef/jabref/issues/12554) - We enhanced support for parsing XMP metadata from PDF files. [#12829](https://github.com/JabRef/jabref/issues/12829) - We added a "Preview" header in the JStyles tab in the "Select style" dialog, to make it consistent with the CSL styles tab. [#12838](https://github.com/JabRef/jabref/pull/12838) -- We added ability to toggle journal abbreviation lists (including built-in and external CSV files) on/off in preferences. [#{Issue Number}](https://github.com/JabRef/jabref/pull/{Issue Number}) - We added automatic PubMed URL insertion when importing from PubMed if no URL is present. [#12832](https://github.com/JabRef/jabref/issues/12832/) - We added a "LTWA" abbreviation feature in the "Quality > Abbreviate journal names > LTWA" menu [#12273](https://github.com/JabRef/jabref/issues/12273/) - We added path validation to file directories in library properties dialog. [#11840](https://github.com/JabRef/jabref/issues/11840) - We now support usage of custom CSL styles in the Open/LibreOffice integration. [#12337](https://github.com/JabRef/jabref/issues/12337) +- We added ability to toggle journal abbreviation lists (including built-in and external CSV files) on/off in preferences. [#12468](https://github.com/JabRef/jabref/pull/12468) ### Changed From 230c70f7664e6a6578ec008bbe5549fe47fdc7c9 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 20 Apr 2025 03:20:29 +0800 Subject: [PATCH 13/41] fix: make changes to src/main/resources/l10n/JabRef_en.properties --- src/main/resources/l10n/JabRef_en.properties | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 0b4b2bc6b97..be2847fd5a0 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -1487,6 +1487,8 @@ Duplicated\ Journal\ File=Duplicated Journal File Error\ Occurred=Error Occurred Journal\ file\ %s\ already\ added=Journal file %s already added Name\ cannot\ be\ empty=Name cannot be empty +Toggle=Toggle +Toggle\ selected\ list\ on/off=Toggle selected list on/off Display\ keywords\ appearing\ in\ ALL\ entries=Display keywords appearing in ALL entries Display\ keywords\ appearing\ in\ ANY\ entry=Display keywords appearing in ANY entry @@ -1495,6 +1497,7 @@ None\ of\ the\ selected\ entries\ have\ citation\ keys.=None of the selected ent None\ of\ the\ selected\ entries\ have\ DOIs.=None of the selected entries have DOIs. Unabbreviate\ journal\ names=Unabbreviate journal names Unabbreviating...=Unabbreviating... +%cannot\ unabbreviate:\ all\ journal\ lists\ are\ disabled.=%cannot unabbreviate: all journal lists are disabled. Usage=Usage From fd4544d962c82a4029a4604eca48d874fe23c43d Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 20 Apr 2025 03:55:11 +0800 Subject: [PATCH 14/41] fix: correct code style violations detected by OpenRewrite --- .../preferences/journals/JournalAbbreviationsTab.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index ba2137b6b2f..314c517c04b 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -11,6 +11,7 @@ import javafx.collections.transformation.FilteredList; import javafx.event.ActionEvent; import javafx.fxml.FXML; +import javafx.scene.Node; import javafx.scene.control.Button; import javafx.scene.control.CheckBox; import javafx.scene.control.ComboBox; @@ -19,6 +20,7 @@ import javafx.scene.control.ProgressIndicator; import javafx.scene.control.TableColumn; import javafx.scene.control.TableView; +import javafx.scene.control.Tooltip; import javafx.scene.control.cell.TextFieldTableCell; import javafx.scene.layout.HBox; import javafx.scene.paint.Color; @@ -120,14 +122,13 @@ private void setUpTable() { private void setUpToggleButton() { Button toggleButton = new Button(Localization.lang("Toggle")); toggleButton.setOnAction(e -> toggleEnableList()); - toggleButton.setTooltip(new javafx.scene.control.Tooltip(Localization.lang("Toggle selected list on/off"))); + toggleButton.setTooltip(new Tooltip(Localization.lang("Toggle selected list on/off"))); toggleButton.getStyleClass().add("icon-button"); - for (javafx.scene.Node node : getChildren()) { - if (node instanceof HBox) { - HBox hbox = (HBox) node; + for (Node node : getChildren()) { + if (node instanceof HBox hbox) { boolean containsComboBox = false; - for (javafx.scene.Node child : hbox.getChildren()) { + for (Node child : hbox.getChildren()) { if (child == journalFilesBox) { containsComboBox = true; break; From d9e6b202e018128077d92d6fc40c7c25bddbb342 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 20 Apr 2025 04:15:14 +0800 Subject: [PATCH 15/41] fix: correct code style violations detected by OpenRewrite during CI --- .../gui/journals/UndoableUnabbreviatorTest.java | 16 ++++++++-------- .../JournalAbbreviationsViewModelTabTest.java | 2 +- .../JournalAbbreviationRepositoryTest.java | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index 16d337c12e7..5476245fdc2 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -57,8 +57,8 @@ private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { @Test void unabbreviateWithBothSourcesEnabled() { - assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -77,8 +77,8 @@ void unabbreviateWithBothSourcesEnabled() { void unabbreviateWithOnlyBuiltInSourceEnabled() { repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -97,8 +97,8 @@ void unabbreviateWithOnlyBuiltInSourceEnabled() { void unabbreviateWithOnlyCustomSourceEnabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -118,8 +118,8 @@ void unabbreviateWithBothSourcesDisabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 56d26f7626b..18279e3fac1 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -508,7 +508,7 @@ void saveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(TestData te // deletion and addition of an entry leads to same size assertEquals(5, viewModel.abbreviationsProperty().size()); - assertEquals(true, viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(ABBREVIATION_6))); + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(ABBREVIATION_6))); // overwrite all files viewModel.saveJournalAbbreviationFiles(); diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index bd6217f2df5..2f6d1d22083 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -593,7 +593,7 @@ void getForUnabbreviationRespectsEnabledSources() { String abbreviation = "Am. J. Public Health"; - assertEquals(true, testRepo.isAbbreviatedName(abbreviation)); + assertTrue(testRepo.isAbbreviatedName(abbreviation)); Optional result = testRepo.getForUnabbreviation(abbreviation); assertEquals("American Journal of Public Health", result.map(Abbreviation::getName).orElse(null)); From fe6069334e342a700fa0932865d91c4af39001ae Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 20 Apr 2025 04:29:27 +0800 Subject: [PATCH 16/41] fix: correct more code style violations detected by OpenRewrite during CI --- .../logic/journals/JournalAbbreviationRepositoryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 2f6d1d22083..7500413f812 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -657,8 +657,8 @@ void getAllAbbreviationsWithSourcesReturnsCorrectSources() { for (Abbreviation abbr : List.of(AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, PHYSICAL_REVIEW)) { boolean found = allWithSources.stream() - .anyMatch(aws -> aws.getSource().equals(JournalAbbreviationRepository.BUILTIN_LIST_ID) && - aws.getAbbreviation().getName().equals(abbr.getName())); + .anyMatch(aws -> JournalAbbreviationRepository.BUILTIN_LIST_ID.equals(aws.getSource()) && + abbr.getName().equals(aws.getAbbreviation().getName())); assertTrue(found, "Should find " + abbr.getName() + " with built-in source"); } } From 7ba2574c6a537b5365aa3703ab66f8cd569892dc Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 20 Apr 2025 05:07:57 +0800 Subject: [PATCH 17/41] fix: correct Checkstyle errors --- .../jabref/gui/journals/AbbreviateAction.java | 4 +- .../gui/journals/UndoableUnabbreviator.java | 2 +- .../journals/AbbreviationsFileViewModel.java | 12 +++-- .../journals/JournalAbbreviationsTab.java | 4 +- .../JournalAbbreviationsTabViewModel.java | 9 +--- .../journals/JournalAbbreviationLoader.java | 4 +- .../JournalAbbreviationPreferences.java | 14 +++--- .../JournalAbbreviationRepository.java | 29 ++++++------ .../preferences/JabRefCliPreferences.java | 3 +- .../gui/journals/AbbreviateActionTest.java | 2 +- .../journals/UndoableUnabbreviatorTest.java | 2 +- .../JournalAbbreviationsViewModelTabTest.java | 2 +- .../logic/journals/AbbreviationsTest.java | 45 +++++++++++++++++-- .../JournalAbbreviationRepositoryTest.java | 13 +++--- 14 files changed, 82 insertions(+), 63 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 22d3c30805b..521bc45bab9 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -1,12 +1,12 @@ package org.jabref.gui.journals; +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.function.Supplier; -import java.nio.file.Path; import javax.swing.undo.UndoManager; @@ -18,9 +18,9 @@ import org.jabref.gui.actions.StandardActions; import org.jabref.gui.undo.NamedCompound; import org.jabref.logic.journals.Abbreviation; +import org.jabref.logic.journals.JournalAbbreviationLoader; import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; -import org.jabref.logic.journals.JournalAbbreviationLoader; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.BackgroundTask; import org.jabref.logic.util.TaskExecutor; diff --git a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java index 17bd14f9666..e559484d9dc 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java @@ -76,4 +76,4 @@ public boolean restoreFromFJournal(BibEntry entry, Field field, CompoundEdit ce) return true; } -} \ No newline at end of file +} diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index b9ec18267fa..297ab45c9e9 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -4,12 +4,12 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; -import java.util.ArrayList; import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyBooleanProperty; @@ -18,12 +18,10 @@ import javafx.beans.property.SimpleListProperty; import javafx.beans.property.SimpleStringProperty; import javafx.collections.FXCollections; -import javafx.collections.ObservableList; import org.jabref.logic.journals.Abbreviation; import org.jabref.logic.journals.AbbreviationWriter; import org.jabref.logic.journals.JournalAbbreviationLoader; -import org.jabref.logic.journals.JournalAbbreviationRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -104,7 +102,7 @@ public void writeOrCreate() throws IOException { /** * Gets the absolute path of this abbreviation file - * + * * @return The optional absolute path of the file, or empty if it's a built-in list */ public Optional getAbsolutePath() { @@ -122,7 +120,7 @@ public Optional getAbsolutePath() { /** * Checks if this source is enabled - * + * * @return true if the source is enabled */ public boolean isEnabled() { @@ -131,7 +129,7 @@ public boolean isEnabled() { /** * Sets the enabled state of this source - * + * * @param enabled true to enable the source, false to disable it */ public void setEnabled(boolean enabled) { @@ -140,7 +138,7 @@ public void setEnabled(boolean enabled) { /** * Gets the enabled property for binding - * + * * @return the enabled property */ public BooleanProperty enabledProperty() { diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index 314c517c04b..5a6ad337f98 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -4,6 +4,7 @@ import javafx.animation.KeyFrame; import javafx.animation.KeyValue; import javafx.animation.Timeline; +import javafx.application.Platform; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.beans.property.SimpleStringProperty; @@ -37,12 +38,11 @@ import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.TaskExecutor; +import com.airhacks.afterburner.injection.Injector; import com.airhacks.afterburner.views.ViewLoader; import com.tobiasdiez.easybind.EasyBind; import jakarta.inject.Inject; import org.controlsfx.control.textfield.CustomTextField; -import javafx.application.Platform; -import com.airhacks.afterburner.injection.Injector; /** * This class controls the user interface of the journal abbreviations dialog. The UI elements and their layout are diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java index 83174859b0c..0ced35ada50 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -1,15 +1,8 @@ package org.jabref.gui.preferences.journals; import java.io.IOException; -import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -423,4 +416,4 @@ public SimpleBooleanProperty isFileRemovableProperty() { public SimpleBooleanProperty useFJournalProperty() { return useFJournal; } -} \ No newline at end of file +} diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java index 4344198aa25..8cead46f021 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java @@ -8,7 +8,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.Map; import org.jabref.logic.journals.ltwa.LtwaRepository; @@ -38,7 +37,7 @@ public static Collection readAbbreviationsFromCsvFile(Path file) t /** * Loads a journal abbreviation repository based on the given preferences. * Takes into account enabled/disabled state of journal abbreviation sources. - * + * * @param journalAbbreviationPreferences The preferences containing journal list paths and enabled states * @return A repository with loaded abbreviations */ @@ -106,7 +105,6 @@ public static JournalAbbreviationRepository loadRepository(JournalAbbreviationPr repository.addCustomAbbreviations(abbreviations, simpleFilename, enabled); repository.addCustomAbbreviations(Collections.emptyList(), prefixedKey, enabled); - } catch (IOException | InvalidPathException e) { // invalid path might come from unix/windows mixup of prefs LOGGER.error("Cannot read external journal list file {}", filename, e); diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java index 7f1f32e29c8..d3b15012ba1 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java @@ -21,7 +21,7 @@ public class JournalAbbreviationPreferences { /** * Constructs a new JournalAbbreviationPreferences with the given external journal lists and FJournal field preference - * + * * @param externalJournalLists List of paths to external journal abbreviation files * @param useFJournalField Whether to use the FJournal field */ @@ -71,7 +71,7 @@ public void setUseFJournalField(boolean useFJournalField) { /** * Checks if a journal abbreviation source is enabled - * + * * @param sourcePath Path to the abbreviation source * @return true if the source is enabled or has no explicit state (default is enabled) */ @@ -84,7 +84,7 @@ public boolean isSourceEnabled(String sourcePath) { /** * Sets the enabled state for a journal abbreviation source - * + * * @param sourcePath Path to the abbreviation source * @param enabled Whether the source should be enabled */ @@ -98,7 +98,7 @@ public void setSourceEnabled(String sourcePath, boolean enabled) { /** * Gets all enabled/disabled states for journal abbreviation sources - * + * * @return Map of source paths to their enabled states */ public Map getEnabledExternalLists() { @@ -107,7 +107,7 @@ public Map getEnabledExternalLists() { /** * Sets all enabled/disabled states for journal abbreviation sources - * + * * @param enabledLists Map of source paths to their enabled states */ public void setEnabledExternalLists(Map enabledLists) { @@ -121,7 +121,7 @@ public void setEnabledExternalLists(Map enabledLists) { /** * Property that changes whenever the enabled states map changes * Used for binding/listening to changes - * + * * @return A boolean property that toggles when enabled states change */ public BooleanProperty enabledListsChangedProperty() { @@ -130,7 +130,7 @@ public BooleanProperty enabledListsChangedProperty() { /** * Checks if a specific source has an explicit enabled/disabled setting - * + * * @param sourcePath Path to check * @return True if there is an explicit setting for this source */ diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 026752568df..81791f5c640 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -5,7 +5,6 @@ import java.util.Collection; import java.util.Comparator; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -21,19 +20,17 @@ import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * A repository for all journal abbreviations, including add and find methods. */ public class JournalAbbreviationRepository { - static final Pattern QUESTION_MARK = Pattern.compile("\\?"); - /** * Identifier for the built-in abbreviation list */ public static final String BUILTIN_LIST_ID = "BUILTIN_LIST"; + + static final Pattern QUESTION_MARK = Pattern.compile("\\?"); private final Map fullToAbbreviationObject = new HashMap<>(); private final Map abbreviationToAbbreviationObject = new HashMap<>(); @@ -255,9 +252,9 @@ private Optional findBestFuzzyMatched(Collection abb } /** - * Adds a custom abbreviation to the repository - * - * @param abbreviation The abbreviation to add + * Adds a journal abbreviation to the list of custom abbreviations. + * + * @param abbreviation The journal abbreviation to add. */ public void addCustomAbbreviation(Abbreviation abbreviation) { Objects.requireNonNull(abbreviation); @@ -272,7 +269,7 @@ public void addCustomAbbreviation(Abbreviation abbreviation) { /** * Adds a custom abbreviation to the repository with source tracking - * + * * @param abbreviation The abbreviation to add * @param sourcePath The path or identifier of the source * @param enabled Whether the source is enabled @@ -293,7 +290,7 @@ public Collection getCustomAbbreviations() { /** * Adds multiple custom abbreviations to the repository - * + * * @param abbreviationsToAdd The abbreviations to add */ public void addCustomAbbreviations(Collection abbreviationsToAdd) { @@ -302,7 +299,7 @@ public void addCustomAbbreviations(Collection abbreviationsToAdd) /** * Adds abbreviations with a specific source key and enabled state - * + * * @param abbreviationsToAdd Collection of abbreviations to add * @param sourceKey The key identifying the source of these abbreviations * @param enabled Whether the source should be enabled initially @@ -318,7 +315,7 @@ public void addCustomAbbreviations(Collection abbreviationsToAdd, /** * Checks if a journal abbreviation source is enabled - * + * * @param sourceKey The key identifying the source * @return true if the source is enabled or has no explicit state (default is enabled) */ @@ -328,7 +325,7 @@ public boolean isSourceEnabled(String sourceKey) { /** * Sets the enabled state for a journal abbreviation source - * + * * @param sourceKey The key identifying the source * @param enabled Whether the source should be enabled */ @@ -383,7 +380,7 @@ public String getSource() { /** * Gets all abbreviations from both custom and built-in sources with their sources - * + * * @return List of all abbreviations with their sources */ public List getAllAbbreviationsWithSources() { @@ -403,7 +400,7 @@ public List getAllAbbreviationsWithSources() { /** * Gets all abbreviations from both custom and built-in sources - * + * * @return List of all abbreviations */ public List getAllAbbreviations() { @@ -414,7 +411,7 @@ public List getAllAbbreviations() { /** * Gets the source identifier for a given abbreviation - * + * * @param abbreviation The abbreviation to look up * @return The source key, or BUILTIN_LIST_ID if not found */ diff --git a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java index f0ff91da3c7..09b66e3f24f 100644 --- a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java +++ b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java @@ -331,6 +331,7 @@ public class JabRefCliPreferences implements CliPreferences { // Journal private static final String EXTERNAL_JOURNAL_LISTS = "externalJournalLists"; + private static final String ENABLED_EXTERNAL_JOURNAL_LISTS = "enabledExternalJournalLists"; private static final String USE_AMS_FJOURNAL = "useAMSFJournal"; // Protected terms @@ -432,8 +433,6 @@ public class JabRefCliPreferences implements CliPreferences { private AiPreferences aiPreferences; private LastFilesOpenedPreferences lastFilesOpenedPreferences; - private static final String ENABLED_EXTERNAL_JOURNAL_LISTS = "enabledExternalJournalLists"; - /** * @implNote The constructor is made protected to enforce this as a singleton class: */ diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 3da2d0d834e..c2dcd2fba9a 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -119,4 +119,4 @@ void checksIfAnyJournalSourcesAreEnabled() { verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); } -} \ No newline at end of file +} diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index 5476245fdc2..dd7c8b98213 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -133,4 +133,4 @@ void unabbreviateWithBothSourcesDisabled() { assertEquals(CUSTOM_1.getAbbreviation(), customEntry.getField(StandardField.JOURNAL).orElse(""), "Journal abbreviation should remain unchanged"); } -} \ No newline at end of file +} diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 18279e3fac1..9ea7347eb40 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -10,9 +10,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import javafx.beans.property.SimpleBooleanProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; -import javafx.beans.property.SimpleBooleanProperty; import org.jabref.gui.DialogService; import org.jabref.logic.journals.Abbreviation; diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index 8c28b5b29ea..3c480fc7db8 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -6,15 +6,17 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; class AbbreviationsTest { - private JournalAbbreviationRepository repository; - - private static final Abbreviation TEST_JOURNAL = new Abbreviation("Test Journal", "Test J."); private static final Abbreviation DOTTED_JOURNAL = new Abbreviation("Dotted Journal", "Dotted J."); + private static final Abbreviation TEST_JOURNAL = new Abbreviation("Test Journal", "Test J."); + + private JournalAbbreviationRepository repository; @BeforeEach void setUp() { @@ -48,4 +50,39 @@ void getNextAbbreviationWrapsBackToFullName() { Optional abbreviation3 = repository.getNextAbbreviation("Test J"); assertEquals("Test Journal", abbreviation3.orElse("WRONG")); } + + @Test + void constructorValidIsoAbbreviation() { + assertDoesNotThrow(() -> new Abbreviation("Test Entry", "Test. Ent.")); + } + + @Test + void constructorInvalidMedlineAbbreviation() { + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> new Abbreviation("Test Entry", null, "TE")); + assertEquals("ISO/MEDLINE abbreviation is null or empty!", exception.getMessage()); + } + + @Test + void getShortestUniqueAbbreviationDifferentFromIsoAbbreviation() { + Abbreviation abbreviation = new Abbreviation("Test Entry", "Test. Ent.", "TE"); + + assertEquals("TE", abbreviation.getShortestUniqueAbbreviation()); + assertNotEquals("Test. Ent.", abbreviation.getShortestUniqueAbbreviation()); + } + + @Test + void getNext() { + Abbreviation abbreviation = new Abbreviation("Test Entry", "Test. Ent."); + + assertEquals("Test. Ent.", abbreviation.getNext("Test Entry")); + assertEquals("Test Ent", abbreviation.getNext("Test. Ent.")); + assertEquals("Test Entry", abbreviation.getNext("Test Ent")); + } + + @Test + void testToString() { + Abbreviation abbreviation = new Abbreviation("Test Entry", "Test. Ent."); + + assertEquals("Abbreviation{name=Test Entry, iso=Test. Ent., medline=Test Ent, shortest=null}", abbreviation.toString()); + } } diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 7500413f812..af4feb357b9 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -1,8 +1,6 @@ package org.jabref.logic.journals; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -10,7 +8,6 @@ import javax.swing.undo.CompoundEdit; import javafx.collections.FXCollections; -import javafx.collections.ObservableList; import org.jabref.architecture.AllowedToUseSwing; import org.jabref.gui.journals.AbbreviationType; @@ -38,17 +35,17 @@ @AllowedToUseSwing("UndoableUnabbreviator and UndoableAbbreviator requires Swing Compound Edit in order test the abbreviation and unabbreviation of journal titles") class JournalAbbreviationRepositoryTest { + private static final Abbreviation ACS_MATERIALS = new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"); + private static final Abbreviation AMERICAN_JOURNAL = new Abbreviation("American Journal of Public Health", "Am. J. Public Health"); + private static final Abbreviation ANTIOXIDANTS = new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"); + private static final Abbreviation PHYSICAL_REVIEW = new Abbreviation("Physical Review B", "Phys. Rev. B"); + private JournalAbbreviationRepository repository; private JournalAbbreviationPreferences abbreviationPreferences; private final BibDatabase bibDatabase = new BibDatabase(); private UndoableUnabbreviator undoableUnabbreviator; - private static final Abbreviation AMERICAN_JOURNAL = new Abbreviation("American Journal of Public Health", "Am. J. Public Health"); - private static final Abbreviation ACS_MATERIALS = new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"); - private static final Abbreviation ANTIOXIDANTS = new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"); - private static final Abbreviation PHYSICAL_REVIEW = new Abbreviation("Physical Review B", "Phys. Rev. B"); - /** * Creates a test repository with pre-defined abbreviations and all sources enabled */ From 8fc0d822a5c803c675f0bde37f6861a928565171 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 20 Apr 2025 05:29:40 +0800 Subject: [PATCH 18/41] fix: fix some unit testing failure --- .../java/org/jabref/gui/journals/AbbreviateActionTest.java | 1 + .../org/jabref/gui/journals/UndoableUnabbreviatorTest.java | 1 + .../java/org/jabref/logic/journals/AbbreviationsTest.java | 7 ++++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index c2dcd2fba9a..0869cb8ac2c 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -120,3 +120,4 @@ void checksIfAnyJournalSourcesAreEnabled() { verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); } } + diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index dd7c8b98213..c2b20ac25fc 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -134,3 +134,4 @@ void unabbreviateWithBothSourcesDisabled() { "Journal abbreviation should remain unchanged"); } } + diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index 3c480fc7db8..5de6ef84c49 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -58,8 +58,8 @@ void constructorValidIsoAbbreviation() { @Test void constructorInvalidMedlineAbbreviation() { - IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> new Abbreviation("Test Entry", null, "TE")); - assertEquals("ISO/MEDLINE abbreviation is null or empty!", exception.getMessage()); + NullPointerException exception = assertThrows(NullPointerException.class, + () -> new Abbreviation("Test Entry", null, "TE")); } @Test @@ -83,6 +83,7 @@ void getNext() { void testToString() { Abbreviation abbreviation = new Abbreviation("Test Entry", "Test. Ent."); - assertEquals("Abbreviation{name=Test Entry, iso=Test. Ent., medline=Test Ent, shortest=null}", abbreviation.toString()); + assertEquals("Abbreviation{name=Test Entry, abbreviation=Test. Ent., dotlessAbbreviation=Test Ent, shortestUniqueAbbreviation=}", + abbreviation.toString()); } } From 10491f559331de53cf7a7c28ad3bde41b8df35dc Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Mon, 21 Apr 2025 02:57:05 +0800 Subject: [PATCH 19/41] fix: fix three failures in LocalizationConsistencyTest --- src/main/java/org/jabref/gui/journals/AbbreviateAction.java | 2 +- src/main/resources/l10n/JabRef_en.properties | 2 +- .../java/org/jabref/gui/journals/AbbreviateActionTest.java | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 521bc45bab9..ef8e0b62251 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -94,7 +94,7 @@ public void execute() { .executeWith(taskExecutor)); } else if (action == StandardActions.UNABBREVIATE) { if (!areAnyJournalSourcesEnabled()) { - dialogService.notify(Localization.lang("%cannot unabbreviate: all journal lists are disabled.")); + dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled.")); return; } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index be2847fd5a0..ae80d2709bb 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -1497,7 +1497,7 @@ None\ of\ the\ selected\ entries\ have\ citation\ keys.=None of the selected ent None\ of\ the\ selected\ entries\ have\ DOIs.=None of the selected entries have DOIs. Unabbreviate\ journal\ names=Unabbreviate journal names Unabbreviating...=Unabbreviating... -%cannot\ unabbreviate:\ all\ journal\ lists\ are\ disabled.=%cannot unabbreviate: all journal lists are disabled. +Cannot\ unabbreviate\:\ all\ journal\ lists\ are\ disabled.=Cannot unabbreviate: all journal lists are disabled. Usage=Usage diff --git a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java index 0869cb8ac2c..4c406ab3ab7 100644 --- a/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java +++ b/src/test/java/org/jabref/gui/journals/AbbreviateActionTest.java @@ -72,7 +72,7 @@ void unabbreviateWithAllSourcesDisabledShowsNotification() { action.execute(); - verify(dialogService).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); + verify(dialogService).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled."))); } @Test @@ -94,7 +94,7 @@ void unabbreviateWithOneSourceEnabledExecutesTask() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); + verify(dialogService, never()).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled."))); verify(dialogService).notify(eq(Localization.lang("Unabbreviating..."))); } @@ -117,7 +117,7 @@ void checksIfAnyJournalSourcesAreEnabled() { action.execute(); - verify(dialogService, never()).notify(eq(Localization.lang("%cannot unabbreviate: all journal lists are disabled."))); + verify(dialogService, never()).notify(eq(Localization.lang("Cannot unabbreviate: all journal lists are disabled."))); } } From 39c822d63d03a0268d251fa5d652826871598ec3 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Mon, 21 Apr 2025 04:25:45 +0800 Subject: [PATCH 20/41] feat: add a null check before attempting to establish the bidirectional binding --- .../journals/JournalAbbreviationsTabViewModel.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java index 0ced35ada50..184c364a184 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -80,9 +80,11 @@ public JournalAbbreviationsTabViewModel(JournalAbbreviationPreferences abbreviat } if (newValue != null) { isFileRemovable.set(!newValue.isBuiltInListProperty().get()); - abbreviations.bindBidirectional(newValue.abbreviationsProperty()); - if (!abbreviations.isEmpty()) { - currentAbbreviation.set(abbreviations.getLast()); + if (newValue.abbreviationsProperty() != null) { + abbreviations.bindBidirectional(newValue.abbreviationsProperty()); + if (!abbreviations.isEmpty()) { + currentAbbreviation.set(abbreviations.getLast()); + } } } else { isFileRemovable.set(false); From 421b08fa94929378ea0c167447f8b8a4e4372bed Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Mon, 21 Apr 2025 12:51:52 +0800 Subject: [PATCH 21/41] fix: address 7 comments in 4 files raised by trag-bot --- .../jabref/gui/journals/AbbreviateAction.java | 12 ++--- .../journals/AbbreviationsFileViewModel.java | 17 ++++++- .../journals/JournalAbbreviationsTab.java | 46 ++++++++++--------- .../JournalAbbreviationsTabViewModel.java | 24 ++++++---- 4 files changed, 60 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index ef8e0b62251..1b5bde6c6bb 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -83,6 +83,11 @@ public AbbreviateAction(StandardActions action, @Override public void execute() { + if (action == StandardActions.UNABBREVIATE && !areAnyJournalSourcesEnabled()) { + dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled.")); + return; + } + if ((action == StandardActions.ABBREVIATE_DEFAULT) || (action == StandardActions.ABBREVIATE_DOTLESS) || (action == StandardActions.ABBREVIATE_SHORTEST_UNIQUE) @@ -92,12 +97,7 @@ public void execute() { BackgroundTask.wrap(() -> abbreviate(stateManager.getActiveDatabase().get(), stateManager.getSelectedEntries())) .onSuccess(dialogService::notify) .executeWith(taskExecutor)); - } else if (action == StandardActions.UNABBREVIATE) { - if (!areAnyJournalSourcesEnabled()) { - dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled.")); - return; - } - + } else if (action == StandardActions.UNABBREVIATE) { dialogService.notify(Localization.lang("Unabbreviating...")); stateManager.getActiveDatabase().ifPresent(_ -> BackgroundTask.wrap(() -> unabbreviate(stateManager.getActiveDatabase().get(), stateManager.getSelectedEntries())) diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index 297ab45c9e9..2d03db1274f 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -73,6 +73,14 @@ public SimpleListProperty abbreviationsProperty() { return abbreviations; } + /** + * Reads journal abbreviations from the associated file and updates the abbreviations list. + * For built-in lists, this method does nothing and returns immediately. + * For file-based lists, it attempts to read abbreviations from the CSV file at the specified path. + * If the file doesn't exist, a debug message is logged but no exception is propagated. + * + * @throws IOException If there is an error reading the abbreviation file + */ public void readAbbreviations() throws IOException { if (isBuiltInList.get()) { return; @@ -85,10 +93,17 @@ public void readAbbreviations() throws IOException { .collect(Collectors.toCollection(ArrayList::new)); abbreviations.setAll(viewModels); } catch (NoSuchFileException e) { - LOGGER.debug("Journal abbreviation list {} does not exist", filePath); + LOGGER.debug("Journal abbreviation list {} does not exist", filePath, e); } } + /** + * Writes the abbreviations to the associated file or creates a new file if it doesn't exist. + * For built-in lists, this method does nothing and returns immediately. + * For file-based lists, it collects all abbreviations from the property and writes them to the file. + * + * @throws IOException If there is an error writing to the file + */ public void writeOrCreate() throws IOException { if (isBuiltInList.get()) { return; diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index 5a6ad337f98..c142fcd603a 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -281,30 +281,32 @@ public String getTabName() { @FXML private void toggleEnableList() { AbbreviationsFileViewModel selected = journalFilesBox.getValue(); - if (selected != null) { - boolean newEnabledState = !selected.isEnabled(); - selected.setEnabled(newEnabledState); - - refreshComboBoxDisplay(); - - JournalAbbreviationPreferences abbreviationPreferences = preferences.getJournalAbbreviationPreferences(); - - if (selected.isBuiltInListProperty().get()) { - abbreviationPreferences.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState); - } else if (selected.getAbsolutePath().isPresent()) { - String fileName = selected.getAbsolutePath().get().getFileName().toString(); - abbreviationPreferences.setSourceEnabled(fileName, newEnabledState); - } - - JournalAbbreviationRepository newRepository = - JournalAbbreviationLoader.loadRepository(abbreviationPreferences); - - Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository); - - abbreviationRepository = newRepository; + if (selected == null) { + return; + } + + boolean newEnabledState = !selected.isEnabled(); + selected.setEnabled(newEnabledState); + + refreshComboBoxDisplay(); + + JournalAbbreviationPreferences abbreviationPreferences = preferences.getJournalAbbreviationPreferences(); - viewModel.markAsDirty(); + if (selected.isBuiltInListProperty().get()) { + abbreviationPreferences.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState); + } else if (selected.getAbsolutePath().isPresent()) { + String fileName = selected.getAbsolutePath().get().getFileName().toString(); + abbreviationPreferences.setSourceEnabled(fileName, newEnabledState); } + + JournalAbbreviationRepository newRepository = + JournalAbbreviationLoader.loadRepository(abbreviationPreferences); + + Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository); + + abbreviationRepository = newRepository; + + viewModel.markAsDirty(); } private void setAnimations() { diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java index 184c364a184..1039c159489 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -78,15 +78,8 @@ public JournalAbbreviationsTabViewModel(JournalAbbreviationPreferences abbreviat abbreviations.unbindBidirectional(oldValue.abbreviationsProperty()); currentAbbreviation.set(null); } - if (newValue != null) { - isFileRemovable.set(!newValue.isBuiltInListProperty().get()); - if (newValue.abbreviationsProperty() != null) { - abbreviations.bindBidirectional(newValue.abbreviationsProperty()); - if (!abbreviations.isEmpty()) { - currentAbbreviation.set(abbreviations.getLast()); - } - } - } else { + + if (newValue == null) { isFileRemovable.set(false); if (journalFiles.isEmpty()) { currentAbbreviation.set(null); @@ -94,6 +87,17 @@ public JournalAbbreviationsTabViewModel(JournalAbbreviationPreferences abbreviat } else { currentFile.set(journalFiles.getFirst()); } + return; + } + + isFileRemovable.set(!newValue.isBuiltInListProperty().get()); + if (newValue.abbreviationsProperty() == null) { + return; + } + + abbreviations.bindBidirectional(newValue.abbreviationsProperty()); + if (!abbreviations.isEmpty()) { + currentAbbreviation.set(abbreviations.getLast()); } }); journalFiles.addListener((ListChangeListener) lcl -> { @@ -367,7 +371,7 @@ public void storeSettings() { Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository); }) - .onFailure(exception -> LOGGER.error("Failed to store journal preferences: {}", exception.getMessage(), exception)) + .onFailure(exception -> LOGGER.error("Failed to store journal preferences", exception)) .executeWith(taskExecutor); } From 305beeec618c94848d8af26b4f98cc3e79726699 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Mon, 21 Apr 2025 13:07:31 +0800 Subject: [PATCH 22/41] fix: correct Javadoc tag '@throws' to be preceded with an empty line --- .../gui/preferences/journals/AbbreviationsFileViewModel.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index 2d03db1274f..b79eb7a5160 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -78,7 +78,7 @@ public SimpleListProperty abbreviationsProperty() { * For built-in lists, this method does nothing and returns immediately. * For file-based lists, it attempts to read abbreviations from the CSV file at the specified path. * If the file doesn't exist, a debug message is logged but no exception is propagated. - * + * @throws IOException If there is an error reading the abbreviation file */ public void readAbbreviations() throws IOException { @@ -101,7 +101,7 @@ public void readAbbreviations() throws IOException { * Writes the abbreviations to the associated file or creates a new file if it doesn't exist. * For built-in lists, this method does nothing and returns immediately. * For file-based lists, it collects all abbreviations from the property and writes them to the file. - * + * @throws IOException If there is an error writing to the file */ public void writeOrCreate() throws IOException { From df1553c0042f8413d88edd4ef8d03f248ab91444 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Mon, 21 Apr 2025 13:36:23 +0800 Subject: [PATCH 23/41] fix: correct JavaDoc formatting --- .../gui/preferences/journals/AbbreviationsFileViewModel.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index b79eb7a5160..cbda6220921 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -78,7 +78,7 @@ public SimpleListProperty abbreviationsProperty() { * For built-in lists, this method does nothing and returns immediately. * For file-based lists, it attempts to read abbreviations from the CSV file at the specified path. * If the file doesn't exist, a debug message is logged but no exception is propagated. - + * * @throws IOException If there is an error reading the abbreviation file */ public void readAbbreviations() throws IOException { @@ -101,7 +101,7 @@ public void readAbbreviations() throws IOException { * Writes the abbreviations to the associated file or creates a new file if it doesn't exist. * For built-in lists, this method does nothing and returns immediately. * For file-based lists, it collects all abbreviations from the property and writes them to the file. - + * * @throws IOException If there is an error writing to the file */ public void writeOrCreate() throws IOException { From a68cc84f3e325096359e4a5d0a696e31e95e0bbf Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Tue, 22 Apr 2025 01:59:19 +0800 Subject: [PATCH 24/41] fix: make some methods to follow the fail-fast principle --- .../JournalAbbreviationsTabViewModel.java | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java index 1039c159489..e8606a43ddf 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -218,11 +218,13 @@ public void openFile() { * except if there are no more files than the {@code activeFile} property will be set to {@code null}. */ public void removeCurrentFile() { - if (isFileRemovable.get()) { - journalFiles.remove(currentFile.get()); - if (journalFiles.isEmpty()) { - currentFile.set(null); - } + if (!isFileRemovable.get()) { + return; + } + + journalFiles.remove(currentFile.get()); + if (journalFiles.isEmpty()) { + currentFile.set(null); } } @@ -253,18 +255,20 @@ public void addAbbreviation() { * Method to change the currentAbbreviation property to a new abbreviation. */ void editAbbreviation(Abbreviation abbreviationObject) { - if (isEditableAndRemovable.get()) { - AbbreviationViewModel abbViewModel = new AbbreviationViewModel(abbreviationObject); - if (abbreviations.contains(abbViewModel)) { - if (abbViewModel.equals(currentAbbreviation.get())) { - setCurrentAbbreviationNameAndAbbreviationIfValid(abbreviationObject); - } else { - dialogService.showErrorDialogAndWait(Localization.lang("Duplicated Journal Abbreviation"), - Localization.lang("Abbreviation '%0' for journal '%1' already defined.", abbreviationObject.getAbbreviation(), abbreviationObject.getName())); - } - } else { + if (!isEditableAndRemovable.get()) { + return; + } + + AbbreviationViewModel abbViewModel = new AbbreviationViewModel(abbreviationObject); + if (abbreviations.contains(abbViewModel)) { + if (abbViewModel.equals(currentAbbreviation.get())) { setCurrentAbbreviationNameAndAbbreviationIfValid(abbreviationObject); + } else { + dialogService.showErrorDialogAndWait(Localization.lang("Duplicated Journal Abbreviation"), + Localization.lang("Abbreviation '%0' for journal '%1' already defined.", abbreviationObject.getAbbreviation(), abbreviationObject.getName())); } + } else { + setCurrentAbbreviationNameAndAbbreviationIfValid(abbreviationObject); } } @@ -294,18 +298,20 @@ private void setCurrentAbbreviationNameAndAbbreviationIfValid(Abbreviation abbre * {@code null} if there are no abbreviations left. */ public void deleteAbbreviation() { - if ((currentAbbreviation.get() != null) && !currentAbbreviation.get().isPseudoAbbreviation()) { - int index = abbreviations.indexOf(currentAbbreviation.get()); - if (index > 1) { - currentAbbreviation.set(abbreviations.get(index - 1)); - } else if ((index + 1) < abbreviationsCount.get()) { - currentAbbreviation.set(abbreviations.get(index + 1)); - } else { - currentAbbreviation.set(null); - } - abbreviations.remove(index); - shouldWriteLists = true; + if ((currentAbbreviation.get() == null) || currentAbbreviation.get().isPseudoAbbreviation()) { + return; + } + + int index = abbreviations.indexOf(currentAbbreviation.get()); + if (index > 1) { + currentAbbreviation.set(abbreviations.get(index - 1)); + } else if ((index + 1) < abbreviationsCount.get()) { + currentAbbreviation.set(abbreviations.get(index + 1)); + } else { + currentAbbreviation.set(null); } + abbreviations.remove(index); + shouldWriteLists = true; } public void removeAbbreviation(AbbreviationViewModel abbreviation) { From b1237af51e4b503e5f1e62f59f1735bd13f0b176 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Wed, 23 Apr 2025 22:16:11 +0800 Subject: [PATCH 25/41] fix: address the comments by trag-bot --- .../journals/JournalAbbreviationRepository.java | 7 ++----- .../gui/journals/UndoableUnabbreviatorTest.java | 16 ++++++++-------- .../JournalAbbreviationsViewModelTabTest.java | 6 +++--- .../JournalAbbreviationRepositoryTest.java | 17 +++++++++++------ 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 81791f5c640..5ac58b563b2 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -14,7 +14,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; - +import org.jspecify.annotations.NonNull; import org.jabref.logic.journals.ltwa.LtwaRepository; import org.jabref.logic.util.strings.StringSimilarity; @@ -274,10 +274,7 @@ public void addCustomAbbreviation(Abbreviation abbreviation) { * @param sourcePath The path or identifier of the source * @param enabled Whether the source is enabled */ - public void addCustomAbbreviation(Abbreviation abbreviation, String sourcePath, boolean enabled) { - Objects.requireNonNull(abbreviation); - Objects.requireNonNull(sourcePath); - + public void addCustomAbbreviation(@NonNull Abbreviation abbreviation, @NonNull String sourcePath, boolean enabled) { customAbbreviations.add(abbreviation); abbreviationSources.put(abbreviation, sourcePath); diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index c2b20ac25fc..93d4ba626d6 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -57,8 +57,8 @@ private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { @Test void unabbreviateWithBothSourcesEnabled() { - assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -77,8 +77,8 @@ void unabbreviateWithBothSourcesEnabled() { void unabbreviateWithOnlyBuiltInSourceEnabled() { repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -97,8 +97,8 @@ void unabbreviateWithOnlyBuiltInSourceEnabled() { void unabbreviateWithOnlyCustomSourceEnabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -118,8 +118,8 @@ void unabbreviateWithBothSourcesDisabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); + assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 9ea7347eb40..2947d40f736 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -550,14 +550,14 @@ void toggleEnabledListChangesEnabledState() { AbbreviationsFileViewModel fileViewModel = viewModel.currentFileProperty().get(); - assertTrue(fileViewModel.isEnabled()); + assertEquals(true, fileViewModel.isEnabled()); fileViewModel.setEnabled(false); - assertFalse(fileViewModel.isEnabled()); + assertEquals(false, fileViewModel.isEnabled()); fileViewModel.setEnabled(true); - assertTrue(fileViewModel.isEnabled()); + assertEquals(true, fileViewModel.isEnabled()); viewModel.markAsDirty(); } diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index af4feb357b9..1e93eb5011e 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -197,16 +197,18 @@ void duplicateKeysWithShortestUniqueAbbreviation() { void getFromFullName() { repository = createTestRepository(); - assertEquals(AMERICAN_JOURNAL, repository.get("American Journal of Public Health") - .orElseThrow(() -> new AssertionError("Repository should contain American Journal of Public Health"))); + Optional result = repository.get("American Journal of Public Health"); + assertTrue(result.isPresent()); + assertEquals(AMERICAN_JOURNAL, result.get()); } @Test void getFromAbbreviatedName() { repository = createTestRepository(); - assertEquals(AMERICAN_JOURNAL, repository.get("Am. J. Public Health") - .orElseThrow(() -> new AssertionError("Repository should find the abbreviation"))); + Optional result = repository.get("Am. J. Public Health"); + assertTrue(result.isPresent()); + assertEquals(AMERICAN_JOURNAL, result.get()); } @Test @@ -214,7 +216,9 @@ void abbreviationsWithEscapedAmpersand() { repository = createTestRepository(); // Standard ampersand - assertEquals(ACS_MATERIALS, repository.get("ACS Applied Materials & Interfaces").orElseThrow()); + Optional result = repository.get("ACS Applied Materials & Interfaces"); + assertTrue(result.isPresent()); + assertEquals(ACS_MATERIALS, result.get()); // Escaped ampersand assertEquals(ACS_MATERIALS, repository.get("ACS Applied Materials \\& Interfaces").orElseThrow()); @@ -485,7 +489,8 @@ void disablingSourcePreventsAccessToAbbreviations() { assertFalse(repository.isSourceEnabled(sourceKey)); - assertEquals("WRONG", repository.getDefaultAbbreviation("Unique Journal").orElse("WRONG")); + Optional abbreviation = repository.getDefaultAbbreviation("Unique Journal"); + assertTrue(abbreviation.isEmpty()); } @Test From 2f9fc82e84d658c332b2b8e34f78742d2b0603b9 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Thu, 24 Apr 2025 02:03:42 +0800 Subject: [PATCH 26/41] fix: address Checkstyle issues --- .../journals/JournalAbbreviationRepository.java | 8 ++++---- .../gui/journals/UndoableUnabbreviatorTest.java | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 5ac58b563b2..10ebaa51392 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -13,10 +13,10 @@ import java.util.TreeSet; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collectors; -import org.jspecify.annotations.NonNull; + import org.jabref.logic.journals.ltwa.LtwaRepository; import org.jabref.logic.util.strings.StringSimilarity; +import org.jspecify.annotations.NonNull; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; @@ -211,7 +211,7 @@ public Optional getForUnabbreviation(String input) { private Optional findAbbreviationFuzzyMatched(String input) { Collection enabledCustomAbbreviations = customAbbreviations.stream() .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) - .collect(Collectors.toList()); + .toList(); Optional customMatch = findBestFuzzyMatched(enabledCustomAbbreviations, input); if (customMatch.isPresent()) { @@ -403,7 +403,7 @@ public List getAllAbbreviationsWithSources() { public List getAllAbbreviations() { return getAllAbbreviationsWithSources().stream() .map(AbbreviationWithSource::getAbbreviation) - .collect(Collectors.toList()); + .toList(); } /** diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index 93d4ba626d6..c2b20ac25fc 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -57,8 +57,8 @@ private BibEntry createEntryWithAbbreviatedJournal(String abbreviatedJournal) { @Test void unabbreviateWithBothSourcesEnabled() { - assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -77,8 +77,8 @@ void unabbreviateWithBothSourcesEnabled() { void unabbreviateWithOnlyBuiltInSourceEnabled() { repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertEquals(true, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertTrue(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -97,8 +97,8 @@ void unabbreviateWithOnlyBuiltInSourceEnabled() { void unabbreviateWithOnlyCustomSourceEnabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); - assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(true, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertTrue(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); @@ -118,8 +118,8 @@ void unabbreviateWithBothSourcesDisabled() { repository.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, false); repository.setSourceEnabled(CUSTOM_SOURCE, false); - assertEquals(false, repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); - assertEquals(false, repository.isSourceEnabled(CUSTOM_SOURCE)); + assertFalse(repository.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID)); + assertFalse(repository.isSourceEnabled(CUSTOM_SOURCE)); BibEntry builtInEntry = createEntryWithAbbreviatedJournal(BUILT_IN_1.getAbbreviation()); boolean builtInResult = unabbreviator.unabbreviate(database, builtInEntry, StandardField.JOURNAL, compoundEdit); From 19cf34b5c7493c59cde54f5be2ae85f830ec185f Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Thu, 24 Apr 2025 02:22:36 +0800 Subject: [PATCH 27/41] fix: address additional Checkstyle issues --- .../logic/journals/JournalAbbreviationRepository.java | 7 ++++--- .../journals/JournalAbbreviationsViewModelTabTest.java | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 10ebaa51392..3f67e0fba51 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -14,12 +14,13 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.h2.mvstore.MVMap; +import org.h2.mvstore.MVStore; + import org.jabref.logic.journals.ltwa.LtwaRepository; import org.jabref.logic.util.strings.StringSimilarity; -import org.jspecify.annotations.NonNull; -import org.h2.mvstore.MVMap; -import org.h2.mvstore.MVStore; +import org.jspecify.annotations.NonNull; /** * A repository for all journal abbreviations, including add and find methods. diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 2947d40f736..9ea7347eb40 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -550,14 +550,14 @@ void toggleEnabledListChangesEnabledState() { AbbreviationsFileViewModel fileViewModel = viewModel.currentFileProperty().get(); - assertEquals(true, fileViewModel.isEnabled()); + assertTrue(fileViewModel.isEnabled()); fileViewModel.setEnabled(false); - assertEquals(false, fileViewModel.isEnabled()); + assertFalse(fileViewModel.isEnabled()); fileViewModel.setEnabled(true); - assertEquals(true, fileViewModel.isEnabled()); + assertTrue(fileViewModel.isEnabled()); viewModel.markAsDirty(); } From 9dafa2ae7e34870ca2aeb2641621289a0597afe6 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Thu, 24 Apr 2025 03:10:13 +0800 Subject: [PATCH 28/41] fix: address the final Checkstyle issue --- .../jabref/logic/journals/JournalAbbreviationRepository.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 3f67e0fba51..a4d32a6777b 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -14,12 +14,11 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.h2.mvstore.MVMap; -import org.h2.mvstore.MVStore; - import org.jabref.logic.journals.ltwa.LtwaRepository; import org.jabref.logic.util.strings.StringSimilarity; +import org.h2.mvstore.MVMap; +import org.h2.mvstore.MVStore; import org.jspecify.annotations.NonNull; /** From 9b4a43f886380868ddc6af20f6e1ee94a9b1874d Mon Sep 17 00:00:00 2001 From: Zikun Zhu <75139323+zikunz@users.noreply.github.com> Date: Thu, 24 Apr 2025 04:26:32 +0800 Subject: [PATCH 29/41] fix: apply suggestions from code review Co-authored-by: Subhramit Basu --- .../gui/preferences/journals/JournalAbbreviationsTab.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index c142fcd603a..54df5aa19de 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -163,12 +163,12 @@ private void setBindings() { journalFilesBox.setCellFactory(listView -> new JournalFileListCell()); journalFilesBox.setButtonCell(new JournalFileListCell()); - viewModel.journalFilesProperty().addListener((observable, oldValue, newValue) -> { + viewModel.journalFilesProperty().addListener((_, _, newValue) -> { if (newValue == null) { return; } for (AbbreviationsFileViewModel fileViewModel : newValue) { - fileViewModel.enabledProperty().addListener((obs, oldVal, newVal) -> { + fileViewModel.enabledProperty().addListener((_, _, _) -> { refreshComboBoxDisplay(); }); } From d555a09866dc939be20c7cbf4d2c933c4d32b937 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Thu, 24 Apr 2025 04:34:56 +0800 Subject: [PATCH 30/41] doc: add JavaDoc suggested by trag-bot --- .../journals/JournalAbbreviationsTab.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index 54df5aa19de..fdb57e6a8bb 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -119,6 +119,12 @@ private void setUpTable() { .install(actionsColumn); } + /** + * Sets up the toggle button that allows enabling/disabling journal abbreviation lists. + * This method creates a button with appropriate styling and tooltip, then adds it + * to the UI next to the journal files dropdown. When clicked, the button toggles + * the enabled state of the currently selected abbreviation list. + */ private void setUpToggleButton() { Button toggleButton = new Button(Localization.lang("Toggle")); toggleButton.setOnAction(e -> toggleEnableList()); @@ -278,6 +284,19 @@ public String getTabName() { return Localization.lang("Journal abbreviations"); } + /** + * Toggles the enabled state of the currently selected journal abbreviation list. + * This method performs several important operations: + *
    + *
  • Toggles the enabled state of the selected list in the UI
    • + *
  • Refreshes the ComboBox display to show the updated state
    • + *
  • Updates the JournalAbbreviationPreferences to persist this change
    • + *
  • Reloads the entire JournalAbbreviationRepository with the new settings
    • + *
  • Updates the dependency injection container with the new repository
    • + *
  • Marks the view model as dirty to ensure changes are saved
    • + *
+ * This is called when the user clicks the toggle button next to the journal files dropdown. + */ @FXML private void toggleEnableList() { AbbreviationsFileViewModel selected = journalFilesBox.getValue(); From c85ec97cd44a4e49da51a279e39683e3fd9a4bf2 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Thu, 24 Apr 2025 05:00:39 +0800 Subject: [PATCH 31/41] fix: address comments by trag-bot --- .../gui/preferences/journals/JournalAbbreviationsTab.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index fdb57e6a8bb..f00693892dc 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -73,6 +73,9 @@ public class JournalAbbreviationsTab extends AbstractPreferenceTabView { if (newVal != null) { - setText((newVal ? "✓ " : "○ ") + item.toString()); + setText((newVal ? ENABLED_SYMBOL : DISABLED_SYMBOL) + item.toString()); } }); } From 5160d7858205bd25d0f57d3ed683e353a0096e41 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Thu, 24 Apr 2025 05:10:20 +0800 Subject: [PATCH 32/41] fix: correct Checkstyle issues --- .../gui/preferences/journals/JournalAbbreviationsTab.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index f00693892dc..9e7cecd05b3 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -50,6 +50,9 @@ */ public class JournalAbbreviationsTab extends AbstractPreferenceTabView implements PreferencesTab { + private static final String ENABLED_SYMBOL = "✓ "; + private static final String DISABLED_SYMBOL = "○ "; + @FXML private Label loadingLabel; @FXML private ProgressIndicator progressIndicator; @@ -73,9 +76,6 @@ public class JournalAbbreviationsTab extends AbstractPreferenceTabView Date: Sun, 27 Apr 2025 01:17:05 +0800 Subject: [PATCH 33/41] fix: restore comment inadvertently removed earlier on --- .../java/org/jabref/gui/journals/UndoableUnabbreviator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java index e559484d9dc..447596e0e65 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java @@ -45,7 +45,7 @@ public boolean unabbreviate(BibDatabase database, BibEntry entry, Field field, C } if (!journalAbbreviationRepository.isAbbreviatedName(text)) { - return false; + return false; // Cannot unabbreviate unabbreviated name. } var abbreviationOpt = journalAbbreviationRepository.get(text); From 26fc7745435273f892756fbbdf3618f567a0f0e9 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 01:48:58 +0800 Subject: [PATCH 34/41] fix: extract the true to be constants to improve readability --- .../jabref/logic/journals/JournalAbbreviationLoader.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java index 8cead46f021..2f69de65a43 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java @@ -26,6 +26,8 @@ public class JournalAbbreviationLoader { private static final Logger LOGGER = LoggerFactory.getLogger(JournalAbbreviationLoader.class); + private static final boolean USE_FJOURNAL_FIELD = true; + private static final boolean BUILTIN_LIST_ENABLED_BY_DEFAULT = true; public static Collection readAbbreviationsFromCsvFile(Path file) throws IOException { LOGGER.debug("Reading journal list from file {}", file); @@ -132,9 +134,9 @@ private static LtwaRepository loadLtwaRepository() throws IOException { } public static JournalAbbreviationRepository loadBuiltInRepository() { - JournalAbbreviationPreferences prefs = new JournalAbbreviationPreferences(Collections.emptyList(), true); + JournalAbbreviationPreferences prefs = new JournalAbbreviationPreferences(Collections.emptyList(), USE_FJOURNAL_FIELD); - prefs.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, true); + prefs.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, BUILTIN_LIST_ENABLED_BY_DEFAULT); return loadRepository(prefs); } } From 153c8ad15f1cf5e4982664f7ba2af7464259e2e2 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 01:57:06 +0800 Subject: [PATCH 35/41] fix: put the default in JabRefCliPreferences using defaults.put(ENABLED_EXTERNAL_JOURNAL_LISTS, false) --- .../jabref/logic/journals/JournalAbbreviationPreferences.java | 4 +++- .../org/jabref/logic/preferences/JabRefCliPreferences.java | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java index d3b15012ba1..763d432d83f 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java @@ -14,10 +14,12 @@ */ public class JournalAbbreviationPreferences { + public static final String ENABLED_EXTERNAL_JOURNAL_LISTS = "enabledExternalJournalLists"; + private final ObservableList externalJournalLists; private final BooleanProperty useFJournalField; private final Map enabledExternalLists = new HashMap<>(); - private final BooleanProperty enabledListsChanged = new SimpleBooleanProperty(false); + private final BooleanProperty enabledListsChanged = new SimpleBooleanProperty(); /** * Constructs a new JournalAbbreviationPreferences with the given external journal lists and FJournal field preference diff --git a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java index f4f9419abb2..44895f611ca 100644 --- a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java +++ b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java @@ -689,6 +689,9 @@ protected JabRefCliPreferences() { // endregion // endregion + + // region:Journal abbreviations + defaults.put(JournalAbbreviationPreferences.ENABLED_EXTERNAL_JOURNAL_LISTS, Boolean.FALSE); } public void setLanguageDependentDefaultValues() { From 2a6604e63492ccbb0faec5ee85e3e2d85ef039fc Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 03:01:39 +0800 Subject: [PATCH 36/41] fix: modernize collection usage, change return and param types, update JavaDoc --- .../jabref/gui/journals/AbbreviateAction.java | 3 +-- .../journals/AbbreviationsFileViewModel.java | 6 ++--- .../JournalAbbreviationsTabViewModel.java | 5 ++-- .../logic/journals/AbbreviationParser.java | 4 ++-- .../journals/JournalAbbreviationLoader.java | 17 ++++++++++---- .../JournalAbbreviationRepository.java | 23 ++++++++++++------- .../preferences/JabRefCliPreferences.java | 10 ++++---- .../JournalAbbreviationsViewModelTabTest.java | 3 +-- 8 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index 1b5bde6c6bb..a9526e7a41b 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -2,7 +2,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; @@ -174,7 +173,7 @@ private String unabbreviate(BibDatabaseContext databaseContext, List e String text = entry.getFieldLatexFree(journalField).orElse(""); List possibleSources = - textToSourceMap.getOrDefault(text, Collections.emptyList()); + textToSourceMap.getOrDefault(text, List.of()); if (!possibleSources.isEmpty()) { boolean allSourcesDisabled = true; diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index cbda6220921..e63df22da10 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -5,10 +5,10 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import javafx.beans.property.BooleanProperty; @@ -86,7 +86,7 @@ public void readAbbreviations() throws IOException { return; } try { - Collection abbreviationsFromFile = JournalAbbreviationLoader.readAbbreviationsFromCsvFile(filePath); + Set abbreviationsFromFile = JournalAbbreviationLoader.readAbbreviationsFromCsvFile(filePath); List viewModels = abbreviationsFromFile.stream() .map(AbbreviationViewModel::new) @@ -111,7 +111,7 @@ public void writeOrCreate() throws IOException { List abbreviationList = abbreviationsProperty().stream() .map(AbbreviationViewModel::getAbbreviationObject) - .collect(Collectors.toList()); + .toList(); AbbreviationWriter.writeOrCreate(filePath, abbreviationList); } diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java index e8606a43ddf..9b5077b1501 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -4,7 +4,6 @@ import java.nio.file.Path; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.property.SimpleIntegerProperty; @@ -150,7 +149,7 @@ public void addBuiltInList() { isLoading.setValue(false); List builtInViewModels = result.stream() .map(AbbreviationViewModel::new) - .collect(Collectors.toList()); + .toList(); AbbreviationsFileViewModel builtInListModel = new AbbreviationsFileViewModel(builtInViewModels, Localization.lang("JabRef built in list")); boolean isEnabled = abbreviationsPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID); @@ -352,7 +351,7 @@ public void storeSettings() { .filter(path -> !path.isBuiltInListProperty().get()) .filter(path -> path.getAbsolutePath().isPresent()) .map(path -> path.getAbsolutePath().get().toAbsolutePath().toString()) - .collect(Collectors.toList()); + .toList(); abbreviationsPreferences.setExternalJournalLists(journalStringList); abbreviationsPreferences.setUseFJournalField(useFJournal.get()); diff --git a/src/main/java/org/jabref/logic/journals/AbbreviationParser.java b/src/main/java/org/jabref/logic/journals/AbbreviationParser.java index 291016c1259..131b085a9bc 100644 --- a/src/main/java/org/jabref/logic/journals/AbbreviationParser.java +++ b/src/main/java/org/jabref/logic/journals/AbbreviationParser.java @@ -6,8 +6,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.Collection; import java.util.LinkedHashSet; +import java.util.Set; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -63,7 +63,7 @@ private char detectDelimiter(Path file) throws IOException { } } - public Collection getAbbreviations() { + public Set getAbbreviations() { return abbreviations; } } diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java index 2f69de65a43..7f423b9ef76 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java @@ -5,9 +5,9 @@ import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; -import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Set; import org.jabref.logic.journals.ltwa.LtwaRepository; @@ -29,7 +29,14 @@ public class JournalAbbreviationLoader { private static final boolean USE_FJOURNAL_FIELD = true; private static final boolean BUILTIN_LIST_ENABLED_BY_DEFAULT = true; - public static Collection readAbbreviationsFromCsvFile(Path file) throws IOException { + /** + * Reads journal abbreviations from a CSV file. + * + * @param file Path to the CSV file containing journal abbreviations + * @return A set of abbreviations read from the file + * @throws IOException If an I/O error occurs while reading the file + */ + public static Set readAbbreviationsFromCsvFile(Path file) throws IOException { LOGGER.debug("Reading journal list from file {}", file); AbbreviationParser parser = new AbbreviationParser(); parser.readJournalListFromFile(file); @@ -102,11 +109,11 @@ public static JournalAbbreviationRepository loadRepository(JournalAbbreviationPr simpleFilename, enabled); } - Collection abbreviations = readAbbreviationsFromCsvFile(filePath); + Set abbreviations = readAbbreviationsFromCsvFile(filePath); repository.addCustomAbbreviations(abbreviations, simpleFilename, enabled); - repository.addCustomAbbreviations(Collections.emptyList(), prefixedKey, enabled); + repository.addCustomAbbreviations(Set.of(), prefixedKey, enabled); } catch (IOException | InvalidPathException e) { // invalid path might come from unix/windows mixup of prefs LOGGER.error("Cannot read external journal list file {}", filename, e); @@ -134,7 +141,7 @@ private static LtwaRepository loadLtwaRepository() throws IOException { } public static JournalAbbreviationRepository loadBuiltInRepository() { - JournalAbbreviationPreferences prefs = new JournalAbbreviationPreferences(Collections.emptyList(), USE_FJOURNAL_FIELD); + JournalAbbreviationPreferences prefs = new JournalAbbreviationPreferences(List.of(), USE_FJOURNAL_FIELD); prefs.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, BUILTIN_LIST_ENABLED_BY_DEFAULT); return loadRepository(prefs); diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index a4d32a6777b..77f1cb748c9 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -209,7 +209,7 @@ public Optional getForUnabbreviation(String input) { } private Optional findAbbreviationFuzzyMatched(String input) { - Collection enabledCustomAbbreviations = customAbbreviations.stream() + List enabledCustomAbbreviations = customAbbreviations.stream() .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) .toList(); @@ -225,6 +225,13 @@ private Optional findAbbreviationFuzzyMatched(String input) { return findBestFuzzyMatched(fullToAbbreviationObject.values(), input); } + /** + * Finds the best matching abbreviation by fuzzy matching from a collection of abbreviations + * + * @param abbreviations Collection of abbreviations to search + * @param input The input string to match against + * @return Optional containing the best matching abbreviation, or empty if no good match is found + */ private Optional findBestFuzzyMatched(Collection abbreviations, String input) { // threshold for edit distance similarity comparison final double SIMILARITY_THRESHOLD = 1.0; @@ -281,27 +288,27 @@ public void addCustomAbbreviation(@NonNull Abbreviation abbreviation, @NonNull S enabledSources.put(sourcePath, enabled); } - public Collection getCustomAbbreviations() { + public Set getCustomAbbreviations() { return customAbbreviations; } /** * Adds multiple custom abbreviations to the repository * - * @param abbreviationsToAdd The abbreviations to add + * @param abbreviationsToAdd The set of abbreviations to add */ - public void addCustomAbbreviations(Collection abbreviationsToAdd) { + public void addCustomAbbreviations(Set abbreviationsToAdd) { abbreviationsToAdd.forEach(this::addCustomAbbreviation); } /** * Adds abbreviations with a specific source key and enabled state * - * @param abbreviationsToAdd Collection of abbreviations to add + * @param abbreviationsToAdd Set of abbreviations to add * @param sourceKey The key identifying the source of these abbreviations * @param enabled Whether the source should be enabled initially */ - public void addCustomAbbreviations(Collection abbreviationsToAdd, String sourceKey, boolean enabled) { + public void addCustomAbbreviations(Set abbreviationsToAdd, String sourceKey, boolean enabled) { enabledSources.put(sourceKey, enabled); for (Abbreviation abbreviation : abbreviationsToAdd) { @@ -350,8 +357,8 @@ public Set getFullNames() { return fullToAbbreviationObject.keySet(); } - public Collection getAllLoaded() { - return fullToAbbreviationObject.values(); + public Set getAllLoaded() { + return Set.copyOf(fullToAbbreviationObject.values()); } /** diff --git a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java index 44895f611ca..b03c3e59abf 100644 --- a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java +++ b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java @@ -730,7 +730,7 @@ static String convertListToString(List value) { @VisibleForTesting static List convertStringToList(String toConvert) { if (StringUtil.isBlank(toConvert)) { - return Collections.emptyList(); + return List.of(); } return Splitter.on(STRINGLIST_DELIMITER).splitToList(toConvert); @@ -1565,10 +1565,10 @@ public FieldPreferences getFieldPreferences() { !getBoolean(DO_NOT_RESOLVE_STRINGS), // mind the ! getStringList(RESOLVE_STRINGS_FOR_FIELDS).stream() .map(FieldFactory::parseField) - .collect(Collectors.toList()), + .toList(), getStringList(NON_WRAPPABLE_FIELDS).stream() .map(FieldFactory::parseField) - .collect(Collectors.toList())); + .toList()); EasyBind.listen(fieldPreferences.resolveStringsProperty(), (obs, oldValue, newValue) -> putBoolean(DO_NOT_RESOLVE_STRINGS, !newValue)); fieldPreferences.getResolvableFields().addListener((InvalidationListener) change -> @@ -1810,7 +1810,7 @@ public CleanupPreferences getCleanupPreferences() { FieldFormatterCleanups.parse(StringUtil.unifyLineBreaks(get(CLEANUP_FIELD_FORMATTERS), "")))); cleanupPreferences.getObservableActiveJobs().addListener((SetChangeListener) c -> - putStringList(CLEANUP_JOBS, cleanupPreferences.getActiveJobs().stream().map(Enum::name).collect(Collectors.toList()))); + putStringList(CLEANUP_JOBS, cleanupPreferences.getActiveJobs().stream().map(Enum::name).toList())); EasyBind.listen(cleanupPreferences.fieldFormatterCleanupsProperty(), (fieldFormatters, oldValue, newValue) -> { putBoolean(CLEANUP_FIELD_FORMATTERS_ENABLED, newValue.isEnabled()); @@ -2017,7 +2017,7 @@ public XmpPreferences getXmpPreferences() { xmpPreferences.getXmpPrivacyFilter().addListener((SetChangeListener) c -> putStringList(XMP_PRIVACY_FILTERS, xmpPreferences.getXmpPrivacyFilter().stream() .map(Field::getName) - .collect(Collectors.toList()))); + .toList())); return xmpPreferences; } diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 9ea7347eb40..0baaf9a1556 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -7,7 +7,6 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import java.util.stream.Stream; import javafx.beans.property.SimpleBooleanProperty; @@ -340,7 +339,7 @@ void builtInListsIncludeAllBuiltInAbbreviations() { ObservableList actualAbbreviations = FXCollections .observableArrayList(viewModel.abbreviationsProperty().stream() .map(AbbreviationViewModel::getAbbreviationObject) - .collect(Collectors.toList())); + .toList()); assertEquals(expected, actualAbbreviations); } From a23d3c851965e949f30f1dbe5ef6a66559e47950 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 03:14:14 +0800 Subject: [PATCH 37/41] fix: address Checkstyle issues --- .../JournalAbbreviationRepository.java | 2 +- .../journals/UndoableUnabbreviatorTest.java | 6 ++--- .../JournalAbbreviationsViewModelTabTest.java | 1 + .../logic/journals/AbbreviationsTest.java | 4 +-- .../JournalAbbreviationRepositoryTest.java | 26 +++++++++---------- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 77f1cb748c9..e9ac00726cf 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -226,7 +226,7 @@ private Optional findAbbreviationFuzzyMatched(String input) { } /** - * Finds the best matching abbreviation by fuzzy matching from a collection of abbreviations + * Finds the best matching abbreviation by fuzzy matching from a collection of abbreviations. * * @param abbreviations Collection of abbreviations to search * @param input The input string to match against diff --git a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java index c2b20ac25fc..f3d010e3351 100644 --- a/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java +++ b/src/test/java/org/jabref/gui/journals/UndoableUnabbreviatorTest.java @@ -1,6 +1,6 @@ package org.jabref.gui.journals; -import java.util.List; +import java.util.Set; import javax.swing.undo.CompoundEdit; @@ -37,11 +37,11 @@ class UndoableUnabbreviatorTest { @BeforeEach void setUp() { repository = new JournalAbbreviationRepository(); - repository.addCustomAbbreviations(List.of(BUILT_IN_1, BUILT_IN_2), + repository.addCustomAbbreviations(Set.of(BUILT_IN_1, BUILT_IN_2), JournalAbbreviationRepository.BUILTIN_LIST_ID, true); - repository.addCustomAbbreviations(List.of(CUSTOM_1, CUSTOM_2), + repository.addCustomAbbreviations(Set.of(CUSTOM_1, CUSTOM_2), CUSTOM_SOURCE, true); diff --git a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java index 0baaf9a1556..bc9ab009495 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -7,6 +7,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import java.util.stream.Stream; import javafx.beans.property.SimpleBooleanProperty; diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java index 5de6ef84c49..6cac4e7cc4d 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationsTest.java @@ -1,7 +1,7 @@ package org.jabref.logic.journals; -import java.util.List; import java.util.Optional; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -22,7 +22,7 @@ class AbbreviationsTest { void setUp() { repository = new JournalAbbreviationRepository(); - repository.addCustomAbbreviations(List.of(TEST_JOURNAL, DOTTED_JOURNAL), + repository.addCustomAbbreviations(Set.of(TEST_JOURNAL, DOTTED_JOURNAL), JournalAbbreviationRepository.BUILTIN_LIST_ID, true); } diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 1e93eb5011e..448c8dabf9e 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -52,7 +52,7 @@ class JournalAbbreviationRepositoryTest { private JournalAbbreviationRepository createTestRepository() { JournalAbbreviationRepository testRepo = new JournalAbbreviationRepository(); - testRepo.addCustomAbbreviations(List.of( + testRepo.addCustomAbbreviations(Set.of( AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, @@ -409,7 +409,7 @@ void dotlessForPhysRevB() { @ParameterizedTest @MethodSource("provideAbbreviationTestCases") void fuzzyMatch(List abbreviationList, String input, String expectedAbbreviation, String expectedDotless, String expectedShortest, String ambiguousInput) { - repository.addCustomAbbreviations(abbreviationList); + repository.addCustomAbbreviations(Set.copyOf(abbreviationList)); assertEquals(expectedAbbreviation, repository.getDefaultAbbreviation(input).orElse("WRONG")); @@ -423,7 +423,7 @@ void fuzzyMatch(List abbreviationList, String input, String expect static Stream provideAbbreviationTestCases() { return Stream.of( Arguments.of( - List.of( + Set.of( new Abbreviation("Journal of Physics A", "J. Phys. A", "JPA"), new Abbreviation("Journal of Physics B", "J. Phys. B", "JPB"), new Abbreviation("Journal of Physics C", "J. Phys. C", "JPC") @@ -435,7 +435,7 @@ static Stream provideAbbreviationTestCases() { "Journal of Physics" ), Arguments.of( - List.of( + Set.of( new Abbreviation("中国物理学报", "物理学报", "ZWP"), new Abbreviation("中国物理学理", "物理学报报", "ZWP"), new Abbreviation("中国科学: 物理学", "中科物理", "ZKP") @@ -447,7 +447,7 @@ static Stream provideAbbreviationTestCases() { "中国物理学" ), Arguments.of( - List.of( + Set.of( new Abbreviation("Zeitschrift für Chem", "Z. Phys. Chem.", "ZPC"), new Abbreviation("Zeitschrift für Chys", "Z. Angew. Chem.", "ZAC") ), @@ -464,7 +464,7 @@ static Stream provideAbbreviationTestCases() { void addCustomAbbreviationsWithEnabledState() { String sourceKey = "test-source"; - repository.addCustomAbbreviations(List.of( + repository.addCustomAbbreviations(Set.of( new Abbreviation("Journal One", "J. One"), new Abbreviation("Journal Two", "J. Two") ), sourceKey, true); @@ -479,7 +479,7 @@ void addCustomAbbreviationsWithEnabledState() { void disablingSourcePreventsAccessToAbbreviations() { String sourceKey = "test-source"; - repository.addCustomAbbreviations(List.of( + repository.addCustomAbbreviations(Set.of( new Abbreviation("Unique Journal", "U. J.") ), sourceKey, true); @@ -497,7 +497,7 @@ void disablingSourcePreventsAccessToAbbreviations() { void reenablingSourceRestoresAccessToAbbreviations() { String sourceKey = "test-source"; - repository.addCustomAbbreviations(List.of( + repository.addCustomAbbreviations(Set.of( new Abbreviation("Disabled Journal", "D. J.") ), sourceKey, true); @@ -545,11 +545,11 @@ void multipleSourcesCanBeToggled() { String sourceKey1 = "source-1-special"; String sourceKey2 = "source-2-special"; - testRepo.addCustomAbbreviations(List.of( + testRepo.addCustomAbbreviations(Set.of( new Abbreviation("Unique Journal Source One XYZ", "UniqueJS1") ), sourceKey1, true); - testRepo.addCustomAbbreviations(List.of( + testRepo.addCustomAbbreviations(Set.of( new Abbreviation("Unique Journal Source Two ABC", "UniqueJS2") ), sourceKey2, true); @@ -624,7 +624,7 @@ void getAllAbbreviationsWithSourcesReturnsCorrectSources() { testRepo.getCustomAbbreviations().clear(); - testRepo.addCustomAbbreviations(List.of( + testRepo.addCustomAbbreviations(Set.of( AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, @@ -632,7 +632,7 @@ void getAllAbbreviationsWithSourcesReturnsCorrectSources() { ), JournalAbbreviationRepository.BUILTIN_LIST_ID, true); String customSource = "test-custom"; - testRepo.addCustomAbbreviations(List.of( + testRepo.addCustomAbbreviations(Set.of( new Abbreviation("Custom Journal", "Cust. J.") ), customSource, true); @@ -657,7 +657,7 @@ void getAllAbbreviationsWithSourcesReturnsCorrectSources() { assertTrue(customAbbr.isPresent(), "Should find custom abbreviation with source"); assertEquals("Custom Journal", customAbbr.get().getAbbreviation().getName()); - for (Abbreviation abbr : List.of(AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, PHYSICAL_REVIEW)) { + for (Abbreviation abbr : Set.of(AMERICAN_JOURNAL, ACS_MATERIALS, ANTIOXIDANTS, PHYSICAL_REVIEW)) { boolean found = allWithSources.stream() .anyMatch(aws -> JournalAbbreviationRepository.BUILTIN_LIST_ID.equals(aws.getSource()) && abbr.getName().equals(aws.getAbbreviation().getName())); From d9437e27682cf88b4049890eff390e6e2d528683 Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 03:29:25 +0800 Subject: [PATCH 38/41] fix: address unit testing issues --- .../logic/journals/JournalAbbreviationRepositoryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 448c8dabf9e..dc36f2e0dc9 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -408,8 +408,8 @@ void dotlessForPhysRevB() { @ParameterizedTest @MethodSource("provideAbbreviationTestCases") - void fuzzyMatch(List abbreviationList, String input, String expectedAbbreviation, String expectedDotless, String expectedShortest, String ambiguousInput) { - repository.addCustomAbbreviations(Set.copyOf(abbreviationList)); + void fuzzyMatch(Set abbreviationSet, String input, String expectedAbbreviation, String expectedDotless, String expectedShortest, String ambiguousInput) { + repository.addCustomAbbreviations(abbreviationSet); assertEquals(expectedAbbreviation, repository.getDefaultAbbreviation(input).orElse("WRONG")); From c15e554215f455b1fe0391278b8faf6b090576ae Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 05:02:14 +0800 Subject: [PATCH 39/41] feat: implement JournalAbbreviationRepositoryManager for efficient repository caching - Add singleton repository manager with proper thread safety to cache repo - Fix issue where abbreviation operations would ignore disabled journal lists - Fix issue where toggling journal lists would update preferences when dialog is canceled - Update both abbreviation and unabbreviation to respect disabled sources - Apply double-checked locking pattern to ensure thread safety while preserving performance --- .../jabref/gui/journals/AbbreviateAction.java | 12 +- .../gui/journals/UndoableAbbreviator.java | 2 +- .../gui/journals/UndoableUnabbreviator.java | 2 +- .../journals/JournalAbbreviationsTab.java | 19 --- .../JournalAbbreviationRepository.java | 36 +++++ .../JournalAbbreviationRepositoryManager.java | 143 ++++++++++++++++++ 6 files changed, 188 insertions(+), 26 deletions(-) create mode 100644 src/main/java/org/jabref/logic/journals/JournalAbbreviationRepositoryManager.java diff --git a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java index a9526e7a41b..f9488848e93 100644 --- a/src/main/java/org/jabref/gui/journals/AbbreviateAction.java +++ b/src/main/java/org/jabref/gui/journals/AbbreviateAction.java @@ -17,9 +17,9 @@ import org.jabref.gui.actions.StandardActions; import org.jabref.gui.undo.NamedCompound; import org.jabref.logic.journals.Abbreviation; -import org.jabref.logic.journals.JournalAbbreviationLoader; import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; +import org.jabref.logic.journals.JournalAbbreviationRepositoryManager; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.BackgroundTask; import org.jabref.logic.util.TaskExecutor; @@ -108,9 +108,10 @@ public void execute() { } private String abbreviate(BibDatabaseContext databaseContext, List entries) { - // Reload repository to ensure latest preferences are used - abbreviationRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); - + // Use the repository manager to get a cached repository or build a new one if needed + abbreviationRepository = JournalAbbreviationRepositoryManager.getInstance() + .getRepository(journalAbbreviationPreferences); + UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator( abbreviationRepository, abbreviationType, @@ -139,7 +140,8 @@ private String abbreviate(BibDatabaseContext databaseContext, List ent private String unabbreviate(BibDatabaseContext databaseContext, List entries) { List filteredEntries = new ArrayList<>(); - JournalAbbreviationRepository freshRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); + JournalAbbreviationRepository freshRepository = JournalAbbreviationRepositoryManager.getInstance() + .getRepository(journalAbbreviationPreferences); Map sourceEnabledStates = new HashMap<>(); String builtInId = JournalAbbreviationRepository.BUILTIN_LIST_ID; diff --git a/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java index b4cb7199a93..89e86dde3ed 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java @@ -42,7 +42,7 @@ public boolean abbreviate(BibDatabase database, BibEntry entry, Field fieldName, String origText = entry.getField(fieldName).get(); String text = database != null ? database.resolveForStrings(origText) : origText; - Optional foundAbbreviation = journalAbbreviationRepository.get(text); + Optional foundAbbreviation = journalAbbreviationRepository.getForAbbreviation(text); if (foundAbbreviation.isEmpty() && abbreviationType != AbbreviationType.LTWA) { return false; // Unknown, cannot abbreviate anything. diff --git a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java index 447596e0e65..6f0103b10d9 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java @@ -48,7 +48,7 @@ public boolean unabbreviate(BibDatabase database, BibEntry entry, Field field, C return false; // Cannot unabbreviate unabbreviated name. } - var abbreviationOpt = journalAbbreviationRepository.get(text); + var abbreviationOpt = journalAbbreviationRepository.getForUnabbreviation(text); if (abbreviationOpt.isEmpty()) { return false; } diff --git a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java index 9e7cecd05b3..810b4c77fd4 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java @@ -32,13 +32,10 @@ import org.jabref.gui.preferences.PreferencesTab; import org.jabref.gui.util.ColorUtil; import org.jabref.gui.util.ValueTableCellFactory; -import org.jabref.logic.journals.JournalAbbreviationLoader; -import org.jabref.logic.journals.JournalAbbreviationPreferences; import org.jabref.logic.journals.JournalAbbreviationRepository; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.TaskExecutor; -import com.airhacks.afterburner.injection.Injector; import com.airhacks.afterburner.views.ViewLoader; import com.tobiasdiez.easybind.EasyBind; import jakarta.inject.Inject; @@ -312,22 +309,6 @@ private void toggleEnableList() { refreshComboBoxDisplay(); - JournalAbbreviationPreferences abbreviationPreferences = preferences.getJournalAbbreviationPreferences(); - - if (selected.isBuiltInListProperty().get()) { - abbreviationPreferences.setSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID, newEnabledState); - } else if (selected.getAbsolutePath().isPresent()) { - String fileName = selected.getAbsolutePath().get().getFileName().toString(); - abbreviationPreferences.setSourceEnabled(fileName, newEnabledState); - } - - JournalAbbreviationRepository newRepository = - JournalAbbreviationLoader.loadRepository(abbreviationPreferences); - - Injector.setModelOrService(JournalAbbreviationRepository.class, newRepository); - - abbreviationRepository = newRepository; - viewModel.markAsDirty(); } diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index e9ac00726cf..fbbb14e33c2 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -337,6 +337,42 @@ public void setSourceEnabled(String sourceKey, boolean enabled) { enabledSources.put(sourceKey, enabled); } + /** + * Specialized method for abbreviation that checks if a journal name is already in its + * full form and requires abbreviation from an enabled source. + * + * @param input The journal name to check and abbreviate. + * @return Optional containing the abbreviation object if the input is a full journal name + * from an enabled source, empty otherwise. + */ + public Optional getForAbbreviation(String input) { + String journal = input.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&"); + + Optional customAbbreviation = customAbbreviations.stream() + .filter(abbreviation -> isSourceEnabled(abbreviationSources.getOrDefault(abbreviation, BUILTIN_LIST_ID))) + .filter(abbreviation -> isMatched(journal, abbreviation)) + .findFirst(); + + if (customAbbreviation.isPresent()) { + return customAbbreviation; + } + + if (!isSourceEnabled(BUILTIN_LIST_ID)) { + return Optional.empty(); + } + + Optional builtInAbbreviation = Optional.ofNullable(fullToAbbreviationObject.get(journal)) + .or(() -> Optional.ofNullable(abbreviationToAbbreviationObject.get(journal))) + .or(() -> Optional.ofNullable(dotlessToAbbreviationObject.get(journal))) + .or(() -> Optional.ofNullable(shortestUniqueToAbbreviationObject.get(journal))); + + if (builtInAbbreviation.isPresent()) { + return builtInAbbreviation; + } + + return findAbbreviationFuzzyMatched(journal); + } + public Optional getNextAbbreviation(String text) { return get(text).map(abbreviation -> abbreviation.getNext(text)); } diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepositoryManager.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepositoryManager.java new file mode 100644 index 00000000000..38db9cefe60 --- /dev/null +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepositoryManager.java @@ -0,0 +1,143 @@ +package org.jabref.logic.journals; + +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +/** + * Manages caching of JournalAbbreviationRepository instances to improve performance. + * This class uses a thread-safe approach with read-write locks to ensure + * concurrent access while minimizing repository rebuilds. + */ +public class JournalAbbreviationRepositoryManager { + + private static final JournalAbbreviationRepositoryManager INSTANCE = new JournalAbbreviationRepositoryManager(); + private static final ReadWriteLock LOCK = new ReentrantReadWriteLock(); + + private JournalAbbreviationRepository repository; + private JournalAbbreviationPreferences lastUsedPreferences; + + /** + * Private constructor for singleton pattern + */ + private JournalAbbreviationRepositoryManager() { + // Private constructor to prevent instantiation + } + + /** + * Returns the singleton instance of the repository manager + * + * @return The singleton instance + */ + public static JournalAbbreviationRepositoryManager getInstance() { + return INSTANCE; + } + + /** + * Gets a repository instance for the given preferences. This method implements + * caching to avoid rebuilding the repository if preferences haven't changed. + * Uses double-checked locking for thread safety and efficiency. + * + * @param preferences The journal abbreviation preferences + * @return A repository instance configured with the given preferences + */ + public JournalAbbreviationRepository getRepository(JournalAbbreviationPreferences preferences) { + Objects.requireNonNull(preferences); + + LOCK.readLock().lock(); + try { + if (repository != null && !preferencesChanged(preferences)) { + return repository; + } + } finally { + LOCK.readLock().unlock(); + } + + LOCK.writeLock().lock(); + try { + if (repository != null && !preferencesChanged(preferences)) { + return repository; + } + + repository = JournalAbbreviationLoader.loadRepository(preferences); + lastUsedPreferences = clonePreferences(preferences); + return repository; + } finally { + LOCK.writeLock().unlock(); + } + } + + /** + * Checks if the preferences have changed since the last repository build + * + * @param preferences The current preferences to check + * @return true if preferences have changed, false otherwise + */ + private boolean preferencesChanged(JournalAbbreviationPreferences preferences) { + if (lastUsedPreferences == null) { + return true; + } + + if (lastUsedPreferences.shouldUseFJournalField() != preferences.shouldUseFJournalField()) { + return true; + } + + List oldLists = lastUsedPreferences.getExternalJournalLists(); + List newLists = preferences.getExternalJournalLists(); + + if (oldLists.size() != newLists.size()) { + return true; + } + + for (int i = 0; i < oldLists.size(); i++) { + if (!Objects.equals(oldLists.get(i), newLists.get(i))) { + return true; + } + } + + Map oldEnabled = lastUsedPreferences.getEnabledExternalLists(); + Map newEnabled = preferences.getEnabledExternalLists(); + + if (oldEnabled.size() != newEnabled.size()) { + return true; + } + + for (Map.Entry entry : newEnabled.entrySet()) { + Boolean oldValue = oldEnabled.get(entry.getKey()); + if (!Objects.equals(oldValue, entry.getValue())) { + return true; + } + } + + return false; + } + + /** + * Creates a clone of the preferences for comparison + * + * @param preferences The preferences to clone + * @return A new preferences instance with the same settings + */ + private JournalAbbreviationPreferences clonePreferences(JournalAbbreviationPreferences preferences) { + return new JournalAbbreviationPreferences( + preferences.getExternalJournalLists(), + preferences.shouldUseFJournalField(), + preferences.getEnabledExternalLists() + ); + } + + /** + * For testing purposes only - clears the cached repository + */ + public void clear() { + LOCK.writeLock().lock(); + try { + repository = null; + lastUsedPreferences = null; + } finally { + LOCK.writeLock().unlock(); + } + } +} From f50883d6e562d55267c636c3b84780ee81c6431a Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 05:27:47 +0800 Subject: [PATCH 40/41] fix: address trag-bot comments --- src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java | 2 ++ .../gui/preferences/journals/AbbreviationsFileViewModel.java | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java b/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java index 89e86dde3ed..7269022a417 100644 --- a/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java +++ b/src/main/java/org/jabref/gui/journals/UndoableAbbreviator.java @@ -27,6 +27,8 @@ public UndoableAbbreviator(JournalAbbreviationRepository journalAbbreviationRepo /** * Abbreviate the journal name of the given entry. + * This method respects the enabled/disabled state of journal abbreviation sources. + * If a journal name comes from a disabled source, it will not be abbreviated. * * @param database The database the entry belongs to, or null if no database. * @param entry The entry to be treated. diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index e63df22da10..48f60c8443a 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -90,7 +90,7 @@ public void readAbbreviations() throws IOException { List viewModels = abbreviationsFromFile.stream() .map(AbbreviationViewModel::new) - .collect(Collectors.toCollection(ArrayList::new)); + .toList(); abbreviations.setAll(viewModels); } catch (NoSuchFileException e) { LOGGER.debug("Journal abbreviation list {} does not exist", filePath, e); From 72470380aa6956b15a6520ad560cd323bec74a8d Mon Sep 17 00:00:00 2001 From: Zikun Zhu Date: Sun, 27 Apr 2025 05:35:02 +0800 Subject: [PATCH 41/41] fix: address Checkstyle issues --- .../gui/preferences/journals/AbbreviationsFileViewModel.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java index 48f60c8443a..2e15aeffe34 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -4,12 +4,10 @@ import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyBooleanProperty;