-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Feature/url based fetcher #15458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/url based fetcher #15458
Changes from all commits
acfde3f
2155a0f
fbe7fc8
3b8cc73
06baac6
57174ac
8edc747
2cad645
7116372
501fe67
2648a6d
d402030
f4b5d1b
868348e
6444a36
5595f37
258a140
d41f837
4368823
7405062
059fff1
0b4c405
0138ca8
368e4b7
7b92325
580d1cf
462ef8b
9cec0cd
6fc6a26
c922846
3e98ae0
e358e88
c8a3bb5
2e22acc
98a60ce
82094db
158586b
0a1245f
3cae28d
d59b52f
d9f024e
e1ecdb0
6a91b76
22d764c
3f23503
4043778
68a9caf
e2e0526
b352767
04b0e36
e011b4d
de5ff86
8be07a0
c52e877
6991fe8
16f3673
1c96e84
a19e09b
aa45263
0cae7f4
ce5d37e
666577c
a1e8208
a5f8efa
917fff6
0dc5b3e
4aa8026
05ee0fe
0dd0056
9626864
cc86e6d
39eee8e
ba81759
60d4b41
e6d5eb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,5 @@ public enum NewEntryDialogTab { | |
| ENTER_IDENTIFIER, | ||
| INTERPRET_CITATIONS, | ||
| SPECIFY_BIBTEX, | ||
| ENTER_URL, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,9 @@ public class NewEntryView extends BaseDialog<BibEntry> { | |
|
|
||
| @FXML private TextArea bibtexText; | ||
|
|
||
| @FXML private Tab tabEnterUrl; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can move this line below |
||
| @FXML private TextField urlText; | ||
|
|
||
| private BibEntry result; | ||
|
|
||
| public NewEntryView(NewEntryDialogTab initialApproach, GuiPreferences preferences, LibraryTab libraryTab, DialogService dialogService) { | ||
|
|
@@ -206,12 +209,17 @@ private void finalizeTabs() { | |
| tabs.getSelectionModel().select(tabSpecifyBibtex); | ||
| switchSpecifyBibtex(); | ||
| break; | ||
| case NewEntryDialogTab.ENTER_URL: | ||
| tabs.getSelectionModel().select(tabEnterUrl); | ||
| switchEnterUrl(); | ||
| break; | ||
| } | ||
|
|
||
| tabAddEntry.setOnSelectionChanged(_ -> switchAddEntry()); | ||
| tabLookupIdentifier.setOnSelectionChanged(_ -> switchLookupIdentifier()); | ||
| tabInterpretCitations.setOnSelectionChanged(_ -> switchInterpretCitations()); | ||
| tabSpecifyBibtex.setOnSelectionChanged(_ -> switchSpecifyBibtex()); | ||
| tabEnterUrl.setOnSelectionChanged(_ -> switchEnterUrl()); | ||
| } | ||
|
|
||
| @FXML | ||
|
|
@@ -234,6 +242,7 @@ public void initialize() { | |
| initializeLookupIdentifier(); | ||
| initializeInterpretCitations(); | ||
| initializeSpecifyBibTeX(); | ||
| initializeEnterUrl(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you change the name? "EnterUrl" is the action of end users. Here, it should be something JabRef does. Also for |
||
| } | ||
|
|
||
| private void initializeAddEntry() { | ||
|
|
@@ -472,6 +481,31 @@ private void switchSpecifyBibtex() { | |
| } | ||
| } | ||
|
|
||
| @FXML | ||
| private void switchEnterUrl() { | ||
| if (!tabEnterUrl.isSelected()) { | ||
| return; | ||
| } | ||
|
|
||
| currentApproach = NewEntryDialogTab.ENTER_URL; | ||
| newEntryPreferences.setLatestApproach(NewEntryDialogTab.ENTER_URL); | ||
|
|
||
| if (urlText != null) { | ||
| Platform.runLater(() -> urlText.requestFocus()); | ||
| } | ||
|
Comment on lines
+493
to
+495
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clipboard content should also be in the text field. |
||
|
|
||
| if (generateButton != null) { | ||
| generateButton.disableProperty().bind( | ||
| viewModel.urlTextValidatorProperty().not() | ||
| ); | ||
| generateButton.setText(Localization.lang("Search")); | ||
| } | ||
| } | ||
|
|
||
| private void initializeEnterUrl() { | ||
| urlText.textProperty().bindBidirectional(viewModel.urlTextProperty()); | ||
| } | ||
|
|
||
| private void onEntryTypeSelected(EntryType type) { | ||
| newEntryPreferences.setLatestImmediateType(type); | ||
| result = new BibEntry(type); | ||
|
|
@@ -507,6 +541,11 @@ private void execute() { | |
| viewModel.executeSpecifyBibtex(); | ||
| switchSpecifyBibtex(); | ||
| break; | ||
| case NewEntryDialogTab.ENTER_URL: | ||
| generateButton.setText(Localization.lang("Searching...")); | ||
| viewModel.executeEnterUrl(); | ||
| switchEnterUrl(); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
InAnYan marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Locale; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
|
|
@@ -33,13 +34,15 @@ | |
| import org.jabref.logic.importer.IdBasedFetcher; | ||
| import org.jabref.logic.importer.ParseException; | ||
| import org.jabref.logic.importer.WebFetchers; | ||
| import org.jabref.logic.importer.fetcher.GenericUrlBasedFetcher; | ||
| import org.jabref.logic.importer.fileformat.BibtexParser; | ||
| import org.jabref.logic.importer.plaincitation.PlainCitationParser; | ||
| import org.jabref.logic.importer.plaincitation.PlainCitationParserChoice; | ||
| import org.jabref.logic.importer.plaincitation.PlainCitationParserFactory; | ||
| import org.jabref.logic.l10n.Localization; | ||
| import org.jabref.logic.layout.LayoutFormatter; | ||
| import org.jabref.logic.layout.format.DOIStrip; | ||
| import org.jabref.logic.util.URLUtil; | ||
| import org.jabref.logic.util.strings.StringUtil; | ||
| import org.jabref.model.TransferInformation; | ||
| import org.jabref.model.TransferMode; | ||
|
|
@@ -91,6 +94,9 @@ public class NewEntryViewModel { | |
| private final StringProperty bibtexText; | ||
| private final Validator bibtexTextValidator; | ||
| private Task<Optional<List<BibEntry>>> bibtexWorker; | ||
| private final StringProperty urlText; | ||
| private final Validator urlTextValidator; | ||
| private Task<Optional<List<BibEntry>>> urlWorker; | ||
| private final Map<String, BibEntry> doiCache; | ||
| private BibEntry duplicateEntry; | ||
|
|
||
|
|
@@ -150,6 +156,22 @@ public NewEntryViewModel(GuiPreferences preferences, | |
| StringUtil::isNotBlank, | ||
| ValidationMessage.error(Localization.lang("You must specify a Bib(La)TeX source."))); | ||
| bibtexWorker = null; | ||
|
|
||
| urlText = new SimpleStringProperty(""); | ||
| urlTextValidator = new FunctionBasedValidator<>(urlText, input -> { | ||
| if (StringUtil.isBlank(input)) { | ||
| return false; | ||
| } | ||
|
|
||
| String normalized = input.trim(); | ||
| String lower = normalized.toLowerCase(Locale.ROOT); | ||
|
|
||
| if (!lower.startsWith("http://") && !lower.startsWith("https://")) { | ||
| normalized = "https://" + normalized; | ||
| } | ||
|
|
||
| return URLUtil.isValidHttpUrl(normalized); | ||
| }, ValidationMessage.error(Localization.lang("Please enter a valid HTTP or HTTPS URL."))); | ||
|
Comment on lines
+160
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Url scheme not stored The URL validator accepts schemeless inputs by prepending "https://" only for validation, but the worker/fetcher persists the original text, so entries can be created with invalid URL values (e.g., "example.com"). This breaks the promise of “valid URL” and can lead to non-clickable/unusable URL fields downstream. Agent Prompt
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
|
|
||
| public void populateDOICache() { | ||
|
|
@@ -238,6 +260,14 @@ public ReadOnlyBooleanProperty bibtexTextValidatorProperty() { | |
| return bibtexTextValidator.getValidationStatus().validProperty(); | ||
| } | ||
|
|
||
| public StringProperty urlTextProperty() { | ||
| return urlText; | ||
| } | ||
|
|
||
| public ReadOnlyBooleanProperty urlTextValidatorProperty() { | ||
| return urlTextValidator.getValidationStatus().validProperty(); | ||
| } | ||
|
|
||
| private BibEntry withCoversDownloaded(BibEntry entry) { | ||
| if (preferences.getPreviewPreferences().shouldDownloadCovers()) { | ||
| bookCoverFetcher.downloadCoversForEntry(entry); | ||
|
|
@@ -469,6 +499,27 @@ protected Optional<List<BibEntry>> call() throws ParseException { | |
| } | ||
| } | ||
|
|
||
| private class WorkerLookupUrl extends Task<Optional<List<BibEntry>>> { | ||
| @Override | ||
| protected Optional<List<BibEntry>> call() throws FetcherException { | ||
| final String text = urlText.getValue(); | ||
| final boolean textValid = urlTextValidator.getValidationStatus().isValid(); | ||
|
|
||
| if (text == null || !textValid) { | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| GenericUrlBasedFetcher fetcher = new GenericUrlBasedFetcher(); | ||
|
|
||
| List<BibEntry> entries = fetcher.fetchEntryFromUrl(text); | ||
|
|
||
| if (entries.isEmpty()) { | ||
| return Optional.empty(); | ||
| } | ||
| return Optional.of(entries); | ||
| } | ||
| } | ||
|
|
||
| public void executeSpecifyBibtex() { | ||
| executing.setValue(true); | ||
|
|
||
|
|
@@ -534,6 +585,42 @@ public void executeSpecifyBibtex() { | |
| taskExecutor.execute(bibtexWorker); | ||
| } | ||
|
|
||
| public void executeEnterUrl() { | ||
| executing.setValue(true); | ||
|
|
||
| cancel(); | ||
| urlWorker = new WorkerLookupUrl(); | ||
|
|
||
| urlWorker.setOnFailed(_ -> { | ||
| final Throwable exception = urlWorker.getException(); | ||
| final String exceptionMessage = Optional.ofNullable(exception.getMessage()).orElse(exception.toString()); | ||
| LOGGER.error("An exception occurred when looking up URL.", exception); | ||
|
|
||
| dialogService.showInformationDialogAndWait(Localization.lang("Failed to look up URL"), Localization.lang("The following error occurred:\n%0", exceptionMessage)); | ||
|
|
||
| executing.set(false); | ||
| }); | ||
|
|
||
| urlWorker.setOnSucceeded(_ -> { | ||
| final Optional<List<BibEntry>> result = urlWorker.getValue(); | ||
|
|
||
| if (result.isEmpty()) { | ||
| dialogService.showWarningDialogAndWait(Localization.lang("Invalid result"), Localization.lang("No entry could be generated from the provided URL.\n" + "This entry may need to be added manually.")); | ||
| executing.set(false); | ||
| return; | ||
| } | ||
|
|
||
| final ImportHandler handler = new ImportHandler(libraryTab.getBibDatabaseContext(), preferences, fileUpdateMonitor, libraryTab.getUndoManager(), stateManager, dialogService, taskExecutor); | ||
|
|
||
| handler.importEntriesWithDuplicateCheck(null, result.get()); | ||
|
|
||
| executedSuccessfully.set(true); | ||
| executing.set(false); | ||
| }); | ||
|
|
||
| taskExecutor.execute(urlWorker); | ||
| } | ||
|
|
||
| public void cancel() { | ||
| if (idLookupWorker != null) { | ||
| idLookupWorker.cancel(); | ||
|
|
@@ -544,5 +631,9 @@ public void cancel() { | |
| if (bibtexWorker != null) { | ||
| bibtexWorker.cancel(); | ||
| } | ||
|
|
||
| if (urlWorker != null) { | ||
| urlWorker.cancel(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| package org.jabref.logic.importer.fetcher; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.jabref.logic.importer.FetcherException; | ||
| import org.jabref.model.entry.BibEntry; | ||
| import org.jabref.model.entry.field.StandardField; | ||
| import org.jabref.model.entry.types.StandardEntryType; | ||
|
|
||
| public class GenericUrlBasedFetcher { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quote from issue description: "Add one implementation GenericUrlBasedFetcher just generating a @misc entry with URL." You mark this PR reviewable again without doing anything from #15458 (review)??? |
||
|
|
||
| public String getName() { | ||
| return "Generic URL Fetcher"; | ||
| } | ||
|
Comment on lines
+12
to
+14
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement What's the use of a method if its only called by test? |
||
|
|
||
| public List<BibEntry> fetchEntryFromUrl(String url) throws FetcherException { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if (url == null || url.trim().isEmpty()) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| String trimmedUrl = url.trim(); | ||
| if (!trimmedUrl.startsWith("http://") && !trimmedUrl.startsWith("https://")) { | ||
| trimmedUrl = "https://" + trimmedUrl; | ||
| } | ||
|
|
||
| BibEntry entry = new BibEntry(StandardEntryType.Misc) | ||
| .withField(StandardField.URL, trimmedUrl); | ||
| return List.of(entry); | ||
|
Comment on lines
+21
to
+28
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too many flaws...
|
||
| } | ||
| } | ||
|
InAnYan marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package org.jabref.logic.importer.fetcher; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
||
| import org.jabref.logic.importer.FetcherException; | ||
| import org.jabref.model.entry.BibEntry; | ||
| import org.jabref.model.entry.field.StandardField; | ||
| import org.jabref.model.entry.types.StandardEntryType; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| class GenericUrlBasedFetcherTest { | ||
| private static final String TEST_URL = "https://gi-radar.de/397-coding-unterstuetzung-im-lauf-der-zeit/"; | ||
| private GenericUrlBasedFetcher fetcher; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| fetcher = new GenericUrlBasedFetcher(); | ||
| } | ||
|
|
||
| @Test | ||
| void getNameReturnsCorrectName() { | ||
| assertEquals("Generic URL Fetcher", fetcher.getName()); | ||
| } | ||
|
|
||
| @Test | ||
| void fetchEntryFromUrlWithValidUrlCreatesCorrectEntry() throws FetcherException { | ||
| List<BibEntry> results = fetcher.fetchEntryFromUrl(TEST_URL); | ||
| assertEquals(1, results.size()); | ||
| BibEntry entry = results.get(0); | ||
| assertEquals(Optional.of(TEST_URL), entry.getField(StandardField.URL)); | ||
| assertEquals(StandardEntryType.Misc, entry.getType()); | ||
| } | ||
|
|
||
| @Test | ||
| void fetchEntryFromUrlWithBlankUrlReturnsEmptyList() throws FetcherException { | ||
| List<BibEntry> results = fetcher.fetchEntryFromUrl(" "); | ||
| assertTrue(results.isEmpty()); | ||
| } | ||
|
|
||
| @Test | ||
| void fetchEntryFromUrlTrimsUrl() throws FetcherException { | ||
| List<BibEntry> results = fetcher.fetchEntryFromUrl(" " + TEST_URL + " "); | ||
| assertEquals(1, results.size()); | ||
| assertEquals(Optional.of(TEST_URL), results.get(0).getField(StandardField.URL)); | ||
| } | ||
| } |


Uh oh!
There was an error while loading. Please reload this page.