Mailing-List: contact commits-help@geode.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@geode.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: klund@apache.org
To: commits@geode.apache.org
Date: Fri, 25 Aug 2017 22:52:40 -0000
Message-Id: <9f888e4c7f984e8290dff264214db3ce@git.apache.org>
Subject: [1/2] geode git commit: GEODE-3506: improve validation/error checking
 for process file control
archived-at: Fri, 25 Aug 2017 22:52:45 -0000

Repository: geode
Updated Branches:
  refs/heads/develop cb011c5c9 -> 0af313241


GEODE-3506: improve validation/error checking for process file control

This closes #738


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

Branch: refs/heads/develop
Commit: a2ab230d685b8ac897a7e155474f797608f711fa
Parents: cb011c5
Author: Kirk Lund <klund@apache.org>
Authored: Wed Aug 23 14:20:18 2017 -0700
Committer: Kirk Lund <klund@apache.org>
Committed: Fri Aug 25 14:50:06 2017 -0700

----------------------------------------------------------------------
 .../internal/process/ControllableProcess.java   | 93 +++++++++++++-------
 .../internal/process/FileProcessController.java |  2 +-
 2 files changed, 61 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/a2ab230d/geode-core/src/main/java/org/apache/geode/internal/process/ControllableProcess.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/process/ControllableProcess.java b/geode-core/src/main/java/org/apache/geode/internal/process/ControllableProcess.java
index 2fdd116..e44b34f 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/process/ControllableProcess.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/process/ControllableProcess.java
@@ -19,6 +19,7 @@ import static org.apache.commons.lang.Validate.notNull;
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 
 import org.apache.logging.log4j.Logger;
 
@@ -137,15 +138,14 @@ public class ControllableProcess {
   }
 
   private void deleteFiles(final File directory, final ProcessType processType) {
-    deleteFile(directory, processType.getStatusRequestFileName());
-    deleteFile(directory, processType.getStatusFileName());
-    deleteFile(directory, processType.getStopRequestFileName());
-  }
-
-  private void deleteFile(final File directory, final String fileName) {
-    File file = new File(directory, fileName);
-    if (file.exists()) {
-      file.delete();
+    try {
+      deleteFileWithValidation(new File(directory, processType.getStatusRequestFileName()),
+          "statusRequestFile");
+      deleteFileWithValidation(new File(directory, processType.getStatusFileName()), "statusFile");
+      deleteFileWithValidation(new File(directory, processType.getStopRequestFileName()),
+          "stopRequestFile");
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
     }
   }
 
@@ -165,30 +165,7 @@ public class ControllableProcess {
   private static ControlRequestHandler createStatusHandler(final ControlNotificationHandler handler,
       final File directory, final ProcessType processType) {
     return () -> {
-      ServiceState<?> state = handler.handleStatus();
-
-      File statusFile = new File(directory, processType.getStatusFileName());
-      if (statusFile.exists()) {
-        boolean deleted = statusFile.delete();
-        assert deleted;
-      }
-
-      File statusFileTmp = new File(directory, processType.getStatusFileName() + ".tmp");
-      if (statusFileTmp.exists()) {
-        boolean deleted = statusFileTmp.delete();
-        assert deleted;
-      }
-
-      boolean created = statusFileTmp.createNewFile();
-      assert created;
-
-      FileWriter writer = new FileWriter(statusFileTmp);
-      writer.write(state.toJson());
-      writer.flush();
-      writer.close();
-
-      boolean renamed = statusFileTmp.renameTo(statusFile);
-      assert renamed;
+      writeStatusToFile(fetchStatusWithValidation(handler), directory, processType);
     };
   }
 
@@ -203,4 +180,54 @@ public class ControllableProcess {
     return new ControlFileWatchdog(directory, processType.getStatusRequestFileName(), statusHandler,
         false);
   }
+
+  private static String fetchStatusWithValidation(final ControlNotificationHandler handler) {
+    ServiceState<?> state = handler.handleStatus();
+    if (state == null) {
+      throw new IllegalStateException("Null ServiceState is invalid");
+    }
+
+    String jsonContent = state.toJson();
+    if (jsonContent == null) {
+      throw new IllegalStateException("Null JSON for status is invalid");
+    } else if (jsonContent.isEmpty()) {
+      throw new IllegalStateException("Empty JSON for status is invalid");
+    }
+
+    return jsonContent;
+  }
+
+  private static void deleteFileWithValidation(final File file, final String fileNameForMessage)
+      throws IOException {
+    if (file.exists()) {
+      if (!file.delete()) {
+        throw new IOException(
+            "Unable to delete " + fileNameForMessage + "'" + file.getCanonicalPath() + "'");
+      }
+    }
+  }
+
+  private static void writeStatusToFile(final String jsonContent, final File directory,
+      final ProcessType processType) throws IOException {
+    File statusFile = new File(directory, processType.getStatusFileName());
+    File statusFileTmp = new File(directory, processType.getStatusFileName() + ".tmp");
+
+    deleteFileWithValidation(statusFile, "statusFile");
+    deleteFileWithValidation(statusFileTmp, "statusFileTmp");
+
+    if (!statusFileTmp.createNewFile()) {
+      throw new IOException(
+          "Unable to create statusFileTmp '" + statusFileTmp.getCanonicalPath() + "'");
+    }
+
+    FileWriter writer = new FileWriter(statusFileTmp);
+    writer.write(jsonContent);
+    writer.flush();
+    writer.close();
+
+    if (!statusFileTmp.renameTo(statusFile)) {
+      throw new IOException("Unable to rename statusFileTmp '" + statusFileTmp.getCanonicalPath()
+          + "' to '" + statusFile.getCanonicalPath() + "'");
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/a2ab230d/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java b/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
index 062cfb6..fcb61d1 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/process/FileProcessController.java
@@ -149,7 +149,7 @@ class FileProcessController implements ProcessController {
 
     String lines = statusRef.get();
     if (isBlank(lines)) {
-      throw new IllegalStateException("Failed to read status file");
+      throw new IllegalStateException("Status file '" + statusFile + "' is blank");
     }
     return lines;
   }