You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/08/30 01:01:25 UTC
[25/26] git commit: address svet code review comments
address svet code review comments
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/225f1820
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/225f1820
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/225f1820
Branch: refs/heads/master
Commit: 225f182078b700ab6282a3aac000d3d304870bdd
Parents: 6a8af41
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri Aug 29 18:35:08 2014 -0400
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri Aug 29 18:35:08 2014 -0400
----------------------------------------------------------------------
.../brooklyn/enricher/basic/AbstractAggregator.java | 2 +-
.../basic/AbstractMultipleSensorAggregator.java | 11 ++++++++---
.../main/java/brooklyn/enricher/basic/Aggregator.java | 12 ++++++++----
.../main/java/brooklyn/entity/basic/AbstractEntity.java | 2 +-
core/src/main/java/brooklyn/entity/basic/Lifecycle.java | 7 -------
.../java/brooklyn/entity/basic/ServiceStateLogic.java | 2 +-
.../policy/loadbalancing/LoadBalancingPolicyTest.java | 7 +++----
.../java/brooklyn/util/collections/MutableList.java | 8 ++++++++
.../main/java/brooklyn/util/collections/MutableMap.java | 5 +++++
.../main/java/brooklyn/util/collections/MutableSet.java | 6 ++++++
.../src/main/java/brooklyn/util/guava/IfFunctions.java | 6 +++---
.../test/java/brooklyn/util/guava/IfFunctionsTest.java | 2 +-
12 files changed, 45 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java b/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java
index ba7762c..9568332 100644
--- a/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java
+++ b/core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java
@@ -85,7 +85,7 @@ public abstract class AbstractAggregator<T,U> extends AbstractEnricher implement
if (fromHardcodedProducers == null && producer == null) producer = entity;
checkState(fromHardcodedProducers != null ^ producer != null, "must specify one of %s (%s) or %s (%s)",
PRODUCER.getName(), producer, FROM_HARDCODED_PRODUCERS.getName(), fromHardcodedProducers);
- checkState(producer != null ? (Boolean.TRUE.equals(fromMembers) || Boolean.TRUE.equals(fromChildren)) : true,
+ checkState(producer == null || Boolean.TRUE.equals(fromMembers) || Boolean.TRUE.equals(fromChildren),
"when specifying producer, must specify at least one of fromMembers (%s) or fromChildren (%s)", fromMembers, fromChildren);
if (fromHardcodedProducers != null) {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java b/core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java
index 85c36d8..07b0477 100644
--- a/core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java
+++ b/core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java
@@ -55,6 +55,7 @@ public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggreg
Preconditions.checkNotNull(getSourceSensors(), "sourceSensors must be set");
}
+ @Override
protected void setEntityBeforeSubscribingProducerChildrenEvents() {
if (LOG.isDebugEnabled()) LOG.debug("{} subscribing to children of {}", new Object[] {this, producer });
for (Sensor<?> sourceSensor: getSourceSensors()) {
@@ -62,6 +63,7 @@ public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggreg
}
}
+ @Override
protected void addProducerHardcoded(Entity producer) {
for (Sensor<?> sourceSensor: getSourceSensors()) {
subscribe(producer, sourceSensor, this);
@@ -69,17 +71,18 @@ public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggreg
onProducerAdded(producer);
}
+ @Override
protected void addProducerChild(Entity producer) {
- // not required due to subscribeToChildren call
-// subscribe(producer, sourceSensor, this);
+ // no `subscribe` call needed here, due to previous subscribeToChildren call
onProducerAdded(producer);
}
+ @Override
protected void addProducerMember(Entity producer) {
addProducerHardcoded(producer);
}
-
+ @Override
protected void onProducerAdded(Entity producer) {
if (LOG.isDebugEnabled()) LOG.debug("{} listening to {}", new Object[] {this, producer});
synchronized (values) {
@@ -106,6 +109,7 @@ public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggreg
}
}
+ @Override
protected void onProducerRemoved(Entity producer) {
synchronized (values) {
for (Sensor<?> sensor: getSourceSensors()) {
@@ -140,5 +144,6 @@ public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggreg
}
}
+ @Override
protected abstract Object compute();
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/core/src/main/java/brooklyn/enricher/basic/Aggregator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/enricher/basic/Aggregator.java b/core/src/main/java/brooklyn/enricher/basic/Aggregator.java
index 034a604..e865799 100644
--- a/core/src/main/java/brooklyn/enricher/basic/Aggregator.java
+++ b/core/src/main/java/brooklyn/enricher/basic/Aggregator.java
@@ -70,29 +70,31 @@ public class Aggregator<T,U> extends AbstractAggregator<T,U> implements SensorEv
this.transformation = (Function<? super Collection<T>, ? extends U>) getRequiredConfig(TRANSFORMATION);
}
-
+ @Override
protected void setEntityBeforeSubscribingProducerChildrenEvents() {
if (LOG.isDebugEnabled()) LOG.debug("{} subscribing to children of {}", new Object[] {this, producer });
subscribeToChildren(producer, sourceSensor, this);
}
+ @Override
protected void addProducerHardcoded(Entity producer) {
subscribe(producer, sourceSensor, this);
onProducerAdded(producer);
}
+ @Override
protected void addProducerChild(Entity producer) {
- // not required due to subscribeToChildren call
-// subscribe(producer, sourceSensor, this);
+ // no subscription needed here, due to the subscribeToChildren call
onProducerAdded(producer);
}
+ @Override
protected void addProducerMember(Entity producer) {
subscribe(producer, sourceSensor, this);
onProducerAdded(producer);
}
-
+ @Override
protected void onProducerAdded(Entity producer) {
if (LOG.isDebugEnabled()) LOG.debug("{} listening to {}", new Object[] {this, producer});
synchronized (values) {
@@ -115,6 +117,7 @@ public class Aggregator<T,U> extends AbstractAggregator<T,U> implements SensorEv
}
}
+ @Override
protected void onProducerRemoved(Entity producer) {
values.remove(producer);
onUpdated();
@@ -142,6 +145,7 @@ public class Aggregator<T,U> extends AbstractAggregator<T,U> implements SensorEv
}
}
+ @Override
protected Object compute() {
synchronized (values) {
// TODO Could avoid copying when filter not needed
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
index 5e21fd7..ec2d5e9 100644
--- a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
+++ b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
@@ -1058,7 +1058,7 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E
// these enrichers do nothing unless Attributes.SERVICE_NOT_UP_INDICATORS are used
// and/or SERVICE_STATE_EXPECTED
protected void initEnrichers() {
- addEnricher(ServiceNotUpLogic.newEnricherForServiceUpIfNoNotUpIndicators());
+ addEnricher(ServiceNotUpLogic.newEnricherForServiceUpIfNotUpIndicatorsEmpty());
addEnricher(ServiceStateLogic.newEnricherForServiceStateFromProblemsAndUp());
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/core/src/main/java/brooklyn/entity/basic/Lifecycle.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/Lifecycle.java b/core/src/main/java/brooklyn/entity/basic/Lifecycle.java
index 5e7d0fe..26d3916 100644
--- a/core/src/main/java/brooklyn/entity/basic/Lifecycle.java
+++ b/core/src/main/java/brooklyn/entity/basic/Lifecycle.java
@@ -46,15 +46,8 @@ public enum Lifecycle {
* When this completes the entity will normally transition to
* {@link Lifecycle#RUNNING}.
*/
-// * {@link Lifecycle#STARTED} or
STARTING,
-// /**
-// * The entity has been started and no further start-up steps are needed from the management plane,
-// * but the entity has not yet been confirmed as running.
-// */
-// STARTED,
-//
/**
* The entity service is expected to be running. In healthy operation, {@link Attributes#SERVICE_UP} will be true,
* or will shortly be true if all service start actions have been completed and we are merely waiting for it to be running.
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java b/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java
index 5154bff..95d5e57 100644
--- a/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java
+++ b/core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java
@@ -136,7 +136,7 @@ public class ServiceStateLogic {
private ServiceNotUpLogic() {}
@SuppressWarnings({ "unchecked", "rawtypes" })
- public static final EnricherSpec<?> newEnricherForServiceUpIfNoNotUpIndicators() {
+ public static final EnricherSpec<?> newEnricherForServiceUpIfNotUpIndicatorsEmpty() {
return Enrichers.builder()
.transforming(SERVICE_NOT_UP_INDICATORS).publishing(Attributes.SERVICE_UP)
.computing( /* cast hacks to support removing */ (Function)
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/policy/src/test/java/brooklyn/policy/loadbalancing/LoadBalancingPolicyTest.java
----------------------------------------------------------------------
diff --git a/policy/src/test/java/brooklyn/policy/loadbalancing/LoadBalancingPolicyTest.java b/policy/src/test/java/brooklyn/policy/loadbalancing/LoadBalancingPolicyTest.java
index c0ff7b7..e0a720b 100644
--- a/policy/src/test/java/brooklyn/policy/loadbalancing/LoadBalancingPolicyTest.java
+++ b/policy/src/test/java/brooklyn/policy/loadbalancing/LoadBalancingPolicyTest.java
@@ -27,8 +27,6 @@ import brooklyn.entity.basic.Entities;
import brooklyn.entity.basic.EntityLocal;
import brooklyn.test.Asserts;
import brooklyn.util.collections.MutableMap;
-import brooklyn.util.time.Duration;
-import brooklyn.util.time.Time;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -235,7 +233,7 @@ public class LoadBalancingPolicyTest extends AbstractLoadBalancingPolicyTest {
ImmutableList.of(item1, item2, item3, item4, item5, item6),
ImmutableList.of(30d, 30d));
- MockItemEntity item7 = newItem(app, containerA, "7", 40);
+ newItem(app, containerA, "7", 40);
assertWorkratesEventually(
ImmutableList.of(containerA, containerB),
@@ -305,6 +303,7 @@ public class LoadBalancingPolicyTest extends AbstractLoadBalancingPolicyTest {
ImmutableList.of(40d, 40d));
}
+ @SuppressWarnings({ "unchecked", "rawtypes" })
@Test
public void testModelIncludesItemsAndContainersStartedBeforePolicyCreated() {
pool.removePolicy(policy);
@@ -312,7 +311,7 @@ public class LoadBalancingPolicyTest extends AbstractLoadBalancingPolicyTest {
// Set-up containers and items.
final MockContainerEntity containerA = newContainer(app, "A", 10, 100);
- MockItemEntity item1 = newItem(app, containerA, "1", 10);
+ newItem(app, containerA, "1", 10);
policy = new LoadBalancingPolicy(MutableMap.of(), TEST_METRIC, model);
pool.addPolicy(policy);
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/utils/common/src/main/java/brooklyn/util/collections/MutableList.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/collections/MutableList.java b/utils/common/src/main/java/brooklyn/util/collections/MutableList.java
index a6638da..1c1b2d2 100644
--- a/utils/common/src/main/java/brooklyn/util/collections/MutableList.java
+++ b/utils/common/src/main/java/brooklyn/util/collections/MutableList.java
@@ -29,6 +29,8 @@ import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import brooklyn.util.exceptions.Exceptions;
+
import com.google.common.collect.ImmutableList;
public class MutableList<V> extends ArrayList<V> {
@@ -84,17 +86,23 @@ public class MutableList<V> extends ArrayList<V> {
public ImmutableList<V> toImmutable() {
return ImmutableList.copyOf(this);
}
+ /** creates an {@link ImmutableList} which is a copy of this list. note that the list should not contain nulls. */
public List<V> asImmutableCopy() {
try {
return ImmutableList.copyOf(this);
} catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
log.warn("Error converting list to Immutable, using unmodifiable instead: "+e, e);
return asUnmodifiableCopy();
}
}
+ /** creates a {@link Collections#unmodifiableList(List)} wrapper around this list. the method is efficient,
+ * as there is no copying, but the returned view might change if the list here is changed. */
public List<V> asUnmodifiable() {
return Collections.unmodifiableList(this);
}
+ /** creates a {@link Collections#unmodifiableList(List)} of a copy of this list.
+ * the returned item is immutable, but unlike {@link #asImmutableCopy()} nulls are permitted. */
public List<V> asUnmodifiableCopy() {
return Collections.unmodifiableList(MutableList.copyOf(this));
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/utils/common/src/main/java/brooklyn/util/collections/MutableMap.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/collections/MutableMap.java b/utils/common/src/main/java/brooklyn/util/collections/MutableMap.java
index 2630dbf..f8b2f6c 100644
--- a/utils/common/src/main/java/brooklyn/util/collections/MutableMap.java
+++ b/utils/common/src/main/java/brooklyn/util/collections/MutableMap.java
@@ -29,6 +29,7 @@ import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.guava.Maybe;
import com.google.common.base.Predicate;
@@ -146,17 +147,21 @@ public class MutableMap<K,V> extends LinkedHashMap<K,V> {
public ImmutableMap<K,V> toImmutable() {
return ImmutableMap.copyOf(this);
}
+ /** as {@link MutableList#asImmutableCopy()} */
public Map<K,V> asImmutableCopy() {
try {
return ImmutableMap.copyOf(this);
} catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
log.warn("Error converting list to Immutable, using unmodifiable instead: "+e, e);
return asUnmodifiableCopy();
}
}
+ /** as {@link MutableList#asUnmodifiable()} */
public Map<K,V> asUnmodifiable() {
return Collections.unmodifiableMap(this);
}
+ /** as {@link MutableList#asUnmodifiableCopy()} */
public Map<K,V> asUnmodifiableCopy() {
return Collections.unmodifiableMap(MutableMap.copyOf(this));
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/utils/common/src/main/java/brooklyn/util/collections/MutableSet.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/collections/MutableSet.java b/utils/common/src/main/java/brooklyn/util/collections/MutableSet.java
index 0653cfd..e795158 100644
--- a/utils/common/src/main/java/brooklyn/util/collections/MutableSet.java
+++ b/utils/common/src/main/java/brooklyn/util/collections/MutableSet.java
@@ -29,6 +29,8 @@ import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import brooklyn.util.exceptions.Exceptions;
+
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
@@ -79,17 +81,21 @@ public class MutableSet<V> extends LinkedHashSet<V> {
// Don't use ImmutableSet as that does not accept nulls
return Collections.unmodifiableSet(Sets.newLinkedHashSet(this));
}
+ /** as {@link MutableList#asImmutableCopy()()} */
public Set<V> asImmutableCopy() {
try {
return ImmutableSet.copyOf(this);
} catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
log.warn("Error converting list to Immutable, using unmodifiable instead: "+e, e);
return asUnmodifiableCopy();
}
}
+ /** as {@link MutableList#asUnmodifiable()} */
public Set<V> asUnmodifiable() {
return Collections.unmodifiableSet(this);
}
+ /** as {@link MutableList#asUnmodifiableCopy()} */
public Set<V> asUnmodifiableCopy() {
return Collections.unmodifiableSet(MutableSet.copyOf(this));
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java b/utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java
index 5384436..ceea8c3 100644
--- a/utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java
+++ b/utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java
@@ -38,13 +38,13 @@ public class IfFunctions {
return new IfFunctionBuilder<I,O>();
}
- public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
+ public static <I> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
return new IfFunctionBuilderApplyingFirst<I>(test);
}
- public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
+ public static <I> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
return ifPredicate(Predicates.equalTo(test));
}
- public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
+ public static <I> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
return ifPredicate(Predicates.not(Predicates.equalTo(test)));
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/225f1820/utils/common/src/test/java/brooklyn/util/guava/IfFunctionsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/brooklyn/util/guava/IfFunctionsTest.java b/utils/common/src/test/java/brooklyn/util/guava/IfFunctionsTest.java
index 74a5d5b..e52dc34 100644
--- a/utils/common/src/test/java/brooklyn/util/guava/IfFunctionsTest.java
+++ b/utils/common/src/test/java/brooklyn/util/guava/IfFunctionsTest.java
@@ -67,7 +67,7 @@ public class IfFunctionsTest {
@Test
public void testWithCast() {
- Function<Boolean, String> f = IfFunctions.<Boolean,String>ifEquals(false).value("F").ifEquals(true).value("T").defaultValue("?").build();
+ Function<Boolean, String> f = IfFunctions.ifEquals(false).value("F").ifEquals(true).value("T").defaultValue("?").build();
checkTF(f, "?");
}