Return-Path: X-Original-To: apmail-brooklyn-commits-archive@minotaur.apache.org Delivered-To: apmail-brooklyn-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 8F3471193D for ; Tue, 29 Jul 2014 19:32:21 +0000 (UTC) Received: (qmail 93492 invoked by uid 500); 29 Jul 2014 19:32:21 -0000 Delivered-To: apmail-brooklyn-commits-archive@brooklyn.apache.org Received: (qmail 93470 invoked by uid 500); 29 Jul 2014 19:32:21 -0000 Mailing-List: contact commits-help@brooklyn.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.incubator.apache.org Delivered-To: mailing list commits@brooklyn.incubator.apache.org Received: (qmail 93461 invoked by uid 99); 29 Jul 2014 19:32:21 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 29 Jul 2014 19:32:21 +0000 X-ASF-Spam-Status: No, hits=-2000.6 required=5.0 tests=ALL_TRUSTED,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Tue, 29 Jul 2014 19:32:18 +0000 Received: (qmail 92779 invoked by uid 99); 29 Jul 2014 19:31:57 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 29 Jul 2014 19:31:57 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 76A72821D79; Tue, 29 Jul 2014 19:31:57 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: heneveld@apache.org To: commits@brooklyn.incubator.apache.org Date: Tue, 29 Jul 2014 19:32:17 -0000 Message-Id: In-Reply-To: <50e9e01d996446b0818219fda11aaae8@git.apache.org> References: <50e9e01d996446b0818219fda11aaae8@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [22/31] git commit: code review - excellent comments @aledsage X-Virus-Checked: Checked by ClamAV on apache.org code review - excellent comments @aledsage Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/8f2a6941 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/8f2a6941 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/8f2a6941 Branch: refs/heads/master Commit: 8f2a6941bc538280a22ed2094f8c078ced5cc7f2 Parents: 40ce411 Author: Alex Heneveld Authored: Tue Jul 29 10:39:07 2014 -0400 Committer: Alex Heneveld Committed: Tue Jul 29 14:39:27 2014 -0400 ---------------------------------------------------------------------- .../brooklyn/entity/group/DynamicCluster.java | 18 ------- .../entity/group/DynamicClusterImpl.java | 33 +++++++----- .../event/basic/DependentConfiguration.java | 3 ++ .../util/task/BasicExecutionManager.java | 54 ++++++++++---------- .../main/java/brooklyn/util/task/BasicTask.java | 9 ++-- .../java/brooklyn/util/task/TaskBuilder.java | 6 +-- .../location/jclouds/JcloudsLocation.java | 2 +- .../brooklyn/location/jclouds/JcloudsUtil.java | 2 + .../entity/webapp/DynamicWebAppClusterImpl.java | 53 +++++++++---------- .../util/concurrent/CallableFromRunnable.java | 54 ++++++++++++++++++++ .../util/exceptions/ReferenceWithError.java | 33 +++++++----- .../brooklyn/util/javalang/Reflections.java | 1 + .../brooklyn/util/javalang/RunnableAdapter.java | 44 ---------------- .../main/java/brooklyn/util/net/Networking.java | 8 +-- .../java/brooklyn/util/repeat/Repeater.java | 8 +-- .../java/brooklyn/util/GroovyJavaMethods.groovy | 4 +- 16 files changed, 173 insertions(+), 159 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/core/src/main/java/brooklyn/entity/group/DynamicCluster.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/group/DynamicCluster.java b/core/src/main/java/brooklyn/entity/group/DynamicCluster.java index 6fec83e..c44f302 100644 --- a/core/src/main/java/brooklyn/entity/group/DynamicCluster.java +++ b/core/src/main/java/brooklyn/entity/group/DynamicCluster.java @@ -45,13 +45,11 @@ import brooklyn.event.basic.BasicAttributeSensor; import brooklyn.event.basic.BasicNotificationSensor; import brooklyn.event.basic.Sensors; import brooklyn.location.Location; -import brooklyn.util.exceptions.ReferenceWithError; import brooklyn.util.flags.SetFromFlag; import brooklyn.util.time.Duration; import com.google.common.annotations.Beta; import com.google.common.base.Function; -import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Multimap; import com.google.common.reflect.TypeToken; @@ -178,22 +176,6 @@ public interface DynamicCluster extends AbstractGroup, Cluster, MemberReplaceabl @Effector(description="Changes the size of the cluster.") Collection resizeByDelta(@EffectorParam(name="delta", description="The change in number of nodes") int delta); - /** - * Adds a node to the cluster in a single {@link Location} - * - * @deprecated since 0.7.0 tricky having this on the interface as implementation details - * may change; for instance we are (22 Jul) changing the return type to be a ReferenceWithError - */ - ReferenceWithError> addInSingleLocation(Location loc, Map extraFlags); - - /** - * Adds a node to the cluster in each {@link Location} - * - * @deprecated since 0.7.0 tricky having this on the interface as implementation details - * may change; for instance we are (22 Jul) changing the return type to be a ReferenceWithError - */ - ReferenceWithError> addInEachLocation(Iterable locs, Map extraFlags); - void setRemovalStrategy(Function, Entity> val); void setZonePlacementStrategy(NodePlacementStrategy val); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java index 271d9e5..b6224a4 100644 --- a/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java +++ b/core/src/main/java/brooklyn/entity/group/DynamicClusterImpl.java @@ -263,7 +263,8 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus resize(initialSize); } catch (Exception e) { Exceptions.propagateIfFatal(e); - // ignore problems here; we extract them below + // apart from logging, ignore problems here; we extract them below + LOG.debug("Error resizing "+this+" to size "+initialSize+" (collecting and handling): "+e, e); } Iterable> failed = Tasks.failed(Tasks.children(Tasks.current())); @@ -487,7 +488,7 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus synchronized (mutex) { ReferenceWithError> added = addInSingleLocation(memberLoc, extraFlags); - if (!added.getIgnoringError().isPresent()) { + if (!added.getMaskingError().isPresent()) { String msg = String.format("In %s, failed to grow, to replace %s; not removing", this, member); if (added.hasError()) throw new IllegalStateException(msg, added.getError()); @@ -501,7 +502,7 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus throw new StopFailedRuntimeException("replaceMember failed to stop and remove old member "+member.getId(), e); } - return added.getOrThrowError().get(); + return added.getThrowingError().get(); } } @@ -584,7 +585,7 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus } // create and start the entities - return addInEachLocation(chosenLocations, ImmutableMap.of()).getOrThrowError(); + return addInEachLocation(chosenLocations, ImmutableMap.of()).getThrowingError(); } /** Note for sub-clases; this method can be called while synchronized on {@link #mutex}. */ @@ -615,16 +616,22 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus } } - @Override - public ReferenceWithError> addInSingleLocation(Location location, Map flags) { + protected ReferenceWithError> addInSingleLocation(Location location, Map flags) { ReferenceWithError> added = addInEachLocation(ImmutableList.of(location), flags); - return ReferenceWithError.newInstanceWithInformativeError( - Iterables.isEmpty(added.getIgnoringError()) ? Optional.absent() : Optional.of(Iterables.getOnlyElement(added.getIgnoringError())), - added.getError()); + + Optional result = Iterables.isEmpty(added.getMaskingError()) ? Optional.absent() : Optional.of(Iterables.getOnlyElement(added.get())); + if (!added.hasError()) { + return ReferenceWithError.newInstanceWithoutError( result ); + } else { + if (added.masksErrorIfPresent()) { + return ReferenceWithError.newInstanceMaskingError( result, added.getError() ); + } else { + return ReferenceWithError.newInstanceThrowingError( result, added.getError() ); + } + } } - @Override - public ReferenceWithError> addInEachLocation(Iterable locations, Map flags) { + protected ReferenceWithError> addInEachLocation(Iterable locations, Map flags) { List addedEntities = Lists.newArrayList(); Map addedEntityLocations = Maps.newLinkedHashMap(); Map> tasks = Maps.newLinkedHashMap(); @@ -669,10 +676,10 @@ public class DynamicClusterImpl extends AbstractGroupImpl implements DynamicClus } else { cleanupFailedNodes(errors.keySet()); } - return ReferenceWithError.newInstanceWithInformativeError(result, Exceptions.create(errors.values())); + return ReferenceWithError.newInstanceMaskingError(result, Exceptions.create(errors.values())); } - return ReferenceWithError.newInstanceWithNoError(result); + return ReferenceWithError.newInstanceWithoutError(result); } protected void quarantineFailedNodes(Collection failedEntities) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/core/src/main/java/brooklyn/event/basic/DependentConfiguration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/event/basic/DependentConfiguration.java b/core/src/main/java/brooklyn/event/basic/DependentConfiguration.java index d569aea..8630214 100644 --- a/core/src/main/java/brooklyn/event/basic/DependentConfiguration.java +++ b/core/src/main/java/brooklyn/event/basic/DependentConfiguration.java @@ -151,6 +151,9 @@ public class DependentConfiguration { return waitInTaskForAttributeReady(source, sensor, ready, ImmutableList.>of()); } + // TODO would be nice to have an easy semantics for whenServiceUp (cf DynamicWebAppClusterImpl.whenServiceUp) + // and TODO would be nice to have it stop when source is unmanaged (with ability to define post-processing) + // probably using the builder for both of these... public static T waitInTaskForAttributeReady(final Entity source, final AttributeSensor sensor, Predicate ready, List> abortConditions) { T value = source.getAttribute(sensor); final List abortion = Lists.newCopyOnWriteArrayList(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/core/src/main/java/brooklyn/util/task/BasicExecutionManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/task/BasicExecutionManager.java b/core/src/main/java/brooklyn/util/task/BasicExecutionManager.java index 18858ee..da8c456 100644 --- a/core/src/main/java/brooklyn/util/task/BasicExecutionManager.java +++ b/core/src/main/java/brooklyn/util/task/BasicExecutionManager.java @@ -288,33 +288,33 @@ public class BasicExecutionManager implements ExecutionManager { } public Task scheduleWith(Task task) { return scheduleWith(Collections.emptyMap(), task); } - public Task scheduleWith(Map flags, Task task) { - synchronized (task) { - if (((TaskInternal)task).getResult()!=null) return task; - return submitNewTask(flags, task); - } - } + public Task scheduleWith(Map flags, Task task) { + synchronized (task) { + if (((TaskInternal)task).getResult()!=null) return task; + return submitNewTask(flags, task); + } + } - @SuppressWarnings("unchecked") + @SuppressWarnings("unchecked") protected Task submitNewScheduledTask(final Map flags, final ScheduledTask task) { - task.submitTimeUtc = System.currentTimeMillis(); - tasksById.put(task.getId(), task); - if (!task.isDone()) { - task.result = delayedRunner.schedule(new ScheduledTaskCallable(task, flags), - task.delay.toNanoseconds(), TimeUnit.NANOSECONDS); - } else { - task.endTimeUtc = System.currentTimeMillis(); - } - return task; - } - - protected class ScheduledTaskCallable implements Callable { - public ScheduledTask task; - public Map flags; - - public ScheduledTaskCallable(ScheduledTask task, Map flags) { - this.task = task; - this.flags = flags; + task.submitTimeUtc = System.currentTimeMillis(); + tasksById.put(task.getId(), task); + if (!task.isDone()) { + task.result = delayedRunner.schedule(new ScheduledTaskCallable(task, flags), + task.delay.toNanoseconds(), TimeUnit.NANOSECONDS); + } else { + task.endTimeUtc = System.currentTimeMillis(); + } + return task; + } + + protected class ScheduledTaskCallable implements Callable { + public ScheduledTask task; + public Map flags; + + public ScheduledTaskCallable(ScheduledTask task, Map flags) { + this.task = task; + this.flags = flags; } @SuppressWarnings({ "rawtypes", "unchecked" }) @@ -352,12 +352,12 @@ public class BasicExecutionManager implements ExecutionManager { afterEnd(flags, task); } } - + @Override public String toString() { return "ScheduledTaskCallable["+task+","+flags+"]"; } - } + } @SuppressWarnings("unchecked") protected Task submitNewTask(final Map flags, final Task task) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/core/src/main/java/brooklyn/util/task/BasicTask.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/task/BasicTask.java b/core/src/main/java/brooklyn/util/task/BasicTask.java index 8370512..5bbd9cb 100644 --- a/core/src/main/java/brooklyn/util/task/BasicTask.java +++ b/core/src/main/java/brooklyn/util/task/BasicTask.java @@ -144,10 +144,11 @@ public class BasicTask implements TaskInternal { @Override public String toString() { // give display name plus id, or job and tags plus id; some jobs have been extended to include nice tostrings - return "Task["+(Strings.isNonEmpty(displayName) ? displayName : - job + - (tags!=null && !tags.isEmpty() ? ";"+tags : "")) + - ":"+getId()+"]"; + return "Task["+ + (Strings.isNonEmpty(displayName) ? + displayName : + (job + (tags!=null && !tags.isEmpty() ? ";"+tags : "")) ) + + ":"+getId()+"]"; } @Override http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/core/src/main/java/brooklyn/util/task/TaskBuilder.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/task/TaskBuilder.java b/core/src/main/java/brooklyn/util/task/TaskBuilder.java index 8b0150d..ed3e282 100644 --- a/core/src/main/java/brooklyn/util/task/TaskBuilder.java +++ b/core/src/main/java/brooklyn/util/task/TaskBuilder.java @@ -130,9 +130,9 @@ public class TaskBuilder { @SuppressWarnings({ "unchecked", "rawtypes" }) public Task build() { MutableMap taskFlags = MutableMap.copyOf(flags); - if (name!=null) taskFlags.add("displayName", name); - if (description!=null) taskFlags.add("description", description); - if (!tags.isEmpty()) taskFlags.add("tags", tags); + if (name!=null) taskFlags.put("displayName", name); + if (description!=null) taskFlags.put("description", description); + if (!tags.isEmpty()) taskFlags.put("tags", tags); if (Boolean.FALSE.equals(dynamic) && children.isEmpty()) { if (swallowChildrenFailures!=null) http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java index f37c3cb..0736a7d 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java @@ -1742,7 +1742,7 @@ public class JcloudsLocation extends AbstractCloudMachineProvisioningLocation im .limitTimeTo(delayMs, MILLISECONDS) .runKeepingError(); - if (!reachable.getIgnoringError()) { + if (!reachable.getMaskingError()) { throw new IllegalStateException("SSH failed for "+ user+"@"+vmIp+" ("+setup.getDescription()+") after waiting "+ Time.makeTimeStringRounded(delayMs), reachable.getError()); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java index 2ac077e..9bb6027 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java @@ -76,6 +76,7 @@ import brooklyn.location.jclouds.config.BrooklynStandardJcloudsGuiceModule; import brooklyn.util.collections.MutableList; import brooklyn.util.collections.MutableMap; import brooklyn.util.config.ConfigBag; +import brooklyn.util.exceptions.Exceptions; import brooklyn.util.net.Protocol; import brooklyn.util.ssh.BashCommands; import brooklyn.util.ssh.IptablesCommands; @@ -391,6 +392,7 @@ public class JcloudsUtil implements JcloudsLocationConfig { try { client = context.utils().sshForNode().apply(node); } catch (Exception e) { + Exceptions.propagateIfFatal(e); /* i've seen: java.lang.IllegalStateException: Optional.get() cannot be called on an absent value * from org.jclouds.crypto.ASN1Codec.createASN1Sequence(ASN1Codec.java:86), if the ssh key has a passphrase, against AWS. * http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/software/webapp/src/main/java/brooklyn/entity/webapp/DynamicWebAppClusterImpl.java ---------------------------------------------------------------------- diff --git a/software/webapp/src/main/java/brooklyn/entity/webapp/DynamicWebAppClusterImpl.java b/software/webapp/src/main/java/brooklyn/entity/webapp/DynamicWebAppClusterImpl.java index 7eb6e25..1b40195 100644 --- a/software/webapp/src/main/java/brooklyn/entity/webapp/DynamicWebAppClusterImpl.java +++ b/software/webapp/src/main/java/brooklyn/entity/webapp/DynamicWebAppClusterImpl.java @@ -30,8 +30,6 @@ import org.slf4j.LoggerFactory; import brooklyn.enricher.Enrichers; import brooklyn.entity.Entity; -import brooklyn.entity.annotation.Effector; -import brooklyn.entity.annotation.EffectorParam; import brooklyn.entity.basic.Attributes; import brooklyn.entity.basic.Entities; import brooklyn.entity.basic.EntityInternal; @@ -68,7 +66,7 @@ import com.google.common.collect.Iterables; public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements DynamicWebAppCluster { private static final Logger log = LoggerFactory.getLogger(DynamicWebAppClusterImpl.class); - private static final FilenameToWebContextMapper filenameToWebContextMapper = new FilenameToWebContextMapper(); + private static final FilenameToWebContextMapper FILENAME_TO_WEB_CONTEXT_MAPPER = new FilenameToWebContextMapper(); /** * Instantiate a new DynamicWebAppCluster. Parameters as per {@link DynamicCluster#DynamicCluster()} @@ -160,6 +158,7 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna } // TODO this will probably be useful elsewhere ... but where to put it? + // TODO add support for this in DependentConfiguration (see TODO there) /** Waits for the given target to report service up, then runs the given task * (often an invocation on that entity), with the given name. * If the target goes away, this task marks itself inessential @@ -174,7 +173,7 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna Tasks.markInessential(); throw new IllegalStateException("Target "+target+" is no longer managed"); } - if (target.getAttribute(Attributes.SERVICE_UP)) { + if (Boolean.TRUE.equals(target.getAttribute(Attributes.SERVICE_UP))) { Tasks.resetBlockingDetails(); TaskTags.markInessential(task); DynamicTasks.queue(task); @@ -202,13 +201,10 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna } @Override - @Effector(description="Deploys the given artifact, from a source URL, to a given deployment filename/context") - public void deploy( - @EffectorParam(name="url", description="URL of WAR file") String url, - @EffectorParam(name="targetName", description="context path where WAR should be deployed (/ for ROOT)") String targetName) { + public void deploy(String url, String targetName) { checkNotNull(url, "url"); checkNotNull(targetName, "targetName"); - targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName); + targetName = FILENAME_TO_WEB_CONTEXT_MAPPER.convertDeploymentTargetNameToContext(targetName); // set it up so future nodes get the right wars addToWarsByContext(this, url, targetName); @@ -224,19 +220,21 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna DynamicTasks.queueIfPossible(tb.build()).orSubmitAsync(this).asTask().getUnchecked(); // Update attribute + // TODO support for atomic sensor update (should be part of standard tooling; NB there is some work towards this, according to @aledsage) Set deployedWars = MutableSet.copyOf(getAttribute(DEPLOYED_WARS)); deployedWars.add(targetName); setAttribute(DEPLOYED_WARS, deployedWars); } @Override - @Effector(description="Undeploys the given context/artifact") - public void undeploy(@EffectorParam(name="targetName") String targetName) { + public void undeploy(String targetName) { checkNotNull(targetName, "targetName"); - targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName); + targetName = FILENAME_TO_WEB_CONTEXT_MAPPER.convertDeploymentTargetNameToContext(targetName); // set it up so future nodes get the right wars - removeFromWarsByContext(this, targetName); + if (!removeFromWarsByContext(this, targetName)) { + DynamicTasks.submit(Tasks.warning("Context "+targetName+" not known at "+this+"; attempting to undeploy regardless", null), this); + } log.debug("Undeploying "+targetName+" across cluster "+this+"; WARs now "+getConfig(WARS_BY_CONTEXT)); @@ -250,12 +248,13 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna // Update attribute Set deployedWars = MutableSet.copyOf(getAttribute(DEPLOYED_WARS)); - deployedWars.remove( filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName) ); + deployedWars.remove( FILENAME_TO_WEB_CONTEXT_MAPPER.convertDeploymentTargetNameToContext(targetName) ); setAttribute(DEPLOYED_WARS, deployedWars); } static void addToWarsByContext(Entity entity, String url, String targetName) { - targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName); + targetName = FILENAME_TO_WEB_CONTEXT_MAPPER.convertDeploymentTargetNameToContext(targetName); + // TODO a better way to do atomic updates, see comment above synchronized (entity) { Map newWarsMap = MutableMap.copyOf(entity.getConfig(WARS_BY_CONTEXT)); newWarsMap.put(targetName, url); @@ -263,18 +262,20 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna } } - static void removeFromWarsByContext(Entity entity, String targetName) { - targetName = filenameToWebContextMapper.convertDeploymentTargetNameToContext(targetName); + static boolean removeFromWarsByContext(Entity entity, String targetName) { + targetName = FILENAME_TO_WEB_CONTEXT_MAPPER.convertDeploymentTargetNameToContext(targetName); + // TODO a better way to do atomic updates, see comment above synchronized (entity) { Map newWarsMap = MutableMap.copyOf(entity.getConfig(WARS_BY_CONTEXT)); String url = newWarsMap.remove(targetName); if (url==null) { - DynamicTasks.submit(Tasks.warning("Context "+targetName+" not known at "+entity+"; attempting to undeploy regardless", null), entity); + return false; } ((EntityInternal)entity).setConfig(WARS_BY_CONTEXT, newWarsMap); + return true; } } - + @Override public void redeployAll() { Map wars = MutableMap.copyOf(getConfig(WARS_BY_CONTEXT)); @@ -282,14 +283,14 @@ public class DynamicWebAppClusterImpl extends DynamicClusterImpl implements Dyna log.debug("Redeplying all WARs across cluster "+this+": "+getConfig(WARS_BY_CONTEXT)); - Iterable targets = Iterables.filter(getChildren(), CanDeployAndUndeploy.class); - TaskBuilder tb = Tasks.builder().parallel(true).name(redeployPrefix+" across cluster (size "+Iterables.size(targets)+")"); - for (Entity target: targets) { - TaskBuilder redeployAllToTarget = Tasks.builder().name(redeployPrefix+" at "+target+" (after ready check)"); - for (String targetName: wars.keySet()) { - redeployAllToTarget.add(Effectors.invocation(target, DEPLOY, MutableMap.of("url", wars.get(targetName), "targetName", targetName))); + Iterable targetEntities = Iterables.filter(getChildren(), CanDeployAndUndeploy.class); + TaskBuilder tb = Tasks.builder().parallel(true).name(redeployPrefix+" across cluster (size "+Iterables.size(targetEntities)+")"); + for (Entity targetEntity: targetEntities) { + TaskBuilder redeployAllToTarget = Tasks.builder().name(redeployPrefix+" at "+targetEntity+" (after ready check)"); + for (String warContextPath: wars.keySet()) { + redeployAllToTarget.add(Effectors.invocation(targetEntity, DEPLOY, MutableMap.of("url", wars.get(warContextPath), "targetName", warContextPath))); } - tb.add(whenServiceUp(target, redeployAllToTarget.build(), redeployPrefix+" at "+target+" when ready")); + tb.add(whenServiceUp(targetEntity, redeployAllToTarget.build(), redeployPrefix+" at "+targetEntity+" when ready")); } DynamicTasks.queueIfPossible(tb.build()).orSubmitAsync(this).asTask().getUnchecked(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/common/src/main/java/brooklyn/util/concurrent/CallableFromRunnable.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/concurrent/CallableFromRunnable.java b/utils/common/src/main/java/brooklyn/util/concurrent/CallableFromRunnable.java new file mode 100644 index 0000000..2160e5f --- /dev/null +++ b/utils/common/src/main/java/brooklyn/util/concurrent/CallableFromRunnable.java @@ -0,0 +1,54 @@ +/* + * 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 brooklyn.util.concurrent; + +import java.util.concurrent.Callable; +import java.util.concurrent.Executors; + +import com.google.common.annotations.Beta; + +/** Wraps a Runnable as a Callable. Like {@link Executors#callable(Runnable, Object)} but including the underlying toString. */ +@Beta +public class CallableFromRunnable implements Callable { + + public static CallableFromRunnable newInstance(Runnable task, T result) { + return new CallableFromRunnable(task, result); + } + + private final Runnable task; + private final T result; + + protected CallableFromRunnable(Runnable task, T result) { + this.task = task; + this.result = result; + } + + public T call() { + task.run(); + return result; + } + + @Override + public String toString() { + if (result!=null) + return "CallableFromRunnable["+task+(result!=null ? "->"+result : "")+"]"; + else + return ""+task; + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/common/src/main/java/brooklyn/util/exceptions/ReferenceWithError.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/exceptions/ReferenceWithError.java b/utils/common/src/main/java/brooklyn/util/exceptions/ReferenceWithError.java index 7de6fb5..cdb5925 100644 --- a/utils/common/src/main/java/brooklyn/util/exceptions/ReferenceWithError.java +++ b/utils/common/src/main/java/brooklyn/util/exceptions/ReferenceWithError.java @@ -27,58 +27,65 @@ public class ReferenceWithError implements Supplier { private final T object; private final Throwable error; - private final boolean throwErrorOnAccess; + private final boolean maskError; /** returns a reference which includes an error, and where attempts to get the content cause the error to throw */ - public static ReferenceWithError newInstanceWithFatalError(T object, Throwable error) { + public static ReferenceWithError newInstanceThrowingError(T object, Throwable error) { return new ReferenceWithError(object, error, true); } /** returns a reference which includes an error, but attempts to get the content do not cause the error to throw */ - public static ReferenceWithError newInstanceWithInformativeError(T object, Throwable error) { + public static ReferenceWithError newInstanceMaskingError(T object, Throwable error) { return new ReferenceWithError(object, error, false); } /** returns a reference which includes an error, but attempts to get the content do not cause the error to throw */ - public static ReferenceWithError newInstanceWithNoError(T object) { + public static ReferenceWithError newInstanceWithoutError(T object) { return new ReferenceWithError(object, null, false); } protected ReferenceWithError(@Nullable T object, @Nullable Throwable error, boolean throwErrorOnAccess) { this.object = object; this.error = error; - this.throwErrorOnAccess = throwErrorOnAccess; + this.maskError = throwErrorOnAccess; } - public boolean throwsErrorOnAccess() { - return throwErrorOnAccess; + /** whether this will mask any error on an attempt to {@link #get()}; + * if false (if created with {@link #newInstanceThrowingError(Object, Throwable)}) a call to {@link #get()} will throw if there is an error; + * true if created with {@link #newInstanceMaskingError(Object, Throwable)} and {@link #get()} will not throw */ + public boolean masksErrorIfPresent() { + return maskError; } + /** returns the underlying value, throwing if there is an error and {@link #throwsErrorOnAccess()} is set */ public T get() { - if (throwsErrorOnAccess()) { - return getOrThrowError(); + if (masksErrorIfPresent()) { + return getMaskingError(); } - return getIgnoringError(); + return getThrowingError(); } - public T getIgnoringError() { + public T getMaskingError() { return object; } - public T getOrThrowError() { + public T getThrowingError() { checkNoError(); return object; } + /** throws if there is an error (even if masked) */ public void checkNoError() { if (hasError()) Exceptions.propagate(error); } - + + /** returns the error (not throwing) */ public Throwable getError() { return error; } + /** true if there is an error (whether masked or not) */ public boolean hasError() { return error!=null; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/common/src/main/java/brooklyn/util/javalang/Reflections.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/javalang/Reflections.java b/utils/common/src/main/java/brooklyn/util/javalang/Reflections.java index 9ea35ce..e5b78c6 100644 --- a/utils/common/src/main/java/brooklyn/util/javalang/Reflections.java +++ b/utils/common/src/main/java/brooklyn/util/javalang/Reflections.java @@ -209,6 +209,7 @@ public class Reflections { /** Invokes a suitable constructor, supporting varargs and primitives */ public static Optional invokeConstructorWithArgs(ClassLoader classLoader, String className, Object...argsArray) { Reflections reflections = new Reflections(classLoader); + @SuppressWarnings("unchecked") Class clazz = (Class) reflections.loadClass(className); return invokeConstructorWithArgs(reflections, clazz, argsArray, false); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/common/src/main/java/brooklyn/util/javalang/RunnableAdapter.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/javalang/RunnableAdapter.java b/utils/common/src/main/java/brooklyn/util/javalang/RunnableAdapter.java deleted file mode 100644 index d61e84b..0000000 --- a/utils/common/src/main/java/brooklyn/util/javalang/RunnableAdapter.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 brooklyn.util.javalang; - -import java.util.concurrent.Callable; - -public class RunnableAdapter implements Callable { - final Runnable task; - final T result; - - public RunnableAdapter(Runnable task, T result) { - this.task = task; - this.result = result; - } - - public T call() { - task.run(); - return result; - } - - @Override - public String toString() { - if (result!=null) - return "RunnableAdapter["+task+(result!=null ? "->"+result : "")+"]"; - else - return ""+task; - } -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/common/src/main/java/brooklyn/util/net/Networking.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/net/Networking.java b/utils/common/src/main/java/brooklyn/util/net/Networking.java index 5c7df18..8c2d4f1 100644 --- a/utils/common/src/main/java/brooklyn/util/net/Networking.java +++ b/utils/common/src/main/java/brooklyn/util/net/Networking.java @@ -387,19 +387,19 @@ public class Networking { // TODO go through nic's, looking for public, private, etc, on localhost /** - * intsall TLSv1, fixing: + * force use of TLSv1, fixing: * http://stackoverflow.com/questions/9828414/receiving-sslhandshakeexception-handshake-failure-despite-my-client-ignoring-al */ - public static void installTlsForHttps() { + public static void installTlsOnlyForHttpsForcing() { System.setProperty("https.protocols", "TLSv1"); } public static void installTlsForHttpsIfAppropriate() { if (System.getProperty("https.protocols")==null && System.getProperty("brooklyn.https.protocols.leave_untouched")==null) { - installTlsForHttps(); + installTlsOnlyForHttpsForcing(); } } static { - installTlsForHttps(); + installTlsForHttpsIfAppropriate(); } /** does nothing, but forces the class to be loaded and do static initialization */ http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/common/src/main/java/brooklyn/util/repeat/Repeater.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/brooklyn/util/repeat/Repeater.java b/utils/common/src/main/java/brooklyn/util/repeat/Repeater.java index b2000fa..be0d3f2 100644 --- a/utils/common/src/main/java/brooklyn/util/repeat/Repeater.java +++ b/utils/common/src/main/java/brooklyn/util/repeat/Repeater.java @@ -301,7 +301,7 @@ public class Repeater { * @return true if the exit condition was satisfied; false if the loop terminated for any other reason. */ public boolean run() { - return runKeepingError().getIgnoringError(); + return runKeepingError().getMaskingError(); } public ReferenceWithError runKeepingError() { @@ -335,7 +335,7 @@ public class Repeater { } if (done) { if (log.isDebugEnabled()) log.debug("{}: condition satisfied", description); - return ReferenceWithError.newInstanceWithNoError(true); + return ReferenceWithError.newInstanceWithoutError(true); } else { if (log.isDebugEnabled()) { String msg = String.format("%s: unsatisfied during iteration %s %s", description, iterations, @@ -357,7 +357,7 @@ public class Repeater { } if (warnOnUnRethrownException && lastError != null) log.warn("{}: error caught checking condition: {}", description, lastError.getMessage()); - return ReferenceWithError.newInstanceWithInformativeError(false, lastError); + return ReferenceWithError.newInstanceMaskingError(false, lastError); } if (timer.isExpired()) { @@ -367,7 +367,7 @@ public class Repeater { log.error("{}: error caught checking condition: {}", description, lastError.getMessage()); throw Exceptions.propagate(lastError); } - return ReferenceWithError.newInstanceWithInformativeError(false, lastError); + return ReferenceWithError.newInstanceMaskingError(false, lastError); } Time.sleep(delayThisIteration); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8f2a6941/utils/groovy/src/main/java/brooklyn/util/GroovyJavaMethods.groovy ---------------------------------------------------------------------- diff --git a/utils/groovy/src/main/java/brooklyn/util/GroovyJavaMethods.groovy b/utils/groovy/src/main/java/brooklyn/util/GroovyJavaMethods.groovy index c34f72d..199f6a5 100644 --- a/utils/groovy/src/main/java/brooklyn/util/GroovyJavaMethods.groovy +++ b/utils/groovy/src/main/java/brooklyn/util/GroovyJavaMethods.groovy @@ -22,7 +22,7 @@ import static brooklyn.util.GroovyJavaMethods.truth import java.util.concurrent.Callable -import brooklyn.util.javalang.RunnableAdapter +import brooklyn.util.concurrent.CallableFromRunnable; import com.google.common.base.Function import com.google.common.base.Predicate @@ -54,7 +54,7 @@ public class GroovyJavaMethods { } public static Callable callableFromRunnable(final Runnable job) { - return (job in Callable) ? callableFromClosure(job) : new RunnableAdapter(job, null); + return (job in Callable) ? callableFromClosure(job) : CallableFromRunnable.newInstance(job, null); } public static Predicate predicateFromClosure(final Closure job) {