David, thanks for adding this to keep track of what plugins have been installed on the server.
I think there is a prob with the change. In InstallModulesMojo.java, as it set installedPluginsList as null. I think this would cause all the plugins that came with the server assembly during build time (using c-m-p) not recorded, as saveHistory and loadHistory only handle cases when installedPluginList is not null. Also, in PluginInstallerGBean.java, I don't see anywhere you specify where we set the default location of the installedPluginsList file to "var/config/installedPlugins.properties"... I only see that in the two test files. Lin On Fri, Sep 26, 2008 at 3:26 AM, <[EMAIL PROTECTED]> wrote: > Author: djencks > Date: Fri Sep 26 00:26:53 2008 > New Revision: 699202 > > URL: http://svn.apache.org/viewvc?rev=699202&view=rev > Log: > GERONIMO-4318 try to indicate when plugins have been installed in the current > server, irrespective of whether they are in the repos > > Modified: > > geronimo/server/trunk/buildsupport/car-maven-plugin/src/main/java/org/apache/geronimo/mavenplugins/car/InstallModulesMojo.java > > geronimo/server/trunk/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/jmx/RemoteDeploymentManager.java > > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstaller.java > > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstallerGBean.java > > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/CopyFileTest.java > > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/PluginInstallerTest.java > > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/AbstractListHandler.java > > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/ViewPluginDownloadHandler.java > > Modified: > geronimo/server/trunk/buildsupport/car-maven-plugin/src/main/java/org/apache/geronimo/mavenplugins/car/InstallModulesMojo.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/buildsupport/car-maven-plugin/src/main/java/org/apache/geronimo/mavenplugins/car/InstallModulesMojo.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/buildsupport/car-maven-plugin/src/main/java/org/apache/geronimo/mavenplugins/car/InstallModulesMojo.java > (original) > +++ > geronimo/server/trunk/buildsupport/car-maven-plugin/src/main/java/org/apache/geronimo/mavenplugins/car/InstallModulesMojo.java > Fri Sep 26 00:26:53 2008 > @@ -162,7 +162,7 @@ > Kernel kernel = new BasicKernel("Assembly"); > PluginRepositoryList pluginRepoList = new > PluginRepositoryDownloader(Collections.singletonMap(localRepo, (String[]) > null), true); > try { > - PluginInstallerGBean installer = new > PluginInstallerGBean(targetRepositoryPath, targetServerPath, servers, > pluginRepoList, kernel, getClass().getClassLoader()); > + PluginInstallerGBean installer = new > PluginInstallerGBean(targetRepositoryPath, targetServerPath, null, servers, > pluginRepoList, kernel, getClass().getClassLoader()); > installer.install(pluginList, sourceRepo, true, null, null, > downloadPoller); > if (overrides != null) { > for (Override override: this.overrides) { > > Modified: > geronimo/server/trunk/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/jmx/RemoteDeploymentManager.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/jmx/RemoteDeploymentManager.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/jmx/RemoteDeploymentManager.java > (original) > +++ > geronimo/server/trunk/framework/modules/geronimo-deploy-jsr88/src/main/java/org/apache/geronimo/deployment/plugin/jmx/RemoteDeploymentManager.java > Fri Sep 26 00:26:53 2008 > @@ -189,10 +189,10 @@ > } > } > > - public void validatePlugin(PluginType plugin) throws > MissingDependencyException { > + public boolean validatePlugin(PluginType plugin) throws > MissingDependencyException { > PluginInstaller installer = getPluginInstaller(); > try { > - installer.validatePlugin(plugin); > + return installer.validatePlugin(plugin); > } finally { > kernel.getProxyManager().destroyProxy(installer); > } > > Modified: > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstaller.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstaller.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstaller.java > (original) > +++ > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstaller.java > Fri Sep 26 00:26:53 2008 > @@ -21,12 +21,14 @@ > import java.net.URL; > import java.util.Map; > import javax.security.auth.login.FailedLoginException; > -import org.apache.geronimo.kernel.repository.Artifact; > -import org.apache.geronimo.kernel.repository.Dependency; > -import org.apache.geronimo.kernel.repository.MissingDependencyException; > + > +import org.apache.geronimo.kernel.config.ConfigurationAlreadyExistsException; > import org.apache.geronimo.kernel.config.ConfigurationManager; > import org.apache.geronimo.kernel.config.NoSuchStoreException; > import org.apache.geronimo.kernel.InvalidGBeanException; > +import org.apache.geronimo.kernel.repository.Artifact; > +import org.apache.geronimo.kernel.repository.Dependency; > +import org.apache.geronimo.kernel.repository.MissingDependencyException; > import org.apache.geronimo.system.plugin.model.PluginListType; > import org.apache.geronimo.system.plugin.model.PluginType; > import org.apache.geronimo.system.plugin.model.AttributesType; > @@ -52,7 +54,7 @@ > * @return A Map with key type String (plugin name) and value type > Artifact > * (config ID of the plugin). > */ > - public Map getInstalledPlugins(); > + public Map<String, Artifact> getInstalledPlugins(); > > /** > * Gets a CofigurationMetadata for a configuration installed in the local > @@ -185,7 +187,7 @@ > * @throws > org.apache.geronimo.kernel.repository.MissingDependencyException > * if a dependency is not satisfied > */ > - public void validatePlugin(PluginType plugin) throws > MissingDependencyException; > + public boolean validatePlugin(PluginType plugin) throws > MissingDependencyException; > > /** > * Ensures that a plugin's prerequisites are installed > > Modified: > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstallerGBean.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstallerGBean.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstallerGBean.java > (original) > +++ > geronimo/server/trunk/framework/modules/geronimo-plugin/src/main/java/org/apache/geronimo/system/plugin/PluginInstallerGBean.java > Fri Sep 26 00:26:53 2008 > @@ -21,6 +21,8 @@ > import java.io.FileOutputStream; > import java.io.IOException; > import java.io.InputStream; > +import java.io.FileInputStream; > +import java.io.OutputStream; > import java.net.URI; > import java.net.URL; > import java.util.ArrayList; > @@ -54,6 +56,7 @@ > import org.apache.geronimo.gbean.ReferenceCollectionEvent; > import org.apache.geronimo.gbean.ReferenceCollectionListener; > import org.apache.geronimo.gbean.annotation.GBean; > +import org.apache.geronimo.gbean.annotation.ParamAttribute; > import org.apache.geronimo.gbean.annotation.ParamReference; > import org.apache.geronimo.gbean.annotation.ParamSpecial; > import org.apache.geronimo.gbean.annotation.SpecialAttributeType; > @@ -68,6 +71,7 @@ > import org.apache.geronimo.kernel.config.NoSuchConfigException; > import org.apache.geronimo.kernel.config.NoSuchStoreException; > import org.apache.geronimo.kernel.config.PersistentConfigurationList; > +import org.apache.geronimo.kernel.config.ConfigurationAlreadyExistsException; > import org.apache.geronimo.kernel.repository.Artifact; > import org.apache.geronimo.kernel.repository.ArtifactManager; > import org.apache.geronimo.kernel.repository.DefaultArtifactManager; > @@ -116,6 +120,9 @@ > private static final Logger log = > LoggerFactory.getLogger(PluginInstallerGBean.class); > > private static int counter; > + private final String installedPluginsList; > + //all plugins that have ever been installed on this server. > + private final Set<Artifact> installedArtifacts = new HashSet<Artifact>(); > private final ConfigurationManager configManager; > private final GeronimoSourceRepository localSourceRepository; > private final WritableListableRepository writeableRepo; > @@ -146,7 +153,8 @@ > * @param classLoader classLoader @throws IOException > exception if server instance cannot be loaded > * @throws java.io.IOException from bad ServerInstance > */ > - public PluginInstallerGBean(@ParamReference(name = "ConfigManager", > namingType = "ConfigurationManager")ConfigurationManager configManager, > + public PluginInstallerGBean(@ParamAttribute(name = > "installedPluginsList")String installedPluginsList, > + @ParamReference(name = "ConfigManager", > namingType = "ConfigurationManager")ConfigurationManager configManager, > @ParamReference(name = "Repository", > namingType = "Repository")WritableListableRepository repository, > @ParamReference(name = "ConfigStore", > namingType = "ConfigurationStore")ConfigurationStore configStore, > @ParamReference(name = "ServerInstances", > namingType = "ServerInstanceData")Collection<ServerInstanceData> > serverInstanceDatas, > @@ -171,6 +179,8 @@ > localSourceRepository = new > GeronimoSourceRepository(configManager.getRepositories(), > configManager.getArtifactResolver()); > setUpServerInstances(serverInstanceDatas, serverInfo, > artifactManager, servers, writeableRepo, true); > this.pluginRepositoryList = pluginRepositoryList; > + this.installedPluginsList = installedPluginsList; > + loadHistory(); > } > > /** > @@ -178,6 +188,7 @@ > * > * @param targetRepositoryPath location of repo to install into (not in > current server) > * @param targetServerPath location of server to install into (not > current server > + * @param installedPluginsList location of file to track installations > * @param serverInstanceDatas set of server layouts > * @param pluginRepositoryList > * @param kernel kernel for current server > @@ -185,6 +196,7 @@ > */ > public PluginInstallerGBean(String targetRepositoryPath, > String targetServerPath, > + String installedPluginsList, > Collection<? extends ServerInstanceData> > serverInstanceDatas, > PluginRepositoryList pluginRepositoryList, > final Kernel kernel, > @@ -206,6 +218,8 @@ > this.configManager = buildConfigurationManager(artifactManager, > writeableRepo, kernel, configStore, classLoader, servers); > localSourceRepository = new > GeronimoSourceRepository(configManager.getRepositories(), > configManager.getArtifactResolver()); > this.pluginRepositoryList = pluginRepositoryList; > + this.installedPluginsList = installedPluginsList; > + loadHistory(); > } > > private static void setUpServerInstances(Collection<? extends > ServerInstanceData> serverInstanceDatas, > @@ -290,7 +304,7 @@ > PluginInstallerGBean(ConfigurationManager configManager, > WritableListableRepository repository, > ConfigurationStore configStore, > - ServerInfo serverInfo, > + String installedPluginsList, ServerInfo serverInfo, > ThreadPool threadPool, > Collection<ServerInstance> servers, > PluginRepositoryList pluginRepositoryList) { > this.configManager = configManager; > @@ -321,6 +335,53 @@ > }); > } > this.pluginRepositoryList = pluginRepositoryList; > + this.installedPluginsList = installedPluginsList; > + loadHistory(); > + } > + > + private void loadHistory() { > + if (installedPluginsList != null) { > + File historyFile = > serverInfo.resolveServer(installedPluginsList); > + Properties properties = new Properties(); > + try { > + InputStream in = new FileInputStream(historyFile); > + try { > + properties.load(in); > + for (Object key : properties.keySet()) { > + Artifact artifact = Artifact.create((String) key); > + installedArtifacts.add(artifact); > + } > + } finally { > + in.close(); > + } > + } catch (IOException e) { > + //give up > + } > + } > + } > + > + private void saveHistory() { > + if (installedPluginsList != null) { > + Properties properties = new Properties(); > + for (Artifact artifact : installedArtifacts) { > + properties.put(artifact.toString(), null); > + } > + try { > + File historyFile = > serverInfo.resolveServer(installedPluginsList); > + File parentFile = historyFile.getParentFile(); > + if (!parentFile.exists()) { > + FileUtils.forceMkdir(parentFile); > + } > + OutputStream out = new FileOutputStream(historyFile); > + try { > + properties.save(out, "All the plugins that have ever > been installed on this server"); > + } finally { > + out.close(); > + } > + } catch (IOException e) { > + //give up > + } > + } > } > > /** > @@ -346,6 +407,7 @@ > PluginInstallerGBean installer = new PluginInstallerGBean( > targetRepositoryPath, > targetServerPathName, > + installedPluginsList, > serverInstanceDatas, > pluginRepositoryList, kernel, > classLoader); > @@ -381,7 +443,7 @@ > * @return A Map with key type String (plugin name) and value type > Artifact > * (config ID of the plugin). > */ > - public Map getInstalledPlugins() { > + public Map<String, Artifact> getInstalledPlugins() { > SortedSet<Artifact> artifacts = writeableRepo.list(); > > Map<String, Artifact> plugins = new HashMap<String, Artifact>(); > @@ -416,7 +478,7 @@ > jar.close(); > } > } catch (IOException e) { > - log.error("Unable to read JAR file " + > dir.getAbsolutePath(), e); > + log.error("Unable to read JAR file {}", > dir.getAbsolutePath(), e); > } > } > } > @@ -622,7 +684,9 @@ > for (PluginType metadata : pluginsToInstall.getPlugin()) { > try { > if (validatePlugins) { > - validatePlugin(metadata); > + if (!validatePlugin(metadata)) { > + throw new MissingDependencyException("Already > installed", toArtifact(metadata.getPluginArtifact().get(0).getModuleId()), > (Stack<Artifact>)null); > + } > verifyPrerequisites(metadata); > } > > @@ -717,6 +781,7 @@ > } finally { > poller.setFinished(); > } > + saveHistory(); > } > > private List<SourceRepository> getRepos(PluginListType pluginsToInstall, > SourceRepository defaultRepository, boolean restrictToDefaultRepository, > PluginArtifactType instance) { > @@ -865,7 +930,11 @@ > } > > // 2. Validate that we can install this > - validatePlugin(data); > + if (!validatePlugin(data)) { > + //already installed > + return; > + } > + > verifyPrerequisites(data); > > PluginArtifactType instance = data.getPluginArtifact().get(0); > @@ -899,10 +968,11 @@ > * Ensures that a plugin is installable. > * > * @param plugin plugin to check > + * @return true if the plugin is not installed > * @throws > org.apache.geronimo.kernel.repository.MissingDependencyException > * if plugin requires a dependency that is not present > */ > - public void validatePlugin(PluginType plugin) throws > MissingDependencyException { > + public boolean validatePlugin(PluginType plugin) throws > MissingDependencyException { > if (plugin.getPluginArtifact().size() != 1) { > throw new MissingDependencyException("A plugin configuration must > include one plugin artifact, not " + plugin.getPluginArtifact().size(), null, > (Stack<Artifact>) null); > } > @@ -919,11 +989,9 @@ > break; > } > } > - if (!upgrade) { > + if (!upgrade && installedArtifacts.contains(artifact)) { > log.debug("Configuration {} is already installed", > artifact); > -// throw new MissingDependencyException( > -// "Configuration " + artifact + " is already > installed.", toArtifact(metadata.getModuleId()), (Stack<Artifact>) null); > - } > + return false; } > } > } > > @@ -938,6 +1006,7 @@ > throw new MissingDependencyException( > "Plugin is not installable on JVM " + > System.getProperty("java.version"), toArtifact(metadata.getModuleId()), > (Stack<Artifact>) null); > } > + return true; > } > > > @@ -1090,7 +1159,10 @@ > throw (IOException) new IOException("Unable to read > plugin metadata: " + e.getMessage()).initCause(e); > } > if (pluginData != null) { // it's a plugin, not a plain JAR > - validatePlugin(pluginData); > + if (!validatePlugin(pluginData)) { > + monitor.getResults().addSkippedConfigID(new > MissingDependencyException("already installed", configID, > (Stack<Artifact>)null)); > + return; > + } > instance = pluginData.getPluginArtifact().get(0); > } > monitor.getResults().setCurrentMessage("Copying " + > result.getArtifact() + " to the repository"); > @@ -1170,8 +1242,9 @@ > throw new IllegalStateException("Installed configuration into > repository but ConfigStore cannot load it: " + e.getMessage(), e); > } > // Copy any files out of the artifact > - for (ServerInstance serverInstance: servers.values()) { > + for (ServerInstance serverInstance : servers.values()) { > if > (serverInstance.getAttributeStore().isModuleInstalled(configID)) { > + installedArtifacts.add(configID); > return; > } > } > @@ -1182,6 +1255,7 @@ > } > } > if (instance != null) { > + installedArtifacts.add(configID); > try { > installConfigXMLData(configID, instance, servers, > loadOverride); > } catch (InvalidGBeanException e) { > > Modified: > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/CopyFileTest.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/CopyFileTest.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/CopyFileTest.java > (original) > +++ > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/CopyFileTest.java > Fri Sep 26 00:26:53 2008 > @@ -47,6 +47,7 @@ > private ConfigurationStore configStore; > private PluginInstallerGBean installer; > private Artifact artifact = new Artifact("test", "module", "1.0", "car"); > + private String installedPluginsList = > "var/config/installedPlugins.properties"; > > protected void setUp() throws Exception { > super.setUp(); > @@ -72,7 +73,7 @@ > installer = new PluginInstallerGBean(new MockConfigurationManager(), > repo, > configStore, > - serverInfo, > + installedPluginsList, serverInfo, > new ThreadPool() { > public int getPoolSize() { > return 0; > > Modified: > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/PluginInstallerTest.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/PluginInstallerTest.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/PluginInstallerTest.java > (original) > +++ > geronimo/server/trunk/framework/modules/geronimo-plugin/src/test/java/org/apache/geronimo/system/plugin/PluginInstallerTest.java > Fri Sep 26 00:26:53 2008 > @@ -41,7 +41,8 @@ > private String fakeRepo; > private String testRepo; > private PluginInstaller installer; > - > + private String installedPluginsList = > "var/config/installedPlugins.properties"; > + > protected void setUp() throws Exception { > super.setUp(); > fakeRepo = "http://nowhere.com/";; > @@ -50,7 +51,7 @@ > testRepo = url.substring(0, pos); > ServerInfo serverInfo = new BasicServerInfo("."); > installer = new PluginInstallerGBean(new MockConfigurationManager(), > new MockWritableListableRepository(), new MockConfigStore(), > - serverInfo, new ThreadPool() { > + installedPluginsList, serverInfo, new ThreadPool() { > public int getPoolSize() { > return 0; > } > > Modified: > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/AbstractListHandler.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/AbstractListHandler.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/AbstractListHandler.java > (original) > +++ > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/AbstractListHandler.java > Fri Sep 26 00:26:53 2008 > @@ -65,7 +65,7 @@ > // determine if the plugin is installable > PluginType holder = PluginInstallerGBean.copy(metadata, > artifact); > try { > - pluginInstaller.validatePlugin(holder); > + > plugin.setInstallable(pluginInstaller.validatePlugin(holder)); > } catch (Exception e) { > plugin.setInstallable(false); > } > > Modified: > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/ViewPluginDownloadHandler.java > URL: > http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/ViewPluginDownloadHandler.java?rev=699202&r1=699201&r2=699202&view=diff > ============================================================================== > --- > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/ViewPluginDownloadHandler.java > (original) > +++ > geronimo/server/trunk/plugins/console/plugin-portlets/src/main/java/org/apache/geronimo/console/car/ViewPluginDownloadHandler.java > Fri Sep 26 00:26:53 2008 > @@ -89,7 +89,7 @@ > StringBuffer validationNotOk = new StringBuffer(); > PluginType holder = PluginInstallerGBean.copy(plugin.getPlugin(), > plugin.getPluginArtifact()); > try { > - pluginInstaller.validatePlugin(holder); > + > plugin.setInstallable(pluginInstaller.validatePlugin(holder)); > } catch (Exception e) { > plugin.setInstallable(false); > validationNotOk.append(e.getMessage()); > > >
