From 43a4393de955fb7debf0d64dd0f29720b6baeeb5 Mon Sep 17 00:00:00 2001 From: Christoph Date: Wed, 6 Dec 2023 22:13:43 +0100 Subject: [PATCH] Fix predatory journal loading resoures (#10693) * Fix resource does not exist * Fix journal loading and resource opening * Use MVStore directly (an experiment) * Add comments --------- Co-authored-by: Oliver Kopp --- .../java/org/jabref/gui/MainApplication.java | 10 +++ .../predatory/PredatoryJournalListLoader.java | 35 ++++++---- .../predatory/PredatoryJournalRepository.java | 22 +++--- .../logic/integrity/IntegrityCheckTest.java | 68 +++++++++++-------- .../PredatoryJournalCheckerTest.java | 10 ++- 5 files changed, 91 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/jabref/gui/MainApplication.java b/src/main/java/org/jabref/gui/MainApplication.java index a187843977d..76a33eb6f24 100644 --- a/src/main/java/org/jabref/gui/MainApplication.java +++ b/src/main/java/org/jabref/gui/MainApplication.java @@ -10,10 +10,15 @@ import org.jabref.model.util.FileUpdateMonitor; import org.jabref.preferences.JabRefPreferences; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * JabRef's main class to process command line options and to start the UI */ public class MainApplication extends Application { + private static final Logger LOGGER = LoggerFactory.getLogger(MainApplication.class); + private static List parserResults; private static boolean isBlank; private static JabRefPreferences preferences; @@ -43,5 +48,10 @@ public void stop() { OOBibBaseConnect.closeOfficeConnection(); Globals.stopBackgroundTasks(); Globals.shutdownThreadPools(); + try { + Globals.predatoryJournalRepository.close(); + } catch (Exception e) { + LOGGER.warn("Cloud not shut down predatoryJournalRepository", e); + } } } diff --git a/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalListLoader.java b/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalListLoader.java index 2b03f9dcb36..011f6e38fd8 100644 --- a/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalListLoader.java +++ b/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalListLoader.java @@ -1,7 +1,8 @@ package org.jabref.logic.journals.predatory; -import java.net.URISyntaxException; -import java.net.URL; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; import java.nio.file.Path; import org.slf4j.Logger; @@ -13,20 +14,24 @@ public class PredatoryJournalListLoader { public static PredatoryJournalRepository loadRepository() { PredatoryJournalRepository repository = new PredatoryJournalRepository(); - - Path path; - try { - URL resource = PredatoryJournalRepository.class.getResource("/journals/predatory-journals.mv"); - if (resource == null) { - LOGGER.error("predatoryJournal-list.mv not found. Using demo list."); - return new PredatoryJournalRepository(); + // Initialize with built-in list + // We cannot use PredatoryJournalRepository.class.getResource, because this is null in JPackage, thus we need "getResourceAsStream" + try (InputStream resourceAsStream = PredatoryJournalListLoader.class.getResourceAsStream("/journals/predatory-journals.mv")) { + if (resourceAsStream == null) { + LOGGER.warn("There is no predatory-journal.mv. We use a default predatory dummy list"); + repository = new PredatoryJournalRepository(); + } else { + // MVStore does not support loading from stream. Thus, we need to have a file copy of the stream. + Path tempDir = Files.createTempDirectory("jabref-journal"); + Path tempJournalList = tempDir.resolve("predatory-journals.mv"); + Files.copy(resourceAsStream, tempJournalList); + repository = new PredatoryJournalRepository(tempJournalList); + tempDir.toFile().deleteOnExit(); + tempJournalList.toFile().deleteOnExit(); } - path = Path.of(resource.toURI()); - } catch (URISyntaxException e) { - LOGGER.error("Could not determine path to predatoryJournal-list.mv. Using demo list."); - return new PredatoryJournalRepository(); + } catch (IOException e) { + LOGGER.error("Error while copying journal list", e); } - - return new PredatoryJournalRepository(path); + return repository; } } diff --git a/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalRepository.java b/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalRepository.java index 64b930026d2..2c73530daff 100644 --- a/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalRepository.java +++ b/src/main/java/org/jabref/logic/journals/predatory/PredatoryJournalRepository.java @@ -1,7 +1,6 @@ package org.jabref.logic.journals.predatory; import java.nio.file.Path; -import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.regex.Matcher; @@ -17,27 +16,27 @@ /** * A repository for all predatory journals and publishers, including add and find methods. */ -public class PredatoryJournalRepository { +public class PredatoryJournalRepository implements AutoCloseable { private final Logger LOGGER = LoggerFactory.getLogger(PredatoryJournalRepository.class); - private final Map predatoryJournals = new HashMap<>(); + private final Map predatoryJournals; private final StringSimilarity match = new StringSimilarity(); + private final MVStore store; /** * Initializes the internal data based on the predatory journals found in the given MV file */ public PredatoryJournalRepository(Path mvStore) { MVMap predatoryJournalsMap; - try (MVStore store = new MVStore.Builder().readOnly().fileName(mvStore.toAbsolutePath().toString()).open()) { - predatoryJournalsMap = store.openMap("PredatoryJournals"); - predatoryJournals.putAll(predatoryJournalsMap); - } + store = new MVStore.Builder().readOnly().fileName(mvStore.toAbsolutePath().toString()).open(); + predatoryJournals = store.openMap("PredatoryJournals"); } /** * Initializes the repository with demonstration data. Used if no abbreviation file is found. */ public PredatoryJournalRepository() { - predatoryJournals.put("Demo", new PredatoryJournalInformation("Demo", "Demo", "")); + store = null; + predatoryJournals = Map.of("Demo", new PredatoryJournalInformation("Demo", "Demo", "")); } /** @@ -58,4 +57,11 @@ public boolean isKnownName(String journalName) { LOGGER.info("Found multiple possible predatory journals {}", String.join(", ", matches)); return !matches.isEmpty(); } + + @Override + public void close() throws Exception { + if (store != null) { + store.close(); + } + } } diff --git a/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java b/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java index 21b2d0b2274..daaa371c3f7 100644 --- a/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java +++ b/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java @@ -1,6 +1,5 @@ package org.jabref.logic.integrity; -import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; @@ -14,6 +13,7 @@ import org.jabref.logic.citationkeypattern.GlobalCitationKeyPattern; import org.jabref.logic.journals.JournalAbbreviationLoader; import org.jabref.logic.journals.predatory.PredatoryJournalListLoader; +import org.jabref.logic.journals.predatory.PredatoryJournalRepository; import org.jabref.model.database.BibDatabase; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.database.BibDatabaseMode; @@ -47,28 +47,28 @@ class IntegrityCheckTest { @Test - void bibTexAcceptsStandardEntryType() { + void bibTexAcceptsStandardEntryType() throws Exception { assertCorrect(withMode(createContext(StandardField.TITLE, "sometitle", StandardEntryType.Article), BibDatabaseMode.BIBTEX)); } @Test - void bibTexDoesNotAcceptIEEETranEntryType() { + void bibTexDoesNotAcceptIEEETranEntryType() throws Exception { assertWrong(withMode(createContext(StandardField.TITLE, "sometitle", IEEETranEntryType.Patent), BibDatabaseMode.BIBTEX)); } @Test - void bibLaTexAcceptsIEEETranEntryType() { + void bibLaTexAcceptsIEEETranEntryType() throws Exception { assertCorrect((withMode(createContext(StandardField.TITLE, "sometitle", IEEETranEntryType.Patent), BibDatabaseMode.BIBLATEX))); } @Test - void bibLaTexAcceptsStandardEntryType() { + void bibLaTexAcceptsStandardEntryType() throws Exception { assertCorrect(withMode(createContext(StandardField.TITLE, "sometitle", StandardEntryType.Article), BibDatabaseMode.BIBLATEX)); } @ParameterizedTest @MethodSource("provideCorrectFormat") - void authorNameChecksCorrectFormat(String input) { + void authorNameChecksCorrectFormat(String input) throws Exception { for (Field field : FieldFactory.getPersonNameFields()) { assertCorrect(withMode(createContext(field, input), BibDatabaseMode.BIBLATEX)); } @@ -76,7 +76,7 @@ void authorNameChecksCorrectFormat(String input) { @ParameterizedTest @MethodSource("provideIncorrectFormat") - void authorNameChecksIncorrectFormat(String input) { + void authorNameChecksIncorrectFormat(String input) throws Exception { for (Field field : FieldFactory.getPersonNameFields()) { assertWrong(withMode(createContext(field, input), BibDatabaseMode.BIBLATEX)); } @@ -94,7 +94,7 @@ private static Stream provideIncorrectFormat() { } @Test - void testFileChecks() { + void testFileChecks() throws Exception { MetaData metaData = mock(MetaData.class); Mockito.when(metaData.getDefaultFileDirectory()).thenReturn(Optional.of(".")); Mockito.when(metaData.getUserFileDirectory(any(String.class))).thenReturn(Optional.empty()); @@ -107,7 +107,7 @@ void testFileChecks() { } @Test - void fileCheckFindsFilesRelativeToBibFile(@TempDir Path testFolder) throws IOException { + void fileCheckFindsFilesRelativeToBibFile(@TempDir Path testFolder) throws Exception { Path bibFile = testFolder.resolve("lit.bib"); Files.createFile(bibFile); Path pdfFile = testFolder.resolve("file.pdf"); @@ -120,7 +120,7 @@ void fileCheckFindsFilesRelativeToBibFile(@TempDir Path testFolder) throws IOExc } @Test - void testEntryIsUnchangedAfterChecks() { + void testEntryIsUnchangedAfterChecks() throws Exception { BibEntry entry = new BibEntry(); // populate with all known fields @@ -137,12 +137,14 @@ void testEntryIsUnchangedAfterChecks() { bibDatabase.insertEntry(entry); BibDatabaseContext context = new BibDatabaseContext(bibDatabase); - new IntegrityCheck(context, - mock(FilePreferences.class), - createCitationKeyPatternPreferences(), - JournalAbbreviationLoader.loadBuiltInRepository(), - PredatoryJournalListLoader.loadRepository(), false) - .check(); + try (PredatoryJournalRepository predatoryJournalRepository = PredatoryJournalListLoader.loadRepository()) { + new IntegrityCheck(context, + mock(FilePreferences.class), + createCitationKeyPatternPreferences(), + JournalAbbreviationLoader.loadBuiltInRepository(), + predatoryJournalRepository, false) + .check(); + } assertEquals(clonedEntry, entry); } @@ -169,25 +171,31 @@ private BibDatabaseContext createContext(Field field, String value) { return createContext(field, value, metaData); } - private void assertWrong(BibDatabaseContext context) { - List messages = new IntegrityCheck(context, - mock(FilePreferences.class), - createCitationKeyPatternPreferences(), - JournalAbbreviationLoader.loadBuiltInRepository(), - PredatoryJournalListLoader.loadRepository(), false) - .check(); + private void assertWrong(BibDatabaseContext context) throws Exception { + List messages; + try (PredatoryJournalRepository predatoryJournalRepository = PredatoryJournalListLoader.loadRepository()) { + messages = new IntegrityCheck(context, + mock(FilePreferences.class), + createCitationKeyPatternPreferences(), + JournalAbbreviationLoader.loadBuiltInRepository(), + predatoryJournalRepository, false) + .check(); + } assertNotEquals(Collections.emptyList(), messages); } - private void assertCorrect(BibDatabaseContext context) { + private void assertCorrect(BibDatabaseContext context) throws Exception { FilePreferences filePreferencesMock = mock(FilePreferences.class); when(filePreferencesMock.shouldStoreFilesRelativeToBibFile()).thenReturn(true); - List messages = new IntegrityCheck(context, - filePreferencesMock, - createCitationKeyPatternPreferences(), - JournalAbbreviationLoader.loadBuiltInRepository(), - PredatoryJournalListLoader.loadRepository(), false) - .check(); + List messages; + try (PredatoryJournalRepository predatoryJournalRepository = PredatoryJournalListLoader.loadRepository()) { + messages = new IntegrityCheck(context, + filePreferencesMock, + createCitationKeyPatternPreferences(), + JournalAbbreviationLoader.loadBuiltInRepository(), + predatoryJournalRepository, false) + .check(); + } assertEquals(Collections.emptyList(), messages); } diff --git a/src/test/java/org/jabref/logic/integrity/PredatoryJournalCheckerTest.java b/src/test/java/org/jabref/logic/integrity/PredatoryJournalCheckerTest.java index b56d997c252..74f9caa093f 100644 --- a/src/test/java/org/jabref/logic/integrity/PredatoryJournalCheckerTest.java +++ b/src/test/java/org/jabref/logic/integrity/PredatoryJournalCheckerTest.java @@ -4,9 +4,11 @@ import java.util.List; import org.jabref.logic.journals.predatory.PredatoryJournalListLoader; +import org.jabref.logic.journals.predatory.PredatoryJournalRepository; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.StandardField; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -15,13 +17,19 @@ class PredatoryJournalCheckerTest { static PredatoryJournalChecker checker; + static PredatoryJournalRepository predatoryJournalRepository = PredatoryJournalListLoader.loadRepository(); @BeforeAll static void initChecker() { - checker = new PredatoryJournalChecker(PredatoryJournalListLoader.loadRepository(), + checker = new PredatoryJournalChecker(predatoryJournalRepository, List.of(StandardField.JOURNAL, StandardField.PUBLISHER, StandardField.BOOKTITLE)); } + @AfterAll + static void close() throws Exception { + predatoryJournalRepository.close(); + } + @Test void journalIsNotPredatory() { BibEntry entry = new BibEntry().withField(StandardField.JOURNAL, "IEEE Software");