You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2014/08/27 08:42:17 UTC

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

GitHub user ahgittin opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/131

    Sensors chained for service up

    This implements the use of service not-up indicators and problem indicators, as discussed on the ML.
    
    See `service-state.md` for docs and `ServiceStateLogicTest` for detailed explanation and checking of subtleties.
    
    The material differences in behaviour are surprisingly few (though in the PR you will notice a lot of changes to tests; most test changes are uninteresting!).  The main functional differences, in addition to dynamic updating, are:
    
    * this removes a lot of ad hoc service-up computations; some quorum up logic e.g. for mongo,  has changed, but this was all quite hokey, we should replace this with quorum keys in cluster
    * this tweaks a recent workaround where AbstractEntity.add{Enricher,Policy} detects a duplicate, we now take the more recent one rather than the older one; this allows us to replace them in a principled fashion when using unique tags.
    * ServiceFailureDetector is now very different, and extends ServiceStateLogic (which previously is somewhat duplicated in the very occasional places it was used); if you were relying on this, or your apps start acting oddly, please let me know!
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/incubator-brooklyn sensors-chained-for-service-up

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/131.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #131
    
----
commit 238fab7f0f520a462175ca88b2a807c081e32145
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-06T15:16:23Z

    prototype of service_up being derived from service_not_up_indicators

commit 45e3035de648e32487535bf7a8050d5b3926b057
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-06T19:38:40Z

    add WhenFunctions which allows `Functionals.when(false).value("F")` approach to building up functions

commit a8bff36ec5cbaaa4c4f10d0adfd464ba8d75b8a7
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-06T21:05:41Z

    add a new "updatingMap" enricher which helps when multiple enrichers contribute to a map, and use it for the new chaining of service_up behaviour

commit 141751b9213a28943b90e767349d11973a278083
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-07T02:04:45Z

    minor test utils fixes, including `assertThat`

commit 3c714ed12259c3909f0244f83f504e6a62be7bdb
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-07T02:09:15Z

    refactor Enrichers.builder() types to give correct generics for published sensor

commit 41deca4d9428867d9c1d54ec3e5334fe183cfea2
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-07T02:38:11Z

    add support for suppressing duplicates in enrichers, defaulting false for most, but true for UpdatingMap; and in Enrichers.builder() use better superclasses to support tags, uniqueTags

commit 843f1fcaa28331e5b3e9b3325899366d0588601f
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-07T03:47:59Z

    simple cleaned up illustration of service_up from not_up computation

commit 94184b2f65bfe8a00fd6fe170f22c276ad9b29db
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-07T03:48:26Z

    a bunch more service-up checks (WIP)

commit c1ebb8f44cb3baa24f05021b262b9bf25c864f39
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-08T03:37:51Z

    fix enricher type test with suppress duplicates

commit bc1d19f755bc32a5ed5ec84ba50198fa39ba47bb
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-12T14:50:46Z

    rename WhenFunctions class to IfFunctions, and tidies/cleanups there and in enrichers

commit 97eed6bdc159ddcba231fc0183635317101ec7b9
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-25T06:23:01Z

    add SERVICE_PROBLEMS and enrichers in ServiceStateLogic to compute state from that.
    add enrichers to populate PROBLEMS and NOT_UP_INDICATORS from children and members: this removes a lot of ad hoc service-up computations; some quorum up logic e.g. for mongo, have changed, but in those cases the previous logic was quite hokey in any case. (to properly support QUORUM_SIZE we should have a key, or use the UP_QUORUM_CHECK config).
    also tweak the recent workaround when AbstractEntity.add{Enricher,Policy} detects a duplicate, we now take the more recent one rather than the older one; this allows us to replace them in a principled fashion when using unique tags.

commit d3886a055353f2efa435c84d45668c1a3ab0aa23
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-25T09:48:19Z

    default children/members enrichers for service_up and service_problems ignores entities that report null as well as those that don't report; this can be overridden with IGNORE_{NULL,TRANSITIONING} config keys.
    and the load balancing policy defaults not to blocking service up simply because children/members are not present. misc other tidies around policies.

commit b2daedf8336a891167602f67e7f8e576256dc5e8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-25T20:17:53Z

    rewrite of ServiceFailureDetector -- part of what it did before is now done by ServiceStateLogic of course,
    so now this extends that, and provides options for emitting notifications of ENTITY_FAILED, ENTITY_RECOVERED, as well as suppressing ON_FIRE

commit 5ec0a00b1c9deb4ff3b7ccc4dc7e7e6755fded26
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-25T21:29:52Z

    use initApp for apps to ensure enrichers get added, fixing tests and other misc tidies

commit bfb8737e647357053f65d5bd305a4f6783ad2866
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-25T21:35:55Z

    fix failing test on service going on_fire not aborting dependent configuration

commit cbc103a3b64cbcc7bcc9530cdd1914606e5b2520
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-27T06:16:18Z

    some notes on tidying the REST API

commit 5db500e43fbc1973d5b941ff41f681e51c134217
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-26T13:46:02Z

    fix all tests and related entities to get service lifecycle, including REST app creation attaching all interfaces (previously it did not get "startable" so entities did not start)

commit 31c5a0c2fd15ec23904bf438ba7b39579064dc8c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-27T03:10:47Z

    write comprehensive tests for ServiceStateLogic, and tidy some of the semantics and usages elsewhere

commit ab1381b413543b69de62b498fe61f05b46cf9cae
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-27T03:17:20Z

    make consistent `Mutable{List,Set,Map}.as{Immutable,Unmodifiable,UnmodifiableCopy}()` methods, and more forgiving if null is present, deprecatign the inconsistent and break-prone `toImmutable()`

commit 099a5496e4ec48bb31d11a3be82c92831aef2ceb
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-27T05:10:08Z

    convert SERVICE_STATE to _ACTUAL or _EXPECTED in many more places, and force recompute of _ACTUAL whenever _EXPECTED is changed

