You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/08/21 17:23:36 UTC

[brooklyn-server] branch master updated (45d015dd5e -> 83b41d0c82)

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git


    from 45d015dd5e tidy names of function feeds
     new 607ea5404e misc better logging
     new faecd8370a replace "unflattened" DSL predicate test with a cleaner "has-element" test
     new 3ccc150992 support key and index in predicate DSL (and check)
     new 379f9184b6 predicate tidy-up and dsl test
     new 7461003c72 add jsonpath for predicates, and list filter, size
     new 907be023cf conditions on some sensor feeds, and code tidy
     new 83b41d0c82 start feeds once entity is fully managed, not before

The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/brooklyn/api/objs/EntityAdjunct.java    |   6 +
 .../java/org/apache/brooklyn/api/sensor/Feed.java  |   2 +-
 .../camp/brooklyn/SshCommandSensorYamlTest.java    |  85 ++++++-
 .../brooklyn/spi/dsl/DslPredicateYamlTest.java     | 120 ++++++++++
 .../apache/brooklyn/core/feed/AbstractFeed.java    |  19 +-
 .../org/apache/brooklyn/core/feed/PollConfig.java  |  15 +-
 .../java/org/apache/brooklyn/core/feed/Poller.java | 116 +++++++---
 .../mgmt/internal/EntityManagementSupport.java     |   7 +-
 .../core/mgmt/rebind/BasicEntityRebindSupport.java |   8 -
 .../core/sensor/AbstractAddTriggerableSensor.java  |  58 ++---
 .../core/sensor/http/HttpRequestSensor.java        |   2 +
 .../brooklyn/core/sensor/ssh/SshCommandSensor.java |   2 +-
 .../brooklyn/entity/group/GroupsChangePolicy.java  |   2 +-
 .../apache/brooklyn/feed/AbstractCommandFeed.java  |  25 +-
 .../brooklyn/feed/function/FunctionFeed.java       |  26 +--
 .../org/apache/brooklyn/feed/http/HttpFeed.java    |  72 +++---
 .../org/apache/brooklyn/feed/shell/ShellFeed.java  |   5 +-
 .../util/core/predicates/DslPredicates.java        | 255 +++++++++++++++------
 .../brooklyn/core/test/entity/TestEntity.java      |   2 +
 .../core/predicates/DslPredicateEntityTest.java    |   4 +-
 .../util/core/predicates/DslPredicateTest.java     |  79 ++++++-
 .../policy/enricher/HttpLatencyDetector.java       |   3 +-
 .../brooklynnode/BrooklynEntityMirrorImpl.java     |   3 +-
 .../java/org/apache/brooklyn/feed/jmx/JmxFeed.java |   6 +-
 .../brooklyn/tasks/kubectl/ContainerSensor.java    |   3 +-
 .../core/sensor/windows/WinRmCommandSensor.java    |   3 +-
 .../windows/WindowsPerformanceCounterFeed.java     |   6 +-
 .../org/apache/brooklyn/util/time/Duration.java    |   4 +-
 28 files changed, 669 insertions(+), 269 deletions(-)
 create mode 100644 camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java


[brooklyn-server] 07/07: start feeds once entity is fully managed, not before

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 83b41d0c82328e93d71d187676427d8a8dc691f0
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Sun Aug 21 17:40:01 2022 +0100

    start feeds once entity is fully managed, not before
    
    otherwise isManagedActive test is false if not fully managed.
    
    also subscribe to service up if feed is enabled only when service is up, and misc tidies.
    
    but might be better to leave this as was, and instead use isManagedActiveOrComingUp (and keep the other changes, just revert to starting feeds directly on feed creation. though this change seems better!)
---
 .../apache/brooklyn/api/objs/EntityAdjunct.java    |  6 ++
 .../java/org/apache/brooklyn/api/sensor/Feed.java  |  2 +-
 .../camp/brooklyn/SshCommandSensorYamlTest.java    | 85 +++++++++++++++++++++-
 .../apache/brooklyn/core/feed/AbstractFeed.java    | 19 ++++-
 .../org/apache/brooklyn/core/feed/PollConfig.java  |  2 +-
 .../java/org/apache/brooklyn/core/feed/Poller.java | 20 +++--
 .../mgmt/internal/EntityManagementSupport.java     |  7 +-
 .../core/mgmt/rebind/BasicEntityRebindSupport.java |  8 --
 .../core/sensor/AbstractAddTriggerableSensor.java  |  2 +
 .../core/sensor/http/HttpRequestSensor.java        |  2 +
 .../brooklyn/core/sensor/ssh/SshCommandSensor.java |  2 +-
 .../apache/brooklyn/feed/AbstractCommandFeed.java  |  6 +-
 .../brooklyn/feed/function/FunctionFeed.java       | 16 +---
 .../org/apache/brooklyn/feed/http/HttpFeed.java    | 10 ++-
 .../org/apache/brooklyn/feed/shell/ShellFeed.java  |  5 +-
 .../policy/enricher/HttpLatencyDetector.java       |  3 +-
 .../brooklynnode/BrooklynEntityMirrorImpl.java     |  3 +-
 .../java/org/apache/brooklyn/feed/jmx/JmxFeed.java |  6 +-
 .../brooklyn/tasks/kubectl/ContainerSensor.java    |  3 +-
 .../core/sensor/windows/WinRmCommandSensor.java    |  3 +-
 .../windows/WindowsPerformanceCounterFeed.java     |  6 +-
 .../org/apache/brooklyn/util/time/Duration.java    |  4 +-
 22 files changed, 161 insertions(+), 59 deletions(-)

diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/EntityAdjunct.java b/api/src/main/java/org/apache/brooklyn/api/objs/EntityAdjunct.java
index af55fecd63..9c7bac58c2 100644
--- a/api/src/main/java/org/apache/brooklyn/api/objs/EntityAdjunct.java
+++ b/api/src/main/java/org/apache/brooklyn/api/objs/EntityAdjunct.java
@@ -55,4 +55,10 @@ public interface EntityAdjunct extends BrooklynObject {
     @Nullable String getUniqueTag();
 
     Map<String, HighlightTuple> getHighlights();
+
+    interface AutoStartEntityAdjunct extends EntityAdjunct {
+        /** for things that should start when the entity is managed, including on rebind;
+         *  replaces logic which started things during creation time */
+        public void start();
+    }
 }
diff --git a/api/src/main/java/org/apache/brooklyn/api/sensor/Feed.java b/api/src/main/java/org/apache/brooklyn/api/sensor/Feed.java
index 01a47148fb..a7364f1973 100644
--- a/api/src/main/java/org/apache/brooklyn/api/sensor/Feed.java
+++ b/api/src/main/java/org/apache/brooklyn/api/sensor/Feed.java
@@ -39,7 +39,7 @@ import com.google.common.annotations.Beta;
  *   </ul>
  */
 @Beta
