You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@reef.apache.org by we...@apache.org on 2017/05/06 01:34:12 UTC

reef git commit: [REEF-1787] Unmanaged REEF on Spark packages global.jar resources incorrectly

Repository: reef
Updated Branches:
  refs/heads/master 4551b471e -> 49c1414d6


[REEF-1787] Unmanaged REEF on Spark packages global.jar resources incorrectly

Fix `JARFileMaker` class to handle symlinks to symlinks properly;
add logging and refactor for readability and maintainability;
add collectionutils tools for empty or null arrays.

JIRA:
  [REEF-1787](https://issues.apache.org/jira/browse/REEF-1787)

Pull Request:
  This closes #1299


Project: http://git-wip-us.apache.org/repos/asf/reef/repo
Commit: http://git-wip-us.apache.org/repos/asf/reef/commit/49c1414d
Tree: http://git-wip-us.apache.org/repos/asf/reef/tree/49c1414d
Diff: http://git-wip-us.apache.org/repos/asf/reef/diff/49c1414d

Branch: refs/heads/master
Commit: 49c1414d6ee4ee7dc803ff729388371bd55bfdb9
Parents: 4551b47
Author: Sergiy Matusevych <mo...@apache.org>
Authored: Wed Apr 26 17:21:20 2017 -0700
Committer: Markus Weimer <we...@apache.org>
Committed: Fri May 5 18:32:32 2017 -0700

----------------------------------------------------------------------
 .../org/apache/reef/util/CollectionUtils.java   | 15 ++-
 .../java/org/apache/reef/util/JARFileMaker.java | 96 +++++++++-----------
 .../apache/reef/util/CollectionUtilsTest.java   | 53 +++++++++++
 3 files changed, 109 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/reef/blob/49c1414d/lang/java/reef-common/src/main/java/org/apache/reef/util/CollectionUtils.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-common/src/main/java/org/apache/reef/util/CollectionUtils.java b/lang/java/reef-common/src/main/java/org/apache/reef/util/CollectionUtils.java
index f936390..593e42b 100644
--- a/lang/java/reef-common/src/main/java/org/apache/reef/util/CollectionUtils.java
+++ b/lang/java/reef-common/src/main/java/org/apache/reef/util/CollectionUtils.java
@@ -21,7 +21,7 @@ package org.apache.reef.util;
 import java.util.Collection;
 
 /**
- * Utilities for collection classes.
+ * Utility functions for arrays and collection classes.
  */
 public final class CollectionUtils {
 
@@ -29,7 +29,6 @@ public final class CollectionUtils {
     // avoid instantiation
   }
 
-
   /**
    * Checks if the collection is null or empty.
    * @param <T> a type of element of collection
@@ -50,5 +49,17 @@ public final class CollectionUtils {
     return !isEmpty(parameter);
   }
 
+  private static final Object[] EMPTY_ARRAY = new Object[0];
 
+  /**
+   * Return an empty array if input is null; works as identity function otherwise.
+   * This function is useful in `for` statements if the iterable can be null.
+   * @param array Input array. Can be null.
+   * @param <T> Type of the elements of the array.
+   * @return A reference to the input array if it is not null, an empty array otherwise.
+   */
+  @SuppressWarnings("unchecked")
+  public static <T> T[] nullToEmpty(final T[] array) {
+    return array == null ? (T[])EMPTY_ARRAY : array;
+  }
 }

http://git-wip-us.apache.org/repos/asf/reef/blob/49c1414d/lang/java/reef-common/src/main/java/org/apache/reef/util/JARFileMaker.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-common/src/main/java/org/apache/reef/util/JARFileMaker.java b/lang/java/reef-common/src/main/java/org/apache/reef/util/JARFileMaker.java
index 418c880..c235ec3 100644
--- a/lang/java/reef-common/src/main/java/org/apache/reef/util/JARFileMaker.java
+++ b/lang/java/reef-common/src/main/java/org/apache/reef/util/JARFileMaker.java
@@ -34,87 +34,77 @@ public class JARFileMaker implements AutoCloseable {
 
   private static final Logger LOG = Logger.getLogger(JARFileMaker.class.getName());
 
-  private final FileOutputStream fileOutputStream;
   private final JarOutputStream jarOutputStream;
-  private String relativeStartCanonicalPath = null;
 
-  public JARFileMaker(final File outputFile, final Manifest manifest) throws IOException {
-    this.fileOutputStream = new FileOutputStream(outputFile);
-    this.jarOutputStream = new JarOutputStream(this.fileOutputStream, manifest);
+  public JARFileMaker(final File outputFile) throws IOException {
+    this(outputFile, null);
   }
 
-  public JARFileMaker(final File outputFile) throws IOException {
-    this.fileOutputStream = new FileOutputStream(outputFile);
-    this.jarOutputStream = new JarOutputStream(this.fileOutputStream);
+  public JARFileMaker(final File outputFile, final Manifest manifest) throws IOException {
+    LOG.log(Level.FINER, "Output jar: {0}", outputFile);
+    final FileOutputStream outputStream = new FileOutputStream(outputFile);
+    this.jarOutputStream = manifest == null ?
+        new JarOutputStream(outputStream) : new JarOutputStream(outputStream, manifest);
   }
 
   /**
    * Adds a file to the JAR. If inputFile is a folder, it will be added recursively.
-   *
-   * @param inputFile
-   * @throws IOException
+   * @param inputFile file or directory to be added to the jar.
+   * @throws IOException if cannot create a jar.
    */
   public JARFileMaker add(final File inputFile) throws IOException {
+    return this.add(inputFile, null);
+  }
 
-    final String fileNameInJAR = makeRelative(inputFile);
-    if (inputFile.isDirectory()) {
-      final JarEntry entry = new JarEntry(fileNameInJAR);
-      entry.setTime(inputFile.lastModified());
-      this.jarOutputStream.putNextEntry(entry);
-      this.jarOutputStream.closeEntry();
-      final File[] files = inputFile.listFiles();
-      if (files != null) {
-        for (final File nestedFile : files) {
-          add(nestedFile);
-        }
-      }
-      return this;
+  public JARFileMaker addChildren(final File folder) throws IOException {
+    LOG.log(Level.FINEST, "Add children: {0}", folder);
+    for (final File nestedFile : CollectionUtils.nullToEmpty(folder.listFiles())) {
+      this.add(nestedFile);
     }
+    return this;
+  }
+
+  private JARFileMaker add(final File inputFile, final String prefix) throws IOException {
+
+    final String fileNameInJAR = createPathInJar(inputFile, prefix);
+    LOG.log(Level.FINEST, "Add {0} as {1}", new Object[] {inputFile, fileNameInJAR});
 
     final JarEntry entry = new JarEntry(fileNameInJAR);
     entry.setTime(inputFile.lastModified());
     this.jarOutputStream.putNextEntry(entry);
-    try (final BufferedInputStream in = new BufferedInputStream(new FileInputStream(inputFile))) {
-      IOUtils.copy(in, this.jarOutputStream);
-      this.jarOutputStream.closeEntry();
-    } catch (final FileNotFoundException ex) {
-      LOG.log(Level.WARNING, "Skip the file: " + inputFile, ex);
-    }
-    return this;
-  }
 
-  public JARFileMaker addChildren(final File folder) throws IOException {
-    this.relativeStartCanonicalPath = folder.getCanonicalPath();
-    final File[] files = folder.listFiles();
-    if (files != null) {
-      for (final File f : files) {
-        this.add(f);
+    if (inputFile.isDirectory()) {
+      this.jarOutputStream.closeEntry();
+      for (final File nestedFile : CollectionUtils.nullToEmpty(inputFile.listFiles())) {
+        this.add(nestedFile, fileNameInJAR);
+      }
+    } else {
+      try (final BufferedInputStream in = new BufferedInputStream(new FileInputStream(inputFile))) {
+        IOUtils.copy(in, this.jarOutputStream);
+      } catch (final FileNotFoundException ex) {
+        LOG.log(Level.WARNING, "Skip the file: " + inputFile, ex);
+      } finally {
+        this.jarOutputStream.closeEntry();
       }
     }
-    this.relativeStartCanonicalPath = null;
+
     return this;
   }
 
-  private String makeRelative(final File input) throws IOException {
-    final String result;
-    if (this.relativeStartCanonicalPath == null) {
-      result = input.getCanonicalPath();
-    } else {
-      result = input.getCanonicalPath()
-          .replace(this.relativeStartCanonicalPath, "") // Drop the absolute prefix
-          .substring(1);                                // drop the '/' at the beginning
+  private static String createPathInJar(final File inputFile, final String prefix) {
+    final StringBuilder buf = new StringBuilder();
+    if (prefix != null) {
+      buf.append(prefix);
     }
-    if (input.isDirectory()) {
-      return result.replace("\\", "/") + "/";
-    } else {
-      return result.replace("\\", "/");
+    buf.append(inputFile.getName());
+    if (inputFile.isDirectory()) {
+      buf.append('/');
     }
-
+    return buf.toString();
   }
 
   @Override
   public void close() throws IOException {
     this.jarOutputStream.close();
-    this.fileOutputStream.close();
   }
 }

http://git-wip-us.apache.org/repos/asf/reef/blob/49c1414d/lang/java/reef-common/src/test/java/org/apache/reef/util/CollectionUtilsTest.java
----------------------------------------------------------------------
diff --git a/lang/java/reef-common/src/test/java/org/apache/reef/util/CollectionUtilsTest.java b/lang/java/reef-common/src/test/java/org/apache/reef/util/CollectionUtilsTest.java
new file mode 100644
index 0000000..dd35d0d
--- /dev/null
+++ b/lang/java/reef-common/src/test/java/org/apache/reef/util/CollectionUtilsTest.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.reef.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests for CollectionUtils functions.
+ */
+public final class CollectionUtilsTest {
+
+  @Test
+  public void testNullToEmptyNull() {
+    Assert.assertNotNull("Method should never return null", CollectionUtils.nullToEmpty(null));
+  }
+
+  @Test
+  public void testNullToEmptyNull2() {
+    final Integer[] array = new Integer[0];
+    Assert.assertArrayEquals("Must return an empty array", array, CollectionUtils.nullToEmpty(null));
+  }
+
+  @Test
+  public void testNullToEmptyIntegers() {
+    final Integer[] array = new Integer[] {1, 2, 3};
+    Assert.assertArrayEquals("Must return the same array", array, CollectionUtils.nullToEmpty(array));
+    Assert.assertTrue("Must return reference to the same object", array == CollectionUtils.nullToEmpty(array));
+  }
+
+  @Test
+  public void testNullToEmptyZeroLength() {
+    final Integer[] array = new Integer[0];
+    Assert.assertArrayEquals("Must return the same array", array, CollectionUtils.nullToEmpty(array));
+    Assert.assertTrue("Must return reference to the same object", array == CollectionUtils.nullToEmpty(array));
+  }
+}