Return-Path: X-Original-To: apmail-brooklyn-commits-archive@minotaur.apache.org Delivered-To: apmail-brooklyn-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id A5AB110B47 for ; Tue, 27 May 2014 21:35:40 +0000 (UTC) Received: (qmail 85395 invoked by uid 500); 27 May 2014 21:35:40 -0000 Delivered-To: apmail-brooklyn-commits-archive@brooklyn.apache.org Received: (qmail 85372 invoked by uid 500); 27 May 2014 21:35:40 -0000 Mailing-List: contact commits-help@brooklyn.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.incubator.apache.org Delivered-To: mailing list commits@brooklyn.incubator.apache.org Received: (qmail 85365 invoked by uid 99); 27 May 2014 21:35:40 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 May 2014 21:35:40 +0000 X-ASF-Spam-Status: No, hits=-2000.7 required=5.0 tests=ALL_TRUSTED,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Tue, 27 May 2014 21:35:39 +0000 Received: (qmail 85020 invoked by uid 99); 27 May 2014 21:35:14 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 May 2014 21:35:14 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id D483732AE5D; Tue, 27 May 2014 21:35:13 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: richard@apache.org To: commits@brooklyn.incubator.apache.org Date: Tue, 27 May 2014 21:35:14 -0000 Message-Id: <5e06055a14704537be40672f33eb3f20@git.apache.org> In-Reply-To: <02b70e3c1fe94ad3a209d2c6e9ab99b0@git.apache.org> References: <02b70e3c1fe94ad3a209d2c6e9ab99b0@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [2/8] git commit: AbstractMembershipTrackingPolicy: make GROUP a config key X-Virus-Checked: Checked by ClamAV on apache.org AbstractMembershipTrackingPolicy: make GROUP a config key Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/3a9e027c Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/3a9e027c Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/3a9e027c Branch: refs/heads/master Commit: 3a9e027cd2ab6833a53dec637ac1ac295124d3a7 Parents: e041842 Author: Aled Sage Authored: Mon May 26 12:08:16 2014 +0100 Committer: Aled Sage Committed: Tue May 27 20:49:34 2014 +0100 ---------------------------------------------------------------------- .../group/AbstractMembershipTrackingPolicy.java | 74 +++++++++++++++----- .../group/MembershipTrackingPolicyTest.java | 52 ++++++++++---- 2 files changed, 97 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/3a9e027c/core/src/main/java/brooklyn/entity/group/AbstractMembershipTrackingPolicy.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/group/AbstractMembershipTrackingPolicy.java b/core/src/main/java/brooklyn/entity/group/AbstractMembershipTrackingPolicy.java index 049d49c..996dc85 100644 --- a/core/src/main/java/brooklyn/entity/group/AbstractMembershipTrackingPolicy.java +++ b/core/src/main/java/brooklyn/entity/group/AbstractMembershipTrackingPolicy.java @@ -14,6 +14,7 @@ import brooklyn.entity.Group; import brooklyn.entity.basic.Attributes; import brooklyn.entity.basic.ConfigKeys; import brooklyn.entity.basic.DynamicGroup; +import brooklyn.entity.basic.EntityLocal; import brooklyn.entity.trait.Startable; import brooklyn.event.Sensor; import brooklyn.event.SensorEvent; @@ -30,9 +31,7 @@ import com.google.common.reflect.TypeToken; public abstract class AbstractMembershipTrackingPolicy extends AbstractPolicy { private static final Logger LOG = LoggerFactory.getLogger(AbstractMembershipTrackingPolicy.class); - private Group group; - - private ConcurrentMap, Object>> entitySensorCache = new ConcurrentHashMap, Object>>(); + public enum EventType { ENTITY_CHANGE, ENTITY_ADDED, ENTITY_REMOVED } @SuppressWarnings("serial") public static final ConfigKey>> SENSORS_TO_TRACK = ConfigKeys.newConfigKey( @@ -44,6 +43,10 @@ public abstract class AbstractMembershipTrackingPolicy extends AbstractPolicy { public static final ConfigKey NOTIFY_ON_DUPLICATES = ConfigKeys.newBooleanConfigKey("notifyOnDuplicates", "Whether to notify listeners when a sensor is published with the same value as last time", true); + + public static final ConfigKey GROUP = ConfigKeys.newConfigKey(Group.class, "group"); + + private ConcurrentMap, Object>> entitySensorCache = new ConcurrentHashMap, Object>>(); public AbstractMembershipTrackingPolicy(Map flags) { super(flags); @@ -60,23 +63,56 @@ public abstract class AbstractMembershipTrackingPolicy extends AbstractPolicy { .build(); } + @Override + public void setEntity(EntityLocal entity) { + super.setEntity(entity); + Group group = getGroup(); + if (group != null) { + subscribeToGroup(group); + } else { + LOG.warn("Deprecated use of "+AbstractMembershipTrackingPolicy.class.getSimpleName()+"; group should be set as config"); + } + } + /** * Sets the group to be tracked; unsubscribes from any previous group, and subscribes to this group. * * Note this must be called *after* adding the policy to the entity. * * @param group + * + * @deprecated since 0.7; instead set the group as config */ + @Deprecated public void setGroup(Group group) { - Preconditions.checkNotNull(group, "The group cannot be null"); - unsubscribeFromGroup(); - this.group = group; - subscribeToGroup(); + // relies on doReconfigureConfig to make the actual change + LOG.warn("Deprecated use of setGroup in "+AbstractMembershipTrackingPolicy.class.getSimpleName()+"; group should be set as config"); + setConfig(GROUP, group); + } + + @Override + protected void doReconfigureConfig(ConfigKey key, T val) { + if (GROUP.getName().equals(key.getName())) { + Preconditions.checkNotNull(val, "%s value must not be null", GROUP.getName()); + Preconditions.checkNotNull(val, "%s value must be a group, but was %s (of type %s)", GROUP.getName(), val, val.getClass()); + if (val.equals(getConfig(GROUP))) { + if (LOG.isDebugEnabled()) LOG.debug("No-op for reconfigure group of "+AbstractMembershipTrackingPolicy.class.getSimpleName()+"; group is still "+val); + } else { + if (LOG.isInfoEnabled()) LOG.info("Membership tracker "+AbstractMembershipTrackingPolicy.class+", resubscribing to group "+val+", previously was "+getGroup()); + unsubscribeFromGroup(); + subscribeToGroup((Group)val); + } + } else { + throw new UnsupportedOperationException("reconfiguring "+key+" unsupported for "+this); + } } /** * Unsubscribes from the group. + * + * @deprecated since 0.7; misleading method name; either remove the policy, or suspend/resume */ + @Deprecated public void reset() { unsubscribeFromGroup(); } @@ -89,16 +125,23 @@ public abstract class AbstractMembershipTrackingPolicy extends AbstractPolicy { @Override public void resume() { + boolean wasSuspended = isSuspended(); super.resume(); - if (group != null) { - subscribeToGroup(); + + Group group = getGroup(); + if (wasSuspended && group != null) { + subscribeToGroup(group); } } + + protected Group getGroup() { + return getConfig(GROUP); + } - protected void subscribeToGroup() { - Preconditions.checkNotNull(group, "The group cannot be null"); + protected void subscribeToGroup(final Group group) { + Preconditions.checkNotNull(group, "The group must not be null"); - LOG.debug("Subscribing to group "+group+", for memberAdded, memberRemoved, serviceUp, and {}", getSensorsToTrack()); + LOG.debug("Subscribing to group "+group+", for memberAdded, memberRemoved, and {}", getSensorsToTrack()); subscribe(group, DynamicGroup.MEMBER_ADDED, new SensorEventListener() { @Override public void onEvent(SensorEvent event) { @@ -144,16 +187,13 @@ public abstract class AbstractMembershipTrackingPolicy extends AbstractPolicy { } for (Entity it : group.getMembers()) { onEntityEvent(EventType.ENTITY_ADDED, it); } - - // FIXME cluster may be remote, we need to make this retrieve the remote values, or store members in local mgmt node, or use children } protected void unsubscribeFromGroup() { - if (getSubscriptionTracker()!=null && group != null) unsubscribe(group); + Group group = getGroup(); + if (getSubscriptionTracker() != null && group != null) unsubscribe(group); } - public enum EventType { ENTITY_CHANGE, ENTITY_ADDED, ENTITY_REMOVED } - /** All entity events pass through this method. Default impl delegates to onEntityXxxx methods, whose default behaviours are no-op. * Callers may override this to intercept all entity events in a single place, and to suppress subsequent processing if desired. */ http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/3a9e027c/core/src/test/java/brooklyn/entity/group/MembershipTrackingPolicyTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/brooklyn/entity/group/MembershipTrackingPolicyTest.java b/core/src/test/java/brooklyn/entity/group/MembershipTrackingPolicyTest.java index e597358..fba8549 100644 --- a/core/src/test/java/brooklyn/entity/group/MembershipTrackingPolicyTest.java +++ b/core/src/test/java/brooklyn/entity/group/MembershipTrackingPolicyTest.java @@ -3,6 +3,7 @@ package brooklyn.entity.group; import static org.testng.Assert.assertEquals; import static org.testng.Assert.fail; +import java.util.Arrays; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -48,9 +49,8 @@ public class MembershipTrackingPolicyTest { group = app.createAndManageChild(EntitySpec.create(BasicGroup.class) .configure("childrenAsMembers", true)); - policy = new RecordingMembershipTrackingPolicy(MutableMap.of("group", group)); - group.addPolicy(policy); - policy.setGroup(group); + policy = app.addPolicy(PolicySpec.create(RecordingMembershipTrackingPolicy.class) + .configure("group", group)); app.start(ImmutableList.of(loc)); } @@ -126,10 +126,23 @@ public class MembershipTrackingPolicyTest { public void testNotifiedOfExtraTrackedSensors() throws Exception { TestEntity e1 = createAndManageChildOf(group); - RecordingMembershipTrackingPolicy policy2 = new RecordingMembershipTrackingPolicy(MutableMap.of("group", group, "sensorsToTrack", ImmutableSet.of(TestEntity.NAME))); + RecordingMembershipTrackingPolicy policy2 = app.addPolicy(PolicySpec.create(RecordingMembershipTrackingPolicy.class) + .configure("group", group) + .configure("sensorsToTrack", ImmutableSet.of(TestEntity.NAME))); + + + e1.setAttribute(TestEntity.NAME, "myname"); + + assertRecordsEventually(policy2, Record.newAdded(e1), Record.newChanged(e1)); + } + + @Test + public void testDeprecatedSetGroupWorks() throws Exception { + RecordingMembershipTrackingPolicy policy2 = new RecordingMembershipTrackingPolicy(MutableMap.of("sensorsToTrack", ImmutableSet.of(TestEntity.NAME))); group.addPolicy(policy2); policy2.setGroup(group); + TestEntity e1 = createAndManageChildOf(group); e1.setAttribute(TestEntity.NAME, "myname"); assertRecordsEventually(policy2, Record.newAdded(e1), Record.newChanged(e1)); @@ -139,14 +152,10 @@ public class MembershipTrackingPolicyTest { public void testNotNotifiedOfExtraTrackedSensorsIfNonDuplicate() throws Exception { TestEntity e1 = createAndManageChildOf(group); - PolicySpec nonDuplicateTrackingPolicySpec = - PolicySpec.create(RecordingMembershipTrackingPolicy.class) + RecordingMembershipTrackingPolicy nonDuplicateTrackingPolicy = app.addPolicy(PolicySpec.create(RecordingMembershipTrackingPolicy.class) .configure(AbstractMembershipTrackingPolicy.SENSORS_TO_TRACK, ImmutableSet.>of(TestEntity.NAME)) - .configure(AbstractMembershipTrackingPolicy.NOTIFY_ON_DUPLICATES, false); - - RecordingMembershipTrackingPolicy nonDuplicateTrackingPolicy = group.addPolicy(nonDuplicateTrackingPolicySpec); - group.addPolicy(nonDuplicateTrackingPolicy); - nonDuplicateTrackingPolicy.setGroup(group); + .configure(AbstractMembershipTrackingPolicy.NOTIFY_ON_DUPLICATES, false) + .configure("group", group)); e1.setAttribute(TestEntity.NAME, "myname"); @@ -161,6 +170,25 @@ public class MembershipTrackingPolicyTest { assertRecordsEventually(nonDuplicateTrackingPolicy, Record.newAdded(e1), Record.newChanged(e1), Record.newChanged(e1)); } + // NOTIFY_ON_DUPLICATES==true is default + @Test + public void testNotifiedOfExtraTrackedSensorsIfDuplicate() throws Exception { + TestEntity e1 = createAndManageChildOf(group); + + RecordingMembershipTrackingPolicy nonDuplicateTrackingPolicy = app.addPolicy(PolicySpec.create(RecordingMembershipTrackingPolicy.class) + .configure(AbstractMembershipTrackingPolicy.SENSORS_TO_TRACK, ImmutableSet.>of(TestEntity.NAME)) + .configure("group", group)); + + e1.setAttribute(TestEntity.NAME, "myname"); + assertRecordsEventually(nonDuplicateTrackingPolicy, Record.newAdded(e1), Record.newChanged(e1)); + + e1.setAttribute(TestEntity.NAME, "myname"); + assertRecordsEventually(nonDuplicateTrackingPolicy, Record.newAdded(e1), Record.newChanged(e1), Record.newChanged(e1)); + + e1.setAttribute(TestEntity.NAME, "mynewname"); + assertRecordsEventually(nonDuplicateTrackingPolicy, Record.newAdded(e1), Record.newChanged(e1), Record.newChanged(e1), Record.newChanged(e1)); + } + private void assertRecordsEventually(final Record... expected) { assertRecordsEventually(policy, expected); } @@ -176,7 +204,7 @@ public class MembershipTrackingPolicyTest { for (List validExpected : validExpecteds) { if (policy.records.equals(validExpected)) return; } - fail("actual="+policy.records+"; valid: "+validExpecteds); + fail("actual="+policy.records+"; valid: "+Arrays.toString(validExpecteds)); }}); }