GEODE-9377 - Removed deploymentName field from Deployment config obje…

…ct. (apache#6610) Co-authored-by: Udo Kohlmeyer <[email protected]>
ringles · Jul 13, 2021 · be569ea · be569ea
1 parent 90bc5b7
commit be569ea
Show file tree

Hide file tree

Showing 43 changed files with 318 additions and 505 deletions.
diff --git a/geode-assembly/src/acceptanceTest/java/org/apache/geode/modules/DeployJarAcceptanceTest.java b/geode-assembly/src/acceptanceTest/java/org/apache/geode/modules/DeployJarAcceptanceTest.java
@@ -97,15 +97,6 @@ public void testDeployExistingJar() throws IOException {
         .execute(gfshRule).getOutputText()).contains(jarFile.getName()).contains("JAR Location");
   }
 
-  @Test
-  public void testDeployJarWithDeploymentName() throws IOException {
-    GfshScript.of(getLocatorGFSHConnectionString(),
-        "deploy --name=myDeployment --jar=" + jarFile.getCanonicalPath()).execute(gfshRule);
-
-    assertThat(GfshScript.of(getLocatorGFSHConnectionString(), "list deployed")
-        .execute(gfshRule).getOutputText()).contains("myDeployment").contains("JAR Location");
-  }
-
   @Test
   public void testUndeployJar() throws IOException {
     GfshScript.of(getLocatorGFSHConnectionString(),

diff --git a/...java/org/apache/geode/management/internal/rest/DeploymentSemanticVersionJarDUnitTest.java b/...java/org/apache/geode/management/internal/rest/DeploymentSemanticVersionJarDUnitTest.java
@@ -201,11 +201,9 @@ public void deployWithPlainWillCleanSemanticVersion() throws Exception {
 
     ClusterManagementListResultAssert<Deployment, DeploymentInfo> listAssert =
         assertManagementListResult(client.list(new Deployment()));
-    listAssert.hasConfigurations().hasSize(1)
-        .extracting(Deployment::getFileName)
+    listAssert.hasConfigurations().hasSize(1).extracting(Deployment::getFileName)
         .containsExactly("def.jar");
-    listAssert.hasRuntimeInfos().hasSize(1)
-        .extracting(DeploymentInfo::getJarLocation).asString()
+    listAssert.hasRuntimeInfos().hasSize(1).extracting(DeploymentInfo::getJarLocation).asString()
         .containsOnlyOnce("def.v2.jar");
   }
 

diff --git a/...a/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java b/...a/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
@@ -60,7 +60,7 @@ public void testDeployToNoServer() throws Exception {
     assertThat(gfshConnector.isConnected()).isTrue();
 
     gfshConnector.executeAndAssertThat("deploy --jar=" + clusterJarPath).statusIsSuccess();
-    ConfigGroup cluster = new ConfigGroup("cluster").deployments("cluster");
+    ConfigGroup cluster = new ConfigGroup("cluster").jars("cluster.jar");
     ClusterConfig expectedClusterConfig = new ClusterConfig(cluster);
 
     expectedClusterConfig.verify(locator);
@@ -81,7 +81,7 @@ public void testDeployToMultipleLocators() throws Exception {
 
     gfshConnector.executeAndAssertThat("deploy --jar=" + clusterJar).statusIsSuccess();
 
-    ConfigGroup cluster = new ConfigGroup("cluster").deployments("cluster");
+    ConfigGroup cluster = new ConfigGroup("cluster").jars("cluster.jar");
     ClusterConfig expectedClusterConfig = new ClusterConfig(cluster);
 
     expectedClusterConfig.verify(locator);
@@ -107,7 +107,7 @@ public void testDeploy() throws Exception {
 
     gfshConnector.executeAndAssertThat("deploy --jar=" + clusterJar).statusIsSuccess();
 
-    ConfigGroup cluster = new ConfigGroup("cluster").deployments("cluster");
+    ConfigGroup cluster = new ConfigGroup("cluster").jars("cluster.jar");
     ClusterConfig expectedClusterConfig = new ClusterConfig(cluster);
     expectedClusterConfig.verify(locator);
     expectedClusterConfig.verify(server1);
@@ -117,7 +117,7 @@ public void testDeploy() throws Exception {
     gfshConnector.executeAndAssertThat("deploy --jar=" + group1Jar + " --group=group1")
         .statusIsSuccess();
 
-    ConfigGroup group1 = new ConfigGroup("group1").deployments("group1");
+    ConfigGroup group1 = new ConfigGroup("group1").jars("group1.jar");
     ClusterConfig expectedGroup1Config = new ClusterConfig(cluster, group1);
     expectedGroup1Config.verify(locator);
     expectedClusterConfig.verify(server1);
@@ -127,7 +127,7 @@ public void testDeploy() throws Exception {
     gfshConnector.executeAndAssertThat("deploy --jar=" + group2Jar + " --group=group2")
         .statusIsSuccess();
 
-    ConfigGroup group2 = new ConfigGroup("group2").deployments("group2");
+    ConfigGroup group2 = new ConfigGroup("group2").jars("group2.jar");
     ClusterConfig expectedGroup1and2Config = new ClusterConfig(cluster, group1, group2);
 
     expectedGroup1and2Config.verify(locator);
@@ -159,7 +159,7 @@ public void testInvalidJarDeploy() throws Exception {
     gfshConnector.executeAndAssertThat("list deployed").statusIsSuccess()
         .containsOutput("No JAR Files Found");
 
-    ConfigGroup cluster = new ConfigGroup("cluster").deployments();
+    ConfigGroup cluster = new ConfigGroup("cluster").jars();
     ClusterConfig expectedClusterConfig = new ClusterConfig(cluster);
     expectedClusterConfig.verify(locator);
   }
@@ -190,7 +190,7 @@ public void testUndeploy() throws Exception {
     gfshConnector.executeAndAssertThat("deploy --jar=" + clusterJar).statusIsSuccess();
 
     // deploy cluster.jar to the cluster
-    cluster.addDeployment("cluster");
+    cluster.addJar("cluster.jar");
     expectedClusterConfig.verify(locator);
     expectedClusterConfig.verify(server1);
     expectedClusterConfig.verify(server2);
@@ -200,21 +200,17 @@ public void testUndeploy() throws Exception {
     gfshConnector.executeAndAssertThat("deploy --jar=" + group1Jar + " --group=group1,group2")
         .statusIsSuccess();
 
-    group1.addDeployment("group1");
-    group2.addDeployment("group1");
+    group1.addJar("group1.jar");
+    group2.addJar("group1.jar");
     server3Config.verify(locator);
     server1Config.verify(server1);
     server2Config.verify(server2);
     server3Config.verify(server3);
 
     // test undeploy cluster
-    gfshConnector.executeAndAssertThat("undeploy --jar=cluster.jar")
-        .statusIsSuccess();
-
-    gfshConnector.executeAndAssertThat("list deployed")
-        .statusIsSuccess();
+    gfshConnector.executeAndAssertThat("undeploy --jar=cluster.jar").statusIsSuccess();
 
-    cluster = cluster.removeDeployment("cluster");
+    cluster = cluster.removeJar("cluster.jar");
     server3Config.verify(locator);
     server1Config.verify(server1);
     server2Config.verify(server2);
@@ -223,15 +219,15 @@ public void testUndeploy() throws Exception {
     gfshConnector.executeAndAssertThat("undeploy --jar=group1.jar --group=group1")
         .statusIsSuccess();
 
-    group1 = group1.removeDeployment("group1");
+    group1 = group1.removeJar("group1.jar");
     /*
      * TODO: This is the current (weird) behavior If you started server4 with group1,group2 after
      * this undeploy command, it would have group1.jar (brought from
      * cluster_config/group2/group1.jar on locator) whereas server3 (also in group1,group2) does not
      * have this jar.
      */
     ClusterConfig weirdServer3Config =
-        new ClusterConfig(cluster, group1, new ConfigGroup(group2).removeDeployment("group1"));
+        new ClusterConfig(cluster, group1, new ConfigGroup(group2).removeJar("group1.jar"));
 
     server3Config.verify(locator);
     server1Config.verify(server1);
@@ -265,7 +261,7 @@ public void testUndeployWithServerBounce() throws Exception {
     gfshConnector.executeAndAssertThat("deploy --jar=" + clusterJar).statusIsSuccess();
 
     // deploy cluster.jar to the cluster
-    cluster.addDeployment("cluster");
+    cluster.addJar("cluster.jar");
     expectedClusterConfig.verify(locator);
     expectedClusterConfig.verify(server1);
     expectedClusterConfig.verify(server2);
@@ -275,8 +271,8 @@ public void testUndeployWithServerBounce() throws Exception {
     gfshConnector.executeAndAssertThat("deploy --jar=" + group1Jar + " --group=group1,group2")
         .statusIsSuccess();
 
-    group1.addDeployment("group1");
-    group2.addDeployment("group1");
+    group1.addJar("group1.jar");
+    group2.addJar("group1.jar");
     server3Config.verify(locator);
     server1Config.verify(server1);
     server2Config.verify(server2);
@@ -285,11 +281,10 @@ public void testUndeployWithServerBounce() throws Exception {
     server3.getVM().bounce();
 
     // test undeploy cluster
-    gfshConnector.executeAndAssertThat("undeploy --jar=cluster.jar")
-        .statusIsSuccess();
+    gfshConnector.executeAndAssertThat("undeploy --jar=cluster.jar").statusIsSuccess();
     server3 = lsRule.startServerVM(3, serverProps, locator.getPort());
 
-    cluster = cluster.removeDeployment("cluster");
+    cluster = cluster.removeJar("cluster.jar");
     server3Config.verify(locator);
     server1Config.verify(server1);
     server2Config.verify(server2);
@@ -298,16 +293,15 @@ public void testUndeployWithServerBounce() throws Exception {
     gfshConnector.executeAndAssertThat("undeploy --jar=group1.jar --group=group1")
         .statusIsSuccess();
 
-    group1 = group1.removeDeployment("group1");
+    group1 = group1.removeJar("group1.jar");
     /*
      * TODO: GEODE-2779 This is the current (weird) behavior If you started server4 with
-     * group1,group2 after this undeploy command, it would have group1.jar (brought
-     * from
+     * group1,group2 after this undeploy command, it would have group1.jar (brought from
      * cluster_config/group2/group1.jar on locator) whereas server3 (also in group1,group2) does not
      * have this jar.
      */
     ClusterConfig weirdServer3Config =
-        new ClusterConfig(cluster, group1, new ConfigGroup(group2).removeDeployment("group1"));
+        new ClusterConfig(cluster, group1, new ConfigGroup(group2).removeJar("group1.jar"));
 
     server3Config.verify(locator);
     server1Config.verify(server1);

diff --git a/...st/java/org/apache/geode/cache/internal/execute/FunctionToFileTrackerIntegrationTest.java b/...st/java/org/apache/geode/cache/internal/execute/FunctionToFileTrackerIntegrationTest.java
@@ -131,7 +131,7 @@ public void unregisterFunctionsForDeployment() throws IOException {
     assertThat(registeredFunctions).containsExactly("JarClassLoaderJUnitFunction");
 
     ClassPathLoader.getLatest().getJarDeploymentService()
-        .undeployByDeploymentName(deployment.getDeploymentName());
+        .undeployByFileName(deployment.getFileName());
     registeredFunctions = FunctionService.getRegisteredFunctions().keySet();
     assertThat(registeredFunctions.size()).isEqualTo(0);
   }

diff --git a/...ionTest/java/org/apache/geode/internal/cache/backup/IncrementalBackupIntegrationTest.java b/...ionTest/java/org/apache/geode/internal/cache/backup/IncrementalBackupIntegrationTest.java
@@ -380,9 +380,9 @@ public void testBackupUserDeployedJarFiles() throws Exception {
     // Remove the "dummy" jar from the VM.
     for (Deployment deployment : ClassPathLoader.getLatest().getJarDeploymentService()
         .listDeployed()) {
-      if (deployment.getDeploymentName().startsWith(jarName)) {
+      if (deployment.getFileName().startsWith(jarName)) {
         ClassPathLoader.getLatest().getJarDeploymentService()
-            .undeployByDeploymentName(deployment.getDeploymentName());
+            .undeployByFileName(deployment.getFileName());
       }
     }
 

diff --git a/geode-core/src/main/java/org/apache/geode/cache/internal/execute/FunctionToFileTracker.java b/geode-core/src/main/java/org/apache/geode/cache/internal/execute/FunctionToFileTracker.java
@@ -27,7 +27,6 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import org.apache.logging.log4j.Logger;
@@ -41,27 +40,24 @@
 import org.apache.geode.internal.classloader.ClassPathLoader;
 import org.apache.geode.logging.internal.log4j.api.LogService;
 import org.apache.geode.management.internal.deployment.FunctionScanner;
+import org.apache.geode.management.internal.utils.JarFileUtils;
 
 /**
  * This class is used to register {@link Function}s from deployed jar files. It provides convenience
  * over working with the {@link FunctionService} directly.
  */
 public class FunctionToFileTracker {
 
-  private static final Pattern PATTERN_SLASH = Pattern.compile("/");
-
   private static final Logger logger = LogService.getLogger();
 
   private final Map<String, List<Function<?>>> deploymentToFunctionsMap = new ConcurrentHashMap<>();
 
   /**
    * Scan the JAR file and attempt to register any function classes found.
    *
-   * @param deploymentName The name of the deployment associated with the jarfile containing
-   *        {@link Function}
    * @param jarFile The {@link File} that contains {@link Function} that are to be registered
    */
-  public synchronized void registerFunctionsFromFile(String deploymentName, File jarFile)
+  public synchronized void registerFunctionsFromFile(File jarFile)
       throws ClassNotFoundException, IOException {
     logger.debug("Registering functions for: {}", jarFile.getName());
 
@@ -86,10 +82,11 @@ public synchronized void registerFunctionsFromFile(String deploymentName, File j
       logger.error("Exception when trying to find function classes from Jar", ioex);
       throw ioex;
     }
+    String artifactId = JarFileUtils.toArtifactId(jarFile.getName());
     List<Function<?>> previouslyRegisteredFunctions =
-        deploymentToFunctionsMap.remove(deploymentName);
+        deploymentToFunctionsMap.remove(artifactId);
     if (!registeredFunctions.isEmpty()) {
-      deploymentToFunctionsMap.put(deploymentName, registeredFunctions);
+      deploymentToFunctionsMap.put(artifactId, registeredFunctions);
     }
     unregisterUndeployedFunctions(previouslyRegisteredFunctions, registeredFunctions);
   }
@@ -130,11 +127,12 @@ private List<Function<?>> registerFunction(String jarFilePath,
    * Unregisters functions from a previously deployed jar that are not present in its replacement.
    * If newJar is null, all functions registered from oldJar will be removed.
    *
-   * @param deploymentName - The name of the deployment that linked to the functions that are to be
+   * @param jarFileName - The name of the deployment that linked to the functions that are to be
    *        unregistered.
    */
-  public void unregisterFunctionsForDeployment(String deploymentName) {
-    List<Function<?>> functions = deploymentToFunctionsMap.remove(deploymentName);
+  public void unregisterFunctionsForDeployment(String jarFileName) {
+    List<Function<?>> functions =
+        deploymentToFunctionsMap.remove(JarFileUtils.getArtifactId(jarFileName));
     if (functions != null) {
       functions.stream().map(Function::getId).forEach(FunctionService::unregisterFunction);
     }

diff --git a/...n/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java b/...n/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
@@ -268,48 +268,38 @@ public void modifyXmlAndProperties(Properties properties, XmlEntity xmlEntity, S
    * Add jar information into the shared configuration and save the jars in the file system used
    * when deploying jars
    */
-  public void addJarsToThisLocator(String deploymentName, List<String> jarFullPaths,
-      String[] groups) throws IOException {
-    addJarsToThisLocator(deploymentName, getDeployedBy(), Instant.now().toString(), jarFullPaths,
-        groups);
+  public void addJarsToThisLocator(List<String> jarFullPaths, String[] groups) throws IOException {
+    addJarsToThisLocator(getDeployedBy(), Instant.now().toString(), jarFullPaths, groups);
   }
 
   @VisibleForTesting
   void addJarsToThisLocator(String deployedBy, String deployedTime, List<String> jarFullPaths,
       String[] groups) throws IOException {
-    addJarsToThisLocator(null, deployedBy, deployedTime, jarFullPaths, groups);
-  }
-
-  private void addJarsToThisLocator(String deploymentName, String deployedBy, String deployedTime,
-      List<String> jarFullPaths, String[] groups) throws IOException {
     lockSharedConfiguration();
     try {
-      addJarsToGroups(listOf(groups), deploymentName, jarFullPaths, deployedBy, deployedTime);
+      addJarsToGroups(listOf(groups), jarFullPaths, deployedBy, deployedTime);
     } finally {
       unlockSharedConfiguration();
     }
   }
 
-  private void addJarsToGroups(List<String> groups, String deploymentName,
-      List<String> jarFullPaths, String deployedBy,
+  private void addJarsToGroups(List<String> groups, List<String> jarFullPaths, String deployedBy,
       String deployedTime) throws IOException {
     for (String group : groups) {
       copyJarsToGroupDir(group, jarFullPaths);
-      addJarsToGroupConfig(group, deploymentName, jarFullPaths, deployedBy, deployedTime);
+      addJarsToGroupConfig(group, jarFullPaths, deployedBy, deployedTime);
     }
   }
 
-  private void addJarsToGroupConfig(String group, String deploymentName, List<String> jarFullPaths,
+  private void addJarsToGroupConfig(String group, List<String> jarFullPaths,
       String deployedBy,
       String deployedTime) throws IOException {
     Region<String, Configuration> configRegion = getConfigurationRegion();
     Configuration configuration = getConfigurationCopy(configRegion, group);
 
     jarFullPaths.stream()
         .map(toFileName())
-        .map(jarFileName -> deploymentName != null
-            ? new Deployment(deploymentName, jarFileName, deployedBy, deployedTime)
-            : new Deployment(jarFileName, deployedBy, deployedTime))
+        .map(jarFileName -> new Deployment(jarFileName, deployedBy, deployedTime))
         .forEach(configuration::putDeployment);
 
     String memberId = cache.getMyId().getId();
@@ -366,11 +356,11 @@ private Configuration getConfigurationCopy(Region<String, Configuration> configR
   /**
    * Removes the jar files from the shared configuration. used when un-deploy jars
    *
-   * @param deploymentsToRemove Names of the jar files.
+   * @param jarNames Names of the jar files.
    * @param groups Names of the groups which had the jar file deployed.
    * @return true on success.
    */
-  public boolean removeDeployments(Map<String, String> deploymentsToRemove, String[] groups) {
+  public boolean removeJars(String[] jarNames, String[] groups) {
     lockSharedConfiguration();
     boolean success = true;
     try {
@@ -387,19 +377,19 @@ public boolean removeDeployments(Map<String, String> deploymentsToRemove, String
         logger.debug("Configuration before removing deployment: " + configuration);
         Configuration configurationCopy = new Configuration(configuration);
 
-        for (Entry<String, String> deploymentToRemove : deploymentsToRemove.entrySet()) {
-          File jar = getPathToJarOnThisLocator(group, deploymentToRemove.getValue()).toFile();
+        for (String jarName : jarNames) {
+          File jar = getPathToJarOnThisLocator(group, jarName).toFile();
           if (jar.exists()) {
             try {
               FileUtils.forceDelete(jar);
               logger.debug("Successfully deleted: " + jar.getName());
               configurationCopy
-                  .removeDeployments(Collections.singleton(deploymentToRemove.getKey()));
-              logger.debug("deploymentToRemove.getKey(): " + deploymentToRemove.getKey());
+                  .removeDeployments(Collections.singleton(jarName));
+              logger.debug("deploymentToRemove.getKey(): " + jarName);
             } catch (IOException e) {
               logger.error(
                   "Exception occurred while attempting to delete a jar from the filesystem: {}",
-                  deploymentToRemove, e);
+                  jarName, e);
             }
           }
         }

diff --git a/geode-core/src/main/java/org/apache/geode/internal/classloader/ClassPathLoader.java b/geode-core/src/main/java/org/apache/geode/internal/classloader/ClassPathLoader.java
@@ -157,8 +157,8 @@ public InputStream getResourceAsStream(String name) {
     };
   }
 
-  public void chainClassloader(File jar, String deploymentName) {
-    classPathService.chainClassloader(jar, deploymentName);
+  public void chainClassloader(File jar) {
+    classPathService.chainClassloader(jar);
   }
 
   public void unloadClassloaderForArtifact(String artifactId) {

diff --git a/geode-core/src/main/java/org/apache/geode/internal/classloader/ClasspathService.java b/geode-core/src/main/java/org/apache/geode/internal/classloader/ClasspathService.java
@@ -36,7 +36,7 @@ public interface ClasspathService {
 
   void init(boolean excludeTCCL, JarDeploymentService jarDeploymentService);
 
-  void chainClassloader(File jar, String deploymentName);
+  void chainClassloader(File jar);
 
   void unloadClassloaderForArtifact(String artifactId);