[java-identity-provider] 02/06: IDP-1499 Installer: First pass tidy up of logging

Rod Widdowson rdw at steadingsoftware.com
Thu Oct 24 12:00:31 EDT 2019


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch master
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=0e397bbe5701100be90ee7e304ccc34dfcb3f8fb

commit 0e397bbe5701100be90ee7e304ccc34dfcb3f8fb
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue Oct 22 15:00:58 2019 +0100

    IDP-1499 Installer: First pass tidy up of logging
    
    https://issues.shibboleth.net/jira/browse/IDP-1499
---
 .../net/shibboleth/idp/installer/BuildWar.java     |  2 +-
 .../shibboleth/idp/installer/CopyDistribution.java |  7 +--
 .../shibboleth/idp/installer/InstallerSupport.java |  9 ++-
 .../net/shibboleth/idp/installer/V4Install.java    |  3 +-
 .../installer/impl/CurrentInstallStateImpl.java    | 68 +++++++++++++---------
 5 files changed, 52 insertions(+), 37 deletions(-)

diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/BuildWar.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/BuildWar.java
index 797f359..8e7f235 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/BuildWar.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/BuildWar.java
@@ -68,7 +68,7 @@ public final class BuildWar extends AbstractInitializableComponent {
         final Path target = installerProps.getTargetDir();
         final Path warFile = target.resolve("war").resolve("idp.war");
 
-        log.info("Rebuilding {}, Version", warFile.toAbsolutePath(), currentState.getInstalledVersion());
+        log.info("Rebuilding {}, Version {}", warFile.toAbsolutePath(), currentState.getInstalledVersion());
         InstallerSupport.deleteTree(target.resolve("webpapp"));
         final Path webAppTmp =target.resolve("webpapp.tmp");
         InstallerSupport.deleteTree(webAppTmp);
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/CopyDistribution.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/CopyDistribution.java
index 831df9a..58f4349 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/CopyDistribution.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/CopyDistribution.java
@@ -86,7 +86,7 @@ public final class CopyDistribution extends AbstractInitializableComponent {
      * @throws BuildException if badness occurs
      */
     private void backup(final Path from, final Path to) throws BuildException {
-        log.debug("Backing up From {} to {}", from, to);
+        log.debug("Backing up from {} to {}", from, to);
         final Copy copy = InstallerSupport.getCopyTask(from, to);
         copy.setFailOnError(false);
         copy.execute();
@@ -99,7 +99,7 @@ public final class CopyDistribution extends AbstractInitializableComponent {
         if (!Files.exists(what)) {
             log.debug("{} doesn't exist, nothing to delete", what);
         } else if (!Files.isDirectory(what)) {
-            log.error("Corrupt install {} is not a directory", what);
+            log.error("Corrupt install: {} is not a directory", what);
             throw new BuildException("Corrupt install - not a directory");
         } else {
             log.debug("Deleting {} ", what);
@@ -117,7 +117,6 @@ public final class CopyDistribution extends AbstractInitializableComponent {
         delete(installerProps.getTargetDir().resolve("doc"));
         final Path system = installerProps.getTargetDir().resolve("system");
         if (Files.exists(system)) {
-            log.debug("Clearing  {} readonly (if Windows)", system);
             InstallerSupport.setReadOnly(system, false);
         }
         delete(system);
@@ -146,7 +145,7 @@ public final class CopyDistribution extends AbstractInitializableComponent {
         InstallerSupport.createDirectory(dist);
         final Path src = installerProps.getSourceDir();
         if (!Files.exists(src)) {
-            log.error("Source distribution {} not found", src);
+            log.error("Source distribution {} not found.", src);
             throw new BuildException("Source distribution not found");
         }
         distCopy(src, dist, "conf");
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java
index e27b9e1..6b5e940 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerSupport.java
@@ -130,6 +130,11 @@ public final class InstallerSupport {
      * @throws BuildException if badness occurrs
      */
     public static void setReadOnly(final Path directory, final boolean readOnly) throws BuildException {
+        if (readOnly) {
+            log.debug("Setting readonly bits on {}", directory);
+        } else {
+            log.debug("Clearing readonly bits on {}", directory);
+        }
         if (!Os.isFamily(Os.FAMILY_WINDOWS)) {
             log.debug("Not windows. Not [re]setting readonly bit");
             return;
@@ -162,6 +167,7 @@ public final class InstallerSupport {
      */
     public static void setMode(final Path directory, final String permissions, final String includes)
             throws BuildException {
+        log.debug("Performing chmod {} on {} including {}", permissions, directory, includes);
         if (Os.isFamily(Os.FAMILY_WINDOWS)) {
             log.debug("Windows. Not performing chmod");
             return;
@@ -183,6 +189,7 @@ public final class InstallerSupport {
      */
     public static void setGroup(final Path directory, final String group, final String includes)
             throws BuildException {
+        log.debug("Performing chgrp {} on {} including {}", group, directory, includes);
         if (Os.isFamily(Os.FAMILY_WINDOWS)) {
             log.debug("Windows. Not performing chown");
             return;
@@ -208,7 +215,7 @@ public final class InstallerSupport {
             return;
         }
         if (!Files.isDirectory(where) ) {
-            log.error("Directory to be delete {} was a file");
+            log.error("Directory to be deleted ({}) was a file");
             throw new BuildException("Wanted a directory, found a file");
         }
         log.debug("Deleting tree {}", where);
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
index ed6faa6..1f9b7d5 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/V4Install.java
@@ -85,7 +85,7 @@ public class V4Install extends AbstractInitializableComponent {
         super.doInitialize();
         keyManager.initialize();
         if (metadataGenerator != null) {
-            log.warn("No metadata generator configured");
+            log.warn("No MetadataGenerator configured");
         }
     }
 
@@ -310,7 +310,6 @@ public class V4Install extends AbstractInitializableComponent {
      */
     protected void generateMetadata() throws BuildException {
         if (metadataGenerator == null) {
-            log.warn("No Metadata Generator registered");
             return;
         }
 
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/CurrentInstallStateImpl.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/CurrentInstallStateImpl.java
index 9bb3c82..93a820a 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/CurrentInstallStateImpl.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/CurrentInstallStateImpl.java
@@ -23,8 +23,10 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Properties;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.installer.CurrentInstallState;
@@ -36,6 +38,9 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 /** Tells the installers about the current install state. */
 public final class CurrentInstallStateImpl extends AbstractInitializableComponent implements CurrentInstallState {
 
+    /** Class logger. */
+    @Nonnull private final Logger log = LoggerFactory.getLogger(CurrentInstallStateImpl.class);
+
     /** Where we are installing to. */
     private final Path targetDir;
 
@@ -55,45 +60,50 @@ public final class CurrentInstallStateImpl extends AbstractInitializableComponen
         targetDir = installerProps.getTargetDir();
     }
 
-    /** {@inheritDoc} */
-    protected void doInitialize() throws ComponentInitializationException {
-        super.doInitialize();
-        idpPropertiesPresent = Files.exists(targetDir.resolve("conf").resolve("idp.properties"));
-        ldapPropertiesPresent = Files.exists(targetDir.resolve("conf").resolve("ldap.properties"));
+    /** Work out what the "current" install state is (before we do any more work).
+     * @throws ComponentInitializationException if we find a strange state
+     */
+    private void findPreviousVersion() throws ComponentInitializationException {
         final Path conf = targetDir.resolve("conf");
+        final Path currentInstall = targetDir.resolve("dist").resolve(InstallerSupport.VERSION_NAME);
         if (!Files.exists(conf.resolve("relying-party.xml"))) {
             // No relying party, no install
+            log.debug("No relying-party.xml file detetected.  Inferring a clean install");
             oldVersion = null;
-            return;
-        }
-        
-        if (!Files.exists(conf.resolve("idp.properties"))) {
+        } else if (!Files.exists(conf.resolve("idp.properties"))) {
             throw new ComponentInitializationException("V2 Installation detected");
-        }
-
-        final Path currentInstall = targetDir.resolve("dist").resolve(InstallerSupport.VERSION_NAME);
-        if (!Files.exists(currentInstall)) {
+        } else if (!Files.exists(currentInstall)) {
+            log.debug("No {} file detetected.  Inferring a V3 install", currentInstall);
             oldVersion= V3_VERSION;
-            return;
-        }
-        final Properties vers = new Properties(1);
-        try {
-            vers.load(new FileInputStream(currentInstall.toFile()));
-        } catch (final IOException e) {
-            LoggerFactory.getLogger(CurrentInstallStateImpl.class).
-                error("Could not load {}", currentInstall.toAbsolutePath(), e);
-            throw new ComponentInitializationException(e);
-        }
-        oldVersion = vers.getProperty(InstallerSupport.VERSION_NAME);
-        if (null == oldVersion) {
-            LoggerFactory.getLogger(CurrentInstallStateImpl.class).
-            error("Failed loading {}", currentInstall.toAbsolutePath());
-            throw new ComponentInitializationException("File " + InstallerSupport.VERSION_NAME +
-                    " did not contain property " + InstallerSupport.VERSION_NAME);
+        } else {
+            final Properties vers = new Properties(1);
+            try {
+                vers.load(new FileInputStream(currentInstall.toFile()));
+            } catch (final IOException e) {
+                LoggerFactory.getLogger(CurrentInstallStateImpl.class).
+                    error("Could not load {}", currentInstall.toAbsolutePath(), e);
+                throw new ComponentInitializationException(e);
+            }
+            oldVersion = vers.getProperty(InstallerSupport.VERSION_NAME);
+            if (null == oldVersion) {
+                LoggerFactory.getLogger(CurrentInstallStateImpl.class).
+                error("Failed loading {}", currentInstall.toAbsolutePath());
+                throw new ComponentInitializationException("File " + InstallerSupport.VERSION_NAME +
+                        " did not contain property " + InstallerSupport.VERSION_NAME);
+            }
+            log.debug("Previous version {}", oldVersion);
         }
     }
 
     /** {@inheritDoc} */
+    protected void doInitialize() throws ComponentInitializationException {
+        super.doInitialize();
+        idpPropertiesPresent = Files.exists(targetDir.resolve("conf").resolve("idp.properties"));
+        ldapPropertiesPresent = Files.exists(targetDir.resolve("conf").resolve("ldap.properties"));
+        findPreviousVersion();
+    }
+
+    /** {@inheritDoc} */
     @Nullable public String getInstalledVersion() {
         return oldVersion;
     }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list