Add connection check #6565

chenyuheng · 2020-05-31T08:38:21Z

I added the connection check function in the network tab of preference settings.

The idea is from #6560, and the basic design is follow the design in IntelliJ. It could be used for checking connection mainly for proxy.

In my implementation, I checked the status code of response, if it is 200, of course the connection is ok, if not, it will give the corresponding statute code or other cause in exception message.

Screenshots

the check connection button

check dialog

check success dialog

check fail dialog

Change in CHANGELOG.md described (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

as JabRef#6560 suggested

koppor

Thank you for the PR. I have some implementation design comments.

koppor · 2020-06-16T19:46:04Z

src/main/java/org/jabref/gui/preferences/NetworkTabViewModel.java

+        settingsPane.setVgap(4.0);
+
+        Label messageLabel = new Label();
+        messageLabel.setText(Localization.lang("Warning: your settings will be saved.\nEnter any URL to check connection to:"));


Why the Warning: your settings will be saved.? I would just remove that.

koppor · 2020-06-16T19:46:32Z

src/main/java/org/jabref/gui/preferences/NetworkTabViewModel.java

+                .ifPresent(btn -> {
+                            if (btn == ButtonType.OK) {
+                                String connectionProblemText = Localization.lang("Problem with connection: ");
+                                String connectionSuccessText = Localization.lang("Connection Successful!");


Suggested change

String connectionSuccessText = Localization.lang("Connection Successful!");

String connectionSuccessText = Localization.lang("Connection successful!");

No capitalization in text.

koppor · 2020-06-16T19:46:52Z

src/main/java/org/jabref/logic/net/URLDownload.java

+    /**
+     * Check the connection by using the HEAD request.
+     * UnirestException can be thrown for invalid request.
+     * @return the status code of the response


Please add an empty line above

koppor · 2020-06-16T19:47:39Z

src/main/java/org/jabref/logic/net/URLDownload.java

+        try {
+            int statusCode = Unirest.head(source.toString()).asString().getStatus();
+            return statusCode;
+        } catch (UnirestException e) {


(obsolte with the refactoring to Boolean return value)

Why this statement? Can't the try..catch constrcut just be removed and a throws UnirestException added to the method?

koppor · 2020-06-16T19:48:19Z

src/test/java/org/jabref/logic/net/URLDownloadTest.java

+        URLDownload nonsense = new URLDownload(new URL("http://nonsenseadddress"));
+        try {
+            int statusCode = nonsense.checkConnection();
+            fail();


(obsolte with the refactoring to Boolean return value)

Why this statement? Can't the try..catch constrcut just be removed and a throws UnirestException added to the method?

Please use AssertThrows for that. Details: https://howtodoinjava.com/junit5/expected-exception-example/

koppor · 2020-06-16T19:54:07Z

src/main/java/org/jabref/logic/net/URLDownload.java

+     * UnirestException can be thrown for invalid request.
+     * @return the status code of the response
+     */
+    public int checkConnection() {


This is a new method. So, you can work an a proper interface.

Why not?

Suggested change

public int checkConnection() {

public boolean canBeReached() {

See Item 69: Use exceptions only for exceptional conditions of Effective Java.

You can log the status code - and the exception (debug level). I think, the end user is not interested in the concrete response code, is he?

The mapping of response codes and exceptions to a proper user information is hard. All responses not falling in the class 2xx are errors. Thus, both the codes and the exceptions should be mapped to a user message. -- To design that properly is too hard. Just go for a boolean response.

koppor · 2020-06-16T19:59:21Z

src/main/java/org/jabref/gui/preferences/NetworkTabViewModel.java

+                                URLDownload dl;
+                                try {
+                                    dl = new URLDownload(url.getText());
+                                    int connectionStatus = dl.checkConnection();


See below - this is IMHO not a proper design for the check. Moreover, any value in the 2xx class is a success. Thus, I would add a proper abstraction (see comment below).

koppor · 2020-06-16T20:00:38Z

...and sorry for the long delay. We are busy in preparing the next release (and the whole project happens in our (unpaid) free time).

koppor · 2020-07-07T06:38:15Z

This is a good start. Since there was no activity since a few weeks, we would like to ask whether there is some interest in continuing in this.

calixtus · 2020-08-18T06:47:59Z

I'll take a look on this. @chenyuheng , thanks for your work. We'll see if we can finish this one.

calixtus · 2020-09-01T23:56:58Z

Hi @chenyuheng , we finished your PR on JabCon2020, as we liked the idea. We hope you don't mind. Thanks for your work!

4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 4b4e8f442d2de454c638393fbb9dd911c8b7aca7

80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 80b3861bce121a64d43ef167581f8d100a4f70aa

…ce (#10048) 80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 80b3861bce121a64d43ef167581f8d100a4f70aa Co-authored-by: github actions <jabrefmail+webfeedback@gmail.com>

a97dbb32c5 Update studii-teologice.csl (#6591) e19e08780e Update acm-sig-proceedings-long-author-list.csl (#6594) c8abbcdd88 Update acm-sig-proceedings.csl (#6595) 725ace4a40 Create uppsala-university-library-harvard.csl (#6574) a973041e0e update bluebook-law-review.csl (#6583) 0891cfc49a Update masarykova-univerzita-pravnicka-fakulta.csl (#6589) 80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: a97dbb32c57c8c00e47422dae4ba4f480e753da5

0749a19b83 Update journal-of-experimental-botany.csl (#6604) b1768724fe Update modern-language-association.csl (#6606) dd364c1815 Create modern-language-association-for-JEI.csl (#6593) 6cb436997b Partial update of APA styles for 1.0.2, including software, legal, most localizations (#6270) d7c4ebec85 fix film/video authors for american-sociological-association.csl (#6602) a97dbb32c5 Update studii-teologice.csl (#6591) e19e08780e Update acm-sig-proceedings-long-author-list.csl (#6594) c8abbcdd88 Update acm-sig-proceedings.csl (#6595) 725ace4a40 Create uppsala-university-library-harvard.csl (#6574) a973041e0e update bluebook-law-review.csl (#6583) 0891cfc49a Update masarykova-univerzita-pravnicka-fakulta.csl (#6589) 80b3861bce Update al-jamiah-journal-of-islamic-studies.csl (#6581) b6fb00e415 Create arachnologische-mitteilungen.csl (#6375) 2dbc8edf8e Update universitat-basel-iberoromanistik.csl (#6580) fd230a7073 Create veterinary-clinical-pathology.csl (#6372) ac0afa3cae Create dedicated Basque APA file (#6370) fee0677a88 Update chicago-author-date.csl (#6289) ca1bf2db6e Create van-yuzuncu-yil-universitesi-fen-bilimleri-enstitusu.csl (#6230) f4116db325 Create Turcica.csl (#6240) 0e4311a802 Create jurnal-teknik-mesin-indonesia.csl (#6211) e73bf46674 Update vancouver.csl (#6156) b73c3ef193 Create independent TF AIP Style befa82e7ef Create harvard-xi-an-jiaotong-liverpool-univeisity.csl (#6181) 048c9bddbc Add csl for SDMI (#6129) 1c2aedd088 Update and rename dependent/energy-research-and-social-science.csl to energy-research-and-social-science.csl (#6567) b77084255f Update the-quarterly-journal-of-economics.csl (#6572) cf66f60f25 Update publicatiewijzer-voor-de-archeologie.csl (#6577) 9e9e08c219 Update isara-iso-690.csl (#6578) 4b4e8f442d Create silva-fennica.csl (#6568) dd8760bb2b Update american-journal-of-botany.csl (#6569) 8b0e505363 Update haute-ecole-de-gestion-de-geneve-iso-690.csl (#6560) 016050c4b7 Update geographie-et-cultures.csl (#6563) e8b62f1c80 Update and rename dependent/retina.csl to retina.csl (#6565) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 0749a19b8306f2e8dcb9bf1a2e3a6992666030ac

chenyuheng added 6 commits May 31, 2020 15:05

add checking connection function

ece765b

as JabRef#6560 suggested

checkstyle fix

c3baca0

add javadoc

94d1386

add tests

3c2252b

trivial fix

0816a99

Update CHANGELOG.md

50af664

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 13, 2020

koppor requested changes Jun 16, 2020

View reviewed changes

koppor added status: changes required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jun 23, 2020

koppor added the status: stale label Jul 7, 2020

calixtus self-assigned this Aug 18, 2020

calixtus mentioned this pull request Sep 1, 2020

Add connection check to NetworkTab in preferences #6838

Merged

5 tasks

calixtus closed this Sep 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add connection check #6565

Add connection check #6565

chenyuheng commented May 31, 2020 •

edited

Loading

koppor left a comment

koppor Jun 16, 2020

koppor Jun 16, 2020

koppor Jun 16, 2020

koppor Jun 16, 2020

koppor Jun 16, 2020

koppor Jun 16, 2020

koppor Jun 16, 2020

koppor commented Jun 16, 2020

koppor commented Jul 7, 2020

calixtus commented Aug 18, 2020

calixtus commented Sep 1, 2020

	String connectionSuccessText = Localization.lang("Connection Successful!");
	String connectionSuccessText = Localization.lang("Connection successful!");

	public int checkConnection() {
	public boolean canBeReached() {

Add connection check #6565

Add connection check #6565

Conversation

chenyuheng commented May 31, 2020 • edited Loading

Screenshots

the check connection button

check dialog

check success dialog

check fail dialog

koppor left a comment

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor Jun 16, 2020

Choose a reason for hiding this comment

koppor commented Jun 16, 2020

koppor commented Jul 7, 2020

calixtus commented Aug 18, 2020

calixtus commented Sep 1, 2020

chenyuheng commented May 31, 2020 •

edited

Loading