You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ja...@apache.org on 2017/08/14 19:07:39 UTC

geode git commit: GEODE-3434: Modifications based on review comments

Repository: geode
Updated Branches:
  refs/heads/feature/GEODE-3434 e9ed8a12f -> ae570225e


GEODE-3434: Modifications based on review comments


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

Branch: refs/heads/feature/GEODE-3434
Commit: ae570225ed1d53dafd7450b4b4e399eb073f8442
Parents: e9ed8a1
Author: Jason Huynh <hu...@gmail.com>
Authored: Mon Aug 14 12:07:08 2017 -0700
Committer: Jason Huynh <hu...@gmail.com>
Committed: Mon Aug 14 12:07:08 2017 -0700

----------------------------------------------------------------------
 .../geode/session/tests/ContainerInstall.java   |  36 +++---
 .../geode/session/tests/ServerContainer.java    |   1 -
 .../geode/session/tests/TomcatInstall.java      | 118 +++----------------
 ...TomcatSessionBackwardsCompatibilityTest.java |  10 --
 .../lucene/internal/LuceneEventListener.java    |   1 -
 geode-old-versions/build.gradle                 |  60 +---------
 6 files changed, 38 insertions(+), 188 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/ae570225/geode-assembly/src/test/java/org/apache/geode/session/tests/ContainerInstall.java
----------------------------------------------------------------------
diff --git a/geode-assembly/src/test/java/org/apache/geode/session/tests/ContainerInstall.java b/geode-assembly/src/test/java/org/apache/geode/session/tests/ContainerInstall.java
index 46779c4..fc3c856 100644
--- a/geode-assembly/src/test/java/org/apache/geode/session/tests/ContainerInstall.java
+++ b/geode-assembly/src/test/java/org/apache/geode/session/tests/ContainerInstall.java
@@ -18,6 +18,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.net.URI;
 import java.net.URL;
 import java.util.HashMap;
 import java.util.Properties;
