Cleanup & fixes

prospero-cli/src/main/java/org/wildfly/prospero/cli/CliMessages.java

@@ -572,4 +572,8 @@ default String featurePackTitle() {
     default String configurationModel() {
         return bundle.getString("prospero.history.configuration_model.title");
     }
+
+    default String diffFeaturesChanges() {
+        return bundle.getString("prospero.changes.diff.features_changes");
+    }
 }

prospero-cli/src/main/java/org/wildfly/prospero/cli/commands/HistoryCommand.java

@@ -63,22 +63,25 @@ public Integer call() throws Exception {
                 console.println(CliMessages.MESSAGES.noChangesFound());
             } else {
                 final DiffPrinter diffPrinter = new DiffPrinter("  ");
+                boolean needsLineBreak = false;
                 if (!changes.getArtifactChanges().isEmpty()) {
                     console.println(CliMessages.MESSAGES.diffUpdates()+ ":");
                     changes.getArtifactChanges().forEach(diffPrinter::print);
+                    needsLineBreak = true;
                 }
                 if (!changes.getChannelChanges().isEmpty()) {
-                    if (!changes.getArtifactChanges().isEmpty()) {
+                    if (needsLineBreak) {
                         console.println("");
                     }
                     console.println(CliMessages.MESSAGES.diffConfigChanges()+ ":");
                     changes.getChannelChanges().forEach(diffPrinter::print);
+                    needsLineBreak = true;
                 }
                 if (!changes.getFeatureChanges().isEmpty()) {
-                    if (!changes.getArtifactChanges().isEmpty()) {
+                    if (needsLineBreak) {
                         console.println("");
                     }
-                    console.println("Installed features changes" + ":");
+                    console.println(CliMessages.MESSAGES.diffFeaturesChanges() + ":");
                     changes.getFeatureChanges().forEach(diffPrinter::print);
                 }
             }

prospero-cli/src/main/resources/UsageMessages.properties

@@ -345,5 +345,6 @@ prospero.changes.diff.removed=Removed
 prospero.changes.diff.conf_changes=Configuration changes
 prospero.changes.diff.artifact=artifact
 prospero.changes.diff.channel=channel
+prospero.changes.diff.features_changes=Installed features changes
 
 prospero.changes.conflict.header=Conflicting changes detected in the update:

prospero-common/src/main/java/org/wildfly/prospero/ProsperoLogger.java

@@ -339,4 +339,8 @@ public interface ProsperoLogger extends BasicLogger {
 
     @Message(id = 257, value = "Feature pack %s is already provisioned")
     FeaturesAddAction.FeaturePackAlreadyInstalledException featurePackAlreadyInstalled(FeaturePackLocation fpl);
+
+    @LogMessage(level = Logger.Level.DEBUG)
+    @Message(id = 258, value = "Adding a feature pack [%s] with configId [%s:%s] and layers [%s]")
+    void addingFeaturePack(FeaturePackLocation fpl, String selectedConfig, String selectedModel, String layers);
 }

prospero-common/src/main/java/org/wildfly/prospero/actions/ApplyCandidateAction.java

@@ -319,13 +319,13 @@ private FsDiff findChanges() throws ProvisioningException, OperationException {
     private void updateMetadata(Type operation) throws ProvisioningException, MetadataException {
         try {
             copyCurrentVersions();
-            ProsperoMetadataUtils.recordProvisioningDefinition(updateDir, installationDir);
             writeProsperoMetadata(operation);
             updateInstallationCache();
             Path installationGalleonPath = PathsUtils.getProvisionedStateDir(installationDir);
             Path updateGalleonPath = PathsUtils.getProvisionedStateDir(updateDir);
             IoUtils.recursiveDelete(installationGalleonPath);
             IoUtils.copy(updateGalleonPath, installationGalleonPath, true);
+            ProsperoMetadataUtils.recordProvisioningDefinition(installationDir, installationDir);
         } catch (IOException ex) {
             throw new ProvisioningException(ex);
         }

prospero-common/src/main/java/org/wildfly/prospero/actions/FeaturesAddAction.java

@@ -18,6 +18,7 @@
 package org.wildfly.prospero.actions;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.jboss.galleon.ProvisioningDescriptionException;
 import org.jboss.galleon.ProvisioningException;
 import org.jboss.galleon.ProvisioningManager;
@@ -41,6 +42,7 @@
 import org.wildfly.prospero.api.MavenOptions;
 import org.wildfly.prospero.api.TemporaryRepositoriesHandler;
 import org.wildfly.prospero.api.exceptions.ArtifactResolutionException;
+import org.wildfly.prospero.api.exceptions.InvalidUpdateCandidateException;
 import org.wildfly.prospero.api.exceptions.MetadataException;
 import org.wildfly.prospero.api.exceptions.OperationException;
 import org.wildfly.prospero.galleon.FeaturePackLocationParser;
@@ -60,6 +62,9 @@
 import java.util.Set;
 import java.util.TreeSet;
 
+/**
+ * Installs a feature pack onto an existing server.
+ */
 public class FeaturesAddAction {
 
     private final MavenSessionManager mavenSessionManager;
@@ -88,6 +93,22 @@ public FeaturesAddAction(MavenOptions mavenOptions, Path installDir, List<Reposi
         this.candidateActionsFactory = candidateActionsFactory;
     }
 
+    /**
+     * performs feature pack installation. The added feature pack can be customized by specifying layers and configuration model name.
+     * In order to install a feature pack, a server is re-provisioned and changes are applied to existing server.
+     *
+     * @param featurePackCoord - maven {@code groupId:artifactId} coordinates of the feature pack to install
+     * @param layers - set of layer names to be provisioned
+     * @param model - used to select layer model, if the feature pack provides multiple models
+     * @param configName - name of the configuration file to generate if supported
+     *
+     * @throws ProvisioningException - if unable to provision the server
+     * @throws ModelNotDefinedException - if requested model is not provided by the feature pack
+     * @throws LayerNotFoundException - if one of the requested layers is not provided by the feature pack
+     * @throws FeaturePackAlreadyInstalledException - if the requested feature pack configuration wouldn't change the server state
+     * @throws InvalidUpdateCandidateException - if the folder at {@code updateDir} is not a valid update
+     * @throws MetadataException - if unable to read or write the installation of update metadata
+     */
     public void addFeaturePack(String featurePackCoord, Set<String> layers, String model, String configName)
             throws ProvisioningException, OperationException {
         if (featurePackCoord == null || featurePackCoord.isEmpty()) {
@@ -99,17 +120,29 @@ public void addFeaturePack(String featurePackCoord, Set<String> layers, String m
 
         FeaturePackLocation fpl = FeaturePackLocationParser.resolveFpl(featurePackCoord);
 
+        if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+            ProsperoLogger.ROOT_LOGGER.trace("Adding feature pack " + fpl);
+        }
+
         final String selectedConfig;
         final String selectedModel;
 
         final Map<String, Set<String>> allLayers = getAllLayers(fpl);
 
+        if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+            ProsperoLogger.ROOT_LOGGER.trace("Found layers");
+            for (String key : allLayers.keySet()) {
+                ProsperoLogger.ROOT_LOGGER.trace(key + ": " + StringUtils.join(allLayers.get(key)));
+            }
+        }
+
         if (allLayers.isEmpty()) {
             selectedModel = null;
         } else {
             selectedModel = getSelectedModel(model, allLayers);
         }
 
+
         verifyLayerAvailable(layers, selectedModel, allLayers);
 
         if (configName == null) {
@@ -122,38 +155,59 @@ public void addFeaturePack(String featurePackCoord, Set<String> layers, String m
             selectedConfig = configName;
         }
 
+        if (ProsperoLogger.ROOT_LOGGER.isDebugEnabled()) {
+            ProsperoLogger.ROOT_LOGGER.addingFeaturePack(fpl, selectedConfig, selectedModel, StringUtils.join(layers));
+        }
+
         final ProvisioningConfig newConfig = buildProvisioningConfig(layers, fpl, selectedConfig, selectedModel);
 
         final Path candidate;
         try {
             candidate = Files.createTempDirectory("prospero-candidate").toAbsolutePath();
+            if (ProsperoLogger.ROOT_LOGGER.isDebugEnabled()) {
+                ProsperoLogger.ROOT_LOGGER.temporaryCandidateFolder(candidate);
+            }
             Runtime.getRuntime().addShutdownHook(new Thread(() -> FileUtils.deleteQuietly(candidate.toFile())));
         } catch (IOException e) {
             throw ProsperoLogger.ROOT_LOGGER.unableToCreateTemporaryDirectory(e);
         }
 
         try (PrepareCandidateAction prepareCandidateAction = candidateActionsFactory.newPrepareCandidateActionInstance(mavenSessionManager, prosperoConfig);
              GalleonEnvironment galleonEnv = getGalleonEnv(candidate)) {
+            ProsperoLogger.ROOT_LOGGER.updateCandidateStarted(installDir);
             prepareCandidateAction.buildCandidate(candidate, galleonEnv, ApplyCandidateAction.Type.FEATURE_ADD, newConfig);
+            ProsperoLogger.ROOT_LOGGER.updateCandidateCompleted(installDir);
         }
 
         final ApplyCandidateAction applyCandidateAction = candidateActionsFactory.newApplyCandidateActionInstance(candidate);
         applyCandidateAction.applyUpdate(ApplyCandidateAction.Type.FEATURE_ADD);
     }
 
+    /**
+     * check if a feature pack with {@code featurePackCoord} can be resolved in available channels.
+     *
+     * @param featurePackCoord - maven {@code groupId:artifactId} coordinates of the feature pack to install
+     * @return true if the feature pack is available, false otherwise
+     * @throws OperationException - if unable to read the metadata
+     * @throws ProvisioningException - if unable to read the metadata
+     */
     public boolean isFeaturePackAvailable(String featurePackCoord) throws OperationException, ProvisioningException {
         if (featurePackCoord == null || featurePackCoord.isEmpty()) {
             throw new IllegalArgumentException("The feature pack coordinate cannot be null");
         }
-        if (featurePackCoord.split(":").length != 2) {
+        final String[] splitCoordinates = featurePackCoord.split(":");
+        if (splitCoordinates.length != 2) {
             throw new IllegalArgumentException("The feature pack coordinate has to consist of <groupId>:<artifactId>");
         }
         final ChannelSession channelSession = GalleonEnvironment
                 .builder(installDir, prosperoConfig.getChannels(), mavenSessionManager).build()
                 .getChannelSession();
 
         try {
-            channelSession.resolveMavenArtifact(featurePackCoord.split(":")[0], featurePackCoord.split(":")[1],
+            if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+                ProsperoLogger.ROOT_LOGGER.trace("Resolving a feature pack: " + featurePackCoord);
+            }
+            channelSession.resolveMavenArtifact(splitCoordinates[0], splitCoordinates[1],
                     "zip", null, null);
         } catch (NoStreamFoundException e) {
             return false;
@@ -213,11 +267,17 @@ private static ConfigModel.Builder buildLayerConfig(Set<String> layers, String s
         final ConfigModel.Builder configBuilder;
         final ConfigId id = new ConfigId(selectedModel, selectedConfig);
         if (existingConfig.hasDefinedConfig(id)) {
+            if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+                ProsperoLogger.ROOT_LOGGER.trace("Replacing existing ConfigModel " + id);
+            }
             ConfigModel cmodel = existingConfig.getDefinedConfig(id);
             configBuilder = ConfigModel.builder(cmodel);
             includeLayers(layers, configBuilder, cmodel);
             builder.removeConfig(id);
         } else {
+            if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+                ProsperoLogger.ROOT_LOGGER.trace("Adding new ConfigModel " + id);
+            }
             configBuilder = ConfigModel.builder(selectedModel, selectedConfig);
             for (String layer: layers) {
                 configBuilder.includeLayer(layer);
@@ -229,9 +289,15 @@ private static ConfigModel.Builder buildLayerConfig(Set<String> layers, String s
     private static void includeLayers(Set<String> layers, ConfigModel.Builder configBuilder, ConfigModel cmodel) throws ProvisioningDescriptionException {
         for (String layer: layers) {
             if (cmodel.getExcludedLayers().contains(layer)){
+                if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+                    ProsperoLogger.ROOT_LOGGER.trace("Un-excluding layer" + layer);
+                }
                 configBuilder.removeExcludedLayer(layer);
             }
             if (!cmodel.getIncludedLayers().contains(layer)) {
+                if (ProsperoLogger.ROOT_LOGGER.isTraceEnabled()) {
+                    ProsperoLogger.ROOT_LOGGER.trace("Adding layer " + layer);
+                }
                 configBuilder.includeLayer(layer);
             }
         }

prospero-common/src/main/java/org/wildfly/prospero/api/Diff.java

@@ -52,6 +52,11 @@ public Optional<Diff> getChild(String name) {
         return children.stream().filter(c->c.getName().orElse("").equals(name)).findFirst();
     }
 
+    /**
+     * checks if the record holds any non-null values.
+     *
+     * @return true if either {@code newValue} or {@code oldValue} has a non-null value.
+     */
     public boolean hasValues() {
         return !newValue.isEmpty() || !oldValue.isEmpty();
     }

prospero-common/src/main/java/org/wildfly/prospero/api/FeatureChange.java

@@ -19,10 +19,16 @@
 
 import java.util.Locale;
 
+/**
+ * Represents changes to feature packs used to provison the server.
+ */
 public class FeatureChange extends Diff {
 
     private final Type type;
 
+    /**
+     * Type of feature change record
+     */
     public enum Type { FEATURE, LAYERS, CONFIG }
 
     public FeatureChange(Type type, String oldValue, String newValue) {

prospero-common/src/main/java/org/wildfly/prospero/api/InstallationMetadata.java

@@ -394,7 +394,12 @@ public void close() {
         }
     }
 
-    public ProvisioningConfig getRecordedProvisioningConfig() throws ProvisioningException {
+    /**
+     * galleon configuration used to provision current state of the server.
+     *
+     * @return
+     */
+    public ProvisioningConfig getRecordedProvisioningConfig() {
         return provisioningConfig;
     }
 }

prospero-common/src/main/java/org/wildfly/prospero/galleon/GalleonUtils.java

@@ -139,16 +139,18 @@ public Path resolve(FeaturePackLocation fpl) throws ProvisioningException {
         }
     }
 
+    /**
+     * {@link ProvisioningLayoutFactory} using {@code maven} to resolve artifacts.
+     *
+     * @param maven
+     * @return
+     * @throws ProvisioningException
+     */
     public static ProvisioningLayoutFactory getProvisioningLayoutFactory(MavenRepoManager maven) throws ProvisioningException {
-        final UniverseResolver.Builder builder = UniverseResolver.builder()
-                .addArtifactResolver(maven);
-        UniverseResolver universeResolver = new UniverseResolver(builder) {
-            @Override
-            public Path resolve(FeaturePackLocation fpl) throws ProvisioningException {
-                return super.resolve(fpl);
-            }
-        };
-        return ProvisioningLayoutFactory.getInstance(universeResolver);
+        final UniverseResolver resolver = UniverseResolver.builder()
+                .addArtifactResolver(maven).build();
+
+        return ProvisioningLayoutFactory.getInstance(resolver);
     }
 
     public static List<String> getInstalledPacks(Path dir) throws ProvisioningException {

prospero-common/src/main/java/org/wildfly/prospero/installation/git/FeatureChangeParser.java

@@ -44,6 +44,9 @@
 import static org.wildfly.prospero.api.FeatureChange.Type.LAYERS;
 import static org.wildfly.prospero.api.FeatureChange.Type.CONFIG;
 
+/**
+ * Generates a {@code Diff} of recorded provisioning state changes.
+ */
 class FeatureChangeParser implements GitStorage.Parser<FeatureChange> {
     @Override
     public List<FeatureChange> parse(Path changed, Path base) throws IOException, MetadataException {

prospero-common/src/main/java/org/wildfly/prospero/installation/git/GitStorage.java

@@ -290,7 +290,7 @@ public List<ChannelChange> getChannelChanges(SavedState savedState) throws Metad
     }
 
     public List<FeatureChange> getFeatureChanges(SavedState latestState) throws MetadataException {
-        return getChanges(latestState, ".provisioning_record.xml", new FeatureChangeParser());
+        return getChanges(latestState, ProsperoMetadataUtils.PROVISIONING_RECORD_XML, new FeatureChangeParser());
     }
 
     private <T> List<T> getChanges(SavedState savedState, String manifestFileName, Parser<T> parser) throws MetadataException {

prospero-metadata/src/main/java/org/wildfly/prospero/metadata/ProsperoMetadataUtils.java

@@ -186,6 +186,17 @@ public static Path configurationPath(Path serverDir) {
         return serverDir.resolve(METADATA_DIR).resolve(INSTALLER_CHANNELS_FILE_NAME);
     }
 
+    /**
+     * creates a copy of Galleon provisioning configuration from {@code Constants.PROVISIONED_STATE_DIR/Constants.PROVISIONING_XML}
+     * in the {@code METADATA_DIR}.
+     *
+     * If the source file doesn't exist no copy is created.
+     * If the provisioning configuration is the same as saved copy, the file is not overwritten.
+     *
+     * @param sourceServer - root folder of the server the provisioning configuration will be stored
+     * @param targetServer - root folder of the server to copy the provisioning configuration to
+     * @throws IOException - if the file cannot be copied.
+     */
     public static void recordProvisioningDefinition(Path sourceServer, Path targetServer) throws IOException {
         final Path provisioningFile = sourceServer.resolve(Constants.PROVISIONED_STATE_DIR).resolve(Constants.PROVISIONING_XML);
         final Path provisioningRecordFile = targetServer.resolve(METADATA_DIR).resolve(PROVISIONING_RECORD_XML);