You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2015/05/29 12:36:01 UTC

[01/13] incubator-brooklyn git commit: misc cleanups for HA and shutdown

Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 42e9aad4e -> 75194760d


misc cleanups for HA and shutdown

* HA list in GUI reports if data is stale (makes it obvious is some servers are likely dead)
* On "Clear HA nodes" false masters are removed (fix bug where lots of masters, including stale, aren't cleared)
* On shutdown, RebindManager.waitForPending wasn't doing the right thing when invoked by stop, meaning state wasn't being written (really bad if you click "stop all apps", as the final deletion isn't actually persisted!)


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/eef78912
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/eef78912
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/eef78912

Branch: refs/heads/master
Commit: eef78912bb1ccc8193d63b6f5e83a71f6083bc72
Parents: fa09efc
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Sat May 9 12:58:29 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Sun May 10 10:02:33 2015 -0700

----------------------------------------------------------------------
 .../catalog/internal/CatalogInitialization.java |   2 +-
 .../rebind/PeriodicDeltaChangeListener.java     | 169 ++++++++++---------
 .../brooklyn/entity/rebind/RebindIteration.java |   6 +-
 .../entity/rebind/RebindManagerImpl.java        |   4 +-
 .../ha/HighAvailabilityManagerImpl.java         |  10 +-
 .../brooklyn/management/ha/HotStandbyTest.java  |  14 +-
 .../main/webapp/assets/js/view/ha-summary.js    |  98 ++++++-----
 .../brooklyn/rest/filter/LoggingFilter.java     |  14 +-
 .../brooklyn/rest/resources/ServerResource.java |  69 +++++---
 .../java/brooklyn/util/javalang/Threads.java    |  12 +-
 10 files changed, 231 insertions(+), 167 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
index 1710384..ded7dc4 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
@@ -128,7 +128,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
                 if (!catalog.getCatalog().isLoaded()) {
                     catalog.load();
                 } else {
-                    if (hasRunOfficial || hasRunBestEffort) {
+                    if (needsInitial && (hasRunOfficial || hasRunBestEffort)) {
                         // an indication that something caused it to load early; not severe, but unusual
                         log.warn("Catalog initialization has not properly run but management context has a catalog; re-populating, possibly overwriting items installed during earlier access (it may have been an early web request)");
                         catalog.reset(ImmutableList.<CatalogItem<?,?>>of());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
index 281f4fa..cd33f01 100644
--- a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
+++ b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
@@ -25,7 +25,6 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicLong;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -54,12 +53,11 @@ import brooklyn.util.task.ScheduledTask;
 import brooklyn.util.task.Tasks;
 import brooklyn.util.time.CountdownTimer;
 import brooklyn.util.time.Duration;
-import brooklyn.util.time.Time;
 
-import com.google.common.collect.Lists;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 
 /**
@@ -164,14 +162,12 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     private final PersistenceExceptionHandler exceptionHandler;
     
     private final Duration period;
-    
-    private final AtomicLong writeCount = new AtomicLong();
-    
+        
     private DeltaCollector deltaCollector = new DeltaCollector();
 
     private volatile boolean running = false;
 
-    private volatile boolean stopped = false;
+    private volatile boolean stopping = false, stopCompleted = false;
 
     private volatile ScheduledTask scheduledTask;
 
@@ -180,7 +176,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     private final boolean persistFeedsEnabled;
     
     private final Semaphore persistingMutex = new Semaphore(1);
-    private final Object startMutex = new Object();
+    private final Object startStopMutex = new Object();
 
     private PersistenceActivityMetrics metrics;
     
@@ -198,42 +194,20 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     
     @SuppressWarnings("unchecked")
     public void start() {
-        synchronized (startMutex) {
+        synchronized (startStopMutex) {
             if (running || (scheduledTask!=null && !scheduledTask.isDone())) {
                 LOG.warn("Request to start "+this+" when already running - "+scheduledTask+"; ignoring");
                 return;
             }
-            stopped = false;
+            stopCompleted = false;
             running = true;
 
             Callable<Task<?>> taskFactory = new Callable<Task<?>>() {
                 @Override public Task<Void> call() {
                     return Tasks.<Void>builder().dynamic(false).name("periodic-persister").body(new Callable<Void>() {
                         public Void call() {
-                            Stopwatch timer = Stopwatch.createStarted();
-                            try {
-                                persistNow();
-                                metrics.noteSuccess(Duration.of(timer));
-                                return null;
-                            } catch (RuntimeInterruptedException e) {
-                                LOG.debug("Interrupted persisting change-delta (rethrowing)", e);
-                                metrics.noteFailure(Duration.of(timer));
-                                metrics.noteError(e.toString());
-                                Thread.currentThread().interrupt();
-                                return null;
-                            } catch (Exception e) {
-                                // Don't rethrow: the behaviour of executionManager is different from a scheduledExecutorService,
-                                // if we throw an exception, then our task will never get executed again
-                                LOG.error("Problem persisting change-delta", e);
-                                metrics.noteFailure(Duration.of(timer));
-                                metrics.noteError(e.toString());
-                                return null;
-                            } catch (Throwable t) {
-                                LOG.warn("Problem persisting change-delta (rethrowing)", t);
-                                metrics.noteFailure(Duration.of(timer));
-                                metrics.noteError(t.toString());
-                                throw Exceptions.propagate(t);
-                            }
+                            persistNowSafely(false);
+                            return null;
                         }}).build();
                 }
             };
@@ -247,68 +221,73 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
         stop(Duration.TEN_SECONDS, Duration.ONE_SECOND);
     }
     void stop(Duration timeout, Duration graceTimeoutForSubsequentOperations) {
-        stopped = true;
-        running = false;
-        
-        if (scheduledTask != null) {
-            CountdownTimer expiry = timeout.countdownTimer();
-            scheduledTask.cancel(false);
+        synchronized (startStopMutex) {
+            running = false;
             try {
-                waitForPendingComplete(expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
-            } catch (Exception e) {
-                throw Exceptions.propagate(e);
-            }
-            scheduledTask.blockUntilEnded(expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
-            scheduledTask.cancel(true);
-            boolean reallyEnded = Tasks.blockUntilInternalTasksEnded(scheduledTask, expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
-            if (!reallyEnded) {
-                LOG.warn("Persistence tasks took too long to complete when stopping persistence (ignoring): "+scheduledTask);
-            }
-            scheduledTask = null;
-        }
+                stopping = true;
+
+                if (scheduledTask != null) {
+                    CountdownTimer expiry = timeout.countdownTimer();
+                    try {
+                        scheduledTask.cancel(false);  
+                        waitForPendingComplete(expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
+                    } catch (Exception e) {
+                        throw Exceptions.propagate(e);
+                    }
+                    scheduledTask.blockUntilEnded(expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
+                    scheduledTask.cancel(true);
+                    boolean reallyEnded = Tasks.blockUntilInternalTasksEnded(scheduledTask, expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
+                    if (!reallyEnded) {
+                        LOG.warn("Persistence tasks took too long to terminate, when stopping persistence, although pending changes were persisted (ignoring): "+scheduledTask);
+                    }
+                    scheduledTask = null;
+                }
 
 
-        // Discard all state that was waiting to be persisted
-        synchronized (this) {
-            deltaCollector = new DeltaCollector();
+                // Discard all state that was waiting to be persisted
+                synchronized (this) {
+                    deltaCollector = new DeltaCollector();
+                }
+            } finally {
+                stopCompleted = true;
+                stopping = false;
+            }
         }
     }
     
     /**
-     * This method must only be used for testing. If required in production, then revisit implementation!
      * @deprecated since 0.7.0, use {@link #waitForPendingComplete(Duration)}
      */
     @VisibleForTesting
     public void waitForPendingComplete(long timeout, TimeUnit unit) throws InterruptedException, TimeoutException {
         waitForPendingComplete(Duration.of(timeout, unit));
     }
+    /** Waits for any in-progress writes to be completed then for or any unwritten data to be written. */
     @VisibleForTesting
     public void waitForPendingComplete(Duration timeout) throws InterruptedException, TimeoutException {
-        // Every time we finish writing, we increment a counter. We note the current val, and then
-        // wait until we can guarantee that a complete additional write has been done. Not sufficient
-        // to wait for `writeCount > origWriteCount` because we might have read the value when almost 
-        // finished a write.
+        if (!isActive() && !stopping) return;
         
-        long startTime = System.currentTimeMillis();
-        long maxEndtime = timeout.isPositive() ? startTime + timeout.toMillisecondsRoundingUp() : Long.MAX_VALUE;
-        long origWriteCount = writeCount.get();
-        while (true) {
-            if (!isActive()) {
-                return; // no pending activity;
-            } else if (writeCount.get() > (origWriteCount+1)) {
-                return;
-            }
-            
-            if (System.currentTimeMillis() > maxEndtime) {
-                throw new TimeoutException("Timeout waiting for pending complete of rebind-periodic-delta, after "+Time.makeTimeStringRounded(timeout));
+        CountdownTimer timer = timeout.isPositive() ? CountdownTimer.newInstanceStarted(timeout) : CountdownTimer.newInstancePaused(Duration.PRACTICALLY_FOREVER);
+        // wait for mutex, so we aren't tricked by an in-progress who has already recycled the collector
+        if (persistingMutex.tryAcquire(timer.getDurationRemaining().toMilliseconds(), TimeUnit.MILLISECONDS)) {
+            try {
+                // now no one else is writing
+                if (!deltaCollector.isEmpty()) {
+                    // but there is data that needs to be written
+                    persistNowSafely(true);
+                }
+            } finally {
+                persistingMutex.release();
             }
-            Thread.sleep(1);
+        } else {
+            // someone else has been writing for the entire time 
+            throw new TimeoutException("Timeout waiting for completion of in-progress write of rebind-periodic-delta, after "+timer.getDurationElapsed());
         }
     }
 
     /**
-     * Indicates whether to persist things now. Even when not active, we will still store what needs
-     * to be persisted unless {@link #isStopped()}.
+     * Indicates whether persistence is active. 
+     * Even when not active, changes will still be tracked unless {@link #isStopped()}.
      */
     private boolean isActive() {
         return running && persister != null && !isStopped();
@@ -318,7 +297,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
      * Whether we have been stopped, in which case will not persist or store anything.
      */
     private boolean isStopped() {
-        return stopped || executionContext.isShutdown();
+        return stopping || stopCompleted || executionContext.isShutdown();
     }
     
     private void addReferencedObjects(DeltaCollector deltaCollector) {
@@ -348,13 +327,40 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     }
     
     @VisibleForTesting
-    public void persistNow() {
-        if (!isActive()) {
+    public boolean persistNowSafely(boolean alreadyHasMutex) {
+        Stopwatch timer = Stopwatch.createStarted();
+        try {
+            persistNowInternal(alreadyHasMutex);
+            metrics.noteSuccess(Duration.of(timer));
+            return true;
+        } catch (RuntimeInterruptedException e) {
+            LOG.debug("Interrupted persisting change-delta (rethrowing)", e);
+            metrics.noteFailure(Duration.of(timer));
+            metrics.noteError(e.toString());
+            Thread.currentThread().interrupt();
+            return false;
+        } catch (Exception e) {
+            // Don't rethrow: the behaviour of executionManager is different from a scheduledExecutorService,
+            // if we throw an exception, then our task will never get executed again
+            LOG.error("Problem persisting change-delta", e);
+            metrics.noteFailure(Duration.of(timer));
+            metrics.noteError(e.toString());
+            return false;
+        } catch (Throwable t) {
+            LOG.warn("Problem persisting change-delta (rethrowing)", t);
+            metrics.noteFailure(Duration.of(timer));
+            metrics.noteError(t.toString());
+            throw Exceptions.propagate(t);
+        }
+    }
+    
+    protected void persistNowInternal(boolean alreadyHasMutex) {
+        if (!isActive() && !stopping) {
             return;
         }
         try {
-            persistingMutex.acquire();
-            if (!isActive()) return;
+            if (!alreadyHasMutex) persistingMutex.acquire();
+            if (!isActive() && !stopping) return;
             
             // Atomically switch the delta, so subsequent modifications will be done in the
             // next scheduled persist
@@ -419,8 +425,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
                 LOG.debug("Problem persisting, but no longer active (ignoring)", e);
             }
         } finally {
-            writeCount.incrementAndGet();
-            persistingMutex.release();
+            if (!alreadyHasMutex) persistingMutex.release();
         }
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
index 136cb5b..6124a54 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
@@ -301,7 +301,7 @@ public abstract class RebindIteration {
     @SuppressWarnings("deprecation")
     protected void rebuildCatalog() {
         
-        // build catalog early so we can load other things
+        // Build catalog early so we can load other things
         checkEnteringPhase(2);
         
         // Instantiate catalog items
@@ -341,7 +341,7 @@ public abstract class RebindIteration {
             }
         }
 
-        // see notes in CatalogInitialization
+        // See notes in CatalogInitialization
         
         Collection<CatalogItem<?, ?>> catalogItems = rebindContext.getCatalogItems();
         CatalogInitialization catInit = ((ManagementContextInternal)managementContext).getCatalogInitialization();
@@ -398,6 +398,8 @@ public abstract class RebindIteration {
         }
 
         // TODO in read-only mode, perhaps do this less frequently than entities etc ?
+        // both in RW and in RO mode, the first run reads the initialization data;
+        // maybe not desired for RO as it defers problems, although if it's standalone it is desired
         catInit.populateCatalog(needsInitialCatalog, itemsForResettingCatalog);
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
index 71d5218..caf04c5 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -447,7 +447,9 @@ public class RebindManagerImpl implements RebindManager {
             }
             persistenceStoreAccess.checkpoint(memento, exceptionHandler);
         } else {
-            persistenceRealChangeListener.persistNow();
+            if (!persistenceRealChangeListener.persistNowSafely(false)) {
+                throw new IllegalStateException("Forced persistence failed; see logs fore more detail");
+            }
         }
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
index 70bb13d..ab033fd 100644
--- a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
+++ b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
@@ -73,6 +73,7 @@ import brooklyn.util.time.Time;
 import com.google.common.annotations.Beta;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
+import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Ticker;
@@ -606,9 +607,14 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
             return;
         }
         brooklyn.management.ha.ManagementPlaneSyncRecordDeltaImpl.Builder db = ManagementPlaneSyncRecordDeltaImpl.builder();
-        for (Map.Entry<String,ManagementNodeSyncRecord> node: plane.getManagementNodes().entrySet())
-            if (!ManagementNodeState.MASTER.equals(node.getValue().getStatus()))
+        for (Map.Entry<String,ManagementNodeSyncRecord> node: plane.getManagementNodes().entrySet()) {
+            // only keep a node if it both claims master and is recognised as master;
+            // else ex-masters who died are kept around!
+            if (!ManagementNodeState.MASTER.equals(node.getValue().getStatus()) || 
+                    !Objects.equal(plane.getMasterNodeId(), node.getValue().getNodeId())) {
                 db.removedNodeId(node.getKey());
+            }
+        }
         persister.delta(db.build());
         // then get, so model is updated
         loadManagementPlaneSyncRecord(true);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java b/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
index 9c36eb8..f33f716 100644
--- a/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
+++ b/core/src/test/java/brooklyn/management/ha/HotStandbyTest.java
@@ -179,7 +179,7 @@ public class HotStandbyTest {
 //        TestApplication app = ApplicationBuilder.newManagedApp(EntitySpec.create(TestApplication.class).impl(TestApplicationNoEnrichersImpl.class), n1.mgmt);
         app.setDisplayName("First App");
         app.start(MutableList.<Location>of());
-        app.setConfig(TestEntity.CONF_NAME, "first-app");
+        app.config().set(TestEntity.CONF_NAME, "first-app");
         app.setAttribute(TestEntity.SEQUENCE, 3);
         
         forcePersistNow(n1);
@@ -251,7 +251,7 @@ public class HotStandbyTest {
         // test changes
 
         app.setDisplayName("First App Renamed");
-        app.setConfig(TestEntity.CONF_NAME, "first-app-renamed");
+        app.config().set(TestEntity.CONF_NAME, "first-app-renamed");
         app.setAttribute(TestEntity.SEQUENCE, 4);
 
         appRO = expectRebindSequenceNumber(n1, n2, app, 4, true);
@@ -262,7 +262,7 @@ public class HotStandbyTest {
         // and change again for good measure!
 
         app.setDisplayName("First App");
-        app.setConfig(TestEntity.CONF_NAME, "first-app-restored");
+        app.config().set(TestEntity.CONF_NAME, "first-app-restored");
         app.setAttribute(TestEntity.SEQUENCE, 5);
         
         appRO = expectRebindSequenceNumber(n1, n2, app, 5, true);
@@ -296,7 +296,7 @@ public class HotStandbyTest {
         TestEntity child = app.addChild(EntitySpec.create(TestEntity.class).configure(TestEntity.CONF_NAME, "first-child"));
         Entities.manage(child);
         TestApplication app2 = TestApplication.Factory.newManagedInstanceForTests(n1.mgmt);
-        app2.setConfig(TestEntity.CONF_NAME, "second-app");
+        app2.config().set(TestEntity.CONF_NAME, "second-app");
         
         app.setAttribute(TestEntity.SEQUENCE, 4);
         appRO = expectRebindSequenceNumber(n1, n2, app, 4, immediate);
@@ -425,7 +425,7 @@ public class HotStandbyTest {
         TestApplication app = createFirstAppAndPersist(n1);        
         noteUsedMemory("Finished seeding");
         Long initialUsed = usedMemory.peekLast();
-        app.setConfig(TestEntity.CONF_OBJECT, new BigObject(SIZE*1000*1000));
+        app.config().set(TestEntity.CONF_OBJECT, new BigObject(SIZE*1000*1000));
         assertUsedMemoryMaxDelta("Set a big config object", SIZE_UP_BOUND);
         forcePersistNow(n1);
         assertUsedMemoryMaxDelta("Persisted a big config object", SIZE_IN_XML);
@@ -443,7 +443,7 @@ public class HotStandbyTest {
         }
         assertUsedMemoryMaxDelta("And more rebinds and more persists", GRACE);
         
-        app.setConfig(TestEntity.CONF_OBJECT, "big is now small");
+        app.config().set(TestEntity.CONF_OBJECT, "big is now small");
         assertUsedMemoryMaxDelta("Big made small at primary", -SIZE_DOWN_BOUND);
         forcePersistNow(n1);
         assertUsedMemoryMaxDelta("And persisted", -SIZE_IN_XML_DOWN);
@@ -521,7 +521,7 @@ public class HotStandbyTest {
         TestEntity child = app.addChild(EntitySpec.create(TestEntity.class).configure(TestEntity.CONF_NAME, "first-child"));
         Entities.manage(child);
         TestApplication app2 = TestApplication.Factory.newManagedInstanceForTests(n1.mgmt);
-        app2.setConfig(TestEntity.CONF_NAME, "second-app");
+        app2.config().set(TestEntity.CONF_NAME, "second-app");
 
         forcePersistNow(n1);
         n2.ha.setPriority(1);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/usage/jsgui/src/main/webapp/assets/js/view/ha-summary.js
----------------------------------------------------------------------
diff --git a/usage/jsgui/src/main/webapp/assets/js/view/ha-summary.js b/usage/jsgui/src/main/webapp/assets/js/view/ha-summary.js
index 2032d1d..250977e 100644
--- a/usage/jsgui/src/main/webapp/assets/js/view/ha-summary.js
+++ b/usage/jsgui/src/main/webapp/assets/js/view/ha-summary.js
@@ -29,10 +29,11 @@ define([
                 "<% if (nodeUri && !isTerminated) { %><a href='<%= nodeUri %>'><%= nodeId %></a><% } else { %><%= nodeId %><%    } %>" +
                 "<% if (isSelf) { %><span class='pull-right badge badge-success'>this</span><% } %>" +
             "</td>" +
-            "<td><%= status %></td>" +
+            "<td><% if (isPretendMaster) {%>EX-MASTER<%} else {%><%= status %><%} if (isStale) { %> (stale)<% } %></td>" +
             "<td><%= timestampDisplayPrefix %><span class='timestamp' data-timestamp='<%= timestamp %>'><%= timestampDisplay %><span><%= timestampDisplaySuffix %></td>" +
         "</tr>");
-    var noServers = "<tr><td colspan='3'><i>Failed to load servers!</i></td></tr>";
+    var noServers = "<tr><td colspan='3'><i>Failed to load data of servers</i></td></tr>";
+    var waitingServers = "<tr><td colspan='3'><i>Waiting on detail for servers...</i></td></tr>";
 
     var HASummaryView = Backbone.View.extend({
         initialize: function() {
@@ -49,53 +50,74 @@ define([
         },
         render: function() {
             this.$el.html(template());
-            if (serverStatus.loaded) {
-                this.renderNodeStatus();
-            }
+            this.renderNodeStatus();
             return this;
         },
         renderNodeStatus: function() {
+            var $target = this.$(".ha-summary-table-body");
+            if (!serverStatus.loaded) {
+                $target.html(waitingServers);
+                return;
+            }
+            
             var serverHa = serverStatus.get("ha") || {};
             var master = serverHa.masterId,
                 self = serverHa.ownId,
-                nodes = serverHa.nodes,
-                $target = this.$(".ha-summary-table-body");
-            $target.empty();
+                nodes = serverHa.nodes;
+                
             // undefined check just in case server returns something odd
             if (nodes == undefined || _.isEmpty(nodes)) {
                 $target.html(noServers);
-            } else {
-                _.each(nodes, function (n) {
-                    var node = _.clone(n);
-                    node.timestampDisplayPrefix = "";
-                    node.timestampDisplaySuffix = "";
-                    if (node['remoteTimestamp']) {
-                        node.timestamp = node.remoteTimestamp;
-                    } else {
-                        node.timestamp = node.localTimestamp;
-                        node.timestampDisplaySuffix = " (local)";
-                    }
-                    if (node.timestamp >= moment().utc() + 10*1000) {
-                        // if server reports time significantly in future, report this, with no timestampe
-                        node.timestampDisplayPrefix = "server clock in future by "+
-                            moment.duration(moment(node.timestamp).diff(moment())).humanize();
-                        node.timestamp = "";
-                        node.timestampDisplay = "";
-                    } else {
-                        // else use timestamp
-                        if (node.timestamp >= moment().utc()) {
-                            // but if just a little bit in future, backdate to show "a few seconds ago"
-                            node.timestamp = moment().utc()-1;
-                        }
-                        node.timestampDisplay = moment(node.timestamp).fromNow();
+                return;
+            }
+            
+            $target.empty();
+            var masterTimestamp;
+            _.each(nodes, function (n) {
+                    if (n.nodeId == master && n.remoteTimestamp) {
+                        masterTimestamp = n.remoteTimestamp;
                     }
-                    
-                    node.isSelf = node.nodeId == self;
-                    node.isMaster = self == master;
-                    node.isTerminated = node.status == "TERMINATED";
-                    $target.append(nodeRowTemplate(node));
                 });
-            }
+            
+            _.each(nodes, function (n) {
+                var node = _.clone(n);
+                node.timestampDisplayPrefix = "";
+                node.timestampDisplaySuffix = "";
+                if (node['remoteTimestamp']) {
+                    node.timestamp = node.remoteTimestamp;
+                } else {
+                    node.timestamp = node.localTimestamp;
+                    node.timestampDisplaySuffix = " (local)";
+                }
+                if (node.timestamp >= moment().utc() + 10*1000) {
+                    // if server reports time significantly in future, report this, with no timestampe
+                    node.timestampDisplayPrefix = "server clock in future by "+
+                        moment.duration(moment(node.timestamp).diff(moment())).humanize();
+                    node.timestamp = "";
+                    node.timestampDisplay = "";
+                } else {
+                    // else use timestamp
+                    if (node.timestamp >= moment().utc()) {
+                        // but if just a little bit in future, backdate to show "a few seconds ago"
+                        node.timestamp = moment().utc()-1;
+                    }
+                    node.timestampDisplay = moment(node.timestamp).fromNow();
+                }
+                
+                node.isSelf = node.nodeId == self;
+                node.isMaster = self == master;
+                if (node.status == "TERMINATED") {
+                    node.isTerminated = true;
+                    node.isPretendMaster = false;
+                    node.isStale = false;
+                } else {
+                    node.isTerminated = false;
+                    node.isPretendMaster = (!node.isMaster && node.status == "MASTER" && master != node.nodeId);
+                    node.isStale = (masterTimestamp && node.timestamp + 30*1000 < masterTimestamp);
+                }
+                 
+                $target.append(nodeRowTemplate(node));
+            });
         },
         updateTimestamps: function() {
             this.$(".timestamp").each(function(index, t) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/usage/rest-server/src/main/java/brooklyn/rest/filter/LoggingFilter.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/LoggingFilter.java b/usage/rest-server/src/main/java/brooklyn/rest/filter/LoggingFilter.java
index 1576555..b57ff72 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/filter/LoggingFilter.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/LoggingFilter.java
@@ -67,7 +67,6 @@ public class LoggingFilter implements Filter {
         HttpServletRequest httpRequest = (HttpServletRequest) request;
         HttpServletResponse httpResponse = (HttpServletResponse) response;
 
-        String uri = httpRequest.getRequestURI();
         String rid = RequestTaggingFilter.getTag();
         boolean isInteresting = INTERESTING_METHODS.contains(httpRequest.getMethod()),
                 shouldLog = (isInteresting && LOG.isDebugEnabled()) || LOG.isTraceEnabled(),
@@ -75,8 +74,8 @@ public class LoggingFilter implements Filter {
         Stopwatch timer = Stopwatch.createUnstarted();
         try {
             if (shouldLog) {
-                String message = "{} starting request {} {}";
-                Object[] args = new Object[]{rid, httpRequest.getMethod(), uri};
+                String message = "Request {} starting: {} {} from {}";
+                Object[] args = new Object[]{rid, httpRequest.getMethod(), httpRequest.getRequestURI(), httpRequest.getRemoteAddr()};
                 if (isInteresting) {
                     LOG.debug(message, args);
                 } else {
@@ -89,7 +88,7 @@ public class LoggingFilter implements Filter {
 
         } catch (Throwable e) {
             requestErrored = true;
-            LOG.warn("REST API request " + rid + " failed: " + e, e);
+            LOG.warn("Request " + rid + " ("+httpRequest.getMethod()+" "+httpRequest.getRequestURI()+" from "+httpRequest.getRemoteAddr()+") failed: " + e, e);
             // Propagate for handling by other filter
             throw Exceptions.propagate(e);
         } finally {
@@ -111,10 +110,11 @@ public class LoggingFilter implements Filter {
 
     private String getRequestCompletedMessage(boolean includeHeaders, Duration elapsed,
             String id, HttpServletRequest httpRequest, HttpServletResponse httpResponse) {
-        StringBuilder message = new StringBuilder(id)
-                .append(" complete in roughly ")
+        StringBuilder message = new StringBuilder("Request ")
+                .append(id)
+                .append(" completed in ")
                 .append(elapsed)
-                .append(". Responding ")
+                .append(": response ")
                 .append(httpResponse.getStatus())
                 .append(" for ")
                 .append(httpRequest.getMethod())

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
index a9a2225..03d51fc 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
@@ -126,45 +126,62 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
 
         new Thread("shutdown") {
             public void run() {
-                if (stopAppsFirst) {
-                    CountdownTimer shutdownTimeoutTimer = null;
-                    if (!shutdownTimeout.equals(Duration.ZERO)) {
-                        shutdownTimeoutTimer = shutdownTimeout.countdownTimer();
-                    }
+                boolean terminateTried = false;
+                try {
+                    if (stopAppsFirst) {
+                        CountdownTimer shutdownTimeoutTimer = null;
+                        if (!shutdownTimeout.equals(Duration.ZERO)) {
+                            shutdownTimeoutTimer = shutdownTimeout.countdownTimer();
+                        }
 
-                    List<Task<?>> stoppers = new ArrayList<Task<?>>();
-                    for (Application app: mgmt().getApplications()) {
-                        if (app instanceof StartableApplication)
-                            stoppers.add(Entities.invokeEffector((EntityLocal)app, app, StartableApplication.STOP));
-                    }
+                        List<Task<?>> stoppers = new ArrayList<Task<?>>();
+                        for (Application app: mgmt().getApplications()) {
+                            if (app instanceof StartableApplication)
+                                stoppers.add(Entities.invokeEffector((EntityLocal)app, app, StartableApplication.STOP));
+                        }
 
-                    try {
                         for (Task<?> t: stoppers) {
                             if (!waitAppShutdown(shutdownTimeoutTimer, t)) {
                                 //app stop error
                                 hasAppErrorsOrTimeout.set(true);
                             }
                         }
-                    } catch (TimeoutException e) {
+                    }
+
+                    terminateTried = true;
+                    ((ManagementContextInternal)mgmt()).terminate(); 
+
+                } catch (Throwable e) {
+                    Throwable interesting = Exceptions.getFirstInteresting(e);
+                    if (interesting instanceof TimeoutException) {
                         //timeout while waiting for apps to stop
+                        log.warn("Timeout shutting down: "+Exceptions.collapseText(e));
+                        log.debug("Timeout shutting down: "+e, e);
                         hasAppErrorsOrTimeout.set(true);
+                        
+                    } else {
+                        // swallow fatal, so we notify the outer loop to continue with shutdown
+                        log.error("Unexpected error shutting down: "+Exceptions.collapseText(e), e);
+                        
                     }
-
-                    if (hasAppErrorsOrTimeout.get() && !forceShutdownOnError) {
-                        complete();
-                        //There are app errors, don't exit the process.
-                        return;
+                    hasAppErrorsOrTimeout.set(true);
+                    
+                    if (!terminateTried) {
+                        ((ManagementContextInternal)mgmt()).terminate(); 
                     }
+                } finally {
+
+                    complete();
+                
+                    if (!hasAppErrorsOrTimeout.get() || forceShutdownOnError) {
+                        //give the http request a chance to complete gracefully
+                        Time.sleep(delayForHttpReturn);
+                        System.exit(0);
+                    }
+                    
+                    // There are app errors, don't exit the process, allowing any exception to continue throwing
+                    log.warn("Abandoning shutdown because there were errors and shutdown was not forced.");
                 }
-
-                ((ManagementContextInternal)mgmt()).terminate(); 
-
-                complete();
-
-                //give the http request a chance to complete gracefully
-                Time.sleep(delayForHttpReturn);
-
-                System.exit(0);
             }
 
             private void complete() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/eef78912/utils/common/src/main/java/brooklyn/util/javalang/Threads.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/javalang/Threads.java b/utils/common/src/main/java/brooklyn/util/javalang/Threads.java
index 1760ae2..b3af17b 100644
--- a/utils/common/src/main/java/brooklyn/util/javalang/Threads.java
+++ b/utils/common/src/main/java/brooklyn/util/javalang/Threads.java
@@ -21,6 +21,8 @@ package brooklyn.util.javalang;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import brooklyn.util.exceptions.Exceptions;
+
 public class Threads {
 
     private static final Logger log = LoggerFactory.getLogger(Threads.class);
@@ -44,7 +46,15 @@ public class Threads {
             return Runtime.getRuntime().removeShutdownHook(hook);
         } catch (IllegalStateException e) {
             // probably shutdown in progress
-            log.debug("cannot remove shutdown hook "+hook+": "+e);
+            String text = Exceptions.collapseText(e);
+            if (text.contains("Shutdown in progress")) {
+                if (log.isTraceEnabled()) {
+                    log.trace("Could not remove shutdown hook "+hook+": "+text);
+                }
+            } else {
+                log.warn("Could not remove shutdown hook "+hook+": "+text);
+                log.debug("Shutdown hook removal details: "+e, e);
+            }
             return false;
         }
     }



[05/13] incubator-brooklyn git commit: more code review/cleanup for catalog CLI

Posted by he...@apache.org.
more code review/cleanup for catalog CLI

especially around the official/final/unofficial initialization interplay with HA modes,
so that after a node has been promoted to master, on subsequent promotions it doesn't re-apply the initializations


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/df21c711
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/df21c711
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/df21c711

Branch: refs/heads/master
Commit: df21c7116adb94986c4cb2c8e12ec6b86d580048
Parents: 9cc7293
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Mon May 25 11:24:55 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Mon May 25 11:44:09 2015 +0100

----------------------------------------------------------------------
 .../catalog/internal/CatalogInitialization.java | 127 ++++++++++++++-----
 .../internal/CatalogItemDtoAbstract.java        |   4 +-
 .../rebind/PeriodicDeltaChangeListener.java     |  38 +++---
 .../entity/rebind/RebindContextImpl.java        |   6 +-
 .../brooklyn/entity/rebind/RebindIteration.java |  51 +++++---
 .../entity/rebind/RebindManagerImpl.java        |   2 +-
 .../ha/HighAvailabilityManagerImpl.java         |  57 +++++----
 .../internal/AbstractManagementContext.java     |   4 +-
 usage/cli/src/main/java/brooklyn/cli/Main.java  |   4 +
 .../java/brooklyn/cli/lister/ClassFinder.java   |   2 +-
 .../brooklyn/launcher/BrooklynLauncher.java     |  13 +-
 .../rest/filter/HaHotCheckResourceFilter.java   |   2 +-
 .../rest/filter/HaMasterCheckFilter.java        |   2 +-
 .../util/javalang/AggregateClassLoader.java     |   2 +-
 14 files changed, 212 insertions(+), 102 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
index 6f0b80e..8f45a8c 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
@@ -29,6 +29,7 @@ import brooklyn.catalog.CatalogItem;
 import brooklyn.config.BrooklynServerConfig;
 import brooklyn.management.ManagementContext;
 import brooklyn.management.ManagementContextInjectable;
+import brooklyn.management.ha.ManagementNodeState;
 import brooklyn.management.internal.ManagementContextInternal;
 import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableList;
@@ -37,6 +38,7 @@ import brooklyn.util.exceptions.FatalRuntimeException;
 import brooklyn.util.exceptions.RuntimeInterruptedException;
 import brooklyn.util.flags.TypeCoercions;
 import brooklyn.util.guava.Maybe;
+import brooklyn.util.javalang.JavaClassNames;
 import brooklyn.util.os.Os;
 import brooklyn.util.text.Strings;
 
@@ -78,7 +80,15 @@ public class CatalogInitialization implements ManagementContextInjectable {
 
     private boolean disallowLocal = false;
     private List<Function<CatalogInitialization, Void>> callbacks = MutableList.of();
-    private boolean hasRunBestEffort = false, hasRunOfficial = false, isPopulating = false;
+    private boolean 
+        /** has run an unofficial initialization (i.e. an early load, triggered by an early read of the catalog) */
+        hasRunUnofficialInitialization = false, 
+        /** has run an official initialization, but it is not a permanent one (e.g. during a hot standby mode, or a run failed) */
+        hasRunTransientOfficialInitialization = false, 
+        /** has run an official initialization which is permanent (node is master, and the new catalog is now set) */
+        hasRunFinalInitialization = false;
+    /** is running a populate method; used to prevent recursive loops */
+    private boolean isPopulating = false;
     
     private ManagementContext managementContext;
     private boolean isStartingUp = false;
@@ -97,6 +107,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
         this(null, false, null, false);
     }
 
+    @Override
     public void injectManagementContext(ManagementContext managementContext) {
         Preconditions.checkNotNull(managementContext, "management context");
         if (this.managementContext!=null && managementContext!=this.managementContext)
@@ -127,56 +138,108 @@ public class CatalogInitialization implements ManagementContextInjectable {
         return reset;
     }
 
-    public boolean hasRunOfficial() { return hasRunOfficial; }
-    public boolean hasRunIncludingBestEffort() { return hasRunOfficial || hasRunBestEffort; }
-
-    /** makes or updates the mgmt catalog, based on the settings in this class */
-    public void populateCatalog(boolean needsInitial, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
+    /** Returns true if the canonical initialization has completed, 
+     * that is, an initialization which is done when a node is rebinded as master
+     * (or an initialization done by the startup routines when not running persistence);
+     * see also {@link #hasRunAnyInitialization()}. */
+    public boolean hasRunFinalInitialization() { return hasRunFinalInitialization; }
+    /** Returns true if an official initialization has run,
+     * even if it was a transient run, e.g. so that the launch sequence can tell whether rebind has triggered initialization */
+    public boolean hasRunOfficialInitialization() { return hasRunFinalInitialization || hasRunTransientOfficialInitialization; }
+    /** Returns true if the initializer has run at all,
+     * including transient initializations which might be needed before a canonical becoming-master rebind,
+     * for instance because the catalog is being accessed before loading rebind information
+     * (done by {@link #populateUnofficial(BasicBrooklynCatalog)}) */
+    public boolean hasRunAnyInitialization() { return hasRunFinalInitialization || hasRunTransientOfficialInitialization || hasRunUnofficialInitialization; }
+
+    /** makes or updates the mgmt catalog, based on the settings in this class 
+     * @param nodeState the management node for which this is being read; if master, then we expect this run to be the last one,
+     *   and so subsequent applications should ignore any initialization data (e.g. on a subsequent promotion to master, 
+     *   after a master -> standby -> master cycle)
+     * @param needsInitialItemsLoaded whether the catalog needs the initial items loaded
+     * @param needsInitialItemsLoaded whether the catalog needs the additiona items loaded
+     * @param optionalExcplicitItemsForResettingCatalog
+     *   if supplied, the catalog is reset to contain only these items, before calling any other initialization
+     *   for use primarily when rebinding
+     */
+    public void populateCatalog(ManagementNodeState nodeState, boolean needsInitialItemsLoaded, boolean needsAdditionsLoaded, Collection<CatalogItem<?, ?>> optionalExcplicitItemsForResettingCatalog) {
+        if (log.isDebugEnabled()) {
+            String message = "Populating catalog for "+nodeState+", needsInitial="+needsInitialItemsLoaded+", needsAdditional="+needsAdditionsLoaded+", explicitItems="+(optionalExcplicitItemsForResettingCatalog==null ? "null" : optionalExcplicitItemsForResettingCatalog.size())+"; from "+JavaClassNames.callerNiceClassAndMethod(1);
+            if (!ManagementNodeState.isHotProxy(nodeState)) {
+                log.debug(message);
+            } else {
+                // in hot modes, make this message trace so we don't get too much output then
+                log.trace(message);
+            }
+        }
         synchronized (populatingCatalogMutex) {
             try {
+                if (hasRunFinalInitialization() && (needsInitialItemsLoaded || needsAdditionsLoaded)) {
+                    // if we have already run "final" then we should only ever be used to reset the catalog, 
+                    // not to initialize or add; e.g. we are being given a fixed list on a subsequent master rebind after the initial master rebind 
+                    log.warn("Catalog initialization called to populate initial, even though it has already run the final official initialization");
+                }
                 isPopulating = true;
                 BasicBrooklynCatalog catalog = (BasicBrooklynCatalog) managementContext.getCatalog();
                 if (!catalog.getCatalog().isLoaded()) {
                     catalog.load();
                 } else {
-                    if (needsInitial && (hasRunOfficial || hasRunBestEffort)) {
+                    if (needsInitialItemsLoaded && hasRunAnyInitialization()) {
                         // an indication that something caused it to load early; not severe, but unusual
-                        log.warn("Catalog initialization has not properly run but management context has a catalog; re-populating, possibly overwriting items installed during earlier access (it may have been an early web request)");
+                        if (hasRunTransientOfficialInitialization) {
+                            log.debug("Catalog initialization now populating, but has noted a previous official run which was not final (probalby loaded while in a standby mode, or a previous run failed); overwriting any items installed earlier");
+                        } else {
+                            log.warn("Catalog initialization now populating, but has noted a previous unofficial run (it may have been an early web request); overwriting any items installed earlier");
+                        }
                         catalog.reset(ImmutableList.<CatalogItem<?,?>>of());
                     }
                 }
-                hasRunOfficial = true;
 
-                populateCatalogImpl(catalog, needsInitial, optionalItemsForResettingCatalog);
+                populateCatalogImpl(catalog, needsInitialItemsLoaded, needsAdditionsLoaded, optionalExcplicitItemsForResettingCatalog);
+                if (nodeState == ManagementNodeState.MASTER) {
+                    // TODO ideally this would remain false until it has *persisted* the changed catalog;
+                    // if there is a subsequent startup failure the forced additions will not be persisted,
+                    // but nor will they be loaded on a subsequent run.
+                    // callers will have to restart a brooklyn, or reach into this class to change this field,
+                    // or (recommended) manually adjust the catalog.
+                    // TODO also, if a node comes up in standby, the addition might not take effector for a while
+                    //
+                    // however since these options are mainly for use on the very first brooklyn run, it's not such a big deal; 
+                    // once up and running the typical way to add items is via the REST API
+                    hasRunFinalInitialization = true;
+                }
             } finally {
-                hasRunOfficial = true;
+                if (!hasRunFinalInitialization) {
+                    hasRunTransientOfficialInitialization = true;
+                }
                 isPopulating = false;
             }
         }
     }
 
-    private void populateCatalogImpl(BasicBrooklynCatalog catalog, boolean needsInitial, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
+    private void populateCatalogImpl(BasicBrooklynCatalog catalog, boolean needsInitialItemsLoaded, boolean needsAdditionsLoaded, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
         applyCatalogLoadMode();
         
         if (optionalItemsForResettingCatalog!=null) {
             catalog.reset(optionalItemsForResettingCatalog);
         }
         
-        if (needsInitial) {
+        if (needsInitialItemsLoaded) {
             populateInitial(catalog);
         }
-        
-        populateAdditions(catalog);
 
-        populateViaCallbacks(catalog);
+        if (needsAdditionsLoaded) {
+            populateAdditions(catalog);
+            populateViaCallbacks(catalog);
+        }
     }
 
     private enum PopulateMode { YAML, XML, AUTODETECT }
     
     protected void populateInitial(BasicBrooklynCatalog catalog) {
         if (disallowLocal) {
-            if (!hasRunOfficial()) {
-                log.debug("CLI initial catalog not being read with disallow-local mode set.");
+            if (!hasRunFinalInitialization()) {
+                log.debug("CLI initial catalog not being read when local catalog load mode is disallowed.");
             }
             return;
         }
@@ -201,13 +264,13 @@ public class CatalogInitialization implements ManagementContextInjectable {
         
         catalogUrl = Os.mergePaths(BrooklynServerConfig.getMgmtBaseDir( managementContext.getConfig() ), "catalog.bom");
         if (new File(catalogUrl).exists()) {
-            populateInitialFromUri(catalog, "file:"+catalogUrl, PopulateMode.YAML);
+            populateInitialFromUri(catalog, new File(catalogUrl).toURI().toString(), PopulateMode.YAML);
             return;
         }
         
         catalogUrl = Os.mergePaths(BrooklynServerConfig.getMgmtBaseDir( managementContext.getConfig() ), "catalog.xml");
         if (new File(catalogUrl).exists()) {
-            populateInitialFromUri(catalog, "file:"+catalogUrl, PopulateMode.XML);
+            populateInitialFromUri(catalog, new File(catalogUrl).toURI().toString(), PopulateMode.XML);
             return;
         }
 
@@ -287,7 +350,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
         if (Strings.isNonBlank(additionsUri)) {
             if (disallowLocal) {
                 if (!hasRunAdditions) {
-                    log.warn("CLI additions supplied but not supported in disallow-local mode; ignoring.");
+                    log.warn("CLI additions supplied but not supported when catalog load mode disallows local loads; ignoring.");
                 }
                 return;
             }   
@@ -340,21 +403,27 @@ public class CatalogInitialization implements ManagementContextInjectable {
         }
     }
 
-    /** makes the catalog, warning if persistence is on and hasn't run yet 
-     * (as the catalog will be subsequently replaced) */
-    public void populateBestEffort(BasicBrooklynCatalog catalog) {
+    /** Creates the catalog based on parameters set here, if not yet loaded,
+     * but ignoring persisted state and warning if persistence is on and we are starting up
+     * (because the official persistence is preferred and the catalog will be subsequently replaced);
+     * for use when the catalog is accessed before persistence is completed. 
+     * <p>
+     * This method is primarily used during testing, which in many cases does not enforce the full startup order
+     * and which wants a local catalog in any case. It may also be invoked if a client requests the catalog
+     * while the server is starting up. */
+    public void populateUnofficial(BasicBrooklynCatalog catalog) {
         synchronized (populatingCatalogMutex) {
-            if (hasRunOfficial || hasRunBestEffort || isPopulating) return;
-            // if a thread calls back in to this, ie calling to it from a getCatalog() call while populating,
-            // it will own the mutex and observe isRunningBestEffort, returning quickly 
+            // check isPopulating in case this method gets called from inside another populate call
+            if (hasRunAnyInitialization() || isPopulating) return;
+            log.debug("Populating catalog unofficially ("+catalog+")");
             isPopulating = true;
             try {
                 if (isStartingUp) {
                     log.warn("Catalog access requested when not yet initialized; populating best effort rather than through recommended pathway. Catalog data may be replaced subsequently.");
                 }
-                populateCatalogImpl(catalog, true, null);
+                populateCatalogImpl(catalog, true, true, null);
             } finally {
-                hasRunBestEffort = true;
+                hasRunUnofficialInitialization = true;
                 isPopulating = false;
             }
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
index c6ff97e..131c26e 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogItemDtoAbstract.java
@@ -184,7 +184,9 @@ public abstract class CatalogItemDtoAbstract<T, SpecT> extends AbstractBrooklynO
         if (!Objects.equal(displayName, other.displayName)) return false;
         if (!Objects.equal(iconUrl, other.iconUrl)) return false;
         if (!Objects.equal(tags, other.tags)) return false;
-        // 'type' not checked, because deprecated, we might want to allow removal in future
+        // 'type' not checked, because deprecated, 
+        // and in future we might want to allow it to be removed/blanked in some impls without affecting equality
+        // (in most cases it is the same as symbolicName so doesn't matter)
         return true;
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
index f3861bc..4203b4b 100644
--- a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
+++ b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
@@ -166,9 +166,8 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
         
     private DeltaCollector deltaCollector = new DeltaCollector();
 
-    private volatile boolean running = false;
-
-    private volatile boolean stopping = false, stopCompleted = false;
+    private enum ListenerState { INIT, RUNNING, STOPPING, STOPPED } 
+    private volatile ListenerState state = ListenerState.INIT;
 
     private volatile ScheduledTask scheduledTask;
 
@@ -197,18 +196,17 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     @SuppressWarnings("unchecked")
     public void start() {
         synchronized (startStopMutex) {
-            if (running || (scheduledTask!=null && !scheduledTask.isDone())) {
+            if (state==ListenerState.RUNNING || (scheduledTask!=null && !scheduledTask.isDone())) {
                 LOG.warn("Request to start "+this+" when already running - "+scheduledTask+"; ignoring");
                 return;
             }
-            stopCompleted = false;
-            running = true;
+            state = ListenerState.RUNNING;
 
             Callable<Task<?>> taskFactory = new Callable<Task<?>>() {
                 @Override public Task<Void> call() {
                     return Tasks.<Void>builder().dynamic(false).name("periodic-persister").body(new Callable<Void>() {
                         public Void call() {
-                            persistNowSafely(false);
+                            persistNowSafely();
                             return null;
                         }}).build();
                 }
@@ -224,9 +222,8 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     }
     void stop(Duration timeout, Duration graceTimeoutForSubsequentOperations) {
         synchronized (startStopMutex) {
-            running = false;
+            state = ListenerState.STOPPING;
             try {
-                stopping = true;
 
                 if (scheduledTask != null) {
                     CountdownTimer expiry = timeout.countdownTimer();
@@ -250,8 +247,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
                     deltaCollector = new DeltaCollector();
                 }
             } finally {
-                stopCompleted = true;
-                stopping = false;
+                state = ListenerState.STOPPED;
             }
         }
     }
@@ -259,7 +255,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     /** Waits for any in-progress writes to be completed then for or any unwritten data to be written. */
     @VisibleForTesting
     public void waitForPendingComplete(Duration timeout, boolean canTrigger) throws InterruptedException, TimeoutException {
-        if (!isActive() && !stopping) return;
+        if (!isActive() && state != ListenerState.STOPPING) return;
         
         CountdownTimer timer = timeout.isPositive() ? CountdownTimer.newInstanceStarted(timeout) : CountdownTimer.newInstancePaused(Duration.PRACTICALLY_FOREVER);
         Integer targetWriteCount = null;
@@ -299,14 +295,16 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
      * Even when not active, changes will still be tracked unless {@link #isStopped()}.
      */
     private boolean isActive() {
-        return running && persister != null && !isStopped();
+        return state == ListenerState.RUNNING && persister != null && !isStopped();
     }
 
     /**
-     * Whether we have been stopped, in which case will not persist or store anything.
+     * Whether we have been stopped, ie are stopping are or fully stopped,
+     * in which case will not persist or store anything
+     * (except for a final internal persistence called while STOPPING.) 
      */
     private boolean isStopped() {
-        return stopping || stopCompleted || executionContext.isShutdown();
+        return state == ListenerState.STOPPING || state == ListenerState.STOPPED || executionContext.isShutdown();
     }
     
     private void addReferencedObjects(DeltaCollector deltaCollector) {
@@ -336,7 +334,11 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     }
     
     @VisibleForTesting
-    public boolean persistNowSafely(boolean alreadyHasMutex) {
+    public boolean persistNowSafely() {
+        return persistNowSafely(false);
+    }
+    
+    private boolean persistNowSafely(boolean alreadyHasMutex) {
         Stopwatch timer = Stopwatch.createStarted();
         try {
             persistNowInternal(alreadyHasMutex);
@@ -364,12 +366,12 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     }
     
     protected void persistNowInternal(boolean alreadyHasMutex) {
-        if (!isActive() && !stopping) {
+        if (!isActive() && state != ListenerState.STOPPING) {
             return;
         }
         try {
             if (!alreadyHasMutex) persistingMutex.acquire();
-            if (!isActive() && !stopping) return;
+            if (!isActive() && state != ListenerState.STOPPING) return;
             
             // Atomically switch the delta, so subsequent modifications will be done in the
             // next scheduled persist

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java
index b7eba1a..b39fea4 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindContextImpl.java
@@ -100,6 +100,10 @@ public class RebindContextImpl implements RebindContext {
         catalogItems.remove(item.getId());
     }
 
+    public void clearCatalogItems() {
+        catalogItems.clear();
+    }
+    
     public Entity getEntity(String id) {
         return entities.get(id);
     }
@@ -180,5 +184,5 @@ public class RebindContextImpl implements RebindContext {
     public LookupContext lookup() {
         return lookupContext;
     }
-    
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
index 2d1e47f..ad4915a 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
@@ -308,7 +308,7 @@ public abstract class RebindIteration {
         if (rebindManager.persistCatalogItemsEnabled) {
             logRebindingDebug("RebindManager instantiating catalog items: {}", mementoManifest.getCatalogItemIds());
             for (CatalogItemMemento catalogItemMemento : mementoManifest.getCatalogItemMementos().values()) {
-                if (LOG.isDebugEnabled()) LOG.debug("RebindManager instantiating catalog item {}", catalogItemMemento);
+                logRebindingDebug("RebindManager instantiating catalog item {}", catalogItemMemento);
                 try {
                     CatalogItem<?, ?> catalogItem = instantiator.newCatalogItem(catalogItemMemento);
                     rebindContext.registerCatalogItem(catalogItemMemento.getId(), catalogItem);
@@ -347,9 +347,9 @@ public abstract class RebindIteration {
         CatalogInitialization catInit = ((ManagementContextInternal)managementContext).getCatalogInitialization();
         catInit.applyCatalogLoadMode();
         Collection<CatalogItem<?,?>> itemsForResettingCatalog = null;
-        boolean needsInitialCatalog;
+        boolean needsInitialItemsLoaded, needsAdditionalItemsLoaded;
         if (rebindManager.persistCatalogItemsEnabled) {
-            if (!catInit.hasRunOfficial() && catInit.isInitialResetRequested()) {
+            if (!catInit.hasRunFinalInitialization() && catInit.isInitialResetRequested()) {
                 String message = "RebindManager resetting catalog on first run (catalog persistence enabled, but reset explicitly specified). ";
                 if (catalogItems.isEmpty()) {
                     message += "Catalog was empty anyway.";
@@ -361,6 +361,15 @@ public abstract class RebindIteration {
                 }
                 logRebindingDebug(message);
 
+                // we will have unnecessarily tried to load the catalog item manifests earlier in this iteration,
+                // and problems there could fail a rebind even when we are resetting;
+                // it might be cleaner to check earlier whether a reset is happening and not load those items at all,
+                // but that would be a significant new code path (to remove a directory in the persistent store, essentially),
+                // and as it stands we don't do much with those manifests (e.g. we won't register them or fail on missing types)
+                // so we think it's only really corrupted XML or CatalogItem schema changes which would cause such problems.
+                // in extremis someone might need to wipe their store but for most purposes i don't think there will be any issue
+                // with loading the catalog item manifests before wiping all those files.
+                
                 itemsForResettingCatalog = MutableList.<CatalogItem<?,?>>of();
                 
                 PersisterDeltaImpl delta = new PersisterDeltaImpl();
@@ -368,36 +377,42 @@ public abstract class RebindIteration {
                 getPersister().queueDelta(delta);
                 
                 mementoRawData.clearCatalogItems();
-                needsInitialCatalog = true;
+                rebindContext.clearCatalogItems();
+                needsInitialItemsLoaded = true;
+                needsAdditionalItemsLoaded = true;
             } else {
                 if (!isEmpty) {
                     logRebindingDebug("RebindManager clearing local catalog and loading from persisted state");
                     itemsForResettingCatalog = rebindContext.getCatalogItems();
-                    needsInitialCatalog = false;
+                    needsInitialItemsLoaded = false;
+                    // only apply "add" if we haven't yet done so while in MASTER mode
+                    needsAdditionalItemsLoaded = !catInit.hasRunFinalInitialization();
                 } else {
-                    if (catInit.hasRunOfficial()) {
-                        logRebindingDebug("RebindManager will re-add any new items (persisted state empty)");
-                        needsInitialCatalog = false;
+                    if (catInit.hasRunFinalInitialization()) {
+                        logRebindingDebug("RebindManager has already done the final official run, not doing anything (even though persisted state empty)");
+                        needsInitialItemsLoaded = false;
+                        needsAdditionalItemsLoaded = false;
                     } else {
-                        logRebindingDebug("RebindManager loading initial catalog locally because persisted state empty");
-                        needsInitialCatalog = true;
+                        logRebindingDebug("RebindManager loading initial catalog locally because persisted state empty and the final official run has not yet been performed");
+                        needsInitialItemsLoaded = true;
+                        needsAdditionalItemsLoaded = true;
                     }
                 }
             }
         } else {
-            if (catInit.hasRunOfficial()) {
+            if (catInit.hasRunFinalInitialization()) {
                 logRebindingDebug("RebindManager skipping catalog init because it has already run (catalog persistence disabled)");
-                needsInitialCatalog = false;
+                needsInitialItemsLoaded = false;
+                needsAdditionalItemsLoaded = false;
             } else {
-                logRebindingDebug("RebindManager will initialize catalog locally because catalog persistence is disabled");
-                needsInitialCatalog = true;
+                logRebindingDebug("RebindManager will initialize catalog locally because catalog persistence is disabled and the final official run has not yet been performed");
+                needsInitialItemsLoaded = true;
+                needsAdditionalItemsLoaded = true;
             }
         }
 
-        // TODO in read-only mode, perhaps do this less frequently than entities etc ?
-        // both in RW and in RO mode, the first run reads the initialization data;
-        // maybe not desired for RO as it defers problems, although if it's standalone it is desired
-        catInit.populateCatalog(needsInitialCatalog, itemsForResettingCatalog);
+        // TODO in read-only mode, perhaps do this less frequently than entities etc, maybe only if things change?
+        catInit.populateCatalog(mode, needsInitialItemsLoaded, needsAdditionalItemsLoaded, itemsForResettingCatalog);
     }
 
     protected void instantiateLocationsAndEntities() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
index ded0049..c94e8fa 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -441,7 +441,7 @@ public class RebindManagerImpl implements RebindManager {
             }
             persistenceStoreAccess.checkpoint(memento, exceptionHandler);
         } else {
-            if (!persistenceRealChangeListener.persistNowSafely(false)) {
+            if (!persistenceRealChangeListener.persistNowSafely()) {
                 throw new IllegalStateException("Forced persistence failed; see logs fore more detail");
             }
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
index ab033fd..96e6bea 100644
--- a/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
+++ b/core/src/main/java/brooklyn/management/ha/HighAvailabilityManagerImpl.java
@@ -408,33 +408,40 @@ public class HighAvailabilityManagerImpl implements HighAvailabilityManager {
             throw new IllegalStateException("Unexpected high availability mode "+startMode+" requested for "+this);
         }
         
-        if ((startMode==HighAvailabilityMode.HOT_STANDBY || startMode==HighAvailabilityMode.HOT_BACKUP) && !ManagementNodeState.isHotProxy(oldState)) {
-            // now transition to hot proxy
-            nodeStateTransitionComplete = false;
-            if (startMode==HighAvailabilityMode.HOT_STANDBY) {
-                // if it should be hot standby, then we may need to promote
-                // inform the world that we are transitioning (but not eligible for promotion while going in to hot standby)
-                // (no harm in doing this twice)
-                publishHealth();
-            }
-            try {
-                activateHotProxy(ManagementNodeState.of(startMode).get()).get();
-                // error above now throws
-                nodeStateTransitionComplete = true;
-                publishHealth();
-                
-                if (getNodeState()==ManagementNodeState.HOT_STANDBY || getNodeState()==ManagementNodeState.HOT_BACKUP) {
-                    LOG.info("Management node "+ownNodeId+" now running as HA "+getNodeState()+"; "
-                        + managementContext.getApplications().size()+" application"+Strings.s(managementContext.getApplications().size())+" loaded");
-                } else {
-                    // shouldn't come here, we should have gotten an error above
-                    LOG.warn("Management node "+ownNodeId+" unable to promote to "+startMode+" (currently "+getNodeState()+"); "
-                        + "(see log for further details)");
+        if ((startMode==HighAvailabilityMode.HOT_STANDBY || startMode==HighAvailabilityMode.HOT_BACKUP)) {
+            if (!ManagementNodeState.isHotProxy(oldState)) {
+                // now transition to hot proxy
+                nodeStateTransitionComplete = false;
+                if (startMode==HighAvailabilityMode.HOT_STANDBY) {
+                    // if it should be hot standby, then we may need to promote
+                    // inform the world that we are transitioning (but not eligible for promotion while going in to hot standby)
+                    // (no harm in doing this twice)
+                    publishHealth();
                 }
-            } catch (Exception e) {
-                LOG.warn("Management node "+ownNodeId+" unable to promote to "+startMode+" (currently "+getNodeState()+"); rethrowing: "+Exceptions.collapseText(e));
+                try {
+                    activateHotProxy(ManagementNodeState.of(startMode).get()).get();
+                    // error above now throws
+                    nodeStateTransitionComplete = true;
+                    publishHealth();
+
+                    if (getNodeState()==ManagementNodeState.HOT_STANDBY || getNodeState()==ManagementNodeState.HOT_BACKUP) {
+                        LOG.info("Management node "+ownNodeId+" now running as HA "+getNodeState()+"; "
+                            + managementContext.getApplications().size()+" application"+Strings.s(managementContext.getApplications().size())+" loaded");
+                    } else {
+                        // shouldn't come here, we should have gotten an error above
+                        LOG.warn("Management node "+ownNodeId+" unable to promote to "+startMode+" (currently "+getNodeState()+"); "
+                            + "(see log for further details)");
+                    }
+                } catch (Exception e) {
+                    LOG.warn("Management node "+ownNodeId+" unable to promote to "+startMode+" (currently "+getNodeState()+"); rethrowing: "+Exceptions.collapseText(e));
+                    nodeStateTransitionComplete = true;
+                    throw Exceptions.propagate(e);
+                }
+            } else {
+                // transitioning among hot proxy states - tell the rebind manager
+                managementContext.getRebindManager().stopReadOnly();
+                managementContext.getRebindManager().startReadOnly(ManagementNodeState.of(startMode).get());
                 nodeStateTransitionComplete = true;
-                throw Exceptions.propagate(e);
             }
         } else {
             nodeStateTransitionComplete = true;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java b/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
index b1fdc0c..81dd26b 100644
--- a/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
@@ -363,11 +363,11 @@ public abstract class AbstractManagementContext implements ManagementContextInte
 
     @Override
     public BrooklynCatalog getCatalog() {
-        if (!getCatalogInitialization().hasRunIncludingBestEffort()) {
+        if (!getCatalogInitialization().hasRunAnyInitialization()) {
             // catalog init is needed; normally this will be done from start sequence,
             // but if accessed early -- and in tests -- we will load it here
             getCatalogInitialization().injectManagementContext(this);
-            getCatalogInitialization().populateBestEffort(catalog);
+            getCatalogInitialization().populateUnofficial(catalog);
         }
         return catalog;
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/usage/cli/src/main/java/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java
index d98e0d8..6e48b3f 100644
--- a/usage/cli/src/main/java/brooklyn/cli/Main.java
+++ b/usage/cli/src/main/java/brooklyn/cli/Main.java
@@ -785,6 +785,10 @@ public class Main extends AbstractMain {
                     .add("startupFailsOnCatalogErrors", startupFailOnCatalogErrors)
                     .add("startupContinueOnWebErrors", startupContinueOnWebErrors)
                     .add("startupFailOnManagedAppsErrors", startupFailOnManagedAppsErrors)
+                    .add("catalogInitial", catalogInitial)
+                    .add("catalogAdd", catalogAdd)
+                    .add("catalogReset", catalogReset)
+                    .add("catalogForce", catalogForce)
                     .add("stopWhichAppsOnShutdown", stopWhichAppsOnShutdown)
                     .add("noShutdownOnExit", noShutdownOnExit)
                     .add("stopOnKeyPress", stopOnKeyPress)

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/usage/cli/src/main/java/brooklyn/cli/lister/ClassFinder.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/lister/ClassFinder.java b/usage/cli/src/main/java/brooklyn/cli/lister/ClassFinder.java
index a66fa7d..2fb913b 100644
--- a/usage/cli/src/main/java/brooklyn/cli/lister/ClassFinder.java
+++ b/usage/cli/src/main/java/brooklyn/cli/lister/ClassFinder.java
@@ -130,7 +130,7 @@ public class ClassFinder {
                     }
                 }
             } else {
-                result.add(new URL("file://"+tidiedFile));
+                result.add(tidiedFile.toURI().toURL());
             }
         }
         

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index 6a201a2..e7d3948 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -598,9 +598,16 @@ public class BrooklynLauncher {
         try {
             // run cat init now if it hasn't yet been run; 
             // will also run if there was an ignored error in catalog above, allowing it to fail startup here if requested
-            if (catInit!=null && !catInit.hasRunOfficial()) {
-                LOG.debug("Loading catalog as part of launcher (persistence did not run it)");
-                catInit.populateCatalog(true, null);
+            if (catInit!=null && !catInit.hasRunOfficialInitialization()) {
+                if (persistMode==PersistMode.DISABLED) {
+                    LOG.debug("Loading catalog as part of launch sequence (it was not loaded as part of any rebind sequence)");
+                    catInit.populateCatalog(ManagementNodeState.MASTER, true, true, null);
+                } else {
+                    // should have loaded during rebind
+                    ManagementNodeState state = managementContext.getHighAvailabilityManager().getNodeState();
+                    LOG.warn("Loading catalog for "+state+" as part of launch sequence (it was not loaded as part of the rebind sequence)");
+                    catInit.populateCatalog(state, true, true, null);
+                }
             }
         } catch (Exception e) {
             handleSubsystemStartupError(ignoreCatalogErrors, "initial catalog", e);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
index dc2e954..bae561a 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
@@ -112,7 +112,7 @@ public class HaHotCheckResourceFilter implements ResourceFilterFactory {
         public ContainerRequest filter(ContainerRequest request) {
             String problem = lookForProblem(request);
             if (Strings.isNonBlank(problem)) {
-                log.warn("Disallowing request as "+problem+": "+request+"/"+am+" (caller should set '"+HaMasterCheckFilter.SKIP_CHECK_HEADER+"' to force)");
+                log.warn("Disallowing web request as "+problem+": "+request+"/"+am+" (caller should set '"+HaMasterCheckFilter.SKIP_CHECK_HEADER+"' to force)");
                 throw new WebApplicationException(ApiError.builder()
                     .message("This request is only permitted against an active hot Brooklyn server")
                     .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
index 9cca507..74e7cf1 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java
@@ -94,7 +94,7 @@ public class HaMasterCheckFilter implements Filter {
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
         String problem = lookForProblem(request);
         if (problem!=null) {
-            log.warn("Disallowing request as "+problem+": "+request.getParameterMap()+" (caller should set '"+SKIP_CHECK_HEADER+"' to force)");
+            log.warn("Disallowing web request as "+problem+": "+request.getParameterMap()+" (caller should set '"+SKIP_CHECK_HEADER+"' to force)");
             WebResourceUtils.applyJsonResponse(servletContext, ApiError.builder()
                 .message("This request is only permitted against an active master Brooklyn server")
                 .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse(), (HttpServletResponse)response);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/df21c711/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java b/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
index a3ca374..db0254b 100644
--- a/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
+++ b/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
@@ -115,7 +115,7 @@ public class AggregateClassLoader extends ClassLoader {
 
     public Iterator<ClassLoader> iterator() {
         synchronized (classLoaders) {
-            // provides iterator of snapshot
+            // CopyOnWriteList iterator is immutable view of snapshot
             return classLoaders.iterator();
         }
     }



[06/13] incubator-brooklyn git commit: fix scanning for osgi, and add test for same

Posted by he...@apache.org.
fix scanning for osgi, and add test for same


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/1207d991
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/1207d991
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/1207d991

Branch: refs/heads/master
Commit: 1207d9917c0f8e0b6186d24b6b76688f935f9e14
Parents: df21c71
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Tue May 26 14:44:51 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Tue May 26 14:45:10 2015 +0100

----------------------------------------------------------------------
 .../catalog/internal/BasicBrooklynCatalog.java  |  48 +++++++++++++----
 .../catalog/internal/CatalogClasspathDo.java    |  39 ++++++++++++--
 .../brooklyn/osgi/tests/more/MoreEntity.java    |   2 +
 .../brooklyn-test-osgi-more-entities_0.2.0.jar  | Bin 12590 -> 13078 bytes
 .../BrooklynComponentTemplateResolver.java      |  12 ++---
 .../camp/brooklyn/AbstractYamlTest.java         |  10 ++--
 .../camp/brooklyn/ReferencedYamlTest.java       |   6 +--
 .../CatalogOsgiVersionMoreEntityTest.java       |  52 +++++++++++++++----
 .../brooklyn/catalog/CatalogYamlCombiTest.java  |   6 +--
 .../brooklyn/catalog/CatalogYamlEntityTest.java |  44 ++++++++--------
 .../catalog/CatalogYamlLocationTest.java        |   4 +-
 .../brooklyn/catalog/CatalogYamlPolicyTest.java |   6 +--
 .../catalog/CatalogYamlTemplateTest.java        |   2 +-
 .../catalog/CatalogYamlVersioningTest.java      |   4 +-
 .../more-entities-osgi-catalog-scan.yaml        |  32 ++++++++++++
 .../src/main/java/brooklyn/util/os/Os.java      |   6 +--
 16 files changed, 199 insertions(+), 74 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
index 8c88974..bba0b73 100644
--- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
@@ -537,7 +537,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
     
     @SuppressWarnings("unchecked")
-    private <T> Maybe<T> getFirstAs(Map<?,?> map, Class<T> type, String firstKey, String ...otherKeys) {
+    private static <T> Maybe<T> getFirstAs(Map<?,?> map, Class<T> type, String firstKey, String ...otherKeys) {
         if (map==null) return Maybe.absent("No map available");
         String foundKey = null;
         Object value = null;
@@ -621,9 +621,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         } else {
             // scan for annotations: if libraries here, scan them; if inherited libraries error; else scan classpath
             if (!libraryBundlesNew.isEmpty()) {
-                result.addAll(scanAnnotationsFromBundles(mgmt, libraryBundlesNew));
+                result.addAll(scanAnnotationsFromBundles(mgmt, libraryBundlesNew, catalogMetadata));
             } else if (libraryBundles.isEmpty()) {
-                result.addAll(scanAnnotationsFromLocal(mgmt));
+                result.addAll(scanAnnotationsFromLocal(mgmt, catalogMetadata));
             } else {
                 throw new IllegalStateException("Cannot scan catalog node no local bundles, and with inherited bundles we will not scan the classpath");
             }
@@ -780,13 +780,13 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         return oldValue;
     }
 
-    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromLocal(ManagementContext mgmt) {
+    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromLocal(ManagementContext mgmt, Map<Object, Object> catalogMetadata) {
         CatalogDto dto = CatalogDto.newNamedInstance("Local Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-local-classpath");
-        return scanAnnotationsInternal(mgmt, new CatalogDo(dto));
+        return scanAnnotationsInternal(mgmt, new CatalogDo(dto), catalogMetadata);
     }
     
-    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromBundles(ManagementContext mgmt, Collection<CatalogBundle> libraries) {
-        CatalogDto dto = CatalogDto.newNamedInstance("Bundles Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-bundles-classpath-"+libraries.hashCode());
+    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromBundles(ManagementContext mgmt, Collection<CatalogBundle> libraries, Map<Object, Object> catalogMetadata) {
+        CatalogDto dto = CatalogDto.newNamedInstance("Bundles Scanned Catalog", "All annotated Brooklyn entities detected in bundles", "scanning-bundles-classpath-"+libraries.hashCode());
         List<String> urls = MutableList.of();
         for (CatalogBundle b: libraries) {
             // TODO currently does not support pre-installed bundles identified by name:version 
@@ -803,10 +803,10 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         
         CatalogDo subCatalog = new CatalogDo(dto);
         subCatalog.addToClasspath(urls.toArray(new String[0]));
-        return scanAnnotationsInternal(mgmt, subCatalog);
+        return scanAnnotationsInternal(mgmt, subCatalog, catalogMetadata);
     }
     
-    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsInternal(ManagementContext mgmt, CatalogDo subCatalog) {
+    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsInternal(ManagementContext mgmt, CatalogDo subCatalog, Map<Object, Object> catalogMetadata) {
         // TODO this does java-scanning only;
         // the call when scanning bundles should use the CatalogItem instead and use OSGi when loading for scanning
         // (or another scanning mechanism).  see comments on CatalogClasspathDo.load
@@ -818,7 +818,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         @SuppressWarnings({ "unchecked", "rawtypes" })
         Collection<CatalogItemDtoAbstract<?, ?>> result = (Collection<CatalogItemDtoAbstract<?, ?>>)(Collection)Collections2.transform(
                 (Collection<CatalogItemDo<Object,Object>>)(Collection)subCatalog.getIdCache().values(), 
-                itemDoToDto());
+                itemDoToDtoAddingSelectedMetadataDuringScan(catalogMetadata));
         return result;
     }
 
@@ -1177,6 +1177,34 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
             }
         };
     }
+    
+    private static <T,SpecT> Function<CatalogItemDo<T, SpecT>, CatalogItem<T,SpecT>> itemDoToDtoAddingSelectedMetadataDuringScan(final Map<Object, Object> catalogMetadata) {
+        return new Function<CatalogItemDo<T,SpecT>, CatalogItem<T,SpecT>>() {
+            @Override
+            public CatalogItem<T,SpecT> apply(@Nullable CatalogItemDo<T,SpecT> item) {
+                if (item==null) return null;
+                CatalogItemDtoAbstract<T, SpecT> dto = (CatalogItemDtoAbstract<T, SpecT>) item.getDto();
+
+                // when scanning we only allow version and libraries to be overwritten
+                
+                String version = getFirstAs(catalogMetadata, String.class, "version").orNull();
+                if (Strings.isNonBlank(version)) dto.setVersion(version);
+                
+                Object librariesCombined = catalogMetadata.get("brooklyn.libraries");
+                if (librariesCombined instanceof Collection) {
+                    // will be set by scan -- slightly longwinded way to retrieve, but scanning for osgi needs an overhaul in any case
+                    Collection<CatalogBundle> libraryBundles = CatalogItemDtoAbstract.parseLibraries((Collection<?>) librariesCombined);
+                    dto.setLibraries(libraryBundles);
+                    // must replace java type with plan yaml here for libraries / catalog item to be picked up
+                    dto.setSymbolicName(dto.getJavaType());
+                    dto.setPlanYaml("services: [{ type: "+dto.getJavaType()+" }]");
+                    dto.setJavaType(null);
+                }
+
+                return dto;
+            }
+        };
+    }
 
     transient CatalogXmlSerializer serializer;
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
index c0716c8..077abaa 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
@@ -18,8 +18,10 @@
  */
 package brooklyn.catalog.internal;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.InputStream;
 import java.lang.reflect.Modifier;
-import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Arrays;
 import java.util.Set;
@@ -39,10 +41,13 @@ import brooklyn.entity.proxying.ImplementedBy;
 import brooklyn.location.Location;
 import brooklyn.management.internal.ManagementContextInternal;
 import brooklyn.policy.Policy;
+import brooklyn.util.ResourceUtils;
 import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.javalang.AggregateClassLoader;
 import brooklyn.util.javalang.ReflectionScanner;
 import brooklyn.util.javalang.UrlClassLoader;
+import brooklyn.util.os.Os;
+import brooklyn.util.stream.Streams;
 import brooklyn.util.text.Strings;
 import brooklyn.util.time.Time;
 
@@ -115,9 +120,35 @@ public class CatalogClasspathDo {
             urls = new URL[classpath.getEntries().size()];
             for (int i=0; i<urls.length; i++) {
                 try {
-                    urls[i] = new URL(classpath.getEntries().get(i));
-                } catch (MalformedURLException e) {
-                    log.error("Invalid URL "+classpath.getEntries().get(i)+" in definition of catalog "+catalog+"; skipping catalog");
+                    String u = classpath.getEntries().get(i);
+                    if (u.startsWith("classpath:")) {
+                        // special support for classpath: url's
+                        // TODO put convenience in ResourceUtils for extracting to a normal url
+                        // (or see below)
+                        InputStream uin = ResourceUtils.create(this).getResourceFromUrl(u);
+                        File f = Os.newTempFile("brooklyn-catalog-"+u, null);
+                        FileOutputStream fout = new FileOutputStream(f);
+                        Streams.copy(uin, fout);
+                        fout.close();
+                        uin.close();
+                        u = f.toURI().toString();
+                    }
+                    urls[i] = new URL(u);
+                    
+                    // TODO potential disk leak above as we have no way to know when the temp file can be removed earlier than server shutdown;
+                    // a better way to handle this is to supply a stream handler:
+//                    urls[i] = new URL(classpath.getEntries().get(i));
+//                    URI uri = URI.create(classpath.getEntries().get(i));
+//                    urls[i] = new URL(uri.getScheme(), uri.getHost(), uri.getPort(), uri.getPath()  // TODO query fragment?
+//                        , new URLStreamHandler() {
+//                            @Override
+//                            protected URLConnection openConnection(URL u) throws IOException {
+//                                new ResourceUtils(null). ???
+//                            }
+//                        });
+                } catch (Exception e) {
+                    Exceptions.propagateIfFatal(e);
+                    log.error("Error loading URL "+classpath.getEntries().get(i)+" in definition of catalog "+catalog+"; skipping definition");
                     throw Exceptions.propagate(e);
                 }
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/core/src/test/dependencies/osgi/more-entities-v2/src/main/java/brooklyn/osgi/tests/more/MoreEntity.java
----------------------------------------------------------------------
diff --git a/core/src/test/dependencies/osgi/more-entities-v2/src/main/java/brooklyn/osgi/tests/more/MoreEntity.java b/core/src/test/dependencies/osgi/more-entities-v2/src/main/java/brooklyn/osgi/tests/more/MoreEntity.java
index 955388c..987a7f8 100644
--- a/core/src/test/dependencies/osgi/more-entities-v2/src/main/java/brooklyn/osgi/tests/more/MoreEntity.java
+++ b/core/src/test/dependencies/osgi/more-entities-v2/src/main/java/brooklyn/osgi/tests/more/MoreEntity.java
@@ -19,11 +19,13 @@
 package brooklyn.osgi.tests.more;
 
 
+import brooklyn.catalog.Catalog;
 import brooklyn.entity.Effector;
 import brooklyn.entity.Entity;
 import brooklyn.entity.effector.Effectors;
 import brooklyn.entity.proxying.ImplementedBy;
 
+@Catalog(name="More Entity v2")
 @ImplementedBy(MoreEntityImpl.class)
 public interface MoreEntity extends Entity {
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/core/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar
----------------------------------------------------------------------
diff --git a/core/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar b/core/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar
index 1567f6e..91cc1e8 100644
Binary files a/core/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar and b/core/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar differ

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
index 492ef65..c157e93 100644
--- a/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
+++ b/usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java
@@ -230,12 +230,12 @@ public class BrooklynComponentTemplateResolver {
         boolean firstOccurrence = (item == null || encounteredCatalogTypes.add(item.getSymbolicName()));
         boolean recursiveButTryJava = !firstOccurrence;
 
-        // - Load a java class from current loader (item == null || entityResolver.isJavaTypePrefix())
-        // - Load a java class specified in an old-style catalog item (item != null && item.getJavaType() != null)
-        //   Old-style catalog items (can be defined in catalog.xml only) don't have structure, only a single type, so
-        //   they are loaded as a simple java type, only taking the class name from the catalog item instead of the
-        //   type value in the YAML. Classpath entries in the item are also used (through the catalog root classloader).
-        if (item == null || item.getJavaType() != null || isJavaTypePrefix()) {
+        // Load a java class from current loader if explicit java prefix, or if no item, or if item is legacy / 
+        // old-style catalog item (item != null && item.getJavaType() != null).
+        // Old-style catalog items (can be defined in catalog.xml only) don't have structure, only a single type, so
+        // they are loaded as a simple java type, only taking the class name from the catalog item instead of the
+        // type value in the YAML. Classpath entries in the item are also used (through the catalog root classloader).
+        if (isJavaTypePrefix() || item == null || item.getJavaType() != null) {
             return createSpecFromJavaType();
 
         // Same as above case, but this time force java type loading (either as plain class or through an old-style

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java
index 41489ba..67a2009 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlTest.java
@@ -144,15 +144,15 @@ public abstract class AbstractYamlTest {
         return app;
     }
 
-    protected void addCatalogItem(Iterable<String> catalogYaml) {
-        addCatalogItem(joinLines(catalogYaml));
+    protected void addCatalogItems(Iterable<String> catalogYaml) {
+        addCatalogItems(joinLines(catalogYaml));
     }
 
-    protected void addCatalogItem(String... catalogYaml) {
-        addCatalogItem(joinLines(catalogYaml));
+    protected void addCatalogItems(String... catalogYaml) {
+        addCatalogItems(joinLines(catalogYaml));
     }
 
-    protected void addCatalogItem(String catalogYaml) {
+    protected void addCatalogItems(String catalogYaml) {
         mgmt().getCatalog().addItems(catalogYaml, forceUpdate);
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ReferencedYamlTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ReferencedYamlTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ReferencedYamlTest.java
index d4f079a..683bdc3 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ReferencedYamlTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/ReferencedYamlTest.java
@@ -93,7 +93,7 @@ public class ReferencedYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCatalogReferencingYamlUrl() throws Exception {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: yaml.reference",
             "  version: " + TEST_VERSION,
@@ -111,7 +111,7 @@ public class ReferencedYamlTest extends AbstractYamlTest {
 
     @Test
     public void testYamlUrlReferencingCatalog() throws Exception {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: yaml.basic",
             "  version: " + TEST_VERSION,
@@ -136,7 +136,7 @@ public class ReferencedYamlTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String parentCatalogId = "my.catalog.app.id.url.parent";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + parentCatalogId,
             "  version: " + TEST_VERSION,

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java
index 19c9ef8..5c52c39 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java
@@ -21,6 +21,8 @@ package io.brooklyn.camp.brooklyn.catalog;
 import io.brooklyn.camp.brooklyn.AbstractYamlTest;
 import io.brooklyn.camp.brooklyn.spi.creation.BrooklynEntityMatcher;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -32,12 +34,15 @@ import brooklyn.management.osgi.OsgiVersionMoreEntityTest;
 import brooklyn.policy.Policy;
 import brooklyn.test.TestResourceUnavailableException;
 import brooklyn.util.ResourceUtils;
+import brooklyn.util.text.Strings;
 
 import com.google.common.collect.Iterables;
 
 /** Many of the same tests as per {@link OsgiVersionMoreEntityTest} but using YAML for catalog and entities, so catalog item ID is set automatically */
 public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
     
+    private static final Logger log = LoggerFactory.getLogger(CatalogOsgiVersionMoreEntityTest.class);
+    
     private static String getLocalResource(String filename) {
         return ResourceUtils.create(CatalogOsgiVersionMoreEntityTest.class).getResourceAsString(
             "classpath:/"+CatalogOsgiVersionMoreEntityTest.class.getPackage().getName().replace('.', '/')+"/"+filename);
@@ -47,7 +52,7 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
     public void testMoreEntityV1() throws Exception {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.1.0.jar");
 
-        addCatalogItem(getLocalResource("more-entity-v1-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v1-osgi-catalog.yaml"));
         CatalogItem<?, ?> item = CatalogUtils.getCatalogItemOptionalVersion(mgmt(), "more-entity");
         Assert.assertNotNull(item);
         Assert.assertEquals(item.getVersion(), "1.0");
@@ -69,8 +74,8 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.1.0.jar");
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-entities.jar");
 
-        addCatalogItem(getLocalResource("simple-policy-osgi-catalog.yaml"));
-        addCatalogItem(getLocalResource("more-entity-v1-with-policy-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("simple-policy-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v1-with-policy-osgi-catalog.yaml"));
         Entity app = createAndStartApplication("services: [ { type: 'more-entity:1.0' } ]");
         Entity moreEntity = Iterables.getOnlyElement(app.getChildren());
         
@@ -87,7 +92,7 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar");
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-entities.jar");
 
-        addCatalogItem(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
         Entity app = createAndStartApplication("services: [ { type: 'more-entity:1.0' } ]");
         Entity moreEntity = Iterables.getOnlyElement(app.getChildren());
         
@@ -108,9 +113,9 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar");
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-entities.jar");
 
-        addCatalogItem(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
         forceCatalogUpdate();
-        addCatalogItem(getLocalResource("more-entity-v1-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v1-osgi-catalog.yaml"));
         Entity app = createAndStartApplication("services: [ { type: 'more-entity:1.0' } ]");
         Entity moreEntity = Iterables.getOnlyElement(app.getChildren());
         
@@ -127,9 +132,9 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar");
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-entities.jar");
 
-        addCatalogItem(getLocalResource("more-entity-v1-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v1-osgi-catalog.yaml"));
         forceCatalogUpdate();
-        addCatalogItem(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
         Entity app = createAndStartApplication("services: [ { type: 'more-entity:1.0' } ]");
         Entity moreEntity = Iterables.getOnlyElement(app.getChildren());
         
@@ -143,8 +148,8 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar");
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-entities.jar");
 
-        addCatalogItem(getLocalResource("more-entity-v1-called-v1-osgi-catalog.yaml"));
-        addCatalogItem(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v1-called-v1-osgi-catalog.yaml"));
+        addCatalogItems(getLocalResource("more-entity-v2-osgi-catalog.yaml"));
         Entity v1 = createAndStartApplication("services: [ { type: 'more-entity-v1:1.0' } ]");
         Entity v2 = createAndStartApplication("services: [ { type: 'more-entity:1.0' } ]");
         
@@ -158,5 +163,32 @@ public class CatalogOsgiVersionMoreEntityTest extends AbstractYamlTest {
         OsgiVersionMoreEntityTest.assertV2MethodCall(moreEntityV2);
     }
 
+    @Test
+    public void testMoreEntityV2AutoscanWithClasspath() throws Exception {
+        TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar");
+        TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-entities.jar");
+        
+        addCatalogItems(getLocalResource("more-entities-osgi-catalog-scan.yaml"));
+        
+        log.info("autoscan for osgi found catalog items: "+Strings.join(mgmt().getCatalog().getCatalogItems(), ", "));
+
+        CatalogItem<?, ?> item = CatalogUtils.getCatalogItemOptionalVersion(mgmt(), "more-entity");
+        Assert.assertNotNull(item);
+        Assert.assertEquals(item.getVersion(), "2.0.test");
+        Assert.assertEquals(item.getCatalogItemType(), CatalogItemType.ENTITY);
+        
+        // this refers to the java item, where the libraries are defined
+        item = CatalogUtils.getCatalogItemOptionalVersion(mgmt(), "brooklyn.osgi.tests.more.MoreEntity");
+        Assert.assertEquals(item.getVersion(), "2.0.test_java");
+        Assert.assertEquals(item.getLibraries().size(), 2);
+        
+        Entity app = createAndStartApplication("services: [ { type: 'more-entity:2.0.test' } ]");
+        Entity moreEntity = Iterables.getOnlyElement(app.getChildren());
+        
+        Assert.assertEquals(moreEntity.getCatalogItemId(), "more-entity:2.0.test");
+        OsgiVersionMoreEntityTest.assertV2EffectorCall(moreEntity);
+        OsgiVersionMoreEntityTest.assertV2MethodCall(moreEntity);
+    }
+
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java
index cbb1d1e..4b33276 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlCombiTest.java
@@ -44,7 +44,7 @@ public class CatalogYamlCombiTest extends AbstractYamlTest {
     
     @Test
     public void testBRefEntityA() throws Exception {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  version: "+TEST_VERSION,
             "  items:",
@@ -86,7 +86,7 @@ public class CatalogYamlCombiTest extends AbstractYamlTest {
 
     @Test
     public void testBRefPolicyALocationZ() throws Exception {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  version: "+TEST_VERSION,
             "  id: Z",
@@ -94,7 +94,7 @@ public class CatalogYamlCombiTest extends AbstractYamlTest {
             "  - item: ",
             "      type: localhost",
             "      brooklyn.config: { z: 9 }");
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  version: "+TEST_VERSION,
             "  items:",

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
index 9353108..af3d0b8 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
@@ -52,7 +52,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     @Test
     public void testAddCatalogItemVerySimple() throws Exception {
         String symbolicName = "my.catalog.app.id.load";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  version: " + TEST_VERSION,
@@ -81,7 +81,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String symbolicName = "my.catalog.app.id.load";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",
@@ -103,7 +103,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String symbolicName = "my.catalog.app.id.load";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",
@@ -126,7 +126,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String symbolicName = "my.catalog.app.id.load";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",
@@ -150,7 +150,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String id = "unversioned.app";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  name: " + id,
             "  libraries:",
@@ -167,7 +167,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String id = "inline_version.app";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  name: " + id+":"+TEST_VERSION,
             "  libraries:",
@@ -357,7 +357,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
 
         String firstItemId = "my.catalog.app.id.register_bundle";
         String secondItemId = "my.catalog.app.id.reference_bundle";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + firstItemId,
             "  version: " + TEST_VERSION,
@@ -368,7 +368,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
             "- type: " + SIMPLE_ENTITY_TYPE);
         deleteCatalogEntity(firstItemId);
 
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + secondItemId,
             "  version: " + TEST_VERSION,
@@ -387,7 +387,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         String nonExistentId = "none-existent-id";
         String nonExistentVersion = "9.9.9";
         try {
-            addCatalogItem(
+            addCatalogItems(
                 "brooklyn.catalog:",
                 "  id: my.catalog.app.id.non_existing.ref",
                 "  version: " + TEST_VERSION,
@@ -406,7 +406,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     @Test
     public void testPartialBundleReferenceFails() {
         try {
-            addCatalogItem(
+            addCatalogItems(
                 "brooklyn.catalog:",
                 "  id: my.catalog.app.id.non_existing.ref",
                 "  version: " + TEST_VERSION,
@@ -420,7 +420,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
             Assert.assertEquals(e.getMessage(), "version");
         }
         try {
-            addCatalogItem(
+            addCatalogItems(
                 "brooklyn.catalog:",
                 "  id: my.catalog.app.id.non_existing.ref",
                 "  version: " + TEST_VERSION,
@@ -440,7 +440,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String itemId = "my.catalog.app.id.full_ref";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + itemId,
             "  version: " + TEST_VERSION,
@@ -466,7 +466,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         String nonExistentId = "non_existent_id";
         String nonExistentVersion = "9.9.9";
         try {
-            addCatalogItem(
+            addCatalogItems(
                 "brooklyn.catalog:",
                 "  id: " + firstItemId,
                 "  version: " + TEST_VERSION,
@@ -545,7 +545,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     @Test
     public void testMissingTypeDoesNotRecurse() {
         String symbolicName = "my.catalog.app.id.basic";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  version: " + TEST_VERSION,
@@ -554,7 +554,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
             "- type: brooklyn.entity.basic.BasicEntity");
 
         try {
-            addCatalogItem(
+            addCatalogItems(
                     "brooklyn.catalog:",
                     "  id: " + symbolicName,
                     "  version: " + TEST_VERSION + "-update",
@@ -570,7 +570,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     @Test
     public void testVersionedTypeDoesNotRecurse() {
         String symbolicName = "my.catalog.app.id.basic";
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  version: " + TEST_VERSION,
@@ -580,7 +580,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
 
         String versionedId = CatalogUtils.getVersionedId(symbolicName, TEST_VERSION);
         try {
-            addCatalogItem(
+            addCatalogItems(
                 "brooklyn.catalog:",
                 "  id: " + symbolicName,
                 "  version: " + TEST_VERSION + "-update",
@@ -597,7 +597,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     public void testOsgiNotLeakingToParent() {
         addCatalogOSGiEntity(SIMPLE_ENTITY_TYPE);
         try {
-            addCatalogItem(
+            addCatalogItems(
                     "brooklyn.catalog:",
                     "  id: " + SIMPLE_ENTITY_TYPE,
                     "  version: " + TEST_VERSION + "-update",
@@ -633,7 +633,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
     }
     
     private void addCatalogOSGiEntity(String symbolicName, String serviceType, boolean extraLib) {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",
@@ -665,10 +665,10 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
             "      type: " + namesAndTypes[i+1]));
         }
             
-        addCatalogItem(lines);
+        addCatalogItems(lines);
     }
     private void addCatalogChildOSGiEntityWithServicesBlock(String symbolicName, String serviceType) {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",
@@ -684,7 +684,7 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
             "      - type: " + serviceType);
     }
     private void addCatalogChildOSGiEntity(String symbolicName, String serviceType) {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
index c0d41d2..bfc8a9a 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlLocationTest.java
@@ -216,7 +216,7 @@ public class CatalogYamlLocationTest extends AbstractYamlTest {
                 "      config2: config2");
         
         
-        addCatalogItem(yaml.build());
+        addCatalogItems(yaml.build());
     }
 
     private void addCatalogLocationTopLevelItemSyntax(String symbolicName, String locationType, List<String> libraries) {
@@ -239,7 +239,7 @@ public class CatalogYamlLocationTest extends AbstractYamlTest {
                 "    config2: config2");
         
         
-        addCatalogItem(yaml.build());
+        addCatalogItems(yaml.build());
     }
 
     private int countCatalogLocations() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java
index d38fee0..f007bae 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlPolicyTest.java
@@ -121,7 +121,7 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest {
         String referrerSymbolicName = "my.catalog.policy.id.referring";
         addCatalogOsgiPolicy(referencedSymbolicName, SIMPLE_POLICY_TYPE);
 
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + referrerSymbolicName,
             "  name: My Catalog App",
@@ -153,7 +153,7 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest {
     private void addCatalogOsgiPolicy(String symbolicName, String policyType) {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog Policy",
@@ -172,7 +172,7 @@ public class CatalogYamlPolicyTest extends AbstractYamlTest {
     private void addCatalogOsgiPolicyTopLevelSyntax(String symbolicName, String policyType) {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog Policy",

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java
index 8fbcf65..d5a5b9c 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java
@@ -66,7 +66,7 @@ public class CatalogYamlTemplateTest extends AbstractYamlTest {
     private CatalogItem<?, ?> makeItem() {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
         
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: t1",
             "  item_type: template",

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
index dda60dc..4e43656 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlVersioningTest.java
@@ -182,7 +182,7 @@ public class CatalogYamlVersioningTest extends AbstractYamlTest {
     }
 
     private void doTestVersionedReferenceJustAdded(boolean isVersionImplicitSyntax) throws Exception {
-        addCatalogItem(            "brooklyn.catalog:",
+        addCatalogItems(            "brooklyn.catalog:",
             "  version: 0.9",
             "  items:",
             "  - id: referrent",
@@ -243,7 +243,7 @@ public class CatalogYamlVersioningTest extends AbstractYamlTest {
     }
     
     private void addCatalogEntity(String symbolicName, String version, String type, String iconUrl) {
-        addCatalogItem(
+        addCatalogItems(
             "brooklyn.catalog:",
             "  id: " + symbolicName,
             "  name: My Catalog App",

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/usage/camp/src/test/resources/io/brooklyn/camp/brooklyn/catalog/more-entities-osgi-catalog-scan.yaml
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/resources/io/brooklyn/camp/brooklyn/catalog/more-entities-osgi-catalog-scan.yaml b/usage/camp/src/test/resources/io/brooklyn/camp/brooklyn/catalog/more-entities-osgi-catalog-scan.yaml
new file mode 100644
index 0000000..852d9e1
--- /dev/null
+++ b/usage/camp/src/test/resources/io/brooklyn/camp/brooklyn/catalog/more-entities-osgi-catalog-scan.yaml
@@ -0,0 +1,32 @@
+#
+# 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.
+#
+
+# test case which demonstrates osgi bundles can be scanned, *if* expand classpath is true
+
+brooklyn.catalog:
+  items:
+  - scanJavaAnnotations: true
+    version: 2.0.test_java
+    libraries:
+    - classpath:/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar
+    - classpath:/brooklyn/osgi/brooklyn-test-osgi-entities.jar
+  - item:
+      id: more-entity
+      type: brooklyn.osgi.tests.more.MoreEntity
+      version: 2.0.test

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/1207d991/utils/common/src/main/java/brooklyn/util/os/Os.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/os/Os.java b/utils/common/src/main/java/brooklyn/util/os/Os.java
index 3cd1ccb..5278988 100644
--- a/utils/common/src/main/java/brooklyn/util/os/Os.java
+++ b/utils/common/src/main/java/brooklyn/util/os/Os.java
@@ -523,10 +523,10 @@ public class Os {
      * either prefix or ext may be null; 
      * if ext is non-empty and not > 4 chars and not starting with a ., then a dot will be inserted */
     public static File newTempFile(String prefix, String ext) {
-        String sanitizedPrefix = (prefix!=null ? prefix + "-" : "");
-        String sanitizedExt = (ext!=null ? ext.startsWith(".") || ext.length()>4 ? ext : "."+ext : "");
+        String sanitizedPrefix = (Strings.isNonEmpty(prefix) ? Strings.makeValidFilename(prefix) + "-" : "");
+        String extWithPrecedingSeparator = (Strings.isNonEmpty(ext) ? ext.startsWith(".") || ext.length()>4 ? ext : "."+ext : "");
         try {
-            File tempFile = File.createTempFile(sanitizedPrefix, sanitizedExt, new File(tmp()));
+            File tempFile = File.createTempFile(sanitizedPrefix, extWithPrecedingSeparator, new File(tmp()));
             tempFile.deleteOnExit();
             return tempFile;
         } catch (IOException e) {



[02/13] incubator-brooklyn git commit: code review for catalog CLI

Posted by he...@apache.org.
code review for catalog CLI


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/d6ae0511
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/d6ae0511
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/d6ae0511

Branch: refs/heads/master
Commit: d6ae0511c35ef4070c8a1637bad1ef3b113bda81
Parents: eef7891
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Mon May 18 16:02:07 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed May 20 00:39:50 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/entity/rebind/RebindManager.java   |   7 +-
 .../mementos/BrooklynMementoPersister.java      |   1 +
 .../catalog/internal/BasicBrooklynCatalog.java  |  43 +++---
 .../brooklyn/catalog/internal/CatalogDo.java    |   1 +
 .../catalog/internal/CatalogInitialization.java | 139 ++++++++++---------
 .../rebind/PeriodicDeltaChangeListener.java     |  37 +++--
 .../brooklyn/entity/rebind/RebindIteration.java |   5 +-
 .../entity/rebind/RebindManagerImpl.java        |  10 +-
 .../BrooklynMementoPersisterToObjectStore.java  |   2 +-
 .../internal/AbstractManagementContext.java     |  16 ++-
 .../NonDeploymentManagementContext.java         |   7 +-
 .../catalog/internal/CatalogScanTest.java       |   1 +
 .../entity/rebind/ActivePartialRebindTest.java  |   2 +-
 .../brooklyn/entity/rebind/RebindTestUtils.java |   2 +-
 ...ntoPersisterInMemorySizeIntegrationTest.java |  21 ++-
 .../osgi/OsgiVersionMoreEntityTest.java         |   4 +-
 usage/cli/src/main/java/brooklyn/cli/Main.java  |   3 +-
 .../brooklyn/launcher/BrooklynLauncher.java     |   6 +-
 .../brooklyn/rest/resources/ServerResource.java |   9 +-
 .../brooklyn/rest/HaMasterCheckFilterTest.java  |   2 +-
 .../util/javalang/AggregateClassLoader.java     |  41 +++++-
 21 files changed, 205 insertions(+), 154 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/api/src/main/java/brooklyn/entity/rebind/RebindManager.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/entity/rebind/RebindManager.java b/api/src/main/java/brooklyn/entity/rebind/RebindManager.java
index 501430d..8e4cb29 100644
--- a/api/src/main/java/brooklyn/entity/rebind/RebindManager.java
+++ b/api/src/main/java/brooklyn/entity/rebind/RebindManager.java
@@ -20,7 +20,6 @@ package brooklyn.entity.rebind;
 
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import javax.annotation.Nullable;
@@ -105,13 +104,9 @@ public interface RebindManager {
      * waiting for activity there to cease (interrupting in the case of {@link #stopReadOnly()}). */
     public void stop();
     
-    /** @deprecated since 0.7.0; use {@link #waitForPendingComplete(Duration)} */
     @VisibleForTesting
-    @Deprecated
-    public void waitForPendingComplete(long timeout, TimeUnit unit) throws InterruptedException, TimeoutException;
     /** waits for any needed or pending writes to complete */
-    @VisibleForTesting
-    public void waitForPendingComplete(Duration duration) throws InterruptedException, TimeoutException;
+    public void waitForPendingComplete(Duration duration, boolean canTrigger) throws InterruptedException, TimeoutException;
     /** Forcibly performs persistence, in the foreground 
      * @deprecated since 0.7.0; use {@link #forcePersistNow(boolean, PersistenceExceptionHandler)}, 
      * default parameter here is false to mean incremental, with null/default exception handler */

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java b/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
index 33f1de7..ec16e9b 100644
--- a/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
+++ b/api/src/main/java/brooklyn/mementos/BrooklynMementoPersister.java
@@ -103,6 +103,7 @@ public interface BrooklynMementoPersister {
     /** applies a partial write of state delta */  
     void delta(Delta delta, PersistenceExceptionHandler exceptionHandler);
     /** inserts an additional delta to be written on the next delta request */
+    @Beta
     void queueDelta(Delta delta);
 
     void enableWriteAccess();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
index 7e4763d..88e5818 100644
--- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
@@ -621,9 +621,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         } else {
             // scan for annotations: if libraries here, scan them; if inherited libraries error; else scan classpath
             if (!libraryBundlesNew.isEmpty()) {
-                result.addAll(scanAnnotations(mgmt, libraryBundlesNew));
+                result.addAll(scanAnnotationsFromBundles(mgmt, libraryBundlesNew));
             } else if (libraryBundles.isEmpty()) {
-                result.addAll(scanAnnotations(mgmt, null));
+                result.addAll(scanAnnotationsFromLocal(mgmt));
             } else {
                 throw new IllegalStateException("Cannot scan catalog node no local bundles, and with inherited bundles we will not scan the classpath");
             }
@@ -780,25 +780,26 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         return oldValue;
     }
 
-    /** scans the given libraries for annotated items, or if null scans the local classpath */ 
-    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotations(ManagementContext mgmt, Collection<CatalogBundle> libraries) {
-//        CatalogDto dto = CatalogDto.newDefaultLocalScanningDto(CatalogClasspathDo.CatalogScanningModes.ANNOTATIONS);
-        CatalogDto dto;
+    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromLocal(ManagementContext mgmt) {
+        CatalogDto dto = CatalogDto.newNamedInstance("Local Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-local-classpath");
+        return scanAnnotationsInternal(mgmt, new CatalogDo(dto));
+    }
+    
+    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromBundles(ManagementContext mgmt, Collection<CatalogBundle> libraries) {
         String[] urls = null;
-        if (libraries==null) {
-            dto = CatalogDto.newNamedInstance("Local Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-local-classpath");
-        } else {
-            dto = CatalogDto.newNamedInstance("Bundles Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-bundles-classpath-"+libraries.hashCode());
-            urls = new String[libraries.size()];
-            int i=0;
-            for (CatalogBundle b: libraries)
-                urls[i++] = b.getUrl();
-        }
+        CatalogDto dto = CatalogDto.newNamedInstance("Bundles Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-bundles-classpath-"+libraries.hashCode());
+        urls = new String[libraries.size()];
+        int i=0;
+        for (CatalogBundle b: libraries)
+            urls[i++] = b.getUrl();
+            
         CatalogDo subCatalog = new CatalogDo(dto);
+        subCatalog.addToClasspath(urls);
+        return scanAnnotationsInternal(mgmt, subCatalog);
+    }
+    
+    private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsInternal(ManagementContext mgmt, CatalogDo subCatalog) {
         subCatalog.mgmt = mgmt;
-        if (urls!=null) {
-            subCatalog.addToClasspath(urls);
-        } // else use local classpath
         subCatalog.setClasspathScanForEntities(CatalogScanningModes.ANNOTATIONS);
         subCatalog.load();
         // TODO apply metadata?  (extract YAML from the items returned)
@@ -1045,7 +1046,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
     
     private CatalogItem<?,?> addItemDto(CatalogItemDtoAbstract<?, ?> itemDto, boolean forceUpdate) {
-        CatalogItem<?, ?> existingDto = checkItemIsDuplicateOrDisallowed(itemDto, true, forceUpdate);
+        CatalogItem<?, ?> existingDto = checkItemAllowedAndIfSoReturnAnyDuplicate(itemDto, true, forceUpdate);
         if (existingDto!=null) {
             // it's a duplicate, and not forced, just return it
             log.trace("Using existing duplicate for catalog item {}", itemDto.getId());
@@ -1072,9 +1073,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
         return itemDto;
     }
 
-    /** returns item DTO if item is an allowed duplicate, null if it should be added, or false if the item is an allowed duplicate,
+    /** returns item DTO if item is an allowed duplicate, or null if it should be added (there is no duplicate), 
      * throwing if item cannot be added */
-    private CatalogItem<?, ?> checkItemIsDuplicateOrDisallowed(CatalogItem<?,?> itemDto, boolean allowDuplicates, boolean forceUpdate) {
+    private CatalogItem<?, ?> checkItemAllowedAndIfSoReturnAnyDuplicate(CatalogItem<?,?> itemDto, boolean allowDuplicates, boolean forceUpdate) {
         if (forceUpdate) return null;
         CatalogItemDo<?, ?> existingItem = getCatalogItemDo(itemDto.getSymbolicName(), itemDto.getVersion());
         if (existingItem == null) return null;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
index 2142ce9..de68213 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
@@ -267,6 +267,7 @@ public class CatalogDo {
         return loadCatalog(child);
     }
     
+    /** adds the given urls; filters out any nulls supplied */
     public synchronized void addToClasspath(String ...urls) {
         if (dto.classpath == null)
             dto.classpath = new CatalogClasspathDto();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
index ded7dc4..6f0b80e 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
@@ -37,7 +37,7 @@ import brooklyn.util.exceptions.FatalRuntimeException;
 import brooklyn.util.exceptions.RuntimeInterruptedException;
 import brooklyn.util.flags.TypeCoercions;
 import brooklyn.util.guava.Maybe;
-import brooklyn.util.net.Urls;
+import brooklyn.util.os.Os;
 import brooklyn.util.text.Strings;
 
 import com.google.common.annotations.Beta;
@@ -53,7 +53,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
 
     A1) if not persisting, go to B1
     A2) if --catalog-reset, delete persisted catalog items
-    A3) read persisted catalog items (possibly deleted in A2), go to C1
+    A3) if there is a persisted catalog (and it wasn't not deleted by A2), read it and go to C1
     A4) go to B1
 
     B1) look for --catalog-initial, if so read it, then go to C1
@@ -71,20 +71,20 @@ public class CatalogInitialization implements ManagementContextInjectable {
 
     private static final Logger log = LoggerFactory.getLogger(CatalogInitialization.class);
     
-    String initialUri;
-    boolean reset;
-    String additionsUri;
-    boolean force;
-
-    boolean disallowLocal = false;
-    List<Function<CatalogInitialization, Void>> callbacks = MutableList.of();
-    boolean hasRunBestEffort = false, hasRunOfficial = false, isPopulating = false;
+    private String initialUri;
+    private boolean reset;
+    private String additionsUri;
+    private boolean force;
+
+    private boolean disallowLocal = false;
+    private List<Function<CatalogInitialization, Void>> callbacks = MutableList.of();
+    private boolean hasRunBestEffort = false, hasRunOfficial = false, isPopulating = false;
     
-    ManagementContext managementContext;
-    boolean isStartingUp = false;
-    boolean failOnStartupErrors = false;
+    private ManagementContext managementContext;
+    private boolean isStartingUp = false;
+    private boolean failOnStartupErrors = false;
     
-    Object mutex = new Object();
+    private Object populatingCatalogMutex = new Object();
     
     public CatalogInitialization(String initialUri, boolean reset, String additionUri, boolean force) {
         this.initialUri = initialUri;
@@ -98,19 +98,30 @@ public class CatalogInitialization implements ManagementContextInjectable {
     }
 
     public void injectManagementContext(ManagementContext managementContext) {
-        if (this.managementContext!=null && managementContext!=null && !this.managementContext.equals(managementContext))
-            throw new IllegalStateException("Cannot switch management context of "+this+"; from "+this.managementContext+" to "+managementContext);
+        Preconditions.checkNotNull(managementContext, "management context");
+        if (this.managementContext!=null && managementContext!=this.managementContext)
+            throw new IllegalStateException("Cannot switch management context, from "+this.managementContext+" to "+managementContext);
         this.managementContext = managementContext;
     }
     
-    public ManagementContext getManagementContext() {
-        return Preconditions.checkNotNull(managementContext, "management context has not been injected into "+this);
+    /** Called by the framework to set true while starting up, and false afterwards,
+     * in order to assist in appropriate logging and error handling. */
+    public void setStartingUp(boolean isStartingUp) {
+        this.isStartingUp = isStartingUp;
+    }
+
+    public void setFailOnStartupErrors(boolean startupFailOnCatalogErrors) {
+        this.failOnStartupErrors = startupFailOnCatalogErrors;
     }
 
     public CatalogInitialization addPopulationCallback(Function<CatalogInitialization, Void> callback) {
         callbacks.add(callback);
         return this;
     }
+    
+    public ManagementContext getManagementContext() {
+        return Preconditions.checkNotNull(managementContext, "management context has not been injected into "+this);
+    }
 
     public boolean isInitialResetRequested() {
         return reset;
@@ -121,9 +132,9 @@ public class CatalogInitialization implements ManagementContextInjectable {
 
     /** makes or updates the mgmt catalog, based on the settings in this class */
     public void populateCatalog(boolean needsInitial, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
-        try {
-            isPopulating = true;
-            synchronized (mutex) {
+        synchronized (populatingCatalogMutex) {
+            try {
+                isPopulating = true;
                 BasicBrooklynCatalog catalog = (BasicBrooklynCatalog) managementContext.getCatalog();
                 if (!catalog.getCatalog().isLoaded()) {
                     catalog.load();
@@ -136,15 +147,15 @@ public class CatalogInitialization implements ManagementContextInjectable {
                 }
                 hasRunOfficial = true;
 
-                populateCatalog(catalog, needsInitial, true, optionalItemsForResettingCatalog);
+                populateCatalogImpl(catalog, needsInitial, optionalItemsForResettingCatalog);
+            } finally {
+                hasRunOfficial = true;
+                isPopulating = false;
             }
-        } finally {
-            hasRunOfficial = true;
-            isPopulating = false;
         }
     }
 
-    private void populateCatalog(BasicBrooklynCatalog catalog, boolean needsInitial, boolean runCallbacks, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
+    private void populateCatalogImpl(BasicBrooklynCatalog catalog, boolean needsInitial, Collection<CatalogItem<?, ?>> optionalItemsForResettingCatalog) {
         applyCatalogLoadMode();
         
         if (optionalItemsForResettingCatalog!=null) {
@@ -157,9 +168,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
         
         populateAdditions(catalog);
 
-        if (runCallbacks) {
-            populateViaCallbacks(catalog);
-        }
+        populateViaCallbacks(catalog);
     }
 
     private enum PopulateMode { YAML, XML, AUTODETECT }
@@ -190,13 +199,13 @@ public class CatalogInitialization implements ManagementContextInjectable {
             return;
         }
         
-        catalogUrl = Urls.mergePaths(BrooklynServerConfig.getMgmtBaseDir( managementContext.getConfig() ), "catalog.bom");
+        catalogUrl = Os.mergePaths(BrooklynServerConfig.getMgmtBaseDir( managementContext.getConfig() ), "catalog.bom");
         if (new File(catalogUrl).exists()) {
             populateInitialFromUri(catalog, "file:"+catalogUrl, PopulateMode.YAML);
             return;
         }
         
-        catalogUrl = Urls.mergePaths(BrooklynServerConfig.getMgmtBaseDir( managementContext.getConfig() ), "catalog.xml");
+        catalogUrl = Os.mergePaths(BrooklynServerConfig.getMgmtBaseDir( managementContext.getConfig() ), "catalog.xml");
         if (new File(catalogUrl).exists()) {
             populateInitialFromUri(catalog, "file:"+catalogUrl, PopulateMode.XML);
             return;
@@ -244,7 +253,14 @@ public class CatalogInitialization implements ManagementContextInjectable {
         
         if (result==null && contents!=null && (mode==PopulateMode.XML || mode==PopulateMode.AUTODETECT)) {
             // then try XML
-            problem = populateInitialFromUriXml(catalog, catalogUrl, problem, contents);
+            try {
+                populateInitialFromUriXml(catalog, catalogUrl, contents);
+                // clear YAML problem
+                problem = null;
+            } catch (Exception e) {
+                Exceptions.propagateIfFatal(e);
+                if (problem==null) problem = e;
+            }
         }
         
         if (result!=null) {
@@ -259,19 +275,11 @@ public class CatalogInitialization implements ManagementContextInjectable {
 
     // deprecated XML format
     @SuppressWarnings("deprecation")
-    private Exception populateInitialFromUriXml(BasicBrooklynCatalog catalog, String catalogUrl, Exception problem, String contents) {
-        CatalogDto dto = null;
-        try {
-            dto = CatalogDto.newDtoFromXmlContents(contents, catalogUrl);
-            problem = null;
-        } catch (Exception e) {
-            Exceptions.propagateIfFatal(e);
-            if (problem==null) problem = e;
-        }
+    private void populateInitialFromUriXml(BasicBrooklynCatalog catalog, String catalogUrl, String contents) {
+        CatalogDto dto = CatalogDto.newDtoFromXmlContents(contents, catalogUrl);
         if (dto!=null) {
             catalog.reset(dto);
         }
-        return problem;
     }
 
     boolean hasRunAdditions = false;
@@ -303,6 +311,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
             callback.apply(this);
     }
 
+    private Object setFromCLMMutex = new Object();
     private boolean setFromCatalogLoadMode = false;
 
     /** @deprecated since introduced in 0.7.0, only for legacy compatibility with 
@@ -310,29 +319,31 @@ public class CatalogInitialization implements ManagementContextInjectable {
      * allowing control of catalog loading from a brooklyn property */
     @Deprecated
     public void applyCatalogLoadMode() {
-        if (setFromCatalogLoadMode) return;
-        setFromCatalogLoadMode = true;
-        Maybe<Object> clmm = ((ManagementContextInternal)managementContext).getConfig().getConfigRaw(BrooklynServerConfig.CATALOG_LOAD_MODE, false);
-        if (clmm.isAbsent()) return;
-        brooklyn.catalog.CatalogLoadMode clm = TypeCoercions.coerce(clmm.get(), brooklyn.catalog.CatalogLoadMode.class);
-        log.warn("Legacy CatalogLoadMode "+clm+" set: applying, but this should be changed to use new CLI --catalogXxx commands");
-        switch (clm) {
-        case LOAD_BROOKLYN_CATALOG_URL:
-            reset = true;
-            break;
-        case LOAD_BROOKLYN_CATALOG_URL_IF_NO_PERSISTED_STATE:
-            // now the default
-            break;
-        case LOAD_PERSISTED_STATE:
-            disallowLocal = true;
-            break;
+        synchronized (setFromCLMMutex) {
+            if (setFromCatalogLoadMode) return;
+            setFromCatalogLoadMode = true;
+            Maybe<Object> clmm = ((ManagementContextInternal)managementContext).getConfig().getConfigRaw(BrooklynServerConfig.CATALOG_LOAD_MODE, false);
+            if (clmm.isAbsent()) return;
+            brooklyn.catalog.CatalogLoadMode clm = TypeCoercions.coerce(clmm.get(), brooklyn.catalog.CatalogLoadMode.class);
+            log.warn("Legacy CatalogLoadMode "+clm+" set: applying, but this should be changed to use new CLI --catalogXxx commands");
+            switch (clm) {
+            case LOAD_BROOKLYN_CATALOG_URL:
+                reset = true;
+                break;
+            case LOAD_BROOKLYN_CATALOG_URL_IF_NO_PERSISTED_STATE:
+                // now the default
+                break;
+            case LOAD_PERSISTED_STATE:
+                disallowLocal = true;
+                break;
+            }
         }
     }
 
     /** makes the catalog, warning if persistence is on and hasn't run yet 
      * (as the catalog will be subsequently replaced) */
     public void populateBestEffort(BasicBrooklynCatalog catalog) {
-        synchronized (mutex) {
+        synchronized (populatingCatalogMutex) {
             if (hasRunOfficial || hasRunBestEffort || isPopulating) return;
             // if a thread calls back in to this, ie calling to it from a getCatalog() call while populating,
             // it will own the mutex and observe isRunningBestEffort, returning quickly 
@@ -341,7 +352,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
                 if (isStartingUp) {
                     log.warn("Catalog access requested when not yet initialized; populating best effort rather than through recommended pathway. Catalog data may be replaced subsequently.");
                 }
-                populateCatalog(catalog, true, true, null);
+                populateCatalogImpl(catalog, true, null);
             } finally {
                 hasRunBestEffort = true;
                 isPopulating = false;
@@ -349,14 +360,6 @@ public class CatalogInitialization implements ManagementContextInjectable {
         }
     }
 
-    public void setStartingUp(boolean isStartingUp) {
-        this.isStartingUp = isStartingUp;
-    }
-
-    public void setFailOnStartupErrors(boolean startupFailOnCatalogErrors) {
-        this.failOnStartupErrors = startupFailOnCatalogErrors;
-    }
-
     public void handleException(Throwable throwable, Object details) {
         if (throwable instanceof InterruptedException)
             throw new RuntimeInterruptedException((InterruptedException) throwable);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
index cd33f01..f3861bc 100644
--- a/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
+++ b/core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java
@@ -25,6 +25,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -177,6 +178,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
     
     private final Semaphore persistingMutex = new Semaphore(1);
     private final Object startStopMutex = new Object();
+    private final AtomicInteger writeCount = new AtomicInteger(0);
 
     private PersistenceActivityMetrics metrics;
     
@@ -230,7 +232,7 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
                     CountdownTimer expiry = timeout.countdownTimer();
                     try {
                         scheduledTask.cancel(false);  
-                        waitForPendingComplete(expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations));
+                        waitForPendingComplete(expiry.getDurationRemaining().lowerBound(Duration.ZERO).add(graceTimeoutForSubsequentOperations), true);
                     } catch (Exception e) {
                         throw Exceptions.propagate(e);
                     }
@@ -243,7 +245,6 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
                     scheduledTask = null;
                 }
 
-
                 // Discard all state that was waiting to be persisted
                 synchronized (this) {
                     deltaCollector = new DeltaCollector();
@@ -255,30 +256,38 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
         }
     }
     
-    /**
-     * @deprecated since 0.7.0, use {@link #waitForPendingComplete(Duration)}
-     */
-    @VisibleForTesting
-    public void waitForPendingComplete(long timeout, TimeUnit unit) throws InterruptedException, TimeoutException {
-        waitForPendingComplete(Duration.of(timeout, unit));
-    }
     /** Waits for any in-progress writes to be completed then for or any unwritten data to be written. */
     @VisibleForTesting
-    public void waitForPendingComplete(Duration timeout) throws InterruptedException, TimeoutException {
+    public void waitForPendingComplete(Duration timeout, boolean canTrigger) throws InterruptedException, TimeoutException {
         if (!isActive() && !stopping) return;
         
         CountdownTimer timer = timeout.isPositive() ? CountdownTimer.newInstanceStarted(timeout) : CountdownTimer.newInstancePaused(Duration.PRACTICALLY_FOREVER);
+        Integer targetWriteCount = null;
         // wait for mutex, so we aren't tricked by an in-progress who has already recycled the collector
         if (persistingMutex.tryAcquire(timer.getDurationRemaining().toMilliseconds(), TimeUnit.MILLISECONDS)) {
             try {
                 // now no one else is writing
                 if (!deltaCollector.isEmpty()) {
-                    // but there is data that needs to be written
-                    persistNowSafely(true);
+                    if (canTrigger) {
+                        // but there is data that needs to be written
+                        persistNowSafely(true);
+                    } else {
+                        targetWriteCount = writeCount.get()+1;
+                    }
                 }
             } finally {
                 persistingMutex.release();
             }
+            if (targetWriteCount!=null) {
+                while (writeCount.get() <= targetWriteCount) {
+                    Duration left = timer.getDurationRemaining();
+                    if (left.isPositive()) {
+                        writeCount.wait(left.lowerBound(Duration.millis(10)).toMilliseconds());
+                    } else {
+                        throw new TimeoutException("Timeout waiting for independent write of rebind-periodic-delta, after "+timer.getDurationElapsed());
+                    }
+                }
+            }
         } else {
             // someone else has been writing for the entire time 
             throw new TimeoutException("Timeout waiting for completion of in-progress write of rebind-periodic-delta, after "+timer.getDurationElapsed());
@@ -425,6 +434,10 @@ public class PeriodicDeltaChangeListener implements ChangeListener {
                 LOG.debug("Problem persisting, but no longer active (ignoring)", e);
             }
         } finally {
+            synchronized (writeCount) {
+                writeCount.incrementAndGet();
+                writeCount.notifyAll();
+            }
             if (!alreadyHasMutex) persistingMutex.release();
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
index 6124a54..2d1e47f 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java
@@ -345,7 +345,6 @@ public abstract class RebindIteration {
         
         Collection<CatalogItem<?, ?>> catalogItems = rebindContext.getCatalogItems();
         CatalogInitialization catInit = ((ManagementContextInternal)managementContext).getCatalogInitialization();
-        catInit.injectManagementContext(managementContext);
         catInit.applyCatalogLoadMode();
         Collection<CatalogItem<?,?>> itemsForResettingCatalog = null;
         boolean needsInitialCatalog;
@@ -365,9 +364,7 @@ public abstract class RebindIteration {
                 itemsForResettingCatalog = MutableList.<CatalogItem<?,?>>of();
                 
                 PersisterDeltaImpl delta = new PersisterDeltaImpl();
-                for (String catalogItemId: mementoRawData.getCatalogItems().keySet()) {
-                    delta.removedCatalogItemIds.add(catalogItemId);
-                }
+                delta.removedCatalogItemIds.addAll(mementoRawData.getCatalogItems().keySet());
                 getPersister().queueDelta(delta);
                 
                 mementoRawData.clearCatalogItems();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
index caf04c5..ded0049 100644
--- a/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
+++ b/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
@@ -421,15 +421,9 @@ public class RebindManagerImpl implements RebindManager {
     
     @Override
     @VisibleForTesting
-    @Deprecated /** @deprecated since 0.7.0 use Duration as argument */
-    public void waitForPendingComplete(long timeout, TimeUnit unit) throws InterruptedException, TimeoutException {
-        waitForPendingComplete(Duration.of(timeout, unit));
-    }
-    @Override
-    @VisibleForTesting
-    public void waitForPendingComplete(Duration timeout) throws InterruptedException, TimeoutException {
+    public void waitForPendingComplete(Duration timeout, boolean canTrigger) throws InterruptedException, TimeoutException {
         if (persistenceStoreAccess == null || !persistenceRunning) return;
-        persistenceRealChangeListener.waitForPendingComplete(timeout);
+        persistenceRealChangeListener.waitForPendingComplete(timeout, canTrigger);
         persistenceStoreAccess.waitForWritesCompleted(timeout);
     }
     @Override

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
index 47eefdc..23e40e9 100644
--- a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
+++ b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java
@@ -565,7 +565,7 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer
 
         while (!queuedDeltas.isEmpty()) {
             Delta extraDelta = queuedDeltas.remove(0);
-            doDelta(extraDelta, exceptionHandler, false);
+            doDelta(extraDelta, exceptionHandler, true);
         }
 
         doDelta(delta, exceptionHandler, false);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java b/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
index 1cbe312..b1fdc0c 100644
--- a/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/AbstractManagementContext.java
@@ -82,6 +82,7 @@ import brooklyn.util.task.Tasks;
 
 import com.google.common.base.Function;
 import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
 
 public abstract class AbstractManagementContext implements ManagementContextInternal {
     private static final Logger log = LoggerFactory.getLogger(AbstractManagementContext.class);
@@ -441,10 +442,10 @@ public abstract class AbstractManagementContext implements ManagementContextInte
         return uri;
     }
     
+    private Object catalogInitMutex = new Object();
     @Override
     public CatalogInitialization getCatalogInitialization() {
-        if (catalogInitialization!=null) return catalogInitialization;
-        synchronized (this) {
+        synchronized (catalogInitMutex) {
             if (catalogInitialization!=null) return catalogInitialization;
             CatalogInitialization ci = new CatalogInitialization();
             setCatalogInitialization(ci);
@@ -453,9 +454,14 @@ public abstract class AbstractManagementContext implements ManagementContextInte
     }
     
     @Override
-    public synchronized void setCatalogInitialization(CatalogInitialization catalogInitialization) {
-        if (catalogInitialization!=null) catalogInitialization.injectManagementContext(this);
-        this.catalogInitialization = catalogInitialization;
+    public void setCatalogInitialization(CatalogInitialization catalogInitialization) {
+        synchronized (catalogInitMutex) {
+            Preconditions.checkNotNull(catalogInitialization, "initialization must not be null");
+            if (this.catalogInitialization!=null && this.catalogInitialization != catalogInitialization)
+                throw new IllegalStateException("Changing catalog init from "+this.catalogInitialization+" to "+catalogInitialization+"; changes not permitted");
+            catalogInitialization.injectManagementContext(this);
+            this.catalogInitialization = catalogInitialization;
+        }
     }
     
     public BrooklynObject lookup(String id) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java b/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
index 00639a5..b437561 100644
--- a/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
+++ b/core/src/main/java/brooklyn/management/internal/NonDeploymentManagementContext.java
@@ -27,7 +27,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import org.slf4j.Logger;
@@ -544,11 +543,7 @@ public class NonDeploymentManagementContext implements ManagementContextInternal
         }
 
         @Override
-        public void waitForPendingComplete(long timeout, TimeUnit unit) throws InterruptedException, TimeoutException {
-            throw new IllegalStateException("Non-deployment context "+NonDeploymentManagementContext.this+" is not valid for this operation.");
-        }
-        @Override
-        public void waitForPendingComplete(Duration timeout) throws InterruptedException, TimeoutException {
+        public void waitForPendingComplete(Duration timeout, boolean canTrigger) throws InterruptedException, TimeoutException {
             throw new IllegalStateException("Non-deployment context "+NonDeploymentManagementContext.this+" is not valid for this operation.");
         }
         @Override

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java b/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
index 07c40af..5b32bed 100644
--- a/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
+++ b/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
@@ -169,6 +169,7 @@ public class CatalogScanTest {
         int numFromAnnots = Iterables.size(annotsCatalog.getCatalogItems(Predicates.alwaysTrue()));
         
         Assert.assertEquals(numInDefault, numFromAnnots);
+        Assert.assertTrue(numInDefault>0, "Expected more than 0 entries");
     }
 
     // a simple test asserting no errors when listing the real catalog, and listing them for reference

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java b/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java
index f43cbe5..afeca2b 100644
--- a/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/ActivePartialRebindTest.java
@@ -83,7 +83,7 @@ public class ActivePartialRebindTest extends RebindTestFixtureWithApp {
     public void testRebindCheckingMemoryLeak() throws Exception {
         TestEntity c1 = origApp.addChild(EntitySpec.create(TestEntity.class));
         Entities.manage(c1);
-        c1.setConfig(TestEntity.CONF_NAME, Strings.makeRandomId(1000000));
+        c1.config().set(TestEntity.CONF_NAME, Strings.makeRandomId(1000000));
         
         gcAndLog("before");
         long used0 = Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/test/java/brooklyn/entity/rebind/RebindTestUtils.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/RebindTestUtils.java b/core/src/test/java/brooklyn/entity/rebind/RebindTestUtils.java
index 939d42e..ad0d407 100644
--- a/core/src/test/java/brooklyn/entity/rebind/RebindTestUtils.java
+++ b/core/src/test/java/brooklyn/entity/rebind/RebindTestUtils.java
@@ -431,7 +431,7 @@ public class RebindTestUtils {
     }
 
     public static void waitForPersisted(ManagementContext managementContext) throws InterruptedException, TimeoutException {
-        managementContext.getRebindManager().waitForPendingComplete(TIMEOUT);
+        managementContext.getRebindManager().waitForPendingComplete(TIMEOUT, true);
     }
 
     public static void checkCurrentMementoSerializable(Application app) throws Exception {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemorySizeIntegrationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemorySizeIntegrationTest.java b/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemorySizeIntegrationTest.java
index d2638cf..ac647c6 100644
--- a/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemorySizeIntegrationTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterInMemorySizeIntegrationTest.java
@@ -47,22 +47,29 @@ public class BrooklynMementoPersisterInMemorySizeIntegrationTest extends Brookly
     }
     
     public void testPersistenceVolumeFast() throws IOException, TimeoutException, InterruptedException {
-        doTestPersistenceVolume(50*1000, false);
+        doTestPersistenceVolume(50*1000, false, true);
     }
     @Test(groups="Integration",invocationCount=20)
     public void testPersistenceVolumeFastManyTimes() throws IOException, TimeoutException, InterruptedException {
-        doTestPersistenceVolume(50*1000, false);
+        doTestPersistenceVolume(50*1000, false, true);
     }
     @Test(groups="Integration")
     public void testPersistenceVolumeWaiting() throws IOException, TimeoutException, InterruptedException {
         // by waiting we ensure there aren't extra writes going on
-        doTestPersistenceVolume(50*1000, true);
+        doTestPersistenceVolume(50*1000, true, true);
+    }
+    public void testPersistenceVolumeFastNoTrigger() throws IOException, TimeoutException, InterruptedException {
+        doTestPersistenceVolume(50*1000, false, false);
+    }
+    @Test(groups="Integration",invocationCount=20)
+    public void testPersistenceVolumeFastNoTriggerManyTimes() throws IOException, TimeoutException, InterruptedException {
+        doTestPersistenceVolume(50*1000, false, false);
     }
     
-    protected void doTestPersistenceVolume(int bigBlockSize, boolean forceDelay) throws IOException, TimeoutException, InterruptedException {
+    protected void doTestPersistenceVolume(int bigBlockSize, boolean forceDelay, boolean canTrigger) throws IOException, TimeoutException, InterruptedException {
         if (forceDelay) Time.sleep(Duration.FIVE_SECONDS);
         else recorder.blockUntilDataWrittenExceeds(512, Duration.FIVE_SECONDS);
-        localManagementContext.getRebindManager().waitForPendingComplete(Duration.FIVE_SECONDS);
+        localManagementContext.getRebindManager().waitForPendingComplete(Duration.FIVE_SECONDS, canTrigger);
         
         long out1 = recorder.getBytesOut();
         int filesOut1 = recorder.getCountDataOut();
@@ -73,7 +80,7 @@ public class BrooklynMementoPersisterInMemorySizeIntegrationTest extends Brookly
         ((EntityInternal)app).setAttribute(TestEntity.NAME, "hello world");
         if (forceDelay) Time.sleep(Duration.FIVE_SECONDS);
         else recorder.blockUntilDataWrittenExceeds(out1+10, Duration.FIVE_SECONDS);
-        localManagementContext.getRebindManager().waitForPendingComplete(Duration.FIVE_SECONDS);
+        localManagementContext.getRebindManager().waitForPendingComplete(Duration.FIVE_SECONDS, canTrigger);
         
         long out2 = recorder.getBytesOut();
         Assert.assertTrue(out2-out1>10, "should have written more data");
@@ -86,7 +93,7 @@ public class BrooklynMementoPersisterInMemorySizeIntegrationTest extends Brookly
         ((EntityInternal)entity).setAttribute(TestEntity.NAME, Identifiers.makeRandomId(bigBlockSize));
         if (forceDelay) Time.sleep(Duration.FIVE_SECONDS);
         else recorder.blockUntilDataWrittenExceeds(out2+bigBlockSize, Duration.FIVE_SECONDS);
-        localManagementContext.getRebindManager().waitForPendingComplete(Duration.FIVE_SECONDS);
+        localManagementContext.getRebindManager().waitForPendingComplete(Duration.FIVE_SECONDS, canTrigger);
 
         long out3 = recorder.getBytesOut();
         Assert.assertTrue(out3-out2 > bigBlockSize, "should have written 50k more data, only wrote "+out3+" compared with "+out2);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java b/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java
index 967bb6d..7f4009d 100644
--- a/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java
+++ b/core/src/test/java/brooklyn/management/osgi/OsgiVersionMoreEntityTest.java
@@ -56,6 +56,7 @@ import brooklyn.util.guava.Maybe;
 import brooklyn.util.os.Os;
 import brooklyn.util.osgi.Osgis;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 
@@ -142,7 +143,8 @@ public class OsgiVersionMoreEntityTest {
     static CatalogItem<?, ?> addCatalogItemWithNameAndType(ManagementContext mgmt, String symName, String version, String type, String ...libraries) {
         CatalogEntityItemDto c1 = newCatalogItemWithNameAndType(symName, version, type, libraries);
         mgmt.getCatalog().addItem(c1);
-        CatalogItem<?, ?> c2 = mgmt.getCatalog().getCatalogItem(type, version);
+        CatalogItem<?, ?> c2 = mgmt.getCatalog().getCatalogItem(symName, version);
+        Preconditions.checkNotNull(c2, "Item "+type+":"+version+" was not found after adding it");
         return c2;
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/usage/cli/src/main/java/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java
index e6330ea..d98e0d8 100644
--- a/usage/cli/src/main/java/brooklyn/cli/Main.java
+++ b/usage/cli/src/main/java/brooklyn/cli/Main.java
@@ -286,8 +286,7 @@ public class Main extends AbstractMain {
         public boolean startBrooklynNode = false;
 
         // Note in some cases, you can get java.util.concurrent.RejectedExecutionException
-        // if shutdown is not co-ordinated, e.g. with Whirr:
-        // looks like: {@linktourl https://gist.github.com/47066f72d6f6f79b953e}
+        // if shutdown is not co-ordinated, looks like: {@linktourl https://gist.github.com/47066f72d6f6f79b953e}
         @Beta
         @Option(name = { "-sk", "--stopOnKeyPress" },
                 description = "Shutdown immediately on user text entry after startup (useful for debugging and demos)")

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index 3a87087..6a201a2 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -676,7 +676,9 @@ public class BrooklynLauncher {
             brooklynProperties.addFromMap(brooklynAdditionalProperties);
         }
         
-        ((ManagementContextInternal)managementContext).setCatalogInitialization(catalogInitialization);
+        if (catalogInitialization!=null) {
+            ((ManagementContextInternal)managementContext).setCatalogInitialization(catalogInitialization);
+        }
         
         if (customizeManagement!=null) {
             customizeManagement.apply(managementContext);
@@ -1009,7 +1011,7 @@ public class BrooklynLauncher {
                 if (managementContext.getHighAvailabilityManager().getPersister() != null) {
                     managementContext.getHighAvailabilityManager().getPersister().waitForWritesCompleted(Duration.TEN_SECONDS);
                 }
-                managementContext.getRebindManager().waitForPendingComplete(Duration.TEN_SECONDS);
+                managementContext.getRebindManager().waitForPendingComplete(Duration.TEN_SECONDS, true);
                 LOG.info("Finished waiting for persist; took "+Time.makeTimeStringRounded(stopwatch));
             } catch (RuntimeInterruptedException e) {
                 Thread.currentThread().interrupt(); // keep going with shutdown

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
index 03d51fc..abfb864 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java
@@ -176,11 +176,14 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv
                     if (!hasAppErrorsOrTimeout.get() || forceShutdownOnError) {
                         //give the http request a chance to complete gracefully
                         Time.sleep(delayForHttpReturn);
+                        
                         System.exit(0);
+                        
+                    } else {
+                        // There are app errors, don't exit the process, allowing any exception to continue throwing
+                        log.warn("Abandoning shutdown because there were errors and shutdown was not forced.");
+                        
                     }
-                    
-                    // There are app errors, don't exit the process, allowing any exception to continue throwing
-                    log.warn("Abandoning shutdown because there were errors and shutdown was not forced.");
                 }
             }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/usage/rest-server/src/test/java/brooklyn/rest/HaMasterCheckFilterTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/HaMasterCheckFilterTest.java b/usage/rest-server/src/test/java/brooklyn/rest/HaMasterCheckFilterTest.java
index 27d8d6c..eabeef1 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/HaMasterCheckFilterTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/HaMasterCheckFilterTest.java
@@ -152,7 +152,7 @@ System.err.println("TEAR DOWN");
 
         writeMgmt = createManagementContext(mementoDir, writeMode);
         appId = createApp(writeMgmt);
-        writeMgmt.getRebindManager().waitForPendingComplete(TIMEOUT);
+        writeMgmt.getRebindManager().waitForPendingComplete(TIMEOUT, true);
 
         if (readMode == HighAvailabilityMode.DISABLED) {
             //no HA, one node only

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d6ae0511/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java b/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
index 1a5dd93..a3ca374 100644
--- a/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
+++ b/utils/common/src/main/java/brooklyn/util/javalang/AggregateClassLoader.java
@@ -23,6 +23,7 @@ import java.net.URL;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -35,6 +36,8 @@ import com.google.common.collect.Sets;
  * exposing more info, a few conveniences, and a nice toString */
 public class AggregateClassLoader extends ClassLoader {
 
+    // thread safe -- and all access in this class is also synchronized, 
+    // so that reset is guaranteed not to interfere with an add(0, cl) 
     private final CopyOnWriteArrayList<ClassLoader> classLoaders = new CopyOnWriteArrayList<ClassLoader>();
 
     private AggregateClassLoader() {
@@ -58,21 +61,38 @@ public class AggregateClassLoader extends ClassLoader {
 
     /** Add a loader to the first position in the search path. */
     public void addFirst(ClassLoader classLoader) {
-        if (classLoader != null) classLoaders.add(0, classLoader);
+        if (classLoader != null) {
+            synchronized (classLoaders) {
+                classLoaders.add(0, classLoader);
+            }
+        }
     }
     /** Add a loader to the last position in the search path. */
     public void addLast(ClassLoader classLoader) {
-        if (classLoader != null) classLoaders.add(classLoader);
+        if (classLoader != null) {
+            synchronized (classLoaders) {
+                classLoaders.add(classLoader);
+            }
+        }
     }
     /** Add a loader to the specific position in the search path. 
      * (It is callers responsibility to ensure that position is valid.) */
     public void add(int index, ClassLoader classLoader) {
-        if (classLoader != null) classLoaders.add(index, classLoader);
+        if (classLoader != null) {
+            synchronized (classLoaders) {
+                classLoaders.add(index, classLoader);
+            }
+        }
     }
     
     /** Resets the classloader shown here to be the given set */
     public void reset(Collection<? extends ClassLoader> newClassLoaders) {
         synchronized (classLoaders) {
+            // synchronize:
+            // * to prevent concurrent invocations
+            // * so add(0, cl) doesn't interfere
+            // * and for good measure we add before removing so that iterator always contains everything
+            //   although since iterator access is synchronized that shouldn't be necessary
             int count = classLoaders.size();
             classLoaders.addAll(newClassLoaders);
             for (int i=0; i<count; i++) {
@@ -93,6 +113,13 @@ public class AggregateClassLoader extends ClassLoader {
         return classLoaders;
     }
 
+    public Iterator<ClassLoader> iterator() {
+        synchronized (classLoaders) {
+            // provides iterator of snapshot
+            return classLoaders.iterator();
+        }
+    }
+    
     @Override
     protected Class<?> findClass(String name) throws ClassNotFoundException {
         for (ClassLoader classLoader: classLoaders) {
@@ -117,7 +144,9 @@ public class AggregateClassLoader extends ClassLoader {
     @Override
     public URL getResource(String name) {
         URL result = null;
-        for (ClassLoader classLoader: classLoaders) {
+        Iterator<ClassLoader> cli = iterator();
+        while (cli.hasNext()) {
+            ClassLoader classLoader=cli.next();
             result = classLoader.getResource(name);
             if (result!=null) return result;
         }
@@ -131,7 +160,9 @@ public class AggregateClassLoader extends ClassLoader {
     @Override
     public Enumeration<URL> getResources(String name) throws IOException {
         Set<URL> resources = Sets.newLinkedHashSet();
-        for (ClassLoader classLoader : classLoaders) {
+        Iterator<ClassLoader> cli = iterator();
+        while (cli.hasNext()) {
+            ClassLoader classLoader=cli.next();
             resources.addAll(Collections.list(classLoader.getResources(name)));
         }
         return Collections.enumeration(resources);


[12/13] incubator-brooklyn git commit: This closes #663

Posted by he...@apache.org.
This closes #663


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/95d82bc7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/95d82bc7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/95d82bc7

Branch: refs/heads/master
Commit: 95d82bc73978c14fa8863dee1522a3122acbd5fd
Parents: b7a4d5e dbcc40a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri May 29 11:31:43 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri May 29 11:31:43 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/entity/basic/EntityInternal.java   |  3 +-
 .../java/brooklyn/rest/api/ApplicationApi.java  |  7 +-
 usage/rest-client/pom.xml                       | 37 ++--------
 .../java/brooklyn/rest/client/BrooklynApi.java  | 58 ++++++++++-----
 .../util/http/BuiltResponsePreservingError.java | 76 ++++++++++++++++++++
 .../ApplicationResourceIntegrationTest.java     | 19 ++---
 .../rest/client/BrooklynApiRestClientTest.java  | 66 ++++++++++++-----
 .../rest/resources/ApplicationResource.java     |  5 --
 .../rest/resources/CatalogResource.java         |  1 +
 .../rest/resources/EntityConfigResource.java    | 14 ++--
 .../rest/resources/PolicyConfigResource.java    |  2 +-
 .../provider/DelegatingSecurityProvider.java    | 10 ++-
 .../rest/transform/LocationTransformer.java     | 11 +--
 .../rest/util/BrooklynRestResourceUtils.java    |  5 +-
 .../rest/util/json/MultimapSerializer.java      |  4 +-
 .../BrooklynRestApiLauncherTestFixture.java     |  1 -
 .../brooklyn/rest/domain/ApplicationTest.java   |  2 +-
 .../rest/domain/LocationSummaryTest.java        |  1 +
 .../ApplicationResourceIntegrationTest.java     |  1 -
 .../resources/EntityConfigResourceTest.java     |  4 +-
 .../rest/resources/LocationResourceTest.java    |  6 +-
 .../rest/resources/ServerResourceTest.java      | 10 ++-
 .../rest/testing/mocks/EverythingGroupImpl.java |  2 +-
 .../testing/mocks/NameMatcherGroupImpl.java     |  2 +-
 .../json/BrooklynJacksonSerializerTest.java     | 10 +--
 25 files changed, 233 insertions(+), 124 deletions(-)
----------------------------------------------------------------------



[13/13] incubator-brooklyn git commit: This closes #656

Posted by he...@apache.org.
This closes #656


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/75194760
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/75194760
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/75194760

Branch: refs/heads/master
Commit: 75194760d8f50680f7de3afb69e5e83c97949116
Parents: 95d82bc b9b8a97
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri May 29 11:35:08 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri May 29 11:35:08 2015 +0100

----------------------------------------------------------------------
 usage/cli/src/main/java/brooklyn/cli/Main.java  | 22 +++++++++++++++-----
 .../brooklyn/launcher/BrooklynLauncher.java     | 10 +++++++--
 2 files changed, 25 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/75194760/usage/cli/src/main/java/brooklyn/cli/Main.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/75194760/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------


[11/13] incubator-brooklyn git commit: This closes #617

Posted by he...@apache.org.
This closes #617


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/b7a4d5ec
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/b7a4d5ec
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/b7a4d5ec

Branch: refs/heads/master
Commit: b7a4d5ec24045ab4f256f998c2d414296153ed29
Parents: 42e9aad 692e47c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Fri May 29 11:29:30 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri May 29 11:29:30 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/entity/rebind/RebindManager.java   |   7 +-
 .../mementos/BrooklynMementoPersister.java      |   1 +
 .../catalog/internal/BasicBrooklynCatalog.java  |  87 +++++--
 .../catalog/internal/CatalogClasspathDo.java    |  41 ++-
 .../brooklyn/catalog/internal/CatalogDo.java    |   1 +
 .../catalog/internal/CatalogInitialization.java | 250 ++++++++++++-------
 .../internal/CatalogItemDtoAbstract.java        |   4 +-
 .../rebind/PeriodicDeltaChangeListener.java     | 210 +++++++++-------
 .../entity/rebind/RebindContextImpl.java        |   6 +-
 .../brooklyn/entity/rebind/RebindIteration.java |  58 +++--
 .../entity/rebind/RebindManagerImpl.java        |  14 +-
 .../BrooklynMementoPersisterToObjectStore.java  |   2 +-
 .../ha/HighAvailabilityManagerImpl.java         |  67 +++--
 .../internal/AbstractManagementContext.java     |  20 +-
 .../NonDeploymentManagementContext.java         |   7 +-
 .../brooklyn/osgi/tests/more/MoreEntity.java    |   2 +
 .../catalog/internal/CatalogScanTest.java       |  22 +-
 .../entity/rebind/ActivePartialRebindTest.java  |   2 +-
 .../brooklyn/entity/rebind/RebindTestUtils.java |   2 +-
 ...ntoPersisterInMemorySizeIntegrationTest.java |  21 +-
 .../brooklyn/management/ha/HotStandbyTest.java  |  14 +-
 .../osgi/OsgiVersionMoreEntityTest.java         |   4 +-
 .../brooklyn-test-osgi-more-entities_0.2.0.jar  | Bin 12590 -> 13078 bytes
 docs/guide/ops/catalog/index.md                 |   5 +-
 .../BrooklynComponentTemplateResolver.java      |  12 +-
 .../camp/brooklyn/AbstractYamlTest.java         |  10 +-
 .../camp/brooklyn/ReferencedYamlTest.java       |   6 +-
 .../CatalogOsgiVersionMoreEntityTest.java       |  52 +++-
 .../brooklyn/catalog/CatalogYamlCombiTest.java  |   6 +-
 .../brooklyn/catalog/CatalogYamlEntityTest.java |  44 ++--
 .../catalog/CatalogYamlLocationTest.java        |   4 +-
 .../brooklyn/catalog/CatalogYamlPolicyTest.java |   6 +-
 .../catalog/CatalogYamlTemplateTest.java        |   2 +-
 .../catalog/CatalogYamlVersioningTest.java      |   4 +-
 .../more-entities-osgi-catalog-scan.yaml        |  32 +++
 usage/cli/src/main/java/brooklyn/cli/Main.java  |   7 +-
 .../java/brooklyn/cli/lister/ClassFinder.java   |   2 +-
 .../brooklyn/cli/lister/ItemDescriptors.java    |   1 -
 .../main/webapp/assets/js/view/ha-summary.js    |  98 +++++---
 .../brooklyn/launcher/BrooklynLauncher.java     |  19 +-
 .../rest/filter/HaHotCheckResourceFilter.java   |   2 +-
 .../rest/filter/HaMasterCheckFilter.java        |   2 +-
 .../brooklyn/rest/filter/LoggingFilter.java     |  14 +-
 .../brooklyn/rest/resources/ServerResource.java |  72 ++++--
 .../brooklyn/rest/HaMasterCheckFilterTest.java  |   2 +-
 .../util/javalang/AggregateClassLoader.java     |  41 ++-
 .../java/brooklyn/util/javalang/Threads.java    |  12 +-
 .../src/main/java/brooklyn/util/os/Os.java      |   6 +-
 48 files changed, 841 insertions(+), 462 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b7a4d5ec/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b7a4d5ec/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
index afed76f,de68213..057b770
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogDo.java
@@@ -278,9 -264,10 +278,10 @@@ public class CatalogDo 
          dto.catalogs.add(child);
          if (!isLoaded())
              return null;
 -        return loadCatalog(child);
 +        return loadCatalog(child, true);
      }
      
+     /** adds the given urls; filters out any nulls supplied */
      public synchronized void addToClasspath(String ...urls) {
          if (dto.classpath == null)
              dto.classpath = new CatalogClasspathDto();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b7a4d5ec/docs/guide/ops/catalog/index.md
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b7a4d5ec/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
----------------------------------------------------------------------
diff --cc usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
index e6ba010,af3d0b8..6e17bfb
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
@@@ -417,10 -417,10 +417,10 @@@ public class CatalogYamlEntityTest exte
                  "- type: " + SIMPLE_ENTITY_TYPE);
              fail();
          } catch (NullPointerException e) {
 -            Assert.assertEquals(e.getMessage(), "version");
 +            Assert.assertEquals(e.getMessage(), "both name and version are required");
          }
          try {
-             addCatalogItem(
+             addCatalogItems(
                  "brooklyn.catalog:",
                  "  id: my.catalog.app.id.non_existing.ref",
                  "  version: " + TEST_VERSION,


[04/13] incubator-brooklyn git commit: Startup option to exit once the app is deployed and running

Posted by he...@apache.org.
Startup option to exit once the app is deployed and running

* Add an option exitAndLeaveAppsRunningAfterStarting to exit the process once the --app entity is up and running
* Force a persist after launching is complete so that the app persistence is guaranteed before exiting
* Pull the startup complete notification before starting the --app entity so that the progress can be monitored in the console.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/b9b8a977
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/b9b8a977
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/b9b8a977

Branch: refs/heads/master
Commit: b9b8a977d61396f0f14133381d88cf6ce6b0a497
Parents: 776ad43
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri May 22 21:22:34 2015 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Fri May 22 21:50:16 2015 +0300

----------------------------------------------------------------------
 usage/cli/src/main/java/brooklyn/cli/Main.java  | 22 +++++++++++++++-----
 .../brooklyn/launcher/BrooklynLauncher.java     | 10 +++++++--
 2 files changed, 25 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b9b8a977/usage/cli/src/main/java/brooklyn/cli/Main.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/Main.java b/usage/cli/src/main/java/brooklyn/cli/Main.java
index d89703e..73d60f8 100644
--- a/usage/cli/src/main/java/brooklyn/cli/Main.java
+++ b/usage/cli/src/main/java/brooklyn/cli/Main.java
@@ -287,6 +287,11 @@ public class Main extends AbstractMain {
                 "allIfNotPersisted: stop all apps IF persistence is not enabled, otherwise leave all running")
         public String stopWhichAppsOnShutdown = STOP_THESE_IF_NOT_PERSISTED;
 
+        @Option(name = { "--exitAndLeaveAppsRunningAfterStarting" },
+                description = "Once the application to start (from --app) is running exit the process, leaving any entities running. "
+                    + "Can be used in combination with --persist auto --persistenceDir <custom folder location> to attach to the running app at a later time.")
+        public boolean exitAndLeaveAppsRunningAfterStarting = false;
+
         /** @deprecated since 0.7.0 see {@link #stopWhichAppsOnShutdown} */
         @Deprecated
         @Option(name = { "-ns", "--noShutdownOnExit" },
@@ -438,8 +443,9 @@ public class Main extends AbstractMain {
                 Entities.dumpInfo(launcher.getApplications());
             }
             
-            waitAfterLaunch(ctx);
-
+            if (!exitAndLeaveAppsRunningAfterStarting) {
+                waitAfterLaunch(ctx);
+            }
             return null;
         }
 
@@ -511,18 +517,23 @@ public class Main extends AbstractMain {
         }
         
         protected StopWhichAppsOnShutdown computeStopWhichAppsOnShutdown() {
+            boolean isDefault = STOP_THESE_IF_NOT_PERSISTED.equals(stopWhichAppsOnShutdown);
             if (noShutdownOnExit) {
-                if (STOP_THESE_IF_NOT_PERSISTED.equals(stopWhichAppsOnShutdown)) {
+                if (isDefault) {
                     // the default; assume it was not explicitly specified so no error
                     stopWhichAppsOnShutdown = STOP_NONE;
                     // but warn of deprecation
                     log.warn("Deprecated paramater `--noShutdownOnExit` detected; this will likely be removed in a future version; "
                         + "replace with `"+STOP_WHICH_APPS_ON_SHUTDOWN+" "+stopWhichAppsOnShutdown+"`");
+                    return StopWhichAppsOnShutdown.NONE;
                 } else {
                     throw new FatalConfigurationRuntimeException("Cannot specify both `--noShutdownOnExit` and `"+STOP_WHICH_APPS_ON_SHUTDOWN+"`");
                 }
+            } else if (exitAndLeaveAppsRunningAfterStarting && isDefault) {
+                return StopWhichAppsOnShutdown.NONE;
+            } else {
+                return Enums.valueOfIgnoreCase(StopWhichAppsOnShutdown.class, stopWhichAppsOnShutdown).get();
             }
-            return Enums.valueOfIgnoreCase(StopWhichAppsOnShutdown.class, stopWhichAppsOnShutdown).get();
         }
         
         @VisibleForTesting
@@ -766,7 +777,8 @@ public class Main extends AbstractMain {
                     .add("persist", persist)
                     .add("persistenceLocation", persistenceLocation)
                     .add("persistenceDir", persistenceDir)
-                    .add("highAvailability", highAvailability);
+                    .add("highAvailability", highAvailability)
+                    .add("exitAndLeaveAppsRunningAfterStarting", exitAndLeaveAppsRunningAfterStarting);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b9b8a977/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index 16f4f76..484c010 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -602,6 +602,10 @@ public class BrooklynLauncher {
         // resolution uses the catalog's classpath to scan for resolvers.)
         locations.addAll(managementContext.getLocationRegistry().resolve(locationSpecs));
 
+        // Already rebinded successfully, so previous apps are now available.
+        // Allow the startup to be visible in console for newly created apps.
+        ((LocalManagementContext)managementContext).noteStartupComplete();
+
         try {
             createApps();
             startApps();
@@ -617,8 +621,10 @@ public class BrooklynLauncher {
             }
         }
         
-        ((LocalManagementContext)managementContext).noteStartupComplete();
-
+        if (persistMode != PersistMode.DISABLED) {
+            // Make sure the new apps are persisted in case process exits immediately.
+            managementContext.getRebindManager().forcePersistNow(false, null);
+        }
         return this;
     }
 


[08/13] incubator-brooklyn git commit: improve rest client handling of unclosed "response" objects

Posted by he...@apache.org.
improve rest client handling of unclosed "response" objects

prevents the error that the client has not been closed,
also tests for this, and simplifies rest client test framework
(which had some complexities which are not needed);
a few other misc cleanups and code review also


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/dbcc40a7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/dbcc40a7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/dbcc40a7

Branch: refs/heads/master
Commit: dbcc40a7c26bb74f4d3fba6929bc5bd5e97c38ac
Parents: dbd0cdc
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 12:41:18 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 13:08:18 2015 +0100

----------------------------------------------------------------------
 usage/rest-client/pom.xml                       | 36 ++--------
 .../java/brooklyn/rest/client/BrooklynApi.java  | 37 ++++++++--
 .../util/http/BuiltResponsePreservingError.java | 76 ++++++++++++++++++++
 .../ApplicationResourceIntegrationTest.java     | 19 ++---
 .../rest/client/BrooklynApiRestClientTest.java  | 43 ++++++++---
 .../provider/DelegatingSecurityProvider.java    |  1 -
 6 files changed, 151 insertions(+), 61 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/pom.xml
----------------------------------------------------------------------
diff --git a/usage/rest-client/pom.xml b/usage/rest-client/pom.xml
index 70c48e2..af001aa 100644
--- a/usage/rest-client/pom.xml
+++ b/usage/rest-client/pom.xml
@@ -118,6 +118,13 @@
         </dependency>
         <dependency>
             <groupId>org.apache.brooklyn</groupId>
+            <artifactId>brooklyn-core</artifactId>
+            <version>${project.version}</version>
+            <classifier>tests</classifier>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.brooklyn</groupId>
             <artifactId>brooklyn-rest-server</artifactId>
             <version>${project.version}</version>
             <scope>test</scope>
@@ -143,33 +150,4 @@
         </dependency>
     </dependencies>
     
-    <build>
-        
-        <plugins>
-            <plugin>
-                <artifactId>maven-dependency-plugin</artifactId>
-                <executions>
-                    <execution>
-                        <id>prep-server</id>
-                        <phase>pre-integration-test</phase>
-                        <goals>
-                            <goal>unpack</goal>
-                        </goals>
-                        <configuration>
-                            <artifactItems>
-                                <artifactItem>
-                                    <groupId>${project.groupId}</groupId>
-                                    <artifactId>brooklyn-rest-server</artifactId>
-                                    <version>${brooklyn.version}</version>
-                                    <overWrite>true</overWrite>
-                                    <outputDirectory>target/test-rest-server</outputDirectory>
-                                </artifactItem>
-                            </artifactItems>
-                        </configuration>
-                    </execution>
-                </executions>
-            </plugin>
-        </plugins>
-    </build>
-
 </project>

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java b/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
index c813864..9765ecf 100644
--- a/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
+++ b/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
@@ -20,6 +20,10 @@ package brooklyn.rest.client;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.net.MalformedURLException;
 import java.net.URL;
 
@@ -52,6 +56,8 @@ import brooklyn.rest.api.SensorApi;
 import brooklyn.rest.api.ServerApi;
 import brooklyn.rest.api.UsageApi;
 import brooklyn.rest.api.VersionApi;
+import brooklyn.util.exceptions.Exceptions;
+import brooklyn.util.http.BuiltResponsePreservingError;
 
 /**
  * @author Adam Lowe
@@ -105,8 +111,29 @@ public class BrooklynApi {
         this.clientExecutor = checkNotNull(clientExecutor, "clientExecutor");
     }
 
+    @SuppressWarnings("unchecked")
     private <T> T proxy(Class<T> clazz) {
-        return ProxyFactory.create(clazz, target, clientExecutor);
+        final T result0 = ProxyFactory.create(clazz, target, clientExecutor);
+        return (T) Proxy.newProxyInstance(clazz.getClassLoader(), new Class<?>[] { clazz }, new InvocationHandler() {
+            @Override
+            public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+                Object result1;
+                try {
+                    result1 = method.invoke(result0, args);
+                } catch (Throwable e) {
+                    if (e instanceof InvocationTargetException) {
+                        // throw the original exception
+                        e = ((InvocationTargetException)e).getTargetException();
+                    }
+                    throw Exceptions.propagate(e);
+                }
+                if (result1 instanceof Response) {
+                    // wrap the original response so it self-closes
+                    result1 = BuiltResponsePreservingError.copyResponseAndClose((Response) result1);
+                }
+                return result1;
+            }
+        });
     }
 
     public ActivityApi getActivityApi() {
@@ -169,21 +196,19 @@ public class BrooklynApi {
         return proxy(AccessApi.class);
     }
 
-    @SuppressWarnings({ "unchecked", "rawtypes" })
     public static <T> T getEntity(Response response, Class<T> type) {
         if (!(response instanceof ClientResponse)) {
             throw new IllegalArgumentException("Response should be instance of ClientResponse, is: " + response.getClass());
         }
-        ClientResponse clientResponse = (ClientResponse) response;
+        ClientResponse<?> clientResponse = (ClientResponse<?>) response;
         return (T) clientResponse.getEntity(type);
     }
 
-    @SuppressWarnings({ "unchecked", "rawtypes" })
-    public static <T> T getEntityGeneric(Response response, GenericType type) {
+    public static <T> T getEntityGeneric(Response response, GenericType<T> type) {
         if (!(response instanceof ClientResponse)) {
             throw new IllegalArgumentException("Response should be instance of ClientResponse, is: " + response.getClass());
         }
-        ClientResponse clientResponse = (ClientResponse) response;
+        ClientResponse<?> clientResponse = (ClientResponse<?>) response;
         return (T) clientResponse.getEntity(type);
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java b/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java
new file mode 100644
index 0000000..1dffb58
--- /dev/null
+++ b/usage/rest-client/src/main/java/brooklyn/util/http/BuiltResponsePreservingError.java
@@ -0,0 +1,76 @@
+/*
+ * 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.http;
+
+import java.lang.annotation.Annotation;
+
+import javax.ws.rs.core.Response;
+
+import org.jboss.resteasy.core.Headers;
+import org.jboss.resteasy.specimpl.BuiltResponse;
+
+import brooklyn.util.exceptions.Exceptions;
+
+/** 
+ * Allows wrapping a {@link Response} with the stream fully read and closed so that the client can be re-used.
+ * <p>
+ * The entity may be stored as a string as type info is not available when it is deserialized, 
+ * and that's a relatively convenient common format.
+ *  
+ * TODO It would be nice to support other parsing, storing the byte array.
+ */
+public class BuiltResponsePreservingError extends BuiltResponse {
+
+    private Throwable error;
+
+    public BuiltResponsePreservingError(int status, Headers<Object> headers, Object entity, Annotation[] annotations, Throwable error) {
+        super(status, headers, entity, annotations);
+        this.error = error;
+    }
+    
+    @SuppressWarnings("deprecation")
+    public static Response copyResponseAndClose(Response source) {
+        int status = -1;
+        Headers<Object> headers = new Headers<Object>();
+        Object entity = null;
+        try {
+            status = source.getStatus();
+            headers.putAll(source.getHeaders());
+            if (source instanceof org.jboss.resteasy.client.ClientResponse) {
+                // ClientResponse requires strong type info, which we don't yet have
+                entity = ((org.jboss.resteasy.client.ClientResponse<?>)source).getEntity(String.class);
+            } else {
+                entity = source.getEntity();
+            }
+            return new BuiltResponsePreservingError(status, headers, entity, new Annotation[0], null);
+        } catch (Exception e) {
+            Exceptions.propagateIfFatal(e);
+            return new BuiltResponsePreservingError(status, headers, entity, new Annotation[0], e);
+        } finally {
+            source.close();
+        }
+    }
+    
+    @Override
+    public Object getEntity() {
+        if (error!=null) Exceptions.propagate(error);
+        return super.getEntity();
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java b/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
index e303571..a55aa45 100644
--- a/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
+++ b/usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java
@@ -27,7 +27,6 @@ import java.util.Collection;
 import javax.ws.rs.core.Response;
 
 import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.webapp.WebAppContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.Assert;
@@ -35,7 +34,6 @@ import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
-import brooklyn.config.BrooklynServiceAttributes;
 import brooklyn.entity.Application;
 import brooklyn.entity.basic.Lifecycle;
 import brooklyn.entity.basic.StartableApplication;
@@ -87,16 +85,8 @@ public class ApplicationResourceIntegrationTest {
 
     @BeforeClass(groups = "Integration")
     public void setUp() throws Exception {
-        WebAppContext context;
-
-        // running in source mode; need to use special classpath        
-        context = new WebAppContext("src/test/webapp", "/");
-        context.setExtraClasspath("./target/test-rest-server/");
-        context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, getManagementContext());
-
         Server server = BrooklynRestApiLauncher.launcher()
-                .managementContext(manager)
-                .customContext(context)
+                .managementContext(getManagementContext())
                 .start();
 
         api = new BrooklynApi("http://localhost:" + server.getConnectors()[0].getPort() + "/");
@@ -179,13 +169,12 @@ public class ApplicationResourceIntegrationTest {
                     } catch (Exception failure) {
                         // expected -- it will be a ClientResponseFailure but that class is deprecated so catching all
                         // and asserting contains the word 404
-                        Assert.assertTrue(failure.toString().indexOf("404") >= 0);
+                        Assert.assertTrue(failure.toString().indexOf("404") >= 0, "wrong failure, got: "+failure);
                     }
                 }});
         } catch (Exception failure) {
-            // expected -- it will be a ClientResponseFailure but that class is deprecated so catching all
-            // and asserting contains the word 404
-            Assert.assertTrue(failure.toString().indexOf("404") >= 0);
+            // expected -- as above
+            Assert.assertTrue(failure.toString().indexOf("404") >= 0, "wrong failure, got: "+failure);
         }
 
         assertEquals(getManagementContext().getApplications().size(), size - 1);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java b/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
index 9e5fbb3..7a320e7 100644
--- a/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
+++ b/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
@@ -21,15 +21,16 @@ package brooklyn.rest.client;
 import java.util.List;
 import java.util.Map;
 
+import javax.ws.rs.core.Response;
+
 import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.webapp.WebAppContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.Assert;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
-import brooklyn.config.BrooklynServiceAttributes;
 import brooklyn.entity.Application;
 import brooklyn.entity.basic.Entities;
 import brooklyn.entity.basic.StartableApplication;
@@ -41,6 +42,8 @@ import brooklyn.rest.BrooklynRestApiLauncherTest;
 import brooklyn.rest.domain.ApplicationSummary;
 import brooklyn.rest.domain.CatalogLocationSummary;
 import brooklyn.rest.security.provider.TestSecurityProvider;
+import brooklyn.test.HttpTestUtils;
+import brooklyn.test.entity.TestEntity;
 
 @Test
 public class BrooklynApiRestClientTest {
@@ -63,17 +66,9 @@ public class BrooklynApiRestClientTest {
 
     @BeforeClass
     public void setUp() throws Exception {
-        WebAppContext context;
-
-        // running in source mode; need to use special classpath        
-        context = new WebAppContext("src/test/webapp", "/");
-        context.setExtraClasspath("./target/test-rest-server/");
-        context.setAttribute(BrooklynServiceAttributes.BROOKLYN_MANAGEMENT_CONTEXT, getManagementContext());
-
         Server server = BrooklynRestApiLauncher.launcher()
                 .managementContext(manager)
                 .securityProvider(TestSecurityProvider.class)
-                .customContext(context)
                 .start();
 
         api = new BrooklynApi("http://localhost:" + server.getConnectors()[0].getPort() + "/",
@@ -107,5 +102,33 @@ public class BrooklynApiRestClientTest {
         List<ApplicationSummary> apps = api.getApplicationApi().list(null);
         log.info("apps are: "+apps);
     }
+    
+    public void testApplicationApiCreate() throws Exception {
+        Response r1 = api.getApplicationApi().createFromYaml("name: test-1234\n"
+            + "services: [ { type: "+TestEntity.class.getName()+" } ]");
+        HttpTestUtils.assertHealthyStatusCode(r1.getStatus());
+        log.info("creation result: "+r1.getEntity());
+        List<ApplicationSummary> apps = api.getApplicationApi().list(null);
+        log.info("apps with test: "+apps);
+        Assert.assertTrue(apps.toString().contains("test-1234"), "should have had test-1234 as an app; instead: "+apps);
+    }
+    
+    public void testApplicationApiHandledError() throws Exception {
+        Response r1 = api.getApplicationApi().createFromYaml("name: test");
+        Assert.assertTrue(r1.getStatus()/100 != 2, "needed an unhealthy status, not "+r1.getStatus());
+        Object entity = r1.getEntity();
+        Assert.assertTrue(entity.toString().indexOf("Unrecognized application blueprint format: no services defined")>=0,
+            "Missing expected text in response: "+entity.toString());
+    }
 
+    public void testApplicationApiThrownError() throws Exception {
+        try {
+            ApplicationSummary summary = api.getApplicationApi().get("test-5678");
+            Assert.fail("Should have thrown, not given: "+summary);
+        } catch (Exception e) {
+            e.printStackTrace();
+            Assert.assertTrue(e.toString().toLowerCase().contains("not found"),
+                "Missing expected text in response: "+e.toString());
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbcc40a7/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java b/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
index f4307ac..2291adb 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
@@ -106,7 +106,6 @@ public class DelegatingSecurityProvider implements SecurityProvider {
             }
         } catch (Exception e) {
             log.warn("REST unable to instantiate security provider " + className + "; all logins are being disallowed", e);
-            e.printStackTrace();
             delegate = new BlackholeSecurityProvider();
         }
         return delegate;


[09/13] incubator-brooklyn git commit: fix catalog scan test dependency on local drive and tidy up url handling for catalog items

Posted by he...@apache.org.
fix catalog scan test dependency on local drive and tidy up url handling for catalog items


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/c72fe8a0
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/c72fe8a0
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/c72fe8a0

Branch: refs/heads/master
Commit: c72fe8a0e1e566cc22955b69d8aee3fbe4c07a47
Parents: 1207d99
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 14:50:39 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 14:51:21 2015 +0100

----------------------------------------------------------------------
 .../catalog/internal/BasicBrooklynCatalog.java  |  9 +++++----
 .../catalog/internal/CatalogClasspathDo.java    | 12 ++++++-----
 .../catalog/internal/CatalogInitialization.java |  2 +-
 .../catalog/internal/CatalogScanTest.java       | 21 +++++++++++++++++---
 4 files changed, 31 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c72fe8a0/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
index bba0b73..7c192d0 100644
--- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
@@ -1195,11 +1195,12 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
                     // will be set by scan -- slightly longwinded way to retrieve, but scanning for osgi needs an overhaul in any case
                     Collection<CatalogBundle> libraryBundles = CatalogItemDtoAbstract.parseLibraries((Collection<?>) librariesCombined);
                     dto.setLibraries(libraryBundles);
-                    // must replace java type with plan yaml here for libraries / catalog item to be picked up
-                    dto.setSymbolicName(dto.getJavaType());
-                    dto.setPlanYaml("services: [{ type: "+dto.getJavaType()+" }]");
-                    dto.setJavaType(null);
                 }
+                // replace java type with plan yaml -- needed for libraries / catalog item to be picked up,
+                // but probably useful to transition away from javaType altogether
+                dto.setSymbolicName(dto.getJavaType());
+                dto.setPlanYaml("services: [{ type: "+dto.getJavaType()+" }]");
+                dto.setJavaType(null);
 
                 return dto;
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c72fe8a0/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
index 077abaa..a30a9dc 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
@@ -128,16 +128,18 @@ public class CatalogClasspathDo {
                         InputStream uin = ResourceUtils.create(this).getResourceFromUrl(u);
                         File f = Os.newTempFile("brooklyn-catalog-"+u, null);
                         FileOutputStream fout = new FileOutputStream(f);
-                        Streams.copy(uin, fout);
-                        fout.close();
-                        uin.close();
+                        try {
+                            Streams.copy(uin, fout);
+                        } finally {
+                            Streams.closeQuietly(fout);
+                            Streams.closeQuietly(uin);
+                        }
                         u = f.toURI().toString();
                     }
                     urls[i] = new URL(u);
                     
                     // TODO potential disk leak above as we have no way to know when the temp file can be removed earlier than server shutdown;
-                    // a better way to handle this is to supply a stream handler:
-//                    urls[i] = new URL(classpath.getEntries().get(i));
+                    // a better way to handle this is to supply a stream handler (but URLConnection is a little bit hard to work with):
 //                    URI uri = URI.create(classpath.getEntries().get(i));
 //                    urls[i] = new URL(uri.getScheme(), uri.getHost(), uri.getPort(), uri.getPath()  // TODO query fragment?
 //                        , new URLStreamHandler() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c72fe8a0/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
index 8f45a8c..51814ea 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogInitialization.java
@@ -157,7 +157,7 @@ public class CatalogInitialization implements ManagementContextInjectable {
      *   and so subsequent applications should ignore any initialization data (e.g. on a subsequent promotion to master, 
      *   after a master -> standby -> master cycle)
      * @param needsInitialItemsLoaded whether the catalog needs the initial items loaded
-     * @param needsInitialItemsLoaded whether the catalog needs the additiona items loaded
+     * @param needsAdditionalItemsLoaded whether the catalog needs the additions loaded
      * @param optionalExcplicitItemsForResettingCatalog
      *   if supplied, the catalog is reset to contain only these items, before calling any other initialization
      *   for use primarily when rebinding

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/c72fe8a0/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java b/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
index 5b32bed..101e306 100644
--- a/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
+++ b/core/src/test/java/brooklyn/catalog/internal/CatalogScanTest.java
@@ -37,6 +37,7 @@ import brooklyn.entity.Application;
 import brooklyn.entity.basic.Entities;
 import brooklyn.entity.proxying.EntitySpec;
 import brooklyn.management.internal.LocalManagementContext;
+import brooklyn.util.ResourceUtils;
 import brooklyn.util.net.Urls;
 import brooklyn.util.text.Strings;
 
@@ -75,10 +76,14 @@ public class CatalogScanTest {
         log.info("ENTITIES loaded for FULL: "+fullCatalog.getCatalogItems(Predicates.alwaysTrue()));
     }
     
-    private synchronized void loadTheDefaultCatalog() {
+    private synchronized void loadTheDefaultCatalog(boolean lookOnDiskForDefaultCatalog) {
         if (defaultCatalog!=null) return;
         BrooklynProperties props = BrooklynProperties.Factory.newEmpty();
-        props.put(BrooklynServerConfig.BROOKLYN_CATALOG_URL.getName(), "");
+        props.put(BrooklynServerConfig.BROOKLYN_CATALOG_URL.getName(),
+            // if default catalog is picked up from the system, we might get random stuff from ~/.brooklyn/ instead of the default;
+            // useful as an integration check that we default correctly, but irritating for people to use if they have such a catalog installed
+            (lookOnDiskForDefaultCatalog ? "" :
+                "data:,"+Urls.encode(new ResourceUtils(this).getResourceAsString("classpath:/brooklyn/default.catalog.bom"))));
         LocalManagementContext managementContext = newManagementContext(props);
         defaultCatalog = managementContext.getCatalog();        
         log.info("ENTITIES loaded for DEFAULT: "+defaultCatalog.getCatalogItems(Predicates.alwaysTrue()));
@@ -162,7 +167,17 @@ public class CatalogScanTest {
     
     @Test
     public void testAnnotationIsDefault() {
-        loadTheDefaultCatalog();
+        doTestAnnotationIsDefault(false);
+    }
+    
+    // see comment in load method; likely fails if a custom catalog is installed in ~/.brooklyn/
+    @Test(groups="Integration", enabled=false)
+    public void testAnnotationIsDefaultOnDisk() {
+        doTestAnnotationIsDefault(true);
+    }
+
+    private void doTestAnnotationIsDefault(boolean lookOnDiskForDefaultCatalog) {
+        loadTheDefaultCatalog(false);
         int numInDefault = Iterables.size(defaultCatalog.getCatalogItems(Predicates.alwaysTrue()));
         
         loadAnnotationsOnlyCatalog();


[03/13] incubator-brooklyn git commit: Explanatory notes on scanning and OSGi

Posted by he...@apache.org.
Explanatory notes on scanning and OSGi


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/9cc7293e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/9cc7293e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/9cc7293e

Branch: refs/heads/master
Commit: 9cc7293eb45efd045edd81793db31fa8b6f64110
Parents: d6ae051
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed May 20 15:07:07 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed May 20 15:07:24 2015 +0100

----------------------------------------------------------------------
 .../catalog/internal/BasicBrooklynCatalog.java  | 25 ++++++++++++++------
 .../catalog/internal/CatalogClasspathDo.java    |  1 +
 docs/guide/ops/catalog/index.md                 |  5 ++--
 .../brooklyn/cli/lister/ItemDescriptors.java    |  1 -
 4 files changed, 22 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9cc7293e/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
index 88e5818..8c88974 100644
--- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
+++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java
@@ -786,19 +786,30 @@ public class BasicBrooklynCatalog implements BrooklynCatalog {
     }
     
     private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsFromBundles(ManagementContext mgmt, Collection<CatalogBundle> libraries) {
-        String[] urls = null;
         CatalogDto dto = CatalogDto.newNamedInstance("Bundles Scanned Catalog", "All annotated Brooklyn entities detected in the classpath", "scanning-bundles-classpath-"+libraries.hashCode());
-        urls = new String[libraries.size()];
-        int i=0;
-        for (CatalogBundle b: libraries)
-            urls[i++] = b.getUrl();
-            
+        List<String> urls = MutableList.of();
+        for (CatalogBundle b: libraries) {
+            // TODO currently does not support pre-installed bundles identified by name:version 
+            // (ie where URL not supplied)
+            if (Strings.isNonBlank(b.getUrl())) {
+                urls.add(b.getUrl());
+            }
+        }
+        
+        if (urls.isEmpty()) {
+            log.warn("No bundles to scan: scanJavaAnnotations currently only applies to OSGi bundles provided by URL"); 
+            return MutableList.of();
+        }
+        
         CatalogDo subCatalog = new CatalogDo(dto);
-        subCatalog.addToClasspath(urls);
+        subCatalog.addToClasspath(urls.toArray(new String[0]));
         return scanAnnotationsInternal(mgmt, subCatalog);
     }
     
     private Collection<CatalogItemDtoAbstract<?, ?>> scanAnnotationsInternal(ManagementContext mgmt, CatalogDo subCatalog) {
+        // TODO this does java-scanning only;
+        // the call when scanning bundles should use the CatalogItem instead and use OSGi when loading for scanning
+        // (or another scanning mechanism).  see comments on CatalogClasspathDo.load
         subCatalog.mgmt = mgmt;
         subCatalog.setClasspathScanForEntities(CatalogScanningModes.ANNOTATIONS);
         subCatalog.load();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9cc7293e/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
index 095ebfd..c0716c8 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
@@ -105,6 +105,7 @@ public class CatalogClasspathDo {
     
     /** causes all scanning-based classpaths to scan the classpaths
     * (but does _not_ load all JARs) */
+    // TODO this does a Java scan; we also need an OSGi scan which uses the OSGi classloaders when loading for scanning and resolving dependencies 
     synchronized void load() {
         if (classpath == null || isLoaded) return;
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9cc7293e/docs/guide/ops/catalog/index.md
----------------------------------------------------------------------
diff --git a/docs/guide/ops/catalog/index.md b/docs/guide/ops/catalog/index.md
index 7cb0e84..a16a453 100644
--- a/docs/guide/ops/catalog/index.md
+++ b/docs/guide/ops/catalog/index.md
@@ -106,10 +106,11 @@ The following optional catalog metadata is supported:
   Icons are instead typically installed either at the server from which the OSGi bundles or catalog items are supplied 
   or in the `conf` folder of the Brooklyn distro.
 - `scanJavaAnnotations` [experimental]: if provided (as `true`), this will scan any locally provided
-  libraries for types annotated `@Catalog` and extract metadata to include them as catalog items.
+  library URLs for types annotated `@Catalog` and extract metadata to include them as catalog items.
   If no libraries are specified this will scan the default classpath.
   This feature is experimental and may change or be removed.
-  Also note that other metadata (such as versions, etc) may not be applied.
+  Also note that external OSGi dependencies are not supported 
+  and other metadata (such as versions, etc) may not be applied.
 - `brooklyn.libraries`: a list of pointers to OSGi bundles required for the catalog item.
   This can be omitted if blueprints are pure YAML and everything required is included in the classpath and catalog.
   Where custom Java code or bundled resources is needed, however, OSGi JARs supply

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9cc7293e/usage/cli/src/main/java/brooklyn/cli/lister/ItemDescriptors.java
----------------------------------------------------------------------
diff --git a/usage/cli/src/main/java/brooklyn/cli/lister/ItemDescriptors.java b/usage/cli/src/main/java/brooklyn/cli/lister/ItemDescriptors.java
index 989a29d..097e3b0 100644
--- a/usage/cli/src/main/java/brooklyn/cli/lister/ItemDescriptors.java
+++ b/usage/cli/src/main/java/brooklyn/cli/lister/ItemDescriptors.java
@@ -45,7 +45,6 @@ import brooklyn.rest.domain.SummaryComparators;
 import brooklyn.rest.transform.EffectorTransformer;
 import brooklyn.rest.transform.EntityTransformer;
 import brooklyn.rest.transform.SensorTransformer;
-import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.exceptions.RuntimeInterruptedException;
 
 import com.google.common.base.Strings;


[07/13] incubator-brooklyn git commit: fix and test better rest client

Posted by he...@apache.org.
fix and test better rest client

other misc rest cleanups


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/dbd0cdc4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/dbd0cdc4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/dbd0cdc4

Branch: refs/heads/master
Commit: dbd0cdc40912c60c69c7bf41eb5dfb3170dbc646
Parents: 42e9aad
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed May 27 19:10:37 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 13:07:29 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/entity/basic/EntityInternal.java   |  3 ++-
 .../java/brooklyn/rest/api/ApplicationApi.java  |  7 ++++--
 usage/rest-client/pom.xml                       |  1 -
 .../java/brooklyn/rest/client/BrooklynApi.java  | 25 ++++++++++----------
 .../rest/client/BrooklynApiRestClientTest.java  | 23 +++++++++++++-----
 .../rest/resources/ApplicationResource.java     |  5 ----
 .../rest/resources/CatalogResource.java         |  1 +
 .../rest/resources/EntityConfigResource.java    | 14 +++++------
 .../rest/resources/PolicyConfigResource.java    |  2 +-
 .../provider/DelegatingSecurityProvider.java    | 11 ++++++++-
 .../rest/transform/LocationTransformer.java     | 11 +++++----
 .../rest/util/BrooklynRestResourceUtils.java    |  5 ++--
 .../rest/util/json/MultimapSerializer.java      |  4 ++--
 .../BrooklynRestApiLauncherTestFixture.java     |  1 -
 .../brooklyn/rest/domain/ApplicationTest.java   |  2 +-
 .../rest/domain/LocationSummaryTest.java        |  1 +
 .../ApplicationResourceIntegrationTest.java     |  1 -
 .../resources/EntityConfigResourceTest.java     |  4 ++--
 .../rest/resources/LocationResourceTest.java    |  6 +++--
 .../rest/resources/ServerResourceTest.java      | 10 ++++----
 .../rest/testing/mocks/EverythingGroupImpl.java |  2 +-
 .../testing/mocks/NameMatcherGroupImpl.java     |  2 +-
 .../json/BrooklynJacksonSerializerTest.java     | 10 ++++----
 23 files changed, 85 insertions(+), 66 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/core/src/main/java/brooklyn/entity/basic/EntityInternal.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/EntityInternal.java b/core/src/main/java/brooklyn/entity/basic/EntityInternal.java
index ba8df78..b0e517e 100644
--- a/core/src/main/java/brooklyn/entity/basic/EntityInternal.java
+++ b/core/src/main/java/brooklyn/entity/basic/EntityInternal.java
@@ -66,7 +66,8 @@ public interface EntityInternal extends BrooklynObjectInternal, EntityLocal, Reb
     /**
      * @return a read-only copy of all the config key/value pairs on this entity.
      * 
-     * @deprecated since 0.7.0; instead just use methods on {@link ConfigurationSupportInternal} returned by {@link #config()}
+     * @deprecated since 0.7.0; instead just use methods on {@link ConfigurationSupportInternal} returned by {@link #config()},
+     * e.g. getBag().getAllConfigAsConfigKeyMap().
      */
     @Deprecated
     @Beta

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-api/src/main/java/brooklyn/rest/api/ApplicationApi.java
----------------------------------------------------------------------
diff --git a/usage/rest-api/src/main/java/brooklyn/rest/api/ApplicationApi.java b/usage/rest-api/src/main/java/brooklyn/rest/api/ApplicationApi.java
index 865024a..7da543b 100644
--- a/usage/rest-api/src/main/java/brooklyn/rest/api/ApplicationApi.java
+++ b/usage/rest-api/src/main/java/brooklyn/rest/api/ApplicationApi.java
@@ -81,8 +81,11 @@ public interface ApplicationApi {
             @DefaultValue(".*")
             @QueryParam("typeRegex") String typeRegex);
 
-    /** As {@link #list(String)}, filtering for <code>.*</code>. */
-    public List<ApplicationSummary> list();
+    // would be nice to have this on the API so default type regex not needed, but
+    // not yet implemented, as per: https://issues.jboss.org/browse/RESTEASY-798
+    // (this method was added to this class, but it breaks the rest client)
+//    /** As {@link #list(String)}, filtering for <code>.*</code>. */
+//    public List<ApplicationSummary> list();
 
     @GET
     @Path("/{application}")

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-client/pom.xml
----------------------------------------------------------------------
diff --git a/usage/rest-client/pom.xml b/usage/rest-client/pom.xml
index f39903c..70c48e2 100644
--- a/usage/rest-client/pom.xml
+++ b/usage/rest-client/pom.xml
@@ -148,7 +148,6 @@
         <plugins>
             <plugin>
                 <artifactId>maven-dependency-plugin</artifactId>
-                <version>${maven-dependency-plugin.version}</version>
                 <executions>
                     <execution>
                         <id>prep-server</id>

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java b/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
index 7bcff46..c813864 100644
--- a/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
+++ b/usage/rest-client/src/main/java/brooklyn/rest/client/BrooklynApi.java
@@ -22,9 +22,13 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.net.MalformedURLException;
 import java.net.URL;
+
+import javax.annotation.Nullable;
 import javax.ws.rs.core.Response;
 
-import org.apache.commons.codec.binary.Base64;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.Credentials;
+import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.impl.client.DefaultHttpClient;
 import org.jboss.resteasy.client.ClientExecutor;
 import org.jboss.resteasy.client.ClientRequest;
@@ -33,8 +37,6 @@ import org.jboss.resteasy.client.ProxyFactory;
 import org.jboss.resteasy.client.core.executors.ApacheHttpClient4Executor;
 import org.jboss.resteasy.util.GenericType;
 
-import com.google.common.base.Charsets;
-
 import brooklyn.rest.api.AccessApi;
 import brooklyn.rest.api.ActivityApi;
 import brooklyn.rest.api.ApplicationApi;
@@ -50,9 +52,6 @@ import brooklyn.rest.api.SensorApi;
 import brooklyn.rest.api.ServerApi;
 import brooklyn.rest.api.UsageApi;
 import brooklyn.rest.api.VersionApi;
-import org.apache.http.auth.AuthScope;
-import org.apache.http.auth.Credentials;
-import org.apache.http.auth.UsernamePasswordCredentials;
 
 /**
  * @author Adam Lowe
@@ -68,7 +67,8 @@ public class BrooklynApi {
     }
 
     public BrooklynApi(String endpoint) {
-        this(endpoint, null, null);
+        // username/password cannot be null, but credentials can
+        this(endpoint, null);
     }
 
     public BrooklynApi(URL endpoint, String username, String password) {
@@ -79,14 +79,13 @@ public class BrooklynApi {
         this(endpoint, new UsernamePasswordCredentials(username, password));
     }
 
-    public BrooklynApi(URL endpoint, Credentials credentials) {
+    public BrooklynApi(URL endpoint, @Nullable Credentials credentials) {
         this(endpoint.toString(), credentials);
     }
 
-    public BrooklynApi(String endpoint, Credentials credentials) {
-        URL target = null;
+    public BrooklynApi(String endpoint, @Nullable Credentials credentials) {
         try {
-            target = new URL(checkNotNull(endpoint, "endpoint"));
+            new URL(checkNotNull(endpoint, "endpoint"));
         } catch (MalformedURLException e) {
             throw new IllegalArgumentException(e);
         }
@@ -170,7 +169,7 @@ public class BrooklynApi {
         return proxy(AccessApi.class);
     }
 
-    @SuppressWarnings("unchecked")
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     public static <T> T getEntity(Response response, Class<T> type) {
         if (!(response instanceof ClientResponse)) {
             throw new IllegalArgumentException("Response should be instance of ClientResponse, is: " + response.getClass());
@@ -179,7 +178,7 @@ public class BrooklynApi {
         return (T) clientResponse.getEntity(type);
     }
 
-    @SuppressWarnings("unchecked")
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     public static <T> T getEntityGeneric(Response response, GenericType type) {
         if (!(response instanceof ClientResponse)) {
             throw new IllegalArgumentException("Response should be instance of ClientResponse, is: " + response.getClass());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java b/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
index b67b972..9e5fbb3 100644
--- a/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
+++ b/usage/rest-client/src/test/java/brooklyn/rest/client/BrooklynApiRestClientTest.java
@@ -19,6 +19,7 @@
 package brooklyn.rest.client;
 
 import java.util.List;
+import java.util.Map;
 
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.webapp.WebAppContext;
@@ -37,9 +38,8 @@ import brooklyn.management.ManagementContext;
 import brooklyn.management.internal.LocalManagementContext;
 import brooklyn.rest.BrooklynRestApiLauncher;
 import brooklyn.rest.BrooklynRestApiLauncherTest;
-import brooklyn.rest.domain.LocationSummary;
-import brooklyn.rest.security.provider.ExplicitUsersSecurityProvider;
-import brooklyn.rest.security.provider.SecurityProvider;
+import brooklyn.rest.domain.ApplicationSummary;
+import brooklyn.rest.domain.CatalogLocationSummary;
 import brooklyn.rest.security.provider.TestSecurityProvider;
 
 @Test
@@ -92,9 +92,20 @@ public class BrooklynApiRestClientTest {
         Entities.destroyAll(getManagementContext());
     }
 
-    public void testListLocations() throws Exception {
-        List<LocationSummary> locations = api.getLocationApi().list();
-        log.info("locations are: "+locations);
+    public void testLocationApi() throws Exception {
+        log.info("Testing location API");
+        Map<String, Map<String, Object>> locations = api.getLocationApi().getLocatedLocations();
+        log.info("locations located are: "+locations);
+    }
+
+    public void testCatalogApiLocations() throws Exception {
+        List<CatalogLocationSummary> locations = api.getCatalogApi().listLocations(".*", null, false);
+        log.info("locations from catalog are: "+locations);
+    }
+
+    public void testApplicationApiList() throws Exception {
+        List<ApplicationSummary> apps = api.getApplicationApi().list(null);
+        log.info("apps are: "+apps);
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/resources/ApplicationResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ApplicationResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ApplicationResource.java
index 0144d56..89d59e5 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ApplicationResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ApplicationResource.java
@@ -218,11 +218,6 @@ public class ApplicationResource extends AbstractBrooklynRestResource implements
     }
 
     @Override
-    public List<ApplicationSummary> list() {
-        return list(".*");
-    }
-
-    @Override
     public List<ApplicationSummary> list(String typeRegex) {
         if (Strings.isBlank(typeRegex)) {
             typeRegex = ".*";

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
index 5332bb0..1e1bc28 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
@@ -126,6 +126,7 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat
         return Response.status(Status.CREATED).entity(result).build();
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public Response resetXml(String xml, boolean ignoreErrors) {
         if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.MODIFY_CATALOG_ITEM, null) ||

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/resources/EntityConfigResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/EntityConfigResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/EntityConfigResource.java
index 2c3ceb6..0984ebf 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/EntityConfigResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/EntityConfigResource.java
@@ -72,9 +72,9 @@ public class EntityConfigResource extends AbstractBrooklynRestResource implement
     public Map<String, Object> batchConfigRead(String application, String entityToken, Boolean raw) {
         // TODO: add test
         EntityLocal entity = brooklyn().getEntity(application, entityToken);
-        Map<ConfigKey<?>, Object> source = ((EntityInternal) entity).getAllConfig();
+        Map<ConfigKey<?>, ?> source = ((EntityInternal) entity).config().getBag().getAllConfigAsConfigKeyMap();
         Map<String, Object> result = Maps.newLinkedHashMap();
-        for (Map.Entry<ConfigKey<?>, Object> ek : source.entrySet()) {
+        for (Map.Entry<ConfigKey<?>, ?> ek : source.entrySet()) {
             Object value = ek.getValue();
             if (Boolean.FALSE.equals(raw)) {
                 value = RendererHints.applyDisplayValueHint(ek.getKey(), value);
@@ -97,7 +97,7 @@ public class EntityConfigResource extends AbstractBrooklynRestResource implement
     public Object get(boolean preferJson, String application, String entityToken, String configKeyName, Boolean raw) {
         EntityLocal entity = brooklyn().getEntity(application, entityToken);
         ConfigKey<?> ck = findConfig(entity, configKeyName);
-        Object value = entity.getConfigRaw(ck, true).orNull();
+        Object value = ((EntityInternal)entity).config().getRaw(ck).orNull();
         if (Boolean.FALSE.equals(raw)) {
             value = RendererHints.applyDisplayValueHint(ck, value);
         }
@@ -127,10 +127,10 @@ public class EntityConfigResource extends AbstractBrooklynRestResource implement
             Object newValue = ((Map.Entry) entry).getValue();
 
             ConfigKey ck = findConfig(entity, configName);
-            ((EntityInternal) entity).setConfig(ck, TypeCoercions.coerce(newValue, ck.getTypeToken()));
+            ((EntityInternal) entity).config().set(ck, TypeCoercions.coerce(newValue, ck.getTypeToken()));
             if (Boolean.TRUE.equals(recurse)) {
                 for (Entity e2 : Entities.descendants(entity, Predicates.alwaysTrue(), false)) {
-                    ((EntityInternal) e2).setConfig(ck, newValue);
+                    ((EntityInternal) e2).config().set(ck, newValue);
                 }
             }
         }
@@ -147,10 +147,10 @@ public class EntityConfigResource extends AbstractBrooklynRestResource implement
 
         ConfigKey ck = findConfig(entity, configName);
         LOG.debug("REST setting config " + configName + " on " + entity + " to " + newValue);
-        ((EntityInternal) entity).setConfig(ck, TypeCoercions.coerce(newValue, ck.getTypeToken()));
+        ((EntityInternal) entity).config().set(ck, TypeCoercions.coerce(newValue, ck.getTypeToken()));
         if (Boolean.TRUE.equals(recurse)) {
             for (Entity e2 : Entities.descendants(entity, Predicates.alwaysTrue(), false)) {
-                ((EntityInternal) e2).setConfig(ck, newValue);
+                ((EntityInternal) e2).config().set(ck, newValue);
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/resources/PolicyConfigResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/PolicyConfigResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/PolicyConfigResource.java
index f9493ea..471cd22 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/PolicyConfigResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/PolicyConfigResource.java
@@ -98,7 +98,7 @@ public class PolicyConfigResource extends AbstractBrooklynRestResource implement
         ConfigKey<?> ck = policy.getPolicyType().getConfigKey(configKeyName);
         if (ck == null) throw WebResourceUtils.notFound("Cannot find config key '%s' in policy '%s' of entity '%s'", configKeyName, policy, entityToken);
 
-        policy.setConfig((ConfigKey) ck, TypeCoercions.coerce(value, ck.getTypeToken()));
+        policy.config().set((ConfigKey) ck, TypeCoercions.coerce(value, ck.getTypeToken()));
 
         return Response.status(Response.Status.OK).build();
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java b/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
index e7669ba..f4307ac 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/security/provider/DelegatingSecurityProvider.java
@@ -20,6 +20,7 @@ package brooklyn.rest.security.provider;
 
 import java.lang.reflect.Constructor;
 import java.util.concurrent.atomic.AtomicLong;
+
 import javax.servlet.http.HttpSession;
 
 import org.slf4j.Logger;
@@ -44,6 +45,8 @@ public class DelegatingSecurityProvider implements SecurityProvider {
     private final AtomicLong modCount = new AtomicLong();
 
     private class PropertiesListener implements ManagementContext.PropertiesReloadListener {
+        private static final long serialVersionUID = 8148722609022378917L;
+
         @Override
         public void reloaded() {
             log.debug("{} reloading security provider", DelegatingSecurityProvider.this);
@@ -94,10 +97,16 @@ public class DelegatingSecurityProvider implements SecurityProvider {
                 delegate = constructor.newInstance(mgmt);
             } catch (Exception e) {
                 constructor = clazz.getConstructor();
-                delegate = constructor.newInstance();
+                Object delegateO = constructor.newInstance();
+                if (!(delegateO instanceof SecurityProvider)) {
+                    // if classloaders get mangled it will be a different CL's SecurityProvider
+                    throw new ClassCastException("Delegate is either not a security provider or has an incompatible classloader: "+delegateO);
+                }
+                delegate = (SecurityProvider) delegateO;
             }
         } catch (Exception e) {
             log.warn("REST unable to instantiate security provider " + className + "; all logins are being disallowed", e);
+            e.printStackTrace();
             delegate = new BlackholeSecurityProvider();
         }
         return delegate;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/transform/LocationTransformer.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/transform/LocationTransformer.java b/usage/rest-server/src/main/java/brooklyn/rest/transform/LocationTransformer.java
index fbf0b7d..6df1d4f 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/transform/LocationTransformer.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/transform/LocationTransformer.java
@@ -24,14 +24,13 @@ import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.Sanitizer;
 import brooklyn.location.Location;
 import brooklyn.location.LocationDefinition;
 import brooklyn.location.basic.BasicLocationDefinition;
 import brooklyn.location.basic.LocationConfigKeys;
 import brooklyn.location.basic.LocationInternal;
 import brooklyn.management.ManagementContext;
-import brooklyn.rest.domain.LocationSpec;
 import brooklyn.rest.domain.LocationSummary;
 import brooklyn.rest.util.WebResourceUtils;
 import brooklyn.util.collections.MutableMap;
@@ -43,16 +42,18 @@ import com.google.common.collect.ImmutableMap;
 
 public class LocationTransformer {
 
+    @SuppressWarnings("unused")
     private static final Logger log = LoggerFactory.getLogger(LocationTransformer.LocationDetailLevel.class);
     
     public static enum LocationDetailLevel { NONE, LOCAL_EXCLUDING_SECRET, FULL_EXCLUDING_SECRET, FULL_INCLUDING_SECRET }
     
     /** @deprecated since 0.7.0 use method taking management context and detail specifier */
     @Deprecated
-    public static LocationSummary newInstance(String id, LocationSpec locationSpec) {
+    public static LocationSummary newInstance(String id, brooklyn.rest.domain.LocationSpec locationSpec) {
         return newInstance(null, id, locationSpec, LocationDetailLevel.LOCAL_EXCLUDING_SECRET);
     }
-    public static LocationSummary newInstance(ManagementContext mgmt, String id, LocationSpec locationSpec, LocationDetailLevel level) {
+    @SuppressWarnings("deprecation")
+    public static LocationSummary newInstance(ManagementContext mgmt, String id, brooklyn.rest.domain.LocationSpec locationSpec, LocationDetailLevel level) {
         // TODO: Remove null checks on mgmt when newInstance(String, LocationSpec) is deleted
         Map<String, ?> config = locationSpec.getConfig();
         if (mgmt != null && (level==LocationDetailLevel.FULL_EXCLUDING_SECRET || level==LocationDetailLevel.FULL_INCLUDING_SECRET)) {
@@ -123,7 +124,7 @@ public class LocationTransformer {
         ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
         if (level!=LocationDetailLevel.NONE) {
             for (Map.Entry<String,?> entry : entries.entrySet()) {
-                if (level==LocationDetailLevel.FULL_INCLUDING_SECRET || !Entities.isSecret(entry.getKey())) {
+                if (level==LocationDetailLevel.FULL_INCLUDING_SECRET || !Sanitizer.IS_SECRET_PREDICATE.apply(entry.getKey())) {
                     builder.put(entry.getKey(), WebResourceUtils.getValueForDisplay(entry.getValue(), true, false));
                 }
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/util/BrooklynRestResourceUtils.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/util/BrooklynRestResourceUtils.java b/usage/rest-server/src/main/java/brooklyn/rest/util/BrooklynRestResourceUtils.java
index c3499b8..eef55f2 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/util/BrooklynRestResourceUtils.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/util/BrooklynRestResourceUtils.java
@@ -205,7 +205,7 @@ public class BrooklynRestResourceUtils {
         return null;
     }
 
-    @SuppressWarnings("unchecked")
+    @SuppressWarnings({ "unchecked", "deprecation" })
     public Application create(ApplicationSpec spec) {
         log.debug("REST creating application instance for {}", spec);
         
@@ -332,7 +332,7 @@ public class BrooklynRestResourceUtils {
         return locations;
     }
 
-    @SuppressWarnings("unchecked")
+    @SuppressWarnings({ "unchecked", "deprecation" })
     private brooklyn.entity.proxying.EntitySpec<? extends Entity> toCoreEntitySpec(brooklyn.rest.domain.EntitySpec spec) {
         String type = spec.getType();
         String name = spec.getName();
@@ -466,6 +466,7 @@ public class BrooklynRestResourceUtils {
     @SuppressWarnings({ "rawtypes" })
     public Response createCatalogEntryFromGroovyCode(String groovyCode) {
         ClassLoader parent = getCatalog().getRootClassLoader();
+        @SuppressWarnings("resource")
         GroovyClassLoader loader = new GroovyClassLoader(parent);
 
         Class clazz = loader.parseClass(groovyCode);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/main/java/brooklyn/rest/util/json/MultimapSerializer.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/util/json/MultimapSerializer.java b/usage/rest-server/src/main/java/brooklyn/rest/util/json/MultimapSerializer.java
index 7ab36c5..b0778b9 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/util/json/MultimapSerializer.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/util/json/MultimapSerializer.java
@@ -23,12 +23,11 @@ import java.util.Collection;
 import java.util.Map;
 
 import org.codehaus.jackson.JsonGenerator;
-import org.codehaus.jackson.map.JsonSerializer;
 import org.codehaus.jackson.map.SerializerProvider;
 import org.codehaus.jackson.map.ser.std.SerializerBase;
 
-import com.google.common.collect.Lists;
 import com.google.common.annotations.Beta;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
 
 /**
@@ -41,6 +40,7 @@ import com.google.common.collect.Multimap;
 @Beta
 public class MultimapSerializer extends SerializerBase<Multimap<?, ?>> {
 
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     protected MultimapSerializer() {
         super((Class<Multimap<?, ?>>) (Class) Multimap.class);
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java
index 1778786..c9aa4f0 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/BrooklynRestApiLauncherTestFixture.java
@@ -30,7 +30,6 @@ import brooklyn.config.BrooklynServiceAttributes;
 import brooklyn.entity.basic.Entities;
 import brooklyn.management.ManagementContext;
 import brooklyn.management.internal.LocalManagementContext;
-import brooklyn.management.internal.ManagementContextInternal;
 import brooklyn.rest.security.provider.AnyoneSecurityProvider;
 import brooklyn.util.exceptions.Exceptions;
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/domain/ApplicationTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/domain/ApplicationTest.java b/usage/rest-server/src/test/java/brooklyn/rest/domain/ApplicationTest.java
index cb98129..f905ed9 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/domain/ApplicationTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/domain/ApplicationTest.java
@@ -21,7 +21,6 @@ package brooklyn.rest.domain;
 import static brooklyn.rest.util.RestApiTestUtils.asJson;
 import static brooklyn.rest.util.RestApiTestUtils.fromJson;
 import static brooklyn.rest.util.RestApiTestUtils.jsonFixture;
-
 import static org.testng.Assert.assertEquals;
 
 import java.io.IOException;
@@ -53,6 +52,7 @@ public class ApplicationTest {
 
     final ApplicationSummary application = new ApplicationSummary(null, applicationSpec, Status.STARTING, null);
 
+    @SuppressWarnings("serial")
     @Test
     public void testSerializeToJSON() throws IOException {
         ApplicationSummary application1 = new ApplicationSummary("myapp_id", applicationSpec, Status.STARTING, null) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/domain/LocationSummaryTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/domain/LocationSummaryTest.java b/usage/rest-server/src/test/java/brooklyn/rest/domain/LocationSummaryTest.java
index 061dde7..ebfd004 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/domain/LocationSummaryTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/domain/LocationSummaryTest.java
@@ -34,6 +34,7 @@ import brooklyn.rest.transform.LocationTransformer;
 
 public class LocationSummaryTest {
 
+    @SuppressWarnings("deprecation")
     final LocationSummary summary = LocationTransformer.newInstance("123", LocationSpec.localhost());
 
     @Test

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java
index 88b7b8d..0eda001 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/ApplicationResourceIntegrationTest.java
@@ -23,7 +23,6 @@ import static org.testng.Assert.assertTrue;
 
 import java.net.URI;
 import java.util.Set;
-import java.util.concurrent.TimeoutException;
 
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/resources/EntityConfigResourceTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/EntityConfigResourceTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/EntityConfigResourceTest.java
index af8ca5d..1d35716 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/resources/EntityConfigResourceTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/EntityConfigResourceTest.java
@@ -149,7 +149,7 @@ public class EntityConfigResourceTest extends BrooklynRestResourceTest {
             String value = client().resource(uri).accept(MediaType.APPLICATION_JSON_TYPE).get(String.class);
             assertEquals(value, "\"hello world\"");
 
-        } finally { entity.setConfig(RestMockSimpleEntity.SAMPLE_CONFIG, RestMockSimpleEntity.SAMPLE_CONFIG.getDefaultValue()); }
+        } finally { entity.config().set(RestMockSimpleEntity.SAMPLE_CONFIG, RestMockSimpleEntity.SAMPLE_CONFIG.getDefaultValue()); }
     }
 
     @Test
@@ -167,7 +167,7 @@ public class EntityConfigResourceTest extends BrooklynRestResourceTest {
             String value = client().resource(uri+"/"+RestMockSimpleEntity.SAMPLE_CONFIG.getName()).accept(MediaType.APPLICATION_JSON_TYPE).get(String.class);
             assertEquals(value, "\"hello world\"");
 
-        } finally { entity.setConfig(RestMockSimpleEntity.SAMPLE_CONFIG, RestMockSimpleEntity.SAMPLE_CONFIG.getDefaultValue()); }
+        } finally { entity.config().set(RestMockSimpleEntity.SAMPLE_CONFIG, RestMockSimpleEntity.SAMPLE_CONFIG.getDefaultValue()); }
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/resources/LocationResourceTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/LocationResourceTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/LocationResourceTest.java
index aab0a43..4ff3a5d 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/resources/LocationResourceTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/LocationResourceTest.java
@@ -36,7 +36,6 @@ import org.testng.annotations.Test;
 
 import brooklyn.location.jclouds.JcloudsLocation;
 import brooklyn.rest.domain.CatalogLocationSummary;
-import brooklyn.rest.domain.LocationSpec;
 import brooklyn.rest.domain.LocationSummary;
 import brooklyn.rest.testing.BrooklynRestResourceTest;
 import brooklyn.test.Asserts;
@@ -67,7 +66,7 @@ public class LocationResourceTest extends BrooklynRestResourceTest {
                 "credential", "CR3dential");
         ClientResponse response = client().resource("/v1/locations")
                 .type(MediaType.APPLICATION_JSON_TYPE)
-                .post(ClientResponse.class, new LocationSpec(legacyLocationName, "aws-ec2:us-east-1", expectedConfig));
+                .post(ClientResponse.class, new brooklyn.rest.domain.LocationSpec(legacyLocationName, "aws-ec2:us-east-1", expectedConfig));
 
         URI addedLegacyLocationUri = response.getLocation();
         log.info("added legacy, at: " + addedLegacyLocationUri);
@@ -83,6 +82,7 @@ public class LocationResourceTest extends BrooklynRestResourceTest {
         Assert.assertEquals(l.getCredential(), "CR3dential");
     }
 
+    @SuppressWarnings("deprecation")
     @Test
     public void testAddNewLocationDefinition() {
         String yaml = Joiner.on("\n").join(ImmutableList.of(
@@ -122,6 +122,7 @@ public class LocationResourceTest extends BrooklynRestResourceTest {
         Assert.assertEquals(l.getCredential(), "CR3dential");
     }
 
+    @SuppressWarnings("deprecation")
     @Test(dependsOnMethods = { "testAddNewLocationDefinition" })
     public void testListAllLocationDefinitions() {
         Set<LocationSummary> locations = client().resource("/v1/locations")
@@ -139,6 +140,7 @@ public class LocationResourceTest extends BrooklynRestResourceTest {
         Assert.assertEquals(location.getLinks().get("self"), expectedLocationUri);
     }
 
+    @SuppressWarnings("deprecation")
     @Test(dependsOnMethods = { "testListAllLocationDefinitions" })
     public void testGetSpecificLocation() {
         URI expectedLocationUri = URI.create("/v1/locations/"+locationName);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
index 26491b3..ed5c1b4 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/resources/ServerResourceTest.java
@@ -21,28 +21,25 @@ package brooklyn.rest.resources;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 
 import java.util.concurrent.atomic.AtomicInteger;
 
-import brooklyn.config.BrooklynProperties;
-import brooklyn.management.internal.ManagementContextInternal;
-import com.google.common.collect.ImmutableMap;
-import com.sun.jersey.api.client.UniformInterfaceException;
-import com.sun.jersey.api.client.WebResource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.annotations.Test;
 
 import brooklyn.BrooklynVersion;
+import brooklyn.config.BrooklynProperties;
 import brooklyn.management.ManagementContext;
+import brooklyn.management.internal.ManagementContextInternal;
 import brooklyn.rest.domain.HighAvailabilitySummary;
 import brooklyn.rest.domain.VersionSummary;
 import brooklyn.rest.testing.BrooklynRestResourceTest;
 import brooklyn.test.Asserts;
 
 import com.google.common.collect.ImmutableSet;
+import com.sun.jersey.api.client.UniformInterfaceException;
 
 @Test(singleThreaded = true)
 public class ServerResourceTest extends BrooklynRestResourceTest {
@@ -80,6 +77,7 @@ public class ServerResourceTest extends BrooklynRestResourceTest {
         assertEquals(summary.getNodes().get(ownNodeId).getLocalTimestamp(), summary.getNodes().get(ownNodeId).getRemoteTimestamp());
     }
 
+    @SuppressWarnings("serial")
     @Test
     public void testReloadsBrooklynProperties() throws Exception {
         final AtomicInteger reloadCount = new AtomicInteger();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/EverythingGroupImpl.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/EverythingGroupImpl.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/EverythingGroupImpl.java
index 8f66682..b9e1667 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/EverythingGroupImpl.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/EverythingGroupImpl.java
@@ -26,7 +26,7 @@ public class EverythingGroupImpl extends DynamicGroupImpl implements EverythingG
 
     public EverythingGroupImpl() {
         super();
-        setConfig(ENTITY_FILTER, Predicates.alwaysTrue());
+        config().set(ENTITY_FILTER, Predicates.alwaysTrue());
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/NameMatcherGroupImpl.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/NameMatcherGroupImpl.java b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/NameMatcherGroupImpl.java
index b0b5854..f0b482d 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/NameMatcherGroupImpl.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/testing/mocks/NameMatcherGroupImpl.java
@@ -27,7 +27,7 @@ public class NameMatcherGroupImpl extends DynamicGroupImpl implements NameMatche
     @Override
     public void init() {
         super.init();
-        setConfig(ENTITY_FILTER, EntityPredicates.displayNameSatisfies(StringPredicates.matchesRegex(getConfig(NAME_REGEX))));
+        config().set(ENTITY_FILTER, EntityPredicates.displayNameSatisfies(StringPredicates.matchesRegex(getConfig(NAME_REGEX))));
         rescanEntities();
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/dbd0cdc4/usage/rest-server/src/test/java/brooklyn/rest/util/json/BrooklynJacksonSerializerTest.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/test/java/brooklyn/rest/util/json/BrooklynJacksonSerializerTest.java b/usage/rest-server/src/test/java/brooklyn/rest/util/json/BrooklynJacksonSerializerTest.java
index 552999f..8437ae3 100644
--- a/usage/rest-server/src/test/java/brooklyn/rest/util/json/BrooklynJacksonSerializerTest.java
+++ b/usage/rest-server/src/test/java/brooklyn/rest/util/json/BrooklynJacksonSerializerTest.java
@@ -299,7 +299,7 @@ public class BrooklynJacksonSerializerTest {
                     .build();
 
             // assert config here is just mgmt.toString()
-            app.setConfig(TestEntity.CONF_OBJECT, mgmt);
+            app.config().set(TestEntity.CONF_OBJECT, mgmt);
             String content = get(client, configUri, ImmutableMap.of("Accept", MediaType.TEXT_PLAIN));
             log.info("CONFIG MGMT is:\n"+content);
             Assert.assertEquals(content, mgmt.toString(), "content="+content);
@@ -332,7 +332,7 @@ public class BrooklynJacksonSerializerTest {
                     .build();
 
             // assert config here is just mgmt
-            app.setConfig(TestEntity.CONF_OBJECT, mgmt);
+            app.config().set(TestEntity.CONF_OBJECT, mgmt);
             String content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
             log.info("CONFIG MGMT is:\n"+content);
             @SuppressWarnings("rawtypes")
@@ -349,7 +349,7 @@ public class BrooklynJacksonSerializerTest {
             Assert.assertNotNull(values.get("links"), "Map should have contained links: values="+values);
 
             // but config etc returns our nicely json serialized
-            app.setConfig(TestEntity.CONF_OBJECT, app);
+            app.config().set(TestEntity.CONF_OBJECT, app);
             content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
             log.info("CONFIG ENTITY is:\n"+content);
             values = new Gson().fromJson(content, Map.class);
@@ -357,13 +357,13 @@ public class BrooklynJacksonSerializerTest {
 
             // and self-ref gives error + toString
             SelfRefNonSerializableClass angry = new SelfRefNonSerializableClass();
-            app.setConfig(TestEntity.CONF_OBJECT, angry);
+            app.config().set(TestEntity.CONF_OBJECT, angry);
             content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
             log.info("CONFIG ANGRY is:\n"+content);
             assertErrorObjectMatchingToString(content, angry);
             
             // as does Server
-            app.setConfig(TestEntity.CONF_OBJECT, server);
+            app.config().set(TestEntity.CONF_OBJECT, server);
             content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
             // NOTE, if using the default visibility / object mapper, the getters of the object are invoked
             // resulting in an object which is huge, 7+MB -- and it wreaks havoc w eclipse console regex parsing!



[10/13] incubator-brooklyn git commit: comment on url re-creating with stream handling

Posted by he...@apache.org.
comment on url re-creating with stream handling


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/692e47c9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/692e47c9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/692e47c9

Branch: refs/heads/master
Commit: 692e47c99fe5f46162f8a4840434e013f85e163c
Parents: c72fe8a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu May 28 14:57:35 2015 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu May 28 14:58:01 2015 +0100

----------------------------------------------------------------------
 .../main/java/brooklyn/catalog/internal/CatalogClasspathDo.java   | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/692e47c9/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
index a30a9dc..add0391 100644
--- a/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
+++ b/core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java
@@ -140,8 +140,7 @@ public class CatalogClasspathDo {
                     
                     // TODO potential disk leak above as we have no way to know when the temp file can be removed earlier than server shutdown;
                     // a better way to handle this is to supply a stream handler (but URLConnection is a little bit hard to work with):
-//                    URI uri = URI.create(classpath.getEntries().get(i));
-//                    urls[i] = new URL(uri.getScheme(), uri.getHost(), uri.getPort(), uri.getPath()  // TODO query fragment?
+//                    urls[i] = new URL(null, classpath.getEntries().get(i)   // (handy construtor for reparsing urls, without splitting into uri first)
 //                        , new URLStreamHandler() {
 //                            @Override
 //                            protected URLConnection openConnection(URL u) throws IOException {