From 3785cb16c88e49808cbd41a58a45c178c804bed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Thu, 19 Sep 2024 23:07:33 +0200 Subject: [PATCH 01/35] Refactor Citations Relations Tab (#11189) * Move repository, cache, and fetcher to logic package * Move citations model to model/citations/semanticscholar package --- .../CitationRelationsTab.java | 14 +- .../CitationsRelationsTabViewModel.java | 2 +- .../repository}/BibEntryRelationsCache.java | 6 +- .../BibEntryRelationsRepository.java | 12 +- .../importer/fetcher}/CitationFetcher.java | 2 +- .../SemanticScholarCitationFetcher.java} | 9 +- .../semanticscholar/AuthorResponse.java | 2 +- .../semanticscholar/CitationDataItem.java | 2 +- .../semanticscholar/CitationsResponse.java | 2 +- .../semanticscholar/PaperDetails.java | 2 +- .../semanticscholar/ReferenceDataItem.java | 2 +- .../semanticscholar/ReferencesResponse.java | 2 +- .../BibEntryRelationsRepositoryTest.java | 61 --------- .../CitationsRelationsTabViewModelTest.java | 4 +- .../BibEntryRelationsRepositoryTest.java | 128 ++++++++++++++++++ 15 files changed, 159 insertions(+), 91 deletions(-) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => logic/citation/repository}/BibEntryRelationsCache.java (96%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => logic/citation/repository}/BibEntryRelationsRepository.java (82%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab/semanticscholar => logic/importer/fetcher}/CitationFetcher.java (94%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab/semanticscholar/SemanticScholarFetcher.java => logic/importer/fetcher/SemanticScholarCitationFetcher.java} (90%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => model/citation}/semanticscholar/AuthorResponse.java (84%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => model/citation}/semanticscholar/CitationDataItem.java (79%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => model/citation}/semanticscholar/CitationsResponse.java (89%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => model/citation}/semanticscholar/PaperDetails.java (98%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => model/citation}/semanticscholar/ReferenceDataItem.java (70%) rename src/main/java/org/jabref/{gui/entryeditor/citationrelationtab => model/citation}/semanticscholar/ReferencesResponse.java (89%) delete mode 100644 src/test/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepositoryTest.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index f6f8e9ca111..048d058462d 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -30,8 +30,10 @@ import org.jabref.gui.StateManager; import org.jabref.gui.desktop.os.NativeDesktop; import org.jabref.gui.entryeditor.EntryEditorTab; -import org.jabref.gui.entryeditor.citationrelationtab.semanticscholar.CitationFetcher; -import org.jabref.gui.entryeditor.citationrelationtab.semanticscholar.SemanticScholarFetcher; +import org.jabref.logic.citation.repository.BibEntryRelationsCache; +import org.jabref.logic.citation.repository.BibEntryRelationsRepository; +import org.jabref.logic.importer.fetcher.CitationFetcher; +import org.jabref.logic.importer.fetcher.SemanticScholarCitationFetcher; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.gui.util.NoSelectionModel; @@ -92,8 +94,10 @@ public CitationRelationsTab(DialogService dialogService, setTooltip(new Tooltip(Localization.lang("Show articles related by citation"))); this.duplicateCheck = new DuplicateCheck(new BibEntryTypesManager()); - this.bibEntryRelationsRepository = new BibEntryRelationsRepository(new SemanticScholarFetcher(preferences.getImporterPreferences()), - new BibEntryRelationsCache()); + this.bibEntryRelationsRepository = new BibEntryRelationsRepository( + new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), + new BibEntryRelationsCache() + ); citationsRelationsTabViewModel = new CitationsRelationsTabViewModel(databaseContext, preferences, undoManager, stateManager, dialogService, fileUpdateMonitor, taskExecutor); } @@ -394,7 +398,7 @@ private void onSearchForRelationsSucceed(BibEntry entry, CheckListView new CitationRelationItem(entr, localEntry, true)) .orElseGet(() -> new CitationRelationItem(entr, false))) .toList() - ); + ); if (!observableList.isEmpty()) { listView.refresh(); diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java index 7ae52965b8f..0d0d5646ff1 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java @@ -8,7 +8,7 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; -import org.jabref.gui.entryeditor.citationrelationtab.semanticscholar.CitationFetcher; +import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.gui.externalfiles.ImportHandler; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.logic.citationkeypattern.CitationKeyGenerator; diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsCache.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsCache.java similarity index 96% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsCache.java rename to src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsCache.java index 55888aa660f..fbccdef495f 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsCache.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsCache.java @@ -1,14 +1,12 @@ -package org.jabref.gui.entryeditor.citationrelationtab; +package org.jabref.logic.citation.repository; import java.util.Collections; import java.util.List; import java.util.Map; - +import org.eclipse.jgit.util.LRUMap; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.identifier.DOI; -import org.eclipse.jgit.util.LRUMap; - public class BibEntryRelationsCache { private static final Integer MAX_CACHED_ENTRIES = 100; private static final Map> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java similarity index 82% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepository.java rename to src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java index f7f29052da2..8c4ff85b36a 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java @@ -1,8 +1,8 @@ -package org.jabref.gui.entryeditor.citationrelationtab; +package org.jabref.logic.citation.repository; import java.util.List; -import org.jabref.gui.entryeditor.citationrelationtab.semanticscholar.SemanticScholarFetcher; +import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.logic.importer.FetcherException; import org.jabref.model.entry.BibEntry; @@ -12,10 +12,10 @@ public class BibEntryRelationsRepository { private static final Logger LOGGER = LoggerFactory.getLogger(BibEntryRelationsRepository.class); - private final SemanticScholarFetcher fetcher; + private final CitationFetcher fetcher; private final BibEntryRelationsCache cache; - public BibEntryRelationsRepository(SemanticScholarFetcher fetcher, BibEntryRelationsCache cache) { + public BibEntryRelationsRepository(CitationFetcher fetcher, BibEntryRelationsCache cache) { this.fetcher = fetcher; this.cache = cache; } @@ -52,11 +52,11 @@ public void forceRefreshCitations(BibEntry entry) { } } - public boolean needToRefreshCitations(BibEntry entry) { + private boolean needToRefreshCitations(BibEntry entry) { return !cache.citationsCached(entry); } - public boolean needToRefreshReferences(BibEntry entry) { + private boolean needToRefreshReferences(BibEntry entry) { return !cache.referencesCached(entry); } diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/CitationFetcher.java similarity index 94% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationFetcher.java rename to src/main/java/org/jabref/logic/importer/fetcher/CitationFetcher.java index 1b87c7ab0bb..58c4f32d080 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationFetcher.java +++ b/src/main/java/org/jabref/logic/importer/fetcher/CitationFetcher.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.logic.importer.fetcher; import java.util.List; diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/SemanticScholarFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java similarity index 90% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/SemanticScholarFetcher.java rename to src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java index 557a135741e..51a4762432d 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/SemanticScholarFetcher.java +++ b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.logic.importer.fetcher; import java.net.MalformedURLException; import java.net.URI; @@ -7,21 +7,22 @@ import org.jabref.logic.importer.FetcherException; import org.jabref.logic.importer.ImporterPreferences; -import org.jabref.logic.importer.fetcher.CustomizableKeyFetcher; import org.jabref.logic.net.URLDownload; import org.jabref.logic.util.BuildInfo; import org.jabref.model.entry.BibEntry; import com.google.gson.Gson; +import org.jabref.model.citation.semanticscholar.CitationsResponse; +import org.jabref.model.citation.semanticscholar.ReferencesResponse; -public class SemanticScholarFetcher implements CitationFetcher, CustomizableKeyFetcher { +public class SemanticScholarCitationFetcher implements CitationFetcher, CustomizableKeyFetcher { private static final String SEMANTIC_SCHOLAR_API = "https://api.semanticscholar.org/graph/v1/"; private static final String API_KEY = new BuildInfo().semanticScholarApiKey; private final ImporterPreferences importerPreferences; - public SemanticScholarFetcher(ImporterPreferences importerPreferences) { + public SemanticScholarCitationFetcher(ImporterPreferences importerPreferences) { this.importerPreferences = importerPreferences; } diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/AuthorResponse.java b/src/main/java/org/jabref/model/citation/semanticscholar/AuthorResponse.java similarity index 84% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/AuthorResponse.java rename to src/main/java/org/jabref/model/citation/semanticscholar/AuthorResponse.java index 539b99cc39d..8489099a4fb 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/AuthorResponse.java +++ b/src/main/java/org/jabref/model/citation/semanticscholar/AuthorResponse.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.model.citation.semanticscholar; /** * Used for GSON diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationDataItem.java b/src/main/java/org/jabref/model/citation/semanticscholar/CitationDataItem.java similarity index 79% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationDataItem.java rename to src/main/java/org/jabref/model/citation/semanticscholar/CitationDataItem.java index 684285b46df..8f9d44535e9 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationDataItem.java +++ b/src/main/java/org/jabref/model/citation/semanticscholar/CitationDataItem.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.model.citation.semanticscholar; /** * Used for GSON diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationsResponse.java b/src/main/java/org/jabref/model/citation/semanticscholar/CitationsResponse.java similarity index 89% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationsResponse.java rename to src/main/java/org/jabref/model/citation/semanticscholar/CitationsResponse.java index 999eb7eca2a..8fdbec26948 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/CitationsResponse.java +++ b/src/main/java/org/jabref/model/citation/semanticscholar/CitationsResponse.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.model.citation.semanticscholar; import java.util.List; diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/PaperDetails.java b/src/main/java/org/jabref/model/citation/semanticscholar/PaperDetails.java similarity index 98% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/PaperDetails.java rename to src/main/java/org/jabref/model/citation/semanticscholar/PaperDetails.java index 58ba269616e..073e15f384d 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/PaperDetails.java +++ b/src/main/java/org/jabref/model/citation/semanticscholar/PaperDetails.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.model.citation.semanticscholar; import java.util.List; import java.util.Map; diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/ReferenceDataItem.java b/src/main/java/org/jabref/model/citation/semanticscholar/ReferenceDataItem.java similarity index 70% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/ReferenceDataItem.java rename to src/main/java/org/jabref/model/citation/semanticscholar/ReferenceDataItem.java index b9c53c355e9..ccbb170355c 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/ReferenceDataItem.java +++ b/src/main/java/org/jabref/model/citation/semanticscholar/ReferenceDataItem.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.model.citation.semanticscholar; /** * Used for GSON diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/ReferencesResponse.java b/src/main/java/org/jabref/model/citation/semanticscholar/ReferencesResponse.java similarity index 89% rename from src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/ReferencesResponse.java rename to src/main/java/org/jabref/model/citation/semanticscholar/ReferencesResponse.java index 0a6ac34af07..a0f9c6426a3 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/semanticscholar/ReferencesResponse.java +++ b/src/main/java/org/jabref/model/citation/semanticscholar/ReferencesResponse.java @@ -1,4 +1,4 @@ -package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar; +package org.jabref.model.citation.semanticscholar; import java.util.List; diff --git a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepositoryTest.java deleted file mode 100644 index 41106d57a6b..00000000000 --- a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/BibEntryRelationsRepositoryTest.java +++ /dev/null @@ -1,61 +0,0 @@ -package org.jabref.gui.entryeditor.citationrelationtab; - -import java.util.List; - -import org.jabref.gui.entryeditor.citationrelationtab.semanticscholar.SemanticScholarFetcher; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.field.StandardField; - -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -class BibEntryRelationsRepositoryTest { - - private List getCitedBy(BibEntry entry) { - return List.of(createCitingBibEntry(entry)); - } - - private BibEntry createBibEntry(int i) { - return new BibEntry() - .withCitationKey("entry" + i) - .withField(StandardField.DOI, "10.1234/5678" + i); - } - - private BibEntry createCitingBibEntry(Integer i) { - return new BibEntry() - .withCitationKey("citing_entry" + i) - .withField(StandardField.DOI, "10.2345/6789" + i); - } - - private BibEntry createCitingBibEntry(BibEntry citedEntry) { - return createCitingBibEntry(Integer.valueOf(citedEntry.getCitationKey().get().substring(5))); - } - - @Test - void getCitations() throws Exception { - SemanticScholarFetcher semanticScholarFetcher = mock(SemanticScholarFetcher.class); - when(semanticScholarFetcher.searchCitedBy(any(BibEntry.class))).thenAnswer(invocation -> { - BibEntry entry = invocation.getArgument(0); - return getCitedBy(entry); - }); - BibEntryRelationsCache bibEntryRelationsCache = new BibEntryRelationsCache(); - - BibEntryRelationsRepository bibEntryRelationsRepository = new BibEntryRelationsRepository(semanticScholarFetcher, bibEntryRelationsCache); - - for (int i = 0; i < 150; i++) { - BibEntry entry = createBibEntry(i); - List citations = bibEntryRelationsRepository.getCitations(entry); - assertEquals(getCitedBy(entry), citations); - } - - for (int i = 0; i < 150; i++) { - BibEntry entry = createBibEntry(i); - List citations = bibEntryRelationsRepository.getCitations(entry); - assertEquals(getCitedBy(entry), citations); - } - } -} diff --git a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java index 9df5f2d2aaa..48e074fd00c 100644 --- a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java +++ b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java @@ -9,7 +9,7 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; -import org.jabref.gui.entryeditor.citationrelationtab.semanticscholar.CitationFetcher; +import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.gui.externalfiles.ImportHandler; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.logic.FilePreferences; @@ -41,9 +41,7 @@ import static org.mockito.Mockito.when; class CitationsRelationsTabViewModelTest { - private ImportHandler importHandler; private BibDatabaseContext bibDatabaseContext; - private BibEntry testEntry; @Mock private GuiPreferences preferences; diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java new file mode 100644 index 00000000000..f17f6b8a99d --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java @@ -0,0 +1,128 @@ +package org.jabref.logic.citation.repository; + +import java.util.HashSet; +import java.util.List; + +import java.util.function.Function; +import java.util.stream.IntStream; +import java.util.stream.Stream; +import org.jabref.logic.importer.FetcherException; +import org.jabref.logic.importer.fetcher.CitationFetcher; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +class BibEntryRelationsRepositoryTest { + + private static Stream createBibEntries() { + return IntStream + .range(0, 150) + .mapToObj(BibEntryRelationsRepositoryTest::createBibEntry); + } + + private static List getCitedBy(BibEntry entry) { + return List.of(BibEntryRelationsRepositoryTest.createCitingBibEntry(entry)); + } + + private static BibEntry createBibEntry(int i) { + return new BibEntry() + .withCitationKey("entry" + i) + .withField(StandardField.DOI, "10.1234/5678" + i); + } + + private static BibEntry createCitingBibEntry(Integer i) { + return new BibEntry() + .withCitationKey("citing_entry" + i) + .withField(StandardField.DOI, "10.2345/6789" + i); + } + + private static BibEntry createCitingBibEntry(BibEntry citedEntry) { + return createCitingBibEntry( + Integer.valueOf(citedEntry.getCitationKey().orElseThrow().substring(5)) + ); + } + + /** + * Simple mock to avoid using Mockito (reduce overall complexity) + */ + private record CitationFetcherMock( + Function> searchCiteByDelegate, + Function> searchCitingDelegate, + String name + ) implements CitationFetcher { + + @Override + public List searchCitedBy(BibEntry entry) throws FetcherException { + return this.searchCiteByDelegate.apply(entry); + } + + @Override + public List searchCiting(BibEntry entry) throws FetcherException { + return this.searchCitingDelegate.apply(entry); + } + + @Override + public String getName() { + return this.name; + } + } + + @ParameterizedTest + @MethodSource("createBibEntries") + @DisplayName( + "Given a new bib entry when reading citations for it should call the fetcher" + ) + void givenANewEntryWhenReadingCitationsForItShouldCallTheFetcher(BibEntry bibEntry) { + // GIVEN + var entryCaptor = new HashSet(); + var citationFetcherMock = new CitationFetcherMock( + entry -> { + entryCaptor.add(entry); + return BibEntryRelationsRepositoryTest.getCitedBy(entry); + }, + null, + null + ); + var bibEntryRelationsCache = new BibEntryRelationsCache(); + var bibEntryRelationsRepository = new BibEntryRelationsRepository( + citationFetcherMock, bibEntryRelationsCache + ); + + // WHEN + var citations = bibEntryRelationsRepository.getCitations(bibEntry); + + // THEN + Assertions.assertFalse(citations.isEmpty()); + Assertions.assertTrue(entryCaptor.contains(bibEntry)); + } + + @Test + @DisplayName( + "Given an empty cache for a valid entry when reading the citations should populate cache" + ) + void givenAnEmptyCacheAndAValidBibEntryWhenReadingCitationsShouldPopulateTheCache() { + // GIVEN + var citationFetcherMock = new CitationFetcherMock( + BibEntryRelationsRepositoryTest::getCitedBy, null, null + ); + var bibEntryRelationsCache = new BibEntryRelationsCache(); + var bibEntryRelationsRepository = new BibEntryRelationsRepository( + citationFetcherMock, bibEntryRelationsCache + ); + var bibEntry = BibEntryRelationsRepositoryTest.createBibEntry(1); + + // WHEN + Assertions.assertEquals(List.of(), bibEntryRelationsCache.getCitations(bibEntry)); + var citations = bibEntryRelationsRepository.getCitations(bibEntry); + var fromCacheCitations = bibEntryRelationsCache.getCitations(bibEntry); + + // THEN + Assertions.assertFalse(fromCacheCitations.isEmpty()); + Assertions.assertEquals(citations, fromCacheCitations); + } +} From 8048da804d5b7905f5ae235128fec04ea3af23ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Tue, 24 Sep 2024 23:37:01 +0200 Subject: [PATCH 02/35] Refactor Citations Relations Tab (#11189) * Introduce service layer * Rename LRU cache implementation * Add tests helpers for repository --- .../BibEntryRelationsRepository.java | 87 +++++------------- ...he.java => LRUBibEntryRelationsCache.java} | 2 +- .../LRUBibEntryRelationsRepository.java | 78 ++++++++++++++++ .../SearchCitationsRelationsService.java | 36 ++++++++ .../BibEntryRelationsRepositoryTest.java | 12 +-- ...ibEntryRelationsRepositoryTestHelpers.java | 39 ++++++++ .../SearchCitationsRelationsServiceTest.java | 89 +++++++++++++++++++ 7 files changed, 269 insertions(+), 74 deletions(-) rename src/main/java/org/jabref/logic/citation/repository/{BibEntryRelationsCache.java => LRUBibEntryRelationsCache.java} (97%) create mode 100644 src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java create mode 100644 src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java create mode 100644 src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java diff --git a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java index 8c4ff85b36a..48f38e2a38d 100644 --- a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java @@ -1,73 +1,26 @@ package org.jabref.logic.citation.repository; import java.util.List; - -import org.jabref.logic.importer.fetcher.CitationFetcher; -import org.jabref.logic.importer.FetcherException; import org.jabref.model.entry.BibEntry; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class BibEntryRelationsRepository { - private static final Logger LOGGER = LoggerFactory.getLogger(BibEntryRelationsRepository.class); - - private final CitationFetcher fetcher; - private final BibEntryRelationsCache cache; - - public BibEntryRelationsRepository(CitationFetcher fetcher, BibEntryRelationsCache cache) { - this.fetcher = fetcher; - this.cache = cache; - } - - public List getCitations(BibEntry entry) { - if (needToRefreshCitations(entry)) { - forceRefreshCitations(entry); - } - - return cache.getCitations(entry); - } - - public List getReferences(BibEntry entry) { - if (needToRefreshReferences(entry)) { - List references; - try { - references = fetcher.searchCiting(entry); - } catch (FetcherException e) { - LOGGER.error("Error while fetching references", e); - references = List.of(); - } - cache.cacheOrMergeReferences(entry, references); - } - - return cache.getReferences(entry); - } - - public void forceRefreshCitations(BibEntry entry) { - try { - List citations = fetcher.searchCitedBy(entry); - cache.cacheOrMergeCitations(entry, citations); - } catch (FetcherException e) { - LOGGER.error("Error while fetching citations", e); - } - } - - private boolean needToRefreshCitations(BibEntry entry) { - return !cache.citationsCached(entry); - } - - private boolean needToRefreshReferences(BibEntry entry) { - return !cache.referencesCached(entry); - } - - public void forceRefreshReferences(BibEntry entry) { - List references; - try { - references = fetcher.searchCiting(entry); - } catch (FetcherException e) { - LOGGER.error("Error while fetching references", e); - references = List.of(); - } - cache.cacheOrMergeReferences(entry, references); - } +public interface BibEntryRelationsRepository { + List readCitations(BibEntry entry); + + List readReferences(BibEntry entry); + + /** + * Fetch citations for a bib entry and update local database. + * @param entry should not be null + * @deprecated fetching citations should be done by the service layer (calling code) + */ + @Deprecated + void forceRefreshCitations(BibEntry entry); + + /** + * Fetch references made by a bib entry and update local database. + * @param entry should not be null + * @deprecated fetching references should be done by the service layer (calling code) + */ + @Deprecated + void forceRefreshReferences(BibEntry entry); } diff --git a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsCache.java b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java similarity index 97% rename from src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsCache.java rename to src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java index fbccdef495f..8e6d491ce75 100644 --- a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsCache.java +++ b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java @@ -7,7 +7,7 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.identifier.DOI; -public class BibEntryRelationsCache { +public class LRUBibEntryRelationsCache { private static final Integer MAX_CACHED_ENTRIES = 100; private static final Map> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); private static final Map> REFERENCES_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java new file mode 100644 index 00000000000..23ab083ee4b --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java @@ -0,0 +1,78 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; + +import org.jabref.logic.importer.fetcher.CitationFetcher; +import org.jabref.logic.importer.FetcherException; +import org.jabref.model.entry.BibEntry; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class LRUBibEntryRelationsRepository implements BibEntryRelationsRepository { + private static final Logger LOGGER = LoggerFactory + .getLogger(LRUBibEntryRelationsRepository.class); + + private final CitationFetcher fetcher; + private final LRUBibEntryRelationsCache cache; + + public LRUBibEntryRelationsRepository(CitationFetcher fetcher, LRUBibEntryRelationsCache cache) { + this.fetcher = fetcher; + this.cache = cache; + } + + @Override + public List readCitations(BibEntry entry) { + if (needToRefreshCitations(entry)) { + forceRefreshCitations(entry); + } + + return cache.getCitations(entry); + } + + @Override + public List readReferences(BibEntry entry) { + if (needToRefreshReferences(entry)) { + List references; + try { + references = fetcher.searchCiting(entry); + } catch (FetcherException e) { + LOGGER.error("Error while fetching references", e); + references = List.of(); + } + cache.cacheOrMergeReferences(entry, references); + } + + return cache.getReferences(entry); + } + + @Override + public void forceRefreshCitations(BibEntry entry) { + try { + List citations = fetcher.searchCitedBy(entry); + cache.cacheOrMergeCitations(entry, citations); + } catch (FetcherException e) { + LOGGER.error("Error while fetching citations", e); + } + } + + private boolean needToRefreshCitations(BibEntry entry) { + return !cache.citationsCached(entry); + } + + private boolean needToRefreshReferences(BibEntry entry) { + return !cache.referencesCached(entry); + } + + @Override + public void forceRefreshReferences(BibEntry entry) { + List references; + try { + references = fetcher.searchCiting(entry); + } catch (FetcherException e) { + LOGGER.error("Error while fetching references", e); + references = List.of(); + } + cache.cacheOrMergeReferences(entry, references); + } +} diff --git a/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java new file mode 100644 index 00000000000..210821d708d --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java @@ -0,0 +1,36 @@ +package org.jabref.logic.citation.service; + +import java.util.List; +import org.jabref.logic.citation.repository.BibEntryRelationsRepository; +import org.jabref.model.entry.BibEntry; + +public class SearchCitationsRelationsService { + + BibEntryRelationsRepository relationsRepository; + + public SearchCitationsRelationsService(BibEntryRelationsRepository repository) { + this.relationsRepository = repository; + } + + public List searchReferences(BibEntry referencer) { + return this.relationsRepository.readReferences(referencer); + } + + public List searchReferences(BibEntry referencer, boolean forceUpdate) { + if (forceUpdate) { + this.relationsRepository.forceRefreshReferences(referencer); + } + return this.searchReferences(referencer); + } + + public List searchCitations(BibEntry cited) { + return this.relationsRepository.readCitations(cited); + } + + public List searchCitations(BibEntry cited, boolean forceUpdate) { + if (forceUpdate) { + this.relationsRepository.forceRefreshCitations(cited); + } + return this.searchCitations(cited); + } +} diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java index f17f6b8a99d..0b436ac9187 100644 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java @@ -88,13 +88,13 @@ void givenANewEntryWhenReadingCitationsForItShouldCallTheFetcher(BibEntry bibEnt null, null ); - var bibEntryRelationsCache = new BibEntryRelationsCache(); - var bibEntryRelationsRepository = new BibEntryRelationsRepository( + var bibEntryRelationsCache = new LRUBibEntryRelationsCache(); + var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( citationFetcherMock, bibEntryRelationsCache ); // WHEN - var citations = bibEntryRelationsRepository.getCitations(bibEntry); + var citations = bibEntryRelationsRepository.readCitations(bibEntry); // THEN Assertions.assertFalse(citations.isEmpty()); @@ -110,15 +110,15 @@ void givenAnEmptyCacheAndAValidBibEntryWhenReadingCitationsShouldPopulateTheCach var citationFetcherMock = new CitationFetcherMock( BibEntryRelationsRepositoryTest::getCitedBy, null, null ); - var bibEntryRelationsCache = new BibEntryRelationsCache(); - var bibEntryRelationsRepository = new BibEntryRelationsRepository( + var bibEntryRelationsCache = new LRUBibEntryRelationsCache(); + var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( citationFetcherMock, bibEntryRelationsCache ); var bibEntry = BibEntryRelationsRepositoryTest.createBibEntry(1); // WHEN Assertions.assertEquals(List.of(), bibEntryRelationsCache.getCitations(bibEntry)); - var citations = bibEntryRelationsRepository.getCitations(bibEntry); + var citations = bibEntryRelationsRepository.readCitations(bibEntry); var fromCacheCitations = bibEntryRelationsCache.getCitations(bibEntry); // THEN diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java new file mode 100644 index 00000000000..12b0a392944 --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java @@ -0,0 +1,39 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; +import java.util.function.Consumer; +import java.util.function.Function; +import org.jabref.model.entry.BibEntry; + +public class BibEntryRelationsRepositoryTestHelpers { + public static class CreateRepository { + public static BibEntryRelationsRepository from( + Function> retrieveCitations, + Function> retrieveReferences, + Consumer forceRefreshCitations, + Consumer forceRefreshReferences + ) { + return new BibEntryRelationsRepository() { + @Override + public List readCitations(BibEntry entry) { + return retrieveCitations.apply(entry); + } + + @Override + public List readReferences(BibEntry entry) { + return retrieveReferences.apply(entry); + } + + @Override + public void forceRefreshCitations(BibEntry entry) { + forceRefreshCitations.accept(entry); + } + + @Override + public void forceRefreshReferences(BibEntry entry) { + forceRefreshReferences.accept(entry); + } + }; + } + } +} diff --git a/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java b/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java new file mode 100644 index 00000000000..61473e8fd05 --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java @@ -0,0 +1,89 @@ +package org.jabref.logic.citation.service; + +import java.util.ArrayList; +import java.util.List; +import org.jabref.logic.citation.repository.BibEntryRelationsRepositoryTestHelpers; +import org.jabref.model.entry.BibEntry; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class SearchCitationsRelationsServiceTest { + + @Test + void serviceShouldSearchForReferences() { + // GIVEN + var referencesToReturn = List.of(new BibEntry()); + var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( + List::of, e -> referencesToReturn, e -> {}, e -> {} + ); + var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); + + // WHEN + var referencer = new BibEntry(); + List references = searchCitationsRelationsService.searchReferences(referencer); + + // THEN + Assertions.assertEquals(referencesToReturn, references); + } + + @Test + void serviceShouldForceReferencesUpdate() { + // GiVEN + var newReference = new BibEntry(); + var referencesToReturn = List.of(newReference); + var referenceToUpdate = new ArrayList(); + var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( + List::of, e -> referencesToReturn, e -> {}, e -> referenceToUpdate.add(newReference) + ); + var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); + + // WHEN + var referencer = new BibEntry(); + var references = searchCitationsRelationsService.searchReferences(referencer, true); + + // THEN + Assertions.assertEquals(referencesToReturn, references); + Assertions.assertEquals(1, referenceToUpdate.size()); + Assertions.assertSame(newReference, referenceToUpdate.getFirst()); + Assertions.assertNotSame(referencesToReturn, referenceToUpdate); + } + + @Test + void serviceShouldSearchForCitations() { + // GIVEN + var citationsToReturn = List.of(new BibEntry()); + var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( + e -> citationsToReturn, List::of, e -> {}, e -> {} + ); + var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); + + // WHEN + var cited = new BibEntry(); + List citations = searchCitationsRelationsService.searchCitations(cited); + + // THEN + Assertions.assertEquals(citationsToReturn, citations); + } + + @Test + void serviceShouldForceCitationsUpdate() { + // GiVEN + var newCitations = new BibEntry(); + var citationsToReturn = List.of(newCitations); + var citationsToUpdate = new ArrayList(); + var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( + e -> citationsToReturn, List::of, e -> citationsToUpdate.add(newCitations), e -> {} + ); + var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); + + // WHEN + var cited = new BibEntry(); + var citations = searchCitationsRelationsService.searchCitations(cited, true); + + // THEN + Assertions.assertEquals(citationsToReturn, citations); + Assertions.assertEquals(1, citationsToUpdate.size()); + Assertions.assertSame(newCitations, citationsToUpdate.getFirst()); + Assertions.assertNotSame(citationsToReturn, citationsToUpdate); + } +} From 18db75ea96874ce8bb5cdb1c999b3a0ab23ce907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Sat, 28 Sep 2024 21:55:32 +0200 Subject: [PATCH 03/35] Refactor Citations Relations Service Layer (#11189) * Move logic from repository to service * Refactor repositories * Update tab configuration --- .../CitationRelationsTab.java | 93 ++++--- .../CitationsRelationsTabViewModel.java | 2 +- .../BibEntryRelationsRepository.java | 24 +- .../repository/LRUBibEntryRelationsCache.java | 39 ++- .../LRUBibEntryRelationsRepository.java | 69 ++--- .../SearchCitationsRelationsService.java | 56 ++-- .../SemanticScholarCitationFetcher.java | 4 +- .../org/jabref/logic/util/BackgroundTask.java | 10 + .../CitationsRelationsTabViewModelTest.java | 3 +- ...ntryRelationsRepositoryHelpersForTest.java | 87 +++++++ .../BibEntryRelationsRepositoryTest.java | 128 ---------- ...ibEntryRelationsRepositoryTestHelpers.java | 39 --- .../LRUBibEntryRelationsRepositoryTest.java | 98 +++++++ .../SearchCitationsRelationsServiceTest.java | 239 ++++++++++++------ .../CitationFetcherHelpersForTest.java | 32 +++ 15 files changed, 552 insertions(+), 371 deletions(-) create mode 100644 src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java delete mode 100644 src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java delete mode 100644 src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java create mode 100644 src/test/java/org/jabref/logic/importer/fetcher/CitationFetcherHelpersForTest.java diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index 048d058462d..e0f6c6c49a7 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -30,15 +30,16 @@ import org.jabref.gui.StateManager; import org.jabref.gui.desktop.os.NativeDesktop; import org.jabref.gui.entryeditor.EntryEditorTab; -import org.jabref.logic.citation.repository.BibEntryRelationsCache; -import org.jabref.logic.citation.repository.BibEntryRelationsRepository; -import org.jabref.logic.importer.fetcher.CitationFetcher; -import org.jabref.logic.importer.fetcher.SemanticScholarCitationFetcher; import org.jabref.gui.icon.IconTheme; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.gui.util.NoSelectionModel; import org.jabref.gui.util.ViewModelListCellFactory; +import org.jabref.logic.citation.repository.LRUBibEntryRelationsCache; +import org.jabref.logic.citation.repository.LRUBibEntryRelationsRepository; +import org.jabref.logic.citation.service.SearchCitationsRelationsService; import org.jabref.logic.database.DuplicateCheck; +import org.jabref.logic.importer.fetcher.CitationFetcher; +import org.jabref.logic.importer.fetcher.SemanticScholarCitationFetcher; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.BackgroundTask; import org.jabref.logic.util.TaskExecutor; @@ -73,7 +74,7 @@ public class CitationRelationsTab extends EntryEditorTab { private final GuiPreferences preferences; private final LibraryTab libraryTab; private final TaskExecutor taskExecutor; - private final BibEntryRelationsRepository bibEntryRelationsRepository; + private final SearchCitationsRelationsService searchCitationsRelationsService; private final CitationsRelationsTabViewModel citationsRelationsTabViewModel; private final DuplicateCheck duplicateCheck; @@ -94,11 +95,22 @@ public CitationRelationsTab(DialogService dialogService, setTooltip(new Tooltip(Localization.lang("Show articles related by citation"))); this.duplicateCheck = new DuplicateCheck(new BibEntryTypesManager()); - this.bibEntryRelationsRepository = new BibEntryRelationsRepository( + var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( + new LRUBibEntryRelationsCache() + ); + this.searchCitationsRelationsService = new SearchCitationsRelationsService( new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), - new BibEntryRelationsCache() + bibEntryRelationsRepository + ); + citationsRelationsTabViewModel = new CitationsRelationsTabViewModel( + databaseContext, + preferences, + undoManager, + stateManager, + dialogService, + fileUpdateMonitor, + taskExecutor ); - citationsRelationsTabViewModel = new CitationsRelationsTabViewModel(databaseContext, preferences, undoManager, stateManager, dialogService, fileUpdateMonitor, taskExecutor); } /** @@ -347,41 +359,54 @@ private void searchForRelations(BibEntry entry, CheckListView> task; - - if (searchType == CitationFetcher.SearchType.CITES) { - task = BackgroundTask.wrap(() -> { - if (shouldRefresh) { - bibEntryRelationsRepository.forceRefreshReferences(entry); - } - return bibEntryRelationsRepository.getReferences(entry); - }); - citingTask = task; - } else { - task = BackgroundTask.wrap(() -> { - if (shouldRefresh) { - bibEntryRelationsRepository.forceRefreshCitations(entry); - } - return bibEntryRelationsRepository.getCitations(entry); - }); - citedByTask = task; - } - - task.onRunning(() -> prepareToSearchForRelations(abortButton, refreshButton, importButton, progress, task)) - .onSuccess(fetchedList -> onSearchForRelationsSucceed(entry, listView, abortButton, refreshButton, - searchType, importButton, progress, fetchedList, observableList)) + this.createBackGroundTask(entry, searchType, shouldRefresh) + .consumeOnRunning(task -> prepareToSearchForRelations( + abortButton, refreshButton, importButton, progress, task + )) + .onSuccess(fetchedList -> onSearchForRelationsSucceed( + entry, + listView, + abortButton, + refreshButton, + searchType, + importButton, + progress, + fetchedList, + observableList + )) .onFailure(exception -> { LOGGER.error("Error while fetching citing Articles", exception); hideNodes(abortButton, progress, importButton); - listView.setPlaceholder(new Label(Localization.lang("Error while fetching citing entries: %0", - exception.getMessage()))); - + listView.setPlaceholder( + new Label(Localization.lang( + "Error while fetching citing entries: %0", exception.getMessage()) + ) + ); refreshButton.setVisible(true); dialogService.notify(exception.getMessage()); }) .executeWith(taskExecutor); } + private BackgroundTask> createBackGroundTask( + BibEntry entry, CitationFetcher.SearchType searchType, boolean shouldRefresh + ) { + return switch (searchType) { + case CitationFetcher.SearchType.CITES -> { + citingTask = BackgroundTask.wrap( + () -> this.searchCitationsRelationsService.searchReferences(entry, shouldRefresh) + ); + yield citingTask; + } + case CitationFetcher.SearchType.CITED_BY -> { + citedByTask = BackgroundTask.wrap( + () -> this.searchCitationsRelationsService.searchCitations(entry, shouldRefresh) + ); + yield citedByTask; + } + }; + } + private void onSearchForRelationsSucceed(BibEntry entry, CheckListView listView, Button abortButton, Button refreshButton, CitationFetcher.SearchType searchType, Button importButton, diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java index 0d0d5646ff1..f095e08e45e 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java @@ -8,11 +8,11 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; -import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.gui.externalfiles.ImportHandler; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.logic.citationkeypattern.CitationKeyGenerator; import org.jabref.logic.citationkeypattern.CitationKeyPatternPreferences; +import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.logic.util.TaskExecutor; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; diff --git a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java index 48f38e2a38d..84b4c73a1d7 100644 --- a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java @@ -1,26 +1,20 @@ package org.jabref.logic.citation.repository; import java.util.List; + import org.jabref.model.entry.BibEntry; public interface BibEntryRelationsRepository { + + void insertCitations(BibEntry entry, List citations); + List readCitations(BibEntry entry); + boolean containsCitations(BibEntry entry); + + void insertReferences(BibEntry entry, List citations); + List readReferences(BibEntry entry); - /** - * Fetch citations for a bib entry and update local database. - * @param entry should not be null - * @deprecated fetching citations should be done by the service layer (calling code) - */ - @Deprecated - void forceRefreshCitations(BibEntry entry); - - /** - * Fetch references made by a bib entry and update local database. - * @param entry should not be null - * @deprecated fetching references should be done by the service layer (calling code) - */ - @Deprecated - void forceRefreshReferences(BibEntry entry); + boolean containsReferences(BibEntry entry); } diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java index 8e6d491ce75..f87455fc033 100644 --- a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java +++ b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java @@ -1,38 +1,57 @@ package org.jabref.logic.citation.repository; -import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import org.eclipse.jgit.util.LRUMap; +import java.util.Set; + import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.identifier.DOI; +import org.eclipse.jgit.util.LRUMap; + public class LRUBibEntryRelationsCache { private static final Integer MAX_CACHED_ENTRIES = 100; - private static final Map> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); - private static final Map> REFERENCES_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); + private static final Map> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); + private static final Map> REFERENCES_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); public List getCitations(BibEntry entry) { - return CITATIONS_MAP.getOrDefault(entry.getDOI().map(DOI::getDOI).orElse(""), Collections.emptyList()); + return entry + .getDOI() + .stream() + .flatMap(doi -> CITATIONS_MAP.getOrDefault(doi, Set.of()).stream()) + .toList(); } public List getReferences(BibEntry entry) { - return REFERENCES_MAP.getOrDefault(entry.getDOI().map(DOI::getDOI).orElse(""), Collections.emptyList()); + return entry + .getDOI() + .stream() + .flatMap(doi -> REFERENCES_MAP.getOrDefault(doi, Set.of()).stream()) + .toList(); } public void cacheOrMergeCitations(BibEntry entry, List citations) { - entry.getDOI().ifPresent(doi -> CITATIONS_MAP.put(doi.getDOI(), citations)); + entry.getDOI().ifPresent(doi -> { + var cachedRelations = CITATIONS_MAP.getOrDefault(doi, new LinkedHashSet<>()); + cachedRelations.addAll(citations); + CITATIONS_MAP.put(doi, cachedRelations); + }); } public void cacheOrMergeReferences(BibEntry entry, List references) { - entry.getDOI().ifPresent(doi -> REFERENCES_MAP.putIfAbsent(doi.getDOI(), references)); + entry.getDOI().ifPresent(doi -> { + var cachedRelations = REFERENCES_MAP.getOrDefault(doi, new LinkedHashSet<>()); + cachedRelations.addAll(references); + REFERENCES_MAP.put(doi, cachedRelations); + }); } public boolean citationsCached(BibEntry entry) { - return CITATIONS_MAP.containsKey(entry.getDOI().map(DOI::getDOI).orElse("")); + return entry.getDOI().map(CITATIONS_MAP::containsKey).orElse(false); } public boolean referencesCached(BibEntry entry) { - return REFERENCES_MAP.containsKey(entry.getDOI().map(DOI::getDOI).orElse("")); + return entry.getDOI().map(REFERENCES_MAP::containsKey).orElse(false); } } diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java index 23ab083ee4b..18c360ec17e 100644 --- a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java @@ -1,78 +1,49 @@ package org.jabref.logic.citation.repository; import java.util.List; +import java.util.Objects; -import org.jabref.logic.importer.fetcher.CitationFetcher; -import org.jabref.logic.importer.FetcherException; import org.jabref.model.entry.BibEntry; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - public class LRUBibEntryRelationsRepository implements BibEntryRelationsRepository { - private static final Logger LOGGER = LoggerFactory - .getLogger(LRUBibEntryRelationsRepository.class); - private final CitationFetcher fetcher; private final LRUBibEntryRelationsCache cache; - public LRUBibEntryRelationsRepository(CitationFetcher fetcher, LRUBibEntryRelationsCache cache) { - this.fetcher = fetcher; + public LRUBibEntryRelationsRepository(LRUBibEntryRelationsCache cache) { this.cache = cache; } @Override - public List readCitations(BibEntry entry) { - if (needToRefreshCitations(entry)) { - forceRefreshCitations(entry); - } - - return cache.getCitations(entry); + public void insertCitations(BibEntry entry, List citations) { + cache.cacheOrMergeCitations( + entry, Objects.requireNonNullElseGet(citations, List::of) + ); } @Override - public List readReferences(BibEntry entry) { - if (needToRefreshReferences(entry)) { - List references; - try { - references = fetcher.searchCiting(entry); - } catch (FetcherException e) { - LOGGER.error("Error while fetching references", e); - references = List.of(); - } - cache.cacheOrMergeReferences(entry, references); - } - - return cache.getReferences(entry); + public List readCitations(BibEntry entry) { + return cache.getCitations(entry); } @Override - public void forceRefreshCitations(BibEntry entry) { - try { - List citations = fetcher.searchCitedBy(entry); - cache.cacheOrMergeCitations(entry, citations); - } catch (FetcherException e) { - LOGGER.error("Error while fetching citations", e); - } + public boolean containsCitations(BibEntry entry) { + return cache.citationsCached(entry); } - private boolean needToRefreshCitations(BibEntry entry) { - return !cache.citationsCached(entry); + @Override + public void insertReferences(BibEntry entry, List references) { + cache.cacheOrMergeReferences( + entry, Objects.requireNonNullElseGet(references, List::of) + ); } - private boolean needToRefreshReferences(BibEntry entry) { - return !cache.referencesCached(entry); + @Override + public List readReferences(BibEntry entry) { + return cache.getReferences(entry); } @Override - public void forceRefreshReferences(BibEntry entry) { - List references; - try { - references = fetcher.searchCiting(entry); - } catch (FetcherException e) { - LOGGER.error("Error while fetching references", e); - references = List.of(); - } - cache.cacheOrMergeReferences(entry, references); + public boolean containsReferences(BibEntry entry) { + return cache.referencesCached(entry); } } diff --git a/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java index 210821d708d..8dfc6ac5659 100644 --- a/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java +++ b/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java @@ -1,36 +1,60 @@ package org.jabref.logic.citation.service; import java.util.List; + import org.jabref.logic.citation.repository.BibEntryRelationsRepository; +import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.model.entry.BibEntry; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class SearchCitationsRelationsService { - BibEntryRelationsRepository relationsRepository; + private static final Logger LOGGER = LoggerFactory + .getLogger(SearchCitationsRelationsService.class); - public SearchCitationsRelationsService(BibEntryRelationsRepository repository) { - this.relationsRepository = repository; - } + private final CitationFetcher citationFetcher; + private final BibEntryRelationsRepository relationsRepository; - public List searchReferences(BibEntry referencer) { - return this.relationsRepository.readReferences(referencer); + public SearchCitationsRelationsService( + CitationFetcher citationFetcher, BibEntryRelationsRepository repository + ) { + this.citationFetcher = citationFetcher; + this.relationsRepository = repository; } public List searchReferences(BibEntry referencer, boolean forceUpdate) { - if (forceUpdate) { - this.relationsRepository.forceRefreshReferences(referencer); + if (forceUpdate || !this.relationsRepository.containsReferences(referencer)) { + try { + var references = this.citationFetcher.searchCiting(referencer); + if (!references.isEmpty()) { + this.relationsRepository.insertReferences(referencer, references); + } + } catch (Exception e) { + var errMsg = "Error while fetching references for entry %s".formatted( + referencer.getTitle() + ); + LOGGER.error(errMsg); + } } - return this.searchReferences(referencer); - } - - public List searchCitations(BibEntry cited) { - return this.relationsRepository.readCitations(cited); + return this.relationsRepository.readReferences(referencer); } public List searchCitations(BibEntry cited, boolean forceUpdate) { - if (forceUpdate) { - this.relationsRepository.forceRefreshCitations(cited); + if (forceUpdate || !this.relationsRepository.containsCitations(cited)) { + try { + var citations = this.citationFetcher.searchCitedBy(cited); + if (!citations.isEmpty()) { + this.relationsRepository.insertCitations(cited, citations); + } + } catch (Exception e) { + var errMsg = "Error while fetching citations for entry %s".formatted( + cited.getTitle() + ); + LOGGER.error(errMsg); + } } - return this.searchCitations(cited); + return this.relationsRepository.readCitations(cited); } } diff --git a/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java index 51a4762432d..5efc66255ee 100644 --- a/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java +++ b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java @@ -9,11 +9,11 @@ import org.jabref.logic.importer.ImporterPreferences; import org.jabref.logic.net.URLDownload; import org.jabref.logic.util.BuildInfo; +import org.jabref.model.citation.semanticscholar.CitationsResponse; +import org.jabref.model.citation.semanticscholar.ReferencesResponse; import org.jabref.model.entry.BibEntry; import com.google.gson.Gson; -import org.jabref.model.citation.semanticscholar.CitationsResponse; -import org.jabref.model.citation.semanticscholar.ReferencesResponse; public class SemanticScholarCitationFetcher implements CitationFetcher, CustomizableKeyFetcher { private static final String SEMANTIC_SCHOLAR_API = "https://api.semanticscholar.org/graph/v1/"; diff --git a/src/main/java/org/jabref/logic/util/BackgroundTask.java b/src/main/java/org/jabref/logic/util/BackgroundTask.java index 1a905432945..11e2b4083e4 100644 --- a/src/main/java/org/jabref/logic/util/BackgroundTask.java +++ b/src/main/java/org/jabref/logic/util/BackgroundTask.java @@ -172,6 +172,16 @@ public BackgroundTask onRunning(Runnable onRunning) { return this; } + /** + * Curry a consumer to on an on running runnable and invoke it after the task is started. + * + * @param onRunningConsumer should not be null + * @see BackgroundTask#consumeOnRunning(Consumer) + */ + public BackgroundTask consumeOnRunning(Consumer> onRunningConsumer) { + return this.onRunning(() -> onRunningConsumer.accept(this)); + } + /** * Sets the {@link Consumer} that is invoked after the task is successfully finished. * The consumer always runs on the JavaFX thread. diff --git a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java index 48e074fd00c..3d9c1b81129 100644 --- a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java +++ b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java @@ -9,8 +9,6 @@ import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; -import org.jabref.logic.importer.fetcher.CitationFetcher; -import org.jabref.gui.externalfiles.ImportHandler; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.logic.FilePreferences; import org.jabref.logic.bibtex.FieldPreferences; @@ -19,6 +17,7 @@ import org.jabref.logic.database.DuplicateCheck; import org.jabref.logic.importer.ImportFormatPreferences; import org.jabref.logic.importer.ImporterPreferences; +import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.logic.preferences.OwnerPreferences; import org.jabref.logic.preferences.TimestampPreferences; import org.jabref.logic.util.CurrentThreadTaskExecutor; diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java new file mode 100644 index 00000000000..f2305b38b33 --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java @@ -0,0 +1,87 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; +import java.util.Map; +import java.util.function.BiConsumer; +import java.util.function.Function; + +import org.jabref.model.entry.BibEntry; + +public class BibEntryRelationsRepositoryHelpersForTest { + public static class Mocks { + public static BibEntryRelationsRepository from( + Function> retrieveCitations, + BiConsumer> insertCitations, + Function> retrieveReferences, + BiConsumer> insertReferences + ) { + return new BibEntryRelationsRepository() { + @Override + public void insertCitations(BibEntry entry, List citations) { + insertCitations.accept(entry, citations); + } + + @Override + public List readCitations(BibEntry entry) { + return retrieveCitations.apply(entry); + } + + @Override + public boolean containsCitations(BibEntry entry) { + return true; + } + + @Override + public void insertReferences(BibEntry entry, List citations) { + insertReferences.accept(entry, citations); + } + + @Override + public List readReferences(BibEntry entry) { + return retrieveReferences.apply(entry); + } + + @Override + public boolean containsReferences(BibEntry entry) { + return true; + } + }; + } + + public static BibEntryRelationsRepository from( + Map> citationsDB, Map> referencesDB + ) { + return new BibEntryRelationsRepository() { + @Override + public void insertCitations(BibEntry entry, List citations) { + citationsDB.put(entry, citations); + } + + @Override + public List readCitations(BibEntry entry) { + return citationsDB.getOrDefault(entry, List.of()); + } + + @Override + public boolean containsCitations(BibEntry entry) { + return citationsDB.containsKey(entry); + } + + @Override + public void insertReferences(BibEntry entry, List citations) { + referencesDB.put(entry, citations); + } + + @Override + public List readReferences(BibEntry entry) { + return referencesDB.getOrDefault(entry, List.of()); + } + + @Override + public boolean containsReferences(BibEntry entry) { + return referencesDB.containsKey(entry); + } + }; + } + } +} diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java deleted file mode 100644 index 0b436ac9187..00000000000 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTest.java +++ /dev/null @@ -1,128 +0,0 @@ -package org.jabref.logic.citation.repository; - -import java.util.HashSet; -import java.util.List; - -import java.util.function.Function; -import java.util.stream.IntStream; -import java.util.stream.Stream; -import org.jabref.logic.importer.FetcherException; -import org.jabref.logic.importer.fetcher.CitationFetcher; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.field.StandardField; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; - -class BibEntryRelationsRepositoryTest { - - private static Stream createBibEntries() { - return IntStream - .range(0, 150) - .mapToObj(BibEntryRelationsRepositoryTest::createBibEntry); - } - - private static List getCitedBy(BibEntry entry) { - return List.of(BibEntryRelationsRepositoryTest.createCitingBibEntry(entry)); - } - - private static BibEntry createBibEntry(int i) { - return new BibEntry() - .withCitationKey("entry" + i) - .withField(StandardField.DOI, "10.1234/5678" + i); - } - - private static BibEntry createCitingBibEntry(Integer i) { - return new BibEntry() - .withCitationKey("citing_entry" + i) - .withField(StandardField.DOI, "10.2345/6789" + i); - } - - private static BibEntry createCitingBibEntry(BibEntry citedEntry) { - return createCitingBibEntry( - Integer.valueOf(citedEntry.getCitationKey().orElseThrow().substring(5)) - ); - } - - /** - * Simple mock to avoid using Mockito (reduce overall complexity) - */ - private record CitationFetcherMock( - Function> searchCiteByDelegate, - Function> searchCitingDelegate, - String name - ) implements CitationFetcher { - - @Override - public List searchCitedBy(BibEntry entry) throws FetcherException { - return this.searchCiteByDelegate.apply(entry); - } - - @Override - public List searchCiting(BibEntry entry) throws FetcherException { - return this.searchCitingDelegate.apply(entry); - } - - @Override - public String getName() { - return this.name; - } - } - - @ParameterizedTest - @MethodSource("createBibEntries") - @DisplayName( - "Given a new bib entry when reading citations for it should call the fetcher" - ) - void givenANewEntryWhenReadingCitationsForItShouldCallTheFetcher(BibEntry bibEntry) { - // GIVEN - var entryCaptor = new HashSet(); - var citationFetcherMock = new CitationFetcherMock( - entry -> { - entryCaptor.add(entry); - return BibEntryRelationsRepositoryTest.getCitedBy(entry); - }, - null, - null - ); - var bibEntryRelationsCache = new LRUBibEntryRelationsCache(); - var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( - citationFetcherMock, bibEntryRelationsCache - ); - - // WHEN - var citations = bibEntryRelationsRepository.readCitations(bibEntry); - - // THEN - Assertions.assertFalse(citations.isEmpty()); - Assertions.assertTrue(entryCaptor.contains(bibEntry)); - } - - @Test - @DisplayName( - "Given an empty cache for a valid entry when reading the citations should populate cache" - ) - void givenAnEmptyCacheAndAValidBibEntryWhenReadingCitationsShouldPopulateTheCache() { - // GIVEN - var citationFetcherMock = new CitationFetcherMock( - BibEntryRelationsRepositoryTest::getCitedBy, null, null - ); - var bibEntryRelationsCache = new LRUBibEntryRelationsCache(); - var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( - citationFetcherMock, bibEntryRelationsCache - ); - var bibEntry = BibEntryRelationsRepositoryTest.createBibEntry(1); - - // WHEN - Assertions.assertEquals(List.of(), bibEntryRelationsCache.getCitations(bibEntry)); - var citations = bibEntryRelationsRepository.readCitations(bibEntry); - var fromCacheCitations = bibEntryRelationsCache.getCitations(bibEntry); - - // THEN - Assertions.assertFalse(fromCacheCitations.isEmpty()); - Assertions.assertEquals(citations, fromCacheCitations); - } -} diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java deleted file mode 100644 index 12b0a392944..00000000000 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java +++ /dev/null @@ -1,39 +0,0 @@ -package org.jabref.logic.citation.repository; - -import java.util.List; -import java.util.function.Consumer; -import java.util.function.Function; -import org.jabref.model.entry.BibEntry; - -public class BibEntryRelationsRepositoryTestHelpers { - public static class CreateRepository { - public static BibEntryRelationsRepository from( - Function> retrieveCitations, - Function> retrieveReferences, - Consumer forceRefreshCitations, - Consumer forceRefreshReferences - ) { - return new BibEntryRelationsRepository() { - @Override - public List readCitations(BibEntry entry) { - return retrieveCitations.apply(entry); - } - - @Override - public List readReferences(BibEntry entry) { - return retrieveReferences.apply(entry); - } - - @Override - public void forceRefreshCitations(BibEntry entry) { - forceRefreshCitations.accept(entry); - } - - @Override - public void forceRefreshReferences(BibEntry entry) { - forceRefreshReferences.accept(entry); - } - }; - } - } -} diff --git a/src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java new file mode 100644 index 00000000000..ee68782e2c9 --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java @@ -0,0 +1,98 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; +import java.util.random.RandomGenerator; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; + +class LRUBibEntryRelationsRepositoryTest { + + private static Stream createBibEntries() { + return IntStream + .range(0, 150) + .mapToObj(LRUBibEntryRelationsRepositoryTest::createBibEntry); + } + + private static BibEntry createBibEntry(int i) { + return new BibEntry() + .withCitationKey(String.valueOf(i)) + .withField(StandardField.DOI, "10.1234/5678" + i); + } + + private static List createRelations(BibEntry entry) { + return entry + .getCitationKey() + .map(key -> RandomGenerator + .StreamableGenerator.of("L128X256MixRandom").ints(150) + .mapToObj(i -> new BibEntry() + .withCitationKey("%s relation %s".formatted(key, i)) + .withField(StandardField.DOI, "10.2345/6789" + i) + ) + ) + .orElseThrow() + .toList(); + } + + @ParameterizedTest + @MethodSource("createBibEntries") + void repositoryShouldMergeCitationsWhenInserting(BibEntry bibEntry) { + // GIVEN + var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( + new LRUBibEntryRelationsCache() + ); + assertFalse(bibEntryRelationsRepository.containsCitations(bibEntry)); + + // WHEN + var firstRelations = createRelations(bibEntry); + var secondRelations = createRelations(bibEntry); + bibEntryRelationsRepository.insertCitations(bibEntry, firstRelations); + bibEntryRelationsRepository.insertCitations(bibEntry, secondRelations); + + // THEN + var uniqueRelations = Stream + .concat(firstRelations.stream(), secondRelations.stream()) + .distinct() + .toList(); + var relationFromCache = bibEntryRelationsRepository.readCitations(bibEntry); + assertFalse(uniqueRelations.isEmpty()); + assertNotSame(uniqueRelations, relationFromCache); + assertEquals(uniqueRelations, relationFromCache); + } + + @ParameterizedTest + @MethodSource("createBibEntries") + void repositoryShouldMergeReferencesWhenInserting(BibEntry bibEntry) { + // GIVEN + var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( + new LRUBibEntryRelationsCache() + ); + assertFalse(bibEntryRelationsRepository.containsReferences(bibEntry)); + + // WHEN + var firstRelations = createRelations(bibEntry); + var secondRelations = createRelations(bibEntry); + bibEntryRelationsRepository.insertReferences(bibEntry, firstRelations); + bibEntryRelationsRepository.insertReferences(bibEntry, secondRelations); + + // THEN + var uniqueRelations = Stream + .concat(firstRelations.stream(), secondRelations.stream()) + .distinct() + .collect(Collectors.toList()); + var relationFromCache = bibEntryRelationsRepository.readReferences(bibEntry); + assertFalse(uniqueRelations.isEmpty()); + assertNotSame(uniqueRelations, relationFromCache); + assertEquals(uniqueRelations, relationFromCache); + } +} diff --git a/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java b/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java index 61473e8fd05..32d8dea0b15 100644 --- a/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java +++ b/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java @@ -1,89 +1,178 @@ package org.jabref.logic.citation.service; -import java.util.ArrayList; +import java.util.HashMap; import java.util.List; -import org.jabref.logic.citation.repository.BibEntryRelationsRepositoryTestHelpers; + +import org.jabref.logic.citation.repository.BibEntryRelationsRepositoryHelpersForTest; +import org.jabref.logic.importer.fetcher.CitationFetcherHelpersForTest; import org.jabref.model.entry.BibEntry; -import org.junit.jupiter.api.Assertions; + +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + class SearchCitationsRelationsServiceTest { - @Test - void serviceShouldSearchForReferences() { - // GIVEN - var referencesToReturn = List.of(new BibEntry()); - var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( - List::of, e -> referencesToReturn, e -> {}, e -> {} - ); - var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); - - // WHEN - var referencer = new BibEntry(); - List references = searchCitationsRelationsService.searchReferences(referencer); - - // THEN - Assertions.assertEquals(referencesToReturn, references); - } + @Nested + class CitationsTests { + @Test + void serviceShouldSearchForCitations() { + // GIVEN + var cited = new BibEntry(); + var citationsToReturn = List.of(new BibEntry()); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + e -> citationsToReturn, null, null, null + ); + var searchService = new SearchCitationsRelationsService(null, repository); - @Test - void serviceShouldForceReferencesUpdate() { - // GiVEN - var newReference = new BibEntry(); - var referencesToReturn = List.of(newReference); - var referenceToUpdate = new ArrayList(); - var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( - List::of, e -> referencesToReturn, e -> {}, e -> referenceToUpdate.add(newReference) - ); - var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); - - // WHEN - var referencer = new BibEntry(); - var references = searchCitationsRelationsService.searchReferences(referencer, true); - - // THEN - Assertions.assertEquals(referencesToReturn, references); - Assertions.assertEquals(1, referenceToUpdate.size()); - Assertions.assertSame(newReference, referenceToUpdate.getFirst()); - Assertions.assertNotSame(referencesToReturn, referenceToUpdate); - } + // WHEN + List citations = searchService.searchCitations(cited, false); + + // THEN + assertEquals(citationsToReturn, citations); + } + + @Test + void serviceShouldForceCitationsUpdate() { + // GiVEN + var cited = new BibEntry(); + var newCitations = new BibEntry(); + var citationsToReturn = List.of(newCitations); + var citationsDatabase = new HashMap>(); + var fetcher = CitationFetcherHelpersForTest.Mocks.from( + entry -> { + if (entry == cited) { + return citationsToReturn; + } + return List.of(); + }, + null + ); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + e -> citationsToReturn, + citationsDatabase::put, + List::of, + (e, r) -> { } + ); + var searchService = new SearchCitationsRelationsService(fetcher, repository); + + // WHEN + var citations = searchService.searchCitations(cited, true); + + // THEN + assertTrue(citationsDatabase.containsKey(cited)); + assertEquals(citationsToReturn, citationsDatabase.get(cited)); + assertEquals(citationsToReturn, citations); + } + + @Test + void serviceShouldFetchCitationsIfRepositoryIsEmpty() { + var cited = new BibEntry(); + var newCitations = new BibEntry(); + var citationsToReturn = List.of(newCitations); + var citationsDatabase = new HashMap>(); + var fetcher = CitationFetcherHelpersForTest.Mocks.from( + entry -> { + if (entry == cited) { + return citationsToReturn; + } + return List.of(); + }, + null + ); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + citationsDatabase, null + ); + var searchService = new SearchCitationsRelationsService(fetcher, repository); - @Test - void serviceShouldSearchForCitations() { - // GIVEN - var citationsToReturn = List.of(new BibEntry()); - var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( - e -> citationsToReturn, List::of, e -> {}, e -> {} - ); - var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); - - // WHEN - var cited = new BibEntry(); - List citations = searchCitationsRelationsService.searchCitations(cited); - - // THEN - Assertions.assertEquals(citationsToReturn, citations); + // WHEN + var citations = searchService.searchCitations(cited, false); + + // THEN + assertTrue(citationsDatabase.containsKey(cited)); + assertEquals(citationsToReturn, citationsDatabase.get(cited)); + assertEquals(citationsToReturn, citations); + } } - @Test - void serviceShouldForceCitationsUpdate() { - // GiVEN - var newCitations = new BibEntry(); - var citationsToReturn = List.of(newCitations); - var citationsToUpdate = new ArrayList(); - var repository = BibEntryRelationsRepositoryTestHelpers.CreateRepository.from( - e -> citationsToReturn, List::of, e -> citationsToUpdate.add(newCitations), e -> {} - ); - var searchCitationsRelationsService = new SearchCitationsRelationsService(repository); - - // WHEN - var cited = new BibEntry(); - var citations = searchCitationsRelationsService.searchCitations(cited, true); - - // THEN - Assertions.assertEquals(citationsToReturn, citations); - Assertions.assertEquals(1, citationsToUpdate.size()); - Assertions.assertSame(newCitations, citationsToUpdate.getFirst()); - Assertions.assertNotSame(citationsToReturn, citationsToUpdate); + @Nested + class ReferencesTests { + @Test + void serviceShouldSearchForReferences() { + // GIVEN + var referencer = new BibEntry(); + var referencesToReturn = List.of(new BibEntry()); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + null, null, e -> referencesToReturn, null + ); + var searchService = new SearchCitationsRelationsService(null, repository); + + // WHEN + List references = searchService.searchReferences(referencer, false); + + // THEN + assertEquals(referencesToReturn, references); + } + + @Test + void serviceShouldCallTheFetcherForReferencesIWhenForceUpdateIsTrue() { + // GIVEN + var referencer = new BibEntry(); + var newReference = new BibEntry(); + var referencesToReturn = List.of(newReference); + var referencesDatabase = new HashMap>(); + var fetcher = CitationFetcherHelpersForTest.Mocks.from(null, entry -> { + if (entry == referencer) { + return referencesToReturn; + } + return List.of(); + }); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + List::of, + (e, c) -> { }, + e -> referencesToReturn, + referencesDatabase::put + ); + var searchService = new SearchCitationsRelationsService(fetcher, repository); + + // WHEN + var references = searchService.searchReferences(referencer, true); + + // THEN + assertTrue(referencesDatabase.containsKey(referencer)); + assertEquals(referencesToReturn, referencesDatabase.get(referencer)); + assertEquals(referencesToReturn, references); + } + + @Test + void serviceShouldFetchReferencesIfRepositoryIsEmpty() { + var reference = new BibEntry(); + var newCitations = new BibEntry(); + var referencesToReturn = List.of(newCitations); + var referencesDatabase = new HashMap>(); + var fetcher = CitationFetcherHelpersForTest.Mocks.from( + null, + entry -> { + if (entry == reference) { + return referencesToReturn; + } + return List.of(); + } + ); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + null, referencesDatabase + ); + var searchService = new SearchCitationsRelationsService(fetcher, repository); + + // WHEN + var references = searchService.searchReferences(reference, false); + + // THEN + assertTrue(referencesDatabase.containsKey(reference)); + assertEquals(referencesToReturn, referencesDatabase.get(reference)); + assertEquals(referencesToReturn, references); + } } } diff --git a/src/test/java/org/jabref/logic/importer/fetcher/CitationFetcherHelpersForTest.java b/src/test/java/org/jabref/logic/importer/fetcher/CitationFetcherHelpersForTest.java new file mode 100644 index 00000000000..8a33679359e --- /dev/null +++ b/src/test/java/org/jabref/logic/importer/fetcher/CitationFetcherHelpersForTest.java @@ -0,0 +1,32 @@ +package org.jabref.logic.importer.fetcher; + +import java.util.List; +import java.util.function.Function; + +import org.jabref.model.entry.BibEntry; + +public class CitationFetcherHelpersForTest { + public static class Mocks { + public static CitationFetcher from( + Function> retrieveCitedBy, + Function> retrieveCiting + ) { + return new CitationFetcher() { + @Override + public List searchCitedBy(BibEntry entry) { + return retrieveCitedBy.apply(entry); + } + + @Override + public List searchCiting(BibEntry entry) { + return retrieveCiting.apply(entry); + } + + @Override + public String getName() { + return "Test citation fetcher"; + } + }; + } + } +} From 9a31735a65d6d83f13d5bdc54ca10e7951017228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Wed, 6 Nov 2024 22:24:16 +0100 Subject: [PATCH 04/35] Address PR comments (#11901) --- .../CitationRelationsTab.java | 16 ++++++++-------- .../SearchCitationsRelationsService.java | 17 ++++++----------- .../SearchCitationsRelationsServiceTest.java | 1 + 3 files changed, 15 insertions(+), 19 deletions(-) rename src/main/java/org/jabref/logic/citation/{service => }/SearchCitationsRelationsService.java (78%) rename src/test/java/org/jabref/logic/citation/{service => }/SearchCitationsRelationsServiceTest.java (99%) diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index 1b36d953ee7..e4af0a3fb5e 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -45,7 +45,7 @@ import org.jabref.logic.bibtex.FieldWriter; import org.jabref.logic.citation.repository.LRUBibEntryRelationsCache; import org.jabref.logic.citation.repository.LRUBibEntryRelationsRepository; -import org.jabref.logic.citation.service.SearchCitationsRelationsService; +import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.database.DuplicateCheck; import org.jabref.logic.exporter.BibWriter; import org.jabref.logic.importer.fetcher.CitationFetcher; @@ -116,8 +116,7 @@ public CitationRelationsTab(DialogService dialogService, new LRUBibEntryRelationsCache() ); this.searchCitationsRelationsService = new SearchCitationsRelationsService( - new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), - bibEntryRelationsRepository + new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), bibEntryRelationsRepository ); citationsRelationsTabViewModel = new CitationsRelationsTabViewModel( databaseContext, @@ -415,6 +414,7 @@ private void searchForRelations(BibEntry entry, CheckListView { LOGGER.error("Error while fetching citing Articles", exception); hideNodes(abortButton, progress, importButton); - listView.setPlaceholder( - new Label(Localization.lang( - "Error while fetching citing entries: %0", exception.getMessage()) - ) - ); + listView.setPlaceholder(new Label(Localization.lang("Error while fetching citing entries: %0", + exception.getMessage()))); refreshButton.setVisible(true); dialogService.notify(exception.getMessage()); }) .executeWith(taskExecutor); } + /** + * TODO: Make the method return a callable and let the calling method create the background task. + */ private BackgroundTask> createBackGroundTask( BibEntry entry, CitationFetcher.SearchType searchType, boolean shouldRefresh ) { diff --git a/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java similarity index 78% rename from src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java rename to src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java index 8dfc6ac5659..11d041abb7f 100644 --- a/src/main/java/org/jabref/logic/citation/service/SearchCitationsRelationsService.java +++ b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java @@ -1,8 +1,9 @@ -package org.jabref.logic.citation.service; +package org.jabref.logic.citation; import java.util.List; import org.jabref.logic.citation.repository.BibEntryRelationsRepository; +import org.jabref.logic.importer.FetcherException; import org.jabref.logic.importer.fetcher.CitationFetcher; import org.jabref.model.entry.BibEntry; @@ -31,11 +32,8 @@ public List searchReferences(BibEntry referencer, boolean forceUpdate) if (!references.isEmpty()) { this.relationsRepository.insertReferences(referencer, references); } - } catch (Exception e) { - var errMsg = "Error while fetching references for entry %s".formatted( - referencer.getTitle() - ); - LOGGER.error(errMsg); + } catch (FetcherException e) { + LOGGER.error("Error while fetching references for entry {}", referencer.getTitle(), e); } } return this.relationsRepository.readReferences(referencer); @@ -48,11 +46,8 @@ public List searchCitations(BibEntry cited, boolean forceUpdate) { if (!citations.isEmpty()) { this.relationsRepository.insertCitations(cited, citations); } - } catch (Exception e) { - var errMsg = "Error while fetching citations for entry %s".formatted( - cited.getTitle() - ); - LOGGER.error(errMsg); + } catch (FetcherException e) { + LOGGER.error("Error while fetching citations for entry {}", cited.getTitle(), e); } } return this.relationsRepository.readCitations(cited); diff --git a/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java similarity index 99% rename from src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java rename to src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java index 32d8dea0b15..25097ed4b87 100644 --- a/src/test/java/org/jabref/logic/citation/service/SearchCitationsRelationsServiceTest.java +++ b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.List; +import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.citation.repository.BibEntryRelationsRepositoryHelpersForTest; import org.jabref.logic.importer.fetcher.CitationFetcherHelpersForTest; import org.jabref.model.entry.BibEntry; From b1133d01c36430a4514ff59679a43bf5acab315c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Mon, 11 Nov 2024 20:42:58 +0100 Subject: [PATCH 05/35] MVStore implementation for citations: first approach (wip for open discussion) --- .../MVStoreBibEntryRelationsRepository.java | 38 +++ .../java/org/jabref/model/entry/BibEntry.java | 8 +- ...VStoreBibEntryRelationsRepositoryTest.java | 274 ++++++++++++++++++ 3 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java new file mode 100644 index 00000000000..6631363df28 --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java @@ -0,0 +1,38 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; + +import org.jabref.model.entry.BibEntry; + +public class MVStoreBibEntryRelationsRepository implements BibEntryRelationsRepository { + + @Override + public void insertCitations(BibEntry entry, List citations) { + throw new UnsupportedOperationException(); + } + + @Override + public List readCitations(BibEntry entry) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsCitations(BibEntry entry) { + throw new UnsupportedOperationException(); + } + + @Override + public void insertReferences(BibEntry entry, List citations) { + throw new UnsupportedOperationException(); + } + + @Override + public List readReferences(BibEntry entry) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean containsReferences(BibEntry entry) { + throw new UnsupportedOperationException(); + } +} diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index f0d2d43689e..0a9c31f4f0f 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -1,5 +1,6 @@ package org.jabref.model.entry; +import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -93,7 +94,7 @@ *

*/ @AllowedToUseLogic("because it needs access to parser and writers") -public class BibEntry implements Cloneable { +public class BibEntry implements Cloneable, Serializable { public static final EntryType DEFAULT_TYPE = StandardEntryType.Misc; private static final Logger LOGGER = LoggerFactory.getLogger(BibEntry.class); @@ -995,6 +996,11 @@ public BibEntry withMonth(Month parsedMonth) { return this; } + public BibEntry withType(EntryType type) { + this.setType(type); + return this; + } + /* * Returns user comments (arbitrary text before the entry), if they exist. If not, returns the empty String */ diff --git a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java new file mode 100644 index 00000000000..26b299f3bfc --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java @@ -0,0 +1,274 @@ +package org.jabref.logic.citation.repository; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.LinkedHashSet; +import java.util.List; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.random.RandomGenerator; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import org.jabref.model.citation.semanticscholar.PaperDetails; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.identifier.DOI; +import org.jabref.model.entry.types.StandardEntryType; + +import org.apache.commons.lang3.tuple.Pair; +import org.h2.mvstore.DataUtils; +import org.h2.mvstore.MVMap; +import org.h2.mvstore.MVStore; +import org.h2.mvstore.WriteBuffer; +import org.h2.mvstore.type.BasicDataType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class MVStoreBibEntryRelationsRepositoryTest { + + private static Stream createBibEntries() { + return IntStream + .range(0, 150) + .mapToObj(MVStoreBibEntryRelationsRepositoryTest::createBibEntry); + } + + private static BibEntry createBibEntry(int i) { + return new BibEntry() + .withCitationKey(String.valueOf(i)) + .withField(StandardField.DOI, "10.1234/5678" + i); + } + + /** + * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic + * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar + * and mapped as BibEntry will be serializable by the MVStore. + * @param entry should not be null + * @return never empty + */ + private static LinkedHashSet createRelations(BibEntry entry) { + return entry + .getCitationKey() + .map(key -> RandomGenerator.StreamableGenerator + .of("L128X256MixRandom").ints(150) + .mapToObj(i -> new BibEntry() + .withField(StandardField.TITLE, "A title:" + i) + .withField(StandardField.YEAR, String.valueOf(2024)) + .withField(StandardField.AUTHOR, "A list of authors:" + i) + .withType(StandardEntryType.Book) + .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.URL, "www.jabref.org/" + i) + .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) + ) + ) + .orElseThrow() + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + + static class BibEntrySerializer extends BasicDataType { + + private final static String FIELD_SEPARATOR = "--"; + + private static String toString(BibEntry entry) { + return String.join( + FIELD_SEPARATOR, + entry.getTitle().orElse("null"), + entry.getField(StandardField.YEAR).orElse("null"), + entry.getField(StandardField.AUTHOR).orElse("null"), + entry.getType().getDisplayName(), + entry.getDOI().map(DOI::getDOI).orElse("null"), + entry.getField(StandardField.URL).orElse("null"), + entry.getField(StandardField.ABSTRACT).orElse("null") + ); + } + + private static Optional extractFieldValue(String field) { + return Objects.equals(field, "null") || field == null + ? Optional.empty() + : Optional.of(field); + } + + private static BibEntry fromString(String serializedString) { + var fields = serializedString.split(FIELD_SEPARATOR); + BibEntry entry = new BibEntry(); + extractFieldValue(fields[0]).ifPresent(title -> entry.setField(StandardField.TITLE, title)); + extractFieldValue(fields[1]).ifPresent(year -> entry.setField(StandardField.YEAR, year)); + extractFieldValue(fields[2]).ifPresent(authors -> entry.setField(StandardField.AUTHOR, authors)); + extractFieldValue(fields[3]).ifPresent(type -> entry.setType(StandardEntryType.valueOf(type))); + extractFieldValue(fields[4]).ifPresent(doi -> entry.setField(StandardField.DOI, doi)); + extractFieldValue(fields[5]).ifPresent(url -> entry.setField(StandardField.URL, url)); + extractFieldValue(fields[6]) + .ifPresent(entryAbstract -> entry.setField(StandardField.ABSTRACT, entryAbstract)); + return entry; + } + + @Override + public int getMemory(BibEntry obj) { + return toString(obj).getBytes(StandardCharsets.UTF_8).length; + } + + @Override + public void write(WriteBuffer buff, BibEntry bibEntry) { + var asBytes = toString(bibEntry).getBytes(StandardCharsets.UTF_8); + buff.putInt(asBytes.length); + buff.put(asBytes); + } + + @Override + public BibEntry read(ByteBuffer buff) { + int serializedEntrySize = buff.getInt(); + var serializedEntry = DataUtils.readString(buff, serializedEntrySize); + return fromString(serializedEntry); + } + + @Override + public int compare(BibEntry a, BibEntry b) { + if (a == null || b == null) { + throw new NullPointerException(); + } + return toString(a).compareTo(toString(b)); + } + + @Override + public BibEntry[] createStorage(int size) { + return new BibEntry[size]; + } + } + + static class BibEntryHashSetSerializer extends BasicDataType> { + + private final BasicDataType bibEntryDataType = new BibEntrySerializer(); + + /** + * Memory size is the sum of all aggregated bibEntries memory size plus 4 bytes. + * Those 4 bytes are used to store the length of the collection itself. + * @param bibEntries should not be null + * @return total size in memory of the serialized collection of bib entries + */ + @Override + public int getMemory(LinkedHashSet bibEntries) { + return bibEntries + .stream() + .map(this.bibEntryDataType::getMemory) + .reduce(0, Integer::sum) + 4; + } + + @Override + public void write(WriteBuffer buff, LinkedHashSet bibEntries) { + buff.putInt(bibEntries.size()); + bibEntries.forEach(entry -> this.bibEntryDataType.write(buff, entry)); + } + + @Override + public LinkedHashSet read(ByteBuffer buff) { + return IntStream.range(0, buff.getInt()) + .mapToObj(it -> this.bibEntryDataType.read(buff)) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + + @Override + public LinkedHashSet[] createStorage(int size) { + return new LinkedHashSet[size]; + } + } + + @Test + void itShouldBePossibleToStoreABibEntryList(@TempDir Path temporaryFolder) throws IOException { + var file = Files.createFile(temporaryFolder.resolve("test_string_store")); + try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { + // GIVEN + MVMap> citations = store.openMap("citations"); + + // WHEN + citations.put("Hello", List.of("The", "World")); + store.commit(); + var fromStore = citations.get("Hello"); + + // THEN + Assertions.assertTrue(Files.exists(file)); + Assertions.assertEquals("Hello The World", "Hello " + String.join(" ", fromStore)); + } + } + + /** + * Fake in memory sequential save and load + */ + @Test + void IWouldLikeToSaveAndLoadCitationsForABibEntryFromAMap(@TempDir Path temporaryFolder) throws IOException { + var file = Files.createFile(temporaryFolder.resolve("bib_entry_citations_test_store")); + try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { + // GIVEN + Map> citationsToBeStored = createBibEntries() + .map(e -> Pair.of(e.getDOI().orElseThrow().getDOI(), createRelations(e))) + .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); + Assertions.assertFalse(citationsToBeStored.isEmpty()); + var mapConfiguration = new MVMap.Builder>() + .valueType(new BibEntryHashSetSerializer()); + + /** + var mapConfiguration = new MVMap.Builder>() + .valueType(new BibEntryHashSetSerializer()); + MVMap> citationsMap = store.openMap("citations", mapConfiguration); + **/ + MVMap> citationsMap = store.openMap("citations", mapConfiguration); + + // WHEN + citationsToBeStored.forEach((entry, citations) -> citationsMap.put(entry, new LinkedHashSet<>(citations))); + + // THEN + citationsToBeStored.forEach((entry, citations) -> { + Assertions.assertTrue(citationsMap.containsKey(entry)); + Assertions.assertEquals(citations, citationsMap.get(entry)); + }); + } + } + + /** + * Fake persisted sequential save and load operations. + */ + @Test + void IWouldLikeToSaveAndLoadCitationsForABibEntryFromAStore(@TempDir Path temporaryFolder) throws IOException { + var file = Files.createFile(temporaryFolder.resolve("bib_entry_citations_test_store")); + + // GIVEN + Map> citationsToBeStored = createBibEntries() + .map(e -> Pair.of(e.getDOI().orElseThrow().getDOI(), createRelations(e))) + .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); + Assertions.assertFalse(citationsToBeStored.isEmpty()); + + var mapConfiguration = new MVMap.Builder>() + .valueType(new BibEntryHashSetSerializer()); + + Map> citationsFromStore = null; + + // WHEN + // STORING AND CLOSING + try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { + MVMap> citationsMap = store.openMap("citations", mapConfiguration); + citationsToBeStored.forEach((entry, citations) -> citationsMap.put(entry, new LinkedHashSet<>(citations))); + store.commit(); + } + + // READING AND CLOSING + try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { + MVMap> citationsMap = store.openMap("citations", mapConfiguration); + citationsFromStore = Map.copyOf(citationsMap); + } + + // THEN + Assertions.assertNotNull(citationsFromStore); + Assertions.assertFalse(citationsFromStore.isEmpty()); + var entriesToBeStored = citationsToBeStored.entrySet(); + for (var entry : entriesToBeStored) { + Assertions.assertTrue(citationsFromStore.containsKey(entry.getKey())); + var citations = citationsFromStore.get(entry.getKey()); + Assertions.assertEquals(entry.getValue(), citations); + } + } +} From 6a8b21baabf8e1a42ac1a42500d30e5199a18552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Sun, 17 Nov 2024 01:19:18 +0100 Subject: [PATCH 06/35] Introduce the DAO layer for relations (#11189): * Implement MVStore for relations as DAO * Implement LRUCache for relations as DAO --- .../repository/BibEntryRelationDAO.java | 14 ++ .../LRUCacheBibEntryRelationsDAO.java | 75 ++++++++ .../MVStoreBibEntryRelationDAO.java | 181 ++++++++++++++++++ .../MVStoreBibEntryRelationsRepository.java | 38 ---- .../LRUCacheBibEntryRelationsDAOTest.java | 114 +++++++++++ ...oreBibEntryRelationsRepositoryDAOTest.java | 119 ++++++++++++ ...ryRelationsRepositoryPrototypingTest.java} | 10 +- 7 files changed, 511 insertions(+), 40 deletions(-) create mode 100644 src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java create mode 100644 src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java create mode 100644 src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java delete mode 100644 src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java rename src/test/java/org/jabref/logic/citation/repository/{MVStoreBibEntryRelationsRepositoryTest.java => MVStoreBibEntryRelationsRepositoryPrototypingTest.java} (97%) diff --git a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java new file mode 100644 index 00000000000..e56b08f4d3f --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java @@ -0,0 +1,14 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; + +import org.jabref.model.entry.BibEntry; + +public interface BibEntryRelationDAO { + + List getRelations(BibEntry entry); + + void cacheOrMergeRelations(BibEntry entry, List relations); + + boolean containsKey(BibEntry entry); +} diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java b/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java new file mode 100644 index 00000000000..996e7661ecc --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java @@ -0,0 +1,75 @@ +package org.jabref.logic.citation.repository; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.identifier.DOI; + +import org.eclipse.jgit.util.LRUMap; + +public abstract class LRUCacheBibEntryRelationsDAO implements BibEntryRelationDAO { + + private static final Integer MAX_CACHED_ENTRIES = 100; + + private final Map> relationsMap; + + LRUCacheBibEntryRelationsDAO(Map> relationsMap) { + this.relationsMap = relationsMap; + } + + @Override + public List getRelations(BibEntry entry) { + return entry + .getDOI() + .stream() + .flatMap(doi -> this.relationsMap.getOrDefault(doi, Set.of()).stream()) + .toList(); + } + + @Override + public synchronized void cacheOrMergeRelations(BibEntry entry, List relations) { + entry.getDOI().ifPresent(doi -> { + var cachedRelations = this.relationsMap.getOrDefault(doi, new LinkedHashSet<>()); + cachedRelations.addAll(relations); + relationsMap.put(doi, cachedRelations); + }); + } + + @Override + public boolean containsKey(BibEntry entry) { + return entry.getDOI().map(this.relationsMap::containsKey).orElse(false); + } + + public void clearEntries() { + this.relationsMap.clear(); + } + + public static class LRUCacheBibEntryCitations extends LRUCacheBibEntryRelationsDAO { + + private final static LRUCacheBibEntryCitations CITATIONS_CACHE = new LRUCacheBibEntryCitations(); + + private LRUCacheBibEntryCitations() { + super(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)); + } + + public static LRUCacheBibEntryCitations getInstance() { + return CITATIONS_CACHE; + } + } + + public static class LRUCacheBibEntryReferences extends LRUCacheBibEntryRelationsDAO { + + private final static LRUCacheBibEntryReferences REFERENCES_CACHE = new LRUCacheBibEntryReferences(); + + private LRUCacheBibEntryReferences() { + super(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)); + } + + public static LRUCacheBibEntryReferences getInstance() { + return REFERENCES_CACHE; + } + } +} diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java new file mode 100644 index 00000000000..7bba5251063 --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -0,0 +1,181 @@ +package org.jabref.logic.citation.repository; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.identifier.DOI; +import org.jabref.model.entry.types.StandardEntryType; + +import org.h2.mvstore.DataUtils; +import org.h2.mvstore.MVMap; +import org.h2.mvstore.MVStore; +import org.h2.mvstore.WriteBuffer; +import org.h2.mvstore.type.BasicDataType; + +public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { + + private final Path path; + private final String mapName; + private final MVMap.Builder> mapConfiguration = + new MVMap.Builder>().valueType(new BibEntryHashSetSerializer()); + + MVStoreBibEntryRelationDAO(Path path, String mapName) { + this.path = Objects.requireNonNull(path); + this.mapName = mapName; + } + + @Override + public List getRelations(BibEntry entry) { + return entry + .getDOI() + .map(doi -> { + try (var store = new MVStore.Builder().fileName(path.toAbsolutePath().toString()).open()) { + MVMap> relationsMap = store.openMap(mapName, mapConfiguration); + return relationsMap.getOrDefault(doi.getDOI(), new LinkedHashSet<>()).stream().toList(); + } + }) + .orElse(List.of()); + } + + @Override + synchronized public void cacheOrMergeRelations(BibEntry entry, List relations) { + entry.getDOI().ifPresent(doi -> { + try (var store = new MVStore.Builder().fileName(path.toAbsolutePath().toString()).open()) { + MVMap> relationsMap = store.openMap(mapName, mapConfiguration); + var relationsAlreadyStored = relationsMap.getOrDefault(doi.getDOI(), new LinkedHashSet<>()); + relationsAlreadyStored.addAll(relations); + relationsMap.put(doi.getDOI(), relationsAlreadyStored); + store.commit(); + } + }); + } + + @Override + public boolean containsKey(BibEntry entry) { + return entry + .getDOI() + .map(doi -> { + try (var store = new MVStore.Builder().fileName(path.toAbsolutePath().toString()).open()) { + MVMap> relationsMap = store.openMap(mapName, mapConfiguration); + return relationsMap.containsKey(doi.getDOI()); + } + }) + .orElse(false); + } + + private static class BibEntrySerializer extends BasicDataType { + + private final static String FIELD_SEPARATOR = "--"; + + private static String toString(BibEntry entry) { + return String.join( + FIELD_SEPARATOR, + entry.getTitle().orElse("null"), + entry.getField(StandardField.YEAR).orElse("null"), + entry.getField(StandardField.AUTHOR).orElse("null"), + entry.getType().getDisplayName(), + entry.getDOI().map(DOI::getDOI).orElse("null"), + entry.getField(StandardField.URL).orElse("null"), + entry.getField(StandardField.ABSTRACT).orElse("null") + ); + } + + private static Optional extractFieldValue(String field) { + return Objects.equals(field, "null") || field == null + ? Optional.empty() + : Optional.of(field); + } + + private static BibEntry fromString(String serializedString) { + var fields = serializedString.split(FIELD_SEPARATOR); + BibEntry entry = new BibEntry(); + extractFieldValue(fields[0]).ifPresent(title -> entry.setField(StandardField.TITLE, title)); + extractFieldValue(fields[1]).ifPresent(year -> entry.setField(StandardField.YEAR, year)); + extractFieldValue(fields[2]).ifPresent(authors -> entry.setField(StandardField.AUTHOR, authors)); + extractFieldValue(fields[3]).ifPresent(type -> entry.setType(StandardEntryType.valueOf(type))); + extractFieldValue(fields[4]).ifPresent(doi -> entry.setField(StandardField.DOI, doi)); + extractFieldValue(fields[5]).ifPresent(url -> entry.setField(StandardField.URL, url)); + extractFieldValue(fields[6]) + .ifPresent(entryAbstract -> entry.setField(StandardField.ABSTRACT, entryAbstract)); + return entry; + } + + @Override + public int getMemory(BibEntry obj) { + return toString(obj).getBytes(StandardCharsets.UTF_8).length; + } + + @Override + public void write(WriteBuffer buff, BibEntry bibEntry) { + var asBytes = toString(bibEntry).getBytes(StandardCharsets.UTF_8); + buff.putInt(asBytes.length); + buff.put(asBytes); + } + + @Override + public BibEntry read(ByteBuffer buff) { + int serializedEntrySize = buff.getInt(); + var serializedEntry = DataUtils.readString(buff, serializedEntrySize); + return fromString(serializedEntry); + } + + @Override + public int compare(BibEntry a, BibEntry b) { + if (a == null || b == null) { + throw new NullPointerException(); + } + return toString(a).compareTo(toString(b)); + } + + @Override + public BibEntry[] createStorage(int size) { + return new BibEntry[size]; + } + } + + private static class BibEntryHashSetSerializer extends BasicDataType> { + + private final BasicDataType bibEntryDataType = new BibEntrySerializer(); + + /** + * Memory size is the sum of all aggregated bibEntries memory size plus 4 bytes. + * Those 4 bytes are used to store the length of the collection itself. + * @param bibEntries should not be null + * @return total size in memory of the serialized collection of bib entries + */ + @Override + public int getMemory(LinkedHashSet bibEntries) { + return bibEntries + .stream() + .map(this.bibEntryDataType::getMemory) + .reduce(0, Integer::sum) + 4; + } + + @Override + public void write(WriteBuffer buff, LinkedHashSet bibEntries) { + buff.putInt(bibEntries.size()); + bibEntries.forEach(entry -> this.bibEntryDataType.write(buff, entry)); + } + + @Override + public LinkedHashSet read(ByteBuffer buff) { + return IntStream.range(0, buff.getInt()) + .mapToObj(it -> this.bibEntryDataType.read(buff)) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + + @Override + public LinkedHashSet[] createStorage(int size) { + return new LinkedHashSet[size]; + } + } +} diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java deleted file mode 100644 index 6631363df28..00000000000 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepository.java +++ /dev/null @@ -1,38 +0,0 @@ -package org.jabref.logic.citation.repository; - -import java.util.List; - -import org.jabref.model.entry.BibEntry; - -public class MVStoreBibEntryRelationsRepository implements BibEntryRelationsRepository { - - @Override - public void insertCitations(BibEntry entry, List citations) { - throw new UnsupportedOperationException(); - } - - @Override - public List readCitations(BibEntry entry) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsCitations(BibEntry entry) { - throw new UnsupportedOperationException(); - } - - @Override - public void insertReferences(BibEntry entry, List citations) { - throw new UnsupportedOperationException(); - } - - @Override - public List readReferences(BibEntry entry) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean containsReferences(BibEntry entry) { - throw new UnsupportedOperationException(); - } -} diff --git a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java new file mode 100644 index 00000000000..9d71799da9c --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java @@ -0,0 +1,114 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; +import java.util.random.RandomGenerator; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import org.jabref.model.citation.semanticscholar.PaperDetails; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.identifier.DOI; +import org.jabref.model.entry.types.StandardEntryType; + +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class LRUCacheBibEntryRelationsDAOTest { + + private static Stream createBibEntries() { + return IntStream + .range(0, 150) + .mapToObj(LRUCacheBibEntryRelationsDAOTest::createBibEntry); + } + + private static BibEntry createBibEntry(int i) { + return new BibEntry() + .withCitationKey(String.valueOf(i)) + .withField(StandardField.DOI, "10.1234/5678" + i); + } + + /** + * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic + * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar + * and mapped as BibEntry will be serializable by the MVStore. + * @param entry should not be null + * @return never empty + */ + private static List createRelations(BibEntry entry) { + return entry + .getCitationKey() + .map(key -> RandomGenerator.StreamableGenerator + .of("L128X256MixRandom").ints(150) + .mapToObj(i -> new BibEntry() + .withField(StandardField.TITLE, "A title:" + i) + .withField(StandardField.YEAR, String.valueOf(2024)) + .withField(StandardField.AUTHOR, "A list of authors:" + i) + .withType(StandardEntryType.Book) + .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.URL, "www.jabref.org/" + i) + .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) + ) + ) + .orElseThrow() + .collect(Collectors.toList()); + } + + private static Stream createCacheAndBibEntry() { + return Stream + .of(LRUCacheBibEntryRelationsDAO.LRUCacheBibEntryCitations.getInstance(), + LRUCacheBibEntryRelationsDAO.LRUCacheBibEntryReferences.getInstance() + ) + .flatMap(dao -> createBibEntries().map(entry -> Arguments.of(dao, entry))); + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void repositoryShouldMergeCitationsWhenInserting(LRUCacheBibEntryRelationsDAO dao, BibEntry entry) { + // GIVEN + assertFalse(dao.containsKey(entry)); + + // WHEN + var firstRelations = createRelations(entry); + var secondRelations = createRelations(entry); + dao.cacheOrMergeRelations(entry, firstRelations); + dao.cacheOrMergeRelations(entry, secondRelations); + + // THEN + var uniqueRelations = Stream + .concat(firstRelations.stream(), secondRelations.stream()) + .distinct() + .toList(); + var relationFromCache = dao.getRelations(entry); + assertTrue(dao.containsKey(entry)); + assertFalse(uniqueRelations.isEmpty()); + assertNotSame(uniqueRelations, relationFromCache); + assertEquals(uniqueRelations, relationFromCache); + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void clearingCacheShouldWork(LRUCacheBibEntryRelationsDAO dao, BibEntry entry) { + // GIVEN + dao.clearEntries(); + var relations = createRelations(entry); + assertFalse(dao.containsKey(entry)); + + // WHEN + dao.cacheOrMergeRelations(entry, relations); + assertTrue(dao.containsKey(entry)); + dao.clearEntries(); + + // THEN + assertFalse(dao.containsKey(entry)); + } +} diff --git a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java new file mode 100644 index 00000000000..ad6a175cfaf --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java @@ -0,0 +1,119 @@ +package org.jabref.logic.citation.repository; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.random.RandomGenerator; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import org.jabref.model.citation.semanticscholar.PaperDetails; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.identifier.DOI; +import org.jabref.model.entry.types.StandardEntryType; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; + +class MVStoreBibEntryRelationsRepositoryDAOTest { + + @TempDir Path temporaryFolder; + + private static Stream createBibEntries() { + return IntStream + .range(0, 150) + .mapToObj(MVStoreBibEntryRelationsRepositoryDAOTest::createBibEntry); + } + + private static BibEntry createBibEntry(int i) { + return new BibEntry() + .withCitationKey(String.valueOf(i)) + .withField(StandardField.DOI, "10.1234/5678" + i); + } + + /** + * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic + * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar + * and mapped as BibEntry will be serializable by the MVStore. + * @param entry should not be null + * @return never empty + */ + private static List createRelations(BibEntry entry) { + return entry + .getCitationKey() + .map(key -> RandomGenerator.StreamableGenerator + .of("L128X256MixRandom").ints(150) + .mapToObj(i -> new BibEntry() + .withField(StandardField.TITLE, "A title:" + i) + .withField(StandardField.YEAR, String.valueOf(2024)) + .withField(StandardField.AUTHOR, "A list of authors:" + i) + .withType(StandardEntryType.Book) + .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.URL, "www.jabref.org/" + i) + .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) + ) + ) + .orElseThrow() + .collect(Collectors.toList()); + } + + @ParameterizedTest + @MethodSource("createBibEntries") + void DAOShouldMergeRelationsWhenInserting(BibEntry bibEntry) throws IOException { + // GIVEN + var file = Files.createFile(temporaryFolder.resolve("bib_entry_relations_test_store")); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), "test-relations"); + Assertions.assertFalse(dao.containsKey(bibEntry)); + var firstRelations = createRelations(bibEntry); + var secondRelations = createRelations(bibEntry); + + // WHEN + dao.cacheOrMergeRelations(bibEntry, firstRelations); + dao.cacheOrMergeRelations(bibEntry, secondRelations); + var relationFromCache = dao.getRelations(bibEntry); + + // THEN + var uniqueRelations = Stream + .concat(firstRelations.stream(), secondRelations.stream()) + .distinct() + .toList(); + assertFalse(uniqueRelations.isEmpty()); + assertNotSame(uniqueRelations, relationFromCache); + assertEquals(uniqueRelations, relationFromCache); + } + + @ParameterizedTest + @MethodSource("createBibEntries") + void containsKeyShouldReturnFalseIfNothingWasInserted(BibEntry entry) throws IOException { + // GIVEN + var file = Files.createFile(temporaryFolder.resolve("bib_entry_relations_test_not_contains_store")); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), "test-relations"); + + // THEN + Assertions.assertFalse(dao.containsKey(entry)); + } + + @ParameterizedTest + @MethodSource("createBibEntries") + void containsKeyShouldReturnTrueIfRelationsWereInserted(BibEntry entry) throws IOException { + // GIVEN + var file = Files.createFile(temporaryFolder.resolve("bib_entry_relations_test_contains_store")); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), "test-relations"); + var relations = createRelations(entry); + + // WHEN + dao.cacheOrMergeRelations(entry, relations); + + // THEN + Assertions.assertTrue(dao.containsKey(entry)); + } +} diff --git a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java similarity index 97% rename from src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java rename to src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java index 26b299f3bfc..8c7d6ced91f 100644 --- a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java @@ -31,12 +31,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -class MVStoreBibEntryRelationsRepositoryTest { +class MVStoreBibEntryRelationsRepositoryPrototypingTest { private static Stream createBibEntries() { return IntStream .range(0, 150) - .mapToObj(MVStoreBibEntryRelationsRepositoryTest::createBibEntry); + .mapToObj(MVStoreBibEntryRelationsRepositoryPrototypingTest::createBibEntry); } private static BibEntry createBibEntry(int i) { @@ -271,4 +271,10 @@ void IWouldLikeToSaveAndLoadCitationsForABibEntryFromAStore(@TempDir Path tempor Assertions.assertEquals(entry.getValue(), citations); } } + + @Test + void test() { + var s = Stream.of(null, "test", null).collect(Collectors.joining()); + System.out.println(s); + } } From 01f6da47bdc018c5905b2c11d91ef99d0ec502cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Sun, 17 Nov 2024 21:13:27 +0100 Subject: [PATCH 07/35] MVStoreDAO implementation for citations relations (#11189): * Solve task 1 * Implementation of a DAO chain: memory cache and MVStore * Persist citations as relations to disk after a fetch * Avoid fetching data if relations are available from MVStore * Avoid reading data from MVStore if available in memory * Consume less from network, minimize disk usage --- .../CitationRelationsTab.java | 27 ++- .../repository/ChainBibEntryRelationDAO.java | 53 ++++++ .../ChainBibEntryRelationsRepository.java | 56 ++++++ .../LRUBibEntryRelationsRepository.java | 49 ------ .../LRUCacheBibEntryRelationsDAO.java | 37 +--- .../MVStoreBibEntryRelationDAO.java | 30 +++- .../ChainBibEntryRelationDAOTest.java | 159 ++++++++++++++++++ ...ChainBibEntryRelationsRepositoryTest.java} | 21 +-- .../LRUCacheBibEntryRelationsDAOTest.java | 5 +- 9 files changed, 330 insertions(+), 107 deletions(-) create mode 100644 src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java create mode 100644 src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java delete mode 100644 src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java create mode 100644 src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java rename src/test/java/org/jabref/logic/citation/repository/{LRUBibEntryRelationsRepositoryTest.java => ChainBibEntryRelationsRepositoryTest.java} (83%) diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index e4af0a3fb5e..1ead7439e22 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -1,8 +1,12 @@ package org.jabref.gui.entryeditor.citationrelationtab; +import java.io.File; import java.io.IOException; import java.io.StringWriter; import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; import java.util.List; @@ -43,8 +47,7 @@ import org.jabref.logic.bibtex.BibEntryWriter; import org.jabref.logic.bibtex.FieldPreferences; import org.jabref.logic.bibtex.FieldWriter; -import org.jabref.logic.citation.repository.LRUBibEntryRelationsCache; -import org.jabref.logic.citation.repository.LRUBibEntryRelationsRepository; +import org.jabref.logic.citation.repository.ChainBibEntryRelationsRepository; import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.database.DuplicateCheck; import org.jabref.logic.exporter.BibWriter; @@ -112,12 +115,20 @@ public CitationRelationsTab(DialogService dialogService, this.entryTypesManager = bibEntryTypesManager; this.duplicateCheck = new DuplicateCheck(entryTypesManager); - var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( - new LRUBibEntryRelationsCache() - ); - this.searchCitationsRelationsService = new SearchCitationsRelationsService( - new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), bibEntryRelationsRepository - ); + + try { + var jabRefPath = Paths.get("/home/sacha/Documents/projects/JabRef"); + var citationsPath = Path.of(jabRefPath.toAbsolutePath() + File.separator + "citations"); + var relationsPath = Path.of(jabRefPath.toAbsolutePath() + File.separator + "references"); + var bibEntryRelationsRepository = new ChainBibEntryRelationsRepository(citationsPath, relationsPath); + this.searchCitationsRelationsService = new SearchCitationsRelationsService( + new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), bibEntryRelationsRepository + ); + } catch (Exception e) { + e.printStackTrace(); + throw new RuntimeException(e); + } + citationsRelationsTabViewModel = new CitationsRelationsTabViewModel( databaseContext, preferences, diff --git a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java new file mode 100644 index 00000000000..3b802f7cada --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java @@ -0,0 +1,53 @@ +package org.jabref.logic.citation.repository; + +import java.util.List; + +import org.jabref.model.entry.BibEntry; + +public class ChainBibEntryRelationDAO implements BibEntryRelationDAO { + + private static final BibEntryRelationDAO EMPTY = new ChainBibEntryRelationDAO(null, null); + + private final BibEntryRelationDAO current; + private final BibEntryRelationDAO next; + + ChainBibEntryRelationDAO(BibEntryRelationDAO current, BibEntryRelationDAO next) { + this.current = current; + this.next = next; + } + + @Override + public List getRelations(BibEntry entry) { + if (this.current.containsKey(entry)) { + return this.current.getRelations(entry); + } + if (this.next == EMPTY) { + return List.of(); + } + var relations = this.next.getRelations(entry); + this.current.cacheOrMergeRelations(entry, relations); + // Makes sure to obtain a copy and not a direct reference to what was inserted + return this.current.getRelations(entry); + } + + @Override + public void cacheOrMergeRelations(BibEntry entry, List relations) { + if (this.next != EMPTY) { + this.next.cacheOrMergeRelations(entry, relations); + } + this.current.cacheOrMergeRelations(entry, relations); + } + + @Override + public boolean containsKey(BibEntry entry) { + return this.current.containsKey(entry) + || (this.next != EMPTY && this.next.containsKey(entry)); + } + + public static BibEntryRelationDAO of(BibEntryRelationDAO... dao) { + return List.of(dao) + .reversed() + .stream() + .reduce(EMPTY, (acc, current) -> new ChainBibEntryRelationDAO(current, acc)); + } +} diff --git a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java new file mode 100644 index 00000000000..289b08edb8f --- /dev/null +++ b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java @@ -0,0 +1,56 @@ +package org.jabref.logic.citation.repository; + +import java.nio.file.Path; +import java.util.List; +import java.util.Objects; + +import org.jabref.model.entry.BibEntry; + +public class ChainBibEntryRelationsRepository implements BibEntryRelationsRepository { + + private final BibEntryRelationDAO citationsDao; + private final BibEntryRelationDAO referencesDao; + + public ChainBibEntryRelationsRepository(Path citationsStore, Path relationsStore) { + this.citationsDao = ChainBibEntryRelationDAO.of( + LRUCacheBibEntryRelationsDAO.CITATIONS, new MVStoreBibEntryRelationDAO(citationsStore, "citations") + ); + this.referencesDao = ChainBibEntryRelationDAO.of( + LRUCacheBibEntryRelationsDAO.REFERENCES, new MVStoreBibEntryRelationDAO(relationsStore, "relations") + ); + } + + @Override + public void insertCitations(BibEntry entry, List citations) { + citationsDao.cacheOrMergeRelations( + entry, Objects.requireNonNullElseGet(citations, List::of) + ); + } + + @Override + public List readCitations(BibEntry entry) { + return citationsDao.getRelations(entry); + } + + @Override + public boolean containsCitations(BibEntry entry) { + return citationsDao.containsKey(entry); + } + + @Override + public void insertReferences(BibEntry entry, List references) { + referencesDao.cacheOrMergeRelations( + entry, Objects.requireNonNullElseGet(references, List::of) + ); + } + + @Override + public List readReferences(BibEntry entry) { + return referencesDao.getRelations(entry); + } + + @Override + public boolean containsReferences(BibEntry entry) { + return referencesDao.containsKey(entry); + } +} diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java deleted file mode 100644 index 18c360ec17e..00000000000 --- a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepository.java +++ /dev/null @@ -1,49 +0,0 @@ -package org.jabref.logic.citation.repository; - -import java.util.List; -import java.util.Objects; - -import org.jabref.model.entry.BibEntry; - -public class LRUBibEntryRelationsRepository implements BibEntryRelationsRepository { - - private final LRUBibEntryRelationsCache cache; - - public LRUBibEntryRelationsRepository(LRUBibEntryRelationsCache cache) { - this.cache = cache; - } - - @Override - public void insertCitations(BibEntry entry, List citations) { - cache.cacheOrMergeCitations( - entry, Objects.requireNonNullElseGet(citations, List::of) - ); - } - - @Override - public List readCitations(BibEntry entry) { - return cache.getCitations(entry); - } - - @Override - public boolean containsCitations(BibEntry entry) { - return cache.citationsCached(entry); - } - - @Override - public void insertReferences(BibEntry entry, List references) { - cache.cacheOrMergeReferences( - entry, Objects.requireNonNullElseGet(references, List::of) - ); - } - - @Override - public List readReferences(BibEntry entry) { - return cache.getReferences(entry); - } - - @Override - public boolean containsReferences(BibEntry entry) { - return cache.referencesCached(entry); - } -} diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java b/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java index 996e7661ecc..017b3b93cb2 100644 --- a/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java @@ -10,9 +10,16 @@ import org.eclipse.jgit.util.LRUMap; -public abstract class LRUCacheBibEntryRelationsDAO implements BibEntryRelationDAO { +import static org.jabref.logic.citation.repository.LRUCacheBibEntryRelationsDAO.Configuration.MAX_CACHED_ENTRIES; - private static final Integer MAX_CACHED_ENTRIES = 100; +public enum LRUCacheBibEntryRelationsDAO implements BibEntryRelationDAO { + + CITATIONS(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)), + REFERENCES(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)); + + public static class Configuration { + public static final int MAX_CACHED_ENTRIES = 100; + } private final Map> relationsMap; @@ -46,30 +53,4 @@ public boolean containsKey(BibEntry entry) { public void clearEntries() { this.relationsMap.clear(); } - - public static class LRUCacheBibEntryCitations extends LRUCacheBibEntryRelationsDAO { - - private final static LRUCacheBibEntryCitations CITATIONS_CACHE = new LRUCacheBibEntryCitations(); - - private LRUCacheBibEntryCitations() { - super(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)); - } - - public static LRUCacheBibEntryCitations getInstance() { - return CITATIONS_CACHE; - } - } - - public static class LRUCacheBibEntryReferences extends LRUCacheBibEntryRelationsDAO { - - private final static LRUCacheBibEntryReferences REFERENCES_CACHE = new LRUCacheBibEntryReferences(); - - private LRUCacheBibEntryReferences() { - super(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)); - } - - public static LRUCacheBibEntryReferences getInstance() { - return REFERENCES_CACHE; - } - } } diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index 7bba5251063..fe0ee41be64 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -15,7 +15,6 @@ import org.jabref.model.entry.identifier.DOI; import org.jabref.model.entry.types.StandardEntryType; -import org.h2.mvstore.DataUtils; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; import org.h2.mvstore.WriteBuffer; @@ -23,14 +22,14 @@ public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { - private final Path path; private final String mapName; + private final MVStore.Builder storeConfiguration; private final MVMap.Builder> mapConfiguration = new MVMap.Builder>().valueType(new BibEntryHashSetSerializer()); MVStoreBibEntryRelationDAO(Path path, String mapName) { - this.path = Objects.requireNonNull(path); this.mapName = mapName; + this.storeConfiguration = new MVStore.Builder().autoCommitDisabled().fileName(path.toAbsolutePath().toString()); } @Override @@ -38,7 +37,7 @@ public List getRelations(BibEntry entry) { return entry .getDOI() .map(doi -> { - try (var store = new MVStore.Builder().fileName(path.toAbsolutePath().toString()).open()) { + try (var store = this.storeConfiguration.open()) { MVMap> relationsMap = store.openMap(mapName, mapConfiguration); return relationsMap.getOrDefault(doi.getDOI(), new LinkedHashSet<>()).stream().toList(); } @@ -49,7 +48,7 @@ public List getRelations(BibEntry entry) { @Override synchronized public void cacheOrMergeRelations(BibEntry entry, List relations) { entry.getDOI().ifPresent(doi -> { - try (var store = new MVStore.Builder().fileName(path.toAbsolutePath().toString()).open()) { + try (var store = this.storeConfiguration.open()) { MVMap> relationsMap = store.openMap(mapName, mapConfiguration); var relationsAlreadyStored = relationsMap.getOrDefault(doi.getDOI(), new LinkedHashSet<>()); relationsAlreadyStored.addAll(relations); @@ -64,7 +63,7 @@ public boolean containsKey(BibEntry entry) { return entry .getDOI() .map(doi -> { - try (var store = new MVStore.Builder().fileName(path.toAbsolutePath().toString()).open()) { + try (var store = this.storeConfiguration.open()) { MVMap> relationsMap = store.openMap(mapName, mapConfiguration); return relationsMap.containsKey(doi.getDOI()); } @@ -82,7 +81,7 @@ private static String toString(BibEntry entry) { entry.getTitle().orElse("null"), entry.getField(StandardField.YEAR).orElse("null"), entry.getField(StandardField.AUTHOR).orElse("null"), - entry.getType().getDisplayName(), + entry.getType().getDisplayName() == null ? "null" : entry.getType().getDisplayName(), entry.getDOI().map(DOI::getDOI).orElse("null"), entry.getField(StandardField.URL).orElse("null"), entry.getField(StandardField.ABSTRACT).orElse("null") @@ -124,8 +123,9 @@ public void write(WriteBuffer buff, BibEntry bibEntry) { @Override public BibEntry read(ByteBuffer buff) { int serializedEntrySize = buff.getInt(); - var serializedEntry = DataUtils.readString(buff, serializedEntrySize); - return fromString(serializedEntry); + var serializedEntry = new byte[serializedEntrySize]; + buff.get(serializedEntry); + return fromString(new String(serializedEntry, StandardCharsets.UTF_8)); } @Override @@ -140,6 +140,11 @@ public int compare(BibEntry a, BibEntry b) { public BibEntry[] createStorage(int size) { return new BibEntry[size]; } + + @Override + public boolean isMemoryEstimationAllowed() { + return false; + } } private static class BibEntryHashSetSerializer extends BasicDataType> { @@ -149,6 +154,7 @@ private static class BibEntryHashSetSerializer extends BasicDataType read(ByteBuffer buff) { } @Override + @SuppressWarnings("unchecked") public LinkedHashSet[] createStorage(int size) { return new LinkedHashSet[size]; } + + @Override + public boolean isMemoryEstimationAllowed() { + return false; + } } } diff --git a/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java new file mode 100644 index 00000000000..6507c9b1d80 --- /dev/null +++ b/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java @@ -0,0 +1,159 @@ +package org.jabref.logic.citation.repository; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.random.RandomGenerator; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import org.jabref.model.citation.semanticscholar.PaperDetails; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.StandardField; +import org.jabref.model.entry.identifier.DOI; +import org.jabref.model.entry.types.StandardEntryType; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class ChainBibEntryRelationDAOTest { + + private static Stream createBibEntries() { + return IntStream + .range(0, 150) + .mapToObj(ChainBibEntryRelationDAOTest::createBibEntry); + } + + private static BibEntry createBibEntry(int i) { + return new BibEntry() + .withCitationKey(String.valueOf(i)) + .withField(StandardField.DOI, "10.1234/5678" + i); + } + + /** + * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic + * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar + * and mapped as BibEntry will be serializable by the MVStore. + * @param entry should not be null + * @return never empty + */ + private static List createRelations(BibEntry entry) { + return entry + .getCitationKey() + .map(key -> RandomGenerator.StreamableGenerator + .of("L128X256MixRandom").ints(150) + .mapToObj(i -> new BibEntry() + .withField(StandardField.TITLE, "A title:" + i) + .withField(StandardField.YEAR, String.valueOf(2024)) + .withField(StandardField.AUTHOR, "A list of authors:" + i) + .withType(StandardEntryType.Book) + .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.URL, "www.jabref.org/" + i) + .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) + ) + ) + .orElseThrow() + .collect(Collectors.toList()); + } + + private static Stream createCacheAndBibEntry() { + return Stream + .of(LRUCacheBibEntryRelationsDAO.CITATIONS, LRUCacheBibEntryRelationsDAO.REFERENCES) + .flatMap(dao -> { + dao.clearEntries(); + return createBibEntries().map(entry -> Arguments.of(dao, entry)); + }); + } + + private class DaoMock implements BibEntryRelationDAO { + + Map> table = new HashMap<>(); + + @Override + public List getRelations(BibEntry entry) { + return this.table.getOrDefault(entry, List.of()); + } + + @Override + public void cacheOrMergeRelations(BibEntry entry, List relations) { + this.table.put(entry, relations); + } + + @Override + public boolean containsKey(BibEntry entry) { + return this.table.containsKey(entry); + } + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void theChainShouldReadFromFirstNode(BibEntryRelationDAO dao, BibEntry entry) { + // GIVEN + var relations = createRelations(entry); + dao.cacheOrMergeRelations(entry, relations); + var secondDao = new DaoMock(); + var doaChain = ChainBibEntryRelationDAO.of(dao, secondDao); + + // WHEN + var relationsFromChain = doaChain.getRelations(entry); + + // THEN + Assertions.assertEquals(relations, relationsFromChain); + Assertions.assertEquals(relations, dao.getRelations(entry)); + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void theChainShouldReadFromSecondNode(BibEntryRelationDAO dao, BibEntry entry) { + // GIVEN + var relations = createRelations(entry); + dao.cacheOrMergeRelations(entry, relations); + var firstDao = new DaoMock(); + var doaChain = ChainBibEntryRelationDAO.of(firstDao, dao); + + // WHEN + var relationsFromChain = doaChain.getRelations(entry); + + // THEN + Assertions.assertEquals(relations, relationsFromChain); + Assertions.assertEquals(relations, dao.getRelations(entry)); + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void theChainShouldReadFromSecondNodeAndRecopyToFirstNode(BibEntryRelationDAO dao, BibEntry entry) { + // GIVEN + var relations = createRelations(entry); + var firstDao = new DaoMock(); + var doaChain = ChainBibEntryRelationDAO.of(firstDao, dao); + + // WHEN + doaChain.cacheOrMergeRelations(entry, relations); + var relationsFromChain = doaChain.getRelations(entry); + + // THEN + Assertions.assertEquals(relations, relationsFromChain); + Assertions.assertEquals(relations, firstDao.getRelations(entry)); + Assertions.assertEquals(relations, dao.getRelations(entry)); + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void theChainShouldContainAKeyEvenIfItWasOnlyInsertedInLastNode(BibEntryRelationDAO dao, BibEntry entry) { + // GIVEN + var relations = createRelations(entry); + var firstDao = new DaoMock(); + var doaChain = ChainBibEntryRelationDAO.of(firstDao, dao); + + // WHEN + dao.cacheOrMergeRelations(entry, relations); + Assertions.assertFalse(firstDao.containsKey(entry)); + boolean doesChainContainsTheKey = doaChain.containsKey(entry); + + // THEN + Assertions.assertTrue(doesChainContainsTheKey); + } +} diff --git a/src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java b/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepositoryTest.java similarity index 83% rename from src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java rename to src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepositoryTest.java index ee68782e2c9..d8d3bf8fbad 100644 --- a/src/test/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsRepositoryTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepositoryTest.java @@ -1,5 +1,6 @@ package org.jabref.logic.citation.repository; +import java.nio.file.Files; import java.util.List; import java.util.random.RandomGenerator; import java.util.stream.Collectors; @@ -16,12 +17,12 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotSame; -class LRUBibEntryRelationsRepositoryTest { +class ChainBibEntryRelationsRepositoryTest { private static Stream createBibEntries() { return IntStream .range(0, 150) - .mapToObj(LRUBibEntryRelationsRepositoryTest::createBibEntry); + .mapToObj(ChainBibEntryRelationsRepositoryTest::createBibEntry); } private static BibEntry createBibEntry(int i) { @@ -46,11 +47,11 @@ private static List createRelations(BibEntry entry) { @ParameterizedTest @MethodSource("createBibEntries") - void repositoryShouldMergeCitationsWhenInserting(BibEntry bibEntry) { + void repositoryShouldMergeCitationsWhenInserting(BibEntry bibEntry) throws Exception { // GIVEN - var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( - new LRUBibEntryRelationsCache() - ); + var tempDir = Files.createTempDirectory("temp"); + var mvStorePath = Files.createTempFile(tempDir, "cache", ""); + var bibEntryRelationsRepository = new ChainBibEntryRelationsRepository(mvStorePath, mvStorePath); assertFalse(bibEntryRelationsRepository.containsCitations(bibEntry)); // WHEN @@ -72,11 +73,11 @@ void repositoryShouldMergeCitationsWhenInserting(BibEntry bibEntry) { @ParameterizedTest @MethodSource("createBibEntries") - void repositoryShouldMergeReferencesWhenInserting(BibEntry bibEntry) { + void repositoryShouldMergeReferencesWhenInserting(BibEntry bibEntry) throws Exception { // GIVEN - var bibEntryRelationsRepository = new LRUBibEntryRelationsRepository( - new LRUBibEntryRelationsCache() - ); + var tempDir = Files.createTempDirectory("temp"); + var mvStorePath = Files.createTempFile(tempDir, "cache", ""); + var bibEntryRelationsRepository = new ChainBibEntryRelationsRepository(mvStorePath, mvStorePath); assertFalse(bibEntryRelationsRepository.containsReferences(bibEntry)); // WHEN diff --git a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java index 9d71799da9c..409352a78b9 100644 --- a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java @@ -65,9 +65,7 @@ private static List createRelations(BibEntry entry) { private static Stream createCacheAndBibEntry() { return Stream - .of(LRUCacheBibEntryRelationsDAO.LRUCacheBibEntryCitations.getInstance(), - LRUCacheBibEntryRelationsDAO.LRUCacheBibEntryReferences.getInstance() - ) + .of(LRUCacheBibEntryRelationsDAO.CITATIONS, LRUCacheBibEntryRelationsDAO.REFERENCES) .flatMap(dao -> createBibEntries().map(entry -> Arguments.of(dao, entry))); } @@ -75,6 +73,7 @@ private static Stream createCacheAndBibEntry() { @MethodSource("createCacheAndBibEntry") void repositoryShouldMergeCitationsWhenInserting(LRUCacheBibEntryRelationsDAO dao, BibEntry entry) { // GIVEN + dao.clearEntries(); assertFalse(dao.containsKey(entry)); // WHEN From 337780d2597693ce1ddc7009a611f3b40c78b5ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Sun, 24 Nov 2024 21:51:19 +0100 Subject: [PATCH 08/35] Implement a search lock mechanism for citations relations (#11189): * Solve part of task 2: make impossible to force a search on a BibEntry over a week since last insertion * The MVStoreDAO search lock is based on a timestamp map (doi -> lastInsertionDate) * All time computation are based on UTC * The LRU cache will always return true -> the computer could stay up during a week, leaving cache in memory --- .../SearchCitationsRelationsService.java | 6 +- .../repository/BibEntryRelationDAO.java | 4 ++ ...DAO.java => BibEntryRelationDAOChain.java} | 14 +++-- .../BibEntryRelationsRepository.java | 4 ++ .../ChainBibEntryRelationsRepository.java | 14 ++++- .../repository/LRUBibEntryRelationsCache.java | 57 ------------------ .../MVStoreBibEntryRelationDAO.java | 44 +++++++++++++- .../SearchCitationsRelationsServiceTest.java | 3 +- ...java => BibEntryRelationDAOChainTest.java} | 44 ++++++++++---- ...ntryRelationsRepositoryHelpersForTest.java | 20 +++++++ ...oreBibEntryRelationsRepositoryDAOTest.java | 59 +++++++++++++++++-- 11 files changed, 184 insertions(+), 85 deletions(-) rename src/main/java/org/jabref/logic/citation/repository/{ChainBibEntryRelationDAO.java => BibEntryRelationDAOChain.java} (74%) delete mode 100644 src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java rename src/test/java/org/jabref/logic/citation/repository/{ChainBibEntryRelationDAOTest.java => BibEntryRelationDAOChainTest.java} (79%) diff --git a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java index 11d041abb7f..7a51b686077 100644 --- a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java +++ b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java @@ -26,7 +26,8 @@ public SearchCitationsRelationsService( } public List searchReferences(BibEntry referencer, boolean forceUpdate) { - if (forceUpdate || !this.relationsRepository.containsReferences(referencer)) { + if ((forceUpdate && this.relationsRepository.isReferencesUpdatable(referencer)) + || !this.relationsRepository.containsReferences(referencer)) { try { var references = this.citationFetcher.searchCiting(referencer); if (!references.isEmpty()) { @@ -40,7 +41,8 @@ public List searchReferences(BibEntry referencer, boolean forceUpdate) } public List searchCitations(BibEntry cited, boolean forceUpdate) { - if (forceUpdate || !this.relationsRepository.containsCitations(cited)) { + if ((forceUpdate && this.relationsRepository.isCitationsUpdatable(cited)) + || !this.relationsRepository.containsCitations(cited)) { try { var citations = this.citationFetcher.searchCitedBy(cited); if (!citations.isEmpty()) { diff --git a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java index e56b08f4d3f..883a4566a3c 100644 --- a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAO.java @@ -11,4 +11,8 @@ public interface BibEntryRelationDAO { void cacheOrMergeRelations(BibEntry entry, List relations); boolean containsKey(BibEntry entry); + + default boolean isUpdatable(BibEntry entry) { + return true; + } } diff --git a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChain.java similarity index 74% rename from src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java rename to src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChain.java index 3b802f7cada..2445e13a6fc 100644 --- a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChain.java @@ -4,14 +4,14 @@ import org.jabref.model.entry.BibEntry; -public class ChainBibEntryRelationDAO implements BibEntryRelationDAO { +public class BibEntryRelationDAOChain implements BibEntryRelationDAO { - private static final BibEntryRelationDAO EMPTY = new ChainBibEntryRelationDAO(null, null); + private static final BibEntryRelationDAO EMPTY = new BibEntryRelationDAOChain(null, null); private final BibEntryRelationDAO current; private final BibEntryRelationDAO next; - ChainBibEntryRelationDAO(BibEntryRelationDAO current, BibEntryRelationDAO next) { + BibEntryRelationDAOChain(BibEntryRelationDAO current, BibEntryRelationDAO next) { this.current = current; this.next = next; } @@ -44,10 +44,16 @@ public boolean containsKey(BibEntry entry) { || (this.next != EMPTY && this.next.containsKey(entry)); } + @Override + public boolean isUpdatable(BibEntry entry) { + return this.current.isUpdatable(entry) + && (this.next == EMPTY || this.next.isUpdatable(entry)); + } + public static BibEntryRelationDAO of(BibEntryRelationDAO... dao) { return List.of(dao) .reversed() .stream() - .reduce(EMPTY, (acc, current) -> new ChainBibEntryRelationDAO(current, acc)); + .reduce(EMPTY, (acc, current) -> new BibEntryRelationDAOChain(current, acc)); } } diff --git a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java index 84b4c73a1d7..cb99b791cad 100644 --- a/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/BibEntryRelationsRepository.java @@ -12,9 +12,13 @@ public interface BibEntryRelationsRepository { boolean containsCitations(BibEntry entry); + boolean isCitationsUpdatable(BibEntry entry); + void insertReferences(BibEntry entry, List citations); List readReferences(BibEntry entry); boolean containsReferences(BibEntry entry); + + boolean isReferencesUpdatable(BibEntry entry); } diff --git a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java index 289b08edb8f..14d26308d98 100644 --- a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java @@ -12,10 +12,10 @@ public class ChainBibEntryRelationsRepository implements BibEntryRelationsReposi private final BibEntryRelationDAO referencesDao; public ChainBibEntryRelationsRepository(Path citationsStore, Path relationsStore) { - this.citationsDao = ChainBibEntryRelationDAO.of( + this.citationsDao = BibEntryRelationDAOChain.of( LRUCacheBibEntryRelationsDAO.CITATIONS, new MVStoreBibEntryRelationDAO(citationsStore, "citations") ); - this.referencesDao = ChainBibEntryRelationDAO.of( + this.referencesDao = BibEntryRelationDAOChain.of( LRUCacheBibEntryRelationsDAO.REFERENCES, new MVStoreBibEntryRelationDAO(relationsStore, "relations") ); } @@ -37,6 +37,11 @@ public boolean containsCitations(BibEntry entry) { return citationsDao.containsKey(entry); } + @Override + public boolean isCitationsUpdatable(BibEntry entry) { + return citationsDao.isUpdatable(entry); + } + @Override public void insertReferences(BibEntry entry, List references) { referencesDao.cacheOrMergeRelations( @@ -53,4 +58,9 @@ public List readReferences(BibEntry entry) { public boolean containsReferences(BibEntry entry) { return referencesDao.containsKey(entry); } + + @Override + public boolean isReferencesUpdatable(BibEntry entry) { + return referencesDao.isUpdatable(entry); + } } diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java b/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java deleted file mode 100644 index f87455fc033..00000000000 --- a/src/main/java/org/jabref/logic/citation/repository/LRUBibEntryRelationsCache.java +++ /dev/null @@ -1,57 +0,0 @@ -package org.jabref.logic.citation.repository; - -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.identifier.DOI; - -import org.eclipse.jgit.util.LRUMap; - -public class LRUBibEntryRelationsCache { - private static final Integer MAX_CACHED_ENTRIES = 100; - private static final Map> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); - private static final Map> REFERENCES_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES); - - public List getCitations(BibEntry entry) { - return entry - .getDOI() - .stream() - .flatMap(doi -> CITATIONS_MAP.getOrDefault(doi, Set.of()).stream()) - .toList(); - } - - public List getReferences(BibEntry entry) { - return entry - .getDOI() - .stream() - .flatMap(doi -> REFERENCES_MAP.getOrDefault(doi, Set.of()).stream()) - .toList(); - } - - public void cacheOrMergeCitations(BibEntry entry, List citations) { - entry.getDOI().ifPresent(doi -> { - var cachedRelations = CITATIONS_MAP.getOrDefault(doi, new LinkedHashSet<>()); - cachedRelations.addAll(citations); - CITATIONS_MAP.put(doi, cachedRelations); - }); - } - - public void cacheOrMergeReferences(BibEntry entry, List references) { - entry.getDOI().ifPresent(doi -> { - var cachedRelations = REFERENCES_MAP.getOrDefault(doi, new LinkedHashSet<>()); - cachedRelations.addAll(references); - REFERENCES_MAP.put(doi, cachedRelations); - }); - } - - public boolean citationsCached(BibEntry entry) { - return entry.getDOI().map(CITATIONS_MAP::containsKey).orElse(false); - } - - public boolean referencesCached(BibEntry entry) { - return entry.getDOI().map(REFERENCES_MAP::containsKey).orElse(false); - } -} diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index fe0ee41be64..8bf4f0473da 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -3,18 +3,22 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.nio.file.Path; +import java.time.Clock; +import java.time.ZoneId; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.IntStream; +import java.time.LocalDateTime; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.StandardField; import org.jabref.model.entry.identifier.DOI; import org.jabref.model.entry.types.StandardEntryType; +import com.google.common.annotations.VisibleForTesting; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; import org.h2.mvstore.WriteBuffer; @@ -22,13 +26,17 @@ public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { + private final static ZoneId TIME_STAMP_ZONE_ID = ZoneId.of("UTC"); + private final String mapName; + private final String insertionTimeStampMapName; private final MVStore.Builder storeConfiguration; private final MVMap.Builder> mapConfiguration = new MVMap.Builder>().valueType(new BibEntryHashSetSerializer()); MVStoreBibEntryRelationDAO(Path path, String mapName) { this.mapName = mapName; + this.insertionTimeStampMapName = mapName + "-insertion-timestamp"; this.storeConfiguration = new MVStore.Builder().autoCommitDisabled().fileName(path.toAbsolutePath().toString()); } @@ -47,19 +55,30 @@ public List getRelations(BibEntry entry) { @Override synchronized public void cacheOrMergeRelations(BibEntry entry, List relations) { + if (relations.isEmpty()) { + return; + } entry.getDOI().ifPresent(doi -> { try (var store = this.storeConfiguration.open()) { + // Save the relations MVMap> relationsMap = store.openMap(mapName, mapConfiguration); var relationsAlreadyStored = relationsMap.getOrDefault(doi.getDOI(), new LinkedHashSet<>()); relationsAlreadyStored.addAll(relations); relationsMap.put(doi.getDOI(), relationsAlreadyStored); + + // Save insertion timestamp + var insertionTime = LocalDateTime.now(TIME_STAMP_ZONE_ID); + MVMap insertionTimeStampMap = store.openMap(insertionTimeStampMapName); + insertionTimeStampMap.put(doi.getDOI(), insertionTime); + + // Commit store.commit(); } }); } @Override - public boolean containsKey(BibEntry entry) { + synchronized public boolean containsKey(BibEntry entry) { return entry .getDOI() .map(doi -> { @@ -71,6 +90,29 @@ public boolean containsKey(BibEntry entry) { .orElse(false); } + @Override + synchronized public boolean isUpdatable(BibEntry entry) { + var clock = Clock.system(TIME_STAMP_ZONE_ID); + return this.isUpdatable(entry, clock); + } + + @VisibleForTesting + boolean isUpdatable(final BibEntry entry, final Clock clock) { + final var executionTime = LocalDateTime.now(clock); + return entry + .getDOI() + .map(doi -> { + try (var store = this.storeConfiguration.open()) { + MVMap insertionTimeStampMap = store.openMap(insertionTimeStampMapName); + return insertionTimeStampMap.getOrDefault(doi.getDOI(), executionTime); + } + }) + .map(lastExecutionTime -> + lastExecutionTime.equals(executionTime) || lastExecutionTime.isBefore(executionTime.minusWeeks(1)) + ) + .orElse(true); + } + private static class BibEntrySerializer extends BasicDataType { private final static String FIELD_SEPARATOR = "--"; diff --git a/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java index 25097ed4b87..1d1f1fbbf8f 100644 --- a/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java +++ b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java @@ -1,9 +1,8 @@ -package org.jabref.logic.citation.service; +package org.jabref.logic.citation; import java.util.HashMap; import java.util.List; -import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.citation.repository.BibEntryRelationsRepositoryHelpersForTest; import org.jabref.logic.importer.fetcher.CitationFetcherHelpersForTest; import org.jabref.model.entry.BibEntry; diff --git a/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java similarity index 79% rename from src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java rename to src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java index 6507c9b1d80..d83156f0745 100644 --- a/src/test/java/org/jabref/logic/citation/repository/ChainBibEntryRelationDAOTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java @@ -19,12 +19,12 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -class ChainBibEntryRelationDAOTest { +class BibEntryRelationDAOChainTest { private static Stream createBibEntries() { return IntStream .range(0, 150) - .mapToObj(ChainBibEntryRelationDAOTest::createBibEntry); + .mapToObj(BibEntryRelationDAOChainTest::createBibEntry); } private static BibEntry createBibEntry(int i) { @@ -37,6 +37,7 @@ private static BibEntry createBibEntry(int i) { * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar * and mapped as BibEntry will be serializable by the MVStore. + * * @param entry should not be null * @return never empty */ @@ -68,7 +69,7 @@ private static Stream createCacheAndBibEntry() { }); } - private class DaoMock implements BibEntryRelationDAO { + private static class DaoMock implements BibEntryRelationDAO { Map> table = new HashMap<>(); @@ -86,6 +87,11 @@ public void cacheOrMergeRelations(BibEntry entry, List relations) { public boolean containsKey(BibEntry entry) { return this.table.containsKey(entry); } + + @Override + public boolean isUpdatable(BibEntry entry) { + return !this.containsKey(entry); + } } @ParameterizedTest @@ -95,7 +101,7 @@ void theChainShouldReadFromFirstNode(BibEntryRelationDAO dao, BibEntry entry) { var relations = createRelations(entry); dao.cacheOrMergeRelations(entry, relations); var secondDao = new DaoMock(); - var doaChain = ChainBibEntryRelationDAO.of(dao, secondDao); + var doaChain = BibEntryRelationDAOChain.of(dao, secondDao); // WHEN var relationsFromChain = doaChain.getRelations(entry); @@ -112,7 +118,7 @@ void theChainShouldReadFromSecondNode(BibEntryRelationDAO dao, BibEntry entry) { var relations = createRelations(entry); dao.cacheOrMergeRelations(entry, relations); var firstDao = new DaoMock(); - var doaChain = ChainBibEntryRelationDAO.of(firstDao, dao); + var doaChain = BibEntryRelationDAOChain.of(firstDao, dao); // WHEN var relationsFromChain = doaChain.getRelations(entry); @@ -128,7 +134,7 @@ void theChainShouldReadFromSecondNodeAndRecopyToFirstNode(BibEntryRelationDAO da // GIVEN var relations = createRelations(entry); var firstDao = new DaoMock(); - var doaChain = ChainBibEntryRelationDAO.of(firstDao, dao); + var doaChain = BibEntryRelationDAOChain.of(firstDao, dao); // WHEN doaChain.cacheOrMergeRelations(entry, relations); @@ -142,18 +148,34 @@ void theChainShouldReadFromSecondNodeAndRecopyToFirstNode(BibEntryRelationDAO da @ParameterizedTest @MethodSource("createCacheAndBibEntry") - void theChainShouldContainAKeyEvenIfItWasOnlyInsertedInLastNode(BibEntryRelationDAO dao, BibEntry entry) { + void theChainShouldContainAKeyEvenIfItWasOnlyInsertedInLastNode(BibEntryRelationDAO secondDao, BibEntry entry) { // GIVEN var relations = createRelations(entry); var firstDao = new DaoMock(); - var doaChain = ChainBibEntryRelationDAO.of(firstDao, dao); + var doaChain = BibEntryRelationDAOChain.of(firstDao, secondDao); // WHEN - dao.cacheOrMergeRelations(entry, relations); + secondDao.cacheOrMergeRelations(entry, relations); + + // THEN Assertions.assertFalse(firstDao.containsKey(entry)); - boolean doesChainContainsTheKey = doaChain.containsKey(entry); + Assertions.assertTrue(doaChain.containsKey(entry)); + } + + @ParameterizedTest + @MethodSource("createCacheAndBibEntry") + void theChainShouldNotBeUpdatableBeforeInsertionAndNotAfterAnInsertion(BibEntryRelationDAO dao, BibEntry entry) { + // GIVEN + var relations = createRelations(entry); + var lastDao = new DaoMock(); + var daoChain = BibEntryRelationDAOChain.of(dao, lastDao); + Assertions.assertTrue(daoChain.isUpdatable(entry)); + + // WHEN + daoChain.cacheOrMergeRelations(entry, relations); // THEN - Assertions.assertTrue(doesChainContainsTheKey); + Assertions.assertTrue(daoChain.containsKey(entry)); + Assertions.assertFalse(daoChain.isUpdatable(entry)); } } diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java index f2305b38b33..c829ed188b7 100644 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java @@ -31,6 +31,11 @@ public boolean containsCitations(BibEntry entry) { return true; } + @Override + public boolean isCitationsUpdatable(BibEntry entry) { + return false; + } + @Override public void insertReferences(BibEntry entry, List citations) { insertReferences.accept(entry, citations); @@ -45,6 +50,11 @@ public List readReferences(BibEntry entry) { public boolean containsReferences(BibEntry entry) { return true; } + + @Override + public boolean isReferencesUpdatable(BibEntry entry) { + return false; + } }; } @@ -67,6 +77,11 @@ public boolean containsCitations(BibEntry entry) { return citationsDB.containsKey(entry); } + @Override + public boolean isCitationsUpdatable(BibEntry entry) { + return false; + } + @Override public void insertReferences(BibEntry entry, List citations) { referencesDB.put(entry, citations); @@ -81,6 +96,11 @@ public List readReferences(BibEntry entry) { public boolean containsReferences(BibEntry entry) { return referencesDB.containsKey(entry); } + + @Override + public boolean isReferencesUpdatable(BibEntry entry) { + return false; + } }; } } diff --git a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java index ad6a175cfaf..1816ed10096 100644 --- a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java @@ -3,6 +3,11 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Clock; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.time.ZoneOffset; import java.util.List; import java.util.random.RandomGenerator; import java.util.stream.Collectors; @@ -26,6 +31,9 @@ class MVStoreBibEntryRelationsRepositoryDAOTest { + private final static String TEMPORARY_FOLDER_NAME = "bib_entry_relations_test_not_contains_store"; + private final static String MAP_NAME = "test-relations"; + @TempDir Path temporaryFolder; private static Stream createBibEntries() { @@ -44,6 +52,7 @@ private static BibEntry createBibEntry(int i) { * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar * and mapped as BibEntry will be serializable by the MVStore. + * * @param entry should not be null * @return never empty */ @@ -70,8 +79,8 @@ private static List createRelations(BibEntry entry) { @MethodSource("createBibEntries") void DAOShouldMergeRelationsWhenInserting(BibEntry bibEntry) throws IOException { // GIVEN - var file = Files.createFile(temporaryFolder.resolve("bib_entry_relations_test_store")); - var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), "test-relations"); + var file = Files.createFile(temporaryFolder.resolve(TEMPORARY_FOLDER_NAME)); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), MAP_NAME); Assertions.assertFalse(dao.containsKey(bibEntry)); var firstRelations = createRelations(bibEntry); var secondRelations = createRelations(bibEntry); @@ -95,8 +104,8 @@ void DAOShouldMergeRelationsWhenInserting(BibEntry bibEntry) throws IOException @MethodSource("createBibEntries") void containsKeyShouldReturnFalseIfNothingWasInserted(BibEntry entry) throws IOException { // GIVEN - var file = Files.createFile(temporaryFolder.resolve("bib_entry_relations_test_not_contains_store")); - var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), "test-relations"); + var file = Files.createFile(temporaryFolder.resolve(TEMPORARY_FOLDER_NAME)); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), MAP_NAME); // THEN Assertions.assertFalse(dao.containsKey(entry)); @@ -106,8 +115,8 @@ void containsKeyShouldReturnFalseIfNothingWasInserted(BibEntry entry) throws IOE @MethodSource("createBibEntries") void containsKeyShouldReturnTrueIfRelationsWereInserted(BibEntry entry) throws IOException { // GIVEN - var file = Files.createFile(temporaryFolder.resolve("bib_entry_relations_test_contains_store")); - var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), "test-relations"); + var file = Files.createFile(temporaryFolder.resolve(TEMPORARY_FOLDER_NAME)); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), MAP_NAME); var relations = createRelations(entry); // WHEN @@ -116,4 +125,42 @@ void containsKeyShouldReturnTrueIfRelationsWereInserted(BibEntry entry) throws I // THEN Assertions.assertTrue(dao.containsKey(entry)); } + + @ParameterizedTest + @MethodSource("createBibEntries") + void isUpdatableShouldReturnTrueBeforeInsertionsAndFalseAfterInsertions(BibEntry entry) throws IOException { + // GIVEN + var file = Files.createFile(temporaryFolder.resolve(TEMPORARY_FOLDER_NAME)); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), MAP_NAME); + var relations = createRelations(entry); + Assertions.assertTrue(dao.isUpdatable(entry)); + + // WHEN + dao.cacheOrMergeRelations(entry, relations); + + // THEN + Assertions.assertFalse(dao.isUpdatable(entry)); + } + + @ParameterizedTest + @MethodSource("createBibEntries") + void isUpdatableShouldReturnTrueAfterOneWeek(BibEntry entry) throws IOException { + // GIVEN + var file = Files.createFile(temporaryFolder.resolve(TEMPORARY_FOLDER_NAME)); + var dao = new MVStoreBibEntryRelationDAO(file.toAbsolutePath(), MAP_NAME); + var relations = createRelations(entry); + var clock = Clock.fixed(Instant.now(), ZoneId.of("UTC")); + Assertions.assertTrue(dao.isUpdatable(entry, clock)); + + // WHEN + dao.cacheOrMergeRelations(entry, relations); + + // THEN + Assertions.assertFalse(dao.isUpdatable(entry, clock)); + var clockOneWeekAfter = Clock.fixed( + LocalDateTime.now(ZoneId.of("UTC")).plusWeeks(1).toInstant(ZoneOffset.UTC), + ZoneId.of("UTC") + ); + Assertions.assertTrue(dao.isUpdatable(entry, clockOneWeekAfter)); + } } From 7c2e32d2e62be0635a83169646d2e7701211f721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Wed, 27 Nov 2024 22:52:28 +0100 Subject: [PATCH 09/35] Avoid user to force update citation relations even the fetcher returned an empty list * Solve completely Task 2: make impossible to force a search on a BibEntry over a week since last insertion --- .../SearchCitationsRelationsService.java | 8 +--- .../MVStoreBibEntryRelationDAO.java | 9 ++-- .../SearchCitationsRelationsServiceTest.java | 44 ++++++++++++++++++- ...ntryRelationsRepositoryHelpersForTest.java | 8 ++-- 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java index 7a51b686077..30b655bdde1 100644 --- a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java +++ b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java @@ -30,9 +30,7 @@ public List searchReferences(BibEntry referencer, boolean forceUpdate) || !this.relationsRepository.containsReferences(referencer)) { try { var references = this.citationFetcher.searchCiting(referencer); - if (!references.isEmpty()) { - this.relationsRepository.insertReferences(referencer, references); - } + this.relationsRepository.insertReferences(referencer, references); } catch (FetcherException e) { LOGGER.error("Error while fetching references for entry {}", referencer.getTitle(), e); } @@ -45,9 +43,7 @@ public List searchCitations(BibEntry cited, boolean forceUpdate) { || !this.relationsRepository.containsCitations(cited)) { try { var citations = this.citationFetcher.searchCitedBy(cited); - if (!citations.isEmpty()) { - this.relationsRepository.insertCitations(cited, citations); - } + this.relationsRepository.insertCitations(cited, citations); } catch (FetcherException e) { LOGGER.error("Error while fetching citations for entry {}", cited.getTitle(), e); } diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index 8bf4f0473da..dd46f6f654b 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -53,11 +53,14 @@ public List getRelations(BibEntry entry) { .orElse(List.of()); } + /** + * Allows insertion of empty list in order to keep track of insertion date for an entry. + * + * @param entry should not be null + * @param relations should not be null + */ @Override synchronized public void cacheOrMergeRelations(BibEntry entry, List relations) { - if (relations.isEmpty()) { - return; - } entry.getDOI().ifPresent(doi -> { try (var store = this.storeConfiguration.open()) { // Save the relations diff --git a/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java index 1d1f1fbbf8f..2e63294f667 100644 --- a/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java +++ b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java @@ -95,6 +95,27 @@ void serviceShouldFetchCitationsIfRepositoryIsEmpty() { assertEquals(citationsToReturn, citationsDatabase.get(cited)); assertEquals(citationsToReturn, citations); } + + @Test + void insertingAnEmptyCitationsShouldBePossible() { + var cited = new BibEntry(); + var citationsDatabase = new HashMap>(); + var fetcher = CitationFetcherHelpersForTest.Mocks.from( + entry -> List.of(), null + ); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + citationsDatabase, null + ); + var searchService = new SearchCitationsRelationsService(fetcher, repository); + + // WHEN + var citations = searchService.searchCitations(cited, false); + + // THEN + assertTrue(citations.isEmpty()); + assertTrue(citationsDatabase.containsKey(cited)); + assertTrue(citationsDatabase.get(cited).isEmpty()); + } } @Nested @@ -117,7 +138,7 @@ void serviceShouldSearchForReferences() { } @Test - void serviceShouldCallTheFetcherForReferencesIWhenForceUpdateIsTrue() { + void serviceShouldCallTheFetcherForReferencesWhenForceUpdateIsTrue() { // GIVEN var referencer = new BibEntry(); var newReference = new BibEntry(); @@ -174,5 +195,26 @@ void serviceShouldFetchReferencesIfRepositoryIsEmpty() { assertEquals(referencesToReturn, referencesDatabase.get(reference)); assertEquals(referencesToReturn, references); } + + @Test + void insertingAnEmptyReferencesShouldBePossible() { + var referencer = new BibEntry(); + var referenceDatabase = new HashMap>(); + var fetcher = CitationFetcherHelpersForTest.Mocks.from( + null, entry -> List.of() + ); + var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( + null, referenceDatabase + ); + var searchService = new SearchCitationsRelationsService(fetcher, repository); + + // WHEN + var citations = searchService.searchReferences(referencer, false); + + // THEN + assertTrue(citations.isEmpty()); + assertTrue(referenceDatabase.containsKey(referencer)); + assertTrue(referenceDatabase.get(referencer).isEmpty()); + } } } diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java index c829ed188b7..14087295e63 100644 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java @@ -33,7 +33,7 @@ public boolean containsCitations(BibEntry entry) { @Override public boolean isCitationsUpdatable(BibEntry entry) { - return false; + return true; } @Override @@ -53,7 +53,7 @@ public boolean containsReferences(BibEntry entry) { @Override public boolean isReferencesUpdatable(BibEntry entry) { - return false; + return true; } }; } @@ -79,7 +79,7 @@ public boolean containsCitations(BibEntry entry) { @Override public boolean isCitationsUpdatable(BibEntry entry) { - return false; + return true; } @Override @@ -99,7 +99,7 @@ public boolean containsReferences(BibEntry entry) { @Override public boolean isReferencesUpdatable(BibEntry entry) { - return false; + return true; } }; } From 187b5d435644995ea43196d90ada41451e66fb80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Cr=C3=A9mieux?= Date: Sun, 8 Dec 2024 22:54:52 +0100 Subject: [PATCH 10/35] Make the citation relations update automatic after the guard delay is exhausted * Remove the isForceUpdate boolean * User is still able to trigger the fetch if an error occurs --- .../CitationRelationsTab.java | 19 ++++++----- .../SearchCitationsRelationsService.java | 14 ++++---- .../LRUCacheBibEntryRelationsDAO.java | 2 +- .../SearchCitationsRelationsServiceTest.java | 32 +++++++++++-------- ...ntryRelationsRepositoryHelpersForTest.java | 8 +++-- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index 1ead7439e22..8761224d060 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -4,7 +4,6 @@ import java.io.IOException; import java.io.StringWriter; import java.net.URI; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; @@ -213,11 +212,11 @@ private SplitPane getPaneAndStartSearch(BibEntry entry) { refreshCitingButton.setOnMouseClicked(event -> { searchForRelations(entry, citingListView, abortCitingButton, - refreshCitingButton, CitationFetcher.SearchType.CITES, importCitingButton, citingProgress, true); + refreshCitingButton, CitationFetcher.SearchType.CITES, importCitingButton, citingProgress); }); refreshCitedByButton.setOnMouseClicked(event -> searchForRelations(entry, citedByListView, abortCitedButton, - refreshCitedByButton, CitationFetcher.SearchType.CITED_BY, importCitedByButton, citedByProgress, true)); + refreshCitedByButton, CitationFetcher.SearchType.CITED_BY, importCitedByButton, citedByProgress)); // Create SplitPane to hold all nodes above SplitPane container = new SplitPane(citingVBox, citedByVBox); @@ -225,10 +224,10 @@ private SplitPane getPaneAndStartSearch(BibEntry entry) { styleFetchedListView(citingListView); searchForRelations(entry, citingListView, abortCitingButton, refreshCitingButton, - CitationFetcher.SearchType.CITES, importCitingButton, citingProgress, false); + CitationFetcher.SearchType.CITES, importCitingButton, citingProgress); searchForRelations(entry, citedByListView, abortCitedButton, refreshCitedByButton, - CitationFetcher.SearchType.CITED_BY, importCitedByButton, citedByProgress, false); + CitationFetcher.SearchType.CITED_BY, importCitedByButton, citedByProgress); return container; } @@ -411,7 +410,7 @@ protected void bindToEntry(BibEntry entry) { */ private void searchForRelations(BibEntry entry, CheckListView listView, Button abortButton, Button refreshButton, CitationFetcher.SearchType searchType, Button importButton, - ProgressIndicator progress, boolean shouldRefresh) { + ProgressIndicator progress) { if (entry.getDOI().isEmpty()) { hideNodes(abortButton, progress); showNodes(refreshButton); @@ -432,7 +431,7 @@ private void searchForRelations(BibEntry entry, CheckListView prepareToSearchForRelations( abortButton, refreshButton, importButton, progress, task )) @@ -462,18 +461,18 @@ private void searchForRelations(BibEntry entry, CheckListView> createBackGroundTask( - BibEntry entry, CitationFetcher.SearchType searchType, boolean shouldRefresh + BibEntry entry, CitationFetcher.SearchType searchType ) { return switch (searchType) { case CitationFetcher.SearchType.CITES -> { citingTask = BackgroundTask.wrap( - () -> this.searchCitationsRelationsService.searchReferences(entry, shouldRefresh) + () -> this.searchCitationsRelationsService.searchReferences(entry) ); yield citingTask; } case CitationFetcher.SearchType.CITED_BY -> { citedByTask = BackgroundTask.wrap( - () -> this.searchCitationsRelationsService.searchCitations(entry, shouldRefresh) + () -> this.searchCitationsRelationsService.searchCitations(entry) ); yield citedByTask; } diff --git a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java index 30b655bdde1..0e800ca1077 100644 --- a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java +++ b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java @@ -25,9 +25,10 @@ public SearchCitationsRelationsService( this.relationsRepository = repository; } - public List searchReferences(BibEntry referencer, boolean forceUpdate) { - if ((forceUpdate && this.relationsRepository.isReferencesUpdatable(referencer)) - || !this.relationsRepository.containsReferences(referencer)) { + public List searchReferences(BibEntry referencer) { + boolean isFetchingAllowed = this.relationsRepository.isReferencesUpdatable(referencer) + || !this.relationsRepository.containsReferences(referencer); + if (isFetchingAllowed) { try { var references = this.citationFetcher.searchCiting(referencer); this.relationsRepository.insertReferences(referencer, references); @@ -38,9 +39,10 @@ public List searchReferences(BibEntry referencer, boolean forceUpdate) return this.relationsRepository.readReferences(referencer); } - public List searchCitations(BibEntry cited, boolean forceUpdate) { - if ((forceUpdate && this.relationsRepository.isCitationsUpdatable(cited)) - || !this.relationsRepository.containsCitations(cited)) { + public List searchCitations(BibEntry cited) { + boolean isFetchingAllowed = this.relationsRepository.isCitationsUpdatable(cited) + || !this.relationsRepository.containsCitations(cited); + if (isFetchingAllowed) { try { var citations = this.citationFetcher.searchCitedBy(cited); this.relationsRepository.insertCitations(cited, citations); diff --git a/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java b/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java index 017b3b93cb2..3b753dc92c0 100644 --- a/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAO.java @@ -18,7 +18,7 @@ public enum LRUCacheBibEntryRelationsDAO implements BibEntryRelationDAO { REFERENCES(new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES)); public static class Configuration { - public static final int MAX_CACHED_ENTRIES = 100; + public static final int MAX_CACHED_ENTRIES = 128; // Let's use a power of two for sizing } private final Map> relationsMap; diff --git a/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java index 2e63294f667..1e15af113f7 100644 --- a/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java +++ b/src/test/java/org/jabref/logic/citation/SearchCitationsRelationsServiceTest.java @@ -23,19 +23,19 @@ void serviceShouldSearchForCitations() { var cited = new BibEntry(); var citationsToReturn = List.of(new BibEntry()); var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( - e -> citationsToReturn, null, null, null + e -> citationsToReturn, null, null, null, entry -> false, entry -> false ); var searchService = new SearchCitationsRelationsService(null, repository); // WHEN - List citations = searchService.searchCitations(cited, false); + List citations = searchService.searchCitations(cited); // THEN assertEquals(citationsToReturn, citations); } @Test - void serviceShouldForceCitationsUpdate() { + void serviceShouldCallTheFetcherForCitationsWhenRepositoryIsUpdatable() { // GiVEN var cited = new BibEntry(); var newCitations = new BibEntry(); @@ -54,12 +54,14 @@ void serviceShouldForceCitationsUpdate() { e -> citationsToReturn, citationsDatabase::put, List::of, - (e, r) -> { } + (e, r) -> { }, + e -> true, + e -> false ); var searchService = new SearchCitationsRelationsService(fetcher, repository); // WHEN - var citations = searchService.searchCitations(cited, true); + var citations = searchService.searchCitations(cited); // THEN assertTrue(citationsDatabase.containsKey(cited)); @@ -88,7 +90,7 @@ void serviceShouldFetchCitationsIfRepositoryIsEmpty() { var searchService = new SearchCitationsRelationsService(fetcher, repository); // WHEN - var citations = searchService.searchCitations(cited, false); + var citations = searchService.searchCitations(cited); // THEN assertTrue(citationsDatabase.containsKey(cited)); @@ -109,7 +111,7 @@ void insertingAnEmptyCitationsShouldBePossible() { var searchService = new SearchCitationsRelationsService(fetcher, repository); // WHEN - var citations = searchService.searchCitations(cited, false); + var citations = searchService.searchCitations(cited); // THEN assertTrue(citations.isEmpty()); @@ -126,19 +128,19 @@ void serviceShouldSearchForReferences() { var referencer = new BibEntry(); var referencesToReturn = List.of(new BibEntry()); var repository = BibEntryRelationsRepositoryHelpersForTest.Mocks.from( - null, null, e -> referencesToReturn, null + null, null, e -> referencesToReturn, null, e -> false, e -> false ); var searchService = new SearchCitationsRelationsService(null, repository); // WHEN - List references = searchService.searchReferences(referencer, false); + List references = searchService.searchReferences(referencer); // THEN assertEquals(referencesToReturn, references); } @Test - void serviceShouldCallTheFetcherForReferencesWhenForceUpdateIsTrue() { + void serviceShouldCallTheFetcherForReferencesWhenRepositoryIsUpdatable() { // GIVEN var referencer = new BibEntry(); var newReference = new BibEntry(); @@ -154,12 +156,14 @@ void serviceShouldCallTheFetcherForReferencesWhenForceUpdateIsTrue() { List::of, (e, c) -> { }, e -> referencesToReturn, - referencesDatabase::put + referencesDatabase::put, + e -> false, + e -> true ); var searchService = new SearchCitationsRelationsService(fetcher, repository); // WHEN - var references = searchService.searchReferences(referencer, true); + var references = searchService.searchReferences(referencer); // THEN assertTrue(referencesDatabase.containsKey(referencer)); @@ -188,7 +192,7 @@ void serviceShouldFetchReferencesIfRepositoryIsEmpty() { var searchService = new SearchCitationsRelationsService(fetcher, repository); // WHEN - var references = searchService.searchReferences(reference, false); + var references = searchService.searchReferences(reference); // THEN assertTrue(referencesDatabase.containsKey(reference)); @@ -209,7 +213,7 @@ void insertingAnEmptyReferencesShouldBePossible() { var searchService = new SearchCitationsRelationsService(fetcher, repository); // WHEN - var citations = searchService.searchReferences(referencer, false); + var citations = searchService.searchReferences(referencer); // THEN assertTrue(citations.isEmpty()); diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java index 14087295e63..d9a9b4bc920 100644 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryHelpersForTest.java @@ -13,7 +13,9 @@ public static BibEntryRelationsRepository from( Function> retrieveCitations, BiConsumer> insertCitations, Function> retrieveReferences, - BiConsumer> insertReferences + BiConsumer> insertReferences, + Function isCitationsUpdatable, + Function isReferencesUpdatable ) { return new BibEntryRelationsRepository() { @Override @@ -33,7 +35,7 @@ public boolean containsCitations(BibEntry entry) { @Override public boolean isCitationsUpdatable(BibEntry entry) { - return true; + return isCitationsUpdatable.apply(entry); } @Override @@ -53,7 +55,7 @@ public boolean containsReferences(BibEntry entry) { @Override public boolean isReferencesUpdatable(BibEntry entry) { - return true; + return isReferencesUpdatable.apply(entry); } }; } From b71d9fc013df75a5fe1fb7f1a153ed142cc52203 Mon Sep 17 00:00:00 2001 From: Alexandre CREMIEUX Date: Sat, 11 Jan 2025 21:55:12 +0100 Subject: [PATCH 11/35] SearchCitationsRelationsService as singleton * Instantiate service in JabRefGui * Inject service in EntryEditor --- src/main/java/org/jabref/gui/JabRefGUI.java | 6 +++++ .../jabref/gui/entryeditor/EntryEditor.java | 11 ++++++++-- .../CitationRelationsTab.java | 22 +++---------------- .../SearchCitationsRelationsService.java | 11 ++++++++++ .../ChainBibEntryRelationsRepository.java | 15 +++++++++++-- .../MVStoreBibEntryRelationDAO.java | 19 +++++++++++++++- .../org/jabref/logic/util/Directories.java | 9 ++++++++ 7 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefGUI.java b/src/main/java/org/jabref/gui/JabRefGUI.java index 820e4468c34..c02225e6321 100644 --- a/src/main/java/org/jabref/gui/JabRefGUI.java +++ b/src/main/java/org/jabref/gui/JabRefGUI.java @@ -27,6 +27,7 @@ import org.jabref.gui.util.UiTaskExecutor; import org.jabref.logic.UiCommand; import org.jabref.logic.ai.AiService; +import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.l10n.Localization; import org.jabref.logic.net.ProxyRegisterer; import org.jabref.logic.remote.RemotePreferences; @@ -169,6 +170,11 @@ public void initialize() { dialogService, taskExecutor); Injector.setModelOrService(AiService.class, aiService); + + Injector.setModelOrService( + SearchCitationsRelationsService.class, + new SearchCitationsRelationsService(preferences.getImporterPreferences()) + ); } private void setupProxy() { diff --git a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java index 6a382a7a59e..331a57ee675 100644 --- a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java +++ b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java @@ -50,6 +50,7 @@ import org.jabref.gui.util.UiTaskExecutor; import org.jabref.logic.ai.AiService; import org.jabref.logic.bibtex.TypedBibEntry; +import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.help.HelpFile; import org.jabref.logic.importer.EntryBasedFetcher; import org.jabref.logic.importer.WebFetchers; @@ -118,6 +119,7 @@ public class EntryEditor extends BorderPane { @Inject private KeyBindingRepository keyBindingRepository; @Inject private JournalAbbreviationRepository journalAbbreviationRepository; @Inject private AiService aiService; + @Inject private SearchCitationsRelationsService searchCitationsRelationsService; private final List allPossibleTabs; private final Collection previewTabs; @@ -296,8 +298,13 @@ private List createTabs() { tabs.add(new MathSciNetTab()); tabs.add(new FileAnnotationTab(libraryTab.getAnnotationCache())); tabs.add(new SciteTab(preferences, taskExecutor, dialogService)); - tabs.add(new CitationRelationsTab(dialogService, databaseContext, - undoManager, stateManager, fileMonitor, preferences, libraryTab, taskExecutor, bibEntryTypesManager)); + tabs.add(new CitationRelationsTab( + dialogService, databaseContext, + undoManager, stateManager, + fileMonitor, preferences, + libraryTab, taskExecutor, + bibEntryTypesManager, searchCitationsRelationsService + )); tabs.add(new RelatedArticlesTab(buildInfo, databaseContext, preferences, dialogService, taskExecutor)); sourceTab = new SourceTab( databaseContext, diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index 8761224d060..e267b856adb 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -1,11 +1,8 @@ package org.jabref.gui.entryeditor.citationrelationtab; -import java.io.File; import java.io.IOException; import java.io.StringWriter; import java.net.URI; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Arrays; import java.util.List; @@ -46,12 +43,10 @@ import org.jabref.logic.bibtex.BibEntryWriter; import org.jabref.logic.bibtex.FieldPreferences; import org.jabref.logic.bibtex.FieldWriter; -import org.jabref.logic.citation.repository.ChainBibEntryRelationsRepository; import org.jabref.logic.citation.SearchCitationsRelationsService; import org.jabref.logic.database.DuplicateCheck; import org.jabref.logic.exporter.BibWriter; import org.jabref.logic.importer.fetcher.CitationFetcher; -import org.jabref.logic.importer.fetcher.SemanticScholarCitationFetcher; import org.jabref.logic.l10n.Localization; import org.jabref.logic.os.OS; import org.jabref.logic.util.BackgroundTask; @@ -103,7 +98,8 @@ public CitationRelationsTab(DialogService dialogService, GuiPreferences preferences, LibraryTab libraryTab, TaskExecutor taskExecutor, - BibEntryTypesManager bibEntryTypesManager) { + BibEntryTypesManager bibEntryTypesManager, + SearchCitationsRelationsService searchCitationsRelationsService) { this.dialogService = dialogService; this.databaseContext = databaseContext; this.preferences = preferences; @@ -114,19 +110,7 @@ public CitationRelationsTab(DialogService dialogService, this.entryTypesManager = bibEntryTypesManager; this.duplicateCheck = new DuplicateCheck(entryTypesManager); - - try { - var jabRefPath = Paths.get("/home/sacha/Documents/projects/JabRef"); - var citationsPath = Path.of(jabRefPath.toAbsolutePath() + File.separator + "citations"); - var relationsPath = Path.of(jabRefPath.toAbsolutePath() + File.separator + "references"); - var bibEntryRelationsRepository = new ChainBibEntryRelationsRepository(citationsPath, relationsPath); - this.searchCitationsRelationsService = new SearchCitationsRelationsService( - new SemanticScholarCitationFetcher(preferences.getImporterPreferences()), bibEntryRelationsRepository - ); - } catch (Exception e) { - e.printStackTrace(); - throw new RuntimeException(e); - } + this.searchCitationsRelationsService = searchCitationsRelationsService; citationsRelationsTabViewModel = new CitationsRelationsTabViewModel( databaseContext, diff --git a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java index 0e800ca1077..07c5b3fb3bf 100644 --- a/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java +++ b/src/main/java/org/jabref/logic/citation/SearchCitationsRelationsService.java @@ -3,8 +3,12 @@ import java.util.List; import org.jabref.logic.citation.repository.BibEntryRelationsRepository; +import org.jabref.logic.citation.repository.ChainBibEntryRelationsRepository; import org.jabref.logic.importer.FetcherException; +import org.jabref.logic.importer.ImporterPreferences; import org.jabref.logic.importer.fetcher.CitationFetcher; +import org.jabref.logic.importer.fetcher.SemanticScholarCitationFetcher; +import org.jabref.logic.util.Directories; import org.jabref.model.entry.BibEntry; import org.slf4j.Logger; @@ -25,6 +29,13 @@ public SearchCitationsRelationsService( this.relationsRepository = repository; } + public SearchCitationsRelationsService(ImporterPreferences importerPreferences) { + this.citationFetcher = new SemanticScholarCitationFetcher(importerPreferences); + this.relationsRepository = ChainBibEntryRelationsRepository.of( + Directories.getCitationsRelationsDirectory() + ); + } + public List searchReferences(BibEntry referencer) { boolean isFetchingAllowed = this.relationsRepository.isReferencesUpdatable(referencer) || !this.relationsRepository.containsReferences(referencer); diff --git a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java index 14d26308d98..cdad83f7f52 100644 --- a/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java +++ b/src/main/java/org/jabref/logic/citation/repository/ChainBibEntryRelationsRepository.java @@ -8,15 +8,20 @@ public class ChainBibEntryRelationsRepository implements BibEntryRelationsRepository { + private static final String CITATIONS_STORE = "citations"; + private static final String REFERENCES_STORE = "references"; + private final BibEntryRelationDAO citationsDao; private final BibEntryRelationDAO referencesDao; public ChainBibEntryRelationsRepository(Path citationsStore, Path relationsStore) { this.citationsDao = BibEntryRelationDAOChain.of( - LRUCacheBibEntryRelationsDAO.CITATIONS, new MVStoreBibEntryRelationDAO(citationsStore, "citations") + LRUCacheBibEntryRelationsDAO.CITATIONS, + new MVStoreBibEntryRelationDAO(citationsStore, CITATIONS_STORE) ); this.referencesDao = BibEntryRelationDAOChain.of( - LRUCacheBibEntryRelationsDAO.REFERENCES, new MVStoreBibEntryRelationDAO(relationsStore, "relations") + LRUCacheBibEntryRelationsDAO.REFERENCES, + new MVStoreBibEntryRelationDAO(relationsStore, REFERENCES_STORE) ); } @@ -63,4 +68,10 @@ public boolean containsReferences(BibEntry entry) { public boolean isReferencesUpdatable(BibEntry entry) { return referencesDao.isUpdatable(entry); } + + public static ChainBibEntryRelationsRepository of(Path citationsRelationsDirectory) { + var citationsPath = citationsRelationsDirectory.resolve("%s.mv".formatted(CITATIONS_STORE)); + var relationsPath = citationsRelationsDirectory.resolve("%s.mv".formatted(REFERENCES_STORE)); + return new ChainBibEntryRelationsRepository(citationsPath, relationsPath); + } } diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index dd46f6f654b..492e31df016 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -1,7 +1,9 @@ package org.jabref.logic.citation.repository; +import java.io.IOException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.time.Clock; import java.time.ZoneId; @@ -23,9 +25,12 @@ import org.h2.mvstore.MVStore; import org.h2.mvstore.WriteBuffer; import org.h2.mvstore.type.BasicDataType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { + private static final Logger LOGGER = LoggerFactory.getLogger(MVStoreBibEntryRelationDAO.class); private final static ZoneId TIME_STAMP_ZONE_ID = ZoneId.of("UTC"); private final String mapName; @@ -35,6 +40,18 @@ public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { new MVMap.Builder>().valueType(new BibEntryHashSetSerializer()); MVStoreBibEntryRelationDAO(Path path, String mapName) { + + try { + if (!Files.exists(path.getParent())) { + Files.createDirectories(path.getParent()); + } + if (!Files.exists(path)) { + Files.createFile(path); + } + } catch (IOException e) { + LOGGER.error("An error occurred while opening {} storage", mapName, e); + } + this.mapName = mapName; this.insertionTimeStampMapName = mapName + "-insertion-timestamp"; this.storeConfiguration = new MVStore.Builder().autoCommitDisabled().fileName(path.toAbsolutePath().toString()); @@ -197,7 +214,7 @@ private static class BibEntryHashSetSerializer extends BasicDataType bibEntryDataType = new BibEntrySerializer(); /** - * Memory size is the sum of all aggregated bibEntries memory size plus 4 bytes. + * Memory size is the sum of all aggregated bibEntries' memory size plus 4 bytes. * Those 4 bytes are used to store the length of the collection itself. * * @param bibEntries should not be null diff --git a/src/main/java/org/jabref/logic/util/Directories.java b/src/main/java/org/jabref/logic/util/Directories.java index 00396975da7..5808fd0d6b3 100644 --- a/src/main/java/org/jabref/logic/util/Directories.java +++ b/src/main/java/org/jabref/logic/util/Directories.java @@ -62,4 +62,13 @@ public static Path getSslDirectory() { "ssl", OS.APP_DIR_APP_AUTHOR)); } + + public static Path getCitationsRelationsDirectory() { + return Path.of( + AppDirsFactory.getInstance() + .getUserDataDir( + OS.APP_DIR_APP_NAME, + "relations", + OS.APP_DIR_APP_AUTHOR)); + } } From 077e9ca6b41497ac0ad51b00661dae71edec8d6d Mon Sep 17 00:00:00 2001 From: Alexandre CREMIEUX Date: Mon, 20 Jan 2025 21:51:16 +0100 Subject: [PATCH 12/35] Update failing tests after merge --- .../BibEntryRelationDAOChainTest.java | 4 +- .../LRUCacheBibEntryRelationsDAOTest.java | 2 +- ...oreBibEntryRelationsRepositoryDAOTest.java | 2 +- ...tryRelationsRepositoryPrototypingTest.java | 280 ------------------ 4 files changed, 4 insertions(+), 284 deletions(-) delete mode 100644 src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java index d83156f0745..90b5c9534b7 100644 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java @@ -51,13 +51,13 @@ private static List createRelations(BibEntry entry) { .withField(StandardField.YEAR, String.valueOf(2024)) .withField(StandardField.AUTHOR, "A list of authors:" + i) .withType(StandardEntryType.Book) - .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.DOI, entry.getDOI().map(DOI::asString).orElse("") + ":" + i) .withField(StandardField.URL, "www.jabref.org/" + i) .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) ) ) .orElseThrow() - .collect(Collectors.toList()); + .toList(); } private static Stream createCacheAndBibEntry() { diff --git a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java index 409352a78b9..0cc141498d7 100644 --- a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java @@ -54,7 +54,7 @@ private static List createRelations(BibEntry entry) { .withField(StandardField.YEAR, String.valueOf(2024)) .withField(StandardField.AUTHOR, "A list of authors:" + i) .withType(StandardEntryType.Book) - .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.DOI, entry.getDOI().map(DOI::asString).orElse("") + ":" + i) .withField(StandardField.URL, "www.jabref.org/" + i) .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) ) diff --git a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java index 1816ed10096..bfb34ba1fa8 100644 --- a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryDAOTest.java @@ -66,7 +66,7 @@ private static List createRelations(BibEntry entry) { .withField(StandardField.YEAR, String.valueOf(2024)) .withField(StandardField.AUTHOR, "A list of authors:" + i) .withType(StandardEntryType.Book) - .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) + .withField(StandardField.DOI, entry.getDOI().map(DOI::asString).orElse("") + ":" + i) .withField(StandardField.URL, "www.jabref.org/" + i) .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) ) diff --git a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java b/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java deleted file mode 100644 index 8c7d6ced91f..00000000000 --- a/src/test/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationsRepositoryPrototypingTest.java +++ /dev/null @@ -1,280 +0,0 @@ -package org.jabref.logic.citation.repository; - -import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; -import java.util.LinkedHashSet; -import java.util.List; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.random.RandomGenerator; -import java.util.stream.Collectors; -import java.util.stream.IntStream; -import java.util.stream.Stream; - -import org.jabref.model.citation.semanticscholar.PaperDetails; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.field.StandardField; -import org.jabref.model.entry.identifier.DOI; -import org.jabref.model.entry.types.StandardEntryType; - -import org.apache.commons.lang3.tuple.Pair; -import org.h2.mvstore.DataUtils; -import org.h2.mvstore.MVMap; -import org.h2.mvstore.MVStore; -import org.h2.mvstore.WriteBuffer; -import org.h2.mvstore.type.BasicDataType; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -class MVStoreBibEntryRelationsRepositoryPrototypingTest { - - private static Stream createBibEntries() { - return IntStream - .range(0, 150) - .mapToObj(MVStoreBibEntryRelationsRepositoryPrototypingTest::createBibEntry); - } - - private static BibEntry createBibEntry(int i) { - return new BibEntry() - .withCitationKey(String.valueOf(i)) - .withField(StandardField.DOI, "10.1234/5678" + i); - } - - /** - * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic - * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar - * and mapped as BibEntry will be serializable by the MVStore. - * @param entry should not be null - * @return never empty - */ - private static LinkedHashSet createRelations(BibEntry entry) { - return entry - .getCitationKey() - .map(key -> RandomGenerator.StreamableGenerator - .of("L128X256MixRandom").ints(150) - .mapToObj(i -> new BibEntry() - .withField(StandardField.TITLE, "A title:" + i) - .withField(StandardField.YEAR, String.valueOf(2024)) - .withField(StandardField.AUTHOR, "A list of authors:" + i) - .withType(StandardEntryType.Book) - .withField(StandardField.DOI, entry.getDOI().map(DOI::getDOI).orElse("") + ":" + i) - .withField(StandardField.URL, "www.jabref.org/" + i) - .withField(StandardField.ABSTRACT, "The Universe is expanding:" + i) - ) - ) - .orElseThrow() - .collect(Collectors.toCollection(LinkedHashSet::new)); - } - - static class BibEntrySerializer extends BasicDataType { - - private final static String FIELD_SEPARATOR = "--"; - - private static String toString(BibEntry entry) { - return String.join( - FIELD_SEPARATOR, - entry.getTitle().orElse("null"), - entry.getField(StandardField.YEAR).orElse("null"), - entry.getField(StandardField.AUTHOR).orElse("null"), - entry.getType().getDisplayName(), - entry.getDOI().map(DOI::getDOI).orElse("null"), - entry.getField(StandardField.URL).orElse("null"), - entry.getField(StandardField.ABSTRACT).orElse("null") - ); - } - - private static Optional extractFieldValue(String field) { - return Objects.equals(field, "null") || field == null - ? Optional.empty() - : Optional.of(field); - } - - private static BibEntry fromString(String serializedString) { - var fields = serializedString.split(FIELD_SEPARATOR); - BibEntry entry = new BibEntry(); - extractFieldValue(fields[0]).ifPresent(title -> entry.setField(StandardField.TITLE, title)); - extractFieldValue(fields[1]).ifPresent(year -> entry.setField(StandardField.YEAR, year)); - extractFieldValue(fields[2]).ifPresent(authors -> entry.setField(StandardField.AUTHOR, authors)); - extractFieldValue(fields[3]).ifPresent(type -> entry.setType(StandardEntryType.valueOf(type))); - extractFieldValue(fields[4]).ifPresent(doi -> entry.setField(StandardField.DOI, doi)); - extractFieldValue(fields[5]).ifPresent(url -> entry.setField(StandardField.URL, url)); - extractFieldValue(fields[6]) - .ifPresent(entryAbstract -> entry.setField(StandardField.ABSTRACT, entryAbstract)); - return entry; - } - - @Override - public int getMemory(BibEntry obj) { - return toString(obj).getBytes(StandardCharsets.UTF_8).length; - } - - @Override - public void write(WriteBuffer buff, BibEntry bibEntry) { - var asBytes = toString(bibEntry).getBytes(StandardCharsets.UTF_8); - buff.putInt(asBytes.length); - buff.put(asBytes); - } - - @Override - public BibEntry read(ByteBuffer buff) { - int serializedEntrySize = buff.getInt(); - var serializedEntry = DataUtils.readString(buff, serializedEntrySize); - return fromString(serializedEntry); - } - - @Override - public int compare(BibEntry a, BibEntry b) { - if (a == null || b == null) { - throw new NullPointerException(); - } - return toString(a).compareTo(toString(b)); - } - - @Override - public BibEntry[] createStorage(int size) { - return new BibEntry[size]; - } - } - - static class BibEntryHashSetSerializer extends BasicDataType> { - - private final BasicDataType bibEntryDataType = new BibEntrySerializer(); - - /** - * Memory size is the sum of all aggregated bibEntries memory size plus 4 bytes. - * Those 4 bytes are used to store the length of the collection itself. - * @param bibEntries should not be null - * @return total size in memory of the serialized collection of bib entries - */ - @Override - public int getMemory(LinkedHashSet bibEntries) { - return bibEntries - .stream() - .map(this.bibEntryDataType::getMemory) - .reduce(0, Integer::sum) + 4; - } - - @Override - public void write(WriteBuffer buff, LinkedHashSet bibEntries) { - buff.putInt(bibEntries.size()); - bibEntries.forEach(entry -> this.bibEntryDataType.write(buff, entry)); - } - - @Override - public LinkedHashSet read(ByteBuffer buff) { - return IntStream.range(0, buff.getInt()) - .mapToObj(it -> this.bibEntryDataType.read(buff)) - .collect(Collectors.toCollection(LinkedHashSet::new)); - } - - @Override - public LinkedHashSet[] createStorage(int size) { - return new LinkedHashSet[size]; - } - } - - @Test - void itShouldBePossibleToStoreABibEntryList(@TempDir Path temporaryFolder) throws IOException { - var file = Files.createFile(temporaryFolder.resolve("test_string_store")); - try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { - // GIVEN - MVMap> citations = store.openMap("citations"); - - // WHEN - citations.put("Hello", List.of("The", "World")); - store.commit(); - var fromStore = citations.get("Hello"); - - // THEN - Assertions.assertTrue(Files.exists(file)); - Assertions.assertEquals("Hello The World", "Hello " + String.join(" ", fromStore)); - } - } - - /** - * Fake in memory sequential save and load - */ - @Test - void IWouldLikeToSaveAndLoadCitationsForABibEntryFromAMap(@TempDir Path temporaryFolder) throws IOException { - var file = Files.createFile(temporaryFolder.resolve("bib_entry_citations_test_store")); - try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { - // GIVEN - Map> citationsToBeStored = createBibEntries() - .map(e -> Pair.of(e.getDOI().orElseThrow().getDOI(), createRelations(e))) - .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); - Assertions.assertFalse(citationsToBeStored.isEmpty()); - var mapConfiguration = new MVMap.Builder>() - .valueType(new BibEntryHashSetSerializer()); - - /** - var mapConfiguration = new MVMap.Builder>() - .valueType(new BibEntryHashSetSerializer()); - MVMap> citationsMap = store.openMap("citations", mapConfiguration); - **/ - MVMap> citationsMap = store.openMap("citations", mapConfiguration); - - // WHEN - citationsToBeStored.forEach((entry, citations) -> citationsMap.put(entry, new LinkedHashSet<>(citations))); - - // THEN - citationsToBeStored.forEach((entry, citations) -> { - Assertions.assertTrue(citationsMap.containsKey(entry)); - Assertions.assertEquals(citations, citationsMap.get(entry)); - }); - } - } - - /** - * Fake persisted sequential save and load operations. - */ - @Test - void IWouldLikeToSaveAndLoadCitationsForABibEntryFromAStore(@TempDir Path temporaryFolder) throws IOException { - var file = Files.createFile(temporaryFolder.resolve("bib_entry_citations_test_store")); - - // GIVEN - Map> citationsToBeStored = createBibEntries() - .map(e -> Pair.of(e.getDOI().orElseThrow().getDOI(), createRelations(e))) - .collect(Collectors.toMap(Pair::getKey, Pair::getValue)); - Assertions.assertFalse(citationsToBeStored.isEmpty()); - - var mapConfiguration = new MVMap.Builder>() - .valueType(new BibEntryHashSetSerializer()); - - Map> citationsFromStore = null; - - // WHEN - // STORING AND CLOSING - try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { - MVMap> citationsMap = store.openMap("citations", mapConfiguration); - citationsToBeStored.forEach((entry, citations) -> citationsMap.put(entry, new LinkedHashSet<>(citations))); - store.commit(); - } - - // READING AND CLOSING - try (var store = new MVStore.Builder().fileName(file.toAbsolutePath().toString()).open()) { - MVMap> citationsMap = store.openMap("citations", mapConfiguration); - citationsFromStore = Map.copyOf(citationsMap); - } - - // THEN - Assertions.assertNotNull(citationsFromStore); - Assertions.assertFalse(citationsFromStore.isEmpty()); - var entriesToBeStored = citationsToBeStored.entrySet(); - for (var entry : entriesToBeStored) { - Assertions.assertTrue(citationsFromStore.containsKey(entry.getKey())); - var citations = citationsFromStore.get(entry.getKey()); - Assertions.assertEquals(entry.getValue(), citations); - } - } - - @Test - void test() { - var s = Stream.of(null, "test", null).collect(Collectors.joining()); - System.out.println(s); - } -} From 0bff9136551d6deb8b93ff2293200882bdd6a44f Mon Sep 17 00:00:00 2001 From: Alexandre CREMIEUX Date: Mon, 20 Jan 2025 22:02:51 +0100 Subject: [PATCH 13/35] Clean code style according to checkstyle --- .../citation/repository/MVStoreBibEntryRelationDAO.java | 3 +-- .../importer/fetcher/SemanticScholarCitationFetcher.java | 5 +++-- .../citation/repository/BibEntryRelationDAOChainTest.java | 1 - .../repository/LRUCacheBibEntryRelationsDAOTest.java | 1 + 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index 44f68317d19..58159955d13 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -7,13 +7,13 @@ import java.nio.file.Path; import java.time.Clock; import java.time.ZoneId; +import java.time.LocalDateTime; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.IntStream; -import java.time.LocalDateTime; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.StandardField; @@ -40,7 +40,6 @@ public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { new MVMap.Builder>().valueType(new BibEntryHashSetSerializer()); MVStoreBibEntryRelationDAO(Path path, String mapName) { - try { if (!Files.exists(path.getParent())) { Files.createDirectories(path.getParent()); diff --git a/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java index 8c60658e49a..92e1f0cf986 100644 --- a/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java +++ b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java @@ -7,17 +7,18 @@ import org.jabref.logic.importer.FetcherException; import org.jabref.logic.importer.ImporterPreferences; import org.jabref.logic.net.URLDownload; +import org.jabref.model.entry.BibEntry; import org.jabref.model.citation.semanticscholar.CitationsResponse; import org.jabref.model.citation.semanticscholar.ReferencesResponse; import org.jabref.logic.util.URLUtil; -import org.jabref.model.entry.BibEntry; import com.google.gson.Gson; public class SemanticScholarCitationFetcher implements CitationFetcher, CustomizableKeyFetcher { - private static final String SEMANTIC_SCHOLAR_API = "https://api.semanticscholar.org/graph/v1/"; public static final String FETCHER_NAME = "Semantic Scholar Citations Fetcher"; + private static final String SEMANTIC_SCHOLAR_API = "https://api.semanticscholar.org/graph/v1/"; + private final ImporterPreferences importerPreferences; public SemanticScholarCitationFetcher(ImporterPreferences importerPreferences) { diff --git a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java index 90b5c9534b7..862e63acbda 100644 --- a/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/BibEntryRelationDAOChainTest.java @@ -4,7 +4,6 @@ import java.util.List; import java.util.Map; import java.util.random.RandomGenerator; -import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; diff --git a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java index 0cc141498d7..dc8f81e2352 100644 --- a/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java +++ b/src/test/java/org/jabref/logic/citation/repository/LRUCacheBibEntryRelationsDAOTest.java @@ -41,6 +41,7 @@ private static BibEntry createBibEntry(int i) { * Create a fake list of relations for a bibEntry based on the {@link PaperDetails#toBibEntry()} logic * that corresponds to this use case: we want to make sure that relations coming from SemanticScholar * and mapped as BibEntry will be serializable by the MVStore. + * * @param entry should not be null * @return never empty */ From ea36f1a4a427719e2d09a397fe87c40f06fcf5c7 Mon Sep 17 00:00:00 2001 From: Alexandre CREMIEUX Date: Mon, 20 Jan 2025 22:15:57 +0100 Subject: [PATCH 14/35] Clean code style according to checkstyle --- .../logic/citation/repository/MVStoreBibEntryRelationDAO.java | 2 +- .../importer/fetcher/SemanticScholarCitationFetcher.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index 58159955d13..90dc9cfbee5 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -6,8 +6,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Clock; -import java.time.ZoneId; import java.time.LocalDateTime; +import java.time.ZoneId; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; diff --git a/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java index 92e1f0cf986..3f0d8cbd6f8 100644 --- a/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java +++ b/src/main/java/org/jabref/logic/importer/fetcher/SemanticScholarCitationFetcher.java @@ -7,10 +7,10 @@ import org.jabref.logic.importer.FetcherException; import org.jabref.logic.importer.ImporterPreferences; import org.jabref.logic.net.URLDownload; -import org.jabref.model.entry.BibEntry; +import org.jabref.logic.util.URLUtil; import org.jabref.model.citation.semanticscholar.CitationsResponse; import org.jabref.model.citation.semanticscholar.ReferencesResponse; -import org.jabref.logic.util.URLUtil; +import org.jabref.model.entry.BibEntry; import com.google.gson.Gson; From d7f9c2d4ae011e70698031d2c2739fc7b9f7f724 Mon Sep 17 00:00:00 2001 From: Alexandre CREMIEUX Date: Mon, 20 Jan 2025 23:52:43 +0100 Subject: [PATCH 15/35] Fix pending PR comments --- .../citationrelationtab/CitationRelationsTab.java | 4 +--- .../repository/ChainBibEntryRelationsRepository.java | 6 ++++++ .../citation/repository/MVStoreBibEntryRelationDAO.java | 7 +++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index aa852b5a9d2..b82c9839fb8 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -434,10 +434,8 @@ private void searchForRelations(BibEntry entry, CheckListView citations) { @Override public List readCitations(BibEntry entry) { + if (entry == null) { + return List.of(); + } return citationsDao.getRelations(entry); } @@ -56,6 +59,9 @@ public void insertReferences(BibEntry entry, List references) { @Override public List readReferences(BibEntry entry) { + if (entry == null) { + return List.of(); + } return referencesDao.getRelations(entry); } diff --git a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java index 90dc9cfbee5..f70b88ced93 100644 --- a/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java +++ b/src/main/java/org/jabref/logic/citation/repository/MVStoreBibEntryRelationDAO.java @@ -32,6 +32,7 @@ public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { private static final Logger LOGGER = LoggerFactory.getLogger(MVStoreBibEntryRelationDAO.class); private final static ZoneId TIME_STAMP_ZONE_ID = ZoneId.of("UTC"); + private final static String TIME_STAMP_SUFFIX = "-insertion-timestamp"; private final String mapName; private final String insertionTimeStampMapName; @@ -52,8 +53,10 @@ public class MVStoreBibEntryRelationDAO implements BibEntryRelationDAO { } this.mapName = mapName; - this.insertionTimeStampMapName = mapName + "-insertion-timestamp"; - this.storeConfiguration = new MVStore.Builder().autoCommitDisabled().fileName(path.toAbsolutePath().toString()); + this.insertionTimeStampMapName = mapName + TIME_STAMP_SUFFIX; + this.storeConfiguration = new MVStore.Builder() + .autoCommitDisabled() + .fileName(path.toAbsolutePath().toString()); } @Override From 97b38c6b9c0ad775ab99494a6d21c17bbbb426e1 Mon Sep 17 00:00:00 2001 From: Alexandre CREMIEUX Date: Wed, 22 Jan 2025 20:52:40 +0100 Subject: [PATCH 16/35] Fix null pointer exception in CitationsRelationsTab --- .../entryeditor/citationrelationtab/CitationRelationsTab.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index b82c9839fb8..c0176ce3ca4 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -434,7 +434,7 @@ private void searchForRelations(BibEntry entry, CheckListView Date: Sat, 25 Jan 2025 18:47:09 +0100 Subject: [PATCH 17/35] Add settings for Citations relations store TTL --- .../CitationRelationsTab.java | 4 ++- .../preferences/websearch/WebSearchTab.fxml | 4 +++ .../preferences/websearch/WebSearchTab.java | 21 ++++++++++++ .../websearch/WebSearchTabViewModel.java | 10 ++++++ .../SearchCitationsRelationsService.java | 3 +- .../ChainBibEntryRelationsRepository.java | 10 +++--- .../MVStoreBibEntryRelationDAO.java | 8 +++-- .../logic/importer/ImporterPreferences.java | 19 ++++++++++- .../preferences/JabRefCliPreferences.java | 6 +++- src/main/resources/l10n/JabRef_de.properties | 2 ++ src/main/resources/l10n/JabRef_en.properties | 2 ++ src/main/resources/l10n/JabRef_fr.properties | 2 ++ src/main/resources/l10n/JabRef_it.properties | 2 ++ src/main/resources/l10n/JabRef_pl.properties | 3 ++ .../resources/l10n/JabRef_pt_BR.properties | 2 ++ .../ChainBibEntryRelationsRepositoryTest.java | 4 +-- ...oreBibEntryRelationsRepositoryDAOTest.java | 32 ++++++++++++++++--- 17 files changed, 115 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java index c0176ce3ca4..aa852b5a9d2 100644 --- a/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java +++ b/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java @@ -434,8 +434,10 @@ private void searchForRelations(BibEntry entry, CheckListView + +