brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request: Transformer enricher: support trigge...
Date Thu, 19 May 2016 23:26:01 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/141#discussion_r63970280
  
    --- Diff: core/src/main/java/org/apache/brooklyn/enricher/stock/AbstractTransformer.java
---
    @@ -59,27 +72,55 @@ public void setEntity(EntityLocal entity) {
             super.setEntity(entity);
     
             this.producer = getConfig(PRODUCER) == null ? entity: getConfig(PRODUCER);
    -        this.sourceSensor = (Sensor<T>) getRequiredConfig(SOURCE_SENSOR);
    +        this.sourceSensor = (Sensor<T>) getConfig(SOURCE_SENSOR);
             Sensor<?> targetSensorSpecified = getConfig(TARGET_SENSOR);
    +        List<? extends Sensor<?>> triggerSensorsSpecified = getConfig(TRIGGER_SENSORS);
    +        List<? extends Sensor<?>> triggerSensors = triggerSensorsSpecified
!= null ? triggerSensorsSpecified : ImmutableList.<Sensor<?>>of();
             this.targetSensor = targetSensorSpecified!=null ? (Sensor<U>) targetSensorSpecified
: (Sensor<U>) this.sourceSensor;
    -        if (producer.equals(entity) && targetSensorSpecified==null) {
    +        if (targetSensor == null) {
    +            throw new IllegalArgumentException("Enricher "+JavaClassNames.simpleClassName(this)+"
has no "+TARGET_SENSOR.getName()+", and it cannot be inferred as "+SOURCE_SENSOR.getName()+"
is also not set");
    +        }
    +        if (sourceSensor == null && triggerSensors.isEmpty()) {
    +            throw new IllegalArgumentException("Enricher "+JavaClassNames.simpleClassName(this)+"
has no "+SOURCE_SENSOR.getName()+" and no "+TRIGGER_SENSORS.getName());
    +        }
    +        if (producer.equals(entity) && (targetSensor.equals(sourceSensor) ||
triggerSensors.contains(targetSensor))) {
                 // We cannot call getTransformation() here to log the tranformation, as it
will attempt
                 // to resolve the transformation, which will cause the entity initialization
thread to block
                 LOG.error("Refusing to add an enricher which reads and publishes on the same
sensor: "+
    -                producer+"."+sourceSensor+" (computing transformation with "+JavaClassNames.simpleClassName(this)+")");
    +                producer+"->"+targetSensor+" (computing transformation with "+JavaClassNames.simpleClassName(this)+")");
                 // we don't throw because this error may manifest itself after a lengthy
deployment, 
                 // and failing it at that point simply because of an enricher is not very
pleasant
                 // (at least not until we have good re-run support across the board)
                 return;
             }
             
    -        subscriptions().subscribe(producer, sourceSensor, this);
    +        if (sourceSensor != null) {
    +            subscriptions().subscribe(MutableMap.of("notifyOfInitialValue", true), producer,
sourceSensor, this);
    +        }
             
    -        if (sourceSensor instanceof AttributeSensor) {
    -            Object value = producer.getAttribute((AttributeSensor<?>)sourceSensor);
    -            // TODO would be useful to have a convenience to "subscribeAndThenIfItIsAlreadySetRunItOnce"
    -            if (value!=null) {
    -                onEvent(new BasicSensorEvent(sourceSensor, producer, value, -1));
    +        if (triggerSensors.size() > 0) {
    +            SensorEventListener<Object> triggerListener = new SensorEventListener<Object>()
{
    +                @Override public void onEvent(SensorEvent<Object> event) {
    +                    if (sourceSensor != null) {
    +                        // Simulate an event, as though our sourceSensor changed
    +                        Object value = producer.getAttribute((AttributeSensor<?>)sourceSensor);
    +                        AbstractTransformer.this.onEvent(new BasicSensorEvent(sourceSensor,
producer, value, event.getTimestamp()));
    +                    } else {
    +                        // Dangerous casting, but will assume that the transform doesn't
care - otherwise 
    +                        // it would have declared a sourceSensor!
    +                        AbstractTransformer.this.onEvent((SensorEvent<T>)event);
    +                    }
    +                }
    +            };
    +            for (Object sensor : triggerSensors) {
    +                if (sensor instanceof String) {
    --- End diff --
    
    See `TransformingEnricherTest.testTriggeringSensorNamesResolvedFromStrings` for a test
that it works with strings passed in. Given that it's yaml, then a user could easily pass
in such a String value. I'm not convinced that our `TypeCoercions` would kick-in for the values
within the triggerSensors' `List`, to automatically convert form String to sensor.


---
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.
---

Mime
View raw message