Skip to content

Commit

Permalink
Refactor Citations Relations Tab (#11189)
Browse files Browse the repository at this point in the history
* Move repository, cache, and fetcher to logic package
* Move citations model to model/citations/semanticscholar package
  • Loading branch information
alexandre-cremieux committed Sep 29, 2024
1 parent e395853 commit 3785cb1
Show file tree
Hide file tree
Showing 15 changed files with 159 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -394,7 +398,7 @@ private void onSearchForRelationsSucceed(BibEntry entry, CheckListView<CitationR
.map(localEntry -> new CitationRelationItem(entr, localEntry, true))
.orElseGet(() -> new CitationRelationItem(entr, false)))
.toList()
);
);

if (!observableList.isEmpty()) {
listView.refresh();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, List<BibEntry>> CITATIONS_MAP = new LRUMap<>(MAX_CACHED_ENTRIES, MAX_CACHED_ENTRIES);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar;
package org.jabref.logic.importer.fetcher;

import java.util.List;

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar;
package org.jabref.model.citation.semanticscholar;

/**
* Used for GSON
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar;
package org.jabref.model.citation.semanticscholar;

/**
* Used for GSON
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar;
package org.jabref.model.citation.semanticscholar;

import java.util.List;

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar;
package org.jabref.model.citation.semanticscholar;

/**
* Used for GSON
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.jabref.gui.entryeditor.citationrelationtab.semanticscholar;
package org.jabref.model.citation.semanticscholar;

import java.util.List;

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<BibEntry> createBibEntries() {
return IntStream
.range(0, 150)
.mapToObj(BibEntryRelationsRepositoryTest::createBibEntry);
}

private static List<BibEntry> 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<BibEntry, List<BibEntry>> searchCiteByDelegate,
Function<BibEntry, List<BibEntry>> searchCitingDelegate,
String name
) implements CitationFetcher {

@Override
public List<BibEntry> searchCitedBy(BibEntry entry) throws FetcherException {
return this.searchCiteByDelegate.apply(entry);
}

@Override
public List<BibEntry> 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<BibEntry>();
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);
}
}

0 comments on commit 3785cb1

Please sign in to comment.