[java-identity-provider] branch main updated: Null handling in the idp-installer pass 1 & 2

Rod Widdowson rdw at steadingsoftware.com
Sat Jan 21 15:34:50 UTC 2023


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

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

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

The following commit(s) were added to refs/heads/main by this push:
     new a3cd0bd55 Null handling in the idp-installer pass 1 & 2
a3cd0bd55 is described below

commit a3cd0bd5584319cc7f6877dbb19c2b77329d0c25
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sat Jan 21 15:33:21 2023 +0000

    Null handling in the idp-installer pass 1 & 2
    
    The PluginInstaller needs refactoring to handle httpClient
    (which is never null but cannot be set as such right now).
---
 .../idp/installer/CurrentInstallState.java         |   6 +-
 .../idp/installer/InstallerProperties.java         |   2 +-
 .../idp/installer/InstallerPropertiesImpl.java     |  90 +++++++++------
 .../idp/installer/PropertiesWithComments.java      |   6 +-
 .../net/shibboleth/idp/installer/V4Install.java    |  48 ++++----
 .../installer/ant/impl/MetadataGeneratorTask.java  |  31 +++--
 .../idp/installer/ant/impl/PasswordHandler.java    |   1 +
 .../idp/installer/ant/impl/V4InstallTask.java      |   2 +-
 .../installer/impl/CurrentInstallStateImpl.java    |  17 +--
 .../idp/installer/impl/InstallationLogger.java     |  13 ++-
 .../metadata/impl/MetadataGeneratorImpl.java       |  13 ++-
 .../impl/MetadataGeneratorParametersImpl.java      |  23 +++-
 .../idp/installer/plugin/impl/LoggingVisitor.java  |   5 +-
 .../idp/installer/plugin/impl/PluginInfo.java      |   2 +-
 .../idp/installer/plugin/impl/PluginInstaller.java |  82 ++++++++-----
 .../plugin/impl/PluginInstallerArguments.java      |  44 ++++---
 .../installer/plugin/impl/PluginInstallerCLI.java  | 128 ++++++++++++++-------
 .../plugin/impl/PluginInstallerSupport.java        |   3 +-
 .../idp/installer/plugin/impl/PluginState.java     |   8 +-
 .../plugin/impl/RollbackPluginInstall.java         |  17 +--
 .../idp/installer/plugin/impl/TrustStore.java      |  14 ++-
 .../java/net/shibboleth/idp/installer/Test.java    |   3 +-
 .../idp/installer/plugin/impl/BasePluginTest.java  |   2 +-
 .../installer/plugin/impl/PluginInstallerTest.java |   8 +-
 .../idp/installer/plugin/impl/PluginStateTest.java |   4 +-
 .../idp/installer/plugin/impl/RollbackTester.java  |  32 +++---
 .../idp/installer/plugin/impl/TestModule.java      |  22 ++--
 .../idp/installer/plugin/impl/TestPlugin.java      |   6 +-
 28 files changed, 405 insertions(+), 227 deletions(-)

diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/CurrentInstallState.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/CurrentInstallState.java
index e79c2f37f..4932d6545 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/CurrentInstallState.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/CurrentInstallState.java
@@ -19,7 +19,6 @@ package net.shibboleth.idp.installer;
 
 import java.nio.file.Path;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Properties;
 
@@ -29,6 +28,7 @@ import javax.annotation.Nullable;
 import org.apache.tools.ant.BuildException;
 
 import net.shibboleth.idp.spring.IdPPropertiesApplicationContextInitializer;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.InitializableComponent;
 
 /** Tells the installers about the current install state. */
