[
https://issues.apache.org/jira/browse/BROOKLYN-278?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15298151#comment-15298151
]
ASF GitHub Bot commented on BROOKLYN-278:
-----------------------------------------
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/154#discussion_r64388028
--- Diff:
core/src/main/java/org/apache/brooklyn/enricher/stock/Propagator.java ---
@@ -87,24 +93,41 @@ public Propagator() {
public void setEntity(EntityLocal entity) {
super.setEntity(entity);
- this.producer = getConfig(PRODUCER) == null ? entity :
getConfig(PRODUCER);
- boolean sensorMappingSet = getConfig(SENSOR_MAPPING)!=null;
- MutableMap<Sensor<?>,Sensor<?>> sensorMappingTemp =
MutableMap.copyOf(getConfig(SENSOR_MAPPING));
- this.propagatingAll =
Boolean.TRUE.equals(getConfig(PROPAGATING_ALL)) ||
getConfig(PROPAGATING_ALL_BUT)!=null;
+ this.producer = getConfig(PRODUCER);
+ this.sensorMapping =
resolveSensorMappings(getConfig(SENSOR_MAPPING));
+ this.propagatingAllBut =
resolveSensorCollection(getConfig(PROPAGATING_ALL_BUT));
+ this.propagatingAll =
Boolean.TRUE.equals(getConfig(PROPAGATING_ALL)) || propagatingAllBut.size() > 0;
+ Collection<Sensor<?>> propagating =
resolveSensorCollection(getConfig(PROPAGATING));
- if (getConfig(PROPAGATING) != null) {
+ if (producer == null) {
+ throw new IllegalStateException("Propagator enricher "+this+"
missing config '"+PRODUCER.getName());
+ }
+ if (propagating.isEmpty() && sensorMapping.isEmpty() &&
!propagatingAll) {
+ throw new IllegalStateException("Propagator enricher "+this+"
must have 'propagating' and/or 'sensorMapping', or 'propagatingAll' or
'propagatingAllBut' set");
+ }
+ if (entity.equals(producer)) {
if (propagatingAll) {
- throw new IllegalStateException("Propagator enricher
"+this+" must not have 'propagating' set at same time as either
'propagatingAll' or 'propagatingAllBut'");
+ throw new IllegalStateException("Propagator enricher
"+this+" must not have "+PROPAGATING_ALL.getName()+" or
"+PROPAGATING_ALL_BUT.getName()+", when publishing to own entity (to avoid
infinite loop)");
+ } else if (propagating.size() > 0) {
+ throw new IllegalStateException("Propagator enricher
"+this+" must not have "+PROPAGATING.getName()+", when publishing to own entity
(to avoid infinite loop)");
+ } else if (filterForKeyEqualsValue(sensorMapping).size() > 0) {
+ Map<? extends Sensor<?>, ? extends Sensor<?>>
selfPublishingSensors = filterForKeyEqualsValue(sensorMapping);
+ throw new IllegalStateException("Propagator enricher
"+this+" must not publish to same sensor in config "+SENSOR_MAPPING.getName()+"
("+selfPublishingSensors.keySet()+"), when publishing to own entity (to avoid
infinite loop)");
}
-
- for (Object sensorO : getConfig(PROPAGATING)) {
- Sensor<?> sensor =
Tasks.resolving(sensorO).as(Sensor.class).timeout(ValueResolver.REAL_QUICK_WAIT).context(producer).get();
- if (!sensorMappingTemp.containsKey(sensor)) {
- sensorMappingTemp.put(sensor, sensor);
+ }
+ if ((propagating.size() > 0 || sensorMapping.size() > 0) &&
propagatingAll) {
+ throw new IllegalStateException("Propagator enricher "+this+"
must not have 'propagating' or 'sensorMapping' set at same time as either
'propagatingAll' or 'propagatingAllBut'");
+ }
+
+ if (propagating.size() > 0) {
+ for (Sensor<?> sensor : propagating) {
+ if (!sensorMapping.containsKey(sensor)) {
+ sensorMapping.put(sensor, sensor);
}
}
- this.sensorMapping = ImmutableMap.copyOf(sensorMappingTemp);
- this.sensorFilter = new Predicate<Sensor<?>>() {
+ this.sensorMapping = ImmutableMap.copyOf(sensorMapping);
--- End diff --
I would get rid of the "this." myself - it's superfluous here and below. In
particular on this line and L140 it gives the visual impression that
"this.sensorMapping" is different from just "sensorMapping" which is not the
case - it's the same variable in both cases.
> default Propagator enricher can cause infinite subscription event loop
> ----------------------------------------------------------------------
>
> Key: BROOKLYN-278
> URL: https://issues.apache.org/jira/browse/BROOKLYN-278
> Project: Brooklyn
> Issue Type: Bug
> Affects Versions: 0.9.0
> Reporter: Aled Sage
>
> The default {{Propagator}} can cause an infinite loop: it subscribes to all
> sensor events from that entity, and republishes them!
> For example, the blueprint below demonstrates this:
> {noformat}
> location: localhost
> services:
> - type: org.apache.brooklyn.entity.stock.BasicApplication
> brooklyn.enrichers:
> - type: org.apache.brooklyn.enricher.stock.Propagator
> {noformat}
> This shows repeated log messages like:
> {noformat}
> 2016-05-23 14:09:09,069 DEBUG o.a.b.c.entity.AbstractEntity
> [brooklyn-execmanager-B5OU5RXp-6]: Emitting sensor notification
> entity.location.added value LocalhostMachineProvisioningLocation{id=U27ANQ3v,
> name=localhost} on BasicApplicationImpl{id=C8KeD18W}
> {noformat}
> I'd expect such missing propagator config to cause it to fail. Or if it was
> misconfigured to cause an infinite loop, then that would also cause a failure.
> For example, the blueprint below also produces an infinite loop and should be
> forbidden:
> {noformat}
> location: localhost
> services:
> - type: org.apache.brooklyn.entity.stock.BasicApplication
> id: app
> brooklyn.enrichers:
> - type: org.apache.brooklyn.enricher.stock.Propagator
> brooklyn.config:
> producer: $brooklyn:entity("app")
> propagatingAll: true
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
