Refactor Citations Relations Tab (#11189)

* Introduce service layer * Rename LRU cache implementation * Add tests helpers for repository
JabRef · Sep 28, 2024 · f603c78 · f603c78
1 parent c183ae1
commit f603c78
Show file tree

Hide file tree

Showing 7 changed files with 269 additions and 74 deletions.
diff --git 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<BibEntry> getCitations(BibEntry entry) {
-        if (needToRefreshCitations(entry)) {
-            forceRefreshCitations(entry);
-        }
-
-        return cache.getCitations(entry);
-    }
-
-    public List<BibEntry> getReferences(BibEntry entry) {
-        if (needToRefreshReferences(entry)) {
-            List<BibEntry> 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<BibEntry> 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<BibEntry> 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<BibEntry> readCitations(BibEntry entry);
+
+    List<BibEntry> 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/...on/repository/BibEntryRelationsCache.java → ...repository/LRUBibEntryRelationsCache.java b/...on/repository/BibEntryRelationsCache.java → ...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<String, List<BibEntry>> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES);
     private static final Map<String, List<BibEntry>> 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
@@ -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<BibEntry> readCitations(BibEntry entry) {
+        if (needToRefreshCitations(entry)) {
+            forceRefreshCitations(entry);
+        }
+
+        return cache.getCitations(entry);
+    }
+
+    @Override
+    public List<BibEntry> readReferences(BibEntry entry) {
+        if (needToRefreshReferences(entry)) {
+            List<BibEntry> 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<BibEntry> 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<BibEntry> 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
@@ -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<BibEntry> searchReferences(BibEntry referencer) {
+        return this.relationsRepository.readReferences(referencer);
+    }
+
+    public List<BibEntry> searchReferences(BibEntry referencer, boolean forceUpdate) {
+        if (forceUpdate) {
+            this.relationsRepository.forceRefreshReferences(referencer);
+        }
+        return this.searchReferences(referencer);
+    }
+
+    public List<BibEntry> searchCitations(BibEntry cited) {
+        return this.relationsRepository.readCitations(cited);
+    }
+
+    public List<BibEntry> 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
@@ -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/...est/java/org/jabref/logic/citation/repository/BibEntryRelationsRepositoryTestHelpers.java b/...est/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<BibEntry, List<BibEntry>> retrieveCitations,
+            Function<BibEntry, List<BibEntry>> retrieveReferences,
+            Consumer<BibEntry> forceRefreshCitations,
+            Consumer<BibEntry> forceRefreshReferences
+        ) {
+            return new BibEntryRelationsRepository() {
+                @Override
+                public List<BibEntry> readCitations(BibEntry entry) {
+                    return retrieveCitations.apply(entry);
+                }
+
+                @Override
+                public List<BibEntry> 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
@@ -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<BibEntry> 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<BibEntry>();
+        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<BibEntry> 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<BibEntry>();
+        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);
+    }
+}