@@ -80,8 +80,8 @@ public interface CurrentInstallState extends InitializableComponent {
     /** Which modules (by ID) are enabled for this release.
      * @return those modules enabled.
      */
-    default @Nonnull Collection<String> getEnabledModules() {
-        return Collections.emptySet();
+    @Nonnull default Collection<String> getEnabledModules() {
+        return CollectionSupport.emptySet();
     }
 
     /** Build a classpath loader which adds all the plugins in.
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java
index 79853b039..c83f22c60 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerProperties.java
@@ -32,7 +32,7 @@ import net.shibboleth.shared.component.InitializableComponent;
 public interface InstallerProperties extends InitializableComponent {
 
     /** Those modules enabled by default. */
-    public static final Set<String> DEFAULT_MODULES = Set.of("idp.authn.Password", "idp.admin.Hello");
+    @Nonnull public static final Set<String> DEFAULT_MODULES = Set.of("idp.authn.Password", "idp.admin.Hello");
 
     /** Get where we are installing/updating/building the war.
      * @return the target directory
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerPropertiesImpl.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerPropertiesImpl.java
index eff78dd04..c0ce97cb2 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerPropertiesImpl.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/InstallerPropertiesImpl.java
@@ -31,7 +31,6 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
-import java.util.function.Supplier;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -48,6 +47,7 @@ import net.shibboleth.idp.installer.impl.InstallationLogger;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.primitive.NonnullSupplier;
 import net.shibboleth.shared.primitive.StringSupport;
 
 /** Class implement {@link InstallerProperties} with properties/UI driven values.
@@ -267,8 +267,10 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
         noPrompt = value != null;
 
         if (needSourceDir) {
+            final String baseDirAsString = baseDir.toString();
+            assert baseDirAsString!=null;
             value = getValue(SOURCE_DIR, "Source (Distribution) Directory (press <enter> to accept default):",
-                    () -> baseDir.toString());
+                    () -> baseDirAsString);
             srcDir = Path.of(value);
             log.debug("Source directory {}", srcDir.toAbsolutePath());
         }
@@ -291,8 +293,8 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
      * @throws BuildException of anything goes wrong
      * @return the value
      */
-    protected String getValue(final String propertyName,
-            final String prompt, final Supplier<String> defaultSupplier) throws BuildException {
+    @Nonnull protected String getValue(final String propertyName,
+            final String prompt, final NonnullSupplier<String> defaultSupplier) throws BuildException {
         String value = installerProperties.getProperty(propertyName);
         if (value != null) {
             return value;
@@ -321,7 +323,7 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
      * @throws BuildException of anything goes wrong
      * @return the value.  this is not repeated to the screen
      */
-    protected String getPassword(final String propertyName, final String prompt) throws BuildException {
+    @Nonnull protected String getPassword(final String propertyName, final String prompt) throws BuildException {
         final String value = installerProperties.getProperty(propertyName);
         if (value != null) {
             return value;
@@ -333,7 +335,11 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
         final InputRequest request = new InputRequest(prompt);
 
         new PasswordHandler().handleInput(request);
-        return request.getInput();
+        @Nullable final String result = request.getInput();
+        if (result == null) {
+            throw new BuildException("Null result from Ant PasswordHandler");
+        }
+        return result;
     }
 
     /** {@inheritDoc}
@@ -351,9 +357,10 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
         } else {
             // build-war or Windows so "here" is also "where"
             defTarget = baseDir.toAbsolutePath().toString();
+            assert defTarget!=null;
         }
-        final String targetValue = getValue(TARGET_DIR, "Installation Directory:", () -> defTarget);
-        targetDir = Path.of(targetValue);
+        final Path targetDir= Path.of(getValue(TARGET_DIR, "Installation Directory:", () -> defTarget));
+        assert targetDir != null;
         return targetDir;
     }
 
@@ -366,10 +373,11 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
      * Defaults to information pulled from {{@link #getHostName()}.
      */
     @Nonnull public String getEntityID() {
-        if (entityID == null) {
-            entityID = getValue(ENTITY_ID, "SAML EntityID:", () -> "https://" + getHostName() + "/idp/shibboleth");
+        String result = entityID;
+        if (result == null) {
+            entityID = result = getValue(ENTITY_ID, "SAML EntityID:", () -> "https://" + getHostName() + "/idp/shibboleth");
         }
-        return entityID;
+        return result;
     }
 
     /** {@inheritDoc} */
@@ -391,7 +399,7 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
      * @return the best name we can work out
      */
     // CheckStyle: CyclomaticComplexity OFF
-    private String bestHostName() {
+    @Nonnull private String bestHostName() {
         InetAddress bestSoFar = null;
         try {
             for (final NetworkInterface netInterface : Collections.list(NetworkInterface.getNetworkInterfaces())) {
@@ -435,7 +443,9 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
         if (bestSoFar == null) {
             return "localhost.localdomain";
         }
-        return bestSoFar.getCanonicalHostName();
+        final String result = bestSoFar.getCanonicalHostName();
+        assert result!=null;
+        return result;
     }
     // CheckStyle: CyclomaticComplexity ON
 
@@ -443,18 +453,22 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
      * Defaults to information pulled from the network.
      */
     @Nonnull public String getHostName() {
-        if (hostname == null) {
-            hostname = getValue(HOST_NAME, "Host Name:", () -> bestHostName());
+        String result = hostname;
+        if (result == null) {
+            result = hostname = getValue(HOST_NAME, "Host Name:", () -> bestHostName());
         }
-        return hostname;
+        return result;
     }
 
     /** {@inheritDoc} */
     @Override @Nonnull public String getCredentialsKeyFileMode() {
-        if (credentialsKeyFileMode == null) {
-            credentialsKeyFileMode = installerProperties.getProperty(MODE_CREDENTIAL_KEYS, "600");
+        String result = credentialsKeyFileMode;
+        if (result != null) {
+            return result;
         }
-        return credentialsKeyFileMode;
+        result = credentialsKeyFileMode = installerProperties.getProperty(MODE_CREDENTIAL_KEYS, "600");
+        assert result != null;
+        return result;
     }
 
     /** {@inheritDoc} */
@@ -475,17 +489,20 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
         final String host = getHostName();
         final int index = host.indexOf('.');
         if (index > 1) {
-            return host.substring(index+1);
+            final String result =host.substring(index+1);
+            assert result != null;
+            return result;
         }
         return "localdomain";
     }
 
     /** {@inheritDoc}. */
     @Override @Nonnull public String getScope() {
-        if (scope == null) {
-            scope = getValue(SCOPE, "Attribute Scope:", () -> defaultScope());
+        String result = scope;
+        if (result  == null) {
+            result = scope = getValue(SCOPE, "Attribute Scope:", () -> defaultScope());
         }
-        return scope;
+        return result;
     }
 
     /** {@inheritDoc}. */
@@ -500,18 +517,20 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
 
     /** {@inheritDoc}. */
     @Override  @Nonnull public String getKeyStorePassword() {
+        @SuppressWarnings("null") @Nonnull String result = keyStorePassword;
         if (keyStorePassword == null) {
-            keyStorePassword = getPassword(KEY_STORE_PASSWORD, "Backchannel PKCS12 Password:");
+            result = keyStorePassword = getPassword(KEY_STORE_PASSWORD, "Backchannel PKCS12 Password:");
         }
-        return keyStorePassword;
+        return result;
     }
 
     /** {@inheritDoc}. */
     @Override @Nonnull public String getSealerPassword() {
-        if (sealerPassword == null) {
-            sealerPassword = getPassword(SEALER_PASSWORD, "Cookie Encryption Key Password:");
+        String result = sealerPassword;
+        if (result == null) {
+            result = sealerPassword = getPassword(SEALER_PASSWORD, "Cookie Encryption Key Password:");
         }
-        return sealerPassword;
+        return result;
     }
 
     /** {@inheritDoc} */
@@ -526,7 +545,9 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
         }
         final String[] modules = prop.split(",");
         if (!additive) {
-            return Set.copyOf(Arrays.asList(modules));
+            final Set<String> result = Set.copyOf(Arrays.asList(modules));
+            assert result != null;
+            return result;
         }
         final Set<String> result = new HashSet<>(modules.length + InstallerProperties.DEFAULT_MODULES.size());
         result.addAll(InstallerProperties.DEFAULT_MODULES);
@@ -536,13 +557,14 @@ public class InstallerPropertiesImpl extends AbstractInitializableComponent impl
 
     /** {@inheritDoc}. */
     @Nonnull public String getSealerAlias() {
-        if (sealerAlias == null) {
-            sealerAlias = installerProperties.getProperty(SEALER_ALIAS);
+        String result = sealerAlias;
+        if (result == null) {
+            result = sealerAlias = installerProperties.getProperty(SEALER_ALIAS);
         }
-        if (sealerAlias == null) {
-            sealerAlias = "secret";
+        if (result == null) {
+            result = sealerAlias = "secret";
         }
-        return sealerAlias;
+        return result;
     }
 
     /** {@inheritDoc}. default is {@value #DEFAULT_KEY_SIZE}. */
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
index e0532bc69..9abfb7cc2 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/PropertiesWithComments.java
@@ -36,6 +36,7 @@ import java.util.Set;
 import javax.annotation.Nonnull;
 
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 
@@ -57,7 +58,7 @@ public final class PropertiesWithComments {
     private Map<String, CommentedProperty> properties;
 
     /** Name Replacement info. */
-    private final Properties nameReplacement;
+    @Nonnull private final Properties nameReplacement = new Properties();
 
     /**  BlackListed property names. */
     @Nonnull private final Set<String> unreplacableNames;
@@ -70,7 +71,7 @@ public final class PropertiesWithComments {
 
     /** Legacy Constructor. */
     public PropertiesWithComments() {
-        this(Collections.emptySet());
+        this(CollectionSupport.emptySet());
     }
 
     /** Constructor.
@@ -78,7 +79,6 @@ public final class PropertiesWithComments {
      */
     public PropertiesWithComments(@Nonnull final Set<String> unreplacable) {
         unreplacableNames = Set.copyOf(unreplacable);
-        nameReplacement = new Properties();
     }
 
     /**
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 754521dca..493d69713 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
@@ -57,6 +57,7 @@ import net.shibboleth.idp.module.ModuleException;
 import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.idp.plugin.PluginVersion;
 import net.shibboleth.idp.spring.IdPPropertiesApplicationContextInitializer;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.component.UninitializedComponentException;
@@ -242,7 +243,7 @@ public class V4Install extends AbstractInitializableComponent {
      */
     // CheckStyle: CyclomaticComplexity|MethodLength OFF
     protected void populatePropertyFiles(final boolean sealerCreated) throws BuildException {
-        final Set<String> doNotReplaceList = Set.of(
+        @Nonnull final Set<String> doNotReplaceList = Set.of(
                 "idp.sealer.storePassword",
                 "idp.sealer.keyPassword",
                 "idp.authn.LDAP.bindDNCredential",
@@ -517,15 +518,16 @@ public class V4Install extends AbstractInitializableComponent {
         }
 
         final Path parentDir = installerProps.getTargetDir().resolve("metadata");
-        final Path metadataFile = parentDir.resolve("idp-metadata.xml");
-        if (Files.exists(metadataFile)) {
+        final File metadataFile = parentDir.resolve("idp-metadata.xml").toFile();
+        assert metadataFile != null;
+        if (metadataFile.exists()) {
             log.debug("Metadata file {} exists", metadataFile.toString());
             return;
         }
         final Resource resource = new ClassPathResource("net/shibboleth/idp/installer/metadata-generator.xml");
         final GenericApplicationContext context = new ApplicationContextBuilder()
                 .setName(MetadataGenerator.class.getName())
-                .setServiceConfigurations(Collections.singletonList(resource))
+                .setServiceConfigurations(CollectionSupport.singletonList(resource))
                 .setContextInitializer(new Initializer())
                 .build();
 
@@ -534,7 +536,7 @@ public class V4Install extends AbstractInitializableComponent {
 
         log.info("Creating Metadata to {}", metadataFile);
         log.debug("Parameters {}", parameters);
-        metadataGenerator.setOutput(metadataFile.toFile());
+        metadataGenerator.setOutput(metadataFile);
         metadataGenerator.setParameters(parameters);
         try {
             metadataGenerator.initialize();
@@ -662,7 +664,7 @@ public class V4Install extends AbstractInitializableComponent {
               generator.setPrivateKeyFile(key.toFile());
               generator.setKeySize(installerProps.getKeySize());
               generator.setHostName(installerProps.getHostName());
-              generator.setURISubjectAltNames(Collections.singletonList(installerProps.getSubjectAltName()));
+              generator.setURISubjectAltNames(CollectionSupport.singletonList(installerProps.getSubjectAltName()));
               log.info("Creating {}, CN = {} URI = {}, keySize={}", fileBase,
                       installerProps.getHostName(), installerProps.getSubjectAltName(), installerProps.getKeySize());
               try {
@@ -704,7 +706,7 @@ public class V4Install extends AbstractInitializableComponent {
                 generator.setKeystoreFile(keyStore.toFile());
                 generator.setKeySize(installerProps.getKeySize());
                 generator.setHostName(installerProps.getHostName());
-                generator.setURISubjectAltNames(Collections.singletonList(installerProps.getSubjectAltName()));
+                generator.setURISubjectAltNames(CollectionSupport.singletonList(installerProps.getSubjectAltName()));
                 generator.setKeystorePassword(installerProps.getKeyStorePassword());
                 log.info("Creating backchannel keystore, CN = {} URI = {}, keySize={}",
                         installerProps.getHostName(), installerProps.getSubjectAltName(), installerProps.getKeySize());
@@ -725,26 +727,26 @@ public class V4Install extends AbstractInitializableComponent {
          */
         private void generateSealer() {
             final Path credentials = installerProps.getTargetDir().resolve("credentials");
-            final Path sealer = credentials.resolve("sealer.jks");
-            final Path versionFile = credentials.resolve("sealer.kver");
-
-            if (Files.exists(sealer)  && Files.exists(versionFile)) {
+            final File sealerFile = credentials.resolve("sealer.jks").toFile();
+            final File versionFile = credentials.resolve("sealer.kver").toFile();
+            assert sealerFile!=null && versionFile!=null;
+            if (sealerFile.exists()  && versionFile.exists()) {
                 if (!currentState.isIdPPropertiesPresent()) {
                     log.error("Cookie encryption files {} and {} exist, but idp.properties does not",
-                            sealer, versionFile);
+                            sealerFile, versionFile);
                     throw new BuildException("Invalid Cookie encryption  file configuration");
                 }
-                log.debug("Cookie encryption files {} and {} exists.  Not generating.", sealer, versionFile);
+                log.debug("Cookie encryption files {} and {} exists.  Not generating.", sealerFile, versionFile);
             } else if (currentState.isIdPPropertiesPresent()) {
-                log.error("idp.properties exists but cookie encryption files {} do not", sealer, versionFile);
+                log.error("idp.properties exists but cookie encryption files {} do not", sealerFile, versionFile);
                 throw new BuildException("Invalid key file configuration");
-            } else if (Files.exists(sealer) || Files.exists(versionFile)) {
-                log.error("One of two expected cookie encryption file {} and {} exist", sealer, versionFile);
+            } else if (sealerFile.exists() || versionFile.exists()) {
+                log.error("One of two expected cookie encryption file {} and {} exist", sealerFile, versionFile);
                 throw new BuildException("Invalid cookie encryption file configuration");
             } else {
                 final BasicKeystoreKeyStrategyTool generator = new BasicKeystoreKeyStrategyTool();
-                generator.setKeystoreFile(sealer.toFile());
-                generator.setVersionFile(versionFile.toFile());
+                generator.setKeystoreFile(sealerFile);
+                generator.setVersionFile(versionFile);
                 generator.setKeyAlias(installerProps.getSealerAlias());
                 generator.setKeystorePassword(installerProps.getSealerPassword());
                 log.info("Creating Sealer KeyStore");
@@ -799,16 +801,20 @@ public class V4Install extends AbstractInitializableComponent {
         /** {@inheritDoc} */
         @Override @Nonnull public String selectSearchLocation(
                 @Nonnull final ConfigurableApplicationContext applicationContext) {
-            return installerProps.getTargetDir().toString();
+            final String result = installerProps.getTargetDir().toString();
+            assert result != null;
+            return result;
         }
 
         /** {@inheritDoc} */
         @Override @Nonnull public String getSearchLocation() {
-            return installerProps.getTargetDir().toString();
+            final String result = installerProps.getTargetDir().toString();
+            assert result != null;
+            return result;
         }
 
         /** {@inheritDoc} */
-        public void initialize(final ConfigurableApplicationContext applicationContext) {
+        public void initialize(@Nonnull final ConfigurableApplicationContext applicationContext) {
             final Properties props = new Properties(2);
             props.setProperty("idp.backchannel.cert",
                     installerProps.getTargetDir().resolve("credentials").resolve("idp-backchannel.crt").toString());
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/MetadataGeneratorTask.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/MetadataGeneratorTask.java
index 3ec55190a..f24a5fd5a 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/MetadataGeneratorTask.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/MetadataGeneratorTask.java
@@ -18,7 +18,6 @@
 package net.shibboleth.idp.installer.ant.impl;
 
 import java.io.File;
-import java.util.Collections;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -34,6 +33,7 @@ import org.springframework.core.io.Resource;
 import net.shibboleth.idp.installer.metadata.impl.MetadataGeneratorImpl;
 import net.shibboleth.idp.installer.metadata.impl.MetadataGeneratorParametersImpl;
 import net.shibboleth.idp.spring.IdPPropertiesApplicationContextInitializer;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.spring.util.ApplicationContextBuilder;
 
 /**
@@ -86,8 +86,7 @@ public class MetadataGeneratorTask extends Task {
      * 
      * @param file what to set.
      */
-    public void setOutput(final File file) {
-
+    public void setOutput(@Nonnull final File file) {
         outputFile = file;
     }
 
@@ -147,6 +146,16 @@ public class MetadataGeneratorTask extends Task {
 
     /** {@inheritDoc} */
     @Override public void execute() {
+        final File file = outputFile;
+        if (file == null) {
+            log("Build Failed - output file not provided", Project.MSG_ERR);
+            throw new BuildException("Build Failed - output file not provided");
+        }
+        final String dns = dnsName;
+        if (dns == null) {
+            log("Build Failed - DNS Name not provided", Project.MSG_ERR);
+            throw new BuildException("DNS Name - output file not provided");
+        }
         try {
             final MetadataGeneratorParametersImpl parameters;
 
@@ -154,21 +163,21 @@ public class MetadataGeneratorTask extends Task {
 
             final GenericApplicationContext context = new ApplicationContextBuilder()
                     .setName(MetadataGeneratorTask.class.getName())
-                    .setServiceConfigurations(Collections.singletonList(resource))
+                    .setServiceConfigurations(CollectionSupport.singletonList(resource))
                     .setContextInitializer(new Initializer())
                     .build();
             
             parameters = context.getBean("IdPConfiguration", MetadataGeneratorParametersImpl.class);
 
             parameters.setBackchannelCert(backchannelCert);
-            parameters.setDnsName(dnsName);
+            parameters.setDnsName(dns);
             parameters.initialize();
 
             final MetadataGeneratorImpl generator = new MetadataGeneratorImpl();
             generator.setSAML2AttributeQueryCommented(saml2AttributeQueryCommented);
             generator.setSAML2LogoutCommented(saml2LogoutCommented);
             generator.setParameters(parameters);
-            generator.setOutput(outputFile);
+            generator.setOutput(file);
             generator.initialize();
             generator.generate();
 
@@ -189,18 +198,20 @@ public class MetadataGeneratorTask extends Task {
         /** {@inheritDoc} */
         @Override @Nonnull public String selectSearchLocation(
                 @Nonnull final ConfigurableApplicationContext applicationContext) {
-            if (null == idpHome) {
+            final String result = idpHome;
+            if (null == result) {
                 return super.selectSearchLocation(applicationContext);
             }
-            return idpHome;
+            return result;
         }
 
         /** {@inheritDoc} */
         @Override @Nonnull public String getSearchLocation() {
-            if (null == idpHome) {
+            final String result = idpHome;
+            if (null == result) {
                 return super.getSearchLocation();
             }
-            return idpHome;
+            return result;
         }
 
     }
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/PasswordHandler.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/PasswordHandler.java
index f0ab1b4da..d271817b1 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/PasswordHandler.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/PasswordHandler.java
@@ -77,6 +77,7 @@ public class PasswordHandler extends SecureInputHandler {
                 continue;
             }
             final String firstPass = String.copyValueOf(result);
+            assert firstPass != null;
             if (!passwordSavesOK(firstPass)) {
                 System.console().printf("Password contains unsafe characters\n");
                 continue;
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/V4InstallTask.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/V4InstallTask.java
index c82d5ea9c..ce3dfa511 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/V4InstallTask.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/ant/impl/V4InstallTask.java
@@ -25,7 +25,6 @@ import javax.annotation.Nonnull;
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Task;
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.installer.BuildWar;
 import net.shibboleth.idp.installer.CopyDistribution;
@@ -37,6 +36,7 @@ import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * A thin veneer around the V4 installer.
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 defc4d9de..46af3f8f0 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
@@ -47,7 +47,6 @@ import javax.annotation.Nullable;
 
 import org.apache.tools.ant.BuildException;
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.installer.CurrentInstallState;
 import net.shibboleth.idp.installer.InstallerProperties;
@@ -58,6 +57,7 @@ import net.shibboleth.idp.spring.IdPPropertiesApplicationContextInitializer;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /** Tells the installers about the current install state. */
 public final class CurrentInstallStateImpl extends AbstractInitializableComponent implements CurrentInstallState {
@@ -66,7 +66,7 @@ public final class CurrentInstallStateImpl extends AbstractInitializableComponen
     @Nonnull private final Logger log = InstallationLogger.getLogger(CurrentInstallStateImpl.class);
 
     /** Where we are installing to. */
-    private final Path targetDir;
+    @Nonnull private final Path targetDir;
     
     /** The files we will delete if they created on upgrade. */
     private final String[][] deleteAfterUpgrades = { { "credentials", "secrets.properties", }, };
@@ -145,17 +145,18 @@ public final class CurrentInstallStateImpl extends AbstractInitializableComponen
         if (!isIdPPropertiesPresent()) {
             return ;
         }
-        props = new Properties();
+        final Properties localProps = props = new Properties();
         try {
             final File idpPropsFile = targetDir.resolve("conf").resolve("idp.properties").toFile();
             final InputStream idpPropsStream = new FileInputStream(idpPropsFile);
-            props.load(idpPropsStream);
+            localProps .load(idpPropsStream);
         } catch (final IOException e) {
             log.error("Error loading idp.properties", e);
             return;
         }
-        final Collection<String> additionalSources = IdPPropertiesApplicationContextInitializer.getAdditionalSources(
-                targetDir.toString(), props);
+        final String targetDirString = targetDir.toString();
+        assert targetDirString!=null;
+        final Collection<String> additionalSources = IdPPropertiesApplicationContextInitializer.getAdditionalSources(targetDirString, localProps);
         for (final String source : additionalSources) {
             final Path path = Path.of(source);
             if (Files.exists(path)) {
@@ -249,7 +250,8 @@ public final class CurrentInstallStateImpl extends AbstractInitializableComponen
     }
 
     /** {@inheritDoc} */
-    public List<Path> getPathsToBeDeleted() {
+    public @Nonnull List<Path> getPathsToBeDeleted() {
+        assert pathsToDelete != null;
         return pathsToDelete;
     }
 
@@ -260,6 +262,7 @@ public final class CurrentInstallStateImpl extends AbstractInitializableComponen
 
     /** {@inheritDoc} */
     @Nonnull public Collection<String> getEnabledModules() {
+        assert enabledModules != null;
         return enabledModules;
     }
 
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/InstallationLogger.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/InstallationLogger.java
index b92c448ff..d07f594dd 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/InstallationLogger.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/InstallationLogger.java
@@ -23,9 +23,9 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.slf4j.Marker;
 import org.slf4j.event.Level;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * Shimmed logger.
@@ -236,6 +236,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void info(final String format, final Object arg) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.info(format,arg);
         }
@@ -281,6 +282,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void info(final Marker marker, final String format, final Object arg) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.info(marker, format, arg);
         }
@@ -328,6 +330,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void warn(final String format, final Object arg) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.warn(format, arg);
         }
@@ -376,6 +379,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void warn(final Marker marker, final String format, final Object arg) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.warn(marker, format, arg);
         }
@@ -384,6 +388,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void warn(final Marker marker, final String format, final Object arg1, final Object arg2) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.warn(marker, format, arg1, arg2);
         }
@@ -424,6 +429,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void error(final String format, final Object arg) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.error(format, arg);
         }
@@ -474,6 +480,7 @@ public final class InstallationLogger implements Logger {
 
     /** {@inheritDoc} */
     public void error(final Marker marker, final String format, final Object arg) {
+        assert(format != null);
         if (encapsulated.isDebugEnabled()) {
             encapsulated.error(marker, format, arg);
         } else {
@@ -509,8 +516,8 @@ public final class InstallationLogger implements Logger {
      * @param clazz what to log
      * @return a logger
      */
-    public static Logger getLogger(final Class<?> clazz) {
+    @Nonnull public static Logger getLogger(@Nonnull final Class<?> clazz) {
         return new InstallationLogger(LoggerFactory.getLogger(clazz));
     }
 
-}
\ No newline at end of file
+}
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorImpl.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorImpl.java
index 9f1861586..f8ec94c96 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorImpl.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorImpl.java
@@ -60,6 +60,7 @@ import net.shibboleth.idp.installer.MetadataGenerator;
 import net.shibboleth.idp.installer.MetadataGeneratorParameters;
 import net.shibboleth.idp.saml.xmlobject.ExtensionsConstants;
 import net.shibboleth.idp.saml.xmlobject.Scope;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
@@ -158,7 +159,7 @@ public class MetadataGeneratorImpl extends AbstractInitializableComponent implem
     /**
      * Where to write to - as {@link BufferedWriter}.
      */
-    @Nonnull private BufferedWriter writer;
+    @NonnullAfterInit private BufferedWriter writer;
 
     /**
      * Where to write to - as {@link File}.
@@ -599,16 +600,18 @@ public class MetadataGeneratorImpl extends AbstractInitializableComponent implem
      */
     protected void writeKeyDescriptors() throws IOException {
         final List<List<String>> signing = new ArrayList<>(2);
-        if (params.getBackchannelCert() != null && !params.getBackchannelCert().isEmpty()) {
+        final List<String> backchannelCert = params.getBackchannelCert();
+        if (backchannelCert != null && !backchannelCert.isEmpty()) {
             writer.write("        ");
             openComment();
             writer.write(" First signing certificate is BackChannel, the Second is FrontChannel");
             closeComment();
             writer.newLine();
-            signing.add(params.getBackchannelCert());
+            signing.add(backchannelCert);
         }
-        if (params.getSigningCert() != null && !params.getSigningCert().isEmpty()) {
-            signing.add(params.getSigningCert());
+        final List<String> signingCert = params.getSigningCert();
+        if (signingCert!= null && !signingCert.isEmpty()) {
+            signing.add(signingCert);
         }
         writeKeyDescriptors(signing, "signing");
         writeKeyDescriptors(Collections.singletonList(params.getEncryptionCert()), "encryption");
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorParametersImpl.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorParametersImpl.java
index 0545c72f1..77308e2ff 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorParametersImpl.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/metadata/impl/MetadataGeneratorParametersImpl.java
@@ -24,11 +24,14 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.springframework.core.io.Resource;
 
 import net.shibboleth.idp.installer.MetadataGeneratorParameters;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
 
@@ -69,10 +72,10 @@ public class MetadataGeneratorParametersImpl extends AbstractInitializableCompon
     private List<String> signingCerts;
 
     /** The entityID. */
-    private String entityID;
+    @NonnullAfterInit private String entityID;
 
     /** The DNS name. */
-    private String dnsName;
+    @NonnullAfterInit private String dnsName;
 
     /** The scope. */
     private String scope;
@@ -86,6 +89,12 @@ public class MetadataGeneratorParametersImpl extends AbstractInitializableCompon
         } catch (final IOException e) {
             throw new ComponentInitializationException(e);
         }
+        if (entityID == null || entityID.isEmpty()) {
+            throw new ComponentInitializationException("Entity ID not specified");
+        }
+        if (dnsName == null || dnsName.isEmpty()) {
+            throw new ComponentInitializationException("DNS name not specified");
+        }
     }
 
     /**  {@inheritDoc} */
@@ -188,7 +197,8 @@ public class MetadataGeneratorParametersImpl extends AbstractInitializableCompon
     }
 
     /**  {@inheritDoc} */
-    public String getEntityID() {
+    @Nonnull @NotEmpty public String getEntityID() {
+        assert entityID != null;
         return entityID;
     }
 
@@ -197,12 +207,13 @@ public class MetadataGeneratorParametersImpl extends AbstractInitializableCompon
      *
      * @param id what to set.
      */
-    public void setEntityID(final String id) {
+    public void setEntityID(@Nonnull final String id) {
         entityID = id;
     }
 
     /**  {@inheritDoc} */
-    public String getDnsName() {
+    @Nonnull @NotEmpty public String getDnsName() {
+        assert dnsName != null;
         return dnsName;
     }
 
@@ -211,7 +222,7 @@ public class MetadataGeneratorParametersImpl extends AbstractInitializableCompon
      *
      * @param name what to set.
      */
-    public void setDnsName(final String name) {
+    public void setDnsName(@Nonnull final String name) {
         dnsName = name;
     }
 
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java
index 98e472b07..f5c01ccda 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/LoggingVisitor.java
@@ -34,7 +34,8 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * A @{link {@link FileVisitor} copies directory trees keeping a note of all copied target files.
@@ -91,4 +92,4 @@ public final class LoggingVisitor extends SimpleFileVisitor<Path> {
     public List<Path> getCopiedList() {
         return copiedFiles;
     }
-}
\ No newline at end of file
+}
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInfo.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInfo.java
index 9b51bb74d..28736e8fa 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInfo.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInfo.java
@@ -68,7 +68,7 @@ public class PluginInfo {
      * @param id the id we care about
      * @param props all the properties.
      */
-    public PluginInfo(final String id, final Properties props) {
+    public PluginInfo(final String id, @Nonnull final Properties props) {
         pluginId = Constraint.isNotNull(StringSupport.trimOrNull(id), "pluginID must be non-null");
         parse(props);
     }
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index 030be2109..7004af228 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -65,8 +65,6 @@ import org.apache.tools.ant.BuildException;
 import org.opensaml.security.httpclient.HttpClientSecurityParameters;
 import org.slf4j.Logger;
 
-import com.google.common.base.Predicates;
-
 import net.shibboleth.idp.Version;
 import net.shibboleth.idp.installer.BuildWar;
 import net.shibboleth.idp.installer.InstallerSupport;
@@ -82,6 +80,7 @@ import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.idp.plugin.PluginVersion;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.httpclient.HttpClientBuilder;
@@ -89,6 +88,7 @@ import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.resource.Resource;
 import net.shibboleth.shared.spring.httpclient.resource.HTTPResource;
+import net.shibboleth.shared.logic.PredicateSupport;
 /**
  *  The class where the heavy lifting of managing a plugin happens. 
  */
@@ -99,7 +99,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     private static final Logger LOG = InstallationLogger.getLogger(PluginInstaller.class);
 
     /** Property Name for version. */
-    private  static final String PLUGIN_VERSION_PROPERTY ="idp.plugin.version";
+    private static final String PLUGIN_VERSION_PROPERTY ="idp.plugin.version";
 
     /** Property Prefix for install files . */
     private static final String PLUGIN_FILE_PROPERTY_PREFIX = "idp.plugin.file.";
@@ -123,7 +123,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     private IdPPlugin description;
 
     /** The callback before we install a key into the TrustStore. */
-    @Nonnull private Predicate<String> acceptKey = Predicates.alwaysFalse();
+    @Nonnull private Predicate<String> acceptKey = PredicateSupport.alwaysFalse();
 
     /** The actual distribution. */
     private Path distribution;
@@ -135,7 +135,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     private HttpClient httpClient;
 
     /** If overridden these are the urls to us for update (rather than what the plugin asks for. */
-    @Nonnull private List<URL> updateOverrideURLs = Collections.emptyList();
+    @Nonnull private List<URL> updateOverrideURLs = CollectionSupport.emptyList();
 
     /** Dumping space for renamed files. */
     @NonnullAfterInit private Path workspacePath;
@@ -249,6 +249,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
                               @Nonnull @NotEmpty final String fileName,
                               final boolean checkVersion) throws BuildException {
         download(baseURL, fileName);
+        assert downloadDirectory != null;
         installPlugin(downloadDirectory, fileName, checkVersion);
     }
 
@@ -277,15 +278,18 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         setupPluginId();
         checkSignature(base, fileName);
         setupDescriptionFromDistribution();
+        //
+        // the above line guarantees a non null description
+        //
         if (checkVersion) {
-            final PluginState state = new PluginState(description, updateOverrideURLs);
+            final PluginState state = new PluginState(getDescription(), updateOverrideURLs);
             state.setHttpClient(httpClient);
             try {
                 state.initialize();
             } catch (final ComponentInitializationException e) {
                throw new BuildException(e);
             }
-            final PluginVersion pluginVersion = new PluginVersion(description);
+            final PluginVersion pluginVersion = new PluginVersion(getDescription());
             final PluginVersion idpVersion = getIdPVersion();
             if (!state.getPluginInfo().isSupportedWithIdPVersion(pluginVersion, idpVersion)) {
                 LOG.error("Plugin {} version {} is not supported with IdP Version {}",
@@ -294,10 +298,10 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
             }
         }
         LOG.info("Installing Plugin {} version {}.{}.{}", pluginId,
-                description.getMajorVersion(),description.getMinorVersion(), description.getPatchVersion());
+                getDescription().getMajorVersion(),getDescription().getMinorVersion(), getDescription().getPatchVersion());
 
         final Set<String> loadedModules = getLoadedModules();
-        try (final RollbackPluginInstall rollBack = new RollbackPluginInstall(moduleContext, moduleChanges)) {
+        try (final RollbackPluginInstall rollBack = new RollbackPluginInstall(getModuleContext(), moduleChanges)) {
             uninstallOld(rollBack);
 
             checkRequiredModules(loadedModules);
@@ -331,10 +335,10 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         if (description == null) {
             LOG.warn("Description for {} not found", pluginId);
         } else {
-            try (final RollbackPluginInstall rollback = new RollbackPluginInstall(moduleContext, moduleChanges)){
-                for (final IdPModule module: description.getDisableOnRemoval()) {
+            try (final RollbackPluginInstall rollback = new RollbackPluginInstall(getModuleContext(), moduleChanges)){
+                for (final IdPModule module: getDescription().getDisableOnRemoval()) {
                     moduleId = module.getId();
-                    captureChanges(module.disable(moduleContext, false));
+                    captureChanges(module.disable(getModuleContext(), false));
                     rollback.getModulesDisabled().add(module);
                 }
                 rollback.completed();
@@ -407,6 +411,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      */
     @Nonnull public List<Path> getInstalledContents() {
         loadCopiedFiles();
+        assert installedContents != null;
         return installedContents;
     }
 
@@ -417,6 +422,25 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         loadCopiedFiles();
         return installedVersionFromContents;
     }
+    
+    /** Check for initialized and if so return the {@link #moduleContext}.
+     * @return the {@link #moduleContext}.
+     */
+    @Nonnull private ModuleContext getModuleContext() {
+        checkComponentActive();
+        assert moduleContext!=null;
+        return moduleContext;
+    }
+    
+    /** Check for non null and then if so return the {@link #description}.
+     * @return the {@link #description}
+     */
+    @Nonnull private IdPPlugin getDescription() {
+        Constraint.isTrue(description != null, "Invalid Plugin Id in Description");
+        assert description!=null;
+        return description;
+    }
+
 
     /** What modules (on the installed plugins Classpath) are currently loaded?
      * @return a set of the names of the currently enabled Modules.
@@ -428,7 +452,8 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         while (modules.hasNext()) {
             try {
                 final IdPModule module = modules.next();
-                if (module.isEnabled(moduleContext)) {
+                assert moduleChanges != null;
+                if (module.isEnabled(getModuleContext())) {
                     enablededModules.add(module.getId());
                 }
             } catch (final ServiceConfigurationError e) {
@@ -445,7 +470,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      * @throws BuildException if any required modules are missing or disabled
      */
     private void checkRequiredModules(final Set<String> loadedModules) throws BuildException  {
-        for (final String moduleId: description.getRequiredModules()) {
+        for (final String moduleId: getDescription().getRequiredModules()) {
             if (!loadedModules.contains(moduleId)) {
                 LOG.warn("Required module {} is missing or not enabled ", moduleId);
                 throw new BuildException("One or more required modules are not enabled");
@@ -464,7 +489,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
                 final IdPModule module = modules.next();
                 if (pluginId.equals(module.getOwnerId()) && loadedModules.contains(module.getId())) {
                     LOG.debug("Re-enabling module {}", module.getId());
-                    captureChanges(module.enable(moduleContext));
+                    captureChanges(module.enable(getModuleContext()));
                 } else {
                     LOG.debug("Not re-enabling module {}, not provided by this plugin", module.getId());
                 }
@@ -489,10 +514,10 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
 
         String moduleId = null;
         try {
-            for (final IdPModule module: description.getEnableOnInstall()) {
+            for (final IdPModule module: getDescription().getEnableOnInstall()) {
                 moduleId = module.getId();
-                if (!module.isEnabled(moduleContext)) {
-                    captureChanges(module.enable(moduleContext));
+                if (!module.isEnabled(getModuleContext())) {
+                    captureChanges(module.enable(getModuleContext()));
                     rollBack.getModulesEnabled().add(module);
                 }
             }
@@ -532,7 +557,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         try {
             Files.createDirectories(pluginsContents);
             final Properties props = new Properties(1+copiedFiles.size());
-            props.setProperty(PLUGIN_VERSION_PROPERTY, new PluginVersion(description).toString());
+            props.setProperty(PLUGIN_VERSION_PROPERTY, new PluginVersion(getDescription()).toString());
             props.setProperty(PLUGIN_RELATIVE_PATHS_PROPERTY, "true");
             int count = 1;
             for (final Path p: copiedFiles) {
@@ -607,7 +632,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
             throw new BuildException(e);
         }
         LOG.debug("Property file {}", props);
-        installedContents = new ArrayList<>(props.size());
+        final List<Path> result = new ArrayList<>(props.size());
         installedVersionFromContents = StringSupport.trimOrNull(props.getProperty(PLUGIN_VERSION_PROPERTY));
         final boolean relativePaths = props.get(PLUGIN_RELATIVE_PATHS_PROPERTY) != null;
         final Path installedIdPHome;
@@ -621,14 +646,15 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         while (val != null) {
             final Path valAsPath = Path.of(val);
             if (relativePaths || installedIdPHome == null) {
-                installedContents.add(idpHome.resolve(valAsPath));
+                result.add(idpHome.resolve(valAsPath));
             } else {
                 final Path relPath = installedIdPHome.relativize(valAsPath);
                 final Path newPath = idpHome.resolve(relPath);
-                installedContents.add(newPath);
+                result.add(newPath);
             }
             val = props.getProperty(PLUGIN_FILE_PROPERTY_PREFIX+Integer.toString(count++));
         }
+        installedContents = result;
     }
 
     /** Method to download a zip file to the {{@link #downloadDirectory}.
@@ -636,7 +662,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      * @param fileName the name.
      * @throws BuildException if badness is detected.
      */
-    private void download(final URL baseURL, final String fileName) throws BuildException {
+    private void download(@Nonnull final URL baseURL, @Nonnull final String fileName) throws BuildException {
         buildHttpClient();
         try {
             downloadDirectory = Files.createTempDirectory("plugin-installer-download");
@@ -656,7 +682,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
             LOG.debug("No HttpClient built, creating default");
             try {
                 httpClient = new HttpClientBuilder().buildClient();
-                moduleContext.setHttpClient(httpClient);
+                getModuleContext().setHttpClient(httpClient);
             } catch (final Exception e) {
                 LOG.error("Could not create HttpClient", e);
                 throw new BuildException(e);
@@ -722,7 +748,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      * @param fileName the file name
      * @throws IOException as required
      */
-    private void download(final Resource baseResource, final String fileName) throws IOException {
+    private void download(final Resource baseResource, @Nonnull final String fileName) throws IOException {
         final Resource fileResource = baseResource.createRelativeResource(fileName);
         final Path filePath = downloadDirectory.resolve(fileName);
         LOG.info("Downloading from {}", fileResource.getDescription());
@@ -902,11 +928,13 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
             throw new ComponentInitializationException("idp.home property must be set");
         }
         try {
+            assert idpHome != null;
             idpHome = PluginInstallerSupport.canonicalPath(idpHome);
         } catch (final IOException e) {
             LOG.error("Could not canonicalize idp home", e);
             throw new ComponentInitializationException(e);
         }
+        assert idpHome != null;
         moduleContext = new ModuleContext(idpHome);
         moduleContext.setHttpClientSecurityParameters(securityParams);
         moduleContext.setHttpClient(httpClient);
@@ -1009,11 +1037,11 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
      * @param name what to find
      * @return the {@link IdPPlugin} or null if not found.
      */
-    @Nullable public IdPPlugin getInstalledPlugin(@Nonnull final String name) {
+    @Nullable public IdPPlugin getInstalledPlugin(final String name) {
         Constraint.isNotNull(name, "Plugin Name must not be null");
         final List<IdPPlugin> plugins = getInstalledPlugins();
         for (final IdPPlugin plugin: plugins) {
-            if (name.equals(plugin.getPluginId())) {
+            if (plugin.getPluginId().equals(name)) {
                 return plugin;
             }
         }
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerArguments.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerArguments.java
index 6e59df9f6..a84dbc436 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerArguments.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerArguments.java
@@ -34,6 +34,7 @@ import com.beust.jcommander.Parameter;
 import net.shibboleth.idp.cli.AbstractIdPHomeAwareCommandLineArguments;
 import net.shibboleth.idp.installer.impl.InstallationLogger;
 import net.shibboleth.idp.plugin.PluginVersion;
+import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 
 /**
@@ -144,10 +145,11 @@ public class PluginInstallerArguments extends AbstractIdPHomeAwareCommandLineArg
     @Nonnull private OperationType operation = OperationType.UNKNOWN;
 
     /** {@inheritDoc} */
-    public Logger getLog() {
+    public @Nonnull Logger getLog() {
         if (log == null) {
             log = InstallationLogger.getLogger(PluginInstallerArguments.class);
         }
+        assert log != null;
         return log;
     }
 
@@ -172,8 +174,12 @@ public class PluginInstallerArguments extends AbstractIdPHomeAwareCommandLineArg
      *
      * Only valid for {@link OperationType#INSTALLREMOTE}.
      */
-    public URL getInputURL() {
-        return inputURL;
+    @Nonnull public URL getInputURL() {
+        final URL result = inputURL;
+        Constraint.isTrue(operation == OperationType.INSTALLREMOTE, "Can only call getInputURL on remote operations");
+        Constraint.isTrue(result != null, "Invalid Remote URL");
+        assert result != null;
+        return result;
     }
 
     /** Get the file Name.
@@ -183,8 +189,13 @@ public class PluginInstallerArguments extends AbstractIdPHomeAwareCommandLineArg
      *
      * @return Returns the digested file Name.
      */
-    public String getInputFileName() {
-        return inputName;
+    @Nonnull public String getInputFileName() {
+        final String result = inputName;
+        Constraint.isTrue(operation == OperationType.INSTALLREMOTE || operation == OperationType.INSTALLDIR, 
+                "Can only call getInputFileName on remote or local installs");
+        Constraint.isTrue(result != null, "Invalid InputFileName");
+        assert result != null;
+        return result;
     }
 
     /** Get the digested input directory.
@@ -193,8 +204,13 @@ public class PluginInstallerArguments extends AbstractIdPHomeAwareCommandLineArg
      *
      * @return Returns the digested input directory.
      */
-    public Path getInputDirectory() {
-        return inputDirectory;
+    @Nonnull public Path getInputDirectory() {
+        final Path  result = inputDirectory;
+        Constraint.isTrue(operation == OperationType.INSTALLDIR, 
+                "Can only call getInputDirectory on local installs");
+        Constraint.isTrue(result != null, "Invalid InputDirectory");
+        assert result != null;
+        return result;
     }
 
     /** Are we doing a full List?
@@ -260,7 +276,7 @@ public class PluginInstallerArguments extends AbstractIdPHomeAwareCommandLineArg
      * Get operation to perform.
      * @return operation
      */
-    @Nullable public OperationType getOperation() {
+    @Nonnull public OperationType getOperation() {
         return operation;
     }
 
@@ -341,13 +357,15 @@ public class PluginInstallerArguments extends AbstractIdPHomeAwareCommandLineArg
     /** Given an input string, work out what the parts are.
      * @return Whether this is a remote install or a local one.
      */
-    private OperationType decodeInput() {
+    @Nonnull private OperationType decodeInput() {
         try {
-            final URL inputAsURL = new URL(input);
+            final String urlInput = input;
+            assert urlInput != null;
+            final URL inputAsURL = new URL(urlInput);
             if ("https".equals(inputAsURL.getProtocol()) || "http".equals(inputAsURL.getProtocol())) {
-                final int i = input.lastIndexOf('/')+1;
-                inputURL = new URL(input.substring(0, i));
-                inputName = input.substring(i);
+                final int i = urlInput.lastIndexOf('/')+1;
+                inputURL = new URL(urlInput.substring(0, i));
+                inputName = urlInput.substring(i);
                 getLog().trace("Found URL: {}\t{}", inputDirectory, inputName);
                 return OperationType.INSTALLREMOTE;
             }
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java
index 3230a1dd6..7d857a403 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerCLI.java
@@ -38,6 +38,7 @@ import java.util.function.Predicate;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.http.client.HttpClient;
 import org.apache.tools.ant.BuildException;
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.slf4j.Logger;
@@ -54,6 +55,7 @@ import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.idp.plugin.PluginSupport.SupportLevel;
 import net.shibboleth.shared.annotation.constraint.NonnullElements;
 import net.shibboleth.shared.cli.AbstractCommandLine;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
@@ -66,7 +68,7 @@ import net.shibboleth.idp.plugin.PluginVersion;
 public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<PluginInstallerArguments> {
 
     /** Class logger. */
-    @Nullable private Logger log;
+    @Nonnull final private Logger log = InstallationLogger.getLogger(PluginInstallerCLI.class);
 
     /** A Plugin Installer to use. */
     @Nullable private PluginInstaller installer;
@@ -84,9 +86,6 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
     /** {@inheritDoc} */
     @Override
     @Nonnull protected Logger getLogger() {
-        if (log == null) {
-            log = InstallationLogger.getLogger(PluginInstallerCLI.class);
-        }
         return log;
     }
 
@@ -98,19 +97,21 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
 
     /** {@inheritDoc} */
     @Override
-    @Nullable protected String getVersion() {
-        return Version.getVersion();
+    @Nonnull protected String getVersion() {
+        final String result = Version.getVersion();
+        assert result != null;
+        return result;
     }
     
     /** {@inheritDoc} */
     @Nonnull @NonnullElements protected List<Resource> getAdditionalSpringResources() {
-        return List.of(
+        return CollectionSupport.singletonList(
                new ClassPathResource("net/shibboleth/idp/conf/http-client.xml"));
     }
     
     /** {@inheritDoc} */
     //CheckStyle: CyclomaticComplexity|MethodLength OFF
-    protected int doRun(final PluginInstallerArguments args) {
+    protected int doRun(@Nonnull final PluginInstallerArguments args) {
         
         if (args.getHttpClientName() == null) {
             args.setHttpClientName("shibboleth.InternalHttpClient");
@@ -120,6 +121,11 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
         if (ret != RC_OK) {
             return ret;
         }
+        //
+        // Sanity check - we rely on a non null HttpClient (see constructPluginInstaller)
+        //
+        Constraint.isTrue(getHttpClient()!=null, "no HttpClient supplied");
+        
         if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
             Security.addProvider(new BouncyCastleProvider());
         }
@@ -136,6 +142,8 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
 
         try (final PluginInstaller inst = new PluginInstaller()){
             constructPluginInstaller(inst, args);
+            assert inst == installer;
+            final String pluginId = args.getPluginId();
 
             switch (args.getOperation()) {
                 case LIST:
@@ -146,38 +154,43 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
                     break;
 
                 case INSTALLDIR:
-                    if (args.getPluginId() != null) {
-                        installer.setPluginId(args.getPluginId());
+                    if (pluginId != null) {
+                        inst.setPluginId(pluginId);
                     }
-                    installer.installPlugin(args.getInputDirectory(), args.getInputFileName(), !args.isNoCheck());
+                    inst.installPlugin(args.getInputDirectory(), args.getInputFileName(), !args.isNoCheck());
                     break;
 
                 case INSTALLREMOTE:
-                    if (args.getPluginId() != null) {
-                        installer.setPluginId(args.getPluginId());
+                    if (pluginId != null) {
+                        inst.setPluginId(pluginId);
                     }
                     if (args.isInstallId()) {
-                        return autoPluginFromId(args.getPluginId(), !args.isNoCheck());
+                        assert(pluginId != null);
+                        return autoPluginFromId(pluginId, !args.isNoCheck());
                     }
-                    installer.installPlugin(args.getInputURL(), args.getInputFileName(), !args.isNoCheck());
+                    inst.installPlugin(args.getInputURL(), args.getInputFileName(), !args.isNoCheck());
                     break;
 
                 case UPDATE:
-                    doUpdate(args.getPluginId(), args.getUpdateVersion(), !args.isNoCheck());
+                    assert pluginId != null;
+                    doUpdate(pluginId, args.getUpdateVersion(), !args.isNoCheck());
                     break;
 
                 case UNINSTALL:
-                    installer.setPluginId(args.getPluginId());
-                    installer.uninstall();
+                    assert pluginId != null;
+                    inst.setPluginId(pluginId);
+                    inst.uninstall();
                     break;
 
                 case OUTPUTLICENSE:
-                    outputLicense(args.getPluginId());
+                    assert pluginId != null;
+                    outputLicense(pluginId);
                     break;
 
                 case LISTCONTENTS:
-                    installer.setPluginId(args.getPluginId());
-                    doContentList(args.getPluginId());
+                    assert pluginId != null;
+                    inst.setPluginId(pluginId);
+                    doContentList(pluginId);
                     break;
 
                 default:
@@ -203,15 +216,21 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
      */
     private void constructPluginInstaller(final PluginInstaller inst,
             final PluginInstallerArguments args) throws ComponentInitializationException {
-        inst.setIdpHome(Path.of(getApplicationContext().getEnvironment().getProperty("idp.home")));
+        final Path idpHome = Path.of(getApplicationContext().getEnvironment().getProperty("idp.home"));
+        assert idpHome != null;
+        inst.setIdpHome(idpHome);
         if (!args.isUnattended()) {
             inst.setAcceptKey(new InstallerQuery("Accept this key"));
         }
         inst.setTrustore(args.getTruststore());
-        if (getHttpClient()!= null) {
-            inst.setHttpClient(getHttpClient());
-        }
+        final HttpClient client = getHttpClient();
+        //
+        // This is null because we set up the bean name before calling super.dorun
+        //
+        assert client != null;
+        inst.setHttpClient(client);
         inst.setModuleContextSecurityParams(getHttpClientSecurityParameters());
+        assert(updateURLs != null);
         inst.setUpdateOverrideURLs(updateURLs);
         inst.setRebuildWar(args.isRebuild());
         inst.initialize();
@@ -236,8 +255,9 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
     private void printDetails(final IdPPlugin plugin) {
         log.debug("Interrogating {}", plugin.getPluginId());
         final PluginState state =  new PluginState(plugin, updateURLs);
-        if (getHttpClient() != null) {
-            state.setHttpClient(getHttpClient());
+        final HttpClient client = getHttpClient();
+        if (client != null) {
+            state.setHttpClient(client);
         }
         try {
             state.initialize();
@@ -268,6 +288,7 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
      * @param pluginId what to list
      */
     private void outputLicense(@Nonnull final String pluginId) {
+        assert installer != null;
         final IdPPlugin plugin = installer.getInstalledPlugin(pluginId);
         if (plugin == null) {
             log.error("Plugin {} not installed", pluginId);
@@ -302,6 +323,7 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
      */
     private void doList(final boolean fullList, @Nullable final String pluginId) {
         boolean list = false;
+        assert installer != null;
         final List<IdPPlugin> plugins = installer.getInstalledPlugins();
         for (final IdPPlugin plugin: plugins) {
             if (pluginId == null || pluginId.equals(plugin.getPluginId())) {
@@ -327,19 +349,22 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
      * @param pluginId the pluginId
      */
     private void doContentList(@Nonnull final String pluginId) {
-        final IdPPlugin thePlugin = installer.getInstalledPlugin(pluginId);
+        final PluginInstaller inst = installer;
+        assert inst != null;
+
+        final IdPPlugin thePlugin = inst.getInstalledPlugin(pluginId);
 
-        final String fromContentsVersion =  installer.getVersionFromContents();
-        final List<Path> contents = installer.getInstalledContents();
+        final String fromContentsVersion =  inst.getVersionFromContents();
+        final List<Path> contents = inst.getInstalledContents();
         if (thePlugin == null) {
             log.warn("Plugin was not installed {}", pluginId);
             if (fromContentsVersion != null) {
                 log.error("Plugin {} not installed, but contents found", pluginId);
                 log.debug("{}", contents);
-            } else {
-                return;
             }
-        } else if (fromContentsVersion == null) {
+            return;
+        }
+        if (fromContentsVersion == null) {
             log.error("Plugin {} found, but no contents listed", pluginId);
             return;
         }
@@ -382,6 +407,7 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
 
         for (final Entry<String, PluginInfo> e: plugins.entrySet()) {
             final PluginVersion nullVersion = new PluginVersion(0, 0, 0);
+            assert installer != null;
             final IdPPlugin existingPlugin = installer.getInstalledPlugin(e.getKey());
             if (existingPlugin == null) {
                 final PluginVersion version = getBestVersion(nullVersion, e.getValue());
@@ -430,9 +456,13 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
             final Resource propertyResource;
             try {
                 if ("file".equals(url.getProtocol())) {
-                    propertyResource = new FileSystemResource(url.getPath());
+                    final String path =url.getPath();
+                    assert path != null;
+                    propertyResource = new FileSystemResource(path);
                 } else if ("http".equals(url.getProtocol()) || "https".equals(url.getProtocol())) {
-                        propertyResource = new HTTPResource(getHttpClient(), url);
+                    final HttpClient client = getHttpClient();
+                    assert client != null;
+                    propertyResource = new HTTPResource(client , url);
                 } else {
                     log.error("Only file and http[s] URLs are allowed");
                     continue;
@@ -459,13 +489,19 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
      * @param checkVersion are we checking the version.
      * @return installation status
      */
-    private int autoPluginFromId(final String pluginId, final boolean checkVersion) {
-        final IdPPlugin existing = installer.getInstalledPlugin(pluginId);
+    private int autoPluginFromId(@Nonnull final String pluginId, final boolean checkVersion) {
+        final PluginInstaller inst = installer;
+        assert inst != null;
+        final IdPPlugin existing = inst.getInstalledPlugin(pluginId);
         if (existing != null) {
             log.error("Plugin {} is already installed", pluginId);
             return RC_INIT;
         }
         final Properties props = loadPluginInfo();
+        if (props == null) {
+            log.error("AutoInstall not possible");
+            return RC_INIT;
+        }
         final PluginInfo info = new PluginInfo(pluginId, props);
         if (!info.isInfoComplete()) {
             log.error("Plugin {}: Information not found", pluginId);
@@ -476,7 +512,9 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
             log.error("Plugin {}: No version available to install", pluginId);
             return RC_INIT;
         }
-        installer.installPlugin(info.getUpdateURL(versionToInstall),
+        final URL updateURL = info.getUpdateURL(versionToInstall); 
+        assert updateURL != null;
+        inst.installPlugin(updateURL,
                 info.getUpdateBaseName(versionToInstall) + ".tar.gz",
                 checkVersion);
         return RC_OK;
@@ -530,15 +568,19 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
     private void doUpdate(@Nonnull final String pluginId, 
             @Nullable final PluginVersion pluginVersion,
             final boolean checkVersion) {
-        final IdPPlugin plugin = installer.getInstalledPlugin(pluginId);
+        
+        final PluginInstaller inst = installer;
+        assert inst != null;
+        final IdPPlugin plugin = inst.getInstalledPlugin(pluginId);
         if (plugin == null) {
             log.error("Plugin {} was not installed", pluginId);
             return;
         }
         log.debug("Interrogating {} ", plugin.getPluginId());
         final PluginState state =  new PluginState(plugin, updateURLs);
-        if (getHttpClient() != null) {
-            state.setHttpClient(getHttpClient());
+        final HttpClient client = getHttpClient();
+        if (client != null) {
+            state.setHttpClient(client);
         }
         try {
             state.initialize();
@@ -563,7 +605,9 @@ public final class PluginInstallerCLI extends AbstractIdPHomeAwareCommandLine<Pl
             }
         }
         // just use the tgz version - its an update so it should be jar files only
-        installer.installPlugin(state.getPluginInfo().getUpdateURL(installVersion),
+        final URL updateURL = state.getPluginInfo().getUpdateURL(installVersion); 
+        assert updateURL != null;
+        inst.installPlugin(updateURL,
                 state.getPluginInfo().getUpdateBaseName(installVersion) + ".tar.gz",
                 checkVersion);
     }
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java
index 1686026d4..35d3f7b6b 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerSupport.java
@@ -56,7 +56,8 @@ public final class PluginInstallerSupport {
      * @return the canonicalized one
      * @throws IOException  as from {@link File#getCanonicalFile()}
      */
-    static Path canonicalPath(final Path from) throws IOException {
+    @SuppressWarnings("null")
+    @Nonnull static Path canonicalPath(@Nonnull final Path from) throws IOException {
         return from.toFile().getCanonicalFile().toPath();
     }
 
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java
index 6b8f2830f..c97b909c5 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginState.java
@@ -32,13 +32,13 @@ import org.springframework.core.io.Resource;
 import net.shibboleth.idp.installer.impl.InstallationLogger;
 import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.idp.plugin.PluginSupport.SupportLevel;
+import net.shibboleth.idp.plugin.PluginVersion;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.httpclient.HttpClientBuilder;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.spring.httpclient.resource.HTTPResource;
-import net.shibboleth.idp.plugin.PluginVersion;
 
 /**
  * A class which will answer questions about a plugin state as of now
@@ -113,6 +113,7 @@ public class PluginState extends AbstractInitializableComponent {
 
     /** {@inheritDoc} */
     // CheckStyle: CyclomaticComplexity OFF
+    @SuppressWarnings("unused")
     protected void doInitialize() throws ComponentInitializationException {
         
         try {
@@ -133,8 +134,11 @@ public class PluginState extends AbstractInitializableComponent {
                 final Resource propertyResource;
                 try {
                     if ("file".equals(url.getProtocol())) {
-                        propertyResource = new FileSystemResource(url.getPath());
+                        final String path = url.getPath();
+                        assert path != null;
+                        propertyResource = new FileSystemResource(path);
                     } else if ("http".equals(url.getProtocol()) || "https".equals(url.getProtocol())) {
+                            assert(httpClient != null);
                             propertyResource = new HTTPResource(httpClient, url);
                     } else {
                         log.error("Plugin {}: Only file and http[s] URLs are allowed: '{}'", plugin.getPluginId(), url);
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/RollbackPluginInstall.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/RollbackPluginInstall.java
index 2d0f56f54..08aa071d0 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/RollbackPluginInstall.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/RollbackPluginInstall.java
@@ -26,7 +26,6 @@ import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -42,6 +41,7 @@ import net.shibboleth.idp.module.IdPModule.ResourceResult;
 import net.shibboleth.idp.module.ModuleContext;
 import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.shared.annotation.constraint.Live;
+import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.collection.Pair;
 import net.shibboleth.shared.logic.Constraint;
 
@@ -177,10 +177,13 @@ public class RollbackPluginInstall implements AutoCloseable {
         }
         for (int i = filesRenamedAway.size()-1; i >=0; i--) {
             final Pair<Path, Path> filePair = filesRenamedAway.get(i);
+            final Path from = filePair.getFirst();
+            final Path to = filePair.getSecond();
+            assert from != null && to != null;
             try (final InputStream in = new BufferedInputStream(
-                         new FileInputStream(filePair.getSecond().toFile()));
+                         new FileInputStream(to.toFile()));
                  final OutputStream out = new BufferedOutputStream(
-                         new FileOutputStream(filePair.getFirst().toFile()))) {
+                         new FileOutputStream(from.toFile()))) {
                 log.trace("Copying {} to {}", filePair.getSecond());
                 in.transferTo(out);
             } catch (final Throwable t) {
@@ -218,10 +221,10 @@ public class RollbackPluginInstall implements AutoCloseable {
     
     /** Signal that the operation completed and that rollback won't be needed. */
     public void completed() {
-        modulesEnabled = Collections.emptyList();
-        modulesDisabled = Collections.emptyList();
-        filesCopied = Collections.emptyList();
-        filesRenamedAway = Collections.emptyList();
+        modulesEnabled = CollectionSupport.emptyList();
+        modulesDisabled = CollectionSupport.emptyList();
+        filesCopied = CollectionSupport.emptyList();
+        filesRenamedAway = CollectionSupport.emptyList();
     }
 
     /** {@inheritDoc} */
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java
index c8cd8718d..f78cf17e5 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java
@@ -51,6 +51,7 @@ import net.shibboleth.idp.installer.impl.InstallationLogger;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.AbstractInitializableComponent;
 import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.logic.Constraint;
 
 /**
  * Code to handle (load, update, check) the trust store for an individual plugin.
@@ -239,7 +240,7 @@ import net.shibboleth.shared.component.ComponentInitializationException;
      * @return the Signature.
      * @throws IOException if there is a problem reading the file of it it doesn't represent a signature
      */
-    public static Signature signatureOf(final InputStream stream) throws IOException {
+    public static Signature signatureOf(@Nonnull final InputStream stream) throws IOException {
         return new Signature(stream);
     }
 
@@ -361,14 +362,19 @@ import net.shibboleth.shared.component.ComponentInitializationException;
             try (final InputStream sigStream =  PGPUtil.getDecoderStream(input)) {
                 final JcaPGPObjectFactory factory = new JcaPGPObjectFactory(sigStream);
                 final Object first = factory.nextObject();
-                if (first instanceof PGPSignatureList) {
+                if (first != null && first instanceof PGPSignatureList) {
                     final PGPSignatureList list = (PGPSignatureList) first;
-                    signature = list.get(0);
+                    if (list.isEmpty()) {
+                        throw new IOException("Provided signature file was empty");
+                    }
+                    signature = Constraint.isNotNull(list.get(0), "PGPSignatureList#get(0) retiurned null for non empty list");
                 } else {
                     throw new IOException("Provided file was not a signature");
                 }
             }
-            keyId = String.format("0x%X", signature.getKeyID());
+            final String kid =String.format("0x%X", signature.getKeyID()) ;
+            assert kid != null;
+            keyId = kid;
         }
 
         /**
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/Test.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/Test.java
index 4710c950e..0123f2323 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/Test.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/Test.java
@@ -22,12 +22,11 @@ import java.io.IOException;
 import javax.annotation.Nonnull;
 
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.installer.impl.CurrentInstallStateImpl;
 import net.shibboleth.idp.installer.metadata.impl.MetadataGeneratorImpl;
 import net.shibboleth.shared.component.ComponentInitializationException;
-
+import net.shibboleth.shared.primitive.LoggerFactory;
 /**
  *
  */
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/BasePluginTest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/BasePluginTest.java
index e74af4040..268f20a18 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/BasePluginTest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/BasePluginTest.java
@@ -25,12 +25,12 @@ import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.springframework.core.io.ClassPathResource;
 import org.testng.annotations.AfterSuite;
 import org.testng.annotations.BeforeSuite;
 
 import net.shibboleth.idp.installer.InstallerSupport;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * set up state for testing.
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
index 53cbe4bcd..6a5add107 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
@@ -29,15 +29,17 @@ import java.util.Map;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
+import javax.annotation.Nonnull;
+
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
 import net.shibboleth.idp.plugin.AbstractIdPPlugin;
 import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.primitive.LoggerFactory;
 
 @SuppressWarnings("javadoc")
 public class PluginInstallerTest extends BasePluginTest {
@@ -117,13 +119,13 @@ public class PluginInstallerTest extends BasePluginTest {
     public static class Wibble extends AbstractIdPPlugin {
 
         /** {@inheritDoc} */
-        public String getPluginId() {
+        public @Nonnull String getPluginId() {
             
             return "org.example.Plugin";
         }
 
         /** {@inheritDoc} */
-        public List<URL> getUpdateURLs() throws IOException {
+        public @Nonnull List<URL> getUpdateURLs() throws IOException {
             return Collections.emptyList();
         }
 
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginStateTest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginStateTest.java
index 0c98febcf..4dc8d26b1 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginStateTest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginStateTest.java
@@ -28,6 +28,8 @@ import java.net.URL;
 import java.util.Collections;
 import java.util.List;
 
+import javax.annotation.Nonnull;
+
 import org.testng.annotations.Test;
 
 import net.shibboleth.idp.plugin.IdPPlugin;
@@ -108,7 +110,7 @@ public class PluginStateTest {
 
         final IdPPlugin simple = new TestPlugin() {
             @Override
-            public java.util.List<URL> getUpdateURLs() {
+            public @Nonnull java.util.List<URL> getUpdateURLs() {
                 try {
                     return List.of(new URL("http://example.org/dir"), super.getUpdateURLs().get(0));
                 } catch (final MalformedURLException e) {
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/RollbackTester.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/RollbackTester.java
index 470ec5a83..197c022c6 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/RollbackTester.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/RollbackTester.java
@@ -59,26 +59,28 @@ public class RollbackTester {
     private void test(boolean commit) throws IOException, ModuleException {
         final Path copied = Files.createTempFile(parent, "copied", "file");
         final Path to = Files.createTempFile(parent, "renamed", "file");
+        final Path mc = Files.createTempDirectory(parent, "mod");
         final Path from = parent.resolve("fromFile");
         final Pair<Path, Path> renamed = new Pair<>(from, to);
         final IdPModule enabled1 = new TestModule("enabled1", null, null);
         final IdPModule enabled2 = new TestModule("enabled2", null, new ModuleException()); 
         final IdPModule disabled1 = new TestModule("disabled1", null, null);
-        final IdPModule disabled2 = new TestModule("disablde2", new ModuleException(), null); 
+        final IdPModule disabled2 = new TestModule("disablde2", new ModuleException(), null);
+        final ModuleContext ctx = new ModuleContext(mc);
 
         try {
             assertFalse(from.toFile().exists());
             assertTrue(to.toFile().exists());
             assertTrue(copied.toFile().exists());
             
-            enabled1.enable(null);
-            assertTrue(enabled1.isEnabled(null));
+            enabled1.enable(ctx);
+            assertTrue(enabled1.isEnabled(ctx));
             
-            enabled2.enable(null);
-            assertTrue(enabled2.isEnabled(null));
+            enabled2.enable(ctx);
+            assertTrue(enabled2.isEnabled(ctx));
             
-            assertFalse(disabled1.isEnabled(null));
-            assertFalse(disabled2.isEnabled(null));
+            assertFalse(disabled1.isEnabled(ctx));
+            assertFalse(disabled2.isEnabled(ctx));
             
             try (final RollbackPluginInstall rp = new RollbackPluginInstall(new ModuleContext(parent), new HashMap<>())) {
                 rp.getFilesCopied().add(copied);
@@ -93,18 +95,18 @@ public class RollbackTester {
             }
 
             if (commit) {
-                assertTrue(enabled1.isEnabled(null));
-                assertTrue(enabled2.isEnabled(null));
-                assertFalse(disabled1.isEnabled(null));
-                assertFalse(disabled2.isEnabled(null));
+                assertTrue(enabled1.isEnabled(ctx));
+                assertTrue(enabled2.isEnabled(ctx));
+                assertFalse(disabled1.isEnabled(ctx));
+                assertFalse(disabled2.isEnabled(ctx));
                 assertFalse(from.toFile().exists());
                 assertTrue(to.toFile().exists());
                 assertTrue(copied.toFile().exists());            
             } else {
-                assertFalse(enabled1.isEnabled(null));
-                assertTrue(enabled2.isEnabled(null)); // threw instead
-                assertTrue(disabled1.isEnabled(null));
-                assertFalse(disabled2.isEnabled(null)); // threw instead
+                assertFalse(enabled1.isEnabled(ctx));
+                assertTrue(enabled2.isEnabled(ctx)); // threw instead
+                assertTrue(disabled1.isEnabled(ctx));
+                assertFalse(disabled2.isEnabled(ctx)); // threw instead
                 
                 assertTrue(from.toFile().exists()); //copied to
                 assertTrue(to.toFile().exists()); // copied from
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestModule.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestModule.java
index a11e9db95..ecaf8dc8e 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestModule.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestModule.java
@@ -18,8 +18,10 @@
 package net.shibboleth.idp.installer.plugin.impl;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Map;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import net.shibboleth.idp.module.IdPModule;
@@ -31,10 +33,10 @@ public class TestModule implements IdPModule {
     
     @Nullable final ModuleException throwOnEnable;
     @Nullable final ModuleException throwOnDisable;
-    @Nullable final String id;
+    @Nonnull final String id;
     boolean enabled;
     
-    public TestModule(String name, ModuleException enable, ModuleException disable) {
+    public TestModule(@Nonnull String name, ModuleException enable, ModuleException disable) {
         throwOnEnable = enable;
         throwOnDisable = disable;
         id = name;
@@ -46,12 +48,12 @@ public class TestModule implements IdPModule {
     }
 
     /** {@inheritDoc} */
-    public String getName(ModuleContext moduleContext) {
+    public @Nonnull String getName(@Nullable final ModuleContext moduleContext) {
         return id;
     }
 
     /** {@inheritDoc} */
-    public String getDescription(ModuleContext moduleContext) {
+    public String getDescription(@Nullable  final ModuleContext moduleContext) {
         return null;
     }
 
@@ -71,8 +73,8 @@ public class TestModule implements IdPModule {
     }
 
     /** {@inheritDoc} */
-    public Collection<ModuleResource> getResources() {
-        return null;
+    public @Nonnull Collection<ModuleResource> getResources() {
+        return Collections.emptyList();
     }
 
     /** {@inheritDoc} */
@@ -81,22 +83,22 @@ public class TestModule implements IdPModule {
     }
 
     /** {@inheritDoc} */
-    public Map<ModuleResource, ResourceResult> enable(ModuleContext moduleContext) throws ModuleException {
+    public @Nonnull Map<ModuleResource, ResourceResult> enable(ModuleContext moduleContext) throws ModuleException {
         if (throwOnEnable != null) {
             throw throwOnEnable;
         }
         enabled = true;
-        return null;
+        return Collections.emptyMap();
     }
 
     /** {@inheritDoc} */
-    public Map<ModuleResource, ResourceResult> disable(ModuleContext moduleContext, boolean clean)
+    public @Nonnull Map<ModuleResource, ResourceResult> disable(ModuleContext moduleContext, boolean clean)
             throws ModuleException {
         if (throwOnDisable != null) {
             throw throwOnDisable;
         }
         enabled = false;
-        return null;
+        return Collections.emptyMap();
     }
     
 }
\ No newline at end of file
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestPlugin.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestPlugin.java
index 145407193..4ac211b1c 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestPlugin.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TestPlugin.java
@@ -22,6 +22,8 @@ import java.net.URL;
 import java.util.Collections;
 import java.util.List;
 
+import javax.annotation.Nonnull;
+
 import org.springframework.core.io.ClassPathResource;
 
 import net.shibboleth.idp.plugin.AbstractIdPPlugin;
@@ -33,13 +35,13 @@ public class TestPlugin extends AbstractIdPPlugin {
 
     /** {@inheritDoc} */
     @Override
-    public String getPluginId() {
+    public @Nonnull String getPluginId() {
         return "net.shibboleth.plugin.test";
     }
 
     /** {@inheritDoc} */
     @Override
-    public List<URL> getUpdateURLs() {
+    public @Nonnull List<URL> getUpdateURLs() {
         ClassPathResource resource = new ClassPathResource("/net/shibboleth/idp/plugin/plugins.props");
         try {
             return Collections.singletonList(resource.getURL());

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


More information about the commits mailing list