-public interface Feed extends EntityAdjunct, Rebindable {
+public interface Feed extends EntityAdjunct, EntityAdjunct.AutoStartEntityAdjunct, Rebindable {
 
     /** 
      * True if everything has been _started_ (or it is starting) but not stopped,
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/SshCommandSensorYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/SshCommandSensorYamlTest.java
index e71713e775..644909b555 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/SshCommandSensorYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/SshCommandSensorYamlTest.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.camp.brooklyn;
 import java.util.List;
 import java.util.Map;
 
+import com.google.common.base.Stopwatch;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;
@@ -31,10 +32,12 @@ import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess;
 import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
 import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool.CustomResponse;
 import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool.ExecParams;
 import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.annotations.BeforeMethod;
@@ -199,5 +202,85 @@ public class SshCommandSensorYamlTest extends AbstractYamlTest {
     protected Logger getLogger() {
         return log;
     }
-    
+
+    @Test
+    public void testSshCommandSensorFeedRunsAtStartup() throws Exception {
+        RecordingSshTool.setCustomResponse(".*myCommand.*", new RecordingSshTool.CustomResponse(0, "myResponse", null));
+
+        Entity app = createAndStartApplication(
+                "location:",
+                "  localhost:",
+                "    sshToolClass: "+RecordingSshTool.class.getName(),
+                "services:",
+                "- type: " + VanillaSoftwareProcess.class.getName(),
+                "  brooklyn.config:",
+                "    onbox.base.dir.skipResolution: true",
+                "  brooklyn.initializers:",
+                "  - type: org.apache.brooklyn.core.sensor.ssh.SshCommandSensor",
+                "    brooklyn.config:",
+                "      name: mySensor",
+                "      command: myCommand",
+                "      period: 5s");
+        waitForApplicationTasks(app);
+
+        VanillaSoftwareProcess entity = (VanillaSoftwareProcess) Iterables.getOnlyElement(app.getChildren());
+        Stopwatch sw = Stopwatch.createStarted();
+        EntityAsserts.assertAttributeEqualsEventually(entity, Sensors.newStringSensor("mySensor"), "myResponse");
+        Asserts.assertThat(Duration.of(sw), d -> d.isShorterThan(Duration.seconds(4)));
+    }
+
+
+    @Test(groups="Integration") // because slow
+    public void testSshCommandSensorPeriodicFeedServiceUpFalseDoesNotRunAtStartup() throws Exception {
+        RecordingSshTool.setCustomResponse(".*myCommand.*", new RecordingSshTool.CustomResponse(0, "myResponse", null));
+
+        Stopwatch sw = Stopwatch.createStarted();
+        Entity app = createAndStartApplication(
+                "location:",
+                "  localhost:",
+                "    sshToolClass: "+RecordingSshTool.class.getName(),
+                "services:",
+                "- type: " + VanillaSoftwareProcess.class.getName(),
+                "  brooklyn.config:",
+                "    onbox.base.dir.skipResolution: true",
+                "  brooklyn.initializers:",
+                "  - type: org.apache.brooklyn.core.sensor.ssh.SshCommandSensor",
+                "    brooklyn.config:",
+                "      name: mySensor",
+                "      command: myCommand",
+                "      period: 5s",
+                "      onlyIfServiceUp: true");
+        waitForApplicationTasks(app);
+
+        VanillaSoftwareProcess entity = (VanillaSoftwareProcess) Iterables.getOnlyElement(app.getChildren());
+        EntityAsserts.assertAttributeEqualsEventually(entity, Sensors.newStringSensor("mySensor"), "myResponse");
+        Asserts.assertThat(Duration.of(sw), d -> d.isLongerThan(Duration.seconds(4)));
+    }
+
+    @Test
+    public void testSshCommandSensorTriggeredFeedDoesRunAtStartup() throws Exception {
+        RecordingSshTool.setCustomResponse(".*myCommand.*", new RecordingSshTool.CustomResponse(0, "myResponse", null));
+
+        Entity app = createAndStartApplication(
+                "location:",
+                "  localhost:",
+                "    sshToolClass: "+RecordingSshTool.class.getName(),
+                "services:",
+                "- type: " + VanillaSoftwareProcess.class.getName(),
+                "  brooklyn.config:",
+                "    onbox.base.dir.skipResolution: true",
+                "  brooklyn.initializers:",
+                "  - type: org.apache.brooklyn.core.sensor.ssh.SshCommandSensor",
+                "    brooklyn.config:",
+                "      name: mySensor",
+                "      command: myCommand",
+                "      triggers:",
+                "      - triggerSensor");
+        waitForApplicationTasks(app);
+
+        VanillaSoftwareProcess entity = (VanillaSoftwareProcess) Iterables.getOnlyElement(app.getChildren());
+        EntityAsserts.assertAttributeEqualsEventually(MutableMap.of("timeout", Duration.seconds(4)), entity, Sensors.newStringSensor("mySensor"), "myResponse");
+    }
+
+
 }
diff --git a/core/src/main/java/org/apache/brooklyn/core/feed/AbstractFeed.java b/core/src/main/java/org/apache/brooklyn/core/feed/AbstractFeed.java
index 7a50fff4db..43c4f7a69b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/feed/AbstractFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/core/feed/AbstractFeed.java
@@ -20,6 +20,8 @@ package org.apache.brooklyn.core.feed;
 
 import java.util.Collection;
 
+import com.google.common.annotations.Beta;
+import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntityLocal;
 import org.apache.brooklyn.api.mgmt.rebind.RebindSupport;
 import org.apache.brooklyn.api.mgmt.rebind.mementos.FeedMemento;
@@ -28,6 +30,7 @@ import org.apache.brooklyn.api.sensor.Sensor;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.BrooklynFeatureEnablement;
 import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityAdjuncts;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.rebind.BasicFeedRebindSupport;
@@ -38,6 +41,8 @@ import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /** 
  * Captures common fields and processes for sensor feeds.
  * These generally poll or subscribe to get sensor values for an entity.
@@ -56,7 +61,17 @@ public abstract class AbstractFeed extends AbstractEntityAdjunct implements Feed
 
     public AbstractFeed() {
     }
-    
+
+    @Beta
+    public static <T extends AbstractFeed> T initAndMaybeStart(T feed, Entity entity) {
+        feed.setEntity(checkNotNull((EntityInternal)entity, "entity"));
+        if (Entities.isManagedActive(entity)) {
+            // start it is entity is already managed (dynamic addition); otherwise rely on EntityManagementSupport to start us (initializer-based addition and after rebind)
+            feed.start();
+        }
+        return feed;
+    }
+
     // Ensure idempotent, as called in builders (in case not registered with entity), and also called
     // when registering with entity
     @Override
@@ -96,7 +111,7 @@ public abstract class AbstractFeed extends AbstractEntityAdjunct implements Feed
     @Override
     public void start() {
         if (log.isDebugEnabled()) log.debug("Starting feed {} for {}", this, entity);
-        if (activated) { 
+        if (activated) {
             throw new IllegalStateException(String.format("Attempt to start feed %s of entity %s when already running", 
                     this, entity));
         }
diff --git a/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java b/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java
index 4533a73b7a..8e6893065e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java
+++ b/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java
@@ -101,7 +101,7 @@ public class PollConfig<V, T, F extends PollConfig<V, T, F>> extends FeedConfig<
     
     @Override protected MutableList<Object> toStringOtherFields() {
         MutableList<Object> result = super.toStringOtherFields().appendIfNotNull(description);
-        if (period>0 && period <= Duration.PRACTICALLY_FOREVER.toMilliseconds()) result.append("period: "+Duration.of(period));
+        if (period>0 && period < Duration.PRACTICALLY_FOREVER.toMilliseconds()) result.append("period: "+Duration.of(period));
         if (otherTriggers!=null) result.append("triggers: "+otherTriggers);
         return result;
     }
diff --git a/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java b/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java
index 2b1a09488e..3b2c331a09 100644
--- a/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java
+++ b/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java
@@ -90,12 +90,19 @@ public class Poller<V> {
             DelegatingPollHandler handlerDelegate = new DelegatingPollHandler(handlers);
             boolean subscribed = false;
             for (PollConfig pc: configs) {
+                Set<Pair<Entity, Sensor>> triggersResolved = MutableSet.of();
                 if (pc.getOtherTriggers()!=null) {
-                    List<Pair<Entity, Sensor>> triggersResolved = AbstractAddTriggerableSensor.resolveTriggers(feed.getEntity(), pc.getOtherTriggers());
-                    for (Pair<Entity, Sensor> pair : triggersResolved) {// TODO initial, condition
-                        subscribe(pollJob, handlerDelegate, pair.getLeft(), pair.getRight(), pc.getCondition());
-                        subscribed = true;
-                    }
+                    triggersResolved.addAll(AbstractAddTriggerableSensor.resolveTriggers(feed.getEntity(), pc.getOtherTriggers()));
+                }
+                if (onlyIfServiceUp) {
+                    // if 'onlyIfServiceUp' is set then automatically subscribe to that sensor.
+                    // this is the default for ssh and other sensors which need a target machine. for others it defaults false.
+                    triggersResolved.add(Pair.of(feed.getEntity(), Attributes.SERVICE_UP));
+                }
+
+                for (Pair<Entity, Sensor> pair : triggersResolved) {
+                    subscribe(pollJob, handlerDelegate, pair.getLeft(), pair.getRight(), pc.getCondition());
+                    subscribed = true;
                 }
             }
             if (minPeriodMillis>0 && (minPeriodMillis < Duration.PRACTICALLY_FOREVER.toMilliseconds() || !subscribed)) {
@@ -124,7 +131,6 @@ public class Poller<V> {
             this.pollTriggerEntity = sensorSource;
             this.pollTriggerSensor = sensor;
             this.pollCondition = pollCondition;
-
             wrappedJob = new Runnable() {
                 @Override
                 public void run() {
@@ -200,7 +206,6 @@ public class Poller<V> {
             throw new IllegalStateException(String.format("Attempt to start poller %s of entity %s when already running", 
                     this, entity));
         }
-        
         started = true;
         
         for (final Callable<?> oneOffJob : oneOffJobs) {
@@ -216,6 +221,7 @@ public class Poller<V> {
 
             Callable<Task<?>> tf = () -> {
                 DynamicSequentialTask<Void> task = new DynamicSequentialTask<Void>(MutableMap.of("displayName", scheduleName, "entity", entity),
+                        /** TODO why the hell is this running before the entity is managed??? and remove logging, and invocationCount=100. */
                         new Callable<Void>() { @Override public Void call() {
                             if (!Entities.isManagedActive(entity)) {
                                 return null;
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
index 7e71a9cc57..a758f21000 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
@@ -210,7 +210,7 @@ public class EntityManagementSupport {
             }
             
             /*
-             * TODO framework starting events - phase 1, including rebind
+             * framework starting events - phase 1, including rebind
              *  - establish hierarchy (child, groups, etc; construction if necessary on rebind)
              *  - set location
              *  - set local config values
@@ -223,6 +223,11 @@ public class EntityManagementSupport {
             
             if (!isReadOnly()) {
                 entity.onManagementStarting();
+
+                // start those policies etc which are labelled as auto-start
+                entity.policies().forEach(adj -> { if (adj instanceof EntityAdjunct.AutoStartEntityAdjunct) ((EntityAdjunct.AutoStartEntityAdjunct)adj).start(); });
+                entity.enrichers().forEach(adj -> { if (adj instanceof EntityAdjunct.AutoStartEntityAdjunct) ((EntityAdjunct.AutoStartEntityAdjunct)adj).start(); });
+                entity.feeds().forEach(f -> { if (!f.isActivated()) f.start(); });
             }
         } catch (Throwable t) {
             managementFailed.set(true);
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java
index 2f2293d1e8..c4f7ba9a74 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java
@@ -159,14 +159,6 @@ public class BasicEntityRebindSupport extends AbstractBrooklynObjectRebindSuppor
                 } catch (Exception e) {
                     rebindContext.getExceptionHandler().onAddFeedFailed(entity, feed, e);
                 }
-                
-                try {
-                    if (!rebindContext.isReadOnly(feed)) {
-                        feed.start();
-                    }
-                } catch (Exception e) {
-                    rebindContext.getExceptionHandler().onRebindFailed(BrooklynObjectType.ENTITY, entity, e);
-                }
             } else {
                 LOG.warn("Feed not found; discarding feed {} of entity {}({})",
                         new Object[] {feedId, memento.getType(), memento.getId()});
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java b/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java
index 0d28f31b7f..e555e46ad4 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java
@@ -53,6 +53,8 @@ public abstract class AbstractAddTriggerableSensor<T> extends AbstractAddSensorF
             "Sensors which should trigger this feed, supplied with list of maps containing sensor (name or sensor instance) and entity (ID or entity instance), or just sensor names or just one sensor");
     public static final ConfigKey<DslPredicates.DslPredicate> CONDITION = ConfigKeys.newConfigKey(DslPredicates.DslPredicate.class, "condition", "Optional condition required for this sensor feed to run");
 
+    public static final ConfigKey<Boolean> ONLY_IF_SERVICE_UP = ConfigKeys.newBooleanConfigKey("onlyIfServiceUp", "Whether to run only if service is up.", null);
+
     protected AbstractAddTriggerableSensor() {}
     public AbstractAddTriggerableSensor(ConfigBag parameters) {
         super(parameters);
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/http/HttpRequestSensor.java b/core/src/main/java/org/apache/brooklyn/core/sensor/http/HttpRequestSensor.java
index e078eaffbe..0c6932badc 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/http/HttpRequestSensor.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/http/HttpRequestSensor.java
@@ -37,6 +37,7 @@ import org.apache.brooklyn.feed.http.HttpValueFunctions;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Functionals;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.http.HttpToolResponse;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
@@ -135,6 +136,7 @@ public class HttpRequestSensor<T> extends AbstractAddTriggerableSensor<T> {
                 .baseUri(uri)
                 .credentialsIfNotNull(username, password)
                 .preemptiveBasicAuth(Boolean.TRUE.equals(preemptiveBasicAuth))
+                .onlyIfServiceUp(Maybe.ofDisallowingNull(EntityInitializers.resolve(initParams(), ONLY_IF_SERVICE_UP)).or(false))
                 .poll(pollConfig);
 
         if (headers != null) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/ssh/SshCommandSensor.java b/core/src/main/java/org/apache/brooklyn/core/sensor/ssh/SshCommandSensor.java
index cdc77112ad..3632257361 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/ssh/SshCommandSensor.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/ssh/SshCommandSensor.java
@@ -165,7 +165,7 @@ public final class SshCommandSensor<T> extends AbstractAddTriggerableSensor<T> {
 
         SshFeed feed = SshFeed.builder()
                 .entity(entity)
-                .onlyIfServiceUp()
+                .onlyIfServiceUp(Maybe.ofDisallowingNull(EntityInitializers.resolve(params, ONLY_IF_SERVICE_UP)).or(true))
                 .poll(pollConfig)
                 .build();
 
diff --git a/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java b/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java
index c4fab5c647..5a9921d166 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java
@@ -38,6 +38,7 @@ import org.apache.brooklyn.core.feed.DelegatingPollHandler;
 import org.apache.brooklyn.core.feed.Poller;
 import org.apache.brooklyn.core.location.Locations;
 import org.apache.brooklyn.core.sensor.AbstractAddTriggerableSensor;
+import org.apache.brooklyn.feed.function.FunctionFeed;
 import org.apache.brooklyn.feed.ssh.SshPollValue;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -157,10 +158,7 @@ public abstract class AbstractCommandFeed extends AbstractFeed {
 
         public T build() {
             built = true;
-            T result = instantiateFeed();
-            result.setEntity(checkNotNull((EntityLocal)entity, "entity"));
-            result.start();
-            return result;
+            return AbstractFeed.initAndMaybeStart(instantiateFeed(), entity);
         }
 
         @Override
diff --git a/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java b/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java
index 378bfe6b56..c9119d13c7 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java
@@ -21,23 +21,13 @@ package org.apache.brooklyn.feed.function;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.util.List;
-import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.feed.AbstractFeed;
-import org.apache.brooklyn.core.feed.AttributePollHandler;
-import org.apache.brooklyn.core.feed.DelegatingPollHandler;
-import org.apache.brooklyn.core.sensor.AbstractAddTriggerableSensor;
-import org.apache.brooklyn.util.core.javalang.BrooklynHttpConfig;
-import org.apache.brooklyn.util.http.HttpToolResponse;
-import org.apache.brooklyn.util.http.auth.UsernamePassword;
-import org.apache.brooklyn.util.http.executor.HttpRequest;
-import org.apache.brooklyn.util.http.executor.HttpResponse;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -46,7 +36,6 @@ import com.google.common.base.Objects;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.SetMultimap;
-import com.google.common.collect.Sets;
 import com.google.common.reflect.TypeToken;
 
 /**
@@ -146,10 +135,7 @@ public class FunctionFeed extends AbstractFeed {
         }
         public FunctionFeed build() {
             built = true;
-            FunctionFeed result = new FunctionFeed(this);
-            result.setEntity(checkNotNull((EntityInternal)entity, "entity"));
-            result.start();
-            return result;
+            return AbstractFeed.initAndMaybeStart(new FunctionFeed(this), entity);
         }
         @Override
         protected void finalize() {
diff --git a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java
index 4a17f04b84..8af0dcfd86 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java
@@ -38,6 +38,7 @@ import org.apache.brooklyn.core.location.Locations;
 import org.apache.brooklyn.core.location.Machines;
 import org.apache.brooklyn.core.location.internal.LocationInternal;
 import org.apache.brooklyn.core.sensor.AbstractAddTriggerableSensor;
+import org.apache.brooklyn.feed.function.FunctionFeed;
 import org.apache.brooklyn.util.core.javalang.BrooklynHttpConfig;
 import org.apache.brooklyn.util.executor.HttpExecutorFactory;
 import org.apache.brooklyn.util.guava.Maybe;
@@ -253,11 +254,18 @@ public class HttpFeed extends AbstractFeed {
             }
         }
         public HttpFeed build() {
+            return build(null);
+        }
+        /** normally no arg is required, but if feed is not attached to entity, it will need starting here */
+        public HttpFeed build(Boolean feedStart) {
             built = true;
             HttpFeed result = new HttpFeed(this);
             result.setEntity(checkNotNull((EntityLocal)entity, "entity"));
             if (suspended) result.suspend();
-            result.start();
+            if ((feedStart==null && Entities.isManagedActive(entity)) || Boolean.TRUE.equals(feedStart)) {
+                // this feed is used a lot without being attached to an entity, not ideal, but let's support it
+                result.start();
+            }
             return result;
         }
         @Override
diff --git a/core/src/main/java/org/apache/brooklyn/feed/shell/ShellFeed.java b/core/src/main/java/org/apache/brooklyn/feed/shell/ShellFeed.java
index ab617f1f39..57dbba4121 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/shell/ShellFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/shell/ShellFeed.java
@@ -138,10 +138,7 @@ public class ShellFeed extends AbstractFeed {
         }
         public ShellFeed build() {
             built = true;
-            ShellFeed result = new ShellFeed(this);
-            result.setEntity(checkNotNull((EntityLocal)entity, "entity"));
-            result.start();
-            return result;
+            return AbstractFeed.initAndMaybeStart(new ShellFeed(this), entity);
         }
         @Override
         protected void finalize() {
diff --git a/policy/src/main/java/org/apache/brooklyn/policy/enricher/HttpLatencyDetector.java b/policy/src/main/java/org/apache/brooklyn/policy/enricher/HttpLatencyDetector.java
index 95d29b628e..d8a62740ba 100644
--- a/policy/src/main/java/org/apache/brooklyn/policy/enricher/HttpLatencyDetector.java
+++ b/policy/src/main/java/org/apache/brooklyn/policy/enricher/HttpLatencyDetector.java
@@ -151,7 +151,7 @@ public class HttpLatencyDetector extends AbstractEnricher implements Enricher {
                         .onResult(new ComputeLatencyAndRecordError())
                         .setOnException(null))
                 .suspended()
-                .build();
+                .build(true);
 
         if (getUniqueTag()==null) 
             uniqueTag = JavaClassNames.simpleClassName(getClass())+":"+
@@ -195,6 +195,7 @@ public class HttpLatencyDetector extends AbstractEnricher implements Enricher {
             Boolean currentVal = entity.getAttribute(Startable.SERVICE_UP);
             if (currentVal != null) {
                 AtomicReferences.setIfDifferent(serviceUp, currentVal);
+                updateEnablement();
             }
         }
 
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorImpl.java b/software/base/src/main/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorImpl.java
index 8b28607a05..7ff2c3a03c 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorImpl.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorImpl.java
@@ -139,7 +139,8 @@ public class BrooklynEntityMirrorImpl extends AbstractEntity implements Brooklyn
                         return null;
                     }
                 }))
-            .poll(HttpPollConfig.forSensor(MIRROR_SUMMARY).onSuccess(new MirrorSummary())).build();
+            .poll(HttpPollConfig.forSensor(MIRROR_SUMMARY).onSuccess(new MirrorSummary()))
+            .build(true);
 
         populateEffectors();
     }
diff --git a/software/base/src/main/java/org/apache/brooklyn/feed/jmx/JmxFeed.java b/software/base/src/main/java/org/apache/brooklyn/feed/jmx/JmxFeed.java
index 4220d05118..6789c9e374 100644
--- a/software/base/src/main/java/org/apache/brooklyn/feed/jmx/JmxFeed.java
+++ b/software/base/src/main/java/org/apache/brooklyn/feed/jmx/JmxFeed.java
@@ -41,6 +41,7 @@ import org.apache.brooklyn.core.feed.DelegatingPollHandler;
 import org.apache.brooklyn.core.feed.PollHandler;
 import org.apache.brooklyn.core.feed.Poller;
 import org.apache.brooklyn.entity.software.base.SoftwareProcessImpl;
+import org.apache.brooklyn.feed.windows.WindowsPerformanceCounterFeed;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -167,10 +168,7 @@ public class JmxFeed extends AbstractFeed {
         }
         public JmxFeed build() {
             built = true;
-            JmxFeed result = new JmxFeed(this);
-            result.setEntity(checkNotNull((EntityLocal)entity, "entity"));
-            result.start();
-            return result;
+            return AbstractFeed.initAndMaybeStart(new JmxFeed(this), entity);
         }
         @Override
         protected void finalize() {
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerSensor.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerSensor.java
index dbcf071622..a757c239e8 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerSensor.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerSensor.java
@@ -32,6 +32,7 @@ import org.apache.brooklyn.feed.function.FunctionFeed;
 import org.apache.brooklyn.feed.function.FunctionPollConfig;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -69,7 +70,7 @@ public class ContainerSensor<T> extends AbstractAddTriggerableSensor<T> implemen
 
         ((EntityInternal) entity).feeds().add(FunctionFeed.builder()
                 .entity(entity)
-                .onlyIfServiceUp()
+                .onlyIfServiceUp(Maybe.ofDisallowingNull(EntityInitializers.resolve(initParams(), ONLY_IF_SERVICE_UP)).or(false))
                 .poll(poll)
                 .build());
     }
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/core/sensor/windows/WinRmCommandSensor.java b/software/winrm/src/main/java/org/apache/brooklyn/core/sensor/windows/WinRmCommandSensor.java
index 08d9a2ec03..8e86435d96 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/core/sensor/windows/WinRmCommandSensor.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/core/sensor/windows/WinRmCommandSensor.java
@@ -42,6 +42,7 @@ import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
 import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
@@ -136,7 +137,7 @@ public final class WinRmCommandSensor<T> extends AbstractAddTriggerableSensor<T>
 
         CmdFeed feed = CmdFeed.builder()
                 .entity(entity)
-                .onlyIfServiceUp()
+                .onlyIfServiceUp(Maybe.ofDisallowingNull(EntityInitializers.resolve(initParams(), ONLY_IF_SERVICE_UP)).or(true))
                 .poll(pollConfig)
                 .build();
 
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/feed/windows/WindowsPerformanceCounterFeed.java b/software/winrm/src/main/java/org/apache/brooklyn/feed/windows/WindowsPerformanceCounterFeed.java
index 55b273ee74..4840d6ade0 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/feed/windows/WindowsPerformanceCounterFeed.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/feed/windows/WindowsPerformanceCounterFeed.java
@@ -46,6 +46,7 @@ import org.apache.brooklyn.core.feed.PollHandler;
 import org.apache.brooklyn.core.feed.Poller;
 import org.apache.brooklyn.core.location.Machines;
 import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.feed.function.FunctionFeed;
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse;
@@ -150,10 +151,7 @@ public class WindowsPerformanceCounterFeed extends AbstractFeed {
         }
         public WindowsPerformanceCounterFeed build() {
             built = true;
-            WindowsPerformanceCounterFeed result = new WindowsPerformanceCounterFeed(this);
-            result.setEntity(checkNotNull((EntityLocal)entity, "entity"));
-            result.start();
-            return result;
+            return AbstractFeed.initAndMaybeStart(new WindowsPerformanceCounterFeed(this), entity);
         }
         @Override
         protected void finalize() {
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java b/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
index aaf562b072..e06315dfc0 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/time/Duration.java
@@ -50,6 +50,8 @@ public class Duration implements Comparable<Duration>, Serializable {
     
     /** longest supported duration, 2^{63}-1 nanoseconds, approx ten billion seconds, or 300 years */ 
     public static final Duration PRACTICALLY_FOREVER = of(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
+    /** used to indicate forever */
+    private static final Duration ALMOST_PRACTICALLY_FOREVER = PRACTICALLY_FOREVER.subtract(Duration.days(365*50));
 
     private final long nanos;
 
@@ -71,7 +73,7 @@ public class Duration implements Comparable<Duration>, Serializable {
 
     @Override
     public String toString() {
-        if (equals(PRACTICALLY_FOREVER)) return "forever";
+        if (isLongerThan(ALMOST_PRACTICALLY_FOREVER)) return "forever";
         return Time.makeTimeStringExact(this);
     }
 


[brooklyn-server] 02/07: replace "unflattened" DSL predicate test with a cleaner "has-element" test

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit faecd8370a3b7989b5f40d73b4224b2e5e7422c6
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Aug 19 12:20:49 2022 +0100

    replace "unflattened" DSL predicate test with a cleaner "has-element" test
---
 .../util/core/predicates/DslPredicates.java        | 84 +++++++++++++---------
 .../util/core/predicates/DslPredicateTest.java     | 72 +++++++++++++++----
 2 files changed, 109 insertions(+), 47 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
index 210bfbcc7f..43074054a5 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
@@ -84,7 +84,7 @@ public class DslPredicates {
         init();
     }
 
-    public static enum WhenPresencePredicate {
+    public enum WhenPresencePredicate {
         /** value cannot be resolved (not even as null) */ ABSENT,
         /** value is null or cannot be resolved */ ABSENT_OR_NULL,
         /** value is available, but might be null */ PRESENT,
@@ -179,6 +179,10 @@ public class DslPredicates {
         public List<DslPredicate> any;
         public List<DslPredicate> all;
 
+        public @JsonProperty("has-element") DslPredicate hasElement;
+//        public @JsonProperty("at-key") DslPredicate containsKey;
+//        public @JsonProperty("at-index") DslPredicate containsValue;
+
         public WhenPresencePredicate when;
 
         public @JsonProperty("in-range") Range inRange;
@@ -265,6 +269,32 @@ public class DslPredicates {
 
             checkWhen(when, result, checker);
 
+            checker.check(hasElement, result, (test,value) -> {
+                if (value instanceof Map) value = ((Map)value).entrySet();
+                if (value instanceof Iterable) {
+                    for (Object v : ((Iterable) value)) {
+                        if (test.apply((T) v)) return true;
+                    }
+                }
+                return false;
+            });
+
+            // TODO at-key, at-value; preserver order
+//            checker.check(element, test -> nestedPredicateCheck(test, result));
+//
+//            if (result.isPresent() && result.get() instanceof Iterable) {
+//                // iterate through lists
+//                for (Object v : ((Iterable) result.get())) {
+//                    CheckCounts checker2 = new CheckCounts();
+//                    applyToResolved(v instanceof Maybe ? (Maybe) v : Maybe.of(v), checker2);
+//                    if (checker2.allPassed(true)) {
+//                        checker.add(checker2);
+//                        return;
+//                    }
+//                }
+//                // did not pass when flattened; try with unflattened
+//            }
+
             checker.check(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
             checker.check(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
 
@@ -302,7 +332,12 @@ public class DslPredicates {
         }
 
         protected boolean nestedPredicateCheck(DslPredicate p, Maybe<Object> result) {
-            return result.isPresent() ? p.apply(result.get()) : p instanceof DslEntityPredicateDefault ? ((DslEntityPredicateDefault)p).applyToResolved(result) : false;
+            return result.isPresent()
+                    ? p.apply(result.get())
+                    : p instanceof DslPredicateBase
+                    // in case it does a when: absent check
+                    ? ((DslEntityPredicateDefault)p).applyToResolved(result)
+                    : false;
         }
     }
 
@@ -324,9 +359,6 @@ public class DslPredicates {
 
         public Object target;
 
-        /** test to be applied prior to any flattening of lists (eg if targetting children */
-        public DslPredicate unflattened;
-
         public String config;
         public String sensor;
         public DslPredicate tag;
@@ -345,36 +377,6 @@ public class DslPredicates {
                 result = resolver.getMaybe();
             }
 
-            return result;
-        }
-
-        protected Maybe<Object> resolveTargetStringAgainstInput(String target, Object input) {
-            if ("location".equals(target) && input instanceof Entity) return Maybe.of( Locations.getLocationsCheckingAncestors(null, (Entity)input) );
-            if ("children".equals(target) && input instanceof Entity) return Maybe.of( ((Entity)input).getChildren() );
-            return Maybe.absent("Unsupported target '"+target+"' on input "+input);
-        }
-
-        @Override
-        public void applyToResolved(Maybe<Object> result, CheckCounts checker) {
-            checker.check(unflattened, test -> nestedPredicateCheck(test, result));
-
-            if (result.isPresent() && result.get() instanceof Iterable) {
-                // iterate through lists
-                for (Object v : ((Iterable) result.get())) {
-                    CheckCounts checker2 = new CheckCounts();
-                    applyToResolvedFlattened(v instanceof Maybe ? (Maybe) v : Maybe.of(v), checker2);
-                    if (checker2.allPassed(true)) {
-                        checker.add(checker2);
-                        return;
-                    }
-                }
-                // did not pass when flattened; try with unflattened
-            }
-
-            applyToResolvedFlattened(result, checker);
-        }
-
-        public void applyToResolvedFlattened(Maybe<Object> result, CheckCounts checker) {
             if (config!=null) {
                 if (sensor!=null) {
                     throw new IllegalStateException("One predicate cannot specify to test both config key '"+config+"' and sensor '"+sensor+"'; use 'all' with children");
@@ -406,7 +408,19 @@ public class DslPredicates {
                 }
             }
 
+            return result;
+        }
+
+        protected Maybe<Object> resolveTargetStringAgainstInput(String target, Object input) {
+            if ("location".equals(target) && input instanceof Entity) return Maybe.of( Locations.getLocationsCheckingAncestors(null, (Entity)input) );
+            if ("children".equals(target) && input instanceof Entity) return Maybe.of( ((Entity)input).getChildren() );
+            return Maybe.absent("Unsupported target '"+target+"' on input "+input);
+        }
+
+        @Override
+        public void applyToResolved(Maybe<Object> result, CheckCounts checker) {
             super.applyToResolved(result, checker);
+
             checker.check(tag, result, this::checkTag);
 
             if (checker.checksDefined==0) {
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
index ff88c116d0..ba06b7e4ef 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
@@ -55,18 +55,16 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
     }
 
     @Test
-    public void testImplicitEquals() {
+    public void testImplicitEqualsAsDslPredicate() {
         Predicate p = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
         Asserts.assertTrue(p.test("x"));
         Asserts.assertFalse(p.test("y"));
     }
 
     @Test
-    public void testImplicitEqualsAsPredicateNotSupported() {
+    public void testImplicitEqualsAsRawPredicateNotSupported() {
         Asserts.assertFailsWith(() -> {
                 Predicate p = TypeCoercions.coerce("x", Predicate.class);
-//                Asserts.assertTrue(p.test("x"));
-//                Asserts.assertFalse(p.test("y"));
         }, e -> {
                 Asserts.assertStringContainsIgnoreCase(e.toString(), "cannot coerce", "string", "predicate");
                 return true;
@@ -217,8 +215,43 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         }
     }
 
+    // auto-unflattening was too weird; instead prompt for "element", also "map-key", "map-value"
+//    @Test
+//    public void testAllWithListWithVariousFlattening() {
+//        Asserts.assertTrue(SetAllowingEqualsToList.of(MutableSet.of("y", "x")).equals(MutableList.of("x", "y")));
+//
+//        DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("equals", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y")));
+//        // list not equal because of order and equal
+//        Asserts.assertFalse(p.test(Arrays.asList("y", "x")));
+//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
+//        // set equality _does_ match without order
+//        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
+//
+//        // "all" works because it attempts unflattened at all, then flattens on each test
+//        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("y", "x")));
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+//        // set equality _does_ match!
+//        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
+//
+//        // specify unflattening also works, but is unnecessary
+//        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("all", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+//
+//        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("equals", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
+//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y")));
+//
+//        p = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
+//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+//
+////        DslPredicates.DslPredicate
+//                p = TypeCoercions.coerce(MutableMap.of("unflattened", "x"), DslPredicates.DslPredicate.class);
+//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
+//    }
     @Test
-    public void testAllWithListWithVariousFlattening() {
+    public void testListsAndElement() {
         Asserts.assertTrue(SetAllowingEqualsToList.of(MutableSet.of("y", "x")).equals(MutableList.of("x", "y")));
 
         DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("equals", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
@@ -229,16 +262,31 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         // set equality _does_ match without order
         Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
 
-        // "all" works because it attempts unflattened at all, then flattens on each test
-        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
+        // no longer automatically unflatted
+        p = TypeCoercions.coerce(MutableMap.of("equals", "x"), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse(p.test(Arrays.asList("y", "x")));
+        Asserts.assertFalse(p.test(Arrays.asList("x")));
+        Asserts.assertTrue(p.test("x"));
+        // and implicit equals gives error
+        Asserts.assertFailsWith(() -> {
+                    DslPredicates.DslPredicate p2 = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
+                    Asserts.assertFalse(p2.test(Arrays.asList("x")));
+                }, e -> Asserts.expectedFailureContainsIgnoreCase(e, "explicit", "equals"));
+
+        // "has-element" now supported
+        p = TypeCoercions.coerce(MutableMap.of("has-element", "x"), DslPredicates.DslPredicate.class);
         Asserts.assertTrue(p.test(Arrays.asList("y", "x")));
-        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
-        // set equality _does_ match!
-        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
+        Asserts.assertFalse(p.test(Arrays.asList("y", "z")));
 
-        // specify unflattening also works, but is unnecessary
-        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("all", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
+        // and can be nested to require multiple elements
+        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of(MutableMap.of("has-element", "x"), MutableMap.of("has-element","y"))), DslPredicates.DslPredicate.class);
         Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
+        Asserts.assertFalse(p.test(Arrays.asList("x", "z")));
+
+        // and can take another predicate
+        p = TypeCoercions.coerce(MutableMap.of("has-element", MutableMap.of("glob", "?")), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(Arrays.asList("xx", "y")));
+        Asserts.assertFalse(p.test(Arrays.asList("xx", "yy")));
     }
 
     @Test


[brooklyn-server] 05/07: add jsonpath for predicates, and list filter, size

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 7461003c72eea27bcb63f9766e7cabaad59a592f
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Aug 19 15:42:24 2022 +0100

    add jsonpath for predicates, and list filter, size
---
 .../util/core/predicates/DslPredicates.java        | 73 ++++++++++++++++++++--
 .../util/core/predicates/DslPredicateTest.java     | 24 +++++++
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
index 5d4ff03a39..d027292fd9 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.util.core.predicates;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.util.TokenBuffer;
@@ -29,6 +30,7 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.reflect.TypeToken;
+import com.jayway.jsonpath.JsonPath;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.api.objs.Configurable;
@@ -37,6 +39,7 @@ import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.location.Locations;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonSerializationUtils;
 import org.apache.brooklyn.core.resolve.jackson.JsonSymbolDependentDeserializer;
 import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
@@ -46,6 +49,7 @@ import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.flags.BrooklynTypeNameResolution;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.core.json.BrooklynObjectsJsonMapper;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.core.task.ValueResolver;
@@ -194,6 +198,8 @@ public class DslPredicates {
         public List<DslPredicate> all;
 
         public @JsonProperty("has-element") DslPredicate hasElement;
+        public DslPredicate size;
+        public DslPredicate filter;
         public Object key;
         public Integer index;
         public String jsonpath;
@@ -268,6 +274,7 @@ public class DslPredicates {
 
             if (index != null) resolvers.put("index", (value) -> {
                 Integer i = index;
+                if (value instanceof Map) value = ((Map)value).entrySet();
                 if (value instanceof Iterable) {
                     int size = Iterables.size((Iterable) value);
                     if (i<0) {
@@ -279,6 +286,48 @@ public class DslPredicates {
                     return Maybe.absent("Cannot evaluate index on non-list target");
                 }
             });
+
+            if (filter != null) resolvers.put("filter", (value) -> {
+                if (value instanceof Map) value = ((Map)value).entrySet();
+                if (value instanceof Iterable) {
+                    return Maybe.of(Iterables.filter((Iterable) value, filter));
+                } else {
+                    return Maybe.absent("Cannot evaluate filter on non-list target");
+                }
+            });
+
+            if (jsonpath!=null) resolvers.put("jsonpath", (value) -> {
+                Entity entity = BrooklynTaskTags.getContextEntity(Tasks.current());
+                String json;
+                try {
+                    json = BeanWithTypeUtils.newMapper(entity!=null ? ((EntityInternal)entity).getManagementContext() : null, false, null, false).writeValueAsString(value);
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    if (LOG.isTraceEnabled()) LOG.trace("Unable to consider jsonpath for non-serializable '"+value+"' due to: "+e, e);
+                    return Maybe.absent("Cannot serialize object as JSON");
+                }
+
+                String jsonpathTidied = jsonpath;
+                if (jsonpathTidied!=null && !jsonpathTidied.startsWith("$")) {
+                    if (jsonpathTidied.startsWith("@") || jsonpathTidied.startsWith(".") || jsonpathTidied.startsWith("[")) {
+                        jsonpathTidied = "$" + jsonpathTidied;
+                    } else {
+                        jsonpathTidied = "$." + jsonpathTidied;
+                    }
+                }
+                Object result;
+                try {
+                    result = JsonPath.read(json, jsonpathTidied);
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    if (LOG.isTraceEnabled()) LOG.trace("Unable to evaluate jsonpath '"+jsonpathTidied+"' for '"+value+"' due to: "+e, e);
+                    return Maybe.absent("Cannot evaluate jsonpath");
+                }
+
+                // above will throw if jsonpath doesn't match anything
+                // this will return single object possibly null, or a list possibly empty
+                return Maybe.ofAllowingNull(result);
+            });
         }
 
         protected Maybe<Object> resolveTargetAgainstInput(Object input) {
@@ -297,7 +346,9 @@ public class DslPredicates {
         public boolean applyToResolved(Maybe<Object> result) {
             CheckCounts counts = new CheckCounts();
             applyToResolved(result, counts);
-            if (counts.checksDefined==0) throw new IllegalStateException("Predicate does not define any checks; if always true or always false is desired, use 'when'");
+            if (counts.checksDefined==0) {
+                throw new IllegalStateException("Predicate does not define any checks; if always true or always false is desired, use 'when'");
+            }
             return counts.allPassed(true);
         }
 
@@ -332,8 +383,17 @@ public class DslPredicates {
                 }
                 return false;
             });
+            checker.check(size, result, (test,value) -> {
+                Integer computedSize = null;
+                if (value instanceof CharSequence) computedSize = ((CharSequence)value).length();
+                else if (value instanceof Map) computedSize = ((Map)value).size();
+                else if (value instanceof Iterable) computedSize = Iterables.size((Iterable)value);
+                else return nestedPredicateCheck(test, Maybe.absent("size not applicable"));
+
+                return nestedPredicateCheck(test, Maybe.of(computedSize));
+            });
 
-            checker.checkTest(check, test -> nestedPredicateCheck(check, result));
+            checker.checkTest(check, test -> nestedPredicateCheck(test, result));
             checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
             checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
 
@@ -394,7 +454,10 @@ public class DslPredicates {
     @Beta
     public static class DslPredicateDefault<T2> extends DslPredicateBase<T2> implements DslPredicate<T2> {
         public DslPredicateDefault() {}
+
+        // allow a string or int to be an implicit equality target
         public DslPredicateDefault(String implicitEquals) { this.implicitEquals = implicitEquals; }
+        public DslPredicateDefault(Integer implicitEquals) { this.implicitEquals = implicitEquals; }
 
         public Object target;
 
@@ -447,7 +510,10 @@ public class DslPredicates {
 
         protected Maybe<Object> resolveTargetStringAgainstInput(String target, Object input) {
             if ("location".equals(target) && input instanceof Entity) return Maybe.of( Locations.getLocationsCheckingAncestors(null, (Entity)input) );
+            if ("locations".equals(target) && input instanceof Entity) return Maybe.of( Locations.getLocationsCheckingAncestors(null, (Entity)input) );
             if ("children".equals(target) && input instanceof Entity) return Maybe.of( ((Entity)input).getChildren() );
+            if ("tags".equals(target) && input instanceof BrooklynObject) return Maybe.of( ((BrooklynObject)input).tags().getTags() );
+
             return Maybe.absent("Unsupported target '"+target+"' on input "+input);
         }
 
@@ -468,9 +534,6 @@ public class DslPredicates {
         public boolean checkTag(DslPredicate tagCheck, Object value) {
             if (value instanceof BrooklynObject) return ((BrooklynObject) value).tags().getTags().stream().anyMatch(tag);
 
-            // not needed, caller iterates
-            //if (value instanceof Iterable) return MutableList.of((Iterable) value).stream().anyMatch(vv -> checkTag(tagCheck, vv));
-
             return false;
         }
 
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
index 779970b50a..957c3e23cf 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
@@ -272,6 +272,30 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         Asserts.assertFalse(p.test(MutableList.of("Astrid", MutableMap.of("name", "Carver"))));
     }
 
+    @Test
+    public void testJsonpath() {
+        DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("jsonpath", "name", "regex", "[Bb].*"), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(MutableMap.of("id", 123, "name", "Bob")));
+        Asserts.assertFalse(p.test(MutableMap.of("id", 124, "name", "Astrid")));
+        Asserts.assertFalse(p.test(MutableMap.of("id", 0)));
+        Asserts.assertFalse(p.test(MutableList.of("id", 0)));
+        Asserts.assertFalse(p.test("not json"));
+
+        p = TypeCoercions.coerce(MutableMap.of("jsonpath", "$.[*].name", "has-element", MutableMap.of("regex", "[Bb].*")), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(MutableList.of(MutableMap.of("id", 123, "name", "Bob"), MutableMap.of("id", 124, "name", "Astrid"))));
+        Asserts.assertFalse(p.test(MutableList.of(MutableMap.of("id", 125), MutableMap.of("id", 124, "name", "Astrid"))));
+        Asserts.assertFalse(p.test(MutableList.of(MutableMap.of("id", 125))));
+
+        p = TypeCoercions.coerce(MutableMap.of("jsonpath", "[*].name",
+                "check",
+                    MutableMap.of("filter", MutableMap.of("regex", "[Bb].*"),
+                        "size", 1))
+                , DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(MutableList.of(MutableMap.of("id", 123, "name", "Bob"), MutableMap.of("id", 124, "name", "Astrid"))));
+        Asserts.assertFalse(p.test(MutableList.of(MutableMap.of("id", 125), MutableMap.of("id", 124, "name", "Astrid"))));
+        Asserts.assertFalse(p.test(MutableList.of(MutableMap.of("id", 125))));
+    }
+
     @Test
     public void testLocationTagImplicitEquals() {
         DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of(


[brooklyn-server] 03/07: support key and index in predicate DSL (and check)

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 3ccc15099258153fb1b30983c8ded4bfdcd0d808
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Aug 19 13:43:14 2022 +0100

    support key and index in predicate DSL (and check)
---
 .../util/core/predicates/DslPredicates.java        | 135 +++++++++++++--------
 .../util/core/predicates/DslPredicateTest.java     |  53 +++-----
 2 files changed, 100 insertions(+), 88 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
index 43074054a5..4756bd50a2 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
@@ -27,6 +27,7 @@ import com.fasterxml.jackson.databind.util.TokenBuffer;
 import com.google.common.annotations.Beta;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.objs.BrooklynObject;
@@ -38,9 +39,11 @@ import org.apache.brooklyn.core.location.Locations;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonSerializationUtils;
 import org.apache.brooklyn.core.resolve.jackson.JsonSymbolDependentDeserializer;
+import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.util.JavaGroovyEquivalents;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.flags.BrooklynTypeNameResolution;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
@@ -63,6 +66,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.function.Supplier;
 
 public class DslPredicates {
 
@@ -85,8 +89,13 @@ public class DslPredicates {
     }
 
     public enum WhenPresencePredicate {
-        /** value cannot be resolved (not even as null) */ ABSENT,
-        /** value is null or cannot be resolved */ ABSENT_OR_NULL,
+        // we might want these, but it is tricky sometimes even in our code to distinguish!
+//        /** value is set but unresolvable (unset or not immediately resolvable) */ UNRESOLVABLE,
+//        /** value is not set (including set to something not resolvable or not immediately resolvable) */ UNSET,
+//        /** value is set to an explicit null */ NULL,
+
+        /** value unavailable (unset or not immediately resolvable) */ ABSENT,
+        /** value is null or unavailable */ ABSENT_OR_NULL,
         /** value is available, but might be null */ PRESENT,
         /** value is available and non-null (but might be 0 or empty) */ PRESENT_NON_NULL,
         /** value is available and ready/truthy (eg not false or empty) */ TRUTHY,
@@ -176,12 +185,15 @@ public class DslPredicates {
         public String regex;
         public String glob;
 
+        /** nested check */
+        public DslPredicate check;
         public List<DslPredicate> any;
         public List<DslPredicate> all;
 
         public @JsonProperty("has-element") DslPredicate hasElement;
-//        public @JsonProperty("at-key") DslPredicate containsKey;
-//        public @JsonProperty("at-index") DslPredicate containsValue;
+        public Object key;
+        public Integer index;
+        public String jsonpath;
 
         public WhenPresencePredicate when;
 
@@ -236,8 +248,45 @@ public class DslPredicates {
             return applyToResolved(result);
         }
 
+        protected void collectApplicableSpecialFieldTargetResolvers(Map<String,Function<Object,Maybe<Object>>> resolvers) {
+            if (key!=null) resolvers.put("key", (value) -> {
+                if (value instanceof Map) {
+                    if (((Map) value).containsKey(key)) {
+                        return Maybe.ofAllowingNull( ((Map) value).get(key) );
+                    } else {
+                        return Maybe.absent("Cannot find indicated key in map");
+                    }
+                } else {
+                    return Maybe.absent("Cannot evaluate key on non-map target");
+                }
+            });
+
+            if (index != null) resolvers.put("index", (value) -> {
+                Integer i = index;
+                if (value instanceof Iterable) {
+                    int size = Iterables.size((Iterable) value);
+                    if (i<0) {
+                        i = size + i;
+                    }
+                    if (i<0 || i>=size) return Maybe.absent("No element at index "+i+"; size "+size);
+                    return Maybe.of(Iterables.get((Iterable)value, i));
+                } else {
+                    return Maybe.absent("Cannot evaluate index on non-list target");
+                }
+            });
+        }
+
         protected Maybe<Object> resolveTargetAgainstInput(Object input) {
-            return Maybe.ofAllowingNull(input);
+            Map<String,Function<Object,Maybe<Object>>> specialResolvers = MutableMap.of();
+            collectApplicableSpecialFieldTargetResolvers(specialResolvers);
+
+            if (!specialResolvers.isEmpty()) {
+                if (specialResolvers.size()>1) throw new IllegalStateException("Predicate has multiple incompatible target specifiers: "+specialResolvers.keySet());
+                return specialResolvers.values().iterator().next().apply(input);
+
+            } else {
+                return Maybe.ofAllowingNull(input);
+            }
         }
 
         public boolean applyToResolved(Maybe<Object> result) {
@@ -279,22 +328,7 @@ public class DslPredicates {
                 return false;
             });
 
-            // TODO at-key, at-value; preserver order
-//            checker.check(element, test -> nestedPredicateCheck(test, result));
-//
-//            if (result.isPresent() && result.get() instanceof Iterable) {
-//                // iterate through lists
-//                for (Object v : ((Iterable) result.get())) {
-//                    CheckCounts checker2 = new CheckCounts();
-//                    applyToResolved(v instanceof Maybe ? (Maybe) v : Maybe.of(v), checker2);
-//                    if (checker2.allPassed(true)) {
-//                        checker.add(checker2);
-//                        return;
-//                    }
-//                }
-//                // did not pass when flattened; try with unflattened
-//            }
-
+            checker.check(check, test -> nestedPredicateCheck(check, result));
             checker.check(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
             checker.check(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
 
@@ -336,7 +370,7 @@ public class DslPredicates {
                     ? p.apply(result.get())
                     : p instanceof DslPredicateBase
                     // in case it does a when: absent check
-                    ? ((DslEntityPredicateDefault)p).applyToResolved(result)
+                    ? ((DslPredicateBase)p).applyToResolved(result)
                     : false;
         }
     }
@@ -363,6 +397,31 @@ public class DslPredicates {
         public String sensor;
         public DslPredicate tag;
 
+        protected void collectApplicableSpecialFieldTargetResolvers(Map<String,Function<Object, Maybe<Object>>> resolvers) {
+            super.collectApplicableSpecialFieldTargetResolvers(resolvers);
+
+            if (config!=null) resolvers.put("config", (value) -> {
+                if (value instanceof Configurable) {
+                    ValueResolver<Object> resolver = Tasks.resolving((DeferredSupplier) () -> ((Configurable)value).config().get(ConfigKeys.newConfigKey(Object.class, config)))
+                            .as(Object.class).allowDeepResolution(true).immediately(true);
+                    if (value instanceof Entity) resolver.context( (Entity)value );
+                    return resolver.getMaybe();
+                } else {
+                    return Maybe.absent("Config not supported on " + value + " (testing config '" + config + "')");
+                }
+            });
+
+            if (sensor!=null) resolvers.put("sensor", (value) -> {
+                if (value instanceof Entity) {
+                    ValueResolver<Object> resolver = Tasks.resolving((DeferredSupplier) () -> ((Entity)value).sensors().get(Sensors.newSensor(Object.class, sensor)))
+                            .as(Object.class).allowDeepResolution(true).immediately(true);
+                    return resolver.getMaybe();
+                } else {
+                    return Maybe.absent("Sensors not supported on " + value + " (testing sensor '" + config + "')");
+                }
+            });
+        }
+
         protected Maybe<Object> resolveTargetAgainstInput(Object input) {
             Object target = this.target;
             Maybe<Object> result;
@@ -377,37 +436,7 @@ public class DslPredicates {
                 result = resolver.getMaybe();
             }
 
-            if (config!=null) {
-                if (sensor!=null) {
-                    throw new IllegalStateException("One predicate cannot specify to test both config key '"+config+"' and sensor '"+sensor+"'; use 'all' with children");
-                }
-
-                if (result.isPresent()) {
-                    Object ro = result.get();
-                    if (ro instanceof Configurable) {
-                        ValueResolver<Object> resolver = Tasks.resolving((DeferredSupplier) () -> ((Configurable)ro).config().get(ConfigKeys.newConfigKey(Object.class, config)))
-                                .as(Object.class).allowDeepResolution(true).immediately(true);
-                        if (ro instanceof Entity) resolver.context( (Entity)ro );
-                        result = resolver.getMaybe();
-                    } else {
-                        result = Maybe.absent("Config not supported on " + ro + " (testing config '" + config + "')");
-                    }
-                }
-            }
-
-            if (sensor!=null) {
-                if (result.isPresent()) {
-                    Object ro = result.get();
-                    if (ro instanceof Entity) {
-                        ValueResolver<Object> resolver = Tasks.resolving((DeferredSupplier) () -> ((Entity)ro).sensors().get(Sensors.newSensor(Object.class, sensor)))
-                                .as(Object.class).allowDeepResolution(true).immediately(true);
-                        result = resolver.getMaybe();
-                    } else {
-                        result = Maybe.absent("Sensors not supported on " + ro + " (testing sensor '" + config + "')");
-                    }
-                }
-            }
-
+            result = result.isPresent() ? super.resolveTargetAgainstInput(result.get()) : result;
             return result;
         }
 
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
index ba06b7e4ef..779970b50a 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java
@@ -215,41 +215,6 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         }
     }
 
-    // auto-unflattening was too weird; instead prompt for "element", also "map-key", "map-value"
-//    @Test
-//    public void testAllWithListWithVariousFlattening() {
-//        Asserts.assertTrue(SetAllowingEqualsToList.of(MutableSet.of("y", "x")).equals(MutableList.of("x", "y")));
-//
-//        DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("equals", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
-//        Asserts.assertTrue(p.test(Arrays.asList("x", "y")));
-//        // list not equal because of order and equal
-//        Asserts.assertFalse(p.test(Arrays.asList("y", "x")));
-//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
-//        // set equality _does_ match without order
-//        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
-//
-//        // "all" works because it attempts unflattened at all, then flattens on each test
-//        p = TypeCoercions.coerce(MutableMap.of("all", MutableList.of("x", "y")), DslPredicates.DslPredicate.class);
-//        Asserts.assertTrue(p.test(Arrays.asList("y", "x")));
-//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
-//        // set equality _does_ match!
-//        Asserts.assertTrue(p.test(SetAllowingEqualsToList.of(MutableSet.of("y", "x"))));
-//
-//        // specify unflattening also works, but is unnecessary
-//        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("all", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
-//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
-//
-//        p = TypeCoercions.coerce(MutableMap.of("unflattened", MutableMap.of("equals", MutableList.of("x", "y"))), DslPredicates.DslPredicate.class);
-//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
-//        Asserts.assertTrue(p.test(Arrays.asList("x", "y")));
-//
-//        p = TypeCoercions.coerce("x", DslPredicates.DslPredicate.class);
-//        Asserts.assertTrue(p.test(Arrays.asList("x", "y", "z")));
-//
-////        DslPredicates.DslPredicate
-//                p = TypeCoercions.coerce(MutableMap.of("unflattened", "x"), DslPredicates.DslPredicate.class);
-//        Asserts.assertFalse(p.test(Arrays.asList("x", "y", "z")));
-//    }
     @Test
     public void testListsAndElement() {
         Asserts.assertTrue(SetAllowingEqualsToList.of(MutableSet.of("y", "x")).equals(MutableList.of("x", "y")));
@@ -289,6 +254,24 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         Asserts.assertFalse(p.test(Arrays.asList("xx", "yy")));
     }
 
+    @Test
+    public void testKeyAndAtIndex() {
+        DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("key", "name", "regex", "[Bb].*"), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(MutableMap.of("id", 123, "name", "Bob")));
+        Asserts.assertFalse(p.test(MutableMap.of("id", 124, "name", "Astrid")));
+
+        p = TypeCoercions.coerce(MutableMap.of("index", -1, "regex", "[Bb].*"), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(MutableList.of("Astrid", "Bob")));
+        Asserts.assertFalse(p.test(MutableList.of("Astrid", "Bob", "Carver")));
+
+        // nested check
+        p = TypeCoercions.coerce(MutableMap.of("index", 1,
+                "check", MutableMap.of("key", "name", "regex", "[Bb].*")), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse(p.test(MutableList.of(MutableMap.of("name", "Bob"))));
+        Asserts.assertTrue(p.test(MutableList.of("Astrid", MutableMap.of("name", "Bob"))));
+        Asserts.assertFalse(p.test(MutableList.of("Astrid", MutableMap.of("name", "Carver"))));
+    }
+
     @Test
     public void testLocationTagImplicitEquals() {
         DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of(


[brooklyn-server] 01/07: misc better logging

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 607ea5404e823e0f380cf2bbd82489130f0c9e59
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Aug 19 12:20:39 2022 +0100

    misc better logging
---
 .../main/java/org/apache/brooklyn/entity/group/GroupsChangePolicy.java  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/src/main/java/org/apache/brooklyn/entity/group/GroupsChangePolicy.java b/core/src/main/java/org/apache/brooklyn/entity/group/GroupsChangePolicy.java
index 10412f08d8..d35f44911d 100644
--- a/core/src/main/java/org/apache/brooklyn/entity/group/GroupsChangePolicy.java
+++ b/core/src/main/java/org/apache/brooklyn/entity/group/GroupsChangePolicy.java
@@ -188,7 +188,7 @@ public class GroupsChangePolicy extends AbstractMembershipTrackingPolicy {
                             EntityInitializer initializer = entityInitializerMaybe.get();
                             initializer.apply((EntityInternal) member);
                         } else {
-                            LOG.debug("Unable to initialize {} due to {}", type, Maybe.getException(entityInitializerMaybe));
+                            LOG.debug("Unable to initialize {} due to {}", type, Maybe.getException(entityInitializerMaybe), Maybe.getException(entityInitializerMaybe));
                         }
                     } catch (Throwable e) {
                         throw Exceptions.propagate(e);


[brooklyn-server] 06/07: conditions on some sensor feeds, and code tidy

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 907be023cf205c52b2136cf3b8c771aba4502157
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Sat Aug 20 00:42:15 2022 +0100

    conditions on some sensor feeds, and code tidy
---
 .../org/apache/brooklyn/core/feed/PollConfig.java  |  13 +++
 .../java/org/apache/brooklyn/core/feed/Poller.java | 108 ++++++++++++++++-----
 .../core/sensor/AbstractAddTriggerableSensor.java  |  56 +++++------
 .../apache/brooklyn/feed/AbstractCommandFeed.java  |  19 +---
 .../brooklyn/feed/function/FunctionFeed.java       |  18 +---
 .../org/apache/brooklyn/feed/http/HttpFeed.java    |  62 ++++--------
 6 files changed, 142 insertions(+), 134 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java b/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java
index 7c8144811d..4533a73b7a 100644
--- a/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java
+++ b/core/src/main/java/org/apache/brooklyn/core/feed/PollConfig.java
@@ -21,9 +21,11 @@ package org.apache.brooklyn.core.feed;
 import static com.google.common.base.Preconditions.checkArgument;
 
 import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
 
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.core.predicates.DslPredicates;
 import org.apache.brooklyn.util.time.Duration;
 
 /**
@@ -36,6 +38,7 @@ public class PollConfig<V, T, F extends PollConfig<V, T, F>> extends FeedConfig<
     private long period = -1;
     private Object otherTriggers;
     private String description;
+    private Supplier<DslPredicates.DslPredicate> condition;
 
     public PollConfig(AttributeSensor<T> sensor) {
         super(sensor);
@@ -45,6 +48,7 @@ public class PollConfig<V, T, F extends PollConfig<V, T, F>> extends FeedConfig<
         super(other);
         this.period = other.period;
         this.otherTriggers = other.otherTriggers;
+        this.condition = other.condition;
         this.description = other.description;
     }
 
@@ -82,6 +86,15 @@ public class PollConfig<V, T, F extends PollConfig<V, T, F>> extends FeedConfig<
         return otherTriggers;
     }
 
+    public F condition(Supplier<DslPredicates.DslPredicate> condition) {
+        this.condition = condition;
+        return self();
+    }
+
+    public Supplier<DslPredicates.DslPredicate> getCondition() {
+        return condition;
+    }
+
     public String getDescription() {
         return description;
     }
diff --git a/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java b/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java
index 997c34fcf3..2b1a09488e 100644
--- a/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java
+++ b/core/src/main/java/org/apache/brooklyn/core/feed/Poller.java
@@ -19,25 +19,37 @@
 package org.apache.brooklyn.core.feed;
 
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
+import com.google.common.collect.SetMultimap;
+import com.google.common.collect.Sets;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.SubscriptionHandle;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.sensor.Sensor;
-import org.apache.brooklyn.api.sensor.SensorEvent;
-import org.apache.brooklyn.api.sensor.SensorEventListener;
+import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.sensor.AbstractAddTriggerableSensor;
+import org.apache.brooklyn.feed.AbstractCommandFeed;
+import org.apache.brooklyn.feed.CommandPollConfig;
+import org.apache.brooklyn.feed.http.HttpFeed;
+import org.apache.brooklyn.feed.http.HttpPollConfig;
+import org.apache.brooklyn.feed.ssh.SshPollValue;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.predicates.DslPredicates;
 import org.apache.brooklyn.util.core.task.DynamicSequentialTask;
 import org.apache.brooklyn.util.core.task.ScheduledTask;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.time.Duration;
+import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -62,30 +74,70 @@ public class Poller<V> {
     private final Set<Task<?>> oneOffTasks = new LinkedHashSet<Task<?>>();
     private final Set<ScheduledTask> tasks = new LinkedHashSet<ScheduledTask>();
     private volatile boolean started = false;
-    
+
+    public <PI,PC extends PollConfig> void scheduleFeed(AbstractFeed feed, SetMultimap<PI,PC> polls, Function<PI,Callable<?>> jobFactory) {
+        for (final PI identifer : polls.keySet()) {
+            Set<PC> configs = polls.get(identifer);
+            long minPeriodMillis = Long.MAX_VALUE;
+            Set<AttributePollHandler<?>> handlers = Sets.newLinkedHashSet();
+
+            for (PC config : configs) {
+                handlers.add(new AttributePollHandler(config, entity, feed));
+                if (config.getPeriod() > 0) minPeriodMillis = Math.min(minPeriodMillis, config.getPeriod());
+            }
+
+            Callable pollJob = jobFactory.apply(identifer);
+            DelegatingPollHandler handlerDelegate = new DelegatingPollHandler(handlers);
+            boolean subscribed = false;
+            for (PollConfig pc: configs) {
+                if (pc.getOtherTriggers()!=null) {
+                    List<Pair<Entity, Sensor>> triggersResolved = AbstractAddTriggerableSensor.resolveTriggers(feed.getEntity(), pc.getOtherTriggers());
+                    for (Pair<Entity, Sensor> pair : triggersResolved) {// TODO initial, condition
+                        subscribe(pollJob, handlerDelegate, pair.getLeft(), pair.getRight(), pc.getCondition());
+                        subscribed = true;
+                    }
+                }
+            }
+            if (minPeriodMillis>0 && (minPeriodMillis < Duration.PRACTICALLY_FOREVER.toMilliseconds() || !subscribed)) {
+                scheduleAtFixedRate(pollJob, handlerDelegate, minPeriodMillis);
+            }
+        }
+    }
+
     private static class PollJob<V> {
         final PollHandler<? super V> handler;
         final Duration pollPeriod;
         final Runnable wrappedJob;
-        final Entity sensorSource;
-        final Sensor<?> sensor;
+        final Entity pollTriggerEntity;
+        final Sensor<?> pollTriggerSensor;
+        final Supplier<DslPredicates.DslPredicate> pollCondition;
         SubscriptionHandle subscription;
         private boolean loggedPreviousException = false;
 
         PollJob(final Callable<V> job, final PollHandler<? super V> handler, Duration period) {
-            this(job, handler, period, null, null);
+            this(job, handler, period, null, null, null);
         }
 
-        PollJob(final Callable<V> job, final PollHandler<? super V> handler, Duration period, Entity sensorSource, Sensor<?> sensor) {
+        PollJob(final Callable<V> job, final PollHandler<? super V> handler, Duration period, Entity sensorSource, Sensor<?> sensor, Supplier<DslPredicates.DslPredicate> pollCondition) {
             this.handler = handler;
             this.pollPeriod = period;
-            this.sensorSource = sensorSource;
-            this.sensor = sensor;
-            
+            this.pollTriggerEntity = sensorSource;
+            this.pollTriggerSensor = sensor;
+            this.pollCondition = pollCondition;
+
             wrappedJob = new Runnable() {
                 @Override
                 public void run() {
                     try {
+                        if (pollCondition!=null) {
+                            DslPredicates.DslPredicate pc = pollCondition.get();
+                            if (pc!=null) {
+                                if (!pc.apply(BrooklynTaskTags.getContextEntity(Tasks.current()))) {
+                                    if (log.isTraceEnabled()) log.trace("PollJob for {} skipped because condition does not apply", job);
+                                    return;
+                                }
+                            }
+                        }
                         V val = job.call();
                         if (handler.checkSuccess(val)) {
                             handler.onSuccess(val);
@@ -126,8 +178,8 @@ public class Poller<V> {
         oneOffJobs.add(job);
     }
 
-    public void scheduleAtFixedRate(Callable<V> job, PollHandler<? super V> handler, long period) {
-        scheduleAtFixedRate(job, handler, Duration.millis(period));
+    public void scheduleAtFixedRate(Callable<V> job, PollHandler<? super V> handler, long periodMillis) {
+        scheduleAtFixedRate(job, handler, Duration.millis(periodMillis));
     }
     public void scheduleAtFixedRate(Callable<V> job, PollHandler<? super V> handler, Duration period) {
         if (started) {
@@ -137,8 +189,8 @@ public class Poller<V> {
         pollJobs.add(foo);
     }
 
-    public void subscribe(Callable<V> job, PollHandler<? super V> handler, Entity sensorSource, Sensor<?> sensor) {
-        pollJobs.add(new PollJob<V>(job, handler, null, sensorSource, sensor));
+    public void subscribe(Callable<V> job, PollHandler<? super V> handler, Entity sensorSource, Sensor<?> sensor, Supplier<DslPredicates.DslPredicate> condition) {
+        pollJobs.add(new PollJob<V>(job, handler, null, sensorSource, sensor, condition));
     }
 
     @SuppressWarnings({ "unchecked" })
@@ -179,28 +231,32 @@ public class Poller<V> {
                 return task;
             };
 
+            ScheduledTask.Builder tb = ScheduledTask.builder(tf)
+                    .cancelOnException(false)
+                    .tag(feed != null ? BrooklynTaskTags.tagForContextAdjunct(feed) : null);
+
             if (pollJob.pollPeriod!=null && pollJob.pollPeriod.compareTo(Duration.ZERO) > 0) {
-                added =true;
-                ScheduledTask t = ScheduledTask.builder(tf)
-                        .displayName("Periodic: " + scheduleName)
-                        .period(pollJob.pollPeriod)
-                        .cancelOnException(false)
-                        .tag(feed!=null ? BrooklynTaskTags.tagForContextAdjunct(feed) : null)
-                        .build();
-                tasks.add(Entities.submit(entity, t));
+                added = true;
+                tb.displayName("Periodic: " + scheduleName);
+                tb.period(pollJob.pollPeriod);
+
                 if (minPeriod==null || (pollJob.pollPeriod.isShorterThan(minPeriod))) {
                     minPeriod = pollJob.pollPeriod;
                 }
+            } else {
+                // if no period, we simply need to run it initially
+                tb.displayName("Initial: "+scheduleName);
             }
+            tasks.add(Entities.submit(entity, tb.build()));
 
-            if (pollJob.sensor!=null) {
+            if (pollJob.pollTriggerSensor !=null) {
                 added = true;
                 if (pollJob.subscription!=null) {
                     throw new IllegalStateException(String.format("Attempt to start poller %s of entity %s when already has subscription %s",
                             this, entity, pollJob.subscription));
                 }
-                sensors.add(pollJob.sensor.getName());
-                pollJob.subscription = feed.subscriptions().subscribe(pollJob.sensorSource!=null ? pollJob.sensorSource : feed.getEntity(), pollJob.sensor, event -> {
+                sensors.add(pollJob.pollTriggerSensor.getName());
+                pollJob.subscription = feed.subscriptions().subscribe(pollJob.pollTriggerEntity !=null ? pollJob.pollTriggerEntity : feed.getEntity(), pollJob.pollTriggerSensor, event -> {
                     // submit this on every event
                     try {
                         feed.getExecutionContext().submit(tf.call());
@@ -211,7 +267,7 @@ public class Poller<V> {
             }
 
             if (!added) {
-                if (log.isDebugEnabled()) log.debug("Activating poll (but leaving off, as period {} and no subscriptions) for {} (using {})", new Object[] {pollJob.pollPeriod, entity, this});
+                if (log.isDebugEnabled()) log.debug("Activating poll (as one-off, as no period and no subscriptions) for {} (using {})", new Object[] {entity, this});
             }
         }
         
diff --git a/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java b/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java
index b443747356..0d28f31b7f 100644
--- a/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java
+++ b/core/src/main/java/org/apache/brooklyn/core/sensor/AbstractAddTriggerableSensor.java
@@ -20,40 +20,29 @@ package org.apache.brooklyn.core.sensor;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
 import com.google.common.base.Predicates;
 import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.sensor.Sensor;
-import org.apache.brooklyn.api.sensor.SensorEvent;
-import org.apache.brooklyn.api.sensor.SensorEventListener;
 import org.apache.brooklyn.config.ConfigKey;
-import org.apache.brooklyn.core.config.BasicConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.effector.AddSensorInitializer;
-import org.apache.brooklyn.core.enricher.AbstractEnricher;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInitializers;
 import org.apache.brooklyn.core.entity.EntityPredicates;
 import org.apache.brooklyn.core.feed.*;
 import org.apache.brooklyn.core.mgmt.internal.AppGroupTraverser;
-import org.apache.brooklyn.feed.http.HttpPollConfig;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.predicates.DslPredicates;
 import org.apache.brooklyn.util.core.task.Tasks;
-import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.http.HttpToolResponse;
-import org.apache.brooklyn.util.javalang.AtomicReferences;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.util.*;
-import java.util.concurrent.Callable;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
-import static com.fasterxml.jackson.databind.type.LogicalType.Collection;
-
 /**
  * Super-class for entity initializers that add feeds.
  */
@@ -62,29 +51,13 @@ public abstract class AbstractAddTriggerableSensor<T> extends AbstractAddSensorF
 
     public static final ConfigKey<Object> SENSOR_TRIGGERS = ConfigKeys.newConfigKey(new TypeToken<Object>() {}, "triggers",
             "Sensors which should trigger this feed, supplied with list of maps containing sensor (name or sensor instance) and entity (ID or entity instance), or just sensor names or just one sensor");
+    public static final ConfigKey<DslPredicates.DslPredicate> CONDITION = ConfigKeys.newConfigKey(DslPredicates.DslPredicate.class, "condition", "Optional condition required for this sensor feed to run");
 
     protected AbstractAddTriggerableSensor() {}
     public AbstractAddTriggerableSensor(ConfigBag parameters) {
         super(parameters);
     }
 
-    public static <V> void scheduleWithTriggers(AbstractFeed feed, Poller<V> poller, Callable<V> pollJob, PollHandler<V> handler, long minPeriod, Set<? extends PollConfig> configs) {
-        // the logic for feeds with pollers is unncessarily convoluted; for now we try to standardize by routing calls that take other triggers
-        // through this method; would be nice to clean up (but a big job)
-
-        if (minPeriod>0 && minPeriod < Duration.PRACTICALLY_FOREVER.toMilliseconds()) {
-            poller.scheduleAtFixedRate(pollJob, handler, minPeriod);
-        }
-        for (PollConfig pc: configs) {
-            if (pc.getOtherTriggers()!=null) {
-                List<Pair<Entity, Sensor>> triggersResolved = resolveTriggers(feed.getEntity(), pc.getOtherTriggers());
-                triggersResolved.forEach(pair -> {
-                    poller.subscribe(pollJob, handler, pair.getLeft(), pair.getRight());
-                });
-            }
-        }
-    }
-
     @JsonIgnore
     protected Duration getPeriod(Entity context, ConfigBag config) {
         if (config.containsKey(SENSOR_PERIOD) || !hasTriggers(config)) {
@@ -99,7 +72,8 @@ public abstract class AbstractAddTriggerableSensor<T> extends AbstractAddSensorF
         return Tasks.resolving(config, SENSOR_TRIGGERS).context(context).deep().immediately(true).getMaybe();
     }
 
-    static List<Pair<Entity,Sensor>> resolveTriggers(Entity context, Object otherTriggers) {
+    @Beta
+    public static List<Pair<Entity,Sensor>> resolveTriggers(Entity context, Object otherTriggers) {
         Object triggers = Tasks.resolving(otherTriggers, Object.class).context(context).deep().immediately(true).get();
 
         if (triggers==null || (triggers instanceof Collection && ((Collection)triggers).isEmpty())) return Collections.emptyList();
@@ -174,7 +148,7 @@ public abstract class AbstractAddTriggerableSensor<T> extends AbstractAddSensorF
         @JsonIgnore
         String sensorName;
 
-        // TODO could support predicates on the value
+        // could support predicates on the value; but we do it on the entity which is enough
 
         public void setEntity(Entity entity) {
             this.entity = entity;
@@ -207,7 +181,23 @@ public abstract class AbstractAddTriggerableSensor<T> extends AbstractAddSensorF
                 .logWarningGraceTimeOnStartup(logWarningGraceTimeOnStartup)
                 .logWarningGraceTime(logWarningGraceTime)
                 .period(getPeriod(entity, initParams()))
-                .otherTriggers(getTriggersMaybe(entity, configBag).orNull());
+                .otherTriggers(getTriggersMaybe(entity, configBag).orNull())
+                .condition(new ConditionSupplier(configBag, entity));
+    }
+
+    static class ConditionSupplier implements Supplier<DslPredicates.DslPredicate> {
+        final ConfigBag configBag;
+        final Entity entity;
+
+        ConditionSupplier(ConfigBag configBag, Entity entity) {
+            this.configBag = configBag;
+            this.entity = entity;
+        }
+
+        @Override
+        public DslPredicates.DslPredicate get() {
+            return Tasks.resolving(configBag, CONDITION).context(entity).deep().immediately(true).get();
+        }
     }
 
 }
diff --git a/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java b/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java
index 79b2b4dd34..c4fab5c647 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/AbstractCommandFeed.java
@@ -227,24 +227,7 @@ public abstract class AbstractCommandFeed extends AbstractFeed {
     
     @Override
     protected void preStart() {
-        SetMultimap<CommandPollIdentifier, CommandPollConfig<?>> polls = config().get(POLLS);
-        
-        for (final CommandPollIdentifier pollInfo : polls.keySet()) {
-            Set<CommandPollConfig<?>> configs = polls.get(pollInfo);
-            long minPeriod = Integer.MAX_VALUE;
-            Set<AttributePollHandler<? super SshPollValue>> handlers = Sets.newLinkedHashSet();
-
-            for (CommandPollConfig<?> config : configs) {
-                handlers.add(new AttributePollHandler<SshPollValue>(config, entity, this));
-                if (config.getPeriod() > 0) minPeriod = Math.min(minPeriod, config.getPeriod());
-            }
-
-            AbstractAddTriggerableSensor.scheduleWithTriggers(this, getPoller(), new Callable<SshPollValue>() {
-                @Override
-                public SshPollValue call() throws Exception {
-                    return exec(pollInfo.command.get(), pollInfo.env.get());
-                }}, new DelegatingPollHandler(handlers), minPeriod, configs);
-        }
+        getPoller().scheduleFeed(this, getConfig(POLLS), pollInfo -> () -> exec(pollInfo.command.get(), pollInfo.env.get()));
     }
     
     @Override
diff --git a/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java b/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java
index f9f2a1867a..378bfe6b56 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/function/FunctionFeed.java
@@ -33,7 +33,11 @@ import org.apache.brooklyn.core.feed.AbstractFeed;
 import org.apache.brooklyn.core.feed.AttributePollHandler;
 import org.apache.brooklyn.core.feed.DelegatingPollHandler;
 import org.apache.brooklyn.core.sensor.AbstractAddTriggerableSensor;
+import org.apache.brooklyn.util.core.javalang.BrooklynHttpConfig;
 import org.apache.brooklyn.util.http.HttpToolResponse;
+import org.apache.brooklyn.util.http.auth.UsernamePassword;
+import org.apache.brooklyn.util.http.executor.HttpRequest;
+import org.apache.brooklyn.util.http.executor.HttpResponse;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -200,18 +204,6 @@ public class FunctionFeed extends AbstractFeed {
     @SuppressWarnings({ "unchecked", "rawtypes" })
     @Override
     protected void preStart() {
-        SetMultimap<FunctionPollIdentifier, FunctionPollConfig<?, ?>> polls = getConfig(POLLS);
-        for (final FunctionPollIdentifier pollInfo : polls.keySet()) {
-            Set<FunctionPollConfig<?,?>> configs = polls.get(pollInfo);
-            long minPeriod = Integer.MAX_VALUE;
-            Set<AttributePollHandler<?>> handlers = Sets.newLinkedHashSet();
-
-            for (FunctionPollConfig<?,?> config : configs) {
-                handlers.add(new AttributePollHandler(config, entity, this));
-                if (config.getPeriod() > 0) minPeriod = Math.min(minPeriod, config.getPeriod());
-            }
-
-            AbstractAddTriggerableSensor.scheduleWithTriggers(this, getPoller(), (Callable)pollInfo.job, new DelegatingPollHandler(handlers), minPeriod, configs);
-        }
+        getPoller().scheduleFeed(this, getConfig(POLLS), pollInfo -> pollInfo.job);
     }
 }
diff --git a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java
index 4d1a13ffa1..4a17f04b84 100644
--- a/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java
+++ b/core/src/main/java/org/apache/brooklyn/feed/http/HttpFeed.java
@@ -375,53 +375,27 @@ public class HttpFeed extends AbstractFeed {
 
     @Override
     protected void preStart() {
-        SetMultimap<HttpPollIdentifier, HttpPollConfig<?>> polls = getConfig(POLLS);
+        getPoller().scheduleFeed(this, getConfig(POLLS), pollInfo -> () -> {
+                if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo);
 
-        for (final HttpPollIdentifier pollInfo : polls.keySet()) {
-            // Though HttpClients are thread safe and can take advantage of connection pooling
-            // and authentication caching, the httpcomponents documentation says:
-            //    "While HttpClient instances are thread safe and can be shared between multiple
-            //     threads of execution, it is highly recommended that each thread maintains its
-            //     own dedicated instance of HttpContext.
-            //  http://hc.apache.org/httpcomponents-client-ga/tutorial/html/connmgmt.html
-
-            Set<HttpPollConfig<?>> configs = polls.get(pollInfo);
-            long minPeriod = Integer.MAX_VALUE;
-            Set<AttributePollHandler<? super HttpToolResponse>> handlers = Sets.newLinkedHashSet();
-
-            for (HttpPollConfig<?> config : configs) {
-                handlers.add(new AttributePollHandler<HttpToolResponse>(config, entity, this));
-                if (config.getPeriod() > 0) minPeriod = Math.min(minPeriod, config.getPeriod());
-            }
-
-            Callable<HttpToolResponse> pollJob;
-            pollJob = new Callable<HttpToolResponse>() {
-                @Override
-                public HttpToolResponse call() throws Exception {
-                    if (log.isTraceEnabled()) log.trace("http polling for {} sensors at {}", entity, pollInfo);
-
-                    UsernamePassword creds = null;
-                    if (pollInfo.credentials.isPresent()) {
-                        creds =  new UsernamePassword(
-                                pollInfo.credentials.get().getUserPrincipal().getName(),
-                                pollInfo.credentials.get().getPassword());
-                    }
-
-                    final long startTime = System.currentTimeMillis();
-                    HttpResponse response =  pollInfo.httpExecutor.execute(new HttpRequest.Builder()
-                            .headers(pollInfo.headers)
-                            .uri(pollInfo.uriProvider.get())
-                            .credentials(creds)
-                            .method(pollInfo.method)
-                            .body(pollInfo.body)
-                            .config(BrooklynHttpConfig.httpConfigBuilder(getEntity()).build())
-                            .build());
-                    return createHttpToolRespose(response, startTime);
+                UsernamePassword creds = null;
+                if (pollInfo.credentials.isPresent()) {
+                    creds = new UsernamePassword(
+                            pollInfo.credentials.get().getUserPrincipal().getName(),
+                            pollInfo.credentials.get().getPassword());
                 }
-            };
 
-            AbstractAddTriggerableSensor.scheduleWithTriggers(this, getPoller(), pollJob, new DelegatingPollHandler<HttpToolResponse>(handlers), minPeriod, configs);
-        }
+                final long startTime = System.currentTimeMillis();
+                HttpResponse response = pollInfo.httpExecutor.execute(new HttpRequest.Builder()
+                        .headers(pollInfo.headers)
+                        .uri(pollInfo.uriProvider.get())
+                        .credentials(creds)
+                        .method(pollInfo.method)
+                        .body(pollInfo.body)
+                        .config(BrooklynHttpConfig.httpConfigBuilder(getEntity()).build())
+                        .build());
+                return createHttpToolRespose(response, startTime);
+        });
     }
 
     @Override


[brooklyn-server] 04/07: predicate tidy-up and dsl test

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 379f9184b6eb9840fb90e575e2e80b9c0cd0c432
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Aug 19 14:59:12 2022 +0100

    predicate tidy-up and dsl test
---
 .../brooklyn/spi/dsl/DslPredicateYamlTest.java     | 120 +++++++++++++++++++++
 .../util/core/predicates/DslPredicates.java        |  29 ++---
 .../brooklyn/core/test/entity/TestEntity.java      |   2 +
 .../core/predicates/DslPredicateEntityTest.java    |   4 +-
 4 files changed, 142 insertions(+), 13 deletions(-)

diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java
new file mode 100644
index 0000000000..4688964004
--- /dev/null
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java
@@ -0,0 +1,120 @@
+/*
+ * Copyright 2016 The Apache Software Foundation.
+ *
+ * Licensed 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 org.apache.brooklyn.camp.brooklyn.spi.dsl;
+
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.location.Location;
+import org.apache.brooklyn.api.sensor.AttributeSensor;
+import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
+import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslTestObjects.DslTestCallable;
+import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslTestObjects.DslTestSupplierWrapper;
+import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslTestObjects.TestDslSupplier;
+import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslTestObjects.TestDslSupplierValue;
+import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.custom.UserSuppliedPackageType;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.entity.Dumper;
+import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.core.test.entity.TestApplication;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.entity.group.DynamicCluster;
+import org.apache.brooklyn.entity.stock.BasicApplication;
+import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.apache.brooklyn.entity.stock.BasicStartable;
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.core.predicates.DslPredicates;
+import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.testng.annotations.Test;
+
+import java.util.concurrent.ExecutionException;
+
+import static org.testng.Assert.assertEquals;
+
+public class DslPredicateYamlTest extends AbstractYamlTest {
+
+    @Test
+    public void testDslConfigSimple() throws Exception {
+        final Entity app = createAndStartApplication(
+                "services:",
+                "- type: " + BasicApplication.class.getName(),
+                "  brooklyn.config:",
+                "    "+TestEntity.CONF_STRING.getName()+": x",
+                "    test.confPredicate:",
+                "      config: "+TestEntity.CONF_STRING.getName(),
+                "      equals: y");
+        DslPredicates.DslPredicate predicate = app.config().get(TestEntity.CONF_PREDICATE);
+        Asserts.assertFalse( predicate.apply(app) );
+        app.config().set(TestEntity.CONF_STRING, "y");
+        Asserts.assertTrue( predicate.apply(app) );
+    }
+
+    @Test
+    public void testDslTags() throws Exception {
+        final Entity app = createAndStartApplication(
+                "services:",
+                "- type: " + BasicApplication.class.getName(),
+                "  brooklyn.tags:",
+                "  - tag1",
+                "  - color: blue");
+
+        DslPredicates.DslPredicate predicate = TypeCoercions.coerce(MutableMap.of("tag", "tag1"), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue( predicate.apply(app) );
+
+        predicate = TypeCoercions.coerce(MutableMap.of("tag", "tag2"), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse( predicate.apply(app) );
+
+        predicate = TypeCoercions.coerce(MutableMap.of("tag", MutableMap.of("key", "color", "equals", "blue")), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue( predicate.apply(app) );
+
+        predicate = TypeCoercions.coerce(MutableMap.of("tag", MutableMap.of("key", "color", "equals", "red")), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse( predicate.apply(app) );
+    }
+
+    @Test
+    public void testDslConfigContainingDsl() throws Exception {
+        final Entity app = createAndStartApplication(
+                "services:",
+                "- type: " + BasicApplication.class.getName(),
+                "  brooklyn.config:",
+                "    "+TestEntity.CONF_STRING.getName()+": x",
+                "    expected: x",
+                "    test.confPredicate:",
+                "      config: "+TestEntity.CONF_STRING.getName(),
+                "      equals: $brooklyn:config(\"expected\")");
+        DslPredicates.DslPredicate predicate = app.config().get(TestEntity.CONF_PREDICATE);
+        Asserts.assertTrue( predicate.apply(app) );
+
+        app.config().set(TestEntity.CONF_STRING, "y");
+        Asserts.assertFalse( predicate.apply(app) );
+
+        // nested DSL is resolved when predicate is _retrieved_, not when predicate is applied
+        // this is simpler and more efficient, although it might be surprising
+        app.config().set(ConfigKeys.newStringConfigKey("expected"), "y");
+        Asserts.assertFalse( predicate.apply(app) );
+
+        // per above, if we re-retrieve the predicate it should work fine
+        predicate = app.config().get(TestEntity.CONF_PREDICATE);
+        Asserts.assertTrue( predicate.apply(app) );
+    }
+
+}
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
index 4756bd50a2..5d4ff03a39 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java
@@ -56,6 +56,8 @@ import org.apache.brooklyn.util.guava.SerializablePredicate;
 import org.apache.brooklyn.util.javalang.Boxing;
 import org.apache.brooklyn.util.text.NaturalOrderComparator;
 import org.apache.brooklyn.util.text.WildcardGlobs;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nullable;
 import java.io.IOException;
@@ -66,10 +68,11 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiFunction;
 import java.util.function.Function;
-import java.util.function.Supplier;
 
 public class DslPredicates {
 
+    private static final Logger LOG = LoggerFactory.getLogger(DslPredicates.class);
+
     static AtomicBoolean initialized = new AtomicBoolean(false);
     public static void init() {
         if (initialized.getAndSet(true)) return;
@@ -210,16 +213,18 @@ public class DslPredicates {
             int checksDefined = 0;
             int checksApplicable = 0;
             int checksPassed = 0;
-            public <T> void check(T test, java.util.function.Predicate<T> check) {
+
+            public <T> void checkTest(T test, java.util.function.Predicate<T> predicateForTest) {
                 if (test!=null) {
                     checksDefined++;
                     checksApplicable++;
-                    if (check.test(test)) checksPassed++;
+                    if (predicateForTest.test(test)) checksPassed++;
                 }
             }
+
             public <T> void check(T test, Maybe<Object> value, java.util.function.BiPredicate<T,Object> check) {
                 if (value.isPresent()) {
-                    check(test, t -> check.test(t, value.get()));
+                    checkTest(test, t -> check.test(t, value.get()));
                 } else {
                     if (test!=null) {
                         checksDefined++;
@@ -297,7 +302,7 @@ public class DslPredicates {
         }
 
         public void applyToResolved(Maybe<Object> result, CheckCounts checker) {
-            checker.check(implicitEquals, result, (test,value) -> {
+            checker.check(implicitEquals, result, (test, value) -> {
                 if ((!(test instanceof BrooklynObject) && value instanceof BrooklynObject) ||
                         (!(test instanceof Iterable) && value instanceof Iterable)) {
                     throw new IllegalStateException("Implicit string used for equality check comparing "+test+" with "+value+", which is probably not what was meant. Use explicit 'equals: ...' syntax for this case.");
@@ -305,8 +310,8 @@ public class DslPredicates {
                 return DslPredicates.coercedEqual(test, value);
             });
             checker.check(equals, result, DslPredicates::coercedEqual);
-            checker.check(regex, result, (test,value) -> asStringTestOrFalse(value, v -> v.matches(test)));
-            checker.check(glob, result, (test,value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(test, v)));
+            checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> v.matches(test)));
+            checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(test, v)));
 
             checker.check(inRange, result, (test,value) ->
                 // current Range only supports Integer, but this code will support any
@@ -328,9 +333,9 @@ public class DslPredicates {
                 return false;
             });
 
-            checker.check(check, test -> nestedPredicateCheck(check, result));
-            checker.check(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
-            checker.check(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
+            checker.checkTest(check, test -> nestedPredicateCheck(check, result));
+            checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
+            checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
 
             checker.check(javaInstanceOf, result, (test, value) -> {
                 Entity ent = null;
@@ -346,11 +351,11 @@ public class DslPredicates {
 
                 return tt.get().isSupertypeOf(value.getClass());
             });
-            checker.check(javaTypeName, (test) -> nestedPredicateCheck(test, result.map(v -> v.getClass().getName())));
+            checker.checkTest(javaTypeName, (test) -> nestedPredicateCheck(test, result.map(v -> v.getClass().getName())));
         }
 
         protected void checkWhen(WhenPresencePredicate when, Maybe<Object> result, CheckCounts checker) {
-            checker.check(when, test -> {
+            checker.checkTest(when, test -> {
                 switch (test) {
                     case PRESENT: return result.isPresent();
                     case PRESENT_NON_NULL: return result.isPresentAndNonNull();
diff --git a/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java b/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java
index 072a5218b0..e1dede1f15 100644
--- a/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java
+++ b/core/src/test/java/org/apache/brooklyn/core/test/entity/TestEntity.java
@@ -45,6 +45,7 @@ import org.apache.brooklyn.core.sensor.AttributeSensorAndConfigKey;
 import org.apache.brooklyn.core.sensor.BasicNotificationSensor;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.util.core.flags.SetFromFlag;
+import org.apache.brooklyn.util.core.predicates.DslPredicates;
 import org.apache.brooklyn.util.time.Duration;
 
 import com.google.common.collect.ImmutableList;
@@ -74,6 +75,7 @@ public interface TestEntity extends Entity, Startable, EntityLocal, EntityIntern
     public static final SetConfigKey<String> CONF_SET_THING = new SetConfigKey<String>(String.class, "test.confSetThing", "Configuration key that's a set thing");
     public static final SetConfigKey<Object> CONF_SET_OBJ_THING = new SetConfigKey<Object>(Object.class, "test.confSetObjThing", "Configuration key that's a set thing, of objects");
     public static final BasicConfigKey<Object> CONF_OBJECT = new BasicConfigKey<Object>(Object.class, "test.confObject", "Configuration key that's an object");
+    public static final BasicConfigKey<DslPredicates.DslPredicate> CONF_PREDICATE = new BasicConfigKey<DslPredicates.DslPredicate>(DslPredicates.DslPredicate.class, "test.confPredicate", "Configuration key that's a predicate/filter");
     public static final ConfigKey<Integer> CONF_INTEGER = ConfigKeys.newConfigKey(Integer.class, "test.confInteger", "Configuration key, an integer");
     public static final ConfigKey<Double> CONF_DOUBLE = ConfigKeys.newConfigKey(Double.class, "test.confDouble", "Configuration key, a double");
     public static final ConfigKey<EntitySpec<? extends Entity>> CHILD_SPEC = ConfigKeys.newConfigKey(new TypeToken<EntitySpec<? extends Entity>>() {}, "test.childSpec", "Spec to be used for creating children");
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java
index 2465003544..431f8c589f 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateEntityTest.java
@@ -87,7 +87,7 @@ public class DslPredicateEntityTest extends BrooklynAppUnitTestSupport {
         Asserts.assertFalse(p.test(app));
     }
 
-    // TODO would be nice to add tag, location tests -- but manual checking has confirmed they work
+    // ----- some weird stuff -----
 
     @Test
     // would be nice to support all the EntityPredicates with the new mechanism; but coercion works well enough in practice,
@@ -133,4 +133,6 @@ public class DslPredicateEntityTest extends BrooklynAppUnitTestSupport {
     }
 
 
+    // note: tags, config, dsl values, etc are tested in camp project DslPredicateYamlTest
+
 }