Improve performance of journal abbreviation loader (#3009)

* Improve performance of journal abbreviation loader

* Fix test

src/main/java/org/jabref/collab/FileUpdateMonitor.java

@@ -20,9 +20,19 @@ public class FileUpdateMonitor implements Runnable {
     private static final Log LOGGER = LogFactory.getLog(FileUpdateMonitor.class);
 
     private static final int WAIT = 4000;
-
-    private int numberOfUpdateListener;
     private final Map<String, Entry> entries = new HashMap<>();
+    private int numberOfUpdateListener;
+
+    private static synchronized Path getTempFile() {
+        Path temporaryFile = null;
+        try {
+            temporaryFile = Files.createTempFile("jabref", null);
+            temporaryFile.toFile().deleteOnExit();
+        } catch (IOException ex) {
+            LOGGER.warn("Could not create temporary file.", ex);
+        }
+        return temporaryFile;
+    }
 
     @Override
     public void run() {
@@ -119,7 +129,7 @@ public void updateTimeStamp(String key) {
      * is used for comparison with the changed on-disk version.
      * @param key String The handle for this monitor.
      * @throws IllegalArgumentException If the handle doesn't correspond to an entry.
-     * @return File The temporary file.
+     * @return Path The temporary file.
      */
     public Path getTempFile(String key) throws IllegalArgumentException {
         Entry entry = entries.get(key);
@@ -129,7 +139,6 @@ public Path getTempFile(String key) throws IllegalArgumentException {
         return entry.getTmpFile();
     }
 
-
     /**
      * A class containing the File, the FileUpdateListener and the current time stamp for one file.
      */
@@ -209,15 +218,4 @@ public void decreaseTimeStamp() {
             timeStamp--;
         }
     }
-
-    private static synchronized Path getTempFile() {
-        Path temporaryFile = null;
-        try {
-            temporaryFile = Files.createTempFile("jabref", null);
-            temporaryFile.toFile().deleteOnExit();
-        } catch (IOException ex) {
-            LOGGER.warn("Could not create temporary file.", ex);
-        }
-        return temporaryFile;
-    }
 }

src/main/java/org/jabref/logic/journals/AbbreviationParser.java

@@ -11,9 +11,11 @@
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -25,7 +27,7 @@ public class AbbreviationParser {
 
     private static final Log LOGGER = LogFactory.getLog(AbbreviationParser.class);
 
-    private final List<Abbreviation> abbreviations = new LinkedList<>();
+    private final Set<Abbreviation> abbreviations = new HashSet<>(5000);
 
     public void readJournalListFromResource(String resourceFileName) {
         URL url = Objects.requireNonNull(JournalAbbreviationRepository.class.getResource(Objects.requireNonNull(resourceFileName)));
@@ -90,9 +92,7 @@ private void addLine(String line) {
             }
 
             Abbreviation abbreviation = new Abbreviation(fullName, abbrName);
-            if (!abbreviations.contains(abbreviation)) {
-                this.abbreviations.add(abbreviation);
-            }
+            this.abbreviations.add(abbreviation);
         }
     }
 

src/main/java/org/jabref/logic/journals/JournalAbbreviationRepository.java

@@ -1,14 +1,11 @@
 package org.jabref.logic.journals;
 
+import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
+import java.util.HashSet;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.SortedSet;
-import java.util.TreeSet;
+import java.util.Set;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -19,32 +16,35 @@
 public class JournalAbbreviationRepository {
 
     private static final Log LOGGER = LogFactory.getLog(JournalAbbreviationRepository.class);
-    private final Map<String, Abbreviation> fullNameLowerCase2Abbreviation = new HashMap<>();
-    private final Map<String, Abbreviation> isoLowerCase2Abbreviation = new HashMap<>();
-
-    private final Map<String, Abbreviation> medlineLowerCase2Abbreviation = new HashMap<>();
-
-    private final SortedSet<Abbreviation> abbreviations = new TreeSet<>();
+    private final Set<Abbreviation> abbreviations = new HashSet<>(16000); // We have over 15.000 abbreviations in the built-in lists
 
     public JournalAbbreviationRepository(Abbreviation... abbreviations) {
         for (Abbreviation abbreviation : abbreviations) {
             addEntry(abbreviation);
         }
     }
 
+    private static boolean isMatched(String name, Abbreviation abbreviation) {
+        return name.equalsIgnoreCase(abbreviation.getName())
+                || name.equalsIgnoreCase(abbreviation.getIsoAbbreviation())
+                || name.equalsIgnoreCase(abbreviation.getMedlineAbbreviation());
+    }
+
+    private static boolean isMatchedAbbreviated(String name, Abbreviation abbreviation) {
+        return name.equalsIgnoreCase(abbreviation.getIsoAbbreviation())
+                || name.equalsIgnoreCase(abbreviation.getMedlineAbbreviation());
+    }
+
     public int size() {
         return abbreviations.size();
     }
 
     public boolean isKnownName(String journalName) {
-        String nameKey = Objects.requireNonNull(journalName).trim().toLowerCase(Locale.ENGLISH);
-        return (fullNameLowerCase2Abbreviation.containsKey(nameKey)) || (isoLowerCase2Abbreviation.containsKey(nameKey))
-                || (medlineLowerCase2Abbreviation.containsKey(nameKey));
+        return abbreviations.stream().anyMatch(abbreviation -> isMatched(journalName.trim(), abbreviation));
     }
 
     public boolean isAbbreviatedName(String journalName) {
-        String nameKey = Objects.requireNonNull(journalName).trim().toLowerCase(Locale.ENGLISH);
-        return (isoLowerCase2Abbreviation.containsKey(nameKey)) || (medlineLowerCase2Abbreviation.containsKey(nameKey));
+        return abbreviations.stream().anyMatch(abbreviation -> isMatchedAbbreviated(journalName.trim(), abbreviation));
     }
 
     /**
@@ -54,75 +54,39 @@ public boolean isAbbreviatedName(String journalName) {
      * @return The abbreviated name
      */
     public Optional<Abbreviation> getAbbreviation(String journalName) {
-        String nameKey = Objects.requireNonNull(journalName).toLowerCase(Locale.ENGLISH).trim();
-
-        if (fullNameLowerCase2Abbreviation.containsKey(nameKey)) {
-            return Optional.of(fullNameLowerCase2Abbreviation.get(nameKey));
-        } else if (isoLowerCase2Abbreviation.containsKey(nameKey)) {
-            return Optional.of(isoLowerCase2Abbreviation.get(nameKey));
-        } else if (medlineLowerCase2Abbreviation.containsKey(nameKey)) {
-            return Optional.of(medlineLowerCase2Abbreviation.get(nameKey));
-        } else {
-            return Optional.empty();
-        }
+        return abbreviations.stream().filter(abbreviation -> isMatched(journalName.trim(), abbreviation)).findFirst();
     }
 
     public void addEntry(Abbreviation abbreviation) {
         Objects.requireNonNull(abbreviation);
 
-        if (isKnownName(abbreviation.getName())) {
+        if (abbreviations.contains(abbreviation)) {
             Abbreviation previous = getAbbreviation(abbreviation.getName()).get();
             abbreviations.remove(previous);
             LOGGER.info("Duplicate journal abbreviation - old one will be overwritten by new one\nOLD: "
                     + previous + "\nNEW: " + abbreviation);
         }
 
         abbreviations.add(abbreviation);
-
-        fullNameLowerCase2Abbreviation.put(abbreviation.getName().toLowerCase(Locale.ENGLISH), abbreviation);
-        isoLowerCase2Abbreviation.put(abbreviation.getIsoAbbreviation().toLowerCase(Locale.ENGLISH), abbreviation);
-        medlineLowerCase2Abbreviation.put(abbreviation.getMedlineAbbreviation().toLowerCase(Locale.ENGLISH),
-                abbreviation);
     }
 
-    public void addEntries(List<Abbreviation> abbreviationsToAdd) {
+    public void addEntries(Collection<Abbreviation> abbreviationsToAdd) {
         abbreviationsToAdd.forEach(this::addEntry);
     }
 
-    public SortedSet<Abbreviation> getAbbreviations() {
-        return Collections.unmodifiableSortedSet(abbreviations);
+    public Set<Abbreviation> getAbbreviations() {
+        return Collections.unmodifiableSet(abbreviations);
     }
 
     public Optional<String> getNextAbbreviation(String text) {
-        Optional<Abbreviation> abbreviation = getAbbreviation(text);
-
-        if (!abbreviation.isPresent()) {
-            return Optional.empty();
-        }
-
-        Abbreviation abbr = abbreviation.get();
-        return Optional.of(abbr.getNext(text));
+        return getAbbreviation(text).map(abbreviation -> abbreviation.getNext(text));
     }
 
     public Optional<String> getMedlineAbbreviation(String text) {
-        Optional<Abbreviation> abbreviation = getAbbreviation(text);
-
-        if (!abbreviation.isPresent()) {
-            return Optional.empty();
-        }
-
-        Abbreviation abbr = abbreviation.get();
-        return Optional.of(abbr.getMedlineAbbreviation());
+        return getAbbreviation(text).map(Abbreviation::getMedlineAbbreviation);
     }
 
     public Optional<String> getIsoAbbreviation(String text) {
-        Optional<Abbreviation> abbreviation = getAbbreviation(text);
-
-        if (!abbreviation.isPresent()) {
-            return Optional.empty();
-        }
-
-        Abbreviation abbr = abbreviation.get();
-        return Optional.of(abbr.getIsoAbbreviation());
+        return getAbbreviation(text).map(Abbreviation::getIsoAbbreviation);
     }
 }

src/test/java/org/jabref/gui/journals/ManageJournalAbbreviationsViewModelTest.java

@@ -430,7 +430,7 @@ public void testSaveAbbreviationsToFilesCreatesNewFilesWithWrittenAbbreviations(
 
         Assert.assertEquals(expected, actual);
 
-        expected = "Abbreviations = Abb" + NEWLINE + "Test Entry = TE" + NEWLINE + "MoreEntries = ME" + NEWLINE
+        expected = "EntryEntry = EE" + NEWLINE + "Abbreviations = Abb" + NEWLINE + "Test Entry = TE" + NEWLINE
                 + "SomeOtherEntry = SOE" + NEWLINE + "";
         actual = Files.contentOf(testFile5EntriesWithDuplicate.toFile(), StandardCharsets.UTF_8);
 

src/test/java/org/jabref/logic/journals/JournalAbbreviationRepositoryTest.java

@@ -40,15 +40,6 @@ public void oneElement() {
 
     }
 
-    @Test
-    public void testSorting() {
-        JournalAbbreviationRepository repository = new JournalAbbreviationRepository();
-        repository.addEntry(new Abbreviation("Long Name", "L. N."));
-        repository.addEntry(new Abbreviation("A Long Name", "AL. N."));
-        assertEquals("A Long Name", repository.getAbbreviations().first().getName());
-        assertEquals("Long Name", repository.getAbbreviations().last().getName());
-    }
-
     @Test
     public void testDuplicates() {
         JournalAbbreviationRepository repository = new JournalAbbreviationRepository();
@@ -63,9 +54,6 @@ public void testDuplicatesIsoOnly() {
         repository.addEntry(new Abbreviation("Old Long Name", "L. N."));
         repository.addEntry(new Abbreviation("New Long Name", "L. N."));
         assertEquals(2, repository.size());
-
-        assertEquals("L N", repository.getNextAbbreviation("L. N.").orElse("WRONG"));
-        assertEquals("New Long Name", repository.getNextAbbreviation("L N").orElse("WRONG"));
     }
 
     @Test