[java-identity-provider] branch main updated: JSPROF-9 Consider broadening the module#enable() API

Rod Widdowson rdw at steadingsoftware.com
Fri Nov 22 14:20:59 UTC 2024


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=17d9250392a8c085034834d91ce9748875c098f8

The following commit(s) were added to refs/heads/main by this push:
     new 17d925039 JSPROF-9 Consider broadening the module#enable() API
17d925039 is described below

commit 17d9250392a8c085034834d91ce9748875c098f8
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Nov 22 14:19:02 2024 +0000

    JSPROF-9 Consider broadening the module#enable() API
    
    https://shibboleth.atlassian.net/browse/JSPROF-9
    
    Where appropriate, add OperationType to the module contexts we summon.
    
    Teach the Installers to tell Module#enable whether this is an enable
    or a re-enable
---
 .../net/shibboleth/idp/module/IdPModuleTest.java   |  2 +
 .../net/shibboleth/idp/cli/impl/JarCheckCLI.java   |  2 +-
 .../shibboleth/idp/cli/impl/ModuleManagerCLI.java  |  2 +
 .../shibboleth/idp/installer/impl/V5Install.java   | 44 +++++++++++++++-------
 .../idp/installer/plugin/impl/PluginInstaller.java | 24 ++++++++----
 .../plugin/impl/RollbackPluginInstall.java         |  6 +--
 .../idp/installer/plugin/impl/PluginCLITest.java   |  4 +-
 7 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/idp-admin-api/src/test/java/net/shibboleth/idp/module/IdPModuleTest.java b/idp-admin-api/src/test/java/net/shibboleth/idp/module/IdPModuleTest.java
index b92bf325d..7c689ad1c 100644
--- a/idp-admin-api/src/test/java/net/shibboleth/idp/module/IdPModuleTest.java
+++ b/idp-admin-api/src/test/java/net/shibboleth/idp/module/IdPModuleTest.java
@@ -56,6 +56,7 @@ import com.google.common.io.ByteStreams;
 import net.shibboleth.idp.Version;
 import net.shibboleth.profile.module.Module;
 import net.shibboleth.profile.module.ModuleContext;
+import net.shibboleth.profile.module.ModuleContext.OperationType;
 import net.shibboleth.profile.module.ModuleException;
 import net.shibboleth.shared.httpclient.HttpClientBuilder;
 import net.shibboleth.shared.testing.RepositorySupport;
