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 2017/11/08 09:10:21 UTC
[1/2] brooklyn-server git commit: Deprecates use of Task in ConfigKey
Repository: brooklyn-server
Updated Branches:
refs/heads/master 97c5cf693 -> b61997d11
Deprecates use of Task in ConfigKey
Also avoid StackOverflowError in ExecutionContext.getImmediately(task)
When a non-BasicTask is passed in.
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/2e4fc3e2
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/2e4fc3e2
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/2e4fc3e2
Branch: refs/heads/master
Commit: 2e4fc3e205df4f9da01751fc2aba4ea004ef96bf
Parents: 3bf16b4
Author: Aled Sage <al...@gmail.com>
Authored: Tue Nov 7 16:51:54 2017 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Nov 7 16:51:54 2017 +0000
----------------------------------------------------------------------
.../brooklyn/api/mgmt/ExecutionContext.java | 24 +++++++----
.../apache/brooklyn/api/objs/Configurable.java | 12 ++++--
.../util/core/task/BasicExecutionContext.java | 16 ++++++-
.../util/core/task/ValueResolverTest.java | 45 ++++++++++++++++++++
4 files changed, 83 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2e4fc3e2/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java b/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
index bb95ea9..d0fead1 100644
--- a/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
+++ b/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java
@@ -85,20 +85,26 @@ public interface ExecutionContext extends Executor {
* Implementations will typically act like {@link #get(TaskAdaptable)} with additional
* tricks to attempt to be non-blocking, such as recognizing some "immediate" markers.
* <p>
- * Supports {@link Callable}, {@link Runnable}, and {@link Supplier} argument types as well as {@link Task}.
+ * Supports {@link Callable}, {@link Runnable}, {@link Supplier}, and {@link TaskFactory} argument types.
* <p>
- * This executes the given code, and in the case of {@link Task} it may cancel it,
- * so the caller should not use this if the argument is going to be used later and
- * is expected to be pristine. Supply a {@link TaskFactory} if this method's {@link Task#cancel(boolean)}
- * is problematic, or consider other utilities (such as ValueResolver with immediate(true)
- * in a downstream project).
+ * Passing in {@link Task} is deprecated and discouraged - see {@link #getImmediately(Task)}.
*/
// TODO reference ImmediateSupplier when that class is moved to utils project
@Beta
- <T> Maybe<T> getImmediately(Object callableOrSupplierOrTask);
- /** As {@link #getImmediately(Object)} but strongly typed for a task. */
+ <T> Maybe<T> getImmediately(Object callableOrSupplierOrTaskFactory);
+
+ /**
+ * As {@link #getImmediately(Object)} but strongly typed for a task.
+ *
+ * @deprecated since 1.0.0; this can cause the task to be interrupted/cancelled, such that subsequent
+ * use of {@code task.get()} will fail (if the value was not resolved immediately previously).
+ * It is only safe to call this if the the given task is for a one-off usage (not expected
+ * to be used again later). Consider supplying a {@link TaskFactory} if this tasks's
+ * {@link Task#cancel(boolean)} is problematic, or consider other utilities
+ * (such as ValueResolver with immediate(true) in the brooklyn-core project).
+ */
@Beta
- <T> Maybe<T> getImmediately(Task<T> callableOrSupplierOrTask);
+ <T> Maybe<T> getImmediately(Task<T> task);
/**
* Efficient implementation of common case when {@link #submit(TaskAdaptable)} is followed by an immediate {@link Task#get()}.
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2e4fc3e2/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
index 40de655..5187d00 100644
--- a/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
+++ b/api/src/main/java/org/apache/brooklyn/api/objs/Configurable.java
@@ -63,7 +63,7 @@ public interface Configurable {
<T> T get(ConfigKey<T> key);
/**
- * @see {@link #getConfig(ConfigKey)}
+ * @see {@link #get(ConfigKey)}
*/
<T> T get(HasConfigKey<T> key);
@@ -73,7 +73,7 @@ public interface Configurable {
<T> T set(ConfigKey<T> key, T val);
/**
- * @see {@link #setConfig(HasConfigKey, Object)}
+ * @see {@link #set(ConfigKey, Object)}
*/
<T> T set(HasConfigKey<T> key, T val);
@@ -83,12 +83,16 @@ public interface Configurable {
* Returns immediately without blocking; subsequent calls to {@link #getConfig(ConfigKey)}
* will execute the task, and block until the task completes.
*
- * @see {@link #setConfig(ConfigKey, Object)}
+ * @deprecated since 1.0.0; do not use task because can be evaluated only once, and if
+ * cancelled will affect all subsequent lookups of the config value.
+ * Consider using a {@link org.apache.brooklyn.api.mgmt.TaskFactory}.
*/
<T> T set(ConfigKey<T> key, Task<T> val);
/**
- * @see {@link #setConfig(ConfigKey, Task)}
+ * @see {@link #set(ConfigKey, Task)}
+ *
+ * @deprecated since 1.0.0 (see {@link #set(ConfigKey, Task)}
*/
<T> T set(HasConfigKey<T> key, Task<T> val);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2e4fc3e2/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
index 8b37e2f..32ca5b7 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
@@ -270,7 +270,21 @@ public class BasicExecutionContext extends AbstractExecutionContext {
}
callableOrSupplier = fakeTaskForContext.getJob();
} else if (callableOrSupplier instanceof TaskAdaptable) {
- return getImmediately( ((TaskAdaptable<T>)callableOrSupplier).asTask() );
+ Task<T> task = ((TaskAdaptable<T>)callableOrSupplier).asTask();
+ if (task == callableOrSupplier) {
+ // Our TaskAdaptable was a task, but not a BasicTask.
+ // Avoid infinite recursion (don't just call ourselves again!).
+ if (task.isDone()) {
+ return Maybe.of(task.getUnchecked());
+ } else if (task.isSubmitted() || task.isBegun()) {
+ throw new ImmediateUnsupportedException("Task is in progress and incomplete: "+task);
+ } else {
+ throw new ImmediateUnsupportedException("Task not a 'BasicTask', so cannot extract job to get immediately: "+task);
+ }
+ } else {
+ // recurse - try again with the task we've just generated
+ return getImmediately(task);
+ }
} else {
fakeTaskForContext = new BasicTask<T>(MutableMap.of("displayName", "Immediate evaluation"));
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2e4fc3e2/core/src/test/java/org/apache/brooklyn/util/core/task/ValueResolverTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/task/ValueResolverTest.java b/core/src/test/java/org/apache/brooklyn/util/core/task/ValueResolverTest.java
index 455f8ad..a9e2f3d 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/task/ValueResolverTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/task/ValueResolverTest.java
@@ -35,6 +35,7 @@ import org.apache.brooklyn.api.mgmt.TaskFactory;
import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.core.task.ImmediateSupplier.ImmediateUnsupportedException;
import org.apache.brooklyn.util.core.task.ImmediateSupplier.ImmediateValueNotAvailableException;
import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.time.Duration;
@@ -188,6 +189,50 @@ public class ValueResolverTest extends BrooklynAppUnitTestSupport {
Asserts.assertThat(t, (tt) -> !tt.isDone());
}
+ public void testExecutionContextGetImmediatelyBasicTaskTooSlow() throws Exception {
+ final Task<String> t = newSleepTask(Duration.ONE_MINUTE, "foo");
+
+ // Extracts job from task, tries to run it, and sees it tries to block so aborts.
+ // It will also render the task unusable (cancelled); not bothering to assert that!
+ Maybe<String> result = app.getExecutionContext().getImmediately(t);
+ Assert.assertFalse(result.isPresent(), "result="+result);
+ }
+
+ public void testExecutionContextGetImmediatelyBasicTaskSucceeds() throws Exception {
+ final Task<String> t = newSleepTask(Duration.ZERO, "foo");
+
+ // Extracts job from task, tries to run it; because it doesn't block we'll get the result.
+ // It will also render the task unusable (cancelled); calling `t.get()` will throw CancellationException.
+ Maybe<String> result = app.getExecutionContext().getImmediately(t);
+ Assert.assertTrue(result.isPresent(), "result="+result);
+ Assert.assertEquals(result.get(), "foo", "result="+result);
+ }
+
+ public void testExecutionContextGetImmediatelyTaskNotBasicFails() throws Exception {
+ final TaskInternal<String> t = (TaskInternal<String>) newSleepTask(Duration.ZERO, "foo");
+ final Task<String> t2 = new ForwardingTask<String>() {
+ @Override
+ protected TaskInternal<String> delegate() {
+ return t;
+ }
+ @Override public boolean cancel(TaskCancellationMode mode) {
+ return delegate().cancel();
+ }
+ public Task<String> asTask() {
+ return this;
+ }
+ };
+
+ // Does not handle non-basic tasks; an acceptable limitation.
+ // Previously it threw StackOverflowError; now says unsupported.
+ try {
+ Maybe<String> result = app.getExecutionContext().getImmediately(t2);
+ Asserts.shouldHaveFailedPreviously("result="+result);
+ } catch (ImmediateUnsupportedException e) {
+ Asserts.expectedFailureContains(e, "cannot extract job");
+ }
+ }
+
public void testSwallowError() {
ValueResolver<String> result = Tasks.resolving(newThrowTask(Duration.ZERO)).as(String.class).context(app).swallowExceptions();
assertMaybeIsAbsent(result);
[2/2] brooklyn-server git commit: This closes #883
Posted by he...@apache.org.
This closes #883
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/b61997d1
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/b61997d1
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/b61997d1
Branch: refs/heads/master
Commit: b61997d11fdf0a06bf90567e667079c4a46300b2
Parents: 97c5cf6 2e4fc3e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Nov 8 09:10:03 2017 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Nov 8 09:10:03 2017 +0000
----------------------------------------------------------------------
.../brooklyn/api/mgmt/ExecutionContext.java | 24 +++++++----
.../apache/brooklyn/api/objs/Configurable.java | 12 ++++--
.../util/core/task/BasicExecutionContext.java | 16 ++++++-
.../util/core/task/ValueResolverTest.java | 45 ++++++++++++++++++++
4 files changed, 83 insertions(+), 14 deletions(-)
----------------------------------------------------------------------