From e6fd725489af7b82b313ad71c548b9c9bc81cf2e Mon Sep 17 00:00:00 2001 From: Johannes Hupe Date: Fri, 18 Oct 2019 15:14:52 +0200 Subject: [PATCH 1/3] Refactor Webfetchers to return Sets instead of Lists This removes the dependency on JavaFx SortedList class --- .../org/jabref/cli/ArgumentProcessor.java | 3 +- .../fetcher/WebSearchPaneViewModel.java | 3 +- .../logic/importer/FulltextFetchers.java | 9 +- .../jabref/logic/importer/WebFetchers.java | 124 ++++++++---------- .../logic/importer/WebFetchersTest.java | 14 +- 5 files changed, 73 insertions(+), 80 deletions(-) diff --git a/src/main/java/org/jabref/cli/ArgumentProcessor.java b/src/main/java/org/jabref/cli/ArgumentProcessor.java index f5390978a82..252d8f8344e 100644 --- a/src/main/java/org/jabref/cli/ArgumentProcessor.java +++ b/src/main/java/org/jabref/cli/ArgumentProcessor.java @@ -9,6 +9,7 @@ import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.Set; import java.util.prefs.BackingStoreException; import org.jabref.Globals; @@ -533,7 +534,7 @@ private Optional fetch(String fetchCommand) { String engine = split[0]; String query = split[1]; - List fetchers = WebFetchers.getSearchBasedFetchers(Globals.prefs.getImportFormatPreferences()); + Set fetchers = WebFetchers.getSearchBasedFetchers(Globals.prefs.getImportFormatPreferences()); Optional selectedFetcher = fetchers.stream() .filter(fetcher -> fetcher.getName().equalsIgnoreCase(engine)) .findFirst(); diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index e2299b31414..ab322e11b5f 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -1,6 +1,7 @@ package org.jabref.gui.importer.fetcher; import java.util.List; +import java.util.SortedSet; import javafx.beans.property.ListProperty; import javafx.beans.property.ObjectProperty; @@ -38,7 +39,7 @@ public WebSearchPaneViewModel(ImportFormatPreferences importPreferences, JabRefF this.frame = frame; this.dialogService = dialogService; - List allFetchers = WebFetchers.getSearchBasedFetchers(importPreferences); + SortedSet allFetchers = WebFetchers.getSearchBasedFetchers(importPreferences); fetchers.setAll(allFetchers); // Choose last-selected fetcher as default diff --git a/src/main/java/org/jabref/logic/importer/FulltextFetchers.java b/src/main/java/org/jabref/logic/importer/FulltextFetchers.java index 4ba2cb69b63..52f2dcbd8d6 100644 --- a/src/main/java/org/jabref/logic/importer/FulltextFetchers.java +++ b/src/main/java/org/jabref/logic/importer/FulltextFetchers.java @@ -3,11 +3,12 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.util.ArrayList; import java.util.Comparator; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; @@ -34,7 +35,7 @@ public class FulltextFetchers { // Timeout in seconds private static final int FETCHER_TIMEOUT = 10; - private final List finders = new ArrayList<>(); + private final Set finders = new HashSet<>(); private final Predicate isPDF = url -> { try { @@ -49,7 +50,7 @@ public FulltextFetchers(ImportFormatPreferences importFormatPreferences) { this(WebFetchers.getFullTextFetchers(importFormatPreferences)); } - FulltextFetchers(List fetcher) { + FulltextFetchers(Set fetcher) { finders.addAll(fetcher); } @@ -108,7 +109,7 @@ private Callable> getCallable(BibEntry entry, FulltextFe }; } - private List>> getCallables(BibEntry entry, List fetchers) { + private List>> getCallables(BibEntry entry, Set fetchers) { return fetchers.stream() .map(f -> getCallable(entry, f)) .collect(Collectors.toList()); diff --git a/src/main/java/org/jabref/logic/importer/WebFetchers.java b/src/main/java/org/jabref/logic/importer/WebFetchers.java index 85f0feca7c3..66319f5e6b4 100644 --- a/src/main/java/org/jabref/logic/importer/WebFetchers.java +++ b/src/main/java/org/jabref/logic/importer/WebFetchers.java @@ -1,13 +1,11 @@ package org.jabref.logic.importer; -import java.util.ArrayList; import java.util.Comparator; -import java.util.List; +import java.util.HashSet; import java.util.Optional; - -import javafx.collections.FXCollections; -import javafx.collections.ObservableList; -import javafx.collections.transformation.SortedList; +import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import org.jabref.logic.importer.fetcher.ACMPortalFetcher; import org.jabref.logic.importer.fetcher.ACS; @@ -81,84 +79,76 @@ public static Optional> getIdFetcherForField(Fie } /** - * @return sorted list containing search based fetchers + * @return sorted set containing search based fetchers */ - public static SortedList getSearchBasedFetchers(ImportFormatPreferences importFormatPreferences) { - ArrayList list = new ArrayList<>(); - list.add(new ArXiv(importFormatPreferences)); - list.add(new INSPIREFetcher(importFormatPreferences)); - list.add(new GvkFetcher()); - list.add(new MedlineFetcher()); - list.add(new AstrophysicsDataSystem(importFormatPreferences)); - list.add(new MathSciNet(importFormatPreferences)); - list.add(new ZbMATH(importFormatPreferences)); - list.add(new ACMPortalFetcher(importFormatPreferences)); - list.add(new GoogleScholar(importFormatPreferences)); - list.add(new DBLPFetcher(importFormatPreferences)); - list.add(new SpringerFetcher()); - list.add(new CrossRef()); - list.add(new CiteSeer()); - list.add(new DOAJFetcher(importFormatPreferences)); - list.add(new IEEE(importFormatPreferences)); - - ObservableList observableList = FXCollections.observableList(list); - return new SortedList<>(observableList, Comparator.comparing(WebFetcher::getName)); + public static SortedSet getSearchBasedFetchers(ImportFormatPreferences importFormatPreferences) { + SortedSet set = new TreeSet<>(Comparator.comparing(WebFetcher::getName)); + set.add(new ArXiv(importFormatPreferences)); + set.add(new INSPIREFetcher(importFormatPreferences)); + set.add(new GvkFetcher()); + set.add(new MedlineFetcher()); + set.add(new AstrophysicsDataSystem(importFormatPreferences)); + set.add(new MathSciNet(importFormatPreferences)); + set.add(new ZbMATH(importFormatPreferences)); + set.add(new ACMPortalFetcher(importFormatPreferences)); + set.add(new GoogleScholar(importFormatPreferences)); + set.add(new DBLPFetcher(importFormatPreferences)); + set.add(new SpringerFetcher()); + set.add(new CrossRef()); + set.add(new CiteSeer()); + set.add(new DOAJFetcher(importFormatPreferences)); + set.add(new IEEE(importFormatPreferences)); + return set; } /** - * @return sorted list containing id based fetchers + * @return sorted set containing id based fetchers */ - public static SortedList getIdBasedFetchers(ImportFormatPreferences importFormatPreferences) { - ArrayList list = new ArrayList<>(); - list.add(new ArXiv(importFormatPreferences)); - list.add(new AstrophysicsDataSystem(importFormatPreferences)); - list.add(new IsbnFetcher(importFormatPreferences)); - list.add(new DiVA(importFormatPreferences)); - list.add(new DoiFetcher(importFormatPreferences)); - list.add(new MedlineFetcher()); - list.add(new TitleFetcher(importFormatPreferences)); - list.add(new MathSciNet(importFormatPreferences)); - list.add(new CrossRef()); - list.add(new LibraryOfCongress(importFormatPreferences)); - list.add(new IacrEprintFetcher(importFormatPreferences)); - list.add(new RfcFetcher(importFormatPreferences)); - - ObservableList observableList = FXCollections.observableList(list); - return new SortedList<>(observableList, Comparator.comparing(WebFetcher::getName)); + public static SortedSet getIdBasedFetchers(ImportFormatPreferences importFormatPreferences) { + SortedSet set = new TreeSet<>(Comparator.comparing(WebFetcher::getName)); + set.add(new ArXiv(importFormatPreferences)); + set.add(new AstrophysicsDataSystem(importFormatPreferences)); + set.add(new IsbnFetcher(importFormatPreferences)); + set.add(new DiVA(importFormatPreferences)); + set.add(new DoiFetcher(importFormatPreferences)); + set.add(new MedlineFetcher()); + set.add(new TitleFetcher(importFormatPreferences)); + set.add(new MathSciNet(importFormatPreferences)); + set.add(new CrossRef()); + set.add(new LibraryOfCongress(importFormatPreferences)); + set.add(new IacrEprintFetcher(importFormatPreferences)); + set.add(new RfcFetcher(importFormatPreferences)); + return set; } /** - * @return sorted list containing entry based fetchers + * @return sorted set containing entry based fetchers */ - public static SortedList getEntryBasedFetchers(ImportFormatPreferences importFormatPreferences) { - ArrayList list = new ArrayList<>(); - list.add(new AstrophysicsDataSystem(importFormatPreferences)); - list.add(new DoiFetcher(importFormatPreferences)); - list.add(new IsbnFetcher(importFormatPreferences)); - list.add(new MathSciNet(importFormatPreferences)); - list.add(new CrossRef()); - - ObservableList observableList = FXCollections.observableList(list); - return new SortedList<>(observableList, Comparator.comparing(WebFetcher::getName)); + public static SortedSet getEntryBasedFetchers(ImportFormatPreferences importFormatPreferences) { + SortedSet set = new TreeSet<>(Comparator.comparing(WebFetcher::getName)); + set.add(new AstrophysicsDataSystem(importFormatPreferences)); + set.add(new DoiFetcher(importFormatPreferences)); + set.add(new IsbnFetcher(importFormatPreferences)); + set.add(new MathSciNet(importFormatPreferences)); + set.add(new CrossRef()); + return set; } /** - * @return sorted list containing id fetchers + * @return sorted set containing id fetchers */ - public static SortedList getIdFetchers(ImportFormatPreferences importFormatPreferences) { - ArrayList list = new ArrayList<>(); - list.add(new CrossRef()); - list.add(new ArXiv(importFormatPreferences)); - - ObservableList observableList = FXCollections.observableList(list); - return new SortedList<>(observableList, Comparator.comparing(WebFetcher::getName)); + public static SortedSet getIdFetchers(ImportFormatPreferences importFormatPreferences) { + SortedSet set = new TreeSet<>(Comparator.comparing(WebFetcher::getName)); + set.add(new CrossRef()); + set.add(new ArXiv(importFormatPreferences)); + return set; } /** - * @return unsorted list containing fulltext fetchers + * @return set containing fulltext fetchers */ - public static List getFullTextFetchers(ImportFormatPreferences importFormatPreferences) { - List fetchers = new ArrayList<>(); + public static Set getFullTextFetchers(ImportFormatPreferences importFormatPreferences) { + Set fetchers = new HashSet<>(); // Original fetchers.add(new DoiResolution()); // Publishers diff --git a/src/test/java/org/jabref/logic/importer/WebFetchersTest.java b/src/test/java/org/jabref/logic/importer/WebFetchersTest.java index 58b2e3b15fd..b5d93a2be54 100644 --- a/src/test/java/org/jabref/logic/importer/WebFetchersTest.java +++ b/src/test/java/org/jabref/logic/importer/WebFetchersTest.java @@ -1,6 +1,6 @@ package org.jabref.logic.importer; -import java.util.List; +import java.util.Collection; import java.util.Set; import java.util.stream.Collectors; @@ -31,7 +31,7 @@ void setUp() throws Exception { @Test void getIdBasedFetchersReturnsAllFetcherDerivingFromIdBasedFetcher() throws Exception { - List idFetchers = WebFetchers.getIdBasedFetchers(importFormatPreferences); + Set idFetchers = WebFetchers.getIdBasedFetchers(importFormatPreferences); try (ScanResult scanResult = classGraph.scan()) { @@ -50,7 +50,7 @@ void getIdBasedFetchersReturnsAllFetcherDerivingFromIdBasedFetcher() throws Exce @Test void getEntryBasedFetchersReturnsAllFetcherDerivingFromEntryBasedFetcher() throws Exception { - List idFetchers = WebFetchers.getEntryBasedFetchers(importFormatPreferences); + Set idFetchers = WebFetchers.getEntryBasedFetchers(importFormatPreferences); try (ScanResult scanResult = classGraph.scan()) { ClassInfoList controlClasses = scanResult.getClassesImplementing(EntryBasedFetcher.class.getCanonicalName()); @@ -64,7 +64,7 @@ void getEntryBasedFetchersReturnsAllFetcherDerivingFromEntryBasedFetcher() throw @Test void getSearchBasedFetchersReturnsAllFetcherDerivingFromSearchBasedFetcher() throws Exception { - List searchBasedFetchers = WebFetchers.getSearchBasedFetchers(importFormatPreferences); + Set searchBasedFetchers = WebFetchers.getSearchBasedFetchers(importFormatPreferences); try (ScanResult scanResult = classGraph.scan()) { ClassInfoList controlClasses = scanResult.getClassesImplementing(SearchBasedFetcher.class.getCanonicalName()); Set> expected = controlClasses.loadClasses().stream().collect(Collectors.toSet()); @@ -76,7 +76,7 @@ void getSearchBasedFetchersReturnsAllFetcherDerivingFromSearchBasedFetcher() thr @Test void getFullTextFetchersReturnsAllFetcherDerivingFromFullTextFetcher() throws Exception { - List fullTextFetchers = WebFetchers.getFullTextFetchers(importFormatPreferences); + Set fullTextFetchers = WebFetchers.getFullTextFetchers(importFormatPreferences); try (ScanResult scanResult = classGraph.scan()) { ClassInfoList controlClasses = scanResult.getClassesImplementing(FulltextFetcher.class.getCanonicalName()); @@ -87,7 +87,7 @@ void getFullTextFetchersReturnsAllFetcherDerivingFromFullTextFetcher() throws Ex @Test void getIdFetchersReturnsAllFetcherDerivingFromIdFetcher() throws Exception { - List idFetchers = WebFetchers.getIdFetchers(importFormatPreferences); + Set idFetchers = WebFetchers.getIdFetchers(importFormatPreferences); try (ScanResult scanResult = classGraph.scan()) { ClassInfoList controlClasses = scanResult.getClassesImplementing(IdFetcher.class.getCanonicalName()); @@ -98,7 +98,7 @@ void getIdFetchersReturnsAllFetcherDerivingFromIdFetcher() throws Exception { } } - private Set> getClasses(List objects) { + private Set> getClasses(Collection objects) { return objects.stream().map(Object::getClass).collect(Collectors.toSet()); } } From 9591d3a003c8f4d6f86af2656aa45b5575632175 Mon Sep 17 00:00:00 2001 From: Johannes Hupe Date: Fri, 18 Oct 2019 15:25:07 +0200 Subject: [PATCH 2/3] Refactor Webfetchers to return Sets instead of Lists This removes the dependency on JavaFx SortedList class --- .../org/jabref/logic/importer/FulltextFetchersTest.java | 9 +++++---- .../logic/importer/fetcher/FulltextFetcherTest.java | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java b/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java index 1f3cc1a5f6d..7854b2dfa74 100644 --- a/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java +++ b/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java @@ -4,6 +4,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.util.Arrays; +import java.util.HashSet; import java.util.Optional; import org.jabref.logic.importer.fetcher.TrustLevel; @@ -34,7 +35,7 @@ public void tearDown() { public void acceptPdfUrls() throws MalformedURLException { URL pdfUrl = new URL("http://docs.oasis-open.org/wsbpel/2.0/OS/wsbpel-v2.0-OS.pdf"); FulltextFetcher finder = (e) -> Optional.of(pdfUrl); - FulltextFetchers fetcher = new FulltextFetchers(Arrays.asList(finder)); + FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finder))); assertEquals(Optional.of(pdfUrl), fetcher.findFullTextPDF(entry)); } @@ -43,7 +44,7 @@ public void acceptPdfUrls() throws MalformedURLException { public void rejectNonPdfUrls() throws MalformedURLException { URL pdfUrl = new URL("https://github.com/JabRef/jabref/blob/master/README.md"); FulltextFetcher finder = (e) -> Optional.of(pdfUrl); - FulltextFetchers fetcher = new FulltextFetchers(Arrays.asList(finder)); + FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finder))); assertEquals(Optional.empty(), fetcher.findFullTextPDF(entry)); } @@ -52,7 +53,7 @@ public void rejectNonPdfUrls() throws MalformedURLException { public void noTrustLevel() throws MalformedURLException { URL pdfUrl = new URL("http://docs.oasis-open.org/wsbpel/2.0/OS/wsbpel-v2.0-OS.pdf"); FulltextFetcher finder = (e) -> Optional.of(pdfUrl); - FulltextFetchers fetcher = new FulltextFetchers(Arrays.asList(finder)); + FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finder))); assertEquals(Optional.of(pdfUrl), fetcher.findFullTextPDF(entry)); } @@ -69,7 +70,7 @@ public void higherTrustLevelWins() throws MalformedURLException, IOException, Fe when(finderHigh.findFullText(entry)).thenReturn(Optional.of(highUrl)); when(finderLow.findFullText(entry)).thenReturn(Optional.of(lowUrl)); - FulltextFetchers fetcher = new FulltextFetchers(Arrays.asList(finderLow, finderHigh)); + FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finderLow, finderHigh))); assertEquals(Optional.of(highUrl), fetcher.findFullTextPDF(entry)); } diff --git a/src/test/java/org/jabref/logic/importer/fetcher/FulltextFetcherTest.java b/src/test/java/org/jabref/logic/importer/fetcher/FulltextFetcherTest.java index 20e70601800..183e011a84b 100644 --- a/src/test/java/org/jabref/logic/importer/fetcher/FulltextFetcherTest.java +++ b/src/test/java/org/jabref/logic/importer/fetcher/FulltextFetcherTest.java @@ -1,7 +1,7 @@ package org.jabref.logic.importer.fetcher; -import java.util.List; import java.util.Optional; +import java.util.Set; import org.jabref.logic.importer.FulltextFetcher; import org.jabref.logic.importer.ImportFormatPreferences; @@ -18,7 +18,7 @@ class FulltextFetcherTest { @SuppressWarnings("unused") - private static List fetcherProvider() { + private static Set fetcherProvider() { return WebFetchers.getFullTextFetchers(mock(ImportFormatPreferences.class)); } From 7b2494b9cc39b795eda44b26c0587842ef4c4fbf Mon Sep 17 00:00:00 2001 From: Johannes Hupe Date: Fri, 18 Oct 2019 15:46:03 +0200 Subject: [PATCH 3/3] Removes nested constructor call Set.of is used instead of new HashSet --- .../jabref/logic/importer/FulltextFetchersTest.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java b/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java index 7854b2dfa74..986165ad0ee 100644 --- a/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java +++ b/src/test/java/org/jabref/logic/importer/FulltextFetchersTest.java @@ -3,9 +3,8 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.util.Arrays; -import java.util.HashSet; import java.util.Optional; +import java.util.Set; import org.jabref.logic.importer.fetcher.TrustLevel; import org.jabref.model.entry.BibEntry; @@ -35,8 +34,7 @@ public void tearDown() { public void acceptPdfUrls() throws MalformedURLException { URL pdfUrl = new URL("http://docs.oasis-open.org/wsbpel/2.0/OS/wsbpel-v2.0-OS.pdf"); FulltextFetcher finder = (e) -> Optional.of(pdfUrl); - FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finder))); - + FulltextFetchers fetcher = new FulltextFetchers(Set.of(finder)); assertEquals(Optional.of(pdfUrl), fetcher.findFullTextPDF(entry)); } @@ -44,7 +42,7 @@ public void acceptPdfUrls() throws MalformedURLException { public void rejectNonPdfUrls() throws MalformedURLException { URL pdfUrl = new URL("https://github.com/JabRef/jabref/blob/master/README.md"); FulltextFetcher finder = (e) -> Optional.of(pdfUrl); - FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finder))); + FulltextFetchers fetcher = new FulltextFetchers(Set.of(finder)); assertEquals(Optional.empty(), fetcher.findFullTextPDF(entry)); } @@ -53,7 +51,7 @@ public void rejectNonPdfUrls() throws MalformedURLException { public void noTrustLevel() throws MalformedURLException { URL pdfUrl = new URL("http://docs.oasis-open.org/wsbpel/2.0/OS/wsbpel-v2.0-OS.pdf"); FulltextFetcher finder = (e) -> Optional.of(pdfUrl); - FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finder))); + FulltextFetchers fetcher = new FulltextFetchers(Set.of(finder)); assertEquals(Optional.of(pdfUrl), fetcher.findFullTextPDF(entry)); } @@ -70,7 +68,7 @@ public void higherTrustLevelWins() throws MalformedURLException, IOException, Fe when(finderHigh.findFullText(entry)).thenReturn(Optional.of(highUrl)); when(finderLow.findFullText(entry)).thenReturn(Optional.of(lowUrl)); - FulltextFetchers fetcher = new FulltextFetchers(new HashSet<>(Arrays.asList(finderLow, finderHigh))); + FulltextFetchers fetcher = new FulltextFetchers(Set.of(finderLow, finderHigh)); assertEquals(Optional.of(highUrl), fetcher.findFullTextPDF(entry)); }