modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/content/ContentProviderDetailsUIBean.java | 28 +++-- modules/enterprise/gui/portal-war/src/main/webapp/rhq/content/repo.xhtml | 4 modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java | 48 +++++----- modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerBean.java | 9 - modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerLocal.java | 8 - modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/metadata/ContentSourceMetadataManagerBean.java | 19 ++- modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManager.java | 23 +--- modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/ContentSourcePluginHandlingTest.java | 1 modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/TestBase.java | 2 modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManagerSyncContentProviderTest.java | 11 +- 10 files changed, 84 insertions(+), 69 deletions(-)
New commits: commit fafa6dddfc95d38c6bb1b893d03d01838295a387 Merge: a6eb18d... acda1f0... Author: Ian P. Springer <ips@jetengine.(none)> Date: Mon Mar 29 19:12:19 2010 -0400
Merge branch 'bugfixes' of ssh://git.fedorahosted.org/git/rhq/rhq into bugfixes
commit a6eb18d233b2781478f2c19602490e14fd75f5ea Author: Ian P. Springer <ips@jetengine.(none)> Date: Mon Mar 29 19:09:55 2010 -0400
fix issue causing ContentSourcePluginHandlingTest.testUpdateContentSourceTypes() to fail; improve some variable names
diff --git a/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/content/ContentProviderDetailsUIBean.java b/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/content/ContentProviderDetailsUIBean.java index 52b011f..4b89f50 100644 --- a/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/content/ContentProviderDetailsUIBean.java +++ b/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/content/ContentProviderDetailsUIBean.java @@ -27,6 +27,7 @@ import org.rhq.core.domain.configuration.definition.ConfigurationDefinition; import org.rhq.core.domain.content.ContentSource; import org.rhq.core.domain.content.DownloadMode; import org.rhq.core.gui.util.FacesContextUtility; +import org.rhq.core.util.exception.ThrowableUtil; import org.rhq.enterprise.gui.util.EnterpriseFacesContextUtility; import org.rhq.enterprise.server.content.ContentSourceManagerLocal; import org.rhq.enterprise.server.util.LookupUtil; @@ -66,13 +67,15 @@ public class ContentProviderDetailsUIBean { public String test() { ContentSourceManagerLocal manager = LookupUtil.getContentSourceManager();
- if (manager.testContentSourceConnection(contentSource.getId())) { + try { + manager.testContentSourceConnection(this.contentSource.getId()); FacesContextUtility.addMessage(FacesMessage.SEVERITY_INFO, - "Test passed - the remote repository for [" + contentSource.getName() + "] is available."); - } else { + "Test passed - the remote repository for [" + this.contentSource.getName() + "] is available."); + } catch (Exception e) { FacesContextUtility.addMessage(FacesMessage.SEVERITY_WARN, - "Test failed - failed to connect to the remote repository for [" + contentSource.getName() - + "] - check the configuration and make sure the remote repository is up and reachable."); + "Test failed - failed to connect to the remote repository for [" + this.contentSource.getName() + + "] - check the configuration and make sure the remote repository is up and reachable. Details: " + + ThrowableUtil.getAllMessages(e)); }
return "success"; @@ -81,21 +84,24 @@ public class ContentProviderDetailsUIBean { public String sync() { // Test the content provider connection before proceeding. ContentSourceManagerLocal contentSourceManager = LookupUtil.getContentSourceManager(); - if (!contentSourceManager.testContentSourceConnection(contentSource.getId())) { + try { + contentSourceManager.testContentSourceConnection(this.contentSource.getId()); + } catch (Exception e) { FacesContextUtility.addMessage(FacesMessage.SEVERITY_ERROR, - "Failed to connect to the remote repository for [" + contentSource.getName() - + "] - check the configuration and make sure the remote repository is up and reachable."); + "Failed to connect to the remote repository for [" + this.contentSource.getName() + + "] - check the configuration and make sure the remote repository is up and reachable. Details: " + + ThrowableUtil.getAllMessages(e)); return "success"; }
Subject subject = EnterpriseFacesContextUtility.getSubject(); try { - contentSourceManager.synchronizeAndLoadContentSource(subject, contentSource.getId()); + contentSourceManager.synchronizeAndLoadContentSource(subject, this.contentSource.getId()); FacesContextUtility.addMessage(FacesMessage.SEVERITY_INFO, "Synchronizing content provider [" - + contentSource.getName() + "] now."); + + this.contentSource.getName() + "] now."); } catch (Exception e) { FacesContextUtility.addMessage(FacesMessage.SEVERITY_WARN, - "Failed to start the synchronization process for [" + contentSource.getName() + "]", e); + "Failed to start the synchronization process for [" + this.contentSource.getName() + "]", e); }
return "success"; diff --git a/modules/enterprise/gui/portal-war/src/main/webapp/rhq/content/repo.xhtml b/modules/enterprise/gui/portal-war/src/main/webapp/rhq/content/repo.xhtml index 91a35b4..a58e826 100644 --- a/modules/enterprise/gui/portal-war/src/main/webapp/rhq/content/repo.xhtml +++ b/modules/enterprise/gui/portal-war/src/main/webapp/rhq/content/repo.xhtml @@ -332,6 +332,9 @@ <!-- colspan 8:7 for the debug 'id' column --> <rich:column colspan="#{param.debug ? 8 : 7}" width="100%"> + + ui:remove + <!-- The ability to associate/disassociate repos with content sources is not needed for JON. --> <h:commandButton action="#{RepoContentSourcesUIBean.associateWithContentProviders}" value="ASSOCIATE..." @@ -341,6 +344,7 @@ value="DISASSOCIATE SELECTED" target="selectedRepoContentProviders" styleClass="on-pager-button buttonsmall" /> + </ui:remove>
<ui:param name="paginationDataTableName" value="repoContentProvidersDataTable" /> diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java index 8814a06..24c9148 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/configuration/metadata/ConfigurationMetadataManagerBean.java @@ -28,6 +28,8 @@ import javax.ejb.Stateless; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext;
+import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.rhq.core.domain.configuration.Configuration; import org.rhq.core.domain.configuration.Property; import org.rhq.core.domain.configuration.PropertySimple; @@ -47,6 +49,8 @@ import org.rhq.enterprise.server.RHQConstants; */ @Stateless public class ConfigurationMetadataManagerBean implements ConfigurationMetadataManagerLocal { + private final Log log = LogFactory.getLog(this.getClass()); + @PersistenceContext(unitName = RHQConstants.PERSISTENCE_UNIT_NAME) private EntityManager entityManager;
@@ -76,9 +80,9 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa }
// delete outdated properties - removeNolongerUsedProperties(newDefinition, existingDefinition, existingPropertyDefinitions, null); + removeNoLongerUsedProperties(newDefinition, existingDefinition, existingPropertyDefinitions); } else { - // TODO what if exisitingDefinitions is null? + // TODO what if existingDefinitions is null? // we probably don't run in here, as the initial persisting is done // somewhere else. } @@ -132,8 +136,8 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa }
// delete outdated properties of this group - removeNolongerUsedProperties(newDefinition, existingDefinition, existingDefinition - .getPropertiesInGroup(groupName), group); + removeNoLongerUsedProperties(newDefinition, existingDefinition, existingDefinition + .getPropertiesInGroup(groupName)); }
entityManager.flush(); @@ -270,28 +274,30 @@ public class ConfigurationMetadataManagerBean implements ConfigurationMetadataMa /** * Removes PropertyDefinition items from the configuration * - * @param newDefinition new configuration to persist - * @param existingDefinition existing persisted configuration - * @param existingProperties list of existing properties - * @param groupDef TODO + * @param newConfigDef new configuration being merged into the existing one + * @param existingConfigDef existing persisted configuration + * @param existingProperties list of existing properties to inspect for potential removal */ - private void removeNolongerUsedProperties(ConfigurationDefinition newDefinition, - ConfigurationDefinition existingDefinition, List<PropertyDefinition> existingProperties, - PropertyGroupDefinition groupDef) { - - List<PropertyDefinition> definitionsToDelete = new ArrayList<PropertyDefinition>(); - for (PropertyDefinition exDef : existingProperties) { - PropertyDefinition nDef = newDefinition.get(exDef.getName()); - if (nDef == null) { + private void removeNoLongerUsedProperties(ConfigurationDefinition newConfigDef, + ConfigurationDefinition existingConfigDef, + List<PropertyDefinition> existingProperties) { + List<PropertyDefinition> propDefsToDelete = new ArrayList<PropertyDefinition>(); + for (PropertyDefinition existingPropDef : existingProperties) { + PropertyDefinition newPropDef = newConfigDef.get(existingPropDef.getName()); + if (newPropDef == null) { // not in new configuration - definitionsToDelete.add(exDef); + propDefsToDelete.add(existingPropDef); } } - // System.out.println("Props to delete " + definitionsToDelete);
- for (PropertyDefinition def : definitionsToDelete) { - existingDefinition.getPropertyDefinitions().remove(def.getName()); - existingProperties.remove(def); // does not operate on original list!! + if (!propDefsToDelete.isEmpty()) { + log.debug("Deleting obsolete props from configDef [" + existingConfigDef + "]: " + propDefsToDelete); + for (PropertyDefinition propDef : propDefsToDelete) { + existingConfigDef.getPropertyDefinitions().remove(propDef.getName()); + existingProperties.remove(propDef); // does not operate on original list!! + entityManager.remove(propDef); + } + entityManager.merge(existingConfigDef); } }
diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerBean.java index 20ad6bc..37c72a0 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerBean.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerBean.java @@ -556,16 +556,15 @@ public class ContentSourceManagerBean implements ContentSourceManagerLocal {
}
- public boolean testContentSourceConnection(int contentSourceId) { + public void testContentSourceConnection(int contentSourceId) throws Exception { try { - ContentServerPluginContainer pc = ContentManagerHelper.getPluginContainer(); - return pc.getAdapterManager().testConnection(contentSourceId); + ContentServerPluginContainer contentServerPluginContainer = ContentManagerHelper.getPluginContainer(); + contentServerPluginContainer.getAdapterManager().testConnection(contentSourceId); } catch (Exception e) { log.info("Failed to test connection to [" + contentSourceId + "]. Cause: " + ThrowableUtil.getAllMessages(e)); log.debug("Content source test connection failure stack follows for [" + contentSourceId + "]", e); - - return false; + throw e; } }
diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerLocal.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerLocal.java index 4ff85fe..a934f95 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerLocal.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/ContentSourceManagerLocal.java @@ -203,13 +203,13 @@ public interface ContentSourceManagerLocal {
/** * Given a content source ID, this will test that the adapter responsible for pulling data from the content source's - * remote repository can actually connect to that repository. + * remote repository can actually connect to that repository, and if not, throw an Exception. * - * @param contentSourceId The id of the content source on which to test the connection. + * @param contentSourceId the id of the content source on which to test the connection * - * @return <code>true</code> if the remote content souce can be reached + * @throws Exception if the test failed */ - boolean testContentSourceConnection(int contentSourceId); + void testContentSourceConnection(int contentSourceId) throws Exception;
/** * Requests that the identified content source be synchronized and if not lazy-loading to also download its diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/metadata/ContentSourceMetadataManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/metadata/ContentSourceMetadataManagerBean.java index 9b83fca..7a41902 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/metadata/ContentSourceMetadataManagerBean.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/content/metadata/ContentSourceMetadataManagerBean.java @@ -96,25 +96,26 @@ public class ContentSourceMetadataManagerBean implements ContentSourceMetadataMa }
private void updateConfigurationDefinition(ContentSourceType newType, ContentSourceType existingType) { - ConfigurationDefinition newConfig = newType.getContentSourceConfigurationDefinition(); - ConfigurationDefinition existingConfig = existingType.getContentSourceConfigurationDefinition(); + ConfigurationDefinition newConfigDef = newType.getContentSourceConfigurationDefinition(); + ConfigurationDefinition existingConfigDef = existingType.getContentSourceConfigurationDefinition();
- if (newConfig != null) { - if (existingConfig == null) { + if (newConfigDef != null) { + if (existingConfigDef == null) { // everything is new - entityManager.persist(newConfig); - existingType.setContentSourceConfigurationDefinition(newConfig); + entityManager.persist(newConfigDef); + existingType.setContentSourceConfigurationDefinition(newConfigDef); } else { // both new and existing had some kind of configuration, update the existing to match the new - configurationMetadataManager.updateConfigurationDefinition(newConfig, existingConfig); + configurationMetadataManager.updateConfigurationDefinition(newConfigDef, existingConfigDef); } } else { // the new config is null -> remove the existing config - if (existingConfig != null) { + if (existingConfigDef != null) { existingType.setContentSourceConfigurationDefinition(null); - entityManager.remove(existingConfig); + entityManager.remove(existingConfigDef); } } + return; }
/** diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManager.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManager.java index a6d8d2a..bba2be0 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManager.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManager.java @@ -512,24 +512,15 @@ public class ContentProviderManager { /** * Tests the connection to the content source that has the given ID. * - * @param contentSourceId - * refers to a valid content source in the database - * @return <code>true</code> if there is an adapter that can successfully - * connect to the given content source <code>false</code> if there - * is an adapter but it cannot connect - * @throws Exception - * if failed to get an adapter to attempt the connection + * @param contentSourceId the id of the content source to be tested + * + * @throws Exception if the test connection to the specified content source failed */ - public boolean testConnection(int contentSourceId) throws Exception { + public void testConnection(int contentSourceId) throws Exception { ContentProvider adapter = getIsolatedContentProvider(contentSourceId); - - try { - adapter.testConnection(); - } catch (Throwable t) { - return false; - } - - return true; + adapter.testConnection(); + //noinspection UnnecessaryReturnStatement + return; }
/** diff --git a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/ContentSourcePluginHandlingTest.java b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/ContentSourcePluginHandlingTest.java index 912deeb..cb310c9 100644 --- a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/ContentSourcePluginHandlingTest.java +++ b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/ContentSourcePluginHandlingTest.java @@ -107,6 +107,7 @@ public class ContentSourcePluginHandlingTest extends TestBase {
// now hot deploy a new version of that plugin registerPlugin("./test/metadata/content-source-update-v2.xml"); + getEntityManager().clear(); type1 = getContentSourceType("testCSPHT1"); type2 = getContentSourceType("testCSPHT2"); type3 = getContentSourceType("testCSPHT3"); diff --git a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/TestBase.java b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/TestBase.java index 13af653..45d250f 100644 --- a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/TestBase.java +++ b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/content/metadata/test/TestBase.java @@ -56,7 +56,7 @@ public class TestBase extends AbstractEJB3Test {
protected ContentSourceType getContentSourceType(String typeName) throws Exception { getTransactionManager().begin(); - EntityManager em = getEntityManager(); + EntityManager em = getEntityManager(); try { Query q = em.createNamedQuery(ContentSourceType.QUERY_FIND_BY_NAME_WITH_CONFIG_DEF); ContentSourceType type = (ContentSourceType) q.setParameter("name", typeName).getSingleResult(); diff --git a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManagerSyncContentProviderTest.java b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManagerSyncContentProviderTest.java index c3bec3a..567f8f5 100644 --- a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManagerSyncContentProviderTest.java +++ b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/pc/content/ContentProviderManagerSyncContentProviderTest.java @@ -211,8 +211,15 @@ public class ContentProviderManagerSyncContentProviderTest extends AbstractEJB3T // Test // -------------------------------------------- // TestContentProviderManager providerManager = new TestContentProviderManager(); - boolean tested = pluginService.getContentProviderManager().testConnection(syncSource.getId()); - assert tested; + boolean tested = false; + try { + pluginService.getContentProviderManager().testConnection(syncSource.getId()); + tested = true; + } catch (Exception e) { + System.err.println("testConnection() failed: " + e); + e.printStackTrace(); + } + assert tested : "testConnection()";
boolean completed = pluginService.getContentProviderManager().synchronizeContentProvider(syncSource.getId()); assert completed;
rhq-commits@lists.fedorahosted.org