diff --git a/CHANGELOG.md b/CHANGELOG.md index 66000c5103f..cc6704ed2bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,8 +21,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We changed database structure: in MySQL/MariaDB we renamed tables by adding a `JABREF_` prefix, and in PGSQL we moved tables in `jabref` schema. We added `VersionDBStructure` variable in `METADATA` table to indicate current version of structure, this variable is needed for automatic migration. [#9312](https://github.com/JabRef/jabref/issues/9312) - We moved some preferences options to a new tab in the preferences dialog. [#9442](https://github.com/JabRef/jabref/pull/9308) -- We renamed "Medline abbrevation" to "dotless abbreviation". [#9504](https://github.com/JabRef/jabref/pull/9504) -- We now have more "dots" in the offered journal abbrevations. [#9504](https://github.com/JabRef/jabref/pull/9504) +- We renamed "Medline abbreviation" to "dotless abbreviation". [#9504](https://github.com/JabRef/jabref/pull/9504) +- We now have more "dots" in the offered journal abbreviations. [#9504](https://github.com/JabRef/jabref/pull/9504) @@ -32,7 +32,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - The tab "deprecated fields" is shown in biblatex-mode only. [#7757](https://github.com/JabRef/jabref/issues/7757) - In case a journal name of an IEEE journal is abbreviated, the "normal" abbreviation is used - and not the one of the IEEE BibTeX strings. [abbrv#91](https://github.com/JabRef/abbrv.jabref.org/issues/91) -- We fixed an issue where the last opened libraries were not remembered when a new unsaved libray was open as well. [#9190](https://github.com/JabRef/jabref/issues/9190) +- We fixed a performance issue when loading large lists of custom journal abbreviations. [#8928](https://github.com/JabRef/jabref/issues/8928) +- We fixed an issue where the last opened libraries were not remembered when a new unsaved library was open as well. [#9190](https://github.com/JabRef/jabref/issues/9190) - We fixed an issue where no context menu for the group "All entries" was present. [forum#3682](https://discourse.jabref.org/t/how-sort-groups-a-z-not-subgroups/3682) - We fixed an issue where extra curly braces in some fields would trigger an exception when selecting the entry or doing an integrity check [#9475](https://github.com/JabRef/jabref/issues/9475), [#9503](https://github.com/JabRef/jabref/issues/9503) - We fixed an issue where entering a date in the format "YYYY/MM" in the entry editor date field caused an exception. [#9492](https://github.com/JabRef/jabref/issues/9492) diff --git a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationViewModel.java b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationViewModel.java index b8f294fb06a..1c13439cf6b 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationViewModel.java @@ -19,6 +19,8 @@ public class AbbreviationViewModel { private final StringProperty name = new SimpleStringProperty(""); private final StringProperty abbreviation = new SimpleStringProperty(""); private final StringProperty shortestUniqueAbbreviation = new SimpleStringProperty(""); + + // Used when a "null" abbreviation object is added private final BooleanProperty pseudoAbbreviation = new SimpleBooleanProperty(); public AbbreviationViewModel(Abbreviation abbreviationObject) { @@ -26,7 +28,15 @@ public AbbreviationViewModel(Abbreviation abbreviationObject) { if (abbreviationObject != null) { this.name.setValue(abbreviationObject.getName()); this.abbreviation.setValue(abbreviationObject.getAbbreviation()); - this.shortestUniqueAbbreviation.setValue(abbreviationObject.getShortestUniqueAbbreviation()); + + // the view model stores the "real" values, not the default fallback + String shortestUniqueAbbreviationOfAbbreviation = abbreviationObject.getShortestUniqueAbbreviation(); + boolean shortestUniqueAbbreviationIsDefaultValue = shortestUniqueAbbreviationOfAbbreviation.equals(abbreviationObject.getAbbreviation()); + if (shortestUniqueAbbreviationIsDefaultValue) { + this.shortestUniqueAbbreviation.set(""); + } else { + this.shortestUniqueAbbreviation.setValue(shortestUniqueAbbreviationOfAbbreviation); + } } } @@ -87,13 +97,15 @@ public boolean equals(Object o) { return false; } AbbreviationViewModel that = (AbbreviationViewModel) o; - return Objects.equals(getName(), that.getName()) && - isPseudoAbbreviation() == that.isPseudoAbbreviation(); + return getName().equals(that.getName()) + && getAbbreviation().equals(that.getAbbreviation()) + && getShortestUniqueAbbreviation().equals(that.getShortestUniqueAbbreviation()) + && (isPseudoAbbreviation() == that.isPseudoAbbreviation()); } @Override public int hashCode() { - return Objects.hash(getName(), isPseudoAbbreviation()); + return Objects.hash(getName(), getAbbreviation(), getShortestUniqueAbbreviation(), isPseudoAbbreviation()); } public boolean containsCaseIndependent(String searchTerm) { 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 4c6961cdf0b..c1e8424d45c 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/AbbreviationsFileViewModel.java @@ -2,9 +2,9 @@ import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -39,7 +39,7 @@ public AbbreviationsFileViewModel(Path filePath) { /** * This constructor should only be called to create a pseudo abbreviation file for built in lists. This means it is - * a placeholder and it's path will be null meaning it has no place on the filesystem. It's isPseudoFile property + * 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. */ public AbbreviationsFileViewModel(List abbreviations, String name) { @@ -51,7 +51,7 @@ public AbbreviationsFileViewModel(List abbreviations, Str public void readAbbreviations() throws IOException { if (path.isPresent()) { - List abbreviationList = JournalAbbreviationLoader.readJournalListFromFile(path.get()); + Collection abbreviationList = JournalAbbreviationLoader.readJournalListFromFile(path.get()); abbreviationList.forEach(abbreviation -> abbreviations.addAll(new AbbreviationViewModel(abbreviation))); } else { throw new FileNotFoundException(); @@ -60,15 +60,16 @@ public void readAbbreviations() throws IOException { /** * 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 built in list and if not it will call + * 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, StandardCharsets.UTF_8); + .map(AbbreviationViewModel::getAbbreviationObject) + .collect(Collectors.toList()); + AbbreviationWriter.writeOrCreate(path.get(), actualAbbreviations); } } 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 43d5cf4b18e..3dc07113edf 100644 --- a/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java @@ -185,7 +185,7 @@ private void openFile(Path filePath) { try { abbreviationsFile.readAbbreviations(); } catch (IOException e) { - LOGGER.debug(e.getLocalizedMessage()); + LOGGER.debug("Could not read abbreviations file", e); } } journalFiles.add(abbreviationsFile); @@ -217,17 +217,12 @@ public void removeCurrentFile() { /** * Method to add a new abbreviation to the abbreviations list property. It also sets the currentAbbreviation * property to the new abbreviation. - * - * @param name name of the abbreviation - * @param abbreviation default abbreviation of the abbreviation - * @param shortestUniqueAbbreviation shortest unique abbreviation of the abbreviation */ - public void addAbbreviation(String name, String abbreviation, String shortestUniqueAbbreviation) { - Abbreviation abbreviationObject = new Abbreviation(name, abbreviation, shortestUniqueAbbreviation); + public void addAbbreviation(Abbreviation abbreviationObject) { AbbreviationViewModel abbreviationViewModel = new AbbreviationViewModel(abbreviationObject); if (abbreviations.contains(abbreviationViewModel)) { dialogService.showErrorDialogAndWait(Localization.lang("Duplicated Journal Abbreviation"), - Localization.lang("Abbreviation '%0' for journal '%1' already defined.", abbreviation, name)); + Localization.lang("Abbreviation '%0' for journal '%1' already defined.", abbreviationObject.getAbbreviation(), abbreviationObject.getName())); } else { abbreviations.add(abbreviationViewModel); currentAbbreviation.set(abbreviationViewModel); @@ -235,65 +230,49 @@ public void addAbbreviation(String name, String abbreviation, String shortestUni } } - public void addAbbreviation(String name, String abbreviation) { - addAbbreviation(name, abbreviation, ""); - } - public void addAbbreviation() { - addAbbreviation( + addAbbreviation(new Abbreviation( Localization.lang("Name"), Localization.lang("Abbreviation"), - Localization.lang("Shortest unique abbreviation")); + Localization.lang("Shortest unique abbreviation"))); } /** * Method to change the currentAbbreviation property to a new abbreviation. - * - * @param name name of the abbreviation - * @param abbreviation default abbreviation of the abbreviation - * @param shortestUniqueAbbreviation shortest unique abbreviation of the abbreviation */ - public void editAbbreviation(String name, String abbreviation, String shortestUniqueAbbreviation) { + void editAbbreviation(Abbreviation abbreviationObject) { if (isEditableAndRemovable.get()) { - Abbreviation abbreviationObject = new Abbreviation(name, abbreviation, shortestUniqueAbbreviation); AbbreviationViewModel abbViewModel = new AbbreviationViewModel(abbreviationObject); if (abbreviations.contains(abbViewModel)) { if (abbViewModel.equals(currentAbbreviation.get())) { - setCurrentAbbreviationNameAndAbbreviationIfValid(name, abbreviation, shortestUniqueAbbreviation); + setCurrentAbbreviationNameAndAbbreviationIfValid(abbreviationObject); } else { dialogService.showErrorDialogAndWait(Localization.lang("Duplicated Journal Abbreviation"), - Localization.lang("Abbreviation '%0' for journal '%1' already defined.", abbreviation, name)); + Localization.lang("Abbreviation '%0' for journal '%1' already defined.", abbreviationObject.getAbbreviation(), abbreviationObject.getName())); } } else { - setCurrentAbbreviationNameAndAbbreviationIfValid(name, abbreviation, shortestUniqueAbbreviation); + setCurrentAbbreviationNameAndAbbreviationIfValid(abbreviationObject); } } } - public void editAbbreviation(String name, String abbreviation) { - editAbbreviation(name, abbreviation, ""); - } - - /** - * Sets the name and the abbreviation of the {@code currentAbbreviation} property to the values of the {@code name}, - * {@code abbreviation}, and {@code shortestUniqueAbbreviation} properties. - * - * @param name name of the abbreviation - * @param abbreviation default abbreviation of the abbreviation - * @param shortestUniqueAbbreviation shortest unique abbreviation of the abbreviation - */ - private void setCurrentAbbreviationNameAndAbbreviationIfValid(String name, String abbreviation, String shortestUniqueAbbreviation) { - if (name.trim().isEmpty()) { + private void setCurrentAbbreviationNameAndAbbreviationIfValid(Abbreviation abbreviationObject) { + if (abbreviationObject.getName().trim().isEmpty()) { dialogService.showErrorDialogAndWait(Localization.lang("Name cannot be empty")); return; } - if (abbreviation.trim().isEmpty()) { + if (abbreviationObject.getAbbreviation().trim().isEmpty()) { dialogService.showErrorDialogAndWait(Localization.lang("Abbreviation cannot be empty")); return; } - currentAbbreviation.get().setName(name); - currentAbbreviation.get().setAbbreviation(abbreviation); - currentAbbreviation.get().setShortestUniqueAbbreviation(shortestUniqueAbbreviation); + AbbreviationViewModel abbreviationViewModel = currentAbbreviation.get(); + abbreviationViewModel.setName(abbreviationObject.getName()); + abbreviationViewModel.setAbbreviation(abbreviationObject.getAbbreviation()); + if (abbreviationObject.isDefaultShortestUniqueAbbreviation()) { + abbreviationViewModel.setShortestUniqueAbbreviation(""); + } else { + abbreviationViewModel.setShortestUniqueAbbreviation(abbreviationObject.getShortestUniqueAbbreviation()); + } shouldWriteLists = true; } @@ -331,14 +310,14 @@ public void removeAbbreviation(AbbreviationViewModel abbreviation) { /** * Calls the {@link AbbreviationsFileViewModel#writeOrCreate()} method for each file in the journalFiles property * which will overwrite the existing files with the content of the abbreviations property of the AbbreviationsFile. - * Non existing files will be created. + * Non-existing files will be created. */ public void saveJournalAbbreviationFiles() { journalFiles.forEach(file -> { try { file.writeOrCreate(); } catch (IOException e) { - LOGGER.debug(e.getLocalizedMessage()); + LOGGER.debug("Error during writing journal CSV", e); } }); } diff --git a/src/main/java/org/jabref/logic/journals/Abbreviation.java b/src/main/java/org/jabref/logic/journals/Abbreviation.java index c3c18ff02bf..a207cc20431 100644 --- a/src/main/java/org/jabref/logic/journals/Abbreviation.java +++ b/src/main/java/org/jabref/logic/journals/Abbreviation.java @@ -10,7 +10,9 @@ public class Abbreviation implements Comparable, Serializable { private transient String name; private final String abbreviation; private transient String dotlessAbbreviation; - private final String shortestUniqueAbbreviation; + + // Is the empty string if not available + private String shortestUniqueAbbreviation; public Abbreviation(String name, String abbreviation) { this(name, abbreviation, ""); @@ -24,7 +26,7 @@ public Abbreviation(String name, String abbreviation, String shortestUniqueAbbre shortestUniqueAbbreviation.trim()); } - public Abbreviation(String name, String abbreviation, String dotlessAbbreviation, String shortestUniqueAbbreviation) { + private Abbreviation(String name, String abbreviation, String dotlessAbbreviation, String shortestUniqueAbbreviation) { this.name = name.intern(); this.abbreviation = abbreviation.intern(); this.dotlessAbbreviation = dotlessAbbreviation.intern(); @@ -40,11 +42,14 @@ public String getAbbreviation() { } public String getShortestUniqueAbbreviation() { - String result = shortestUniqueAbbreviation; - if (result.isEmpty()) { - return getAbbreviation(); + if (shortestUniqueAbbreviation.isEmpty()) { + shortestUniqueAbbreviation = getAbbreviation(); } - return result; + return shortestUniqueAbbreviation; + } + + public boolean isDefaultShortestUniqueAbbreviation() { + return (shortestUniqueAbbreviation.isEmpty()) || this.shortestUniqueAbbreviation.equals(this.abbreviation); } public String getDotlessAbbreviation() { @@ -53,7 +58,17 @@ public String getDotlessAbbreviation() { @Override public int compareTo(Abbreviation toCompare) { - return getName().compareTo(toCompare.getName()); + int nameComparison = getName().compareTo(toCompare.getName()); + if (nameComparison != 0) { + return nameComparison; + } + + int abbreviationComparison = getAbbreviation().compareTo(toCompare.getAbbreviation()); + if (abbreviationComparison != 0) { + return abbreviationComparison; + } + + return getShortestUniqueAbbreviation().compareTo(toCompare.getShortestUniqueAbbreviation()); } public String getNext(String current) { @@ -80,22 +95,19 @@ public String toString() { } @Override - public boolean equals(Object obj) { - if (this == obj) { + public boolean equals(Object o) { + if (this == o) { return true; } - - if ((obj == null) || (getClass() != obj.getClass())) { + if (o == null || getClass() != o.getClass()) { return false; } - - Abbreviation that = (Abbreviation) obj; - - return Objects.equals(getName(), that.getName()); + Abbreviation that = (Abbreviation) o; + return getName().equals(that.getName()) && getAbbreviation().equals(that.getAbbreviation()) && getShortestUniqueAbbreviation().equals(that.getShortestUniqueAbbreviation()); } @Override public int hashCode() { - return Objects.hash(getName()); + return Objects.hash(getName(), getAbbreviation(), getShortestUniqueAbbreviation()); } } diff --git a/src/main/java/org/jabref/logic/journals/AbbreviationParser.java b/src/main/java/org/jabref/logic/journals/AbbreviationParser.java index 6506469d7da..9a9f67f0bfe 100644 --- a/src/main/java/org/jabref/logic/journals/AbbreviationParser.java +++ b/src/main/java/org/jabref/logic/journals/AbbreviationParser.java @@ -9,10 +9,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Set; +import java.util.Collection; +import java.util.LinkedHashSet; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -26,9 +24,10 @@ public class AbbreviationParser { private static final Logger LOGGER = LoggerFactory.getLogger(AbbreviationParser.class); - private final Set abbreviations = new HashSet<>(5000); + // Ensures ordering while preventing duplicates + private final LinkedHashSet abbreviations = new LinkedHashSet<>(); - public void readJournalListFromResource(String resourceFileName) { + void readJournalListFromResource(String resourceFileName) { try (InputStream stream = JournalAbbreviationRepository.class.getResourceAsStream(resourceFileName); BufferedReader reader = new BufferedReader(new InputStreamReader(stream, StandardCharsets.UTF_8))) { readJournalList(reader); @@ -65,12 +64,13 @@ private void readJournalList(Reader reader) throws IOException { return; } - abbreviations.add(new Abbreviation(name, abbreviation, shortestUniqueAbbreviation)); + Abbreviation abbreviationToAdd = new Abbreviation(name, abbreviation, shortestUniqueAbbreviation); + abbreviations.add(abbreviationToAdd); } } } - public List getAbbreviations() { - return new LinkedList<>(abbreviations); + public Collection getAbbreviations() { + return abbreviations; } } diff --git a/src/main/java/org/jabref/logic/journals/AbbreviationWriter.java b/src/main/java/org/jabref/logic/journals/AbbreviationWriter.java index d922ddd488d..4a3054f7634 100644 --- a/src/main/java/org/jabref/logic/journals/AbbreviationWriter.java +++ b/src/main/java/org/jabref/logic/journals/AbbreviationWriter.java @@ -2,7 +2,7 @@ import java.io.IOException; import java.io.OutputStreamWriter; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -24,11 +24,11 @@ private AbbreviationWriter() { * @param path to a file (doesn't have to exist just yet) * @param abbreviations as a list specifying which entries should be written */ - public static void writeOrCreate(Path path, List abbreviations, Charset encoding) throws IOException { - try (OutputStreamWriter writer = new OutputStreamWriter(Files.newOutputStream(path), encoding); + public static void writeOrCreate(Path path, List abbreviations) throws IOException { + try (OutputStreamWriter writer = new OutputStreamWriter(Files.newOutputStream(path), StandardCharsets.UTF_8); CSVPrinter csvPrinter = new CSVPrinter(writer, AbbreviationFormat.getCSVFormat())) { for (Abbreviation entry : abbreviations) { - if (entry.getShortestUniqueAbbreviation().isEmpty()) { + if (entry.isDefaultShortestUniqueAbbreviation()) { csvPrinter.printRecord(entry.getName(), entry.getAbbreviation()); } else { csvPrinter.printRecord(entry.getName(), entry.getAbbreviation(), entry.getShortestUniqueAbbreviation()); diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java index d74b6a9fc9d..2c67a734a8b 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java @@ -4,6 +4,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -22,7 +23,7 @@ public class JournalAbbreviationLoader { private static final Logger LOGGER = LoggerFactory.getLogger(JournalAbbreviationLoader.class); - public static List readJournalListFromFile(Path file) throws IOException { + public static Collection readJournalListFromFile(Path file) throws IOException { LOGGER.debug(String.format("Reading journal list from file %s", file)); AbbreviationParser parser = new AbbreviationParser(); parser.readJournalListFromFile(file); diff --git a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java index 29b61f0d960..ca0db044368 100644 --- a/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java +++ b/src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java @@ -1,14 +1,13 @@ package org.jabref.logic.journals; import java.nio.file.Path; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -25,7 +24,7 @@ public class JournalAbbreviationRepository { private final Map abbreviationToAbbreviationObject = new HashMap<>(); private final Map dotlessToAbbreviationObject = new HashMap<>(); private final Map shortestUniqueToAbbreviationObject = new HashMap<>(); - private final List customAbbreviations = new ArrayList<>(); + private final TreeSet customAbbreviations = new TreeSet<>(); public JournalAbbreviationRepository(Path journalList) { MVStore store = new MVStore.Builder().readOnly().fileName(journalList.toAbsolutePath().toString()).open(); @@ -106,7 +105,7 @@ public Optional get(String input) { Optional customAbbreviation = customAbbreviations.stream() .filter(abbreviation -> isMatched(journal, abbreviation)) - .findAny(); + .findFirst(); if (customAbbreviation.isPresent()) { return customAbbreviation; } @@ -120,13 +119,13 @@ public Optional get(String input) { public void addCustomAbbreviation(Abbreviation abbreviation) { Objects.requireNonNull(abbreviation); - // We do not want to keep duplicates, thus remove the old abbreviation - // (abbreviation equality is tested on name only, so we cannot use a Set instead) - customAbbreviations.remove(abbreviation); + // We do NOT want to keep duplicates + // 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); } - public List getCustomAbbreviations() { + public Collection getCustomAbbreviations() { return customAbbreviations; } 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 c7db04c42e4..10e87331d04 100644 --- a/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java +++ b/src/test/java/org/jabref/gui/preferences/journals/JournalAbbreviationsViewModelTabTest.java @@ -4,6 +4,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -24,11 +25,11 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import static org.jabref.logic.util.OS.NEWLINE; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; @@ -39,8 +40,28 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +@Execution(ExecutionMode.CONCURRENT) class JournalAbbreviationsViewModelTabTest { + private final static TestAbbreviation ABBREVIATION_0 = new TestAbbreviation("Full0", "Abb0"); + private final static TestAbbreviation ABBREVIATION_0_OTHER_SHORT_UNIQUE = new TestAbbreviation("Full0", "Abb0", "A0"); + + private final static TestAbbreviation ABBREVIATION_1 = new TestAbbreviation("Full1", "Abb1"); + private final static TestAbbreviation ABBREVIATION_1_SHOW = new TestAbbreviation("Full1", "Abb1", true); + private final static TestAbbreviation ABBREVIATION_1_OTHER_SHORT_UNIQUE = new TestAbbreviation("Full1", "Abb1", "A1"); + + private final static TestAbbreviation ABBREVIATION_2 = new TestAbbreviation("Full2", "Abb2"); + private final static TestAbbreviation ABBREVIATION_2_OTHER_SHORT_UNIQUE = new TestAbbreviation("Full2", "Abb2", "A2"); + + private final static TestAbbreviation ABBREVIATION_3 = new TestAbbreviation("Full3", "Abb3"); + private final static TestAbbreviation ABBREVIATION_3_OTHER_SHORT_UNIQUE = new TestAbbreviation("Full3", "Abb3", "A3"); + + private final static TestAbbreviation ABBREVIATION_4 = new TestAbbreviation("Full4", "Abb4"); + + private final static TestAbbreviation ABBREVIATION_5 = new TestAbbreviation("Full5", "Abb5"); + + private final static TestAbbreviation ABBREVIATION_6 = new TestAbbreviation("Full6", "Abb6"); + private JournalAbbreviationsTabViewModel viewModel; private Path emptyTestFile; private Path tempFolder; @@ -48,37 +69,113 @@ class JournalAbbreviationsViewModelTabTest { private final JournalAbbreviationRepository repository = JournalAbbreviationLoader.loadBuiltInRepository(); private DialogService dialogService; - public static Stream provideTestFiles() { + static class TestAbbreviation extends Abbreviation { + + private final boolean showShortestUniqueAbbreviation; + + public TestAbbreviation(String name, String abbreviation) { + this(name, abbreviation, false); + } + + public TestAbbreviation(String name, String abbreviation, boolean showShortestUniqueAbbreviation) { + super(name, abbreviation); + this.showShortestUniqueAbbreviation = showShortestUniqueAbbreviation; + } + + public TestAbbreviation(String name, String abbreviation, String shortestUniqueAbbreviation) { + super(name, abbreviation, shortestUniqueAbbreviation); + this.showShortestUniqueAbbreviation = true; + } + + @Override + public String toString() { + if (showShortestUniqueAbbreviation) { + return this.getName() + ";" + this.getAbbreviation() + ";" + this.getShortestUniqueAbbreviation(); + } + return this.getName() + ";" + this.getAbbreviation(); + } + } + + private static String csvListOfAbbreviations(List testAbbreviations) { + return testAbbreviations.stream() + .map(TestAbbreviation::toString) + .collect(Collectors.joining("\n")); + } + + private static String csvListOfAbbreviations(TestAbbreviation... testAbbreviations) { + return csvListOfAbbreviations(Arrays.stream(testAbbreviations).toList()); + } + + private record CsvFileNameAndContent(String fileName, String content) { + CsvFileNameAndContent(String fileName, TestAbbreviation... testAbbreviations) { + this(fileName, csvListOfAbbreviations(testAbbreviations)); + } + } + + private record TestData( + List csvFiles, + TestAbbreviation abbreviationToCheck, + List finalContentsOfFile2, + List finalContentsOfFile3 + ) { + /** + * Note that we have a **different** ordering at the constructor, because Java generics have "type erasure" + */ + public TestData( + List csvFiles, + List finalContentsOfFile2, + List finalContentsOfFile3, + TestAbbreviation abbreviationToCheck + ) { + this(csvFiles, + abbreviationToCheck, + finalContentsOfFile2.stream().map(TestAbbreviation::toString).toList(), + finalContentsOfFile3.stream().map(TestAbbreviation::toString).toList()); + } + } + + public static Stream provideTestFiles() { + // filenameing: testfileXY, where X is the number of test (count starts at 1), and Y is the number of the file in the test (count starts at 0) + // testfile_3 has 5 entries after de-duplication return Stream.of( - // Mixed abbreviations - Arguments.of( - List.of(List.of("testFile1Entries.csv", "Test Entry;TE" + NEWLINE + ""), - List.of("testFile3Entries.csv", "Abbreviations;Abb;A" + NEWLINE + "Test Entry;TE" + NEWLINE + "MoreEntries;ME;M" + NEWLINE + ""), - List.of("testFile4Entries.csv", "Abbreviations;Abb" + NEWLINE + "Test Entry;TE;T" + NEWLINE + "MoreEntries;ME;M" + NEWLINE + "Entry;E" + NEWLINE + ""), - List.of("testFile5Entries.csv", "Abbreviations;Abb" + NEWLINE + "Test Entry;TE;T" + NEWLINE + "Test Entry;TE" + NEWLINE + "MoreEntries;ME;M" + NEWLINE + "EntryEntry;EE" + NEWLINE + "")), + // No shortest unique abbreviations in files + // Shortest unique abbreviations in entries (being the same then the abbreviation) + new TestData( List.of( - List.of("Abbreviations;Abb;Abb", "Test Entry;TE;T", "MoreEntries;ME;M", "JabRefTestEntry;JTE;JTE"), - List.of("EntryEntry;EE;EE", "Abbreviations;Abb;Abb", "Test Entry;TE;T", "SomeOtherEntry;SOE;SOE"))), - - // No shortest unique abbreviations - Arguments.of( - List.of(List.of("testFile1Entries.csv", "Test Entry;TE" + NEWLINE + ""), - List.of("testFile3Entries.csv", "Abbreviations;Abb" + NEWLINE + "Test Entry;TE" + NEWLINE + "MoreEntries;ME" + NEWLINE + ""), - List.of("testFile4Entries.csv", "Abbreviations;Abb" + NEWLINE + "Test Entry;TE" + NEWLINE + "MoreEntries;ME" + NEWLINE + "Entry;E" + NEWLINE + ""), - List.of("testFile5Entries.csv", "Abbreviations;Abb" + NEWLINE + "Test Entry;TE" + NEWLINE + "Test Entry;TE" + NEWLINE + "MoreEntries;ME" + NEWLINE + "EntryEntry;EE" + NEWLINE + "")), - List.of( - List.of("Abbreviations;Abb;Abb", "Test Entry;TE;TE", "MoreEntries;ME;ME", "JabRefTestEntry;JTE;JTE"), - List.of("EntryEntry;EE;EE", "Abbreviations;Abb;Abb", "Test Entry;TE;TE", "SomeOtherEntry;SOE;SOE"))), + new CsvFileNameAndContent("testFile10.csv", ABBREVIATION_1), + new CsvFileNameAndContent("testFile11.csv", ABBREVIATION_0, ABBREVIATION_1, ABBREVIATION_2), + new CsvFileNameAndContent("testFile12.csv", ABBREVIATION_0, ABBREVIATION_1, ABBREVIATION_2, ABBREVIATION_3), + new CsvFileNameAndContent("testFile13.csv", ABBREVIATION_0, ABBREVIATION_1, ABBREVIATION_1, ABBREVIATION_2, ABBREVIATION_3, ABBREVIATION_4)), + List.of(ABBREVIATION_0, ABBREVIATION_1, ABBREVIATION_2, ABBREVIATION_5), + List.of(ABBREVIATION_0, ABBREVIATION_1, ABBREVIATION_2, ABBREVIATION_3, ABBREVIATION_6), + ABBREVIATION_1_SHOW), // Shortest unique abbreviations - Arguments.of( - List.of(List.of("testFile1Entries.csv", "Test Entry;TE;T" + NEWLINE + ""), - List.of("testFile3Entries.csv", "Abbreviations;Abb;A" + NEWLINE + "Test Entry;TE;T" + NEWLINE + "MoreEntries;ME;M" + NEWLINE + ""), - List.of("testFile4Entries.csv", "Abbreviations;Abb;A" + NEWLINE + "Test Entry;TE;T" + NEWLINE + "MoreEntries;ME;M" + NEWLINE + "Entry;En;E" + NEWLINE + ""), - List.of("testFile5Entries.csv", "Abbreviations;Abb;A" + NEWLINE + "Test Entry;TE;T" + NEWLINE + "Test Entry;TE;T" + NEWLINE + "MoreEntries;ME;M" + NEWLINE + "EntryEntry;EE" + NEWLINE + "")), + new TestData( + List.of( + new CsvFileNameAndContent("testFile20.csv", ABBREVIATION_1_OTHER_SHORT_UNIQUE), + new CsvFileNameAndContent("testFile21.csv", ABBREVIATION_0_OTHER_SHORT_UNIQUE, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE), + new CsvFileNameAndContent("testFile22.csv", ABBREVIATION_0_OTHER_SHORT_UNIQUE, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_3_OTHER_SHORT_UNIQUE), + // contains duplicate entry ABBREVIATION_1_OTHER_SHORT_UNIQUE, therefore 6 entries + new CsvFileNameAndContent("testFile23.csv", ABBREVIATION_0_OTHER_SHORT_UNIQUE, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_3, ABBREVIATION_4)), + List.of(ABBREVIATION_0_OTHER_SHORT_UNIQUE, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_5), + // without duplicates + List.of(ABBREVIATION_0_OTHER_SHORT_UNIQUE, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_3, ABBREVIATION_6), + ABBREVIATION_1_OTHER_SHORT_UNIQUE), + + // Mixed abbreviations (some have shortest unique, some have not) + new TestData( List.of( - List.of("Abbreviations;Abb;A", "Test Entry;TE;T", "MoreEntries;ME;M", "JabRefTestEntry;JTE;JTE"), - List.of("EntryEntry;EE;EE", "Abbreviations;Abb;A", "Test Entry;TE;T", "SomeOtherEntry;SOE;SOE"))) + new CsvFileNameAndContent("testFile30.csv", ABBREVIATION_1), + new CsvFileNameAndContent("testFile31.csv", ABBREVIATION_0_OTHER_SHORT_UNIQUE, ABBREVIATION_1, ABBREVIATION_2_OTHER_SHORT_UNIQUE), + new CsvFileNameAndContent("testFile32.csv", ABBREVIATION_0, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_3), + // contains ABBREVIATION_1 in two variants, therefore 5 in total + new CsvFileNameAndContent("testFile33.csv", ABBREVIATION_0, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_1, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_4)), + // Entries of testFile2 plus ABBREVIATION_5_SHOW minus ABBREVIATION_3 + List.of(ABBREVIATION_0, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_5), + // Entries of testFile3 plus ABBREVIATION_6_SHOW minus ABBREVIATION_4 + List.of(ABBREVIATION_0, ABBREVIATION_1_OTHER_SHORT_UNIQUE, ABBREVIATION_1, ABBREVIATION_2_OTHER_SHORT_UNIQUE, ABBREVIATION_6), + ABBREVIATION_1_OTHER_SHORT_UNIQUE) ); } @@ -94,7 +191,7 @@ void setUpViewModel(@TempDir Path tempFolder) throws Exception { TaskExecutor taskExecutor = new CurrentThreadTaskExecutor(); viewModel = new JournalAbbreviationsTabViewModel(preferencesService, dialogService, taskExecutor, repository); - emptyTestFile = createTestFile("emptyTestFile.csv", ""); + emptyTestFile = createTestFile(new CsvFileNameAndContent("emptyTestFile.csv", "")); } @Test @@ -105,19 +202,19 @@ void testInitialHasNoFilesAndNoAbbreviations() { @ParameterizedTest @MethodSource("provideTestFiles") - void testInitialWithSavedFilesIncrementsFilesCounter(List> testFiles) throws IOException { - addFourTestFileToViewModelAndPreferences(testFiles); + void testInitialWithSavedFilesIncrementsFilesCounter(TestData testData) throws IOException { + addFourTestFileToViewModelAndPreferences(testData); assertEquals(4, viewModel.journalFilesProperty().size()); } @ParameterizedTest @MethodSource("provideTestFiles") - void testRemoveDuplicatesWhenReadingFiles(List> testFiles) throws IOException { - addFourTestFileToViewModelAndPreferences(testFiles); + void testRemoveDuplicatesWhenReadingFiles(TestData testData) throws IOException { + addFourTestFileToViewModelAndPreferences(testData); viewModel.selectLastJournalFile(); assertEquals(4, viewModel.journalFilesProperty().size()); - assertEquals(4, viewModel.abbreviationsProperty().size()); + assertEquals(5, viewModel.abbreviationsProperty().size()); } @Test @@ -131,8 +228,8 @@ void addFileIncreasesCounterOfOpenFilesAndHasNoAbbreviations() { @ParameterizedTest @MethodSource("provideTestFiles") - void addDuplicatedFileResultsInErrorDialog(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(0)))); + void addDuplicatedFileResultsInErrorDialog(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(0)))); viewModel.addNewFile(); viewModel.addNewFile(); verify(dialogService).showErrorDialogAndWait(anyString(), anyString()); @@ -140,8 +237,8 @@ void addDuplicatedFileResultsInErrorDialog(List> testFiles) throws @ParameterizedTest @MethodSource("provideTestFiles") - void testOpenDuplicatedFileResultsInAnException(List> testFiles) throws IOException { - when(dialogService.showFileOpenDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(0)))); + void testOpenDuplicatedFileResultsInAnException(TestData testData) throws IOException { + when(dialogService.showFileOpenDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(0)))); viewModel.openFile(); viewModel.openFile(); verify(dialogService).showErrorDialogAndWait(anyString(), anyString()); @@ -149,13 +246,13 @@ void testOpenDuplicatedFileResultsInAnException(List> testFiles) th @ParameterizedTest @MethodSource("provideTestFiles") - void testSelectLastJournalFileSwitchesFilesAndTheirAbbreviations(List> testFiles) throws IOException { + void testSelectLastJournalFileSwitchesFilesAndTheirAbbreviations(TestData testData) throws IOException { when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); viewModel.addNewFile(); viewModel.selectLastJournalFile(); assertEquals(0, viewModel.abbreviationsCountProperty().get()); - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(0)))); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(0)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); assertEquals(1, viewModel.abbreviationsCountProperty().get()); @@ -163,21 +260,21 @@ void testSelectLastJournalFileSwitchesFilesAndTheirAbbreviations(List> testFiles) throws IOException { - Abbreviation testAbbreviation = new Abbreviation("Test Entry", "TE"); - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(1)))); + void testOpenValidFileContainsTheSpecificEntryAndEnoughAbbreviations(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(2)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); assertEquals(1, viewModel.journalFilesProperty().size()); - assertEquals(3, viewModel.abbreviationsProperty().size()); - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); + assertEquals(4, viewModel.abbreviationsProperty().size()); + + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); } @ParameterizedTest @MethodSource("provideTestFiles") - void testRemoveLastListSetsCurrentFileAndCurrentAbbreviationToNull(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(0)))); + void testRemoveLastListSetsCurrentFileAndCurrentAbbreviationToNull(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(0)))); viewModel.addNewFile(); viewModel.removeCurrentFile(); @@ -189,23 +286,22 @@ void testRemoveLastListSetsCurrentFileAndCurrentAbbreviationToNull(List> testFiles) throws IOException { - Abbreviation testAbbreviation = new Abbreviation("Entry", "E"); - Abbreviation testAbbreviation2 = new Abbreviation("EntryEntry", "EE"); - + void testMixedFileUsage(TestData testData) throws IOException { // simulate open file button twice - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(1)))); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(1)))); viewModel.addNewFile(); - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(2)))); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(2)))); viewModel.addNewFile(); viewModel.currentFileProperty().set(viewModel.journalFilesProperty().get(1)); // size of the list of journal files should be incremented by two assertEquals(2, viewModel.journalFilesProperty().size()); - // our second test file has 4 abbreviations + + // our third test file has 4 abbreviations assertEquals(4, viewModel.abbreviationsProperty().size()); - // check some abbreviation - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); + + // check "arbitrary" abbreviation + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); // simulate add new file button when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); @@ -214,20 +310,23 @@ void testMixedFileUsage(List> testFiles) throws IOException { // size of the list of journal files should be incremented by one assertEquals(3, viewModel.journalFilesProperty().size()); + // a new file has zero abbreviations assertEquals(0, viewModel.abbreviationsProperty().size()); - // simulate open file button - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(3)))); + // simulate opening of testFile_3 + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(3)))); viewModel.addNewFile(); viewModel.currentFileProperty().set(viewModel.journalFilesProperty().get(3)); - // size of the list of journal files should be incremented by one + // size of the list of journal files should have been incremented by one assertEquals(4, viewModel.journalFilesProperty().size()); - assertEquals(4, viewModel.abbreviationsProperty().size()); - // check some abbreviation - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation2))); + // after de-duplication + assertEquals(5, viewModel.abbreviationsProperty().size()); + + // check "arbitrary" abbreviation + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testData.abbreviationToCheck))); } @Test @@ -246,8 +345,8 @@ void testBuiltInListsIncludeAllBuiltInAbbreviations() { @ParameterizedTest @MethodSource("provideTestFiles") - void testCurrentFilePropertyChangeActiveFile(List> testFiles) throws IOException { - for (List testFile : testFiles) { + void testCurrentFilePropertyChangeActiveFile(TestData testData) throws IOException { + for (CsvFileNameAndContent testFile : testData.csvFiles) { when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFile))); viewModel.addNewFile(); } @@ -259,7 +358,7 @@ void testCurrentFilePropertyChangeActiveFile(List> testFiles) throw AbbreviationsFileViewModel test5 = viewModel.journalFilesProperty().get(3); // test if the last opened file is active, but duplicated entry has been removed - assertEquals(4, viewModel.abbreviationsProperty().size()); + assertEquals(5, viewModel.abbreviationsProperty().size()); viewModel.currentFileProperty().set(test1); @@ -273,32 +372,32 @@ void testCurrentFilePropertyChangeActiveFile(List> testFiles) throw viewModel.currentFileProperty().set(test4); assertEquals(4, viewModel.abbreviationsProperty().size()); viewModel.currentFileProperty().set(test5); - assertEquals(4, viewModel.abbreviationsProperty().size()); + assertEquals(5, viewModel.abbreviationsProperty().size()); } @ParameterizedTest @MethodSource("provideTestFiles") - void testAddAbbreviationIncludesAbbreviationsInAbbreviationList(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(2)))); + void testAddAbbreviationIncludesAbbreviationsInAbbreviationList(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(2)))); viewModel.addNewFile(); - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(3)))); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(3)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); Abbreviation testAbbreviation = new Abbreviation("YetAnotherEntry", "YAE"); - addAbbreviation(testAbbreviation); + viewModel.addAbbreviation(testAbbreviation); - assertEquals(5, viewModel.abbreviationsProperty().size()); + assertEquals(6, viewModel.abbreviationsProperty().size()); assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); } @ParameterizedTest @MethodSource("provideTestFiles") - void testAddDuplicatedAbbreviationResultsInException(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(1)))); + void testAddDuplicatedAbbreviationResultsInException(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(1)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); - viewModel.addAbbreviation("YetAnotherEntry", "YAE"); - viewModel.addAbbreviation("YetAnotherEntry", "YAE"); + viewModel.addAbbreviation(new Abbreviation("YetAnotherEntry", "YAE")); + viewModel.addAbbreviation(new Abbreviation("YetAnotherEntry", "YAE")); verify(dialogService).showErrorDialogAndWait(anyString(), anyString()); } @@ -308,25 +407,25 @@ void testEditSameAbbreviationWithNoChangeDoesNotResultInException() { viewModel.addNewFile(); viewModel.selectLastJournalFile(); Abbreviation testAbbreviation = new Abbreviation("YetAnotherEntry", "YAE"); - addAbbreviation(testAbbreviation); - editAbbreviation(testAbbreviation); + viewModel.addAbbreviation(testAbbreviation); + viewModel.editAbbreviation(testAbbreviation); assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); } @ParameterizedTest @MethodSource("provideTestFiles") - void testEditAbbreviationIncludesNewAbbreviationInAbbreviationsList(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(2)))); + void testEditAbbreviationIncludesNewAbbreviationInAbbreviationsList(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(2)))); viewModel.addNewFile(); - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(3)))); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(3)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); selectLastAbbreviation(); Abbreviation testAbbreviation = new Abbreviation("YetAnotherEntry", "YAE"); - addAbbreviation(testAbbreviation); + viewModel.addAbbreviation(testAbbreviation); - assertEquals(5, viewModel.abbreviationsProperty().size()); + assertEquals(6, viewModel.abbreviationsProperty().size()); assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(emptyTestFile)); @@ -340,99 +439,92 @@ void testEditAbbreviationIncludesNewAbbreviationInAbbreviationsList(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(1)))); + void testEditAbbreviationToExistingOneResultsInException(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(1)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); selectLastAbbreviation(); assertEquals(3, viewModel.abbreviationsProperty().size()); - viewModel.editAbbreviation("YetAnotherEntry", "YAE"); + viewModel.editAbbreviation(new Abbreviation("YetAnotherEntry", "YAE")); viewModel.currentAbbreviationProperty().set(viewModel.abbreviationsProperty().get(1)); - viewModel.editAbbreviation("YetAnotherEntry", "YAE"); + viewModel.editAbbreviation(new Abbreviation("YetAnotherEntry", "YAE")); verify(dialogService).showErrorDialogAndWait(anyString(), anyString()); } @ParameterizedTest @MethodSource("provideTestFiles") - void testEditAbbreviationToEmptyNameResultsInException(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(1)))); + void testEditAbbreviationToEmptyNameResultsInException(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(1)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); selectLastAbbreviation(); assertEquals(3, viewModel.abbreviationsProperty().size()); - viewModel.editAbbreviation("", "YAE"); + viewModel.editAbbreviation(new Abbreviation("", "YAE")); verify(dialogService).showErrorDialogAndWait(anyString()); } @ParameterizedTest @MethodSource("provideTestFiles") - void testEditAbbreviationToEmptyAbbreviationResultsInException(List> testFiles) throws IOException { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFiles.get(1)))); + void testEditAbbreviationToEmptyAbbreviationResultsInException(TestData testData) throws IOException { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testData.csvFiles.get(1)))); viewModel.addNewFile(); viewModel.selectLastJournalFile(); selectLastAbbreviation(); assertEquals(3, viewModel.abbreviationsProperty().size()); - viewModel.editAbbreviation("YetAnotherEntry", ""); + viewModel.editAbbreviation(new Abbreviation("YetAnotherEntry", "")); verify(dialogService).showErrorDialogAndWait(anyString()); } @ParameterizedTest @MethodSource("provideTestFiles") - void testSaveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(List> testFiles, List> testEntries) throws Exception { - Path testFile4Entries = createTestFile(testFiles.get(2)); - Path testFile5EntriesWithDuplicate = createTestFile(testFiles.get(3)); - - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(testFile4Entries)); + void testSaveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(TestData testData) throws Exception { + Path testFile2 = createTestFile(testData.csvFiles.get(2)); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(testFile2)); viewModel.addNewFile(); viewModel.selectLastJournalFile(); selectLastAbbreviation(); - Abbreviation testAbbreviation = new Abbreviation("JabRefTestEntry", "JTE"); - editAbbreviation(testAbbreviation); - + viewModel.editAbbreviation(ABBREVIATION_5); assertEquals(4, viewModel.abbreviationsProperty().size()); - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation))); + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(ABBREVIATION_5))); - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(testFile5EntriesWithDuplicate)); + Path testFile3 = createTestFile(testData.csvFiles.get(3)); + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(testFile3)); viewModel.addNewFile(); + assertEquals(5, viewModel.abbreviationsProperty().size()); + viewModel.selectLastJournalFile(); + assertTrue(viewModel.currentFileProperty().get().getAbsolutePath().get().getFileName().toString().endsWith("3.csv")); selectLastAbbreviation(); viewModel.deleteAbbreviation(); - Abbreviation testAbbreviation1 = new Abbreviation("SomeOtherEntry", "SOE"); - addAbbreviation(testAbbreviation1); + viewModel.addAbbreviation(ABBREVIATION_6); - assertEquals(4, viewModel.abbreviationsProperty().size()); - assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(testAbbreviation1))); + // deletion and addition of an entry leads to same size + assertEquals(5, viewModel.abbreviationsProperty().size()); + assertTrue(viewModel.abbreviationsProperty().contains(new AbbreviationViewModel(ABBREVIATION_6))); + // overwrite all files viewModel.saveJournalAbbreviationFiles(); - List actual = Files.readAllLines(testFile4Entries, StandardCharsets.UTF_8); - assertEquals(testEntries.get(0), actual); + List actual = Files.readAllLines(testFile2, StandardCharsets.UTF_8); + assertEquals(testData.finalContentsOfFile2, actual); - actual = Files.readAllLines(testFile5EntriesWithDuplicate, StandardCharsets.UTF_8); - assertEquals(testEntries.get(1), actual); + actual = Files.readAllLines(testFile3, StandardCharsets.UTF_8); + assertEquals(testData.finalContentsOfFile3, actual); } @ParameterizedTest @MethodSource("provideTestFiles") - void testSaveExternalFilesListToPreferences(List> testFiles) throws IOException { - addFourTestFileToViewModelAndPreferences(testFiles); + void testSaveExternalFilesListToPreferences(TestData testData) throws IOException { + addFourTestFileToViewModelAndPreferences(testData); verify(preferencesService).storeJournalAbbreviationPreferences(any()); } - private void addAbbreviation(Abbreviation testAbbreviation) { - viewModel.addAbbreviation(testAbbreviation.getName(), testAbbreviation.getAbbreviation()); - } - - private void editAbbreviation(Abbreviation testAbbreviation) { - viewModel.editAbbreviation(testAbbreviation.getName(), testAbbreviation.getAbbreviation()); - } - /** * Select the last abbreviation in the list of abbreviations */ @@ -441,21 +533,17 @@ private void selectLastAbbreviation() { .set(viewModel.abbreviationsProperty().get(viewModel.abbreviationsCountProperty().get() - 1)); } - private void addFourTestFileToViewModelAndPreferences(List> testFiles) throws IOException { - for (List testFile : testFiles) { - when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(testFile.get(0), testFile.get(1)))); + private void addFourTestFileToViewModelAndPreferences(TestData testData) throws IOException { + for (CsvFileNameAndContent csvFile : testData.csvFiles) { + when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(createTestFile(csvFile))); viewModel.addNewFile(); } viewModel.storeSettings(); } - private Path createTestFile(String name, String content) throws IOException { - Path file = this.tempFolder.resolve(name); - Files.writeString(file, content); + private Path createTestFile(CsvFileNameAndContent testFile) throws IOException { + Path file = this.tempFolder.resolve(testFile.fileName); + Files.writeString(file, testFile.content); return file; } - - private Path createTestFile(List testFile) throws IOException { - return createTestFile(testFile.get(0), testFile.get(1)); - } } diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationTest.java index 38a0d7ee132..1f3ba7a8eab 100644 --- a/src/test/java/org/jabref/logic/journals/AbbreviationTest.java +++ b/src/test/java/org/jabref/logic/journals/AbbreviationTest.java @@ -3,8 +3,7 @@ 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; +import static org.junit.jupiter.api.Assertions.assertNotEquals; class AbbreviationTest { @@ -108,7 +107,14 @@ void testDefaultAndShortestUniqueAbbreviationsAreSame() { void testEquals() { Abbreviation abbreviation = new Abbreviation("Long Name", "L N", "LN"); Abbreviation otherAbbreviation = new Abbreviation("Long Name", "L N", "LN"); - assertTrue(abbreviation.equals(otherAbbreviation)); - assertFalse(abbreviation.equals("String")); + assertEquals(abbreviation, otherAbbreviation); + assertNotEquals(abbreviation, "String"); + } + + @Test + void equalAbbrevationsWithFourComponentsAreAlsoCompareZero() { + Abbreviation abbreviation1 = new Abbreviation("Long Name", "L. N.", "LN"); + Abbreviation abbreviation2 = new Abbreviation("Long Name", "L. N.", "LN"); + assertEquals(0, abbreviation1.compareTo(abbreviation2)); } } diff --git a/src/test/java/org/jabref/logic/journals/AbbreviationWriterTest.java b/src/test/java/org/jabref/logic/journals/AbbreviationWriterTest.java new file mode 100644 index 00000000000..c16e24b23ab --- /dev/null +++ b/src/test/java/org/jabref/logic/journals/AbbreviationWriterTest.java @@ -0,0 +1,36 @@ +package org.jabref.logic.journals; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +@Execution(ExecutionMode.CONCURRENT) +class AbbreviationWriterTest { + + @Test + void shortestUniqueAbbreviationWrittenIfItDiffers(@TempDir Path tempDir) throws Exception { + Abbreviation abbreviation = new Abbreviation("Full", "Abbr", "A"); + Path csvFile = tempDir.resolve("test.csv"); + AbbreviationWriter.writeOrCreate( + csvFile, + List.of(abbreviation)); + assertEquals(List.of("Full;Abbr;A"), Files.readAllLines(csvFile)); + } + + @Test + void doNotWriteShortestUniqueAbbreviationWrittenIfItDiffers(@TempDir Path tempDir) throws Exception { + Abbreviation abbreviation = new Abbreviation("Full", "Abbr"); + Path csvFile = tempDir.resolve("test.csv"); + AbbreviationWriter.writeOrCreate( + csvFile, + List.of(abbreviation)); + assertEquals(List.of("Full;Abbr"), Files.readAllLines(csvFile)); + } +} diff --git a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java index 42194347d92..a0e8465c542 100644 --- a/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java +++ b/src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java @@ -1,5 +1,7 @@ package org.jabref.logic.journals; +import java.util.Set; + import javax.swing.undo.CompoundEdit; import org.jabref.architecture.AllowedToUseSwing; @@ -120,26 +122,36 @@ void testDuplicatesIsoOnlyWithShortestUniqueAbbreviation() { @Test void testDuplicateKeys() { - repository.addCustomAbbreviation(new Abbreviation("Long Name", "L. N.")); - assertEquals(1, repository.getCustomAbbreviations().size()); + Abbreviation abbreviationOne = new Abbreviation("Long Name", "L. N."); + repository.addCustomAbbreviation(abbreviationOne); + assertEquals(Set.of(abbreviationOne), repository.getCustomAbbreviations()); assertEquals("L. N.", repository.getDefaultAbbreviation("Long Name").orElse("WRONG")); - repository.addCustomAbbreviation(new Abbreviation("Long Name", "LA. N.")); - assertEquals(1, repository.getCustomAbbreviations().size()); - assertEquals("LA. N.", repository.getDefaultAbbreviation("Long Name").orElse("WRONG")); + Abbreviation abbreviationTwo = new Abbreviation("Long Name", "LA. N."); + repository.addCustomAbbreviation(abbreviationTwo); + assertEquals(Set.of(abbreviationOne, abbreviationTwo), repository.getCustomAbbreviations()); + + // Both abbreviations are kept in the repository + // "L. N." is smaller than "LA. N.", therefore "L. N." is returned + assertEquals("L. N.", repository.getDefaultAbbreviation("Long Name").orElse("WRONG")); } @Test void testDuplicateKeysWithShortestUniqueAbbreviation() { - repository.addCustomAbbreviation(new Abbreviation("Long Name", "L. N.", "LN")); - assertEquals(1, repository.getCustomAbbreviations().size()); + Abbreviation abbreviationOne = new Abbreviation("Long Name", "L. N.", "LN"); + repository.addCustomAbbreviation(abbreviationOne); + assertEquals(Set.of(abbreviationOne), repository.getCustomAbbreviations()); assertEquals("L. N.", repository.getDefaultAbbreviation("Long Name").orElse("WRONG")); assertEquals("LN", repository.getShortestUniqueAbbreviation("Long Name").orElse("WRONG")); - repository.addCustomAbbreviation(new Abbreviation("Long Name", "LA. N.", "LAN")); - assertEquals(1, repository.getCustomAbbreviations().size()); - assertEquals("LA. N.", repository.getDefaultAbbreviation("Long Name").orElse("WRONG")); - assertEquals("LAN", repository.getShortestUniqueAbbreviation("Long Name").orElse("WRONG")); + Abbreviation abbreviationTwo = new Abbreviation("Long Name", "LA. N.", "LAN"); + repository.addCustomAbbreviation(abbreviationTwo); + assertEquals(Set.of(abbreviationOne, abbreviationTwo), repository.getCustomAbbreviations()); + + // Both abbreviations are kept in the repository + // "L. N." is smaller than "LA. N.", therefore "L. N." is returned + assertEquals("L. N.", repository.getDefaultAbbreviation("Long Name").orElse("WRONG")); + assertEquals("LN", repository.getShortestUniqueAbbreviation("Long Name").orElse("WRONG")); } @Test diff --git a/src/test/resources/junit-platform.properties b/src/test/resources/junit-platform.properties new file mode 100644 index 00000000000..4f6f62fc89c --- /dev/null +++ b/src/test/resources/junit-platform.properties @@ -0,0 +1,4 @@ +# See https://www.baeldung.com/junit-5-parallel-tests for details +junit.jupiter.execution.parallel.enabled = true + +# junit.jupiter.execution.parallel.mode.default = concurrent