@@ -94,6 +95,7 @@ public class IdPModuleTest {
         final HttpClientSecurityParameters params = new HttpClientSecurityParameters();
         params.setTLSTrustEngine(buildExplicitKeyTrustEngine());
         context.setHttpClientSecurityParameters(params);
+        context.setOperationType(OperationType.CommandLine);
     }
     
     private void tearDownWorker() throws IOException {
diff --git a/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/JarCheckCLI.java b/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/JarCheckCLI.java
index ed4c44a86..6c471f590 100644
--- a/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/JarCheckCLI.java
+++ b/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/JarCheckCLI.java
@@ -222,7 +222,7 @@ public final class JarCheckCLI extends AbstractIdPHomeAwareCommandLine<JarCheckA
     private void processClassNames(@Nonnull final Map<String, String> namesSoFar, @Nonnull final Path jar) {
         final String source = jar.toString();
         try (final InputStream inStream = new BufferedInputStream(new FileInputStream(jar.toFile()));
-             final ArchiveInputStream classes = new ZipArchiveInputStream(inStream)) {
+             final ArchiveInputStream<?> classes = new ZipArchiveInputStream(inStream)) {
             
             ArchiveEntry entry = null;
             while ((entry = classes.getNextEntry()) != null) {
diff --git a/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/ModuleManagerCLI.java b/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/ModuleManagerCLI.java
index ebfd6acdb..94d52a3ac 100644
--- a/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/ModuleManagerCLI.java
+++ b/idp-cli/src/main/java/net/shibboleth/idp/cli/impl/ModuleManagerCLI.java
@@ -43,6 +43,7 @@ import net.shibboleth.idp.module.IdPModule;
 import net.shibboleth.profile.module.Module.ModuleResource;
 import net.shibboleth.profile.module.Module.ResourceResult;
 import net.shibboleth.profile.module.ModuleContext;
+import net.shibboleth.profile.module.ModuleContext.OperationType;
 import net.shibboleth.profile.module.ModuleException;
 import net.shibboleth.shared.annotation.constraint.NotLive;
 import net.shibboleth.shared.annotation.constraint.Unmodifiable;
@@ -105,6 +106,7 @@ public final class ModuleManagerCLI extends AbstractIdPHomeAwareCommandLine<Modu
             moduleContext.setHttpClient(getHttpClient());
             moduleContext.setHttpClientSecurityParameters(getHttpClientSecurityParameters());
             moduleContext.setLanguageRanges(args.getLanguageRanges());
+            moduleContext.setOperationType(OperationType.CommandLine);
             
             if (args.getList() || !args.getInfoModuleIds().isEmpty() || !args.getTestModuleIds().isEmpty()) {
                 return doList(moduleContext, args);
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/V5Install.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/V5Install.java
index 366a27a71..f355a7f75 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/V5Install.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/impl/V5Install.java
@@ -60,6 +60,7 @@ import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.idp.spring.IdPPropertiesApplicationContextInitializer;
 import net.shibboleth.profile.installablecomponent.InstallableComponentVersion;
 import net.shibboleth.profile.module.ModuleContext;
+import net.shibboleth.profile.module.ModuleContext.OperationType;
 import net.shibboleth.profile.module.ModuleException;
 import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.ComponentInitializationException;
@@ -93,6 +94,9 @@ public class V5Install {
     /** The Injected security parameters. */
     @Nullable private final HttpClientSecurityParameters httpClientSecurityParameters;
 
+    /** Local cache of the module Context. */
+    @Nullable private ModuleContext theModuleContext;
+
     /** Constructor.
      * @param props The properties to drive the installs.
      * @param installState The current install.
@@ -368,17 +372,35 @@ public class V5Install {
         }
     }
 
+    /**
+     * Build an appropriate {@link ModuleContext} , cache it and return it.
+     * @return a guaranteed not null context
+     */
+    @Nonnull private ModuleContext ensureModuleContext() {
+        ModuleContext context = theModuleContext;
+        if (context == null) {
+            final String targetDir = installerProps.getTargetDir().toString();
+            assert targetDir!=null;
+            context = new ModuleContext(targetDir);
+            context.setHttpClient(httpClient);
+            context.setHttpClientSecurityParameters(httpClientSecurityParameters);
+            if (currentState.getInstalledVersion() == null) {
+                context.setOperationType(OperationType.Install);
+            } else {
+                context.setOperationType(OperationType.Upgrade);
+            }
+            theModuleContext = context;
+        }
+        return context;
+    }
+
     /**
      * Enable Core modules if this is a new install.
      * 
      * @throws BuildException if badness occurs
      */
     protected void enableCoreModules() throws BuildException {
-        final String targetDir = installerProps.getTargetDir().toString();
-        assert targetDir!=null;
-        final ModuleContext moduleContext = new ModuleContext(targetDir);
-        moduleContext.setHttpClient(httpClient);
-        moduleContext.setHttpClientSecurityParameters(httpClientSecurityParameters);
+        final ModuleContext moduleContext = ensureModuleContext();
         final Iterator<IdPModule> modules = ServiceLoader.load(IdPModule.class).iterator();
 
         while (modules.hasNext()) {
@@ -387,7 +409,7 @@ public class V5Install {
                 final String id = module.getId();
                 if (installerProps.getCoreModules().contains(id) && !currentState.getEnabledModules().contains(id)) {
                     try {
-                        module.enable(moduleContext);
+                        module.enable(moduleContext, false);
                     } catch (final ModuleException e) {
                         log.error("Error performing initial enable on module {}", id, e);
                         throw new BuildException(e);
@@ -404,11 +426,7 @@ public class V5Install {
      * @throws BuildException if badness occurs
      */
     protected void enableModules() throws BuildException {
-        final String targetDir = installerProps.getTargetDir().toString();
-        assert targetDir!=null;
-        final ModuleContext moduleContext = new ModuleContext(targetDir);
-        moduleContext.setHttpClient(httpClient);
-        moduleContext.setHttpClientSecurityParameters(httpClientSecurityParameters);
+        final ModuleContext moduleContext = ensureModuleContext();
         final Iterator<IdPModule> modules = ServiceLoader.load(IdPModule.class).iterator();
 
         while (modules.hasNext()) {
@@ -418,7 +436,7 @@ public class V5Install {
                 if (currentState.getEnabledModules().contains(id)) {
                     log.debug("Re-enabling Module {}", id);
                     try {
-                        module.enable(moduleContext);
+                        module.enable(moduleContext, true);
                     } catch (final ModuleException e) {
                         log.error("Error re-enabling module {}", id, e);
                         throw new BuildException(e);
@@ -426,7 +444,7 @@ public class V5Install {
                 }
                 if (currentState.getInstalledVersion() == null && installerProps.getModulesToEnable().contains(id)) {
                     try {
-                        module.enable(moduleContext);
+                        module.enable(moduleContext, false);
                     } catch (final ModuleException e) {
                         log.error("Error performing initial enable on module {}", id, e);
                         throw new BuildException(e);
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 edad1b38d..f6981b502 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
@@ -73,6 +73,7 @@ import net.shibboleth.profile.installablecomponent.InstallableComponentVersion;
 import net.shibboleth.profile.module.Module.ModuleResource;
 import net.shibboleth.profile.module.Module.ResourceResult;
 import net.shibboleth.profile.module.ModuleContext;
+import net.shibboleth.profile.module.ModuleContext.OperationType;
 import net.shibboleth.profile.module.ModuleException;
 import net.shibboleth.profile.plugin.Plugin.Package;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
@@ -145,7 +146,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     /** Pluginss webapp. */
     @NonnullAfterInit private Path pluginsWebapp;
 
-    /** Pluginss webapp. */
+    /** Plugins webapp. */
     @NonnullAfterInit private Path pluginsContents;
 
     /** The absolute paths of what was installed - this is setup by {@link #loadCopiedFiles()}. */
@@ -314,6 +315,12 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
                 getDescription().getPatchVersion());
 
         final Set<String> loadedModules = getLoadedModules();
+        if (getVersionFromContents() == null) {
+            getModuleContext().setOperationType(OperationType.Install);
+        } else {
+            getModuleContext().setOperationType(OperationType.Upgrade);
+        }
+        getModuleContext().setPluginId(pluginId);
         try (final RollbackPluginInstall rollBack = new RollbackPluginInstall(getModuleContext(), moduleChanges)) {
             uninstallOld(rollBack);
 
@@ -340,8 +347,11 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     public void uninstall() throws BuildException {
 
         String moduleId = null;
-        assert pluginId != null;
-        description = getInstalledPlugin(pluginId);
+        final String pId = pluginId;
+        assert pId != null;
+        getModuleContext().setPluginId(pId);
+        getModuleContext().setOperationType(OperationType.Uninstall);
+        description = getInstalledPlugin(pId);
         if (description == null) {
             LOG.warn("Description for {} not found", pluginId);
         } else {
@@ -439,7 +449,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
     }
     
     /** Check for initialized and if so return the {@link #pluginsWebapp}.
-     * @return the {@link #moduleContext}.
+     * @return the {@link #pluginsWebapp}.
      */
     @Nonnull private Path getPluginsWebapp() {
         checkComponentActive();
@@ -560,7 +570,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
                         LOG.debug("Module {} not previously enabled", moduleId);
                     } else {
                         LOG.debug("Re-enabling module {}", moduleId);
-                        captureChanges(module.enable(getModuleContext()));
+                        captureChanges(module.enable(getModuleContext(), true));
                     }
                 } else {
                     LOG.debug("Module {}, not provided by this plugin", module.getId());
@@ -634,11 +644,11 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
                 moduleId = module.getId();
                 if (!module.isEnabled(getModuleContext())) {
                     LOG.debug("Enabling Module {}", moduleId);
-                    captureChanges(module.enable(getModuleContext()));
+                    captureChanges(module.enable(getModuleContext(), false));
                     rollBack.getModulesEnabled().add(module);
                 } else {
                     LOG.debug("Re-enabling Module {}", moduleId);
-                    captureChanges(module.enable(getModuleContext()));
+                    captureChanges(module.enable(getModuleContext(), true));
                 }
                 reenabledPluginModules.add(moduleId);
             }
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 6ce0c3114..3dbceef69 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
@@ -138,10 +138,10 @@ public class RollbackPluginInstall implements AutoCloseable {
         for (int i = modulesDisabled.size()-1; i >=0; i--) {
             final IdPModule module = modulesDisabled.get(i);
             try {
-                log.trace("Deleting {}", module.getId());
-                captureChanges(module.enable(moduleContext));
+                log.trace("Re-Enabling {}", module.getId());
+                captureChanges(module.enable(moduleContext, true));
             } catch (final Throwable t) {
-                log.error("Could not disable {}, continuing ", module.getId(), t);
+                log.error("Could not re-enable {}, continuing ", module.getId(), t);
             }            
         }
         return true;
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginCLITest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginCLITest.java
index 5ffd47118..79a701440 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginCLITest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginCLITest.java
@@ -41,9 +41,9 @@ public class PluginCLITest extends BasePluginTest {
     
     private final String PLUGIN_DISTRO = "http://test.shibboleth.net/downloads/identity-provider/plugins/oidc-common/3.1.0/oidc-common-dist-3.1.0.tar.gz";
 
-    private final String PLUGIN_IDS[] = { "net.shibboleth.oidc.common", "net.shibboleth.idp.plugin.jettybase","net.shibboleth.idp.plugin.authn.oidc.rp"};
+    private final String PLUGIN_IDS[] = { "net.shibboleth.oidc.common", "net.shibboleth.idp.plugin.authn.oidc.rp"};
 
-    private final String PLUGIN_ID = PLUGIN_IDS[1];
+    private final String PLUGIN_ID = PLUGIN_IDS[0];
 
     @BeforeSuite public void setUp() throws IOException
     {

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


More information about the commits mailing list