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

[jgus]

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 <konstant...@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 <rha...@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
j...@apache.org.