You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by js...@apache.org on 2017/09/01 18:42:45 UTC

geode git commit: GEODE-3547: Simplify behavior for non-writable deploy directory

Repository: geode
Updated Branches:
  refs/heads/develop dedfd8ec3 -> 836451a5a


GEODE-3547: Simplify behavior for non-writable deploy directory

 - Also fixes the flaky JarDeployerIntegrationTest


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/836451a5
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/836451a5
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/836451a5

Branch: refs/heads/develop
Commit: 836451a5acd939d54b1d45ad8701fac7ec54ac53
Parents: dedfd8e
Author: Jared Stewart <js...@pivotal.io>
Authored: Thu Aug 31 14:09:46 2017 -0700
Committer: Jared Stewart <js...@pivotal.io>
Committed: Fri Sep 1 11:42:13 2017 -0700

----------------------------------------------------------------------
 .../org/apache/geode/internal/JarDeployer.java  | 34 ++++++-----------
 .../internal/JarDeployerIntegrationTest.java    | 39 ++++----------------
 2 files changed, 19 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/836451a5/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java b/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
index c21fb9d..44b6ac1 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
@@ -18,10 +18,6 @@ import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
-import org.apache.commons.io.FileUtils;
-import org.apache.geode.internal.logging.LogService;
-import org.apache.logging.log4j.Logger;
-
 import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.FileInputStream;
@@ -47,6 +43,11 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
+import org.apache.commons.io.FileUtils;
+import org.apache.logging.log4j.Logger;
+
+import org.apache.geode.internal.logging.LogService;
+
 public class JarDeployer implements Serializable {
   private static final long serialVersionUID = 1L;
   private static final Logger logger = LogService.getLogger();
@@ -308,27 +309,14 @@ public class JarDeployer implements Serializable {
    * @throws IOException If the directory isn't writable
    */
   public void verifyWritableDeployDirectory() throws IOException {
-    Exception exception = null;
-    int tryCount = 0;
-    do {
-      try {
-        if (this.deployDirectory.canWrite()) {
-          return;
-        }
-      } catch (Exception ex) {
-        exception = ex;
-        // We'll just ignore exceptions and loop to try again
-      }
-      try {
-        Thread.sleep(100);
-      } catch (InterruptedException iex) {
-        logger.error("Interrupted while testing writable deploy directory", iex);
+    try {
+      if (this.deployDirectory.canWrite()) {
+        return;
       }
-    } while (tryCount++ < 20);
-
-    if (exception != null) {
-      throw new IOException("Unable to write to deploy directory", exception);
+    } catch (SecurityException ex) {
+      throw new IOException("Unable to write to deploy directory", ex);
     }
+
     throw new IOException(
         "Unable to write to deploy directory: " + this.deployDirectory.getCanonicalPath());
   }

http://git-wip-us.apache.org/repos/asf/geode/blob/836451a5/geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java
index b81e3e9..16b18aa 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java
@@ -20,24 +20,19 @@ package org.apache.geode.internal;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
-import org.apache.geode.test.junit.categories.IntegrationTest;
-import org.awaitility.Awaitility;
+import java.io.File;
+import java.io.IOException;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.junit.rules.TemporaryFolder;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.Set;
-import java.util.concurrent.CyclicBarrier;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
+import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
 public class JarDeployerIntegrationTest {
@@ -101,25 +96,7 @@ public class JarDeployerIntegrationTest {
     // Test to verify that deployment fails if the directory doesn't exist.
     assertThatThrownBy(() -> {
       jarDeployer.deployWithoutRegistering("JarDeployerIntegrationTest.jar", jarBytes);
-    }).isInstanceOf(IOException.class);
-
-    // Test to verify that deployment succeeds if the directory doesn't
-    // initially exist, but is then created while the JarDeployer is looping
-    // looking for a valid directory.
-    final AtomicBoolean isDeployed = new AtomicBoolean(false);
-    final CyclicBarrier barrier = new CyclicBarrier(2);
-
-    Executors.newSingleThreadExecutor().submit(() -> {
-      barrier.await();
-      jarDeployer.deployWithoutRegistering("JarDeployerIntegrationTest.jar", jarBytes);
-      isDeployed.set(true);
-      return true;
-    });
-
-    barrier.await();
-    alternateDir.mkdirs();
-    Awaitility.await().atMost(1, TimeUnit.MINUTES)
-        .until(() -> assertThat(isDeployed.get()).isTrue());
+    }).isInstanceOf(IOException.class).hasMessageContaining("Unable to write to deploy directory:");
   }
 
   @Test