commit 6a8af41e70ab96e361fb58953c2c3f072ff12678
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-08-26T13:46:48Z

    docs for service state

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16906544
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java ---
    @@ -0,0 +1,215 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import static com.google.common.base.Preconditions.checkState;
    +
    +import java.util.Set;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.AbstractEntity;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.trait.Changeable;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.guava.Maybe;
    +
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.collect.Iterables;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Abstract superclass for enrichers which aggregate from children and/or members */
    +@SuppressWarnings("serial")
    +public abstract class AbstractAggregator<T,U> extends AbstractEnricher implements SensorEventListener<T> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractAggregator.class);
    +
    +    public static final ConfigKey<Entity> PRODUCER = ConfigKeys.newConfigKey(Entity.class, "enricher.producer", "The entity whose children/members will be aggregated");
    +
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +
    +    // FIXME this is not just for "members" i think -Alex
    --- End diff --
    
    Easier to rename the config now than once merged. Or is something else involved as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923536
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java ---
    @@ -0,0 +1,546 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.basic;
    +
    +import java.util.Collection;
    +import java.util.Date;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +
    +import javax.annotation.Nullable;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.enricher.Enrichers;
    +import brooklyn.enricher.basic.AbstractEnricher;
    +import brooklyn.enricher.basic.AbstractMultipleSensorAggregator;
    +import brooklyn.enricher.basic.UpdatingMap;
    +import brooklyn.entity.Effector;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.Lifecycle.Transition;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.policy.Enricher;
    +import brooklyn.policy.EnricherSpec;
    +import brooklyn.policy.EnricherSpec.ExtensibleEnricherSpec;
    +import brooklyn.util.collections.CollectionFunctionals;
    +import brooklyn.util.collections.MutableList;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.collections.MutableSet;
    +import brooklyn.util.guava.Functionals;
    +import brooklyn.util.text.Strings;
    +
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableSet;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Logic, sensors and enrichers, and conveniences, for computing service status */ 
    +public class ServiceStateLogic {
    +
    +    private static final Logger log = LoggerFactory.getLogger(ServiceStateLogic.class);
    +    
    +    public static final AttributeSensor<Boolean> SERVICE_UP = Attributes.SERVICE_UP;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_NOT_UP_INDICATORS = Attributes.SERVICE_NOT_UP_INDICATORS;
    +    
    +    public static final AttributeSensor<Lifecycle> SERVICE_STATE_ACTUAL = Attributes.SERVICE_STATE_ACTUAL;
    +    public static final AttributeSensor<Lifecycle.Transition> SERVICE_STATE_EXPECTED = Attributes.SERVICE_STATE_EXPECTED;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_PROBLEMS = Attributes.SERVICE_PROBLEMS;
    +
    +    /** static only; not for instantiation */
    +    private ServiceStateLogic() {}
    +
    +    public static <TKey,TVal> TVal getMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +        if (map==null) return null;
    +        return map.get(key);
    +    }
    +    
    +    @SuppressWarnings("unchecked")
    +    public static <TKey,TVal> void clearMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        updateMapSensorEntry(entity, sensor, key, (TVal)Entities.REMOVE);
    +    }
    +
    +    /** update the given key in the given map sensor */
    +    public static <TKey,TVal> void updateMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key, TVal v) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +
    +        // TODO synchronize
    +        
    +        boolean created = (map==null);
    +        if (created) map = MutableMap.of();
    +                
    +        boolean changed;
    +        if (v == Entities.REMOVE) {
    +            changed = map.containsKey(key);
    +            if (changed)
    +                map.remove(key);
    +        } else {
    +            TVal oldV = map.get(key);
    +            if (oldV==null)
    +                changed = (v!=null || !map.containsKey(key));
    +            else
    +                changed = !oldV.equals(v);
    +            if (changed)
    +                map.put(key, (TVal)v);
    +        }
    +        if (changed || created)
    +            entity.setAttribute(sensor, map);
    +    }
    +    
    +    public static void setExpectedState(Entity entity, Lifecycle state) {
    +        ((EntityInternal)entity).setAttribute(Attributes.SERVICE_STATE_EXPECTED, new Lifecycle.Transition(state, new Date()));
    +        
    +        Enricher enricher = EntityAdjuncts.findWithUniqueTag(entity.getEnrichers(), ComputeServiceState.DEFAULT_ENRICHER_UNIQUE_TAG);
    +        if (enricher instanceof ComputeServiceState) {
    +            ((ComputeServiceState)enricher).onEvent(null);
    +        }
    +    }
    +    public static Lifecycle getExpectedState(Entity entity) {
    +        Transition expected = entity.getAttribute(Attributes.SERVICE_STATE_EXPECTED);
    +        if (expected==null) return null;
    +        return expected.getState();
    +    }
    +    public static boolean isExpectedState(Entity entity, Lifecycle state) {
    +        return getExpectedState(entity)==state;
    +    }
    +    
    +    public static class ServiceNotUpLogic {
    +        public static final String DEFAULT_ENRICHER_UNIQUE_TAG = "service.isUp if no service.notUp.indicators";
    +        
    +        /** static only; not for instantiation */
    +        private ServiceNotUpLogic() {}
    +        
    +        @SuppressWarnings({ "unchecked", "rawtypes" })
    +        public static final EnricherSpec<?> newEnricherForServiceUpIfNoNotUpIndicators() {
    --- End diff --
    
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893971
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java ---
    @@ -0,0 +1,144 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.entity.Entity;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableMap;
    +
    +/** Building on {@link AbstractAggregator} for a single source sensor (on multiple children and/or members) */
    +public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggregator<Object,U> implements SensorEventListener<Object> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractMultipleSensorAggregator.class);
    +
    +    
    +    /** access via {@link #getValues(Sensor)} */
    +    private final Map<String, Map<Entity,Object>> values = Collections.synchronizedMap(new LinkedHashMap<String, Map<Entity,Object>>());
    +
    +    public AbstractMultipleSensorAggregator() {}
    +
    +    protected abstract Collection<Sensor<?>> getSourceSensors();
    +    
    +    @Override
    +    protected void setEntityLoadingConfig() {
    +        super.setEntityLoadingConfig();
    +        Preconditions.checkNotNull(getSourceSensors(), "sourceSensors must be set");
    +    }
    +    
    +    protected void setEntityBeforeSubscribingProducerChildrenEvents() {
    +        if (LOG.isDebugEnabled()) LOG.debug("{} subscribing to children of {}", new Object[] {this, producer });
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribeToChildren(producer, sourceSensor, this);
    +        }
    +    }
    +
    +    protected void addProducerHardcoded(Entity producer) {
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribe(producer, sourceSensor, this);
    +        }
    +        onProducerAdded(producer);
    +    }
    +
    +    protected void addProducerChild(Entity producer) {
    --- End diff --
    
    Good to have @Override annotation. Same for the other methods (addProducerHardcoded and addProducerMember) in this and other subclasses of AbstractAggregator.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923689
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/AbstractApplication.java ---
    @@ -174,18 +194,20 @@ public void postStart(Collection<? extends Location> locations) {
         public void stop() {
             logApplicationLifecycle("Stopping");
     
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL, "Application stopping");
             setAttribute(SERVICE_UP, false);
    -        setAttribute(Attributes.SERVICE_STATE, Lifecycle.STOPPING);
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
             recordApplicationEvent(Lifecycle.STOPPING);
             try {
                 doStop();
             } catch (Exception e) {
    -            setAttribute(Attributes.SERVICE_STATE, Lifecycle.ON_FIRE);
    +            ServiceStateLogic.setExpectedState(this, Lifecycle.ON_FIRE);
    --- End diff --
    
    setting expected state is also valid.  it just means it will get cleared on a stop or start/restart.
    
    still not sure what best way is, in other places you may have seen, i used the `START` effector as a problem indicator, and cleared it when starting/stopping.  neither felt perfect, tho neither felt that bad.  thoughts?  (happy to leave for subsequent PR.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923699
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/Lifecycle.java ---
    @@ -51,12 +40,21 @@
     
         /**
          * The entity is starting.
    -     *
    -     * This stage is entered when the {@link brooklyn.entity.trait.Startable#START} {@link brooklyn.entity.Effector} is called. 
    -     * The entity will have its location set and and setup helper object created.
    +     * <p>
    +     * This stage is typically entered when the {@link brooklyn.entity.trait.Startable#START} {@link brooklyn.entity.Effector} 
    +     * is called, to undertake the startup operations from the management plane.
    +     * 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,
    +//
    --- End diff --
    
    ah yes missed this, thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893928
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java ---
    @@ -0,0 +1,144 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.entity.Entity;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableMap;
    +
    +/** Building on {@link AbstractAggregator} for a single source sensor (on multiple children and/or members) */
    +public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggregator<Object,U> implements SensorEventListener<Object> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractMultipleSensorAggregator.class);
    +
    +    
    +    /** access via {@link #getValues(Sensor)} */
    +    private final Map<String, Map<Entity,Object>> values = Collections.synchronizedMap(new LinkedHashMap<String, Map<Entity,Object>>());
    +
    +    public AbstractMultipleSensorAggregator() {}
    +
    +    protected abstract Collection<Sensor<?>> getSourceSensors();
    +    
    +    @Override
    +    protected void setEntityLoadingConfig() {
    +        super.setEntityLoadingConfig();
    +        Preconditions.checkNotNull(getSourceSensors(), "sourceSensors must be set");
    +    }
    +    
    +    protected void setEntityBeforeSubscribingProducerChildrenEvents() {
    +        if (LOG.isDebugEnabled()) LOG.debug("{} subscribing to children of {}", new Object[] {this, producer });
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribeToChildren(producer, sourceSensor, this);
    +        }
    +    }
    +
    +    protected void addProducerHardcoded(Entity producer) {
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribe(producer, sourceSensor, this);
    +        }
    +        onProducerAdded(producer);
    +    }
    +
    +    protected void addProducerChild(Entity producer) {
    +        // not required due to subscribeToChildren call
    +//        subscribe(producer, sourceSensor, this);
    --- End diff --
    
    Not sure if this is a note to self or explicitly left for future generations :). Either delete or fix comment alignment. Same case in Aggregator.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/131


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923213
  
    --- Diff: utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java ---
    @@ -0,0 +1,158 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.util.guava;
    +
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.base.Supplier;
    +
    +/** Utilities for building {@link Function} instances which return specific values
    + * (or {@link Supplier} or {@link Function} instances) when certain predicates are satisfied,
    + * tested in order and returning the first matching,
    + * with support for an "else" default value if none are satisfied (null by default). */
    +public class IfFunctions {
    +
    +    public static <I,O> IfFunctionBuilder<I,O> newInstance(Class<I> testType, Class<O> returnType) {
    +        return new IfFunctionBuilder<I,O>();
    +    }
    +    
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
    +        return new IfFunctionBuilderApplyingFirst<I>(test);
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
    +        return ifPredicate(Predicates.equalTo(test));
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
    +        return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +    }
    +    
    +    @Beta
    +    public static class IfFunction<I,O> implements Function<I,O> {
    +        protected final Map<Predicate<? super I>,Function<? super I,? extends O>> tests = new LinkedHashMap<Predicate<? super I>,Function<? super I,? extends O>>();
    +        protected Function<? super I,? extends O> defaultFunction = null;
    --- End diff --
    
    the `toString()` check treats null different from an explicit `Function.constant(null)` and I think that's useful


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16924107
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/AbstractApplication.java ---
    @@ -123,23 +137,29 @@ public AbstractApplication setParent(Entity parent) {
         public void start(Collection<? extends Location> locations) {
             this.addLocations(locations);
             Collection<? extends Location> locationsToUse = getLocations();
    -        setAttribute(Attributes.SERVICE_STATE, Lifecycle.STARTING);
    +        ServiceProblemsLogic.clearProblemsIndicator(this, START);
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL, "Application starting");
             recordApplicationEvent(Lifecycle.STARTING);
             try {
                 preStart(locationsToUse);
    +            // if there are other items which should block service_up, they should be done in preStart
    +            ServiceStateLogic.ServiceNotUpLogic.clearNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL);
    +            
                 doStart(locationsToUse);
                 postStart(locationsToUse);
             } catch (Exception e) {
    -            setAttribute(Attributes.SERVICE_STATE, Lifecycle.ON_FIRE);
    +            // TODO should probably remember these problems then clear?  if so, do it here ... or on all effectors?
    +//            ServiceProblemsLogic.updateProblemsIndicator(this, START, e);
    +            
                 recordApplicationEvent(Lifecycle.ON_FIRE);
    --- End diff --
    
    we don't currently -- this assumes that a failure during start does not necessarily mean the application isn't running.  it's marked TODO while we better figure out the right logic.
    
    in concrete terms most failures are if a child fails to start.  the child is then on fire and so is the app.  if restarting the child fixes it then the app will also, correctly, be fixed.  however if the app has some other startup problem (eg locally) then it might not report the problem and incorrectly advertise health.
    
    let's continue to think on this, not try to solve all such issues in this PR


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16924014
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/UpdatingMap.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.flags.SetFromFlag;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Maps;
    +import com.google.common.reflect.TypeToken;
    +
    +/**
    + * Enricher which updates an entry in a sensor map ({@link #TARGET_SENSOR}) 
    + * based on the value of another sensor ({@link #SOURCE_SENSOR}.
    + * <p>
    + * The key used defaults to the name of the source sensor but can be specified with {@link #KEY_IN_TARGET_SENSOR}.
    + * The value placed in the map is the result of applying the function in {@link #COMPUTING} to the sensor value,
    + * with default behaviour being to remove an entry if <code>null</code> is returned
    + * but this can be overriden by setting {@link #REMOVING_IF_RESULT_IS_NULL} false.
    + * {@link Entities#REMOVE} and {@link Entities#UNCHANGED} are also respeced as return values for the computation
    + * (ignoring generics).
    + * Unlike most other enrichers, this defaults to {@link AbstractEnricher#SUPPRESS_DUPLICATES} being true
    + *  
    + * @author alex
    + *
    + * @param <S> source sensor type
    + * @param <TKey> key type in target sensor map
    + * @param <TVal> value type in target sensor map
    + */
    +@SuppressWarnings("serial")
    +public class UpdatingMap<S,TKey,TVal> extends AbstractEnricher implements SensorEventListener<S> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(UpdatingMap.class);
    +
    +    @SetFromFlag("fromSensor")
    +    public static final ConfigKey<Sensor<?>> SOURCE_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.sourceSensor");
    +    @SetFromFlag("targetSensor")
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +    @SetFromFlag("key")
    +    public static final ConfigKey<?> KEY_IN_TARGET_SENSOR = ConfigKeys.newConfigKey(Object.class, "enricher.updatingMap.keyInTargetSensor",
    +        "Key to update in the target sensor map, defaulting to the name of the source sensor");
    +    @SetFromFlag("computing")
    +    public static final ConfigKey<Function<?, ?>> COMPUTING = ConfigKeys.newConfigKey(new TypeToken<Function<?,?>>() {}, "enricher.updatingMap.computing");
    +    @SetFromFlag("removingIfResultIsNull")
    +    public static final ConfigKey<Boolean> REMOVING_IF_RESULT_IS_NULL = ConfigKeys.newBooleanConfigKey("enricher.updatingMap.removingIfResultIsNull", 
    +        "Whether the key in the target map is removed if the result if the computation is null");
    +
    +    protected AttributeSensor<S> sourceSensor;
    +    protected AttributeSensor<Map<TKey,TVal>> targetSensor;
    +    protected TKey key;
    +    protected Function<S,? extends TVal> computing;
    +    protected Boolean removingIfResultIsNull;
    +
    +    public UpdatingMap() {
    +        this(Maps.newLinkedHashMap());
    +    }
    +
    +    public UpdatingMap(Map<Object, Object> flags) {
    +        super(flags);
    +        if (suppressDuplicates==null) {
    +            // this defaults to suppressing duplicates
    +            suppressDuplicates = true;
    +        }
    +    }
    +
    +    @SuppressWarnings({ "unchecked", "rawtypes" })
    +    @Override
    +    public void setEntity(EntityLocal entity) {
    +        super.setEntity(entity);
    +        this.sourceSensor = (AttributeSensor<S>) getRequiredConfig(SOURCE_SENSOR);
    +        this.targetSensor = (AttributeSensor<Map<TKey,TVal>>) getRequiredConfig(TARGET_SENSOR);
    +        this.key = (TKey) getConfig(KEY_IN_TARGET_SENSOR);
    +        this.computing = (Function) getRequiredConfig(COMPUTING);
    +        this.removingIfResultIsNull = getConfig(REMOVING_IF_RESULT_IS_NULL);
    +
    +        subscribe(entity, sourceSensor, this);
    +        onUpdated();
    +    }
    +    
    +    @Override
    +    public void onEvent(SensorEvent<S> event) {
    +        onUpdated();
    +    }
    +
    +    /**
    +     * Called whenever the values for the set of producers changes (e.g. on an event, or on a member added/removed).
    +     */
    +    @SuppressWarnings("unchecked")
    +    protected void onUpdated() {
    --- End diff --
    
    don't see it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894482
  
    --- Diff: policy/src/test/java/brooklyn/policy/loadbalancing/LoadBalancingPolicyTest.java ---
    @@ -27,6 +27,8 @@
     import brooklyn.entity.basic.EntityLocal;
     import brooklyn.test.Asserts;
     import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.time.Duration;
    +import brooklyn.util.time.Time;
    --- End diff --
    
    Unused comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923826
  
    --- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/PolicyApi.java ---
    @@ -57,6 +57,7 @@
       @GET
       @Path("/current-state")
       @ApiOperation(value = "Fetch policy states in batch", notes="Returns a map of policy ID to whether it is active")
    +  // FIXME method name -- this is nothing to do with config!
    --- End diff --
    
    good points.  fwiw i've left these to come back to -- they were just comments i noted in passing and thought recording them was better than not


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923600
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java ---
    @@ -0,0 +1,144 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.entity.Entity;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableMap;
    +
    +/** Building on {@link AbstractAggregator} for a single source sensor (on multiple children and/or members) */
    +public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggregator<Object,U> implements SensorEventListener<Object> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractMultipleSensorAggregator.class);
    +
    +    
    +    /** access via {@link #getValues(Sensor)} */
    +    private final Map<String, Map<Entity,Object>> values = Collections.synchronizedMap(new LinkedHashMap<String, Map<Entity,Object>>());
    +
    +    public AbstractMultipleSensorAggregator() {}
    +
    +    protected abstract Collection<Sensor<?>> getSourceSensors();
    +    
    +    @Override
    +    protected void setEntityLoadingConfig() {
    +        super.setEntityLoadingConfig();
    +        Preconditions.checkNotNull(getSourceSensors(), "sourceSensors must be set");
    +    }
    +    
    +    protected void setEntityBeforeSubscribingProducerChildrenEvents() {
    +        if (LOG.isDebugEnabled()) LOG.debug("{} subscribing to children of {}", new Object[] {this, producer });
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribeToChildren(producer, sourceSensor, this);
    +        }
    +    }
    +
    +    protected void addProducerHardcoded(Entity producer) {
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribe(producer, sourceSensor, this);
    +        }
    +        onProducerAdded(producer);
    +    }
    +
    +    protected void addProducerChild(Entity producer) {
    +        // not required due to subscribeToChildren call
    +//        subscribe(producer, sourceSensor, this);
    --- End diff --
    
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923172
  
    --- Diff: utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java ---
    @@ -0,0 +1,158 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.util.guava;
    +
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.base.Supplier;
    +
    +/** Utilities for building {@link Function} instances which return specific values
    + * (or {@link Supplier} or {@link Function} instances) when certain predicates are satisfied,
    + * tested in order and returning the first matching,
    + * with support for an "else" default value if none are satisfied (null by default). */
    +public class IfFunctions {
    +
    +    public static <I,O> IfFunctionBuilder<I,O> newInstance(Class<I> testType, Class<O> returnType) {
    +        return new IfFunctionBuilder<I,O>();
    +    }
    +    
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
    +        return new IfFunctionBuilderApplyingFirst<I>(test);
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
    +        return ifPredicate(Predicates.equalTo(test));
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
    +        return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +    }
    +    
    +    @Beta
    +    public static class IfFunction<I,O> implements Function<I,O> {
    +        protected final Map<Predicate<? super I>,Function<? super I,? extends O>> tests = new LinkedHashMap<Predicate<? super I>,Function<? super I,? extends O>>();
    +        protected Function<? super I,? extends O> defaultFunction = null;
    +        
    +        protected IfFunction(IfFunction<I,O> input) {
    +            this.tests.putAll(input.tests);
    +            this.defaultFunction = input.defaultFunction;
    +        }
    +
    +        protected IfFunction() {
    +        }
    +        
    +        @Override
    +        public O apply(I input) {
    +            for (Map.Entry<Predicate<? super I>,Function<? super I,? extends O>> test: tests.entrySet()) {
    +                if (test.getKey().apply(input)) 
    +                    return test.getValue().apply(input);
    +            }
    +            return defaultFunction==null ? null : defaultFunction.apply(input);
    +        }
    +        
    +        @Override
    +        public String toString() {
    +            return "if["+tests+"]"+(defaultFunction!=null ? "-else["+defaultFunction+"]" : "");
    +        }
    +    }
    +    
    +    @Beta
    +    public static class IfFunctionBuilder<I,O> extends IfFunction<I,O> {
    +        protected IfFunctionBuilder() { super(); }
    +        protected IfFunctionBuilder(IfFunction<I,O> input) { super(input); }
    +        
    +        public IfFunction<I,O> build() {
    +            return new IfFunction<I,O>(this);
    +        }
    +        
    +        public IfFunctionBuilderApplying<I,O> ifPredicate(Predicate<I> test) {
    +            return new IfFunctionBuilderApplying<I,O>(this, (Predicate<I>)test);
    +        }
    +        public IfFunctionBuilderApplying<I,O> ifEquals(I test) {
    +            return ifPredicate(Predicates.equalTo(test));
    +        }
    +        public IfFunctionBuilderApplying<I,O> ifNotEquals(I test) {
    +            return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +        }
    +
    +        public IfFunctionBuilder<I,O> defaultValue(O defaultValue) {
    +            return defaultApply(new Functionals.ConstantFunction<I,O>(defaultValue, defaultValue));
    +        }
    +        @SuppressWarnings("unchecked")
    +        public IfFunctionBuilder<I,O> defaultGet(Supplier<? extends O> defaultSupplier) {
    +            return defaultApply((Function<I,O>)Functions.forSupplier(defaultSupplier));
    +        }
    +        public IfFunctionBuilder<I,O> defaultApply(Function<? super I,? extends O> defaultFunction) {
    +            IfFunctionBuilder<I, O> result = new IfFunctionBuilder<I,O>(this);
    +            result.defaultFunction = defaultFunction;
    +            return result;
    +        }
    +    }
    +
    +    @Beta
    +    public static class IfFunctionBuilderApplying<I,O> {
    +        private IfFunction<I, O> input;
    +        private Predicate<? super I> test;
    +        
    +        private IfFunctionBuilderApplying(IfFunction<I,O> input, Predicate<? super I> test) {
    +            this.input = input;
    +            this.test = test;
    +        }
    +        
    +        public IfFunctionBuilder<I,O> value(O value) {
    +            return apply(new Functionals.ConstantFunction<I,O>(value, value));
    +        }
    +        @SuppressWarnings("unchecked")
    +        public IfFunctionBuilder<I,O> get(Supplier<? extends O> supplier) {
    +            return apply((Function<I,O>)Functions.forSupplier(supplier));
    +        }
    +        public IfFunctionBuilder<I,O> apply(Function<? super I,? extends O> function) {
    +            IfFunctionBuilder<I, O> result = new IfFunctionBuilder<I,O>(input);
    +            result.tests.put(test, function);
    +            return result;
    +        }
    +    }
    +
    +    @Beta
    +    public static class IfFunctionBuilderApplyingFirst<I> {
    --- End diff --
    
    Not sure I follow.  The generics signature is very different.  Are you saying we should use a Void generic type or something and then switch the signature on some of the return types?
    
    Feel free to redo it if there's a cleaner way -- I'm kinda curious -- but don't think it's important.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893450
  
    --- Diff: utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java ---
    @@ -0,0 +1,158 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.util.guava;
    +
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.base.Supplier;
    +
    +/** Utilities for building {@link Function} instances which return specific values
    + * (or {@link Supplier} or {@link Function} instances) when certain predicates are satisfied,
    + * tested in order and returning the first matching,
    + * with support for an "else" default value if none are satisfied (null by default). */
    +public class IfFunctions {
    +
    +    public static <I,O> IfFunctionBuilder<I,O> newInstance(Class<I> testType, Class<O> returnType) {
    +        return new IfFunctionBuilder<I,O>();
    +    }
    +    
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
    +        return new IfFunctionBuilderApplyingFirst<I>(test);
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
    +        return ifPredicate(Predicates.equalTo(test));
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
    +        return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +    }
    +    
    +    @Beta
    +    public static class IfFunction<I,O> implements Function<I,O> {
    +        protected final Map<Predicate<? super I>,Function<? super I,? extends O>> tests = new LinkedHashMap<Predicate<? super I>,Function<? super I,? extends O>>();
    +        protected Function<? super I,? extends O> defaultFunction = null;
    +        
    +        protected IfFunction(IfFunction<I,O> input) {
    +            this.tests.putAll(input.tests);
    +            this.defaultFunction = input.defaultFunction;
    +        }
    +
    +        protected IfFunction() {
    +        }
    +        
    +        @Override
    +        public O apply(I input) {
    +            for (Map.Entry<Predicate<? super I>,Function<? super I,? extends O>> test: tests.entrySet()) {
    +                if (test.getKey().apply(input)) 
    +                    return test.getValue().apply(input);
    +            }
    +            return defaultFunction==null ? null : defaultFunction.apply(input);
    +        }
    +        
    +        @Override
    +        public String toString() {
    +            return "if["+tests+"]"+(defaultFunction!=null ? "-else["+defaultFunction+"]" : "");
    +        }
    +    }
    +    
    +    @Beta
    +    public static class IfFunctionBuilder<I,O> extends IfFunction<I,O> {
    --- End diff --
    
    I like the builder approach, should remember it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16907284
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/AbstractApplication.java ---
    @@ -123,23 +137,29 @@ public AbstractApplication setParent(Entity parent) {
         public void start(Collection<? extends Location> locations) {
             this.addLocations(locations);
             Collection<? extends Location> locationsToUse = getLocations();
    -        setAttribute(Attributes.SERVICE_STATE, Lifecycle.STARTING);
    +        ServiceProblemsLogic.clearProblemsIndicator(this, START);
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL, "Application starting");
             recordApplicationEvent(Lifecycle.STARTING);
             try {
                 preStart(locationsToUse);
    +            // if there are other items which should block service_up, they should be done in preStart
    +            ServiceStateLogic.ServiceNotUpLogic.clearNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL);
    +            
                 doStart(locationsToUse);
                 postStart(locationsToUse);
             } catch (Exception e) {
    -            setAttribute(Attributes.SERVICE_STATE, Lifecycle.ON_FIRE);
    +            // TODO should probably remember these problems then clear?  if so, do it here ... or on all effectors?
    +//            ServiceProblemsLogic.updateProblemsIndicator(this, START, e);
    +            
                 recordApplicationEvent(Lifecycle.ON_FIRE);
                 // no need to log here; the effector invocation should do that
                 throw Exceptions.propagate(e);
    +        } finally {
    +            ServiceStateLogic.setExpectedState(this, Lifecycle.RUNNING);
    --- End diff --
    
    If we set SERVICE_STATE_EXPECTED to ON_FIRE in the above catch clause this will overwrite it. So seems the better approach is to use the commented updateProblemsIndicator(this, START, e);


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#issuecomment-53893777
  
    There are lots of TODO comments related to concurrency/synchronization issues with the code, should they be addressed before merging?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923776
  
    --- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/EffectorApi.java ---
    @@ -79,7 +79,8 @@ public Response invoke(
           @QueryParam("timeout")
           String timeout,
           
    -      @ApiParam(name = "parameters", value = "Effector parameters (as key value pairs)", required = false)
    +      @ApiParam(/* FIXME: giving a `name` in swagger @ApiParam seems wrong as this object is the body, not a named argument */ name = "parameters",
    --- End diff --
    
    '"just" to open a jira issue' ... that takes minutes compared to seconds to type this.  and personally i'm more likely to see it if it's here.
    
    agree jira if we need visibility but for small aide-memoires having this here is better than not having it here, imho


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16907180
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/AbstractApplication.java ---
    @@ -123,23 +137,29 @@ public AbstractApplication setParent(Entity parent) {
         public void start(Collection<? extends Location> locations) {
             this.addLocations(locations);
             Collection<? extends Location> locationsToUse = getLocations();
    -        setAttribute(Attributes.SERVICE_STATE, Lifecycle.STARTING);
    +        ServiceProblemsLogic.clearProblemsIndicator(this, START);
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL, "Application starting");
             recordApplicationEvent(Lifecycle.STARTING);
             try {
                 preStart(locationsToUse);
    +            // if there are other items which should block service_up, they should be done in preStart
    +            ServiceStateLogic.ServiceNotUpLogic.clearNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL);
    +            
                 doStart(locationsToUse);
                 postStart(locationsToUse);
             } catch (Exception e) {
    -            setAttribute(Attributes.SERVICE_STATE, Lifecycle.ON_FIRE);
    +            // TODO should probably remember these problems then clear?  if so, do it here ... or on all effectors?
    +//            ServiceProblemsLogic.updateProblemsIndicator(this, START, e);
    +            
                 recordApplicationEvent(Lifecycle.ON_FIRE);
    --- End diff --
    
    Does "recordApplicationEvent(Lifecycle.ON_FIRE);" change SERVICE_STATE_EXPECTED? How is it marked ON_FIRE atm?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16924023
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java ---
    @@ -0,0 +1,215 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import static com.google.common.base.Preconditions.checkState;
    +
    +import java.util.Set;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.AbstractEntity;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.trait.Changeable;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.guava.Maybe;
    +
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.collect.Iterables;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Abstract superclass for enrichers which aggregate from children and/or members */
    +@SuppressWarnings("serial")
    +public abstract class AbstractAggregator<T,U> extends AbstractEnricher implements SensorEventListener<T> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractAggregator.class);
    +
    +    public static final ConfigKey<Entity> PRODUCER = ConfigKeys.newConfigKey(Entity.class, "enricher.producer", "The entity whose children/members will be aggregated");
    +
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +
    +    // FIXME this is not just for "members" i think -Alex
    --- End diff --
    
    this was a refactoring of existing code, not new code


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#issuecomment-53899974
  
    No functional issues so can be merged as is, but better to address/discuss comments first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894809
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java ---
    @@ -0,0 +1,215 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import static com.google.common.base.Preconditions.checkState;
    +
    +import java.util.Set;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.AbstractEntity;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.trait.Changeable;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.guava.Maybe;
    +
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.collect.Iterables;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Abstract superclass for enrichers which aggregate from children and/or members */
    +@SuppressWarnings("serial")
    +public abstract class AbstractAggregator<T,U> extends AbstractEnricher implements SensorEventListener<T> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractAggregator.class);
    +
    +    public static final ConfigKey<Entity> PRODUCER = ConfigKeys.newConfigKey(Entity.class, "enricher.producer", "The entity whose children/members will be aggregated");
    +
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +
    +    // FIXME this is not just for "members" i think -Alex
    +    public static final ConfigKey<?> DEFAULT_MEMBER_VALUE = ConfigKeys.newConfigKey(Object.class, "enricher.defaultMemberValue");
    +
    +    public static final ConfigKey<Set<? extends Entity>> FROM_HARDCODED_PRODUCERS = ConfigKeys.newConfigKey(new TypeToken<Set<? extends Entity>>() {}, "enricher.aggregating.fromHardcodedProducers");
    +
    +    public static final ConfigKey<Boolean> FROM_MEMBERS = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromMembers");
    +
    +    public static final ConfigKey<Boolean> FROM_CHILDREN = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromChildren");
    +
    +    public static final ConfigKey<Predicate<? super Entity>> ENTITY_FILTER = ConfigKeys.newConfigKey(new TypeToken<Predicate<? super Entity>>() {}, "enricher.aggregating.entityFilter");
    +
    +    public static final ConfigKey<Predicate<?>> VALUE_FILTER = ConfigKeys.newConfigKey(new TypeToken<Predicate<?>>() {}, "enricher.aggregating.valueFilter");
    +
    +    protected Entity producer;
    +    protected Sensor<U> targetSensor;
    +    protected T defaultMemberValue;
    +    protected Set<? extends Entity> fromHardcodedProducers;
    +    protected Boolean fromMembers;
    +    protected Boolean fromChildren;
    +    protected Predicate<? super Entity> entityFilter;
    +    protected Predicate<? super T> valueFilter;
    +    
    +    public AbstractAggregator() {}
    +
    +    @Override
    +    public void setEntity(EntityLocal entity) {
    +        super.setEntity(entity);
    +        setEntityLoadingConfig();
    +
    +        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, 
    --- End diff --
    
    Better use boolean expression: producer == null || (Boolean.TRUE.equals(fromMembers) || Boolean.TRUE.equals(fromChildren)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894563
  
    --- Diff: utils/common/src/main/java/brooklyn/util/collections/MutableList.java ---
    @@ -74,10 +80,25 @@ public MutableList(Iterable<? extends V> source) {
             }
         }
         
    +    /** @deprecated since 0.7.0, use {@link #asImmutableCopy()}, or {@link #asUnmodifiable()} / {@link #asUnmodifiableCopy()} */ @Deprecated
         public ImmutableList<V> toImmutable() {
             return ImmutableList.copyOf(this);
         }
    -    
    +    public List<V> asImmutableCopy() {
    --- End diff --
    
    Add comment explaining the difference with asUnmodifiableCopy. It should be used only for lists which don't contain nulls?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894587
  
    --- Diff: utils/common/src/main/java/brooklyn/util/collections/MutableList.java ---
    @@ -74,10 +80,25 @@ public MutableList(Iterable<? extends V> source) {
             }
         }
         
    +    /** @deprecated since 0.7.0, use {@link #asImmutableCopy()}, or {@link #asUnmodifiable()} / {@link #asUnmodifiableCopy()} */ @Deprecated
         public ImmutableList<V> toImmutable() {
             return ImmutableList.copyOf(this);
         }
    -    
    +    public List<V> asImmutableCopy() {
    +        try {
    +            return ImmutableList.copyOf(this);
    +        } catch (Exception e) {
    --- End diff --
    
    Either catch NPE only or add Exceptions.propagateIfFatal.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923482
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java ---
    @@ -0,0 +1,546 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.basic;
    +
    +import java.util.Collection;
    +import java.util.Date;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +
    +import javax.annotation.Nullable;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.enricher.Enrichers;
    +import brooklyn.enricher.basic.AbstractEnricher;
    +import brooklyn.enricher.basic.AbstractMultipleSensorAggregator;
    +import brooklyn.enricher.basic.UpdatingMap;
    +import brooklyn.entity.Effector;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.Lifecycle.Transition;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.policy.Enricher;
    +import brooklyn.policy.EnricherSpec;
    +import brooklyn.policy.EnricherSpec.ExtensibleEnricherSpec;
    +import brooklyn.util.collections.CollectionFunctionals;
    +import brooklyn.util.collections.MutableList;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.collections.MutableSet;
    +import brooklyn.util.guava.Functionals;
    +import brooklyn.util.text.Strings;
    +
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableSet;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Logic, sensors and enrichers, and conveniences, for computing service status */ 
    +public class ServiceStateLogic {
    +
    +    private static final Logger log = LoggerFactory.getLogger(ServiceStateLogic.class);
    +    
    +    public static final AttributeSensor<Boolean> SERVICE_UP = Attributes.SERVICE_UP;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_NOT_UP_INDICATORS = Attributes.SERVICE_NOT_UP_INDICATORS;
    +    
    +    public static final AttributeSensor<Lifecycle> SERVICE_STATE_ACTUAL = Attributes.SERVICE_STATE_ACTUAL;
    +    public static final AttributeSensor<Lifecycle.Transition> SERVICE_STATE_EXPECTED = Attributes.SERVICE_STATE_EXPECTED;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_PROBLEMS = Attributes.SERVICE_PROBLEMS;
    +
    +    /** static only; not for instantiation */
    +    private ServiceStateLogic() {}
    +
    +    public static <TKey,TVal> TVal getMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +        if (map==null) return null;
    +        return map.get(key);
    +    }
    +    
    +    @SuppressWarnings("unchecked")
    +    public static <TKey,TVal> void clearMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        updateMapSensorEntry(entity, sensor, key, (TVal)Entities.REMOVE);
    +    }
    +
    +    /** update the given key in the given map sensor */
    +    public static <TKey,TVal> void updateMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key, TVal v) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +
    +        // TODO synchronize
    +        
    +        boolean created = (map==null);
    +        if (created) map = MutableMap.of();
    +                
    +        boolean changed;
    +        if (v == Entities.REMOVE) {
    +            changed = map.containsKey(key);
    +            if (changed)
    +                map.remove(key);
    +        } else {
    +            TVal oldV = map.get(key);
    +            if (oldV==null)
    +                changed = (v!=null || !map.containsKey(key));
    +            else
    +                changed = !oldV.equals(v);
    --- End diff --
    
    `oldV` is guaranteed to be null, doesn't matter if `v` is null here does it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16924116
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/AbstractApplication.java ---
    @@ -123,23 +137,29 @@ public AbstractApplication setParent(Entity parent) {
         public void start(Collection<? extends Location> locations) {
             this.addLocations(locations);
             Collection<? extends Location> locationsToUse = getLocations();
    -        setAttribute(Attributes.SERVICE_STATE, Lifecycle.STARTING);
    +        ServiceProblemsLogic.clearProblemsIndicator(this, START);
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL, "Application starting");
             recordApplicationEvent(Lifecycle.STARTING);
             try {
                 preStart(locationsToUse);
    +            // if there are other items which should block service_up, they should be done in preStart
    +            ServiceStateLogic.ServiceNotUpLogic.clearNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL);
    +            
                 doStart(locationsToUse);
                 postStart(locationsToUse);
             } catch (Exception e) {
    -            setAttribute(Attributes.SERVICE_STATE, Lifecycle.ON_FIRE);
    +            // TODO should probably remember these problems then clear?  if so, do it here ... or on all effectors?
    +//            ServiceProblemsLogic.updateProblemsIndicator(this, START, e);
    +            
                 recordApplicationEvent(Lifecycle.ON_FIRE);
                 // no need to log here; the effector invocation should do that
                 throw Exceptions.propagate(e);
    +        } finally {
    +            ServiceStateLogic.setExpectedState(this, Lifecycle.RUNNING);
    --- End diff --
    
    needs more thought.  or maybe a `clearProblemsIndicators` lifecycle action!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923651
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractMultipleSensorAggregator.java ---
    @@ -0,0 +1,144 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Collection;
    +import java.util.Collections;
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.entity.Entity;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableMap;
    +
    +/** Building on {@link AbstractAggregator} for a single source sensor (on multiple children and/or members) */
    +public abstract class AbstractMultipleSensorAggregator<U> extends AbstractAggregator<Object,U> implements SensorEventListener<Object> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractMultipleSensorAggregator.class);
    +
    +    
    +    /** access via {@link #getValues(Sensor)} */
    +    private final Map<String, Map<Entity,Object>> values = Collections.synchronizedMap(new LinkedHashMap<String, Map<Entity,Object>>());
    +
    +    public AbstractMultipleSensorAggregator() {}
    +
    +    protected abstract Collection<Sensor<?>> getSourceSensors();
    +    
    +    @Override
    +    protected void setEntityLoadingConfig() {
    +        super.setEntityLoadingConfig();
    +        Preconditions.checkNotNull(getSourceSensors(), "sourceSensors must be set");
    +    }
    +    
    +    protected void setEntityBeforeSubscribingProducerChildrenEvents() {
    +        if (LOG.isDebugEnabled()) LOG.debug("{} subscribing to children of {}", new Object[] {this, producer });
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribeToChildren(producer, sourceSensor, this);
    +        }
    +    }
    +
    +    protected void addProducerHardcoded(Entity producer) {
    +        for (Sensor<?> sourceSensor: getSourceSensors()) {
    +            subscribe(producer, sourceSensor, this);
    +        }
    +        onProducerAdded(producer);
    +    }
    +
    +    protected void addProducerChild(Entity producer) {
    --- End diff --
    
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16897282
  
    --- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/PolicyApi.java ---
    @@ -57,6 +57,7 @@
       @GET
       @Path("/current-state")
       @ApiOperation(value = "Fetch policy states in batch", notes="Returns a map of policy ID to whether it is active")
    +  // FIXME method name -- this is nothing to do with config!
    --- End diff --
    
    Just to note, technically they are exposed via the rest-client project. I'm not sure anybody is using it though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923469
  
    --- Diff: core/src/main/java/brooklyn/enricher/Enrichers.java ---
    @@ -443,41 +518,106 @@ public String toString() {
             }
         }
     
    -    private static class ConcreteInitialBuilder extends InitialBuilder<ConcreteInitialBuilder> {
    +    public abstract static class AbstractUpdatingMapBuilder<S, TKey, TVal, B extends AbstractUpdatingMapBuilder<S, TKey, TVal, B>> extends AbstractEnricherBuilder<B> {
    +        protected AttributeSensor<Map<TKey,TVal>> targetSensor;
    +        protected AttributeSensor<? extends S> fromSensor;
    +        protected TKey key;
    +        protected Function<S, ? extends TVal> computing;
    +        protected Boolean removingIfResultIsNull;
    +        
    +        public AbstractUpdatingMapBuilder(AttributeSensor<Map<TKey,TVal>> target) {
    +            super(UpdatingMap.class);
    +            this.targetSensor = target;
    +        }
    +        @SuppressWarnings({ "unchecked", "rawtypes" })
    +        public <S2 extends S> UpdatingMapBuilder<S2,TKey,TVal> from(AttributeSensor<S2> fromSensor) {
    --- End diff --
    
    see e.g. `EnrichersTest.testUpdatingMap1()`.  this cheats generics to return an appropriately further-genericised builder.  assuming `S` is was not specified it is `Object` after the compiler hits the static method.  without the cast to `S2` generic type the builder would want a function returning `Object`.  *with* this cast if we give a `Sensor<String>` the other method calls are further constrainged to need to be e.g. `computing` a `String`.  HTH.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894398
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/Lifecycle.java ---
    @@ -51,12 +40,21 @@
     
         /**
          * The entity is starting.
    -     *
    -     * This stage is entered when the {@link brooklyn.entity.trait.Startable#START} {@link brooklyn.entity.Effector} is called. 
    -     * The entity will have its location set and and setup helper object created.
    +     * <p>
    +     * This stage is typically entered when the {@link brooklyn.entity.trait.Startable#START} {@link brooklyn.entity.Effector} 
    +     * is called, to undertake the startup operations from the management plane.
    +     * 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,
    +//
    --- End diff --
    
    Remove STARTED comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893655
  
    --- Diff: utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java ---
    @@ -0,0 +1,158 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.util.guava;
    +
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.base.Supplier;
    +
    +/** Utilities for building {@link Function} instances which return specific values
    + * (or {@link Supplier} or {@link Function} instances) when certain predicates are satisfied,
    + * tested in order and returning the first matching,
    + * with support for an "else" default value if none are satisfied (null by default). */
    +public class IfFunctions {
    +
    +    public static <I,O> IfFunctionBuilder<I,O> newInstance(Class<I> testType, Class<O> returnType) {
    +        return new IfFunctionBuilder<I,O>();
    +    }
    +    
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
    +        return new IfFunctionBuilderApplyingFirst<I>(test);
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
    +        return ifPredicate(Predicates.equalTo(test));
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
    +        return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +    }
    +    
    +    @Beta
    +    public static class IfFunction<I,O> implements Function<I,O> {
    +        protected final Map<Predicate<? super I>,Function<? super I,? extends O>> tests = new LinkedHashMap<Predicate<? super I>,Function<? super I,? extends O>>();
    +        protected Function<? super I,? extends O> defaultFunction = null;
    --- End diff --
    
    Better to use null returning function as the default so the "defaultFunction==null ? ..." checks can be avoided.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923989
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/AbstractAggregator.java ---
    @@ -0,0 +1,215 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import static com.google.common.base.Preconditions.checkState;
    +
    +import java.util.Set;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.AbstractEntity;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.trait.Changeable;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.guava.Maybe;
    +
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.collect.Iterables;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Abstract superclass for enrichers which aggregate from children and/or members */
    +@SuppressWarnings("serial")
    +public abstract class AbstractAggregator<T,U> extends AbstractEnricher implements SensorEventListener<T> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(AbstractAggregator.class);
    +
    +    public static final ConfigKey<Entity> PRODUCER = ConfigKeys.newConfigKey(Entity.class, "enricher.producer", "The entity whose children/members will be aggregated");
    +
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +
    +    // FIXME this is not just for "members" i think -Alex
    +    public static final ConfigKey<?> DEFAULT_MEMBER_VALUE = ConfigKeys.newConfigKey(Object.class, "enricher.defaultMemberValue");
    +
    +    public static final ConfigKey<Set<? extends Entity>> FROM_HARDCODED_PRODUCERS = ConfigKeys.newConfigKey(new TypeToken<Set<? extends Entity>>() {}, "enricher.aggregating.fromHardcodedProducers");
    +
    +    public static final ConfigKey<Boolean> FROM_MEMBERS = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromMembers");
    +
    +    public static final ConfigKey<Boolean> FROM_CHILDREN = ConfigKeys.newBooleanConfigKey("enricher.aggregating.fromChildren");
    +
    +    public static final ConfigKey<Predicate<? super Entity>> ENTITY_FILTER = ConfigKeys.newConfigKey(new TypeToken<Predicate<? super Entity>>() {}, "enricher.aggregating.entityFilter");
    +
    +    public static final ConfigKey<Predicate<?>> VALUE_FILTER = ConfigKeys.newConfigKey(new TypeToken<Predicate<?>>() {}, "enricher.aggregating.valueFilter");
    +
    +    protected Entity producer;
    +    protected Sensor<U> targetSensor;
    +    protected T defaultMemberValue;
    +    protected Set<? extends Entity> fromHardcodedProducers;
    +    protected Boolean fromMembers;
    +    protected Boolean fromChildren;
    +    protected Predicate<? super Entity> entityFilter;
    +    protected Predicate<? super T> valueFilter;
    +    
    +    public AbstractAggregator() {}
    +
    +    @Override
    +    public void setEntity(EntityLocal entity) {
    +        super.setEntity(entity);
    +        setEntityLoadingConfig();
    +
    +        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, 
    --- End diff --
    
    :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923527
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/UpdatingMap.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.flags.SetFromFlag;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Maps;
    +import com.google.common.reflect.TypeToken;
    +
    +/**
    + * Enricher which updates an entry in a sensor map ({@link #TARGET_SENSOR}) 
    + * based on the value of another sensor ({@link #SOURCE_SENSOR}.
    + * <p>
    + * The key used defaults to the name of the source sensor but can be specified with {@link #KEY_IN_TARGET_SENSOR}.
    + * The value placed in the map is the result of applying the function in {@link #COMPUTING} to the sensor value,
    + * with default behaviour being to remove an entry if <code>null</code> is returned
    + * but this can be overriden by setting {@link #REMOVING_IF_RESULT_IS_NULL} false.
    + * {@link Entities#REMOVE} and {@link Entities#UNCHANGED} are also respeced as return values for the computation
    + * (ignoring generics).
    + * Unlike most other enrichers, this defaults to {@link AbstractEnricher#SUPPRESS_DUPLICATES} being true
    + *  
    + * @author alex
    + *
    + * @param <S> source sensor type
    + * @param <TKey> key type in target sensor map
    + * @param <TVal> value type in target sensor map
    + */
    +@SuppressWarnings("serial")
    +public class UpdatingMap<S,TKey,TVal> extends AbstractEnricher implements SensorEventListener<S> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(UpdatingMap.class);
    +
    +    @SetFromFlag("fromSensor")
    +    public static final ConfigKey<Sensor<?>> SOURCE_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.sourceSensor");
    +    @SetFromFlag("targetSensor")
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +    @SetFromFlag("key")
    +    public static final ConfigKey<?> KEY_IN_TARGET_SENSOR = ConfigKeys.newConfigKey(Object.class, "enricher.updatingMap.keyInTargetSensor",
    +        "Key to update in the target sensor map, defaulting to the name of the source sensor");
    +    @SetFromFlag("computing")
    +    public static final ConfigKey<Function<?, ?>> COMPUTING = ConfigKeys.newConfigKey(new TypeToken<Function<?,?>>() {}, "enricher.updatingMap.computing");
    +    @SetFromFlag("removingIfResultIsNull")
    +    public static final ConfigKey<Boolean> REMOVING_IF_RESULT_IS_NULL = ConfigKeys.newBooleanConfigKey("enricher.updatingMap.removingIfResultIsNull", 
    +        "Whether the key in the target map is removed if the result if the computation is null");
    +
    +    protected AttributeSensor<S> sourceSensor;
    +    protected AttributeSensor<Map<TKey,TVal>> targetSensor;
    +    protected TKey key;
    +    protected Function<S,? extends TVal> computing;
    +    protected Boolean removingIfResultIsNull;
    +
    +    public UpdatingMap() {
    +        this(Maps.newLinkedHashMap());
    +    }
    +
    +    public UpdatingMap(Map<Object, Object> flags) {
    +        super(flags);
    +        if (suppressDuplicates==null) {
    +            // this defaults to suppressing duplicates
    +            suppressDuplicates = true;
    +        }
    +    }
    +
    +    @SuppressWarnings({ "unchecked", "rawtypes" })
    +    @Override
    +    public void setEntity(EntityLocal entity) {
    +        super.setEntity(entity);
    +        this.sourceSensor = (AttributeSensor<S>) getRequiredConfig(SOURCE_SENSOR);
    +        this.targetSensor = (AttributeSensor<Map<TKey,TVal>>) getRequiredConfig(TARGET_SENSOR);
    +        this.key = (TKey) getConfig(KEY_IN_TARGET_SENSOR);
    +        this.computing = (Function) getRequiredConfig(COMPUTING);
    +        this.removingIfResultIsNull = getConfig(REMOVING_IF_RESULT_IS_NULL);
    +
    +        subscribe(entity, sourceSensor, this);
    +        onUpdated();
    +    }
    +    
    +    @Override
    +    public void onEvent(SensorEvent<S> event) {
    +        onUpdated();
    +    }
    +
    +    /**
    +     * Called whenever the values for the set of producers changes (e.g. on an event, or on a member added/removed).
    +     */
    +    @SuppressWarnings("unchecked")
    +    protected void onUpdated() {
    +        try {
    +            Object v = computing.apply(entity.getAttribute(sourceSensor));
    +            if (v == null && !Boolean.FALSE.equals(removingIfResultIsNull)) {
    +                v = Entities.REMOVE;
    +            }
    +            if (v == Entities.UNCHANGED) {
    +                // nothing
    +            } else {
    +                // TODO check synchronization
    +                TKey key = this.key;
    +                if (key==null) key = (TKey) sourceSensor.getName();
    +                
    +                Map<TKey, TVal> map = entity.getAttribute(targetSensor);
    +
    +                boolean created = (map==null);
    +                if (created) map = MutableMap.of();
    +                
    +                boolean changed;
    +                if (v == Entities.REMOVE) {
    +                    changed = map.containsKey(key);
    +                    if (changed)
    +                        map.remove(key);
    +                } else {
    +                    TVal oldV = map.get(key);
    +                    if (oldV==null)
    +                        changed = (v!=null || !map.containsKey(key));
    +                    else
    +                        changed = !oldV.equals(v);
    --- End diff --
    
    as before.  note `Object.equals` contract says:
    
         * <li>For any non-null reference value <code>x</code>,
         *     <code>x.equals(null)</code> should return <code>false</code>.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893800
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java ---
    @@ -0,0 +1,546 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.basic;
    +
    +import java.util.Collection;
    +import java.util.Date;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +
    +import javax.annotation.Nullable;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.enricher.Enrichers;
    +import brooklyn.enricher.basic.AbstractEnricher;
    +import brooklyn.enricher.basic.AbstractMultipleSensorAggregator;
    +import brooklyn.enricher.basic.UpdatingMap;
    +import brooklyn.entity.Effector;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.Lifecycle.Transition;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.policy.Enricher;
    +import brooklyn.policy.EnricherSpec;
    +import brooklyn.policy.EnricherSpec.ExtensibleEnricherSpec;
    +import brooklyn.util.collections.CollectionFunctionals;
    +import brooklyn.util.collections.MutableList;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.collections.MutableSet;
    +import brooklyn.util.guava.Functionals;
    +import brooklyn.util.text.Strings;
    +
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableSet;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Logic, sensors and enrichers, and conveniences, for computing service status */ 
    +public class ServiceStateLogic {
    +
    +    private static final Logger log = LoggerFactory.getLogger(ServiceStateLogic.class);
    +    
    +    public static final AttributeSensor<Boolean> SERVICE_UP = Attributes.SERVICE_UP;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_NOT_UP_INDICATORS = Attributes.SERVICE_NOT_UP_INDICATORS;
    +    
    +    public static final AttributeSensor<Lifecycle> SERVICE_STATE_ACTUAL = Attributes.SERVICE_STATE_ACTUAL;
    +    public static final AttributeSensor<Lifecycle.Transition> SERVICE_STATE_EXPECTED = Attributes.SERVICE_STATE_EXPECTED;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_PROBLEMS = Attributes.SERVICE_PROBLEMS;
    +
    +    /** static only; not for instantiation */
    +    private ServiceStateLogic() {}
    +
    +    public static <TKey,TVal> TVal getMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +        if (map==null) return null;
    +        return map.get(key);
    +    }
    +    
    +    @SuppressWarnings("unchecked")
    +    public static <TKey,TVal> void clearMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        updateMapSensorEntry(entity, sensor, key, (TVal)Entities.REMOVE);
    +    }
    +
    +    /** update the given key in the given map sensor */
    +    public static <TKey,TVal> void updateMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key, TVal v) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +
    +        // TODO synchronize
    +        
    +        boolean created = (map==null);
    +        if (created) map = MutableMap.of();
    +                
    +        boolean changed;
    +        if (v == Entities.REMOVE) {
    +            changed = map.containsKey(key);
    +            if (changed)
    +                map.remove(key);
    +        } else {
    +            TVal oldV = map.get(key);
    +            if (oldV==null)
    +                changed = (v!=null || !map.containsKey(key));
    +            else
    +                changed = !oldV.equals(v);
    --- End diff --
    
    "v" could be null, while equals doesn't guarantee it is null safe.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893810
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/UpdatingMap.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.flags.SetFromFlag;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Maps;
    +import com.google.common.reflect.TypeToken;
    +
    +/**
    + * Enricher which updates an entry in a sensor map ({@link #TARGET_SENSOR}) 
    + * based on the value of another sensor ({@link #SOURCE_SENSOR}.
    + * <p>
    + * The key used defaults to the name of the source sensor but can be specified with {@link #KEY_IN_TARGET_SENSOR}.
    + * The value placed in the map is the result of applying the function in {@link #COMPUTING} to the sensor value,
    + * with default behaviour being to remove an entry if <code>null</code> is returned
    + * but this can be overriden by setting {@link #REMOVING_IF_RESULT_IS_NULL} false.
    + * {@link Entities#REMOVE} and {@link Entities#UNCHANGED} are also respeced as return values for the computation
    + * (ignoring generics).
    + * Unlike most other enrichers, this defaults to {@link AbstractEnricher#SUPPRESS_DUPLICATES} being true
    + *  
    + * @author alex
    + *
    + * @param <S> source sensor type
    + * @param <TKey> key type in target sensor map
    + * @param <TVal> value type in target sensor map
    + */
    +@SuppressWarnings("serial")
    +public class UpdatingMap<S,TKey,TVal> extends AbstractEnricher implements SensorEventListener<S> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(UpdatingMap.class);
    +
    +    @SetFromFlag("fromSensor")
    +    public static final ConfigKey<Sensor<?>> SOURCE_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.sourceSensor");
    +    @SetFromFlag("targetSensor")
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +    @SetFromFlag("key")
    +    public static final ConfigKey<?> KEY_IN_TARGET_SENSOR = ConfigKeys.newConfigKey(Object.class, "enricher.updatingMap.keyInTargetSensor",
    +        "Key to update in the target sensor map, defaulting to the name of the source sensor");
    +    @SetFromFlag("computing")
    +    public static final ConfigKey<Function<?, ?>> COMPUTING = ConfigKeys.newConfigKey(new TypeToken<Function<?,?>>() {}, "enricher.updatingMap.computing");
    +    @SetFromFlag("removingIfResultIsNull")
    +    public static final ConfigKey<Boolean> REMOVING_IF_RESULT_IS_NULL = ConfigKeys.newBooleanConfigKey("enricher.updatingMap.removingIfResultIsNull", 
    +        "Whether the key in the target map is removed if the result if the computation is null");
    +
    +    protected AttributeSensor<S> sourceSensor;
    +    protected AttributeSensor<Map<TKey,TVal>> targetSensor;
    +    protected TKey key;
    +    protected Function<S,? extends TVal> computing;
    +    protected Boolean removingIfResultIsNull;
    +
    +    public UpdatingMap() {
    +        this(Maps.newLinkedHashMap());
    +    }
    +
    +    public UpdatingMap(Map<Object, Object> flags) {
    +        super(flags);
    +        if (suppressDuplicates==null) {
    +            // this defaults to suppressing duplicates
    +            suppressDuplicates = true;
    +        }
    +    }
    +
    +    @SuppressWarnings({ "unchecked", "rawtypes" })
    +    @Override
    +    public void setEntity(EntityLocal entity) {
    +        super.setEntity(entity);
    +        this.sourceSensor = (AttributeSensor<S>) getRequiredConfig(SOURCE_SENSOR);
    +        this.targetSensor = (AttributeSensor<Map<TKey,TVal>>) getRequiredConfig(TARGET_SENSOR);
    +        this.key = (TKey) getConfig(KEY_IN_TARGET_SENSOR);
    +        this.computing = (Function) getRequiredConfig(COMPUTING);
    +        this.removingIfResultIsNull = getConfig(REMOVING_IF_RESULT_IS_NULL);
    +
    +        subscribe(entity, sourceSensor, this);
    +        onUpdated();
    +    }
    +    
    +    @Override
    +    public void onEvent(SensorEvent<S> event) {
    +        onUpdated();
    +    }
    +
    +    /**
    +     * Called whenever the values for the set of producers changes (e.g. on an event, or on a member added/removed).
    +     */
    +    @SuppressWarnings("unchecked")
    +    protected void onUpdated() {
    +        try {
    +            Object v = computing.apply(entity.getAttribute(sourceSensor));
    +            if (v == null && !Boolean.FALSE.equals(removingIfResultIsNull)) {
    +                v = Entities.REMOVE;
    +            }
    +            if (v == Entities.UNCHANGED) {
    +                // nothing
    +            } else {
    +                // TODO check synchronization
    +                TKey key = this.key;
    +                if (key==null) key = (TKey) sourceSensor.getName();
    +                
    +                Map<TKey, TVal> map = entity.getAttribute(targetSensor);
    +
    +                boolean created = (map==null);
    +                if (created) map = MutableMap.of();
    +                
    +                boolean changed;
    +                if (v == Entities.REMOVE) {
    +                    changed = map.containsKey(key);
    +                    if (changed)
    +                        map.remove(key);
    +                } else {
    +                    TVal oldV = map.get(key);
    +                    if (oldV==null)
    +                        changed = (v!=null || !map.containsKey(key));
    +                    else
    +                        changed = !oldV.equals(v);
    --- End diff --
    
    "v" could be null, while equals doesn't guarantee it is null safe.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894383
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/AbstractApplication.java ---
    @@ -174,18 +194,20 @@ public void postStart(Collection<? extends Location> locations) {
         public void stop() {
             logApplicationLifecycle("Stopping");
     
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(this, Attributes.SERVICE_STATE_ACTUAL, "Application stopping");
             setAttribute(SERVICE_UP, false);
    -        setAttribute(Attributes.SERVICE_STATE, Lifecycle.STOPPING);
    +        ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
             recordApplicationEvent(Lifecycle.STOPPING);
             try {
                 doStop();
             } catch (Exception e) {
    -            setAttribute(Attributes.SERVICE_STATE, Lifecycle.ON_FIRE);
    +            ServiceStateLogic.setExpectedState(this, Lifecycle.ON_FIRE);
    --- End diff --
    
    Shouldn't we always set ON_FIRE through the PROBLEMS map?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893742
  
    --- Diff: core/src/main/java/brooklyn/enricher/Enrichers.java ---
    @@ -443,41 +518,106 @@ public String toString() {
             }
         }
     
    -    private static class ConcreteInitialBuilder extends InitialBuilder<ConcreteInitialBuilder> {
    +    public abstract static class AbstractUpdatingMapBuilder<S, TKey, TVal, B extends AbstractUpdatingMapBuilder<S, TKey, TVal, B>> extends AbstractEnricherBuilder<B> {
    +        protected AttributeSensor<Map<TKey,TVal>> targetSensor;
    +        protected AttributeSensor<? extends S> fromSensor;
    +        protected TKey key;
    +        protected Function<S, ? extends TVal> computing;
    +        protected Boolean removingIfResultIsNull;
    +        
    +        public AbstractUpdatingMapBuilder(AttributeSensor<Map<TKey,TVal>> target) {
    +            super(UpdatingMap.class);
    +            this.targetSensor = target;
    +        }
    +        @SuppressWarnings({ "unchecked", "rawtypes" })
    +        public <S2 extends S> UpdatingMapBuilder<S2,TKey,TVal> from(AttributeSensor<S2> fromSensor) {
    --- End diff --
    
    If the point of the class is to re-use it for subclassing then why the method returns a specific sub-class? There are similar cases for the other enricher builders, I can't see the reasoning.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923963
  
    --- Diff: utils/common/src/main/java/brooklyn/util/collections/MutableList.java ---
    @@ -74,10 +80,25 @@ public MutableList(Iterable<? extends V> source) {
             }
         }
         
    +    /** @deprecated since 0.7.0, use {@link #asImmutableCopy()}, or {@link #asUnmodifiable()} / {@link #asUnmodifiableCopy()} */ @Deprecated
         public ImmutableList<V> toImmutable() {
             return ImmutableList.copyOf(this);
         }
    -    
    +    public List<V> asImmutableCopy() {
    +        try {
    +            return ImmutableList.copyOf(this);
    +        } catch (Exception e) {
    --- End diff --
    
    good spot, done for all Mutable{L,S,M}


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16906219
  
    --- Diff: core/src/main/java/brooklyn/enricher/basic/UpdatingMap.java ---
    @@ -0,0 +1,160 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.enricher.basic;
    +
    +import java.util.Map;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.exceptions.Exceptions;
    +import brooklyn.util.flags.SetFromFlag;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Maps;
    +import com.google.common.reflect.TypeToken;
    +
    +/**
    + * Enricher which updates an entry in a sensor map ({@link #TARGET_SENSOR}) 
    + * based on the value of another sensor ({@link #SOURCE_SENSOR}.
    + * <p>
    + * The key used defaults to the name of the source sensor but can be specified with {@link #KEY_IN_TARGET_SENSOR}.
    + * The value placed in the map is the result of applying the function in {@link #COMPUTING} to the sensor value,
    + * with default behaviour being to remove an entry if <code>null</code> is returned
    + * but this can be overriden by setting {@link #REMOVING_IF_RESULT_IS_NULL} false.
    + * {@link Entities#REMOVE} and {@link Entities#UNCHANGED} are also respeced as return values for the computation
    + * (ignoring generics).
    + * Unlike most other enrichers, this defaults to {@link AbstractEnricher#SUPPRESS_DUPLICATES} being true
    + *  
    + * @author alex
    + *
    + * @param <S> source sensor type
    + * @param <TKey> key type in target sensor map
    + * @param <TVal> value type in target sensor map
    + */
    +@SuppressWarnings("serial")
    +public class UpdatingMap<S,TKey,TVal> extends AbstractEnricher implements SensorEventListener<S> {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(UpdatingMap.class);
    +
    +    @SetFromFlag("fromSensor")
    +    public static final ConfigKey<Sensor<?>> SOURCE_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.sourceSensor");
    +    @SetFromFlag("targetSensor")
    +    public static final ConfigKey<Sensor<?>> TARGET_SENSOR = ConfigKeys.newConfigKey(new TypeToken<Sensor<?>>() {}, "enricher.targetSensor");
    +    @SetFromFlag("key")
    +    public static final ConfigKey<?> KEY_IN_TARGET_SENSOR = ConfigKeys.newConfigKey(Object.class, "enricher.updatingMap.keyInTargetSensor",
    +        "Key to update in the target sensor map, defaulting to the name of the source sensor");
    +    @SetFromFlag("computing")
    +    public static final ConfigKey<Function<?, ?>> COMPUTING = ConfigKeys.newConfigKey(new TypeToken<Function<?,?>>() {}, "enricher.updatingMap.computing");
    +    @SetFromFlag("removingIfResultIsNull")
    +    public static final ConfigKey<Boolean> REMOVING_IF_RESULT_IS_NULL = ConfigKeys.newBooleanConfigKey("enricher.updatingMap.removingIfResultIsNull", 
    +        "Whether the key in the target map is removed if the result if the computation is null");
    +
    +    protected AttributeSensor<S> sourceSensor;
    +    protected AttributeSensor<Map<TKey,TVal>> targetSensor;
    +    protected TKey key;
    +    protected Function<S,? extends TVal> computing;
    +    protected Boolean removingIfResultIsNull;
    +
    +    public UpdatingMap() {
    +        this(Maps.newLinkedHashMap());
    +    }
    +
    +    public UpdatingMap(Map<Object, Object> flags) {
    +        super(flags);
    +        if (suppressDuplicates==null) {
    +            // this defaults to suppressing duplicates
    +            suppressDuplicates = true;
    +        }
    +    }
    +
    +    @SuppressWarnings({ "unchecked", "rawtypes" })
    +    @Override
    +    public void setEntity(EntityLocal entity) {
    +        super.setEntity(entity);
    +        this.sourceSensor = (AttributeSensor<S>) getRequiredConfig(SOURCE_SENSOR);
    +        this.targetSensor = (AttributeSensor<Map<TKey,TVal>>) getRequiredConfig(TARGET_SENSOR);
    +        this.key = (TKey) getConfig(KEY_IN_TARGET_SENSOR);
    +        this.computing = (Function) getRequiredConfig(COMPUTING);
    +        this.removingIfResultIsNull = getConfig(REMOVING_IF_RESULT_IS_NULL);
    +
    +        subscribe(entity, sourceSensor, this);
    +        onUpdated();
    +    }
    +    
    +    @Override
    +    public void onEvent(SensorEvent<S> event) {
    +        onUpdated();
    +    }
    +
    +    /**
    +     * Called whenever the values for the set of producers changes (e.g. on an event, or on a member added/removed).
    +     */
    +    @SuppressWarnings("unchecked")
    +    protected void onUpdated() {
    --- End diff --
    
    Missing @Override


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893857
  
    --- Diff: core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java ---
    @@ -0,0 +1,546 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.entity.basic;
    +
    +import java.util.Collection;
    +import java.util.Date;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.Set;
    +
    +import javax.annotation.Nullable;
    +
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.enricher.Enrichers;
    +import brooklyn.enricher.basic.AbstractEnricher;
    +import brooklyn.enricher.basic.AbstractMultipleSensorAggregator;
    +import brooklyn.enricher.basic.UpdatingMap;
    +import brooklyn.entity.Effector;
    +import brooklyn.entity.Entity;
    +import brooklyn.entity.Group;
    +import brooklyn.entity.basic.Lifecycle.Transition;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.Sensor;
    +import brooklyn.event.SensorEvent;
    +import brooklyn.event.SensorEventListener;
    +import brooklyn.policy.Enricher;
    +import brooklyn.policy.EnricherSpec;
    +import brooklyn.policy.EnricherSpec.ExtensibleEnricherSpec;
    +import brooklyn.util.collections.CollectionFunctionals;
    +import brooklyn.util.collections.MutableList;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.collections.MutableSet;
    +import brooklyn.util.guava.Functionals;
    +import brooklyn.util.text.Strings;
    +
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import com.google.common.collect.ImmutableList;
    +import com.google.common.collect.ImmutableSet;
    +import com.google.common.reflect.TypeToken;
    +
    +/** Logic, sensors and enrichers, and conveniences, for computing service status */ 
    +public class ServiceStateLogic {
    +
    +    private static final Logger log = LoggerFactory.getLogger(ServiceStateLogic.class);
    +    
    +    public static final AttributeSensor<Boolean> SERVICE_UP = Attributes.SERVICE_UP;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_NOT_UP_INDICATORS = Attributes.SERVICE_NOT_UP_INDICATORS;
    +    
    +    public static final AttributeSensor<Lifecycle> SERVICE_STATE_ACTUAL = Attributes.SERVICE_STATE_ACTUAL;
    +    public static final AttributeSensor<Lifecycle.Transition> SERVICE_STATE_EXPECTED = Attributes.SERVICE_STATE_EXPECTED;
    +    public static final AttributeSensor<Map<String,Object>> SERVICE_PROBLEMS = Attributes.SERVICE_PROBLEMS;
    +
    +    /** static only; not for instantiation */
    +    private ServiceStateLogic() {}
    +
    +    public static <TKey,TVal> TVal getMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +        if (map==null) return null;
    +        return map.get(key);
    +    }
    +    
    +    @SuppressWarnings("unchecked")
    +    public static <TKey,TVal> void clearMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key) {
    +        updateMapSensorEntry(entity, sensor, key, (TVal)Entities.REMOVE);
    +    }
    +
    +    /** update the given key in the given map sensor */
    +    public static <TKey,TVal> void updateMapSensorEntry(EntityLocal entity, AttributeSensor<Map<TKey,TVal>> sensor, TKey key, TVal v) {
    +        Map<TKey, TVal> map = entity.getAttribute(sensor);
    +
    +        // TODO synchronize
    +        
    +        boolean created = (map==null);
    +        if (created) map = MutableMap.of();
    +                
    +        boolean changed;
    +        if (v == Entities.REMOVE) {
    +            changed = map.containsKey(key);
    +            if (changed)
    +                map.remove(key);
    +        } else {
    +            TVal oldV = map.get(key);
    +            if (oldV==null)
    +                changed = (v!=null || !map.containsKey(key));
    +            else
    +                changed = !oldV.equals(v);
    +            if (changed)
    +                map.put(key, (TVal)v);
    +        }
    +        if (changed || created)
    +            entity.setAttribute(sensor, map);
    +    }
    +    
    +    public static void setExpectedState(Entity entity, Lifecycle state) {
    +        ((EntityInternal)entity).setAttribute(Attributes.SERVICE_STATE_EXPECTED, new Lifecycle.Transition(state, new Date()));
    +        
    +        Enricher enricher = EntityAdjuncts.findWithUniqueTag(entity.getEnrichers(), ComputeServiceState.DEFAULT_ENRICHER_UNIQUE_TAG);
    +        if (enricher instanceof ComputeServiceState) {
    +            ((ComputeServiceState)enricher).onEvent(null);
    +        }
    +    }
    +    public static Lifecycle getExpectedState(Entity entity) {
    +        Transition expected = entity.getAttribute(Attributes.SERVICE_STATE_EXPECTED);
    +        if (expected==null) return null;
    +        return expected.getState();
    +    }
    +    public static boolean isExpectedState(Entity entity, Lifecycle state) {
    +        return getExpectedState(entity)==state;
    +    }
    +    
    +    public static class ServiceNotUpLogic {
    +        public static final String DEFAULT_ENRICHER_UNIQUE_TAG = "service.isUp if no service.notUp.indicators";
    +        
    +        /** static only; not for instantiation */
    +        private ServiceNotUpLogic() {}
    +        
    +        @SuppressWarnings({ "unchecked", "rawtypes" })
    +        public static final EnricherSpec<?> newEnricherForServiceUpIfNoNotUpIndicators() {
    --- End diff --
    
    A better name would be newEnricherForServiceUpIfNotUpIndicatorsEmpty. Avoding the double negation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893523
  
    --- Diff: utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java ---
    @@ -0,0 +1,158 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.util.guava;
    +
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.base.Supplier;
    +
    +/** Utilities for building {@link Function} instances which return specific values
    + * (or {@link Supplier} or {@link Function} instances) when certain predicates are satisfied,
    + * tested in order and returning the first matching,
    + * with support for an "else" default value if none are satisfied (null by default). */
    +public class IfFunctions {
    +
    +    public static <I,O> IfFunctionBuilder<I,O> newInstance(Class<I> testType, Class<O> returnType) {
    +        return new IfFunctionBuilder<I,O>();
    +    }
    +    
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
    +        return new IfFunctionBuilderApplyingFirst<I>(test);
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
    +        return ifPredicate(Predicates.equalTo(test));
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
    --- End diff --
    
    Remove the O type parameter, unused (all three methods).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#issuecomment-53937556
  
    good points, have addressed most comments with code fixes, and others with discussion here.
    
    going to merge if no objection.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16893613
  
    --- Diff: utils/common/src/main/java/brooklyn/util/guava/IfFunctions.java ---
    @@ -0,0 +1,158 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package brooklyn.util.guava;
    +
    +import java.util.LinkedHashMap;
    +import java.util.Map;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.base.Function;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Predicate;
    +import com.google.common.base.Predicates;
    +import com.google.common.base.Supplier;
    +
    +/** Utilities for building {@link Function} instances which return specific values
    + * (or {@link Supplier} or {@link Function} instances) when certain predicates are satisfied,
    + * tested in order and returning the first matching,
    + * with support for an "else" default value if none are satisfied (null by default). */
    +public class IfFunctions {
    +
    +    public static <I,O> IfFunctionBuilder<I,O> newInstance(Class<I> testType, Class<O> returnType) {
    +        return new IfFunctionBuilder<I,O>();
    +    }
    +    
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifPredicate(Predicate<? super I> test) {
    +        return new IfFunctionBuilderApplyingFirst<I>(test);
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifEquals(I test) {
    +        return ifPredicate(Predicates.equalTo(test));
    +    }
    +    public static <I,O> IfFunctionBuilderApplyingFirst<I> ifNotEquals(I test) {
    +        return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +    }
    +    
    +    @Beta
    +    public static class IfFunction<I,O> implements Function<I,O> {
    +        protected final Map<Predicate<? super I>,Function<? super I,? extends O>> tests = new LinkedHashMap<Predicate<? super I>,Function<? super I,? extends O>>();
    +        protected Function<? super I,? extends O> defaultFunction = null;
    +        
    +        protected IfFunction(IfFunction<I,O> input) {
    +            this.tests.putAll(input.tests);
    +            this.defaultFunction = input.defaultFunction;
    +        }
    +
    +        protected IfFunction() {
    +        }
    +        
    +        @Override
    +        public O apply(I input) {
    +            for (Map.Entry<Predicate<? super I>,Function<? super I,? extends O>> test: tests.entrySet()) {
    +                if (test.getKey().apply(input)) 
    +                    return test.getValue().apply(input);
    +            }
    +            return defaultFunction==null ? null : defaultFunction.apply(input);
    +        }
    +        
    +        @Override
    +        public String toString() {
    +            return "if["+tests+"]"+(defaultFunction!=null ? "-else["+defaultFunction+"]" : "");
    +        }
    +    }
    +    
    +    @Beta
    +    public static class IfFunctionBuilder<I,O> extends IfFunction<I,O> {
    +        protected IfFunctionBuilder() { super(); }
    +        protected IfFunctionBuilder(IfFunction<I,O> input) { super(input); }
    +        
    +        public IfFunction<I,O> build() {
    +            return new IfFunction<I,O>(this);
    +        }
    +        
    +        public IfFunctionBuilderApplying<I,O> ifPredicate(Predicate<I> test) {
    +            return new IfFunctionBuilderApplying<I,O>(this, (Predicate<I>)test);
    +        }
    +        public IfFunctionBuilderApplying<I,O> ifEquals(I test) {
    +            return ifPredicate(Predicates.equalTo(test));
    +        }
    +        public IfFunctionBuilderApplying<I,O> ifNotEquals(I test) {
    +            return ifPredicate(Predicates.not(Predicates.equalTo(test)));
    +        }
    +
    +        public IfFunctionBuilder<I,O> defaultValue(O defaultValue) {
    +            return defaultApply(new Functionals.ConstantFunction<I,O>(defaultValue, defaultValue));
    +        }
    +        @SuppressWarnings("unchecked")
    +        public IfFunctionBuilder<I,O> defaultGet(Supplier<? extends O> defaultSupplier) {
    +            return defaultApply((Function<I,O>)Functions.forSupplier(defaultSupplier));
    +        }
    +        public IfFunctionBuilder<I,O> defaultApply(Function<? super I,? extends O> defaultFunction) {
    +            IfFunctionBuilder<I, O> result = new IfFunctionBuilder<I,O>(this);
    +            result.defaultFunction = defaultFunction;
    +            return result;
    +        }
    +    }
    +
    +    @Beta
    +    public static class IfFunctionBuilderApplying<I,O> {
    +        private IfFunction<I, O> input;
    +        private Predicate<? super I> test;
    +        
    +        private IfFunctionBuilderApplying(IfFunction<I,O> input, Predicate<? super I> test) {
    +            this.input = input;
    +            this.test = test;
    +        }
    +        
    +        public IfFunctionBuilder<I,O> value(O value) {
    +            return apply(new Functionals.ConstantFunction<I,O>(value, value));
    +        }
    +        @SuppressWarnings("unchecked")
    +        public IfFunctionBuilder<I,O> get(Supplier<? extends O> supplier) {
    +            return apply((Function<I,O>)Functions.forSupplier(supplier));
    +        }
    +        public IfFunctionBuilder<I,O> apply(Function<? super I,? extends O> function) {
    +            IfFunctionBuilder<I, O> result = new IfFunctionBuilder<I,O>(input);
    +            result.tests.put(test, function);
    +            return result;
    +        }
    +    }
    +
    +    @Beta
    +    public static class IfFunctionBuilderApplyingFirst<I> {
    --- End diff --
    
    No point in duplicating IfFunctionBuilderApplying's code just for one differing line. Better instantiate IfFunctionBuilderApplying with Null object for the input argument (i.e. empty IfFunction).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16923837
  
    --- Diff: utils/common/src/main/java/brooklyn/util/collections/MutableList.java ---
    @@ -74,10 +80,25 @@ public MutableList(Iterable<? extends V> source) {
             }
         }
         
    +    /** @deprecated since 0.7.0, use {@link #asImmutableCopy()}, or {@link #asUnmodifiable()} / {@link #asUnmodifiableCopy()} */ @Deprecated
         public ImmutableList<V> toImmutable() {
             return ImmutableList.copyOf(this);
         }
    -    
    +    public List<V> asImmutableCopy() {
    --- End diff --
    
    yes, will add comments


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894465
  
    --- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/EffectorApi.java ---
    @@ -79,7 +79,8 @@ public Response invoke(
           @QueryParam("timeout")
           String timeout,
           
    -      @ApiParam(name = "parameters", value = "Effector parameters (as key value pairs)", required = false)
    +      @ApiParam(/* FIXME: giving a `name` in swagger @ApiParam seems wrong as this object is the body, not a named argument */ name = "parameters",
    --- End diff --
    
    Aren't these kind of comments forgotten the moment they are committed? I think there are other instances of this issue and it would be better just to open Jira issue?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Sensors chained for service up

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/131#discussion_r16894503
  
    --- Diff: usage/rest-api/src/main/java/brooklyn/rest/api/PolicyApi.java ---
    @@ -57,6 +57,7 @@
       @GET
       @Path("/current-state")
       @ApiOperation(value = "Fetch policy states in batch", notes="Returns a map of policy ID to whether it is active")
    +  // FIXME method name -- this is nothing to do with config!
    --- End diff --
    
    Better just to rename it. The REST API method names are not exposed to clients.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---