modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/server/drift/ManageDriftDefinitionsTest.java | 123 +++++++--- modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/drift/DriftManagerBean.java | 109 ++++---- 2 files changed, 151 insertions(+), 81 deletions(-)
New commits: commit 615c99b2a0d856b316b558a4e6fd7989b4004a15 Author: John Sanda jsanda@redhat.com Date: Mon Dec 19 09:20:56 2011 -0500
[BZ 768099] Add validation checks when creating a drift definition
The following validation checks have been added:
* verify that the resource supports drift management * verify that the template attached to the def exists in the db * verify that that the template is from the correct resource type
diff --git a/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/server/drift/ManageDriftDefinitionsTest.java b/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/server/drift/ManageDriftDefinitionsTest.java index 6e93bb3..ac55445 100644 --- a/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/server/drift/ManageDriftDefinitionsTest.java +++ b/modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/server/drift/ManageDriftDefinitionsTest.java @@ -19,56 +19,64 @@
package org.rhq.enterprise.server.drift;
+import org.rhq.core.domain.common.EntityContext; +import org.rhq.core.domain.configuration.Configuration; +import org.rhq.core.domain.criteria.DriftDefinitionCriteria; +import org.rhq.core.domain.criteria.JPADriftChangeSetCriteria; +import org.rhq.core.domain.drift.*; +import org.rhq.core.domain.resource.Resource; +import org.rhq.core.domain.resource.ResourceType; +import org.rhq.core.domain.shared.ResourceBuilder; +import org.rhq.core.domain.shared.ResourceTypeBuilder; +import org.rhq.core.domain.util.PageList; +import org.rhq.enterprise.server.safeinvoker.HibernateDetachUtility; +import org.rhq.test.AssertUtils; +import org.rhq.test.TransactionCallback; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import javax.ejb.EJBException; +import javax.persistence.EntityManager; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + import static java.util.Arrays.asList; import static org.rhq.core.domain.drift.DriftCategory.FILE_ADDED; import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; import static org.rhq.core.domain.drift.DriftConfigurationDefinition.BaseDirValueContext.fileSystem; import static org.rhq.core.domain.drift.DriftConfigurationDefinition.DriftHandlingMode.normal; import static org.rhq.core.domain.drift.DriftDefinitionComparator.CompareMode.BOTH_BASE_INFO_AND_DIRECTORY_SPECIFICATIONS; +import static org.rhq.core.domain.resource.ResourceCategory.SERVER; import static org.rhq.enterprise.server.util.LookupUtil.getDriftManager; import static org.rhq.enterprise.server.util.LookupUtil.getDriftTemplateManager;
-import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; - -import javax.persistence.EntityManager; - -import org.testng.annotations.BeforeClass; +public class ManageDriftDefinitionsTest extends DriftServerTest {
-import org.rhq.core.domain.common.EntityContext; -import org.rhq.core.domain.configuration.Configuration; -import org.rhq.core.domain.criteria.DriftDefinitionCriteria; -import org.rhq.core.domain.criteria.JPADriftChangeSetCriteria; -import org.rhq.core.domain.drift.Drift; -import org.rhq.core.domain.drift.DriftChangeSet; -import org.rhq.core.domain.drift.DriftComplianceStatus; -import org.rhq.core.domain.drift.DriftConfigurationDefinition; -import org.rhq.core.domain.drift.DriftDefinition; -import org.rhq.core.domain.drift.DriftDefinitionComparator; -import org.rhq.core.domain.drift.DriftDefinitionTemplate; -import org.rhq.core.domain.drift.DriftSnapshot; -import org.rhq.core.domain.drift.JPADrift; -import org.rhq.core.domain.drift.JPADriftChangeSet; -import org.rhq.core.domain.drift.JPADriftFile; -import org.rhq.core.domain.drift.JPADriftSet; -import org.rhq.core.domain.util.PageList; -import org.rhq.enterprise.server.safeinvoker.HibernateDetachUtility; -import org.rhq.test.AssertUtils; -import org.rhq.test.TransactionCallback; + private final String DRIFT_NOT_SUPPORTED_TYPE = getClass().getSimpleName() + "DRIFT_NOT_SUPPORTED_RESOURCE_TYPE";
-public class ManageDriftDefinitionsTest extends DriftServerTest { + private final String DRIFT_NOT_SUPPORTED_RESOURCE = getClass().getSimpleName() + "DRIFT_NOT_SUPPORTED_RESOURCE";
private DriftManagerLocal driftMgr;
private DriftTemplateManagerLocal templateMgr;
+ private ResourceType driftNotSupportedType; + + private Resource driftNotSupportedResource; + @BeforeClass public void initClass() throws Exception { driftMgr = getDriftManager(); templateMgr = getDriftTemplateManager(); }
+ @Override + protected void purgeDB(EntityManager em) { + deleteEntity(Resource.class, DRIFT_NOT_SUPPORTED_RESOURCE, em); + deleteEntity(ResourceType.class, DRIFT_NOT_SUPPORTED_TYPE, em); + } + public void createDefinitionFromUnpinnedTemplate() { // first create a template final DriftDefinition templateDef = new DriftDefinition(new Configuration()); @@ -96,6 +104,52 @@ public class ManageDriftDefinitionsTest extends DriftServerTest { assertEquals("The template association was not set on the definition", template, newDef.getTemplate()); }
+ public void createEntitiesThatDoNotSupportDrift() { + // first create the resource type that does not support drift + driftNotSupportedType = new ResourceTypeBuilder() + .createResourceType() + .withId(0) + .withName(DRIFT_NOT_SUPPORTED_TYPE) + .withCategory(SERVER) + .withPlugin(DRIFT_NOT_SUPPORTED_TYPE.toLowerCase()) + .build(); + + // create a resource of the type that does not support drift + driftNotSupportedResource = new ResourceBuilder() + .createResource() + .withId(0) + .withName(DRIFT_NOT_SUPPORTED_RESOURCE) + .withResourceKey(DRIFT_NOT_SUPPORTED_RESOURCE) + .withRandomUuid() + .withResourceType(driftNotSupportedType) + .build(); + + executeInTransaction(new TransactionCallback() { + @Override + public void execute() throws Exception { + EntityManager em = getEntityManager(); + em.persist(driftNotSupportedType); + em.persist(driftNotSupportedResource); + } + }); + } + + @Test(dependsOnMethods = "createEntitiesThatDoNotSupportDrift", + expectedExceptions = EJBException.class, + expectedExceptionsMessageRegExp = ".*Cannot create drift definition.*type.*does not support drift management") + @InitDB(false) + public void doNotAllowDefinitionToBeCreatedForTypeThatDoesNotSupportDrift() { + DriftDefinition driftDef = new DriftDefinition(new Configuration()); + driftDef.setName("test_typeDoesNotSupportDrift"); + driftDef.setEnabled(true); + driftDef.setInterval(1800L); + driftDef.setDriftHandlingMode(normal); + driftDef.setBasedir(new DriftDefinition.BaseDirectory(fileSystem, "/foo/bar/test")); + + driftMgr.updateDriftDefinition(getOverlord(), EntityContext.forResource(driftNotSupportedResource.getId()), + driftDef); + } + @SuppressWarnings("unchecked") public void createDefinitionFromPinnedTemplate() throws Exception { // We first need to create a pinned template. Users can only create a pinned @@ -209,8 +263,19 @@ public class ManageDriftDefinitionsTest extends DriftServerTest { }
public void unpinDefinition() { + // First create the template + final DriftDefinition templateDef = new DriftDefinition(new Configuration()); + templateDef.setName("test_unpin_def_template"); + templateDef.setEnabled(true); + templateDef.setDriftHandlingMode(normal); + templateDef.setInterval(2400L); + templateDef.setBasedir(new DriftDefinition.BaseDirectory(fileSystem, "/foo/bar/test")); + + final DriftDefinitionTemplate template = templateMgr.createTemplate(getOverlord(), resourceType.getId(), true, + templateDef); + // First create the definition - DriftDefinition definition = new DriftDefinition(new Configuration()); + DriftDefinition definition = template.createDefinition(); definition.setName("test_unpin"); definition.setEnabled(true); definition.setBasedir(new DriftDefinition.BaseDirectory(fileSystem, "/foo/bar/test")); diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/drift/DriftManagerBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/drift/DriftManagerBean.java index 12c7233..3da8bd2 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/drift/DriftManagerBean.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/drift/DriftManagerBean.java @@ -18,73 +18,25 @@ */ package org.rhq.enterprise.server.drift;
-import static java.util.Arrays.asList; -import static javax.ejb.TransactionAttributeType.NOT_SUPPORTED; -import static javax.ejb.TransactionAttributeType.REQUIRES_NEW; -import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; -import static org.rhq.core.domain.drift.DriftComplianceStatus.IN_COMPLIANCE; -import static org.rhq.core.domain.drift.DriftComplianceStatus.OUT_OF_COMPLIANCE_DRIFT; - -import java.io.File; -import java.io.InputStream; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import javax.ejb.EJB; -import javax.ejb.Stateless; -import javax.ejb.TransactionAttribute; -import javax.jms.Connection; -import javax.jms.ConnectionFactory; -import javax.jms.MessageProducer; -import javax.jms.ObjectMessage; -import javax.jms.Queue; -import javax.jms.Session; -import javax.persistence.EntityManager; -import javax.persistence.PersistenceContext; - import difflib.DiffUtils; import difflib.Patch; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.hibernate.Hibernate; - import org.jboss.remoting.CannotConnectException; - import org.rhq.core.clientapi.agent.drift.DriftAgentService; import org.rhq.core.domain.auth.Subject; import org.rhq.core.domain.common.EntityContext; -import org.rhq.core.domain.criteria.DriftChangeSetCriteria; -import org.rhq.core.domain.criteria.DriftCriteria; -import org.rhq.core.domain.criteria.DriftDefinitionCriteria; -import org.rhq.core.domain.criteria.GenericDriftChangeSetCriteria; -import org.rhq.core.domain.criteria.GenericDriftCriteria; -import org.rhq.core.domain.drift.Drift; -import org.rhq.core.domain.drift.DriftCategory; -import org.rhq.core.domain.drift.DriftChangeSet; -import org.rhq.core.domain.drift.DriftChangeSetCategory; -import org.rhq.core.domain.drift.DriftComplianceStatus; -import org.rhq.core.domain.drift.DriftComposite; -import org.rhq.core.domain.drift.DriftConfigurationDefinition; +import org.rhq.core.domain.criteria.*; +import org.rhq.core.domain.drift.*; import org.rhq.core.domain.drift.DriftConfigurationDefinition.DriftHandlingMode; -import org.rhq.core.domain.drift.DriftDefinition; import org.rhq.core.domain.drift.DriftDefinition.BaseDirectory; -import org.rhq.core.domain.drift.DriftDefinitionComparator; import org.rhq.core.domain.drift.DriftDefinitionComparator.CompareMode; -import org.rhq.core.domain.drift.DriftDefinitionComposite; -import org.rhq.core.domain.drift.DriftDefinitionTemplate; -import org.rhq.core.domain.drift.DriftDetails; -import org.rhq.core.domain.drift.DriftFile; -import org.rhq.core.domain.drift.DriftSnapshot; -import org.rhq.core.domain.drift.DriftSnapshotRequest; -import org.rhq.core.domain.drift.FileDiffReport; -import org.rhq.core.domain.drift.Filter; import org.rhq.core.domain.drift.dto.DriftChangeSetDTO; import org.rhq.core.domain.drift.dto.DriftDTO; import org.rhq.core.domain.drift.dto.DriftFileDTO; import org.rhq.core.domain.resource.Resource; +import org.rhq.core.domain.resource.ResourceType; import org.rhq.core.domain.util.PageControl; import org.rhq.core.domain.util.PageList; import org.rhq.core.domain.util.PageOrdering; @@ -103,6 +55,28 @@ import org.rhq.enterprise.server.util.CriteriaQueryGenerator; import org.rhq.enterprise.server.util.CriteriaQueryRunner; import org.rhq.enterprise.server.util.LookupUtil;
+import javax.ejb.EJB; +import javax.ejb.Stateless; +import javax.ejb.TransactionAttribute; +import javax.jms.*; +import javax.persistence.EntityManager; +import javax.persistence.PersistenceContext; +import java.io.File; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static java.util.Arrays.asList; +import static javax.ejb.TransactionAttributeType.NOT_SUPPORTED; +import static javax.ejb.TransactionAttributeType.REQUIRES_NEW; +import static org.rhq.core.domain.drift.DriftChangeSetCategory.COVERAGE; +import static org.rhq.core.domain.drift.DriftComplianceStatus.IN_COMPLIANCE; +import static org.rhq.core.domain.drift.DriftComplianceStatus.OUT_OF_COMPLIANCE_DRIFT; + +import java.lang.IllegalStateException; + /** * The SLSB supporting Drift management to clients. * @@ -716,7 +690,6 @@ public class DriftManagerBean implements DriftManagerLocal, DriftManagerRemote {
@Override public void updateDriftDefinition(Subject subject, EntityContext entityContext, DriftDefinition driftDef) { - // before we do anything, validate certain field values to prevent downstream errors validateDriftDefinition(driftDef);
@@ -728,6 +701,11 @@ public class DriftManagerBean implements DriftManagerLocal, DriftManagerRemote { throw new IllegalArgumentException("Entity not found [" + entityContext + "]"); }
+ if (!isDriftMgmtSupported(resource)) { + throw new IllegalArgumentException("Cannot create drift definition. The resource type " + + resource.getResourceType() + " does not support drift management"); + } + // Update or add the driftDef as necessary DriftDefinitionComparator comparator = new DriftDefinitionComparator( CompareMode.ONLY_DIRECTORY_SPECIFICATIONS); @@ -752,6 +730,7 @@ public class DriftManagerBean implements DriftManagerLocal, DriftManagerRemote { }
if (!isUpdated) { + validateTemplateForNewDef(driftDef, resource); resource.addDriftDefinition(driftDef); // We call persist here because if this definition is created // from a pinned template, then we need to generate the initial @@ -826,6 +805,32 @@ public class DriftManagerBean implements DriftManagerLocal, DriftManagerRemote { } }
+ private boolean isDriftMgmtSupported(Resource resource) { + ResourceType type = resource.getResourceType(); + return type.getDriftDefinitionTemplates() != null && !type.getDriftDefinitionTemplates().isEmpty(); + } + + private void validateTemplateForNewDef(DriftDefinition driftDef, Resource resource) { + if (driftDef.getTemplate() == null) { + return; + } + + DriftDefinitionTemplate template = entityManager.find(DriftDefinitionTemplate.class, + driftDef.getTemplate().getId()); + + if (template == null) { + throw new IllegalArgumentException("Cannot create drift definition with template " + + DriftDefinitionTemplate.class.getSimpleName() + "[" + driftDef.getTemplate().getName() + + "] that has not been saved"); + } + + if (!template.getResourceType().equals(resource.getResourceType())) { + throw new IllegalArgumentException("Cannot create drift definition with template " + + DriftDefinitionTemplate.class.getSimpleName() + "[" + driftDef.getTemplate().getName() + + "] that is from a different resource type, " + template.getResourceType()); + } + } + @Override public boolean isBinaryFile(Subject subject, Drift<?, ?> drift) { return DriftUtil.isBinaryFile(drift);
rhq-commits@lists.fedorahosted.org