kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ij...@apache.org
Subject kafka git commit: MINOR: Java 9 version handling improvements
Date Wed, 04 Oct 2017 07:06:09 GMT
Repository: kafka
Updated Branches:
  refs/heads/trunk cbef33f3d -> 28aaba281


MINOR: Java 9 version handling improvements

- Upgrade Gradle to 4.2.1, which handles Azul Zulu 9's version
correctly.
- Add tests to our Java version handling code
- Refactor the code to make it possible to add tests
- Rename `isIBMJdk` method to use consistent naming
convention.

Author: Ismael Juma <ismael@juma.me.uk>

Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>

Closes #4007 from ijuma/java-9-version-handling-improvements


Project: http://git-wip-us.apache.org/repos/asf/kafka/repo
Commit: http://git-wip-us.apache.org/repos/asf/kafka/commit/28aaba28
Tree: http://git-wip-us.apache.org/repos/asf/kafka/tree/28aaba28
Diff: http://git-wip-us.apache.org/repos/asf/kafka/diff/28aaba28

Branch: refs/heads/trunk
Commit: 28aaba2818053ffc36b1ae65b7714638ef41b91e
Parents: cbef33f
Author: Ismael Juma <ismael@juma.me.uk>
Authored: Wed Oct 4 08:06:05 2017 +0100
Committer: Ismael Juma <ismael@juma.me.uk>
Committed: Wed Oct 4 08:06:05 2017 +0100

----------------------------------------------------------------------
 build.gradle                                    |  2 +-
 .../common/network/SaslChannelBuilder.java      |  2 +-
 .../org/apache/kafka/common/utils/Java.java     | 59 ++++++++++++++------
 .../org/apache/kafka/common/utils/JavaTest.java | 46 ++++++++++++++-
 .../scala/kafka/security/minikdc/MiniKdc.scala  |  2 +-
 .../scala/unit/kafka/utils/JaasTestUtils.scala  |  6 +-
 6 files changed, 90 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kafka/blob/28aaba28/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index d7b799b..4b7c10c 100644
