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] 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); + } +}