#334 Fixing some bugs with combined CMA requests

Turns out amps can't be included with databases and forests because REST API instances are created after them, but amps may depend on a modules database created by a REST API instance.

Also removed custom forests from a combined request, as there was little improvement to that, and it could lead to a bug in DHF with its custom command for deploying database fields.

Author: rjrudin

src/main/java/com/marklogic/appdeployer/command/AbstractCommand.java

@@ -7,6 +7,8 @@
 import com.marklogic.mgmt.admin.AdminManager;
 import com.marklogic.mgmt.api.API;
 import com.marklogic.mgmt.api.Resource;
+import com.marklogic.mgmt.api.configuration.Configuration;
+import com.marklogic.mgmt.api.configuration.Configurations;
 import com.marklogic.mgmt.cma.ConfigurationManager;
 import com.marklogic.mgmt.mapper.DefaultResourceMapper;
 import com.marklogic.mgmt.mapper.ResourceMapper;
@@ -480,6 +482,19 @@ protected boolean cmaEndpointExists(CommandContext context) {
 		return new ConfigurationManager(context.getManageClient()).endpointExists();
 	}
 
+	/**
+	 * Subclasses may override this to defer submission of a configuration so that it can be combined with other
+	 * configurations.
+	 *
+	 * @param context
+	 * @param config
+	 */
+	protected void deployConfiguration(CommandContext context, Configuration config) {
+		if (config.hasResources()) {
+			new Configurations(config).submit(context.getManageClient());
+		}
+	}
+
 	protected void setIncrementalMode(boolean incrementalMode) {
 		if (resourceFilenameFilter instanceof IncrementalFilenameFilter) {
 			((IncrementalFilenameFilter) resourceFilenameFilter).setIncrementalMode(incrementalMode);

src/main/java/com/marklogic/appdeployer/command/AbstractResourceCommand.java

@@ -175,19 +175,6 @@ protected void saveMergedResourcesViaCma(CommandContext context, List<ResourceRe
 		deployConfiguration(context, config);
 	}
 
-	/**
-	 * Subclasses may override this to defer submission of a configuration so that it can be combined with other
-	 * configurations.
-	 *
-	 * @param context
-	 * @param config
-	 */
-	protected void deployConfiguration(CommandContext context, Configuration config) {
-		if (config.hasResources()) {
-			new Configurations(config).submit(context.getManageClient());
-		}
-	}
-
 	/**
 	 * Defaults to the parent method. This was extracted so that a subclass can override it and have access to the
 	 * CommandContext, which allows for reading in the contents of each file and replacing tokens, which may impact the

src/main/java/com/marklogic/appdeployer/command/SortOrderConstants.java

@@ -32,11 +32,6 @@ public abstract class SortOrderConstants {
 	public static Integer DEPLOY_OTHER_DATABASES = 120;
     public static Integer DEPLOY_FORESTS = 150;
 
-	// The modules database must exist before we deploy amps
-	// For 3.15.0, amps are now deployed after databases and before servers so they can be included in a combined
-	// CMA request with databases and forests.
-	public static Integer DEPLOY_AMPS = 170;
-
 	public static Integer DEPLOY_REST_API_SERVERS = 200;
     public static Integer UPDATE_REST_API_SERVERS = 250;
     public static Integer DEPLOY_OTHER_SERVERS = 300;
@@ -51,7 +46,10 @@ public abstract class SortOrderConstants {
     public static Integer LOAD_MODULES = 400;
     public static Integer DELETE_TEST_MODULES = 410;
 
-    public static Integer DEPLOY_TRIGGERS = 700;
+	// The modules database must exist before we deploy amps
+	public static Integer DEPLOY_AMPS = 450;
+
+	public static Integer DEPLOY_TRIGGERS = 700;
 
 	public static Integer DEPLOY_TEMPORAL_AXIS = 750;
 	public static Integer DEPLOY_TEMPORAL_COLLECTIONS = 751;

src/main/java/com/marklogic/appdeployer/command/databases/DeployOtherDatabasesCommand.java

@@ -372,19 +372,12 @@ protected void deployDatabasesAndForestsViaCma(CommandContext context, List<Data
 			}
 		});
 
-		if (context.getAppConfig().getCmaConfig().isCombineRequests()) {
-			logger.info("Adding databases and forests to combined CMA request");
-			context.addCmaConfigurationToCombinedRequest(dbConfig);
-			context.addCmaConfigurationToCombinedRequest(forestConfig);
-			context.getContextMap().put("database-plans", databasePlans);
-		} else {
-			new Configurations(dbConfig, forestConfig).submit(context.getManageClient());
+		new Configurations(dbConfig, forestConfig).submit(context.getManageClient());
 
-			// Now account for sub-databases, but not yet (as of 3.15.0) with CMA
-			databasePlans.forEach(plan -> {
-				plan.getDeployDatabaseCommand().deploySubDatabases(plan.getDatabaseName(), context);
-			});
-		}
+		// Now account for sub-databases, but not yet (as of 3.15.0) with CMA
+		databasePlans.forEach(plan -> {
+			plan.getDeployDatabaseCommand().deploySubDatabases(plan.getDatabaseName(), context);
+		});
 	}
 
 	/**

src/main/java/com/marklogic/appdeployer/command/forests/DeployCustomForestsCommand.java

@@ -54,7 +54,7 @@ public void execute(CommandContext context) {
 		}
 
 		if (configuration != null) {
-			context.addCmaConfigurationToCombinedRequest(configuration);
+			deployConfiguration(context, configuration);
 		}
 	}
 

src/main/java/com/marklogic/appdeployer/command/security/DeployAmpsCommand.java

@@ -75,14 +75,4 @@ protected boolean ampIsUnchanged(ObjectNode amp) {
 		}
 		return false;
 	}
-
-	@Override
-	protected void deployConfiguration(CommandContext context, Configuration config) {
-		if (context.getAppConfig().getCmaConfig().isCombineRequests()) {
-			logger.info("Adding amps to combined CMA request");
-			context.addCmaConfigurationToCombinedRequest(config);
-		} else {
-			super.deployConfiguration(context, config);
-		}
-	}
 }

src/main/java/com/marklogic/appdeployer/impl/CmaDeployerListener.java

@@ -3,8 +3,6 @@
 import com.marklogic.appdeployer.command.Command;
 import com.marklogic.appdeployer.command.CommandContext;
 import com.marklogic.appdeployer.command.SortOrderConstants;
-import com.marklogic.appdeployer.command.databases.DatabasePlan;
-import com.marklogic.appdeployer.command.security.DeployAmpsCommand;
 import com.marklogic.appdeployer.command.security.DeployUsersCommand;
 import com.marklogic.mgmt.api.configuration.Configurations;
 
@@ -15,10 +13,10 @@
  * should be submitted when any of the following are true:
  *
  * <ol>
- * <li>DeployUsersCommand was just executed, meaning that a combined request of privileges, roles, and users should be submitted.</li>
- * <li>DeployOtherDatabaseCommand was just executed, meaning that a combined request of databases and forests should be submitted.</li>
+ * <li>Users were just deployed, meaning that a combined request of privileges, roles, protected paths,
+ * query rolesets, and users should be submitted.</li>
  * <li>No commands remain to be executed. In this case, need to check for a pending combined request that should be submitted. This can easily
- * happen in e.g. an ml-gradle context when running a task like mlDeployPrivileges or mlDeployDatabases.
+ * happen in e.g. an ml-gradle context when running a task like mlDeployPrivileges.
  * </li>
  * </ol>
  * <p>
@@ -30,33 +28,22 @@ public class CmaDeployerListener extends DeployerListenerSupport {
 
 	@Override
 	public void afterCommandExecuted(Command command, DeploymentContext context, List<Command> remainingCommands) {
-		if (combinedRequestBeSubmitted(command, remainingCommands)) {
+		if (combinedRequestShouldBeSubmitted(command, remainingCommands)) {
 			CommandContext commandContext = context.getCommandContext();
 
 			Configurations configs = commandContext.getCombinedCmaRequest();
 			if (configs != null) {
 				commandContext.removeCombinedCmaRequest();
-
 				if (configs.hasResources()) {
 					logger.info("Submitting combined CMA request");
 					configs.submit(commandContext.getManageClient());
-
-					if (command instanceof DeployAmpsCommand || remainingCommands.isEmpty()) {
-						List<DatabasePlan> databasePlans = (List<DatabasePlan>) commandContext.getContextMap().get("database-plans");
-						if (databasePlans != null) {
-							commandContext.getContextMap().remove("database-plans");
-							databasePlans.forEach(plan -> {
-								plan.getDeployDatabaseCommand().deploySubDatabases(plan.getDatabaseName(), commandContext);
-							});
-						}
-					}
 				}
 			}
 		}
 	}
 
-	protected boolean combinedRequestBeSubmitted(Command command, List<Command> remainingCommands) {
-		if (command instanceof DeployUsersCommand || command instanceof DeployAmpsCommand) {
+	protected boolean combinedRequestShouldBeSubmitted(Command command, List<Command> remainingCommands) {
+		if (command instanceof DeployUsersCommand) {
 			return true;
 		}
 
@@ -65,12 +52,11 @@ protected boolean combinedRequestBeSubmitted(Command command, List<Command> rema
 		}
 
 		/**
-		 * At least for many ml-app-deployer tests, a small subset of commands are used - for example, perhaps only the
-		 * commands for deploying databases and triggers are used. To ensure that databases are still deployed, we look
-		 * at the next command to see if it's being executed after amps are deployed, as amps are the last resource
-		 * we include in a combined request. If so, then we know we need to submit the combined request.
+		 * At least for many ml-app-deployer tests, a small subset of commands are used. To ensure a combined request
+		 * is submitted, we look at the next command to see if it executes after users are deployed, and if so, then
+		 * the request is submitted.
 		 */
 		Command nextCommand = remainingCommands.get(0);
-		return nextCommand.getExecuteSortOrder() >= SortOrderConstants.DEPLOY_AMPS;
+		return nextCommand.getExecuteSortOrder() >= SortOrderConstants.DEPLOY_USERS;
 	}
 }