This closes #565

Author: ahgittin

camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslDeferredFunctionCall.java

@@ -27,6 +27,7 @@
 import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.BrooklynDslCommon;
 import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslToStringHelpers;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.util.core.task.ImmediateSupplier;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
@@ -94,7 +95,7 @@ protected Maybe<Object> invokeOnDeferred(Object obj, boolean immediate) {
             return invokeOn(instance);
         } else {
             if (immediate) {
-                return Maybe.absent("Could not evaluate immediately: " + obj);
+                return Maybe.absent(new ImmediateSupplier.ImmediateValueNotAvailableException("Could not evaluate immediately: " + obj));
             } else {
                 return Maybe.absent(Maybe.getException(resolvedMaybe));
             }

camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java

@@ -596,18 +596,11 @@ public Maybe<Object> getImmediately() {
             final Class<?> clazz = getOrLoadType();
             final ExecutionContext executionContext = entity().getExecutionContext();
 
-            // Marker exception that one of our component-parts cannot yet be resolved - 
-            // throwing and catching this allows us to abort fast.
-            // A bit messy to use exceptions in normal control flow, but this allows the Maps util methods to be used.
-            @SuppressWarnings("serial")
-            class UnavailableException extends RuntimeException {
-            }
-            
             final Function<Object, Object> resolver = new Function<Object, Object>() {
                 @Override public Object apply(Object value) {
                     Maybe<Object> result = Tasks.resolving(value, Object.class).context(executionContext).deep(true).immediately(true).getMaybe();
                     if (result.isAbsent()) {
-                        throw new UnavailableException();
+                        throw new ImmediateValueNotAvailableException();
                     } else {
                         return result.get();
                     }
@@ -627,8 +620,8 @@ class UnavailableException extends RuntimeException {
                     result = create(clazz, factoryMethodName, resolvedFactoryMethodArgs, resolvedFields, resolvedConfig);
                 }
                 return Maybe.of(result);
-            } catch (UnavailableException e) {
-                return Maybe.absent();
+            } catch (ImmediateValueNotAvailableException e) {
+                return ImmediateValueNotAvailableException.newAbsentWithExceptionSupplier();
             }
         }
 
@@ -881,7 +874,7 @@ public EntitySupplier(String entityId) {
             public Maybe<Entity> getImmediately() {
                 EntityInternal entity = entity();
                 if (entity == null) {
-                    return Maybe.absent();
+                    return Maybe.absent("No entity available");
                 }
                 Entity targetEntity = entity.getManagementContext().getEntityManager().getEntity(entityId);
                 return Maybe.of(targetEntity);

camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java

@@ -290,7 +290,7 @@ protected Maybe<Entity> callImpl(boolean immediate) throws Exception {
 
                 if (immediate) {
                     if (maybeComponentId.isAbsent()) {
-                        return Maybe.absent(Maybe.getException(maybeComponentId));
+                        return ImmediateValueNotAvailableException.newAbsentWrapping("Cannot find component ID", maybeComponentId);
                     }
                 }
 
@@ -420,7 +420,7 @@ public EntityId(DslComponent component) {
         @Override
         public Maybe<Object> getImmediately() {
             Maybe<Entity> targetEntityMaybe = component.getImmediately();
-            if (targetEntityMaybe.isAbsent()) return Maybe.absent("Target entity not available");
+            if (targetEntityMaybe.isAbsent()) return ImmediateValueNotAvailableException.newAbsentWrapping("Target entity is not available: "+component, targetEntityMaybe);
             Entity targetEntity = targetEntityMaybe.get();
 
             return Maybe.<Object>of(targetEntity.getId());
@@ -480,7 +480,7 @@ protected String resolveSensorName(boolean immediately) {
         @Override
         public final Maybe<Object> getImmediately() {
             Maybe<Entity> targetEntityMaybe = component.getImmediately();
-            if (targetEntityMaybe.isAbsent()) return Maybe.absent("Target entity not available");
+            if (targetEntityMaybe.isAbsent()) return ImmediateValueNotAvailableException.newAbsentWrapping("Target entity not available: "+component, targetEntityMaybe);
             Entity targetEntity = targetEntityMaybe.get();
 
             String sensorNameS = resolveSensorName(true);
@@ -489,7 +489,7 @@ public final Maybe<Object> getImmediately() {
                 targetSensor = Sensors.newSensor(Object.class, sensorNameS);
             }
             Object result = targetEntity.sensors().get(targetSensor);
-            return GroovyJavaMethods.truth(result) ? Maybe.of(result) : Maybe.absent();
+            return GroovyJavaMethods.truth(result) ? Maybe.of(result) : ImmediateValueNotAvailableException.newAbsentWithExceptionSupplier();
         }
 
         @SuppressWarnings("unchecked")
@@ -665,7 +665,7 @@ protected Maybe<Sensor<?>> getImmediately(Object si, boolean resolved) {
                 return Maybe.<Sensor<?>>of((Sensor<?>)si);
             } else if (si instanceof String) {
                 Maybe<Entity> targetEntityMaybe = component.getImmediately();
-                if (targetEntityMaybe.isAbsent()) return Maybe.absent("Target entity not available");
+                if (targetEntityMaybe.isAbsent()) return ImmediateValueNotAvailableException.newAbsentWrapping("Target entity is not available: "+component, targetEntityMaybe);
                 Entity targetEntity = targetEntityMaybe.get();
 
                 Sensor<?> result = null;

camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java

@@ -307,16 +307,7 @@ public void testDeferredSupplierToConfig() throws Exception {
         assertEquals(entity.config().get(TestEntity.CONF_SET_PLAIN), ImmutableSet.of("myOther"));
     }
 
-    /**
-     * TODO The {@code entity.config().getNonBlocking()} can return absent. When it's called with 
-     * a deferred supplier value, it will kick off a task and then wait just a few millis for that 
-     * task to execute deferredSupplier.get(). If it times out, then it returns Maybe.absent. 
-     * However, on apache jenkins the machine is often slow so the task doesn't complete in the 
-     * given number of millis (even though deferredSupplier.get() doesn't need to block for anything).
-     * Same for {@link #testDeferredSupplierToAttributeWhenReadyInSpecialTypes()}.
-     * See https://issues.apache.org/jira/browse/BROOKLYN-272.
-     */
-    @Test(groups="Broken")
+    @Test
     public void testDeferredSupplierToAttributeWhenReady() throws Exception {
         String yaml = Joiner.on("\n").join(
                 "services:",
@@ -344,17 +335,9 @@ public Object call() {
 
     /**
      * This tests config keys of type {@link org.apache.brooklyn.core.config.MapConfigKey}, etc.
-     * For plain maps, see {@link #testDeferredSupplierToAttributeWhenReadyInPlainCollections()}.
-     * 
-     * TODO The {@code entity.config().getNonBlocking()} can return absent. When it's called with 
-     * a deferred supplier value, it will kick off a task and then wait just a few millis for that 
-     * task to execute deferredSupplier.get(). If it times out, then it returns Maybe.absent. 
-     * However, on apache jenkins the machine is often slow so the task doesn't complete in the 
-     * given number of millis (even though deferredSupplier.get() doesn't need to block for anything).
-     * Same for {@link #testDeferredSupplierToAttributeWhenReady()}.
-     * See https://issues.apache.org/jira/browse/BROOKLYN-272.
+     * For plain maps, see {@link #testDeferredSupplierToAttributeWhenReadyInPlainCollections()}
      */
-    @Test(groups="Broken")
+    @Test
     public void testDeferredSupplierToAttributeWhenReadyInSpecialTypes() throws Exception {
         String yaml = Joiner.on("\n").join(
                 "services:",
@@ -372,10 +355,10 @@ public void testDeferredSupplierToAttributeWhenReadyInSpecialTypes() throws Exce
         final TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren());
 
         // Attribute not yet set; non-blocking will return promptly without the value
-        assertTrue(entity.config().getNonBlocking(TestEntity.CONF_NAME).isAbsent());
-        assertTrue(entity.config().getNonBlocking(TestEntity.CONF_MAP_THING).isAbsent());
-        assertTrue(entity.config().getNonBlocking(TestEntity.CONF_LIST_THING).isAbsent());
-        assertTrue(entity.config().getNonBlocking(TestEntity.CONF_SET_THING).isAbsent());
+        Asserts.assertNotPresent(entity.config().getNonBlocking(TestEntity.CONF_NAME));
+        Asserts.assertNotPresent(entity.config().getNonBlocking(TestEntity.CONF_MAP_THING));
+        Asserts.assertNotPresent(entity.config().getNonBlocking(TestEntity.CONF_LIST_THING));
+        Asserts.assertNotPresent(entity.config().getNonBlocking(TestEntity.CONF_SET_THING));
 
         // Now set the attribute: get will return once that has happened
         executor.submit(new Callable<Object>() {
@@ -399,15 +382,8 @@ public Object call() {
      * This tests config keys of type {@link java.util.Map}, etc.
      * For special types (e.g. {@link org.apache.brooklyn.core.config.MapConfigKey}), see 
      * {@link #testDeferredSupplierToAttributeWhenReadyInPlainCollections()}.
-     * 
-     * TODO test doesn't work because getNonBlocking returns even when no value.
-     *      For example, we get back: Present[value={mykey=attributeWhenReady("myOtherSensor")}].
-     *      However, the `config().get()` does behave as desired.
-     * 
-     * Including the "WIP" group because this test would presumably have never worked!
-     * Added to demonstrate the short-coming.
      */
-    @Test(groups={"Broken", "WIP"})
+    @Test
     public void testDeferredSupplierToAttributeWhenReadyInPlainCollections() throws Exception {
         String yaml = Joiner.on("\n").join(
                 "services:",

core/src/main/java/org/apache/brooklyn/core/config/internal/AbstractConfigMapImpl.java

@@ -313,6 +313,9 @@ protected Maybe<Object> resolveRawValueFromContainer(TContainer container, Confi
                     // wasteful to make a copy to look up; maybe try once opportunistically?
                     ownCopy = MutableMap.copyOf(oc);
                 }
+                // would be cleaner here to have an extractValueMaybe but semantics can get confusing whether absent
+                // means no value can be extracted (getRaw semantics) and immediate mode is on but blocking is needed (ImmediateSupplier semantics);
+                // simpler not to support maybe, in which case here null means the former, and the latter throws something (which the caller catches)
                 Maybe<Object> result = Maybe.of((Object) ((ConfigKeySelfExtracting<?>) key).extractValue(ownCopy, getExecutionContext(container)) );
                 postLocalEvaluate(key, bo, value, result);
                 return result;

core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java

@@ -22,8 +22,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 
 import javax.annotation.Nullable;
 
@@ -41,8 +39,9 @@
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.core.task.ImmediateSupplier.ImmediateUnsupportedException;
+import org.apache.brooklyn.util.core.task.ImmediateSupplier.ImmediateValueNotAvailableException;
 import org.apache.brooklyn.util.core.task.Tasks;
-import org.apache.brooklyn.util.core.task.ValueResolver;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.RuntimeInterruptedException;
 import org.apache.brooklyn.util.guava.Maybe;
@@ -53,6 +52,7 @@
 
 public abstract class AbstractConfigurationSupportInternal implements BrooklynObjectInternal.ConfigurationSupportInternal {
 
+    @SuppressWarnings("unused")
     private static final Logger LOG = LoggerFactory.getLogger(AbstractConfigurationSupportInternal.class);
 
     @Override
@@ -77,10 +77,16 @@ public <T> Maybe<T> getNonBlocking(HasConfigKey<T> key) {
 
     @Override
     public <T> Maybe<T> getNonBlocking(final ConfigKey<T> key) {
-        if (key instanceof StructuredConfigKey || key instanceof SubElementConfigKey) {
-            return getNonBlockingResolvingStructuredKey(key);
-        } else {
-            return getNonBlockingResolvingSimple(key);
+        try {
+            if (key instanceof StructuredConfigKey || key instanceof SubElementConfigKey) {
+                return getNonBlockingResolvingStructuredKey(key);
+            } else {
+                return getNonBlockingResolvingSimple(key);
+            }
+        } catch (ImmediateValueNotAvailableException e) {
+            return Maybe.absent(e);
+        } catch (ImmediateUnsupportedException e) {
+            return Maybe.absent(e);
         }
     }
 
@@ -89,12 +95,6 @@ public <T> Maybe<T> getNonBlocking(final ConfigKey<T> key) {
      * execute the custom logic, as is done by {@link #get(ConfigKey)}, but non-blocking!
      */
     protected <T> Maybe<T> getNonBlockingResolvingStructuredKey(final ConfigKey<T> key) {
-        // TODO This is a poor implementation. We risk timing out when it's just doing its
-        // normal work (e.g. because job's thread was starved), rather than when it's truly 
-        // blocked. Really we'd need to dig into the implementation of get(key), so that the 
-        // underlying work can be configured with a timeout, for when it finally calls 
-        // ValueResolver.
-
         Callable<T> job = new Callable<T>() {
             @Override
             public T call() {
@@ -106,22 +106,15 @@ public T call() {
             }
         };
 
-        Task<T> t = getContext().submit(Tasks.<T>builder().body(job)
+        Task<T> t = Tasks.<T>builder().body(job)
                 .displayName("Resolving dependent value")
                 .description("Resolving "+key.getName())
                 .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-                .build());
+                .build();
         try {
-            T result = t.get(ValueResolver.NON_BLOCKING_WAIT);
-            return Maybe.of(result);
-        } catch (TimeoutException e) {
-            t.cancel(true);
-            return Maybe.<T>absent();
-        } catch (ExecutionException e) {
-            LOG.debug("Problem resolving "+key.getName()+", returning <absent>", e);
-            return Maybe.<T>absent();
-        } catch (InterruptedException e) {
-            throw Exceptions.propagate(e);
+            return getContext().getImmediately(t);
+        } catch (ImmediateUnsupportedException e) {
+            return Maybe.absent();
         }
     }
 
@@ -138,18 +131,17 @@ protected <T> Maybe<T> getNonBlockingResolvingSimple(ConfigKey<T> key) {
         // getRaw returns Maybe(val) if the key was explicitly set (where val can be null)
         // or Absent if the config key was unset.
         Object unresolved = getRaw(key).or(key.getDefaultValue());
-        final Object marker = new Object();
-        // Give tasks a short grace period to resolve.
-        Object resolved = Tasks.resolving(unresolved)
+        Maybe<Object> resolved = Tasks.resolving(unresolved)
                 .as(Object.class)
-                .defaultValue(marker)
                 .immediately(true)
                 .deep(true)
                 .context(getContext())
-                .get();
-        return (resolved != marker)
-                ? TypeCoercions.tryCoerce(resolved, key.getTypeToken())
-                        : Maybe.<T>absent();
+                .getMaybe();
+        if (resolved.isAbsent()) return Maybe.Absent.<T>castAbsent(resolved);
+        
+        // likely we don't need this coercion if we set  as(key.getType())  above, 
+        // but that needs confirmation and quite a lot of testing 
+        return TypeCoercions.tryCoerce(resolved.get(), key.getTypeToken());
     }
 
     @Override

core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java

@@ -28,6 +28,8 @@
 import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
 import org.apache.brooklyn.config.ConfigMap.ConfigMapWithInheritance;
 import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.task.ImmediateSupplier;
+import org.apache.brooklyn.util.core.task.ImmediateSupplier.ImmediateUnsupportedException;
 import org.apache.brooklyn.util.guava.Maybe;
 
 import com.google.common.annotations.Beta;
@@ -113,13 +115,20 @@ public interface ConfigurationSupportInternal extends Configurable.Configuration
 
         /**
          * Attempts to coerce the value for this config key, if available,
-         * taking a default and {@link Maybe#absent absent} if the uncoerced
-         * cannot be resolved within a short timeframe.
+         * including returning a default if the config key is unset,
+         * returning a {@link Maybe#absent absent} if the uncoerced
+         * does not support immediate resolution.
          * <p>
          * Note: if no value for the key is available, not even as a default,
          * this returns a {@link Maybe#isPresent()} containing <code>null</code>
          * (following the semantics of {@link #get(ConfigKey)} 
          * rather than {@link #getRaw(ConfigKey)}).
+         * Thus a {@link Maybe#absent()} definitively indicates that
+         * the absence is due to the request to evaluate immediately.
+         * <p>
+         * This will include catching {@link ImmediateUnsupportedException} 
+         * and returning it as an absence, thus making the semantics here slightly
+         * "safer" than that of {@link ImmediateSupplier#getImmediately()}.
          */
         @Beta
         <T> Maybe<T> getNonBlocking(ConfigKey<T> key);