Skip to content
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

Refactor of DOI import failure dialog, import format reader and clipboard manager #8839

Merged
merged 37 commits into from
Aug 6, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
02481fa
Rename cleanup actions: "Make LaTeX ready" and "Make BibTeX ready"
zkl-ai Apr 30, 2022
07345ea
modify dio exception handler
zkl-ai May 22, 2022
f786412
Merge branch 'JabRef:main' into improve-DOI-import-failure-dialog
zkl-ai May 22, 2022
f06021a
modify dio exception handler change dialog from "No DOI data exits" t…
zkl-ai May 22, 2022
2348a34
Merge remote-tracking branch 'origin/improve-DOI-import-failure-dialo…
zkl-ai May 22, 2022
d32eae2
modify Localization.lang(message)
zkl-ai May 22, 2022
987d5df
Merge branch 'JabRef:main' into improve-DOI-import-failure-dialog
zkl-ai May 22, 2022
a6ec469
modify Localization.lang("Client Error") and
zkl-ai May 22, 2022
b3ac167
pass LocalizationConsistencyTest
zkl-ai May 22, 2022
6bf38c4
pass fetcher test
zkl-ai May 22, 2022
58361cb
Merge branch 'JabRef:main' into improve-DOI-import-failure-dialog
zkl-ai May 24, 2022
8e5b82a
improve the implementation with two add Exception DOIDataNotFoundExce…
zkl-ai May 24, 2022
d406d5a
Merge remote-tracking branch 'origin/improve-DOI-import-failure-dialo…
zkl-ai May 24, 2022
38f3e52
improve the implementation with two add Exception DOIDataNotFoundExce…
zkl-ai May 24, 2022
6b7dfa9
Merge remote-tracking branch 'upstream/main' into improve-DOI-import-…
Siedlerchr Jul 14, 2022
2561d32
Add FetcherClientException and FetcherServerException
Siedlerchr Jul 14, 2022
a02f5d4
rename test
Siedlerchr Jul 14, 2022
5dfdc8e
refactor impor format reader and clipboard manager
Siedlerchr Jul 14, 2022
3c0ee30
Merge remote-tracking branch 'upstream/main' into improve-DOI-import-…
Siedlerchr Jul 15, 2022
9ed8131
Refactor clipboard manager
Siedlerchr Jul 15, 2022
890e623
remove unused prefs
Siedlerchr Jul 15, 2022
c6deb0c
revert style changes
Siedlerchr Jul 15, 2022
4b0aeb2
fix checkstlye
Siedlerchr Jul 15, 2022
a03e25f
do not throw 404 errors from UrlDownload
Siedlerchr Jul 16, 2022
32fd370
better error messages
Siedlerchr Jul 16, 2022
a9c4589
checkstyle
Siedlerchr Jul 16, 2022
0165d14
Merge remote-tracking branch 'upstream/main' into improve-DOI-import-…
Siedlerchr Jul 18, 2022
73a88b7
improve dialog messages
Siedlerchr Jul 18, 2022
5060e34
change l10n
Siedlerchr Jul 19, 2022
b8a83ac
Merge remote-tracking branch 'upstream/main' into improve-DOI-import-…
Siedlerchr Jul 19, 2022
4c0ebd1
fix merge conflicts
Siedlerchr Jul 19, 2022
0cc4826
fix l10n messages
Siedlerchr Jul 19, 2022
369310e
fix checkstyle and fetcher
Siedlerchr Jul 20, 2022
d26e288
Merge remote-tracking branch 'upstream/main' into improve-DOI-import-…
Siedlerchr Aug 5, 2022
c47ea2d
improve error messages
Siedlerchr Aug 5, 2022
be99801
further fixes
Siedlerchr Aug 5, 2022
7c609f3
fix l10n
Siedlerchr Aug 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,17 @@ public void fetchAndMerge(BibEntry entry, List<Field> fields) {
})
.onFailure(exception -> {
LOGGER.error("Error while fetching bibliographic information", exception);
dialogService.showErrorDialogAndWait(exception);
String localMessage = exception.getCause().getLocalizedMessage();
// client error
if (localMessage.startsWith("Client")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the check, I am talking about. Personally, I think this should not be a string comparison, but a different, more robust check.

I assume, that @Siedlerchr had something similarly in mind with his remark.

dialogService.showInformationDialogAndWait(Localization.lang("Lookup DOI"), Localization.lang("No DOI data exists"));
ThiloteE marked this conversation as resolved.
Show resolved Hide resolved
// server error
} else if (localMessage.startsWith("Server")) {
dialogService.showInformationDialogAndWait(Localization.lang("Lookup DOI"), Localization.lang("DOI server not available"));
// default error
} else {
dialogService.showErrorDialogAndWait(exception);
}
})
.executeWith(Globals.TASK_EXECUTOR);
}
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/jabref/logic/net/URLDownload.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;

import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.io.FileUtil;
import org.jabref.model.util.FileHelper;

Expand Down Expand Up @@ -360,6 +361,14 @@ private URLConnection openConnection() throws IOException {
connection = new URLDownload(newUrl).openConnection();
}
}
// status code 4xx
if (status > HttpURLConnection.HTTP_BAD_REQUEST && status < HttpURLConnection.HTTP_INTERNAL_ERROR) {
throw new IOException(Localization.lang("Client Error " + status));

// status code 5xx
} else if (status >= HttpURLConnection.HTTP_INTERNAL_ERROR) {
throw new IOException(Localization.lang("Server Error " + status));
}
}

// this does network i/o: GET + read returned headers
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/org/jabref/logic/net/URLDownloadTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ public void testStringDownloadWithSetEncoding() throws IOException {
assertTrue(dl.asString().contains("Google"), "google.com should contain google");
}

// test client error 404
@Test
public void testStringDownloadThrowClientError() throws IOException {
URLDownload dl = new URLDownload(new URL("http://httpstat.us/404"));
IOException exception = assertThrows(IOException.class, dl::asString);
assertTrue(exception.getLocalizedMessage().contentEquals("Client Error 404"), "Should be Client Error.");
}

// test server error 503
@Test
public void testStringDownloadThrowServerError() throws IOException {
URLDownload dl = new URLDownload(new URL("http://httpstat.us/503"));
IOException exception = assertThrows(IOException.class, dl::asString);
assertTrue(exception.getLocalizedMessage().contentEquals("Server Error 503"), "Should be Server Error.");
}

@Test
public void testStringDownload() throws IOException {
URLDownload dl = new URLDownload(new URL("http://www.google.com"));
Expand Down