From 1e615f55c71f23dfc8f65d8ddd2bb79254fce71a Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 11 May 2017 13:20:15 +0100 Subject: [PATCH 1/8] REST call to get entity activities now supports optional "recursive" parameter --- .../apache/brooklyn/rest/api/EntityApi.java | 10 ++++++- .../rest/resources/EntityResource.java | 28 ++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java index de9cc77e03..7330365344 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java @@ -111,8 +111,16 @@ public Response addChildren( }) public List listTasks( @ApiParam(value = "Application ID or name", required = true) @PathParam("application") String applicationId, - @ApiParam(value = "Entity ID or name", required = true) @PathParam("entity") String entityId); + @ApiParam(value = "Entity ID or name", required = true) @PathParam("entity") String entityId, + @ApiParam(value = "Whether to include subtasks recursively across different entities", required = false) + @QueryParam("recurse") @DefaultValue("false") Boolean recurse); + /** @deprecated since 0.12.0 use {@link #listTasks(String, String, Boolean)} */ + @Deprecated + public List listTasks( + String applicationId, + String entityId); + @GET @Path("/{entity}/activities/{task}") @ApiOperation(value = "Fetch task details", response = org.apache.brooklyn.rest.domain.TaskSummary.class) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java index e914d1e8fe..47ec3eeb70 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java @@ -24,6 +24,7 @@ import static org.apache.brooklyn.rest.util.WebResourceUtils.serviceAbsoluteUriBuilder; import java.net.URI; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -38,6 +39,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.api.mgmt.HasTaskChildren; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.core.mgmt.BrooklynTags; import org.apache.brooklyn.core.mgmt.BrooklynTags.NamedStringTag; @@ -58,6 +60,7 @@ import org.apache.brooklyn.rest.transform.TaskTransformer; import org.apache.brooklyn.rest.util.WebResourceUtils; import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.time.Duration; import org.slf4j.Logger; @@ -129,11 +132,28 @@ public Response addChildren(String applicationToken, String entityToken, Boolean } @Override - public List listTasks(String applicationId, String entityId) { + public List listTasks(String applicationId, String entityId, Boolean recurse) { Entity entity = brooklyn().getEntity(applicationId, entityId); - Set> tasks = BrooklynTaskTags.getTasksInEntityContext(mgmt().getExecutionManager(), entity); - return new LinkedList(Collections2.transform(tasks, - TaskTransformer.fromTask(ui.getBaseUriBuilder()))); + List> tasksToScan = MutableList.copyOf(BrooklynTaskTags.getTasksInEntityContext(mgmt().getExecutionManager(), entity)); + Map> tasksLoaded = MutableMap.of(); + + while (!tasksToScan.isEmpty()) { + Task t = tasksToScan.remove(0); + if (tasksLoaded.put(t.getId(), t)==null) { + if (Boolean.TRUE.equals(recurse)) { + if (t instanceof HasTaskChildren) { + Iterables.addAll(tasksToScan, ((HasTaskChildren) t).getChildren() ); + } + } + } + } + return new LinkedList(Collections2.transform(tasksLoaded.values(), + TaskTransformer.fromTask(ui.getBaseUriBuilder()))); + } + + @Override @Deprecated + public List listTasks(String applicationId, String entityId) { + return listTasks(applicationId, entityId, false); } @Override From 4f9564f7232287ea52944c98f51967678caa0abc Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Wed, 5 Jul 2017 14:45:24 +0100 Subject: [PATCH 2/8] identify NPE earlier --- .../org/apache/brooklyn/core/entity/EntityDynamicType.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java b/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java index 1709540126..ecf2a176ea 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/EntityDynamicType.java @@ -47,6 +47,7 @@ import com.google.common.annotations.Beta; import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.Maps; @@ -125,7 +126,7 @@ public Map> getEffectors() { */ @Beta public void addEffector(Effector newEffector) { - Effector oldEffector = effectors.put(newEffector.getName(), newEffector); + Effector oldEffector = effectors.put(Preconditions.checkNotNull(newEffector.getName(), "Missing 'name' for effector"), newEffector); invalidateSnapshot(); if (oldEffector!=null) instance.sensors().emit(AbstractEntity.EFFECTOR_CHANGED, newEffector.getName()); From fd99a2e56b030cfd95b5c802a4b12e48cd737d6a Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Wed, 5 Jul 2017 14:45:37 +0100 Subject: [PATCH 3/8] sample effector for use in UI tests --- .../effector/SampleManyTasksEffector.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java diff --git a/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java b/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java new file mode 100644 index 0000000000..6cf4e2c132 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java @@ -0,0 +1,87 @@ +package org.apache.brooklyn.core.effector; + +import java.util.List; +import java.util.concurrent.Callable; + +import org.apache.brooklyn.api.effector.Effector; +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.api.mgmt.Task; +import org.apache.brooklyn.api.mgmt.TaskAdaptable; +import org.apache.brooklyn.core.effector.EffectorTasks.EffectorTaskFactory; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.core.config.ConfigBag; +import org.apache.brooklyn.util.core.task.DynamicTasks; +import org.apache.brooklyn.util.core.task.TaskBuilder; +import org.apache.brooklyn.util.core.task.Tasks; +import org.apache.brooklyn.util.time.Duration; +import org.apache.brooklyn.util.time.Time; + +/** Effector which can be used to create a lot of tasks with delays. + * Mainly used for manual UI testing, with a blueprint such as the following: + +
+
+name: Test with many tasks
+location: localhost
+services:
+- type: org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess
+  brooklyn.initializers:
+  - type: org.apache.brooklyn.core.effector.SampleManyTasksEffector
+  launch.command: |
+    echo hello | nc -l 4321 &
+    echo $! > $PID_FILE
+    ## to experiment with errors or sleeping
+    # sleep 10
+    # exit 3
+
+
+ + */ +public class SampleManyTasksEffector extends AddEffector { + + public SampleManyTasksEffector(ConfigBag params) { + super(Effectors.effector(String.class, params.get(EFFECTOR_NAME)).name("eatand").impl(body(params)).build()); + } + + private static EffectorTaskFactory body(ConfigBag params) { + return new EffectorTaskFactory() { + @Override + public TaskAdaptable newTask(Entity entity, Effector effector, ConfigBag parameters) { + return Tasks.builder().displayName("eat-sleep-rave-repeat").addAll(tasks(0)).build(); + } + List> tasks(final int depth) { + List> result = MutableList.of(); + do { + TaskBuilder t = Tasks.builder(); + double x = Math.random(); + if (depth>4) x *= Math.random(); + if (depth>6) x *= Math.random(); + if (x<0.3) { + t.displayName("eat").body(new Callable() { public Object call() { return "eat"; }}); + } else if (x<0.6) { + final Duration time = Duration.millis(Math.round(10*1000*Math.random()*Math.random()*Math.random()*Math.random()*Math.random())); + t.displayName("sleep").description("Sleeping "+time).body(new Callable() { public Object call() { + Tasks.setBlockingDetails("sleeping "+time); + Time.sleep(time); + return "slept "+time; + }}); + } else if (x<0.8) { + t.displayName("rave").body(new Callable() { public Object call() { + List> ts = tasks(depth+1); + for (Task tt: ts) { + DynamicTasks.queue(tt); + } + return "raved with "+ts.size()+" tasks"; + }}); + } else { + t.displayName("repeat").addAll(tasks(depth+1)); + } + result.add(t.build()); + + } while (Math.random()<0.8); + return result; + } + }; + } + +} From a6c4e1301dc7d5cd4161e34e2631deecee7231ec Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 6 Jul 2017 14:58:58 +0100 Subject: [PATCH 4/8] cleanup how tasks are loaded, better error message for GC case --- .../rest/resources/ActivityResource.java | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java index 81439d5faa..af49856e4a 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java @@ -47,25 +47,26 @@ public class ActivityResource extends AbstractBrooklynRestResource implements Ac @Override public TaskSummary get(String taskId) { - Task t = mgmt().getExecutionManager().getTask(taskId); - if (t == null) { - throw WebResourceUtils.notFound("Cannot find task '%s'", taskId); - } - checkEntityEntitled(t); + Task t = findTask(taskId); return TaskTransformer.fromTask(ui.getBaseUriBuilder()).apply(t); } @Override public Map getAllChildrenAsMap(final String taskId) { - final Task parentTask = mgmt().getExecutionManager().getTask(taskId); - if (parentTask == null) { - throw WebResourceUtils.notFound("Cannot find task '%s'", taskId); - } - checkEntityEntitled(parentTask); + final Task parentTask = findTask(taskId); return getAllDescendantTasks(parentTask); } + protected Task findTask(final String taskId) { + final Task task = mgmt().getExecutionManager().getTask(taskId); + if (task == null) { + throw WebResourceUtils.notFound("Cannot find task '%s' - possibly garbage collected to save memory", taskId); + } + checkEntityEntitled(task); + return task; + } + private LinkedHashMap getAllDescendantTasks(final Task parentTask) { final LinkedHashMap result = Maps.newLinkedHashMap(); if (!(parentTask instanceof HasTaskChildren)) { @@ -81,11 +82,7 @@ private LinkedHashMap getAllDescendantTasks(final Task p @Override public List children(String taskId, Boolean includeBackground) { - Task t = mgmt().getExecutionManager().getTask(taskId); - if (t == null) { - throw WebResourceUtils.notFound("Cannot find task '%s'", taskId); - } - checkEntityEntitled(t); + Task t = findTask(taskId); Set result = MutableSet.copyOf(getSubTaskChildren(t)); if (Boolean.TRUE.equals(includeBackground)) { @@ -118,11 +115,7 @@ private List getSubTaskChildren(Task t) { @Override public String stream(String taskId, String streamId) { - Task t = mgmt().getExecutionManager().getTask(taskId); - if (t == null) { - throw WebResourceUtils.notFound("Cannot find task '%s'", taskId); - } - checkEntityEntitled(t); + Task t = findTask(taskId); checkStreamEntitled(t, streamId); WrappedStream stream = BrooklynTaskTags.stream(t, streamId); From 36fd318c21a54493042cc985c13efeaeaa91a715 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 6 Jul 2017 15:49:14 +0100 Subject: [PATCH 5/8] add limit when retrieving tasks --- .../apache/brooklyn/rest/api/ActivityApi.java | 12 ++++++-- .../brooklyn/rest/api/ApplicationApi.java | 9 +++--- .../apache/brooklyn/rest/api/EntityApi.java | 6 ++-- .../rest/resources/ActivityResource.java | 30 +++++++++++++++---- .../rest/resources/EntityResource.java | 10 ++++--- 5 files changed, 48 insertions(+), 19 deletions(-) diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ActivityApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ActivityApi.java index 45cd25cf37..31c4d92e01 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ActivityApi.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ActivityApi.java @@ -61,13 +61,21 @@ public List children( @GET @Path("/{task}/children/recurse") @ApiOperation( - value = "Fetch all child tasks details as Map map key == Task ID", + value = "Fetch all child tasks and their descendants with details as Map map key == Task ID", response = Map.class) @ApiResponses(value = { @ApiResponse(code = 404, message = "Could not find task") }) public Map getAllChildrenAsMap( - @ApiParam(value = "Task ID", required = true) @PathParam("task") String taskId); + @ApiParam(value = "Task ID", required = true) @PathParam("task") String taskId, + @ApiParam(value = "Max number of tasks to include, or -1 for all (default 200)", required = false) + @QueryParam("limit") @DefaultValue("200") int limit, + @ApiParam(value = "Max depth to traverse, or -1 for all (default)", required = false) + @QueryParam("maxDepth") @DefaultValue("-1") int maxDepth); + + /** @deprecated since 0.12.0 use {@link #getAllChildrenAsMap(String, int, int)} with depth -1 */ + @Deprecated + public Map getAllChildrenAsMap(String taskId); @GET @Path("/{task}/stream/{streamId}") diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java index edfdd7d32f..d0e4d19437 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java @@ -18,7 +18,6 @@ */ package org.apache.brooklyn.rest.api; -import io.swagger.annotations.Api; import java.util.List; import java.util.Map; @@ -35,15 +34,15 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import org.apache.brooklyn.rest.domain.ApplicationSpec; import org.apache.brooklyn.rest.domain.ApplicationSummary; -import org.apache.brooklyn.rest.domain.EntitySummary; import org.apache.brooklyn.rest.domain.EntityDetail; +import org.apache.brooklyn.rest.domain.EntitySummary; -import io.swagger.annotations.ApiResponse; -import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; @Path("/applications") @Api("Applications") diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java index 7330365344..d0788fa926 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/EntityApi.java @@ -112,10 +112,12 @@ public Response addChildren( public List listTasks( @ApiParam(value = "Application ID or name", required = true) @PathParam("application") String applicationId, @ApiParam(value = "Entity ID or name", required = true) @PathParam("entity") String entityId, - @ApiParam(value = "Whether to include subtasks recursively across different entities", required = false) + @ApiParam(value = "Max number of tasks, or -1 for all (default 200)", required = false) + @QueryParam("limit") @DefaultValue("200") int limit, + @ApiParam(value = "Whether to include subtasks recursively across different entities (default false)", required = false) @QueryParam("recurse") @DefaultValue("false") Boolean recurse); - /** @deprecated since 0.12.0 use {@link #listTasks(String, String, Boolean)} */ + /** @deprecated since 0.12.0 use {@link #listTasks(String, String, Integer, Boolean)} */ @Deprecated public List listTasks( String applicationId, diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java index af49856e4a..2bc90ad2e4 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java @@ -40,6 +40,7 @@ import org.apache.brooklyn.util.collections.MutableSet; import com.google.common.collect.Collections2; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -52,10 +53,15 @@ public TaskSummary get(String taskId) { return TaskTransformer.fromTask(ui.getBaseUriBuilder()).apply(t); } - @Override + @Override @Deprecated public Map getAllChildrenAsMap(final String taskId) { + return getAllChildrenAsMap(taskId, 200, -1); + } + + @Override + public Map getAllChildrenAsMap(final String taskId, final int limit, final int maxDepth) { final Task parentTask = findTask(taskId); - return getAllDescendantTasks(parentTask); + return getAllDescendantTasks(parentTask, limit, maxDepth); } protected Task findTask(final String taskId) { @@ -67,14 +73,26 @@ protected Task findTask(final String taskId) { return task; } - private LinkedHashMap getAllDescendantTasks(final Task parentTask) { + private LinkedHashMap getAllDescendantTasks(final Task parentTask, int limit, int maxDepth) { final LinkedHashMap result = Maps.newLinkedHashMap(); if (!(parentTask instanceof HasTaskChildren)) { return result; } - for (final Task childTask : ((HasTaskChildren) parentTask).getChildren()) { - result.put(childTask.getId(), TaskTransformer.fromTask(ui.getBaseUriBuilder()).apply(childTask)); - result.putAll(getAllDescendantTasks(childTask)); + Set> nextLayer = MutableSet.copyOf( ((HasTaskChildren) parentTask).getChildren() ); + outer: while (!nextLayer.isEmpty() && maxDepth-- != 0) { + Set> thisLayer = nextLayer; + nextLayer = MutableSet.of(); + for (final Task childTask : thisLayer) { + TaskSummary wasThere = result.put(childTask.getId(), TaskTransformer.fromTask(ui.getBaseUriBuilder()).apply(childTask)); + if (wasThere==null) { + if (limit-- == 0) { + break outer; + } + if (childTask instanceof HasTaskChildren) { + Iterables.addAll(nextLayer, ((HasTaskChildren)childTask).getChildren()); + } + } + } } return result; } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java index 47ec3eeb70..7007a7f397 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java @@ -24,11 +24,9 @@ import static org.apache.brooklyn.rest.util.WebResourceUtils.serviceAbsoluteUriBuilder; import java.net.URI; -import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Set; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; @@ -132,7 +130,8 @@ public Response addChildren(String applicationToken, String entityToken, Boolean } @Override - public List listTasks(String applicationId, String entityId, Boolean recurse) { + public List listTasks(String applicationId, String entityId, int limit, Boolean recurse) { + int sizeRemaining = limit; Entity entity = brooklyn().getEntity(applicationId, entityId); List> tasksToScan = MutableList.copyOf(BrooklynTaskTags.getTasksInEntityContext(mgmt().getExecutionManager(), entity)); Map> tasksLoaded = MutableMap.of(); @@ -140,6 +139,9 @@ public List listTasks(String applicationId, String entityId, Boolea while (!tasksToScan.isEmpty()) { Task t = tasksToScan.remove(0); if (tasksLoaded.put(t.getId(), t)==null) { + if (--sizeRemaining==0) { + break; + } if (Boolean.TRUE.equals(recurse)) { if (t instanceof HasTaskChildren) { Iterables.addAll(tasksToScan, ((HasTaskChildren) t).getChildren() ); @@ -153,7 +155,7 @@ public List listTasks(String applicationId, String entityId, Boolea @Override @Deprecated public List listTasks(String applicationId, String entityId) { - return listTasks(applicationId, entityId, false); + return listTasks(applicationId, entityId, -1, false); } @Override From 157c3858e62d3345ac81cefb093cae1d64480c43 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 6 Jul 2017 23:43:01 +0100 Subject: [PATCH 6/8] tests for activity limits/depths --- .../brooklyn/core/effector/AddEffector.java | 2 +- .../apache/brooklyn/core/entity/Entities.java | 4 + .../apache/brooklyn/util/core/task/Tasks.java | 29 +- .../effector/SampleManyTasksEffector.java | 23 +- .../rest/resources/ActivityResource.java | 4 +- .../rest/resources/ActivityRestTest.java | 344 ++++++++++++++++++ .../resources/ApplicationResourceTest.java | 13 +- .../rest/resources/EffectorUtilsRestTest.java | 1 + .../rest/resources/ErrorResponseTest.java | 2 - 9 files changed, 403 insertions(+), 19 deletions(-) create mode 100644 rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ActivityRestTest.java diff --git a/core/src/main/java/org/apache/brooklyn/core/effector/AddEffector.java b/core/src/main/java/org/apache/brooklyn/core/effector/AddEffector.java index 9590bcff57..04c136433c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/effector/AddEffector.java +++ b/core/src/main/java/org/apache/brooklyn/core/effector/AddEffector.java @@ -63,7 +63,7 @@ public class AddEffector implements EntityInitializer { public static final ConfigKey> EFFECTOR_PARAMETER_DEFS = new MapConfigKey(Object.class, "parameters"); - final Effector effector; + protected final Effector effector; public AddEffector(Effector effector) { this.effector = Preconditions.checkNotNull(effector, "effector"); diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java b/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java index 46a9ceafbb..df904d3b3d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/Entities.java @@ -480,6 +480,10 @@ public static void dumpInfo(Location loc, Writer out, String currentIndentation, out.flush(); } + public static void dumpInfo(Task t) { + Tasks.dumpInfo(t); + } + public static void dumpInfo(Enricher enr) { try { dumpInfo(enr, new PrintWriter(System.out), "", " "); diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java b/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java index 09116af7e4..0677f74cdb 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java @@ -18,6 +18,9 @@ */ package org.apache.brooklyn.util.core.task; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.Writer; import java.util.Collections; import java.util.List; import java.util.concurrent.Callable; @@ -39,7 +42,6 @@ import org.apache.brooklyn.util.time.CountdownTimer; import org.apache.brooklyn.util.time.Duration; import org.apache.brooklyn.util.time.Time; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -492,4 +494,29 @@ private static String getTimeoutString(Repeater repeater) { return "in "+timeout; } + + public static void dumpInfo(Task t) { + try { + dumpInfo(t, new PrintWriter(System.out), "", " "); + } catch (IOException exc) { + // system.out throwing an exception is odd, so don't have IOException on signature + throw new RuntimeException(exc); + } + } + public static void dumpInfo(Task t, Writer out) throws IOException { + dumpInfo(t, out, "", " "); + } + public static void dumpInfo(Task t, String currentIndentation, String tab) throws IOException { + dumpInfo(t, new PrintWriter(System.out), currentIndentation, tab); + } + public static void dumpInfo(Task t, Writer out, String currentIndentation, String tab) throws IOException { + out.append(currentIndentation+t+": "+t.getStatusDetail(false)+"\n"); + + if (t instanceof HasTaskChildren) { + for (Task child: ((HasTaskChildren)t).getChildren()) { + dumpInfo(child, out, currentIndentation+tab, tab); + } + } + out.flush(); + } } diff --git a/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java b/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java index 6cf4e2c132..da36de3c4f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java +++ b/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java @@ -1,12 +1,15 @@ package org.apache.brooklyn.core.effector; import java.util.List; +import java.util.Random; import java.util.concurrent.Callable; import org.apache.brooklyn.api.effector.Effector; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.mgmt.TaskAdaptable; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.effector.EffectorTasks.EffectorTaskFactory; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -39,11 +42,21 @@ */ public class SampleManyTasksEffector extends AddEffector { + public static final ConfigKey RANDOM_SEED = ConfigKeys.newIntegerConfigKey("random.seed"); + public SampleManyTasksEffector(ConfigBag params) { super(Effectors.effector(String.class, params.get(EFFECTOR_NAME)).name("eatand").impl(body(params)).build()); } + public Effector getEffector() { + return effector; + } + private static EffectorTaskFactory body(ConfigBag params) { + Integer seed = params.get(RANDOM_SEED); + final Random random = seed!=null ? new Random(seed) : new Random(); + + // NOTE: not nicely serializable return new EffectorTaskFactory() { @Override public TaskAdaptable newTask(Entity entity, Effector effector, ConfigBag parameters) { @@ -53,13 +66,13 @@ List> tasks(final int depth) { List> result = MutableList.of(); do { TaskBuilder t = Tasks.builder(); - double x = Math.random(); - if (depth>4) x *= Math.random(); - if (depth>6) x *= Math.random(); + double x = random.nextDouble(); + if (depth>4) x *= random.nextDouble(); + if (depth>6) x *= random.nextDouble(); if (x<0.3) { t.displayName("eat").body(new Callable() { public Object call() { return "eat"; }}); } else if (x<0.6) { - final Duration time = Duration.millis(Math.round(10*1000*Math.random()*Math.random()*Math.random()*Math.random()*Math.random())); + final Duration time = Duration.millis(Math.round(10*1000*random.nextDouble()*random.nextDouble()*random.nextDouble()*random.nextDouble()*random.nextDouble())); t.displayName("sleep").description("Sleeping "+time).body(new Callable() { public Object call() { Tasks.setBlockingDetails("sleeping "+time); Time.sleep(time); @@ -78,7 +91,7 @@ List> tasks(final int depth) { } result.add(t.build()); - } while (Math.random()<0.8); + } while (random.nextDouble()<0.8); return result; } }; diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java index 2bc90ad2e4..6e3c2eb4b4 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ActivityResource.java @@ -79,13 +79,13 @@ private LinkedHashMap getAllDescendantTasks(final Task p return result; } Set> nextLayer = MutableSet.copyOf( ((HasTaskChildren) parentTask).getChildren() ); - outer: while (!nextLayer.isEmpty() && maxDepth-- != 0) { + outer: while (limit!=0 && !nextLayer.isEmpty() && maxDepth-- != 0) { Set> thisLayer = nextLayer; nextLayer = MutableSet.of(); for (final Task childTask : thisLayer) { TaskSummary wasThere = result.put(childTask.getId(), TaskTransformer.fromTask(ui.getBaseUriBuilder()).apply(childTask)); if (wasThere==null) { - if (limit-- == 0) { + if (--limit == 0) { break outer; } if (childTask instanceof HasTaskChildren) { diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ActivityRestTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ActivityRestTest.java new file mode 100644 index 0000000000..a5424f873b --- /dev/null +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ActivityRestTest.java @@ -0,0 +1,344 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.rest.resources; + +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeoutException; + +import javax.ws.rs.core.GenericType; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +import org.apache.brooklyn.api.effector.Effector; +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.api.entity.EntitySpec; +import org.apache.brooklyn.api.mgmt.HasTaskChildren; +import org.apache.brooklyn.api.mgmt.Task; +import org.apache.brooklyn.core.effector.SampleManyTasksEffector; +import org.apache.brooklyn.core.entity.Entities; +import org.apache.brooklyn.core.mgmt.EntityManagementUtils; +import org.apache.brooklyn.core.mgmt.EntityManagementUtils.CreationResult; +import org.apache.brooklyn.core.mgmt.internal.TestEntityWithEffectors; +import org.apache.brooklyn.entity.stock.BasicApplication; +import org.apache.brooklyn.rest.domain.TaskSummary; +import org.apache.brooklyn.rest.testing.BrooklynRestResourceTest; +import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.core.config.ConfigBag; +import org.apache.brooklyn.util.core.task.Tasks; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.http.HttpAsserts; +import org.apache.brooklyn.util.time.CountdownTimer; +import org.apache.brooklyn.util.time.Duration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.Assert; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.collect.Iterables; + +/** Tests {@link ActivityResource} and activity methods on {@link EntityResource} */ +public class ActivityRestTest extends BrooklynRestResourceTest { + + private static final Logger log = LoggerFactory.getLogger(ActivityRestTest.class); + + /* a nice seed, initial run as follows; + +Task[eatand]@J90TKfIX: Waiting on Task[eat-sleep-rave-repeat]@QPa5o4kF + Task[eat-sleep-rave-repeat]@QPa5o4kF: Waiting on Task[rave]@yP9KjuWD + Task[rave]@yP9KjuWD: Waiting on Task[repeat]@Dd1AqB7Q + Task[repeat]@Dd1AqB7Q: Waiting on Task[repeat]@remQL5eD + Task[repeat]@remQL5eD: Waiting on Task[repeat]@g1ReP4BP + Task[sleep]@iV3iWg2N: Completed, result: slept 46ms + Task[eat]@fpIttX07: Completed, result: eat + Task[eat]@w6sxLefq: Completed, result: eat + Task[repeat]@g1ReP4BP: Waiting on Task[sleep]@zRTOQ4ak + Task[eat]@TvcdOUx7: Completed, result: eat + Task[rave]@yJndzNLf: Completed, result: raved with 1 tasks + Task[eat]@oiJ3eZZQ: Completed, result: eat + Task[sleep]@zRTOQ4ak: sleeping 74ms + Task[eat]@qoFRPEfM: Not submitted + Task[sleep]@fNX16uvi: Not submitted + + */ + private final int SEED = 1; + + private Entity entity; + private Effector effector; + + private Task lastTask; + + @BeforeClass(alwaysRun = true) + public void setUp() throws Exception { + startServer(); + } + + @BeforeMethod(alwaysRun = true) + public void setUpOneTest() throws Exception { + initEntity(SEED); + } + + @SuppressWarnings("deprecation") + protected void initEntity(int seed) { + if (entity!=null) { + Entities.destroy(entity.getApplication()); + } + + CreationResult app = EntityManagementUtils.createStarting(getManagementContext(), + EntitySpec.create(BasicApplication.class) + .child(EntitySpec.create(TestEntityWithEffectors.class)) ); + app.blockUntilComplete(); + entity = Iterables.getOnlyElement( app.get().getChildren() ); + + SampleManyTasksEffector manyTasksAdder = new SampleManyTasksEffector(ConfigBag.newInstance().configure(SampleManyTasksEffector.RANDOM_SEED, seed)); + effector = manyTasksAdder.getEffector(); + manyTasksAdder.apply((org.apache.brooklyn.api.entity.EntityLocal) entity); + } + + /** finds a good seed, in case the effector changes */ + public static void main(String[] args) throws Exception { + ActivityRestTest me = new ActivityRestTest(); + me.setUpClass(); + int i=0; + do { + me.initEntity(i); + try { + log.info("Trying seed "+i+"..."); + me.testGood(Duration.millis(200)); + break; + } catch (Throwable e) { + log.info(" "+Exceptions.collapseText(e)); + // e.printStackTrace(); + // continue + } + i++; + } while (true); + Tasks.dumpInfo(me.lastTask); + log.info("Seed "+i+" is good ^"); + } + + @Test + public void testGood() { + testGood(Duration.ONE_SECOND); + } + + void testGood(Duration timeout) { + lastTask = entity.invoke(effector, null); + Task leaf = waitForCompletedDescendantWithChildAndSibling(lastTask, lastTask, CountdownTimer.newInstanceStarted(timeout), 0); + Assert.assertTrue(depthOf(leaf)>=4, "Not deep enough: "+depthOf(leaf)); + } + + @Test + public void testGetActivity() { + Task t = entity.invoke(effector, null); + + Response response = client().path("/activities/"+t.getId()) + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + TaskSummary task = response.readEntity(TaskSummary.class); + Assert.assertEquals(task.getId(), t.getId()); + } + + @Test + public void testGetActivitiesChildren() { + Task t = entity.invoke(effector, null); + Task leaf = waitForCompletedDescendantWithChildAndSibling(t, t, CountdownTimer.newInstanceStarted(Duration.ONE_SECOND), 0); + + Response response = client().path("/activities/"+leaf.getSubmittedByTask().getId()+"/children") + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + List tasks = response.readEntity(new GenericType>() {}); + log.info("Tasks children: "+tasks.size()); + Assert.assertTrue(tasksContain(tasks, leaf), "tasks should have included leaf "+leaf+"; was "+tasks); + } + + @Test + public void testGetActivitiesRecursiveAndWithLimit() { + Task t = entity.invoke(effector, null); + Task leaf = waitForCompletedDescendantWithChildAndSibling(t, t, CountdownTimer.newInstanceStarted(Duration.ONE_SECOND), 0); + Task leafParent = leaf.getSubmittedByTask(); + Task leafGrandparent = leafParent.getSubmittedByTask(); + + Response response = client().path("/activities/"+leafGrandparent.getId()+"/children") + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + List tasksL = response.readEntity(new GenericType>() {}); + Assert.assertFalse(tasksContain(tasksL, leaf), "non-recursive tasks should not have included leaf "+leaf+"; was "+tasksL); + Assert.assertTrue(tasksContain(tasksL, leafParent), "non-recursive tasks should have included leaf parent "+leafParent+"; was "+tasksL); + Assert.assertFalse(tasksContain(tasksL, leafGrandparent), "non-recursive tasks should not have included leaf grandparent "+leafGrandparent+"; was "+tasksL); + + response = client().path("/activities/"+leafGrandparent.getId()+"/children/recurse") + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + Map tasks = response.readEntity(new GenericType>() {}); + Assert.assertTrue(tasksContain(tasks, leaf), "recursive tasks should have included leaf "+leaf+"; was "+tasks); + Assert.assertTrue(tasksContain(tasks, leafParent), "recursive tasks should have included leaf parent "+leafParent+"; was "+tasks); + Assert.assertFalse(tasksContain(tasks, leafGrandparent), "recursive tasks should not have included leaf grandparent "+leafGrandparent+"; was "+tasks); + + response = client().path("/activities/"+leafGrandparent.getId()+"/children/recurse") + .query("maxDepth", 1) + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + tasks = response.readEntity(new GenericType>() {}); + Assert.assertFalse(tasksContain(tasks, leaf), "depth 1 recursive tasks should nont have included leaf "+leaf+"; was "+tasks); + Assert.assertTrue(tasksContain(tasks, leafParent), "depth 1 recursive tasks should have included leaf parent "+leafParent+"; was "+tasks); + + response = client().path("/activities/"+leafGrandparent.getId()+"/children/recurse") + .query("maxDepth", 2) + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + tasks = response.readEntity(new GenericType>() {}); + Assert.assertTrue(tasksContain(tasks, leaf), "depth 2 recursive tasks should have included leaf "+leaf+"; was "+tasks); + Assert.assertTrue(tasksContain(tasks, leafParent), "depth 2 recursive tasks should have included leaf parent "+leafParent+"; was "+tasks); + Assert.assertFalse(tasksContain(tasks, leafGrandparent), "depth 2 recursive tasks should not have included leaf grandparent "+leafGrandparent+"; was "+tasks); + + Assert.assertTrue(children(leafGrandparent).size() >= 2, "children: "+children(leafGrandparent)); + response = client().path("/activities/"+leafGrandparent.getId()+"/children/recurse") + .query("limit", children(leafGrandparent).size()) + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + tasks = response.readEntity(new GenericType>() {}); + Assert.assertEquals(tasks.size(), children(leafGrandparent).size()); + Assert.assertTrue(tasksContain(tasks, leafParent), "count limited recursive tasks should have included leaf parent "+leafParent+"; was "+tasks); + Assert.assertFalse(tasksContain(tasks, leaf), "count limited recursive tasks should not have included leaf "+leaf+"; was "+tasks); + + response = client().path("/activities/"+leafGrandparent.getId()+"/children/recurse") + .query("limit", children(leafGrandparent).size()+1) + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + tasks = response.readEntity(new GenericType>() {}); + Assert.assertEquals(tasks.size(), children(leafGrandparent).size()+1); + tasks = response.readEntity(new GenericType>() {}); + Assert.assertTrue(tasksContain(tasks, leafParent), "count+1 limited recursive tasks should have included leaf parent "+leafParent+"; was "+tasks); + Assert.assertTrue(tasksContain(tasks, leaf), "count+1 limited recursive tasks should have included leaf "+leaf+"; was "+tasks); + } + + private boolean tasksContain(Map tasks, Task leaf) { + return tasks.keySet().contains(leaf.getId()); + } + + private List> children(Task t) { + return MutableList.copyOf( ((HasTaskChildren)t).getChildren() ); + } + + @Test + public void testGetEntityActivitiesAndWithLimit() { + Task t = entity.invoke(effector, null); + Task leaf = waitForCompletedDescendantWithChildAndSibling(t, t, CountdownTimer.newInstanceStarted(Duration.ONE_SECOND), 0); + + Response response = client().path("/applications/"+entity.getApplicationId()+ + "/entities/"+entity.getId()+"/activities") + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + List tasks = response.readEntity(new GenericType>() {}); + log.info("Tasks now: "+tasks.size()); + Assert.assertTrue(tasks.size() > 4, "tasks should have been big; was "+tasks); + Assert.assertTrue(tasksContain(tasks, leaf), "tasks should have included leaf "+leaf+"; was "+tasks); + + response = client().path("/applications/"+entity.getApplicationId()+ + "/entities/"+entity.getId()+"/activities") + .query("limit", 3) + .accept(MediaType.APPLICATION_JSON) + .get(); + assertHealthy(response); + tasks = response.readEntity(new GenericType>() {}); + log.info("Tasks limited: "+tasks.size()); + Assert.assertEquals(tasks.size(), 3, "tasks should have been limited; was "+tasks); + Assert.assertFalse(tasksContain(tasks, leaf), "tasks should not have included leaf "+leaf+"; was "+tasks); + } + + private void assertHealthy(Response response) { + if (!HttpAsserts.isHealthyStatusCode(response.getStatus())) { + Asserts.fail("Bad response: "+response.getStatus()+" "+response.readEntity(String.class)); + } + } + + private static boolean tasksContain(List tasks, Task leaf) { + for (TaskSummary ts: tasks) { + if (ts.getId().equals(leaf.getId())) return true; + } + return false; + } + + private int depthOf(Task t) { + int depth = -1; + while (t!=null) { + t = t.getSubmittedByTask(); + depth++; + } + return depth; + } + + private Task waitForCompletedDescendantWithChildAndSibling(Task tRoot, Task t, CountdownTimer timer, int depthSoFar) { + while (timer.isLive()) { + Iterable> children = ((HasTaskChildren)t).getChildren(); + Iterator> ci = children.iterator(); + Task bestFinishedDescendant = null; + while (ci.hasNext()) { + Task tc = ci.next(); + Task finishedDescendant = waitForCompletedDescendantWithChildAndSibling(tRoot, tc, timer, depthSoFar+1); + if (depthOf(finishedDescendant) > depthOf(bestFinishedDescendant)) { + bestFinishedDescendant = finishedDescendant; + } + int finishedDescendantDepth = depthOf(bestFinishedDescendant); + // log.info("finished "+tc+", depth "+finishedDescendantDepth); + if (finishedDescendantDepth < 2) { + if (ci.hasNext()) continue; + throw new IllegalStateException("not deep enough: "+finishedDescendantDepth); + } + if (finishedDescendantDepth == depthSoFar+1) { + // child completed; now check we complete soon, and assert we have siblings + if (ci.hasNext()) continue; + if (!t.blockUntilEnded(timer.getDurationRemaining())) { + Entities.dumpInfo(tRoot); + // log.info("Incomplete after "+t+": "+t.getStatusDetail(false)); + throw Exceptions.propagate( new TimeoutException("parent didn't complete after child depth "+finishedDescendantDepth) ); + } + } + if (finishedDescendantDepth == depthSoFar+2) { + if (Iterables.size(children)<2) { + Entities.dumpInfo(tRoot); + throw new IllegalStateException("finished child's parent has no sibling"); + } + } + + return bestFinishedDescendant; + } + Thread.yield(); + + // leaf nodeå + if (t.isDone()) return t; + } + throw Exceptions.propagate( new TimeoutException("expired waiting for children") ); + } + +} diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ApplicationResourceTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ApplicationResourceTest.java index 277fd3225e..76c08e8d84 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ApplicationResourceTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ApplicationResourceTest.java @@ -32,7 +32,11 @@ import java.util.Set; import java.util.concurrent.TimeoutException; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.GenericType; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; @@ -71,7 +75,7 @@ import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.http.HttpAsserts; import org.apache.brooklyn.util.text.Strings; -import org.apache.brooklyn.util.time.Duration; +import org.apache.cxf.jaxrs.client.WebClient; import org.apache.http.HttpHeaders; import org.apache.http.entity.ContentType; import org.slf4j.Logger; @@ -87,13 +91,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.client.Entity; -import javax.ws.rs.core.GenericType; -import javax.ws.rs.core.MultivaluedHashMap; - -import org.apache.cxf.jaxrs.client.WebClient; - @Test(singleThreaded = true, // by using a different suite name we disallow interleaving other tests between the methods of this test class, which wrecks the test fixtures suiteName = "ApplicationResourceTest") diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java index 5fe729d4f5..d7157fd37e 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/EffectorUtilsRestTest.java @@ -85,6 +85,7 @@ public void testInvokingEffector() { } catch (InterruptedException|ExecutionException e) { throw Exceptions.propagate(e); } + log.debug("Result description: "+summary.getDescription()); assertEquals( summary.getDescription(), "Invoking effector resetPassword on "+TestEntityWithEffectors.class.getSimpleName()+":"+entity.getId().substring(0,4) diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ErrorResponseTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ErrorResponseTest.java index 27d8ea5609..af5007d460 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ErrorResponseTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/ErrorResponseTest.java @@ -23,7 +23,6 @@ import static org.testng.Assert.assertTrue; import java.io.IOException; -import java.io.InputStream; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; @@ -36,7 +35,6 @@ import org.apache.brooklyn.rest.testing.BrooklynRestResourceTest; import org.apache.brooklyn.rest.testing.mocks.RestMockSimpleEntity; import org.apache.brooklyn.rest.testing.mocks.RestMockSimplePolicy; -import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; From 36e0a8f200f7f9252de853b06741b0ba0e14c713 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Fri, 7 Jul 2017 02:27:33 +0100 Subject: [PATCH 7/8] obscure, but when there are lots of tasks give preference to top-level and recent --- .../rest/resources/EntityResource.java | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java index 7007a7f397..7d764a4c57 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java @@ -24,6 +24,7 @@ import static org.apache.brooklyn.rest.util.WebResourceUtils.serviceAbsoluteUriBuilder; import java.net.URI; +import java.util.Comparator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -64,10 +65,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.Beta; +import com.google.common.base.Objects; import com.google.common.collect.Collections2; import com.google.common.collect.FluentIterable; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import com.google.common.collect.Ordering; import com.google.common.io.Files; @HaHotStateRequired @@ -134,6 +138,9 @@ public List listTasks(String applicationId, String entityId, int li int sizeRemaining = limit; Entity entity = brooklyn().getEntity(applicationId, entityId); List> tasksToScan = MutableList.copyOf(BrooklynTaskTags.getTasksInEntityContext(mgmt().getExecutionManager(), entity)); + if (limit>0) { + tasksToScan = MutableList.copyOf(Ordering.from(new InterestingTasksFirstComparator(entity)).leastOf(tasksToScan, limit)); + } Map> tasksLoaded = MutableMap.of(); while (!tasksToScan.isEmpty()) { @@ -153,6 +160,84 @@ public List listTasks(String applicationId, String entityId, int li TaskTransformer.fromTask(ui.getBaseUriBuilder()))); } + /** API does not guarantee order, but this is a the one we use (when there are lots of tasks): + * prefer top-level tasks and to recent tasks, + * balanced such that the following are equal: + *
  • something manually submitted here, submitted two hours ago + *
  • something submitted from another entity, submitted ten minutes ago + *
  • anything in progress, submitted one minute ago + *
  • anything not started, submitted ten seconds ago + *
  • anything completed, submitted one second ago + *

    + * So if there was a manual "foo" effector invoked via REST on this entity a day ago, + * a "bar" effector invoked from a parent effector would only be preferred + * if invoked in the last two hours; + * active subtasks of "bar" would be preferred if submitted within the last 12 minutes, + * unstarted subtasks if submitted within 2 minutes, + * and completed subtasks if within the last 12 seconds. + * Thus there is a heavy bias over time to show the top-level tasks, + * but there is also a bias for detail for very recent activity. + *

    + * It's far from perfect but provides a way -- when there are lots of tasks -- + * that we can show important things, where important things are the top level + * and very recent. + */ + @Beta + public static class InterestingTasksFirstComparator implements Comparator> { + Entity context; + public InterestingTasksFirstComparator() { this(null); } + public InterestingTasksFirstComparator(Entity entity) { this.context = entity; } + @Override + public int compare(Task o1, Task o2) { + // absolute pref for submitted items + if (!Objects.equal(o1.isSubmitted(), o2.isSubmitted())) { + return o1.isSubmitted() ? -1 : 1; + } + + // big pref for top-level tasks (manual operations), where submitter null + int weight = 0; + Task o1s = o1.getSubmittedByTask(); + Task o2s = o2.getSubmittedByTask(); + if ("start".equals(o1.getDisplayName()) ||"start".equals(o2.getDisplayName())) { + weight = 0; + } + if (!Objects.equal(o1s==null, o2s==null)) + weight += 2*60*60 * (o1s==null ? -1 : 1); + + // pretty big pref for things invoked by other entities + if (context!=null && o1s!=null && o2s!=null) { + boolean o1se = context.equals(BrooklynTaskTags.getContextEntity(o1s)); + boolean o2se = context.equals(BrooklynTaskTags.getContextEntity(o2s)); + if (!Objects.equal(o1se, o2se)) + weight += 10*60 * (o2se ? -1 : 1); + } + // slight pref for things in progress + if (!Objects.equal(o1.isBegun() && !o1.isDone(), o2.isBegun() && !o2.isDone())) + weight += 60 * (o1.isBegun() && !o1.isDone() ? -1 : 1); + // and very slight pref for things not begun + if (!Objects.equal(o1.isBegun(), o2.isBegun())) + weight += 10 * (!o1.isBegun() ? -1 : 1); + + // sort based on how recently the task changed state + long now = System.currentTimeMillis(); + long t1 = o1.isDone() ? o1.getEndTimeUtc() : o1.isBegun() ? o1.getStartTimeUtc() : o1.getSubmitTimeUtc(); + long t2 = o2.isDone() ? o2.getEndTimeUtc() : o2.isBegun() ? o2.getStartTimeUtc() : o2.getSubmitTimeUtc(); + long u1 = now - t1; + long u2 = now - t2; + // so smaller = more recent + // and if there is a weight, increase the other side so it is de-emphasised + // IE if weight was -10 that means T1 is "10 times more interesting" + // or more precisely, a task T1 from 10 mins ago equals a task T2 from 1 min ago + if (weight<0) u2 *= -weight; + else if (weight>0) u1 *= weight; + if (u1!=u2) return u1 > u2 ? 1 : -1; + // if equal under mapping, use weight + if (weight!=0) return weight < 0 ? -1 : 1; + // lastly use ID to ensure canonical order + return o1.getId().compareTo(o2.getId()); + } + } + @Override @Deprecated public List listTasks(String applicationId, String entityId) { return listTasks(applicationId, entityId, -1, false); From ca9efdbb8ddde482a375f5ca3ae68bed5225471e Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Fri, 7 Jul 2017 02:40:15 +0100 Subject: [PATCH 8/8] add license header --- .../core/effector/SampleManyTasksEffector.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java b/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java index da36de3c4f..d4cb965a3d 100644 --- a/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java +++ b/core/src/test/java/org/apache/brooklyn/core/effector/SampleManyTasksEffector.java @@ -1,3 +1,21 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.core.effector; import java.util.List;