@@ -97,7 +98,7 @@ public abstract class ContainerInstall {
 
   public ContainerInstall(String installDir, String downloadURL, ConnectionType connType,
       String moduleName) throws IOException {
-    this(installDir, downloadURL, connType, moduleName, null);
+    this(installDir, downloadURL, connType, moduleName, DEFAULT_MODULE_LOCATION);
   }
 
   /**
@@ -118,6 +119,8 @@ public abstract class ContainerInstall {
       String moduleName, String geodeModuleLocation) throws IOException {
     this.connType = connType;
 
+    clearPreviousInstall(installDir);
+
     logger.info("Installing container from URL " + downloadURL);
 
     // Optional step to install the container from a URL pointing to its distribution
@@ -150,17 +153,12 @@ public abstract class ContainerInstall {
   /**
    * Cleans up the installation by deleting the extracted module and downloaded installation folders
    */
-  public void clearPreviousRuns() throws IOException {
-    File modulesFolder = new File(DEFAULT_MODULE_EXTRACTION_DIR);
-    File installsFolder = new File(DEFAULT_INSTALL_DIR);
-
-    // Remove default modules extraction from previous runs
-    if (modulesFolder.exists()) {
-      FileUtils.deleteDirectory(modulesFolder);
-    }
-    // Remove default installs from previous runs
-    if (installsFolder.exists()) {
-      FileUtils.deleteDirectory(installsFolder);
+  public void clearPreviousInstall(String installDir) throws IOException {
+    File installFolder = new File(installDir);
+    // Remove installs from previous runs in the same folder
+    if (installFolder.exists()) {
+      logger.info("Deleting previous install folder " + installFolder.getAbsolutePath());
+      FileUtils.deleteDirectory(installFolder);
     }
   }
 
@@ -326,16 +324,22 @@ public abstract class ContainerInstall {
       }
     }
 
+    String extractedModulePath =
+        modulePath.getName().substring(0, modulePath.getName().length() - 4);
+    // Get the name of the new module folder within the extraction directory
+    File newModuleFolder = new File(DEFAULT_MODULE_EXTRACTION_DIR + extractedModulePath);
+    // Remove any previous module folders extracted here
+    if (newModuleFolder.exists()) {
+      logger.info("Deleting previous modules directory " + newModuleFolder.getAbsolutePath());
+      FileUtils.deleteDirectory(newModuleFolder);
+    }
+
     // Unzip if it is a zip file
     if (archive) {
       if (!FilenameUtils.getExtension(modulePath.getAbsolutePath()).equals("zip")) {
         throw new IOException("Bad module archive " + modulePath);
       }
 
-      // Get the name of the new module folder within the extraction directory
-      File newModuleFolder = new File(DEFAULT_MODULE_EXTRACTION_DIR
-          + modulePath.getName().substring(0, modulePath.getName().length() - 4));
-
       // Extract folder to location if not already there
       if (!newModuleFolder.exists()) {
         ZipUtils.unzip(modulePath.getAbsolutePath(), newModuleFolder.getAbsolutePath());

http://git-wip-us.apache.org/repos/asf/geode/blob/ae570225/geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java
----------------------------------------------------------------------
diff --git a/geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java b/geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java
index c50a1f2..dbd438a 100644
--- a/geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java
+++ b/geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java
@@ -134,7 +134,6 @@ public abstract class ServerContainer {
 
     // Set cacheXML file
     File installXMLFile = install.getCacheXMLFile();
-    String path = logDir.getAbsolutePath() + "/" + installXMLFile.getName();
     setCacheXMLFile(new File(logDir.getAbsolutePath() + "/" + installXMLFile.getName()));
     // Copy the cacheXML file to a new, unique location for this container
     FileUtils.copyFile(installXMLFile, cacheXMLFile);

http://git-wip-us.apache.org/repos/asf/geode/blob/ae570225/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java
----------------------------------------------------------------------
diff --git a/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java b/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java
index 08d75fa..57dc519 100644
--- a/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java
+++ b/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java
@@ -32,6 +32,8 @@ import java.util.regex.Pattern;
  */
 public class TomcatInstall extends ContainerInstall {
 
+  public static final String GEODE_BUILD_HOME_LIB = GEODE_BUILD_HOME + "/lib/";
+
   /**
    * Version of tomcat that this class will install
    *
@@ -110,49 +112,37 @@ public class TomcatInstall extends ContainerInstall {
   }
 
   private static final String[] tomcatRequiredJars =
-      {"antlr", "commons-lang", "fastutil", "geode-core", "geode-modules", "geode-modules-tomcat7",
-          "geode-modules-tomcat8", "javax.transaction-api", "jgroups", "log4j-api", "log4j-core",
-          "log4j-jul", "shiro-core", "slf4j-api", "slf4j-jdk14", "commons-validator"};
+      {"antlr", "commons-lang", "fastutil", "geode-core", "javax.transaction-api", "jgroups",
+          "log4j-api", "log4j-core", "log4j-jul", "shiro-core", "commons-validator"};
 
   private final TomcatVersion version;
 
-  public TomcatInstall(TomcatVersion version) throws Exception {
-    this(version, ConnectionType.PEER_TO_PEER, DEFAULT_INSTALL_DIR, null, null);
-  }
-
   public TomcatInstall(TomcatVersion version, String installDir) throws Exception {
-    this(version, ConnectionType.PEER_TO_PEER, installDir, null, null);
-  }
-
-  public TomcatInstall(TomcatVersion version, ConnectionType connType) throws Exception {
-    this(version, connType, DEFAULT_INSTALL_DIR, null, null);
+    this(version, ConnectionType.PEER_TO_PEER, installDir, DEFAULT_MODULE_LOCATION,
+        GEODE_BUILD_HOME_LIB);
   }
 
   public TomcatInstall(TomcatVersion version, ConnectionType connType, String installDir)
       throws Exception {
-    this(version, connType, installDir, null, null);
+    this(version, connType, installDir, DEFAULT_MODULE_LOCATION, GEODE_BUILD_HOME_LIB);
   }
 
   /**
    * Download and setup an installation tomcat using the {@link ContainerInstall} constructor and
    * some extra functions this class provides
    *
-   * Specifically, this function uses {@link #copyTomcatGeodeReqFiles(String)} to install geode
-   * session into Tomcat, {@link #setupDefaultSettings()} to modify the context and server XML files
-   * within the installation's 'conf' folder, and {@link #updateProperties()} to set the jar
+   * Specifically, this function uses {@link #copyTomcatGeodeReqFiles(String, String)} to install
+   * geode session into Tomcat, {@link #setupDefaultSettings()} to modify the context and server XML
+   * files within the installation's 'conf' folder, and {@link #updateProperties()} to set the jar
    * skipping properties needed to speedup container startup.
    */
   public TomcatInstall(TomcatVersion version, ConnectionType connType, String installDir,
       String modulesJarLocation, String extraJarsPath) throws Exception {
     // Does download and install from URL
-    super(installDir, version.getDownloadURL(), connType, "tomcat",
-        modulesJarLocation == null ? DEFAULT_MODULE_LOCATION : modulesJarLocation);
+    super(installDir, version.getDownloadURL(), connType, "tomcat", modulesJarLocation);
 
     this.version = version;
-    // if (modulesJarLocation == null || modulesJarLocation.endsWith("zip"))
     modulesJarLocation = getModulePath() + "/lib/";
-    if (extraJarsPath == null)
-      extraJarsPath = GEODE_BUILD_HOME + "/lib/";
 
     // Install geode sessions into tomcat install
     copyTomcatGeodeReqFiles(modulesJarLocation, extraJarsPath);
@@ -166,66 +156,6 @@ public class TomcatInstall extends ContainerInstall {
   }
 
   /**
-   * Copies jars specified by {@link #tomcatRequiredJars} from the {@link #getModulePath()} and the
-   * specified other directory passed to the function
-   *
-   * @throws IOException if the {@link #getModulePath()}, installation lib directory, or extra
-   *         directory passed in contain no files.
-   */
-  private void copyTomcatGeodeReqFiles(String moduleJarDir, String extraJarsPath)
-      throws IOException {
-    ArrayList<File> requiredFiles = new ArrayList<>();
-    // The library path for the current tomcat installation
-    String tomcatLibPath = getHome() + "/lib/";
-
-    // List of required jars and form version regexps from them
-    String versionRegex = "-?[0-9]*.*\\.jar";
-    ArrayList<Pattern> patterns = new ArrayList<>(tomcatRequiredJars.length);
-    for (String jar : tomcatRequiredJars)
-      patterns.add(Pattern.compile(jar + versionRegex));
-
-    // // Don't need to copy any jars already in the tomcat install
-    File tomcatLib = new File(tomcatLibPath);
-
-    // Find all the required jars in the tomcatModulePath
-    try {
-      for (File file : (new File(moduleJarDir)).listFiles()) {
-        if (file.isFile() && file.getName().endsWith(".jar")) {
-          requiredFiles.add(file);
-        }
-      }
-    } catch (NullPointerException e) {
-      throw new IOException(
-          "No files found in tomcat module directory " + getModulePath() + "/lib/");
-    }
-
-    // Find all the required jars in the extraJarsPath
-    try {
-      for (File file : (new File(extraJarsPath)).listFiles()) {
-        for (Pattern pattern : patterns) {
-          if (pattern.matcher(file.getName()).find()) {
-            requiredFiles.add(file);
-            // patterns.remove(pattern);
-            break;
-          }
-        }
-      }
-    } catch (NullPointerException e) {
-      throw new IOException("No files found in extra jars directory " + extraJarsPath);
-    }
-
-    // Copy the required jars to the given tomcat lib folder
-    for (File file : requiredFiles) {
-      Files.copy(file.toPath(), tomcatLib.toPath().resolve(file.toPath().getFileName()),
-          StandardCopyOption.REPLACE_EXISTING);
-      logger.debug("Copied required jar from " + file.toPath() + " to "
-          + (new File(tomcatLibPath)).toPath().resolve(file.toPath().getFileName()));
-    }
-
-    logger.info("Copied required jars into the Tomcat installation");
-  }
-
-  /**
    * Modifies the context and server XML files in the installation's 'conf' directory so that they
    * contain the session manager class ({@link #getContextSessionManagerClass()}) and life cycle
    * listener class ({@link #getServerLifeCycleListenerClass()}) respectively
@@ -331,39 +261,26 @@ public class TomcatInstall extends ContainerInstall {
    * @throws IOException if the {@link #getModulePath()}, installation lib directory, or extra
    *         directory passed in contain no files.
    */
-  private void copyTomcatGeodeReqFiles(String extraJarsPath) throws IOException {
+  private void copyTomcatGeodeReqFiles(String moduleJarDir, String extraJarsPath)
+      throws IOException {
     ArrayList<File> requiredFiles = new ArrayList<>();
     // The library path for the current tomcat installation
     String tomcatLibPath = getHome() + "/lib/";
 
     // List of required jars and form version regexps from them
-    String versionRegex = "-[0-9]+.*\\.jar";
+    String versionRegex = "-?[0-9]*.*\\.jar";
     ArrayList<Pattern> patterns = new ArrayList<>(tomcatRequiredJars.length);
     for (String jar : tomcatRequiredJars)
       patterns.add(Pattern.compile(jar + versionRegex));
 
     // Don't need to copy any jars already in the tomcat install
     File tomcatLib = new File(tomcatLibPath);
-    if (tomcatLib.exists()) {
-      try {
-        for (File file : tomcatLib.listFiles())
-          patterns.removeIf(pattern -> pattern.matcher(file.getName()).find());
-      } catch (NullPointerException e) {
-        throw new IOException("No files found in tomcat lib directory " + tomcatLibPath);
-      }
-    } else {
-      tomcatLib.mkdir();
-    }
 
     // Find all the required jars in the tomcatModulePath
     try {
-      for (File file : (new File(getModulePath() + "/lib/")).listFiles()) {
-        for (Pattern pattern : patterns) {
-          if (pattern.matcher(file.getName()).find()) {
-            requiredFiles.add(file);
-            patterns.remove(pattern);
-            break;
-          }
+      for (File file : (new File(moduleJarDir)).listFiles()) {
+        if (file.isFile() && file.getName().endsWith(".jar")) {
+          requiredFiles.add(file);
         }
       }
     } catch (NullPointerException e) {
@@ -377,7 +294,6 @@ public class TomcatInstall extends ContainerInstall {
         for (Pattern pattern : patterns) {
           if (pattern.matcher(file.getName()).find()) {
             requiredFiles.add(file);
-            patterns.remove(pattern);
             break;
           }
         }

http://git-wip-us.apache.org/repos/asf/geode/blob/ae570225/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java
----------------------------------------------------------------------
diff --git a/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java b/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java
index 6a9de92..aac851f 100644
--- a/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java
+++ b/geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java
@@ -16,11 +16,8 @@ package org.apache.geode.session.tests;
 
 import static org.junit.Assert.assertEquals;
 
-import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
 import java.net.URISyntaxException;
 import java.util.Collection;
 import java.util.List;
@@ -166,11 +163,6 @@ public class TomcatSessionBackwardsCompatibilityTest {
     manager.stopAllActiveContainers();
     manager.cleanUp();
 
-    tomcat8AndCurrentModules.clearPreviousRuns();
-    tomcat8AndOldModules.clearPreviousRuns();
-    tomcat7079AndCurrentModules.clearPreviousRuns();
-    tomcat7079AndOldModules.clearPreviousRuns();
-
     CommandStringBuilder locStop = new CommandStringBuilder(CliStrings.STOP_LOCATOR);
     locStop.addOption(CliStrings.STOP_LOCATOR__DIR, "loc");
     gfsh.executeAndVerifyCommand(locStop.toString());
@@ -225,13 +217,11 @@ public class TomcatSessionBackwardsCompatibilityTest {
     doPutAndGetSessionOnAllClients();
   }
 
-
   @Test
   public void tomcat8WithOldModuleCanDoPuts() throws Exception {
     startClusterWithTomcat(classPathTomcat8);
     manager.addContainer(tomcat8AndOldModules);
     manager.addContainer(tomcat8AndOldModules);
-
     doPutAndGetSessionOnAllClients();
   }
 

http://git-wip-us.apache.org/repos/asf/geode/blob/ae570225/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java
----------------------------------------------------------------------
diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java
index db53184..ac5c971 100644
--- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java
@@ -34,7 +34,6 @@ import org.apache.geode.cache.lucene.internal.repository.IndexRepository;
 import org.apache.geode.cache.query.internal.DefaultQuery;
 import org.apache.geode.internal.cache.BucketNotFoundException;
 import org.apache.geode.internal.cache.PrimaryBucketException;
-import org.apache.geode.internal.cache.tier.sockets.command.Default;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.lucene.store.AlreadyClosedException;
 

http://git-wip-us.apache.org/repos/asf/geode/blob/ae570225/geode-old-versions/build.gradle
----------------------------------------------------------------------
diff --git a/geode-old-versions/build.gradle b/geode-old-versions/build.gradle
index a5c729f..b5adc56 100644
--- a/geode-old-versions/build.gradle
+++ b/geode-old-versions/build.gradle
@@ -25,7 +25,6 @@ disableMavenPublishing()
 project.ext.installs = new Properties();
 
 def addOldVersion(def source, def geodeVersion) {
-//  def sourceSet =
   sourceSets.create(source, {
     compileClasspath += configurations.provided
     runtimeClasspath += configurations.provided
@@ -41,11 +40,6 @@ def addOldVersion(def source, def geodeVersion) {
 
   project.ext.installs.setProperty(source, "$buildDir/apache-geode-${geodeVersion}")
 
-//  task "downloadZipFile${source}" (type: Download) {
-//    src "http://apache.mirrors.ionfish.org/geode/${geodeVersion}/apache-geode-${geodeVersion}.zip"
-//    dest new File(buildDir, "apache-geode-${geodeVersion}.zip")
-//  }
-
   task "downloadZipFile${source}" (type: Download) {
     src "https://www.apache.org/dyn/closer.cgi?action=download&filename=geode/$geodeVersion/apache-geode-${geodeVersion}.tar.gz"
     dest new File(buildDir, "apache-geode-${geodeVersion}.tar.gz")
@@ -65,14 +59,12 @@ def addOldVersion(def source, def geodeVersion) {
     }
   }
 
-  //task "downloadAndUnzipFile${source}" (dependsOn: "downloadZipFile${source}", type: Copy) {
   task "downloadAndUnzipFile${source}" (dependsOn: "verifyGeode${source}", type: Copy) {
     from tarTree(tasks["downloadZipFile${source}"].dest)
     into buildDir
   }
 
   createGeodeClasspathsFile.dependsOn tasks["downloadAndUnzipFile${source}"]
-  //build.dependsOn tasks["downloadAndUnzipFile${source}"];
 }
 
 def generatedResources = "$buildDir/generated-resources/main"
@@ -109,62 +101,12 @@ task createGeodeClasspathsFile  {
   }
 
   // Add sourceSets for backwards compatibility, rolling upgrade, and
-// pdx testing.
+  // pdx testing.
   addOldVersion('test100', '1.0.0-incubating')
   addOldVersion('test110', '1.1.0')
   addOldVersion('test111', '1.1.1')
   addOldVersion('test120', '1.2.0')
 
-
-
-//
-//  def downloadUrl = (geodeReleaseUrl != "") ? geodeReleaseUrl :
-//          "https://www.apache.org/dyn/closer.cgi?action=download&filename=geode/$geodeVersion"
-//  def verificationUrl = (geodeReleaseUrl != "") ? geodeReleaseUrl :
-//          "https://www.apache.org/dist/geode/$geodeVersion"
-//
-//  def downloadFile = "apache-geode-${geodeVersion}.tar.gz"
-//  def installFile = "$buildDir/$downloadFile"
-//  def installDir = "$buildDir/apache-geode-${geodeVersion}"
-//
-//
-
-
-
-//  task "downloadZipFile${source}" (type: Download) {
-//    src ([
-//            "https://www.apache.org/dyn/closer.cgi?action=download&filename=geode/$geodeVersion/apache-geode-${geodeVersion}.tar.gz",
-//            "https://www.apache.org/dist/geode/$geodeVersion/apache-geode-${geodeVersion}.tar.gz.sha256"
-//            ])
-//
-//    dest new File(buildDir, "apache-geode-${geodeVersion}.zip")
-//  }
-//
-//  task downloadGeode {
-//    inputs.property 'geodeVersion', geodeVersion
-//    outputs.file installFile
-//    outputs.file "${installFile}.sha256"
-//
-//    doLast {
-//      download {
-//        src([
-//                "$downloadUrl/$downloadFile",
-//                "$verificationUrl/${downloadFile}.sha256"
-//        ])
-//        dest buildDir
-//      }
-//    }
-//  }
-//
-
-//
-//  task installGeode(type: Copy, dependsOn: verifyGeode) {
-//    inputs.file installFile
-//    outputs.dir installDir
-//
-//    from tarTree(resources.gzip(installFile))
-//    into buildDir
-//  }
 }