--- a/build.gradle
+++ b/build.gradle
@@ -77,7 +77,7 @@ allprojects {
 }
 
 ext {
-  gradleVersion = "4.2"
+  gradleVersion = "4.2.1"
   buildVersionFileName = "kafka-version.properties"
 
   maxPermSizeArgs = []

http://git-wip-us.apache.org/repos/asf/kafka/blob/28aaba28/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
----------------------------------------------------------------------
diff --git a/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
b/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
index d00442e..5e1e407 100644
--- a/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
+++ b/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
@@ -177,7 +177,7 @@ public class SaslChannelBuilder implements ChannelBuilder {
         Class<?> classRef;
         Method getInstanceMethod;
         Method getDefaultRealmMethod;
-        if (Java.isIBMJdk()) {
+        if (Java.isIbmJdk()) {
             classRef = Class.forName("com.ibm.security.krb5.internal.Config");
         } else {
             classRef = Class.forName("sun.security.krb5.Config");

http://git-wip-us.apache.org/repos/asf/kafka/blob/28aaba28/clients/src/main/java/org/apache/kafka/common/utils/Java.java
----------------------------------------------------------------------
diff --git a/clients/src/main/java/org/apache/kafka/common/utils/Java.java b/clients/src/main/java/org/apache/kafka/common/utils/Java.java
index 38d9541..fb8cafa 100644
--- a/clients/src/main/java/org/apache/kafka/common/utils/Java.java
+++ b/clients/src/main/java/org/apache/kafka/common/utils/Java.java
@@ -20,32 +20,55 @@ import java.util.StringTokenizer;
 
 public final class Java {
 
-    private Java() {
+    private Java() { }
+
+    private static final Version VERSION = parseVersion(System.getProperty("java.specification.version"));
+
+    // Package private for testing
+    static Version parseVersion(String versionString) {
+        final StringTokenizer st = new StringTokenizer(versionString, ".");
+        int majorVersion = Integer.parseInt(st.nextToken());
+        int minorVersion;
+        if (st.hasMoreTokens())
+            minorVersion = Integer.parseInt(st.nextToken());
+        else
+            minorVersion = 0;
+        return new Version(majorVersion, minorVersion);
     }
 
-    public static final String JVM_SPEC_VERSION = System.getProperty("java.specification.version");
+    // Having these as static final provides the best opportunity for compilar optimization
+    public static final boolean IS_JAVA9_COMPATIBLE = VERSION.isJava9Compatible();
+    public static final boolean IS_JAVA8_COMPATIBLE = VERSION.isJava8Compatible();
+
+    public static boolean isIbmJdk() {
+        return System.getProperty("java.vendor").contains("IBM");
+    }
 
-    private static final int JVM_MAJOR_VERSION;
-    private static final int JVM_MINOR_VERSION;
+    // Package private for testing
+    static class Version {
+        public final int majorVersion;
+        public final int minorVersion;
 
-    static {
-        final StringTokenizer st = new StringTokenizer(JVM_SPEC_VERSION, ".");
-        JVM_MAJOR_VERSION = Integer.parseInt(st.nextToken());
-        if (st.hasMoreTokens()) {
-            JVM_MINOR_VERSION = Integer.parseInt(st.nextToken());
-        } else {
-            JVM_MINOR_VERSION = 0;
+        private Version(int majorVersion, int minorVersion) {
+            this.majorVersion = majorVersion;
+            this.minorVersion = minorVersion;
         }
-    }
 
-    public static final boolean IS_JAVA9_COMPATIBLE = JVM_MAJOR_VERSION > 1 ||
-            (JVM_MAJOR_VERSION == 1 && JVM_MINOR_VERSION >= 9);
+        @Override
+        public String toString() {
+            return "Version(majorVersion=" + majorVersion +
+                    ", minorVersion=" + minorVersion + ")";
+        }
 
-    public static final boolean IS_JAVA8_COMPATIBLE = JVM_MAJOR_VERSION > 1 ||
-            (JVM_MAJOR_VERSION == 1 && JVM_MINOR_VERSION >= 8);
+        // Package private for testing
+        boolean isJava9Compatible() {
+            return majorVersion >= 9;
+        }
 
-    public static boolean isIBMJdk() {
-        return System.getProperty("java.vendor").contains("IBM");
+        // Package private for testing
+        boolean isJava8Compatible() {
+            return majorVersion > 1 || (majorVersion == 1 && minorVersion >=
8);
+        }
     }
 
 }

http://git-wip-us.apache.org/repos/asf/kafka/blob/28aaba28/clients/src/test/java/org/apache/kafka/common/utils/JavaTest.java
----------------------------------------------------------------------
diff --git a/clients/src/test/java/org/apache/kafka/common/utils/JavaTest.java b/clients/src/test/java/org/apache/kafka/common/utils/JavaTest.java
index 057ff50..4810f92 100644
--- a/clients/src/test/java/org/apache/kafka/common/utils/JavaTest.java
+++ b/clients/src/test/java/org/apache/kafka/common/utils/JavaTest.java
@@ -16,6 +16,7 @@
  */
 package org.apache.kafka.common.utils;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -40,17 +41,56 @@ public class JavaTest {
     @Test
     public void testIsIBMJdk() {
         System.setProperty("java.vendor", "Oracle Corporation");
-        assertFalse(Java.isIBMJdk());
+        assertFalse(Java.isIbmJdk());
         System.setProperty("java.vendor", "IBM Corporation");
-        assertTrue(Java.isIBMJdk());
+        assertTrue(Java.isIbmJdk());
     }
 
     @Test
     public void testLoadKerberosLoginModule() throws ClassNotFoundException {
-        String clazz = Java.isIBMJdk()
+        String clazz = Java.isIbmJdk()
                 ? "com.ibm.security.auth.module.Krb5LoginModule"
                 : "com.sun.security.auth.module.Krb5LoginModule";
         Class.forName(clazz);
     }
 
+    @Test
+    public void testJavaVersion() {
+        Java.Version v = Java.parseVersion("9");
+        assertEquals(9, v.majorVersion);
+        assertEquals(0, v.minorVersion);
+        assertTrue(v.isJava9Compatible());
+        assertTrue(v.isJava8Compatible());
+
+        v = Java.parseVersion("9.0.1");
+        assertEquals(9, v.majorVersion);
+        assertEquals(0, v.minorVersion);
+        assertTrue(v.isJava9Compatible());
+        assertTrue(v.isJava8Compatible());
+
+        v = Java.parseVersion("9.0.0.15"); // Azul Zulu
+        assertEquals(9, v.majorVersion);
+        assertEquals(0, v.minorVersion);
+        assertTrue(v.isJava9Compatible());
+        assertTrue(v.isJava8Compatible());
+
+        v = Java.parseVersion("9.1");
+        assertEquals(9, v.majorVersion);
+        assertEquals(1, v.minorVersion);
+        assertTrue(v.isJava9Compatible());
+        assertTrue(v.isJava8Compatible());
+
+        v = Java.parseVersion("1.8.0_152");
+        assertEquals(1, v.majorVersion);
+        assertEquals(8, v.minorVersion);
+        assertFalse(v.isJava9Compatible());
+        assertTrue(v.isJava8Compatible());
+
+        v = Java.parseVersion("1.7.0_80");
+        assertEquals(1, v.majorVersion);
+        assertEquals(7, v.minorVersion);
+        assertFalse(v.isJava9Compatible());
+        assertFalse(v.isJava8Compatible());
+
+    }
 }

http://git-wip-us.apache.org/repos/asf/kafka/blob/28aaba28/core/src/test/scala/kafka/security/minikdc/MiniKdc.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/kafka/security/minikdc/MiniKdc.scala b/core/src/test/scala/kafka/security/minikdc/MiniKdc.scala
index 2ff281b..6c964d7 100644
--- a/core/src/test/scala/kafka/security/minikdc/MiniKdc.scala
+++ b/core/src/test/scala/kafka/security/minikdc/MiniKdc.scala
@@ -261,7 +261,7 @@ class MiniKdc(config: Properties, workDir: File) extends Logging {
 
   private def refreshJvmKerberosConfig(): Unit = {
     val klass =
-      if (Java.isIBMJdk)
+      if (Java.isIbmJdk)
         Class.forName("com.ibm.security.krb5.internal.Config")
       else
         Class.forName("sun.security.krb5.Config")

http://git-wip-us.apache.org/repos/asf/kafka/blob/28aaba28/core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala b/core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala
index cd31f2c..1f9ccf3 100644
--- a/core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala
+++ b/core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala
@@ -31,13 +31,13 @@ object JaasTestUtils {
                              serviceName: Option[String]) extends JaasModule {
 
     def name =
-      if (Java.isIBMJdk)
+      if (Java.isIbmJdk)
         "com.ibm.security.auth.module.Krb5LoginModule"
       else
         "com.sun.security.auth.module.Krb5LoginModule"
 
     def entries: Map[String, String] =
-      if (Java.isIBMJdk)
+      if (Java.isIbmJdk)
         Map(
           "principal" -> principal,
           "credsType" -> "both"
@@ -142,7 +142,7 @@ object JaasTestUtils {
     }
     // IBM Kerberos module doesn't support the serviceName JAAS property, hence it needs
to be
     // passed as a Kafka property
-    if (Java.isIBMJdk && !result.contains(KafkaConfig.SaslKerberosServiceNameProp))
+    if (Java.isIbmJdk && !result.contains(KafkaConfig.SaslKerberosServiceNameProp))
       result.put(KafkaConfig.SaslKerberosServiceNameProp, serviceName)
     result
   }


Mime
View raw message