brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From henev...@apache.org
Subject [25/26] git commit: address svet code review comments
Date Fri, 29 Aug 2014 23:01:25 GMT
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 <alex.heneveld@cloudsoftcorp.com>
Authored: Fri Aug 29 18:35:08 2014 -0400
Committer: Alex Heneveld <alex.heneveld@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, "?");
     }
 


Mime
View raw message