Add connection check to NetworkTab in preferences (#6838)

calixtus · chenyuheng · web-flow · commit 1b3c270c48f9 · 2020-09-02T02:05:32.000+02:00
* add checking connection function as #6560 suggested * checkstyle fix * add javadoc * add tests * trivial fix * Update CHANGELOG.md * Removed dialog * Refactored according to remarks in PR * Fixed CHANGELOG.md Co-authored-by: chenyuheng <pass@chenyuheng.cn>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 
 - We added a query parser and mapping layer to enable conversion of queries formulated in simplified lucene syntax by the user into api queries. [#6799](https://github.com/JabRef/jabref/pull/6799)
 - We added some basic functionality to customise the look of JabRef by importing a css theme file. [#5790](https://github.com/JabRef/jabref/issues/5790)
+- We added connection check function in network preference setting [#6560](https://github.com/JabRef/jabref/issues/6560)
 
 ### Changed
 
diff --git a/src/main/java/org/jabref/gui/preferences/NetworkTab.fxml b/src/main/java/org/jabref/gui/preferences/NetworkTab.fxml
@@ -53,5 +53,7 @@
                              GridPane.rowIndex="5"/>
         <Label fx:id="proxyAttentionLabel" styleClass="warning-message"
                text="%Attention: Password is stored in plain text!" GridPane.columnIndex="2" GridPane.rowIndex="5"/>
+        <Button fx:id="checkConnectionButton" text="%Check connection" onAction="#checkConnection"
+                prefWidth="200.0" GridPane.columnIndex="1" GridPane.rowIndex="6"/>
     </GridPane>
 </fx:root>
diff --git a/src/main/java/org/jabref/gui/preferences/NetworkTabView.java b/src/main/java/org/jabref/gui/preferences/NetworkTabView.java
@@ -40,6 +40,7 @@ public class NetworkTabView extends AbstractPreferenceTabView<NetworkTabViewMode
     @FXML private Label proxyPasswordLabel;
     @FXML private CustomPasswordField proxyPassword;
     @FXML private Label proxyAttentionLabel;
+    @FXML private Button checkConnectionButton;
 
     private String proxyPasswordText = "";
     private int proxyPasswordCaretPosition = 0;
@@ -120,4 +121,9 @@ private void proxyPasswordMask(MouseEvent event) {
             proxyPasswordCaretPosition = 0;
         }
     }
+
+    @FXML
+    void checkConnection() {
+        viewModel.checkConnection();
+    }
 }
diff --git a/src/main/java/org/jabref/gui/preferences/NetworkTabViewModel.java b/src/main/java/org/jabref/gui/preferences/NetworkTabViewModel.java
@@ -1,5 +1,6 @@
 package org.jabref.gui.preferences;
 
+import java.net.MalformedURLException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -15,6 +16,7 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.net.ProxyPreferences;
 import org.jabref.logic.net.ProxyRegisterer;
+import org.jabref.logic.net.URLDownload;
 import org.jabref.logic.remote.RemotePreferences;
 import org.jabref.logic.remote.RemoteUtil;
 import org.jabref.model.strings.StringUtil;
@@ -25,6 +27,7 @@
 import de.saxsys.mvvmfx.utils.validation.ValidationMessage;
 import de.saxsys.mvvmfx.utils.validation.ValidationStatus;
 import de.saxsys.mvvmfx.utils.validation.Validator;
+import kong.unirest.UnirestException;
 
 public class NetworkTabViewModel implements PreferenceTabViewModel {
     private final BooleanProperty remoteServerProperty = new SimpleBooleanProperty();
@@ -44,16 +47,16 @@ public class NetworkTabViewModel implements PreferenceTabViewModel {
 
     private final DialogService dialogService;
     private final PreferencesService preferences;
-    private final RemotePreferences remotePreferences;
-    private final ProxyPreferences proxyPreferences;
+    private final RemotePreferences initialRemotePreferences;
+    private final ProxyPreferences initialProxyPreferences;
 
     private final List<String> restartWarning = new ArrayList<>();
 
     public NetworkTabViewModel(DialogService dialogService, PreferencesService preferences) {
         this.dialogService = dialogService;
         this.preferences = preferences;
-        this.remotePreferences = preferences.getRemotePreferences();
-        this.proxyPreferences = preferences.getProxyPreferences();
+        this.initialRemotePreferences = preferences.getRemotePreferences();
+        this.initialProxyPreferences = preferences.getProxyPreferences();
 
         remotePortValidator = new FunctionBasedValidator<>(
                 remotePortProperty,
@@ -104,15 +107,19 @@ public NetworkTabViewModel(DialogService dialogService, PreferencesService prefe
     }
 
     public void setValues() {
-        remoteServerProperty.setValue(remotePreferences.useRemoteServer());
-        remotePortProperty.setValue(String.valueOf(remotePreferences.getPort()));
+        remoteServerProperty.setValue(initialRemotePreferences.useRemoteServer());
+        remotePortProperty.setValue(String.valueOf(initialRemotePreferences.getPort()));
 
-        proxyUseProperty.setValue(proxyPreferences.isUseProxy());
-        proxyHostnameProperty.setValue(proxyPreferences.getHostname());
-        proxyPortProperty.setValue(proxyPreferences.getPort());
-        proxyUseAuthenticationProperty.setValue(proxyPreferences.isUseAuthentication());
-        proxyUsernameProperty.setValue(proxyPreferences.getUsername());
-        proxyPasswordProperty.setValue(proxyPreferences.getPassword());
+        setProxyValues();
+    }
+
+    private void setProxyValues() {
+        proxyUseProperty.setValue(initialProxyPreferences.isUseProxy());
+        proxyHostnameProperty.setValue(initialProxyPreferences.getHostname());
+        proxyPortProperty.setValue(initialProxyPreferences.getPort());
+        proxyUseAuthenticationProperty.setValue(initialProxyPreferences.isUseAuthentication());
+        proxyUsernameProperty.setValue(initialProxyPreferences.getUsername());
+        proxyPasswordProperty.setValue(initialProxyPreferences.getPassword());
     }
 
     public void storeSettings() {
@@ -122,12 +129,12 @@ public void storeSettings() {
 
     private void storeRemoteSettings() {
         RemotePreferences newRemotePreferences = new RemotePreferences(
-                remotePreferences.getPort(),
+                initialRemotePreferences.getPort(),
                 remoteServerProperty.getValue()
         );
 
         getPortAsInt(remotePortProperty.getValue()).ifPresent(newPort -> {
-            if (remotePreferences.isDifferentPort(newPort)) {
+            if (initialRemotePreferences.isDifferentPort(newPort)) {
                 newRemotePreferences.setPort(newPort);
 
                 if (newRemotePreferences.useRemoteServer()) {
@@ -137,7 +144,7 @@ private void storeRemoteSettings() {
         });
 
         if (newRemotePreferences.useRemoteServer()) {
-            Globals.REMOTE_LISTENER.openAndStart(new JabRefMessageHandler(), remotePreferences.getPort());
+            Globals.REMOTE_LISTENER.openAndStart(new JabRefMessageHandler(), initialRemotePreferences.getPort());
         } else {
             Globals.REMOTE_LISTENER.stop();
         }
@@ -155,7 +162,7 @@ private void storeProxySettings() {
                 proxyPasswordProperty.getValue()
         );
 
-        if (!newProxyPreferences.equals(proxyPreferences)) {
+        if (!newProxyPreferences.equals(initialProxyPreferences)) {
             ProxyRegisterer.register(newProxyPreferences);
         }
         preferences.storeProxyPreferences(newProxyPreferences);
@@ -215,6 +222,39 @@ public boolean validateSettings() {
         return true;
     }
 
+    /**
+     * Check the connection by using the given url. Used for validating the http proxy.
+     * The checking result will be appear when request finished.
+     * The checking result could be either success or fail, if fail, the cause will be displayed.
+     */
+    public void checkConnection() {
+        final String connectionSuccessText = Localization.lang("Connection successful!");
+        final String connectionFailedText = Localization.lang("Connection failed!");
+        final String dialogTitle = Localization.lang("Check Proxy Setting");
+
+        final String testUrl = "http://jabref.org";
+
+        // Workaround for testing, since the URLDownload uses stored proxy settings, see
+        // preferences.storeProxyPreferences(...) below.
+        storeProxySettings();
+
+        URLDownload urlDownload;
+        try {
+            urlDownload = new URLDownload(testUrl);
+            if (urlDownload.canBeReached()) {
+                dialogService.showInformationDialogAndWait(dialogTitle, connectionSuccessText);
+            } else {
+                dialogService.showErrorDialogAndWait(dialogTitle, connectionFailedText);
+            }
+        } catch (MalformedURLException e) {
+            // Why would that happen? Because one of developers inserted a failing url in testUrl...
+        } catch (UnirestException e) {
+            dialogService.showErrorDialogAndWait(dialogTitle, connectionFailedText);
+        }
+
+        preferences.storeProxyPreferences(initialProxyPreferences);
+    }
+
     @Override
     public List<String> getRestartWarnings() {
         return restartWarning;
diff --git a/src/main/java/org/jabref/logic/net/URLDownload.java b/src/main/java/org/jabref/logic/net/URLDownload.java
@@ -42,6 +42,7 @@
 import org.jabref.model.util.FileHelper;
 
 import kong.unirest.Unirest;
+import kong.unirest.UnirestException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -171,6 +172,19 @@ public String getMimeType() {
         return "";
     }
 
+    /**
+     * Check the connection by using the HEAD request.
+     * UnirestException can be thrown for invalid request.
+     *
+     * @return the status code of the response
+     */
+    public boolean canBeReached() throws UnirestException {
+        Unirest.config().setDefaultHeader("User-Agent", "Mozilla/5.0 (Windows; U; WindowsNT 5.1; en-US; rv1.8.1.6) Gecko/20070725 Firefox/2.0.0.6");
+
+        int statusCode = Unirest.head(source.toString()).asString().getStatus();
+        return statusCode >= 200 && statusCode < 300;
+    }
+
     public boolean isMimeType(String type) {
         String mime = getMimeType();
 
@@ -289,8 +303,7 @@ public String toString() {
     }
 
     private void copy(InputStream in, Writer out, Charset encoding) throws IOException {
-        InputStream monitoredInputStream = in;
-        Reader r = new InputStreamReader(monitoredInputStream, encoding);
+        Reader r = new InputStreamReader(in, encoding);
         try (BufferedReader read = new BufferedReader(r)) {
 
             String line;
diff --git a/src/test/java/org/jabref/logic/net/URLDownloadTest.java b/src/test/java/org/jabref/logic/net/URLDownloadTest.java
@@ -2,15 +2,18 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 
 import org.jabref.support.DisabledOnCIServer;
 
+import kong.unirest.UnirestException;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class URLDownloadTest {
@@ -91,4 +94,19 @@ public void downloadOfHttpsSucceeds() throws IOException {
         Path path = ftp.toTemporaryFile();
         assertNotNull(path);
     }
+
+    @Test
+    public void testCheckConnectionSuccess() throws MalformedURLException {
+        URLDownload google = new URLDownload(new URL("http://www.google.com"));
+
+        assertTrue(google.canBeReached());
+    }
+
+    @Test
+    public void testCheckConnectionFail() throws MalformedURLException {
+        URLDownload nonsense = new URLDownload(new URL("http://nonsenseadddress"));
+
+        assertThrows(UnirestException.class, nonsense::canBeReached);
+    }
+
 }

Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,7 @@ public class NetworkTabView extends AbstractPreferenceTabView<NetworkTabViewMode`
`40`	`40`	`@FXML private Label proxyPasswordLabel;`
`41`	`41`	`@FXML private CustomPasswordField proxyPassword;`
`42`	`42`	`@FXML private Label proxyAttentionLabel;`
	`43`	`+ @FXML private Button checkConnectionButton;`
`43`	`44`
`44`	`45`	`private String proxyPasswordText = "";`
`45`	`46`	`private int proxyPasswordCaretPosition = 0;`
`@@ -120,4 +121,9 @@ private void proxyPasswordMask(MouseEvent event) {`
`120`	`121`	`proxyPasswordCaretPosition = 0;`
`121`	`122`	`}`
`122`	`123`	`}`
	`124`	`+`
	`125`	`+ @FXML`
	`126`	`+ void checkConnection() {`
	`127`	`+ viewModel.checkConnection();`
	`128`	`+ }`
`123`	`129`	`}`