Refactor code, make it more stable and add test

CHANGELOG.md

@@ -31,6 +31,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - Integrity check "Abbreviation Detection" detects abbreviated names for journals and booktitles based on the internal list instead of only looking for "." signs. Fixes [#3144](https://github.com/JabRef/jabref/issues/3144).
 - We added a dialog to show that JabRef is working on checking integrity. [#3358](https://github.com/JabRef/jabref/issues/3358)
 - We added an option to mass append to fields via the Quality -> set/clear/append/rename fields dialog. [#2721](https://github.com/JabRef/jabref/issues/2721)
+- We added a check on startup to ensure JabRef is run with an adequate Java version. [3310](https://github.com/JabRef/jabref/issues/3310)
 
 ### Fixed
  - We fixed the translation of \textendash in the entry preview [#3307](https://github.com/JabRef/jabref/issues/3307)

build.gradle

@@ -38,6 +38,7 @@ sourceCompatibility = 1.8
 targetCompatibility = 1.8
 mainClassName = "org.jabref.JabRefMain"
 
+// These are the Java version requirements we will check on each start of JabRef
 ext.minRequiredJavaVersion = "1.8.0_144"
 ext.allowJava9 = false
 

src/main/java/org/jabref/JabRefMain.java

@@ -27,7 +27,7 @@
 import org.jabref.logic.remote.RemotePreferences;
 import org.jabref.logic.remote.client.RemoteListenerClient;
 import org.jabref.logic.util.BuildInfo;
-import org.jabref.logic.util.JavaVersionCheck;
+import org.jabref.logic.util.JavaVersion;
 import org.jabref.logic.util.OS;
 import org.jabref.migrations.PreferencesMigrations;
 import org.jabref.model.EntryTypes;
@@ -57,26 +57,28 @@ public void start(Stage mainStage) throws Exception {
         SwingUtilities.invokeLater(() -> start(arguments));
     }
 
-    private static void start(String[] args) {
-        FallbackExceptionHandler.installExceptionHandler();
-
-        JabRefPreferences preferences = JabRefPreferences.getInstance();
-
-        ProxyPreferences proxyPreferences = preferences.getProxyPreferences();
-        ProxyRegisterer.register(proxyPreferences);
-        if (proxyPreferences.isUseProxy() && proxyPreferences.isUseAuthentication()) {
-            Authenticator.setDefault(new ProxyAuthenticator());
-        }
-
-        Globals.prefs = preferences;
-
+    /**
+     * Tests if we are running an acceptable Java and terminates JabRef when we are sure the version is not supported.
+     * This test uses the requirements for the Java version as specified in <code>gradle.build</code>. It is possible to
+     * define a minimum version including the built number and to indicate whether Java 9 can be use (which it currently
+     * can't). It tries to compare this version number to the version of the currently running JVM. The check is
+     * optimistic and will rather return true even if we could not exactly determine the version.
+     * <p>
+     * Note: Users with an very old version like 1.6 will not profit from this since class versions are incompatible and
+     * JabRef won't even start.
+     */
+    private static void ensureCorrectJavaVersion() {
         // Check if we are running an acceptable version of Java
         final BuildInfo buildInfo = Globals.BUILD_INFO;
-        if (!buildInfo.isAcceptableJavaVersion()) {
+        JavaVersion checker = new JavaVersion();
+        final boolean java9Fail = !buildInfo.isAllowJava9() && checker.isJava9();
+        final boolean versionFail = !checker.isAtLeast(buildInfo.getMinRequiredJavaVersion());
+
+        if (java9Fail || versionFail) {
             StringBuilder versionError = new StringBuilder(
                     Localization.lang("Your current Java version (%0) is not supported. Please install version %1 or higher.",
-                    JavaVersionCheck.getJavaVersion(),
-                    buildInfo.getMinRequiredJavaVersion()
+                            checker.getJavaVersion(),
+                            buildInfo.getMinRequiredJavaVersion()
                     )
             );
             if (!buildInfo.isAllowJava9()) {
@@ -86,8 +88,28 @@ private static void start(String[] args) {
             final JFrame frame = new JFrame();
             JOptionPane.showMessageDialog(frame, versionError, Localization.lang("Error"), JOptionPane.ERROR_MESSAGE);
             frame.dispose();
-            System.exit(0);
+
+            // We exit on Java 9 error since this will definitely not work
+            if (java9Fail) {
+                System.exit(0);
+            }
         }
+    }
+
+    private static void start(String[] args) {
+        FallbackExceptionHandler.installExceptionHandler();
+
+        JabRefPreferences preferences = JabRefPreferences.getInstance();
+
+        ensureCorrectJavaVersion();
+
+        ProxyPreferences proxyPreferences = preferences.getProxyPreferences();
+        ProxyRegisterer.register(proxyPreferences);
+        if (proxyPreferences.isUseProxy() && proxyPreferences.isUseAuthentication()) {
+            Authenticator.setDefault(new ProxyAuthenticator());
+        }
+
+        Globals.prefs = preferences;
         Globals.startBackgroundTasks();
 
         // Note that the language was already set during the initialization of the preferences and it is safe to

src/main/java/org/jabref/logic/util/BuildInfo.java

@@ -78,16 +78,4 @@ public String getMinRequiredJavaVersion() {
     public boolean isAllowJava9() {
         return allowJava9;
     }
-
-    /**
-     * Tests if we are running an acceptable Java. This test uses the requirements for the Java version as specified in
-     * <code>gradle.build</code>. It is possible to define a minimum version up to the built number and to indicate whether
-     * Java 9 can be use (which it currently can't). It tries to compare this version number to the version of the currently
-     * running JVM. The check is optimistic and will rather return true even if we could not exactly determine the version.
-     *
-     * @return false if we are sure the version is not sufficient; otherwise it returns true
-     */
-    public boolean isAcceptableJavaVersion() {
-        return (!JavaVersionCheck.isJava9() || allowJava9) && JavaVersionCheck.isAtLeast(minRequiredJavaVersion);
-    }
 }

src/main/java/org/jabref/logic/util/JavaVersionCheck.java → src/main/java/org/jabref/logic/util/JavaVersion.java

@@ -10,58 +10,67 @@
  * The version check should always happen through the <code>Globals#BUILD_INFO</code> instance which is available at a
  * very early stage in the JabRef startup.
  */
-public class JavaVersionCheck {
+public class JavaVersion {
 
-    // We expect version in the form 1.8.0_101 and need . and _ as delimiter to separate numbers
-    private static final Pattern DELIMITER = Pattern.compile("[._]");
-    // Be adventurous and assume that we can access this property always!
-    private static final String JAVA_VERSION = System.getProperty("java.version");
+    // See http://openjdk.java.net/jeps/223
+    private static final Pattern DELIMITER = Pattern.compile("[._\\-+]");
+    private final String JAVA_VERSION;
 
-    private JavaVersionCheck() {
+    public JavaVersion() {
+        // Be adventurous and assume that we can always access this property!
+        JAVA_VERSION = System.getProperty("java.version");
+    }
+
+    public JavaVersion(final String version) {
+        JAVA_VERSION = version;
     }
 
     /**
      * Tries to determine if we are running on Java 9. This test should return false, when we cannot extract the correct
-     * Java version.
+     * Java version. Note that Java 9 has a different version scheme like "9-internal".
      *
      * @return true if Java 9 is used
      */
-    static boolean isJava9() {
-        final String[] toParse = JAVA_VERSION.split(DELIMITER.pattern());
-        if (toParse.length >= 2) {
-            try {
-                final float versionNumber = Float.parseFloat(toParse[0] + '.' + toParse[1]);
-                return versionNumber >= 1.9;
-            } catch (final NumberFormatException nfe) {
-                // assume it's not Java 9
+    public boolean isJava9() {
+        if (JAVA_VERSION != null) {
+            // Since isAtLeast is very optimistic, we first need to check if we have a "number" in the version string
+            // at all. Otherwise we would get false-positives.
+            final Scanner scanner = new Scanner(JAVA_VERSION);
+            scanner.useDelimiter(DELIMITER);
+            if (scanner.hasNextInt()) {
+                return isAtLeast("1.9");
             }
         }
         return false;
     }
 
     /**
-     * A very optimistic test for ensuring we at least have a minimal required Java version.
+     * A very optimistic test for ensuring we at least have a minimal required Java version. It will not fail when we
+     * cannot determine the result. In essence, this method splits a version string using {@link
+     * JavaVersion#DELIMITER} and compares two version number by number.
      *
      * @param version Should be in the form X.X.X_XXX where X are integers.
      * @return true if the numbers in version available for comparison are all greater-equals the currently running Java
      * version.
      */
-    static boolean isAtLeast(final String version) {
+    public boolean isAtLeast(final String version) {
+        if (JAVA_VERSION == null || version == null) {
+            return true;
+        }
         final Scanner scannerRunningVersion = new Scanner(JAVA_VERSION);
         final Scanner scannerRequiredVersion = new Scanner(version);
         scannerRunningVersion.useDelimiter(DELIMITER);
         scannerRequiredVersion.useDelimiter(DELIMITER);
         while (scannerRunningVersion.hasNextInt() && scannerRequiredVersion.hasNextInt()) {
             final int running = scannerRunningVersion.nextInt();
             final int required = scannerRequiredVersion.nextInt();
-            if (running < required) {
-                return false;
-            }
+            if (running == required) continue;
+            return running >= required;
         }
         return true;
     }
 
-    public static String getJavaVersion() {
+    public String getJavaVersion() {
         return JAVA_VERSION;
     }
 }

src/test/java/org/jabref/logic/util/JavaVersionTest.java

@@ -0,0 +1,81 @@
+package org.jabref.logic.util;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests for ensuring we can compare most appearing version strings
+ */
+public class JavaVersionTest {
+
+    private static List<String> java = new ArrayList<>();
+    private static List<String > java9 = new ArrayList<>();
+
+    private final static JavaVersion nullCheck = new JavaVersion(null);
+    private final static JavaVersion noSenseCheck = new JavaVersion("U.N.K.N.O.W.N");
+
+
+    static {
+        java.add("1.6.0_10"); // Oracle
+        java.add("1.6.0_45"); // Oracle
+        java.add("1.7.0_13"); // Oracle
+        java.add("1.8.0_76-release"); //openjdk
+        java.add("1.8.0_92"); //Oracle
+        java.add("1.8.0_111"); //Oracle
+        java.add("1.8.0_112-release"); //openjdk
+        java.add("1.8.0_152-release"); //openjdk
+        java.add("1.8.0_144"); //Oracle
+
+        // Examples http://openjdk.java.net/jeps/223
+        // Note that it might be possible that java 9 versions are either 9.1.4+8 or new style 1.9.0_31-b08
+        java9.add("9-internal"); // openjdk
+        java9.add("1.9.0_20-b62");
+        java9.add("1.9.0_20-b62");
+        java9.add("9.2.4+45");
+    }
+
+    @Test
+    public void isJava9() throws Exception {
+        // Check that all valid java versions below 9 are recognized as not java 9
+        for (String versionString : java) {
+            final JavaVersion java8 = new JavaVersion(versionString);
+            Assert.assertFalse(java8.isJava9());
+        }
+        // Check if all valid version 9 strings are recognized as being version 9
+        for (String version9String : java9) {
+            final JavaVersion java9 = new JavaVersion(version9String);
+            Assert.assertTrue(java9.isJava9());
+        }
+
+        // For impossible comparisons we assume it's not java 9
+        Assert.assertFalse(nullCheck.isJava9());
+        Assert.assertFalse(noSenseCheck.isJava9());
+    }
+
+    @Test
+    public void isAtLeast() throws Exception {
+        final JavaVersion java8 = new JavaVersion("1.8");
+        for (String version8 : java) {
+            Assert.assertTrue(java8.isAtLeast(version8));
+            final JavaVersion java8Example = new JavaVersion(version8);
+            Assert.assertTrue(java8Example.isAtLeast("1.5"));
+
+            // Check if we optimistically return true if we cannot determine the result
+            Assert.assertTrue(java8Example.isAtLeast(null));
+            Assert.assertTrue(nullCheck.isAtLeast(version8));
+            Assert.assertTrue(noSenseCheck.isAtLeast(version8));
+            Assert.assertTrue(java8Example.isAtLeast("useless"));
+
+
+            // Check against all java 9 entries in both directions
+            for (String version9 : java9) {
+                Assert.assertFalse(java8Example.isAtLeast(version9));
+                final JavaVersion java9 = new JavaVersion(version9);
+                Assert.assertTrue(java9.isAtLeast(version8));
+            }
+        }
+    }
+}