modules/core/domain/src/main/java/org/rhq/core/domain/resource/ResourceType.java
| 3 -
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilter.java
| 24 ++++++++-
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
| 13 ++++-
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryServerServiceImpl.java
| 6 ++
modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/InventoryReportFilter.java
| 9 ---
modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilterTest.java
| 26 +++++-----
6 files changed, 57 insertions(+), 24 deletions(-)
New commits:
commit c5c722c853ca0d99d30dda4c9b7c50dee6476640
Author: John Sanda <jsanda(a)redhat.com>
Date: Mon Oct 22 17:09:41 2012 -0400
[BZ 869015] Add more detail logging when handling inventory report with stale types
This commit contains a few changes:
* Add logging in DiscoveryBossBean when inventory report contains deleted types
There is detailed log statements that include both the deleted resource types
as well as the agent sending the report.
* Remove ERROR log statement from DiscoveryServerServiceImpl
No need to log the exception since it is not an error. We just want to throw
the exception back to the agent so it knows to resync its plugins.
* Revert ResourceType.toString
Not sure why but in commit bb2b881045 was changed. The version prior to that
commit was already concise yet detailed and specific. I have reverted back
to that version.
diff --git
a/modules/core/domain/src/main/java/org/rhq/core/domain/resource/ResourceType.java
b/modules/core/domain/src/main/java/org/rhq/core/domain/resource/ResourceType.java
index bf36ee0..ee2586d 100644
--- a/modules/core/domain/src/main/java/org/rhq/core/domain/resource/ResourceType.java
+++ b/modules/core/domain/src/main/java/org/rhq/core/domain/resource/ResourceType.java
@@ -906,7 +906,8 @@ public class ResourceType implements Serializable,
Comparable<ResourceType> {
@Override
public String toString() {
- return "{" + this.plugin + "}" + this.name;
+ return "ResourceType[id=" + this.id + ", name=" + this.name
+ ", plugin=" + this.plugin +
+ ", category=" + this.category + "]";
}
}
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilter.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilter.java
index 42106ae..c2371bd 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilter.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilter.java
@@ -15,7 +15,7 @@ import org.rhq.enterprise.server.auth.SubjectManagerLocal;
import org.rhq.enterprise.server.resource.ResourceTypeManagerLocal;
import org.rhq.enterprise.server.resource.metadata.PluginManagerLocal;
-public class DeletedResourceTypeFilter implements InventoryReportFilter {
+public class DeletedResourceTypeFilter {
private SubjectManagerLocal subjectMgr;
@@ -77,6 +77,28 @@ public class DeletedResourceTypeFilter implements InventoryReportFilter
{
return true;
}
+ public Set<ResourceType> apply(InventoryReport report) {
+ Set<ResourceType> resourceTypes =
getResourceTypes(report.getAddedRoots());
+ Set<ResourceType> staleTypes = new HashSet<ResourceType>();
+
+ for (ResourceType type : resourceTypes) {
+ // We check two things to determine whether a report should be rejected.
First we check
+ // that the plugin from which the type comes is installed. We check that the
plugin is
+ // installed as opposed to checking that it is deleted because the plugin
could also
+ // be purged in which case checking against deleted plugins wouldn't
catch it. Secondly,
+ // we check to see if the type is a deleted type. Here we check against
deleted types
+ // instead of installed types because the number of deleted types at any
given time
+ // should be much smaller than the number of installed types, resulting in
faster look
+ // ups. If we have a stale type in the report and that type has already been
purged from
+ // the database, it will get flagged by the check against installed plugins.
+ if (!installedPlugins.contains(type.getPlugin()) ||
+ deletedTypes.contains(type.getName() + "::" +
type.getPlugin())) {
+ staleTypes.add(type);
+ }
+ }
+ return staleTypes;
+ }
+
private Set<ResourceType> getResourceTypes(Set<Resource> resources) {
Set<ResourceType> types = new HashSet<ResourceType>();
for (Resource resource : resources) {
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
index 2511d28..ded24ce 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryBossBean.java
@@ -139,8 +139,17 @@ public class DiscoveryBossBean implements DiscoveryBossLocal,
DiscoveryBossRemot
public ResourceSyncInfo mergeInventoryReport(InventoryReport report) throws
InvalidInventoryReportException {
validateInventoryReport(report);
- InventoryReportFilter filter = new DeletedResourceTypeFilter(subjectManager,
resourceTypeManager, pluginManager);
- if (!filter.accept(report)) {
+ DeletedResourceTypeFilter filter = new DeletedResourceTypeFilter(subjectManager,
resourceTypeManager, pluginManager);
+ Set<ResourceType> deletedTypes = filter.apply(report);
+
+ if (!deletedTypes.isEmpty()) {
+ if (log.isDebugEnabled()) {
+ log.debug("The inventory report from " + report.getAgent() +
" with added roots " +
+ report.getAddedRoots() + " contains these deleted resource types
" + deletedTypes);
+ } else {
+ log.info("The inventory report from " + report.getAgent() +
" contains these deleted resource types " +
+ deletedTypes);
+ }
throw new StaleTypeException("The report contains one or more resource
types that have been marked for "
+ "deletion.");
}
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryServerServiceImpl.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryServerServiceImpl.java
index 153144a..75d97f7 100644
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryServerServiceImpl.java
+++
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/DiscoveryServerServiceImpl.java
@@ -73,6 +73,12 @@ public class DiscoveryServerServiceImpl implements
DiscoveryServerService {
ResourceSyncInfo syncInfo;
try {
syncInfo = discoveryBoss.mergeInventoryReport(report);
+ } catch (StaleTypeException e) {
+ // There is no need to log this exception as it is part of a normal work
flow
+ // that occurs as a result of a user deleting a plugin. DiscoveryBossBean
+ // already logs a message about the stale types that can be useful for
+ // debugging; so, we just need to propagate the exception to the agent.
+ throw e;
} catch (InvalidInventoryReportException e) {
Agent agent = report.getAgent();
if (log.isDebugEnabled()) {
diff --git
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/InventoryReportFilter.java
b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/InventoryReportFilter.java
deleted file mode 100644
index 78ac3e1..0000000
---
a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/discovery/InventoryReportFilter.java
+++ /dev/null
@@ -1,9 +0,0 @@
-package org.rhq.enterprise.server.discovery;
-
-import org.rhq.core.clientapi.server.discovery.InventoryReport;
-
-public interface InventoryReportFilter {
-
- boolean accept(InventoryReport report);
-
-}
diff --git
a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilterTest.java
b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilterTest.java
index 1c3dc7d..c0ac4a1 100644
---
a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilterTest.java
+++
b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/discovery/DeletedResourceTypeFilterTest.java
@@ -1,5 +1,9 @@
package org.rhq.enterprise.server.discovery;
+import static org.rhq.test.AssertUtils.assertCollectionEqualsNoOrder;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
@@ -7,19 +11,17 @@ import java.util.List;
import java.util.Set;
import org.jmock.Expectations;
-
-import org.rhq.core.domain.plugin.Plugin;
-import org.rhq.core.domain.resource.Resource;
-import org.rhq.core.domain.resource.ResourceCategory;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import org.rhq.core.clientapi.server.discovery.InventoryReport;
import org.rhq.core.domain.auth.Subject;
import org.rhq.core.domain.criteria.ResourceTypeCriteria;
+import org.rhq.core.domain.plugin.Plugin;
import org.rhq.core.domain.resource.Agent;
-import org.rhq.core.domain.shared.ResourceBuilder;
+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.auth.SubjectManagerLocal;
@@ -27,8 +29,6 @@ import org.rhq.enterprise.server.resource.ResourceTypeManagerLocal;
import org.rhq.enterprise.server.resource.metadata.PluginManagerLocal;
import org.rhq.test.JMockTest;
-import static org.testng.Assert.*;
-
public class DeletedResourceTypeFilterTest extends JMockTest {
SubjectManagerLocal subjectMgr;
@@ -74,8 +74,9 @@ public class DeletedResourceTypeFilterTest extends JMockTest {
filter = new DeletedResourceTypeFilter(subjectMgr, resourceTypeMgr, pluginMgr);
- assertTrue(filter.accept(report), "Expected report to be accepted when it
does not contain any deleted " +
- "resource types");
+ Set<ResourceType> staleTypes = filter.apply(report);
+ assertTrue(staleTypes.isEmpty(), "Expected filter to return empty set when
report does not contain any " +
+ "deleted resource types, but filter returned " + staleTypes);
}
@Test
@@ -119,8 +120,10 @@ public class DeletedResourceTypeFilterTest extends JMockTest {
}});
filter = new DeletedResourceTypeFilter(subjectMgr, resourceTypeMgr, pluginMgr);
+ Set<ResourceType> staleTypes = filter.apply(report);
- assertFalse(filter.accept(report), "Expected report to be rejected since it
contains deleted resource types");
+ assertCollectionEqualsNoOrder(staleTypes, deletedTypes, "Expected report to
be rejected since it contains " +
+ "deleted resource types");
}
@Test
@@ -157,7 +160,8 @@ public class DeletedResourceTypeFilterTest extends JMockTest {
filter = new DeletedResourceTypeFilter(subjectMgr, resourceTypeMgr, pluginMgr);
- assertFalse(filter.accept(report), "Expected report to be rejected since it
contains a deleted plugin");
+ assertFalse(filter.apply(report).isEmpty(), "Expected filter to return
non-empty since it contains a deleted " +
+ "plugin but empty set was returned");
}
InventoryReport createReport() {