[kafka] branch 1.0 updated: KAFKA-6467; Enforce layout of dependencies within a connect plugin to be deterministic (#4459)

[jg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jgus pushed a commit to branch 1.0
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/1.0 by this push:
     new 6cb5375  KAFKA-6467; Enforce layout of dependencies within a connect plugin to be deterministic (#4459)
6cb5375 is described below

commit 6cb53753ed08a9aa86608373bf17815caca4272c
Author: Konstantine Karantasis <ko...@confluent.io>
AuthorDate: Fri Jan 26 08:39:57 2018 -0800

    KAFKA-6467; Enforce layout of dependencies within a connect plugin to be deterministic (#4459)
    
    Adds alphanumeric ordering of dependencies as they added to a Connect plugin's class loader path.
    
    This makes the layout of the dependencies consistent across systems and deployments. Dependencies should still, in principle, not include conflicts and ideally order should not matter.
    
    Reviewers: Randall Hauch <rh...@gmail.com>, Jason Gustafson <ja...@confluent.io>
---
 .../connect/runtime/isolation/PluginUtils.java     |  3 ++-
 .../connect/runtime/isolation/PluginUtilsTest.java | 22 +++++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
index 9ef7c3a..c1774e9 100644
--- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
+++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java
@@ -34,6 +34,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
+import java.util.TreeSet;
 
 /**
  * Connect plugin utility methods.
@@ -208,7 +209,7 @@ public class PluginUtils {
      */
     public static List<Path> pluginUrls(Path topPath) throws IOException {
         boolean containsClassFiles = false;
-        Set<Path> archives = new HashSet<>();
+        Set<Path> archives = new TreeSet<>();
         LinkedList<DirectoryEntry> dfs = new LinkedList<>();
         Set<Path> visited = new HashSet<>();
 
diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
index f9532a6..61ece1c 100644
--- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
+++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginUtilsTest.java
@@ -26,6 +26,7 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -159,6 +160,22 @@ public class PluginUtilsTest {
     }
 
     @Test
+    public void testOrderOfPluginUrlsWithJars() throws Exception {
+        createBasicDirectoryLayout();
+        // Here this method is just used to create the files. The result is not used.
+        createBasicExpectedUrls();
+
+        List<Path> actual = PluginUtils.pluginUrls(pluginPath);
+        // 'simple-transform.jar' is created first. In many cases, without sorting within the
+        // PluginUtils, this jar will be placed before 'another-transform.jar'. However this is
+        // not guaranteed because a DirectoryStream does not maintain a certain order in its
+        // results. Besides this test case, sorted order in every call to assertUrls below.
+        int i = Arrays.toString(actual.toArray()).indexOf("another-transform.jar");
+        int j = Arrays.toString(actual.toArray()).indexOf("simple-transform.jar");
+        assertTrue(i < j);
+    }
+
+    @Test
     public void testPluginUrlsWithZips() throws Exception {
         createBasicDirectoryLayout();
 
@@ -262,9 +279,8 @@ public class PluginUtilsTest {
     }
 
     private void assertUrls(List<Path> expected, List<Path> actual) {
-        List<Path> actualCopy = new ArrayList<>(actual);
         Collections.sort(expected);
-        Collections.sort(actualCopy);
-        assertEquals(expected, actualCopy);
+        // not sorting 'actual' because it should be returned sorted from withing the PluginUtils.
+        assertEquals(expected, actual);
     }
 }

-- 
To stop receiving notification emails like this one, please contact
jgus@apache.org.