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 2014/08/07 23:55:43 UTC

[9/9] git commit: This closes #110

This closes #110

Merge remote-tracking branch 'apache-gh/pr/110'

Conflicts:
	api/src/main/java/brooklyn/basic/BrooklynObject.java
	core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
	core/src/main/java/brooklyn/enricher/basic/AbstractEnricher.java
	core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
	core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
	core/src/main/java/brooklyn/entity/rebind/BasicLocationRebindSupport.java
	core/src/main/java/brooklyn/location/basic/AbstractLocation.java
	core/src/test/java/brooklyn/entity/basic/EntitiesTest.java

Conflicts mainly new tags methods from #109, resolved and confirmed tests pass.
Also added LocationType and related classes.


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

Branch: refs/heads/master
Commit: d4a6328ebf2fdab2306d84261bfd412ec1dc04e7
Parents: 4891355 79b49a9
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Thu Aug 7 17:54:08 2014 -0400
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Thu Aug 7 17:55:14 2014 -0400

----------------------------------------------------------------------
 .../java/brooklyn/basic/BrooklynObject.java     |  30 +-
 .../main/java/brooklyn/basic/BrooklynType.java  |  54 ++++
 api/src/main/java/brooklyn/entity/Entity.java   |  29 +-
 .../main/java/brooklyn/entity/EntityType.java   |  24 +-
 .../brooklyn/entity/rebind/ChangeListener.java  |  44 +--
 .../java/brooklyn/location/LocationType.java    |  32 +++
 .../java/brooklyn/mementos/EntityMemento.java   |   3 -
 .../main/java/brooklyn/mementos/Memento.java    |   3 +
 .../main/java/brooklyn/policy/EnricherType.java |  24 +-
 .../java/brooklyn/policy/EntityAdjunct.java     |  11 -
 .../main/java/brooklyn/policy/PolicyType.java   |  22 +-
 .../brooklyn/basic/AbstractBrooklynObject.java  | 198 ++++++++++---
 .../brooklyn/basic/BrooklynDynamicType.java     | 283 +++++++++++++++++++
 .../brooklyn/basic/BrooklynTypeSnapshot.java    | 100 +++++++
 .../main/java/brooklyn/basic/BrooklynTypes.java | 119 ++++++++
 .../enricher/basic/AbstractEnricher.java        |  15 +-
 .../enricher/basic/EnricherDynamicType.java     |  39 +++
 .../enricher/basic/EnricherTypeSnapshot.java    |  39 +++
 .../brooklyn/entity/basic/AbstractEntity.java   | 169 +++++------
 .../entity/basic/EntityDynamicType.java         | 252 ++---------------
 .../entity/basic/EntityInitializers.java        |   2 +-
 .../entity/basic/EntityTypeSnapshot.java        |  58 +---
 .../java/brooklyn/entity/basic/EntityTypes.java |  86 +-----
 .../proxying/BasicEntityTypeRegistry.java       |   2 -
 .../entity/proxying/InternalEntityFactory.java  |   2 +-
 .../proxying/InternalLocationFactory.java       |  11 +-
 .../entity/proxying/InternalPolicyFactory.java  |  19 +-
 .../AbstractBrooklynObjectRebindSupport.java    |  88 ++++++
 .../rebind/BasicEnricherRebindSupport.java      |  43 +--
 .../entity/rebind/BasicEntityRebindSupport.java |  64 ++---
 .../rebind/BasicLocationRebindSupport.java      |  40 +--
 .../entity/rebind/BasicPolicyRebindSupport.java |  41 +--
 .../rebind/ImmediateDeltaChangeListener.java    | 179 +++++-------
 .../rebind/PeriodicDeltaChangeListener.java     | 153 ++++------
 .../entity/rebind/RebindManagerImpl.java        | 104 +------
 .../entity/rebind/dto/AbstractMemento.java      |  11 +-
 .../entity/rebind/dto/BasicEntityMemento.java   |  13 +-
 .../entity/rebind/dto/MementosGenerators.java   |  87 +++---
 .../location/basic/AbstractLocation.java        | 137 +++------
 .../AggregatingMachineProvisioningLocation.java |   4 +-
 .../FixedListMachineProvisioningLocation.java   |   4 +-
 .../LocalhostMachineProvisioningLocation.java   |  10 +-
 .../location/basic/LocationDynamicType.java     |  39 +++
 .../location/basic/LocationTypeSnapshot.java    |  40 +++
 .../location/basic/SshMachineLocation.java      |   3 +-
 .../policy/basic/AbstractEntityAdjunct.java     | 111 ++------
 .../brooklyn/policy/basic/AbstractPolicy.java   |  12 +-
 .../brooklyn/policy/basic/EnricherTypeImpl.java |  75 -----
 .../policy/basic/PolicyDynamicType.java         |  39 +++
 .../brooklyn/policy/basic/PolicyTypeImpl.java   |  75 -----
 .../policy/basic/PolicyTypeSnapshot.java        |  39 +++
 .../enricher/basic/BasicEnricherTest.java       |   2 +-
 .../brooklyn/entity/basic/EntitiesTest.java     |  22 +-
 .../entity/rebind/RebindEnricherTest.java       |  16 +-
 .../entity/rebind/RebindEntityTest.java         |  14 +-
 .../entity/rebind/RebindLocationTest.java       |  17 +-
 .../entity/rebind/RebindPolicyTest.java         |  27 +-
 .../location/basic/AbstractLocationTest.java    |   3 +-
 .../brooklyn/policy/basic/BasicPolicyTest.java  |   2 +-
 .../location/jclouds/JcloudsLocation.java       |   4 +-
 .../brooklyn/rest/resources/EntityResource.java |  14 +-
 .../rest/transform/CatalogTransformer.java      |   4 +-
 .../rest/util/BrooklynRestResourceUtils.java    |   4 +-
 .../rest/resources/EntityResourceTest.java      |  11 +-
 64 files changed, 1687 insertions(+), 1534 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/api/src/main/java/brooklyn/basic/BrooklynObject.java
----------------------------------------------------------------------
diff --cc api/src/main/java/brooklyn/basic/BrooklynObject.java
index dd26528,44cd489..88b50c2
--- a/api/src/main/java/brooklyn/basic/BrooklynObject.java
+++ b/api/src/main/java/brooklyn/basic/BrooklynObject.java
@@@ -34,12 -35,25 +36,28 @@@ public interface BrooklynObject extend
       */
      String getDisplayName();
      
-     /**
-      * A set of tags associated to this adjunct.
+     /** 
+      * Tags are arbitrary objects which can be attached to an entity for subsequent reference.
+      * They must not be null (as {@link ImmutableMap} may be used under the covers; also there is little point!);
+      * and they should be amenable to our persistence (on-disk serialization) and our JSON serialization in the REST API.
       */
-     @Nonnull Set<Object> getTags();
- 
-     /** whether the given object is contained as a tag */
-     boolean containsTag(Object tag);
+     TagSupport getTagSupport();
      
+     public static interface TagSupport {
+         /**
+          * @return An immutable copy of the set of tags on this entity. 
+          * Note {@link #containsTag(Object)} will be more efficient,
+          * and {@link #addTag(Object)} and {@link #removeTag(Object)} will not work on the returned set.
+          */
 -        Set<Object> getTags();
++        @Nonnull Set<Object> getTags();
+         
+         boolean containsTag(@Nonnull Object tag);
+         
+         boolean addTag(@Nonnull Object tag);
+         
++        boolean addTags(@Nonnull Iterable<?> tags);
++        
+         boolean removeTag(@Nonnull Object tag);
+     }
++
  }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/api/src/main/java/brooklyn/entity/Entity.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/api/src/main/java/brooklyn/location/LocationType.java
----------------------------------------------------------------------
diff --cc api/src/main/java/brooklyn/location/LocationType.java
index 0000000,0000000..7bdb179
new file mode 100644
--- /dev/null
+++ b/api/src/main/java/brooklyn/location/LocationType.java
@@@ -1,0 -1,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.
++ */
++package brooklyn.location;
++
++import brooklyn.basic.BrooklynType;
++
++import com.google.common.annotations.Beta;
++
++/**
++ * Gives type information for a {@link Location}. It is immutable.
++ 
++ * @since 0.7.0
++ */
++@Beta
++public interface LocationType extends BrooklynType {
++}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/api/src/main/java/brooklyn/policy/EntityAdjunct.java
----------------------------------------------------------------------
diff --cc api/src/main/java/brooklyn/policy/EntityAdjunct.java
index 97acc08,fa05a2a..ed42605
--- a/api/src/main/java/brooklyn/policy/EntityAdjunct.java
+++ b/api/src/main/java/brooklyn/policy/EntityAdjunct.java
@@@ -18,11 -18,6 +18,8 @@@
   */
  package brooklyn.policy;
  
- import java.util.Set;
- 
- import javax.annotation.Nonnull;
 +import javax.annotation.Nullable;
 +
  import brooklyn.basic.BrooklynObject;
  
  /**
@@@ -48,25 -43,7 +45,17 @@@ public interface EntityAdjunct extends 
      boolean isDestroyed();
      
      /**
 -     * Whether the adjunct is available
 +     * Whether the adjunct is available/active
       */
      boolean isRunning();
 +    
 +    /**
 +     * An optional tag used to identify adjuncts with a specific purpose, typically created by the caller.
 +     * This is used to prevent multiple instances with the same purpose from being created,
 +     * and to access and customize adjuncts so created.
 +     * <p>
 +     * This will be included in the call to {@link #getTags()}.
 +     */
 +    @Nullable String getUniqueTag();
-     
-     /**
-      * A set of tags associated to this adjunct.
-      * <p>
-      * This will include {@link #getUniqueTag()} if that is set.
-      */
-     @Override
-     @Nonnull Set<Object> getTags();
 +
  }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
index e977fec,b15e73d..d06eb2f
--- a/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
+++ b/core/src/main/java/brooklyn/basic/AbstractBrooklynObject.java
@@@ -24,7 -34,7 +34,8 @@@ import brooklyn.util.flags.SetFromFlag
  import brooklyn.util.text.Identifiers;
  
  import com.google.common.collect.ImmutableSet;
 +import com.google.common.collect.Iterables;
+ import com.google.common.collect.Maps;
  import com.google.common.collect.Sets;
  
  public abstract class AbstractBrooklynObject implements BrooklynObjectInternal {
@@@ -32,55 -46,150 +47,166 @@@
      @SetFromFlag(value="id")
      private String id = Identifiers.makeRandomId(8);
      
 +    /** subclasses should synchronize on this for all access */
 +    @SetFromFlag(value="tags")
-     protected Set<Object> tags = Sets.newLinkedHashSet();
+     private final Set<Object> tags = Sets.newLinkedHashSet();
+ 
+     private volatile ManagementContext managementContext;
+ 
+     public abstract void setDisplayName(String newName);
+ 
+     public AbstractBrooklynObject() {
+         this(Maps.newLinkedHashMap());
+     }
+     
+     public AbstractBrooklynObject(Map<?,?> properties) {
+         _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing();
+         
+         if (!_legacyConstruction && properties!=null && !properties.isEmpty()) {
+             log.warn("Forcing use of deprecated old-style location construction for "+getClass().getName()+" because properties were specified ("+properties+"); instead use specs (e.g. LocationSpec, EntitySpec, etc)");
+             if (log.isDebugEnabled())
+                 log.debug("Source of use of old-style construction", new Throwable("Source of use of old-style construction"));
+             _legacyConstruction = true;
+         }
+         
+         // rely on sub-class to call configure(properties), because otherwise its fields will not have been initialised
+     }
+ 
+     /**
+      * See {@link #configure(Map)}
+      * 
+      * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
+      */ 
+     @Deprecated
+     protected AbstractBrooklynObject configure() {
+         return configure(Collections.emptyMap());
+     }
+     
+     /**
+      * Will set fields from flags, and put the remaining ones into the 'leftovers' map.
+      * For some types, you can find unused config via {@link ConfigBag#getUnusedConfig()}.
+      * <p>
+      * To be overridden by AbstractEntity, AbstractLoation, AbstractPolicy, AbstractEnricher, etc.
+      * <p>
+      * But should not be overridden by specific entity types. If you do, the entity may break in
+      * subsequent releases. Also note that if you require fields to be initialized you must do that 
+      * in this method. You must *not* rely on field initializers because they may not run until *after* 
+      * this method (this method is invoked by the constructor in this class, so initializers
+      * in subclasses will not have run when this overridden method is invoked.)
+      * 
+      * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
+      */ 
+     @Deprecated
 -    protected AbstractBrooklynObject configure(Map flags) {
 -        return this;
 -    }
++    protected abstract AbstractBrooklynObject configure(Map<?,?> flags);
+     
+     protected boolean isLegacyConstruction() {
+         return _legacyConstruction;
+     }
+ 
+     /**
+      * Called by framework (in new-style instances where spec was used) after configuring etc,
+      * but before a reference to this instance is shared.
+      * 
+      * To preserve backwards compatibility for if the instance is constructed directly, one
+      * can call the code below, but that means it will be called after references to this 
+      * policy have been shared with other entities.
+      * <pre>
+      * {@code
+      * if (isLegacyConstruction()) {
+      *     init();
+      * }
+      * }
+      * </pre>
+      */
+     public void init() {
+         // no-op
+     }
+     
+     /**
+      * Called by framework on rebind (in new-style instances),
+      * after configuring but before the instance is managed (or is attached to an entity, depending on its type), 
+      * and before a reference to this policy is shared.
+      * Note that {@link #init()} will not be called on rebind.
+      */
+     public void rebind() {
+         // no-op
+     }
+     
+     public void setManagementContext(ManagementContextInternal managementContext) {
+         this.managementContext = managementContext;
+     }
+     
+     public ManagementContext getManagementContext() {
+         return managementContext;
+     }
+ 
+     protected boolean isRebinding() {
+         return RebindManagerImpl.RebindTracker.isRebinding();
+     }
+     
+     protected void requestPersist() {
 -        // TODO Could add PolicyChangeListener, similar to EntityChangeListener; should we do that?
+         if (getManagementContext() != null) {
+             getManagementContext().getRebindManager().getChangeListener().onChanged(this);
+         }
+     }
      
      @Override
      public String getId() {
          return id;
      }
      
-     @Override
-     public Set<Object> getTags() {
-         synchronized (tags) {
-             return ImmutableSet.copyOf(tags);
-         }
++    protected void onTagsChanged() {
++        requestPersist();
++    }
++    
+     public TagSupport getTagSupport() {
 -        return new TagSupport() {
 -            @Override
 -            public Set<Object> getTags() {
 -                synchronized (tags) {
 -                    return ImmutableSet.copyOf(tags);
 -                }
++        return new BasicTagSupport();
 +    }
 +
-     public boolean addTag(Object tag) {
-         boolean result;
-         synchronized (tags) {
-             result = tags.add(tag);
++    protected class BasicTagSupport implements TagSupport {
++        @Override
++        public Set<Object> getTags() {
++            synchronized (tags) {
++                return ImmutableSet.copyOf(tags);
+             }
 -    
 -            @Override
 -            public boolean containsTag(Object tag) {
 -                synchronized (tags) {
 -                    return tags.contains(tag);
 -                }
 +        }
-         onTagsChanged();
-         return result;
-     }    
- 
-     public boolean addTags(Iterable<Object> tags) {
-         boolean result;
-         synchronized (tags) {
-             result = Iterables.addAll(this.tags, tags);
-         }
-         onTagsChanged();
-         return result;
-     }    
- 
-     public boolean removeTag(Object tag) {
-         boolean result;
-         synchronized (tags) {
-             result = tags.remove(tag);
-         }
-         onTagsChanged();
-         return result;
-     }    
 +
-     public boolean containsTag(Object tag) {
-         synchronized (tags) {
-             return tags.contains(tag);
++        @Override
++        public boolean containsTag(Object tag) {
++            synchronized (tags) {
++                return tags.contains(tag);
+             }
 -            
 -            @Override
 -            public boolean addTag(Object tag) {
 -                boolean result;
 -                synchronized (tags) {
 -                    result = tags.add(tag);
 -                }
 -                requestPersist();
 -                return result;
 -            }    
 -    
 -            @Override
 -            public boolean removeTag(Object tag) {
 -                boolean result;
 -                synchronized (tags) {
 -                    result = tags.remove(tag);
 -                }
 -                requestPersist();
 -                return result;
 -            }    
 -        };
 +        }
-     }    
++        
++        @Override
++        public boolean addTag(Object tag) {
++            boolean result;
++            synchronized (tags) {
++                result = tags.add(tag);
++            }
++            onTagsChanged();
++            return result;
++        }    
 +
-     protected abstract void onTagsChanged();
++        @Override
++        public boolean addTags(Iterable<?> newTags) {
++            boolean result;
++            synchronized (tags) {
++                result = Iterables.addAll(tags, newTags);
++            }
++            onTagsChanged();
++            return result;
++        }    
++
++        @Override
++        public boolean removeTag(Object tag) {
++            boolean result;
++            synchronized (tags) {
++                result = tags.remove(tag);
++            }
++            onTagsChanged();
++            return result;
++        }    
+     }
 +    
  }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/basic/BrooklynDynamicType.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/basic/BrooklynDynamicType.java
index 0000000,0ae7400..011c499
mode 000000,100644..100644
--- a/core/src/main/java/brooklyn/basic/BrooklynDynamicType.java
+++ b/core/src/main/java/brooklyn/basic/BrooklynDynamicType.java
@@@ -1,0 -1,283 +1,283 @@@
+ /*
+  * 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.basic;
+ 
+ import static com.google.common.base.Preconditions.checkNotNull;
+ 
+ import java.lang.reflect.Field;
+ import java.lang.reflect.Modifier;
+ import java.util.ArrayList;
+ import java.util.Collection;
+ import java.util.Collections;
+ import java.util.LinkedHashMap;
+ import java.util.LinkedHashSet;
+ import java.util.List;
+ import java.util.Map;
+ import java.util.concurrent.ConcurrentHashMap;
+ import java.util.concurrent.atomic.AtomicBoolean;
+ 
+ import org.slf4j.Logger;
+ import org.slf4j.LoggerFactory;
+ 
+ import brooklyn.config.ConfigKey;
+ import brooklyn.config.ConfigKey.HasConfigKey;
+ import brooklyn.event.basic.BasicConfigKey.BasicConfigKeyOverwriting;
+ import brooklyn.util.flags.FlagUtils;
+ import brooklyn.util.javalang.Reflections;
+ import brooklyn.util.text.Strings;
+ 
+ import com.google.common.base.Joiner;
+ import com.google.common.base.Objects;
+ import com.google.common.collect.ArrayListMultimap;
+ import com.google.common.collect.ListMultimap;
+ import com.google.common.collect.Lists;
+ 
+ /**
+  * This is the actual type of a brooklyn object instance at runtime,
+  * which can change from the static {@link BrooklynType}, and can change over time;
+  * for this reason it does *not* implement BrooklynType, but 
+  * callers can call {@link #getSnapshot()} to get a snapshot such instance.  
+  */
+ public abstract class BrooklynDynamicType<T extends BrooklynObject, AbstractT extends AbstractBrooklynObject> {
+ 
+     private static final Logger LOG = LoggerFactory.getLogger(BrooklynDynamicType.class);
+ 
+     protected final Class<? extends T> brooklynClass;
+     protected final AbstractT instance;
+     protected volatile String name;
+     
+     /** 
+      * Map of config keys (and their fields) on this instance, by name.
+      */
+     protected final Map<String,FieldAndValue<ConfigKey<?>>> configKeys = new ConcurrentHashMap<String, FieldAndValue<ConfigKey<?>>>();
+ 
+     private volatile BrooklynTypeSnapshot snapshot;
+     private final AtomicBoolean snapshotValid = new AtomicBoolean(false);
+ 
++    @SuppressWarnings("unchecked")
+     public BrooklynDynamicType(AbstractT instance) {
+         this((Class<? extends T>) instance.getClass(), instance);
+     }
+     public BrooklynDynamicType(Class<? extends T> clazz) {
+         this(clazz, null);
+     }
+     protected BrooklynDynamicType(Class<? extends T> clazz, AbstractT instance) {
+         this.brooklynClass = checkNotNull(clazz, "brooklyn class");
+         this.instance = instance;
+         // NB: official name is usually injected later, e.g. from AbstractEntity.setManagementContext
+         this.name = (clazz.getCanonicalName() == null) ? clazz.getName() : clazz.getCanonicalName();
+         
+         buildConfigKeys(clazz, null, configKeys);
+         if (LOG.isTraceEnabled())
+             LOG.trace("Entity {} config keys: {}", (instance==null ? clazz.getName() : instance.getId()), Joiner.on(", ").join(configKeys.keySet()));
+     }
+     
+     protected abstract BrooklynTypeSnapshot newSnapshot();
+ 
+     protected void invalidateSnapshot() {
+         snapshotValid.set(false);
+     }
+ 
+     public void setName(String name) {
+         if (Strings.isBlank(name)) {
+             throw new IllegalArgumentException("Invalid name "+(name == null ? "null" : "'"+name+"'")+"; name must be non-empty and not just white space");
+         }
+         this.name = name;
+         invalidateSnapshot();
+     }
+     
+     public synchronized BrooklynType getSnapshot() {
+         return refreshSnapshot();
+     }
+     
+     public Class<? extends T> getBrooklynClass() {
+         return brooklynClass;
+     }
+     
+     // --------------------------------------------------
+     
+     /**
+      * ConfigKeys available on this entity.
+      */
+     public Map<String,ConfigKey<?>> getConfigKeys() {
+         return Collections.unmodifiableMap(value(configKeys));
+     }
+ 
+     /**
+      * ConfigKeys available on this entity.
+      */
+     public ConfigKey<?> getConfigKey(String keyName) { 
+         return value(configKeys.get(keyName)); 
+     }
+ 
+     /** field where a config key is defined, for use getting annotations. note annotations are not inherited. */
+     public Field getConfigKeyField(String keyName) { 
+         return field(configKeys.get(keyName)); 
+     }
+ 
+     protected BrooklynTypeSnapshot refreshSnapshot() {
+         if (snapshotValid.compareAndSet(false, true)) {
+             snapshot = newSnapshot();
+         }
+         return snapshot;
+     }
+ 
+     /**
+      * Finds the config keys defined on the entity's class, statics and optionally any non-static (discouraged).
+      * Prefers keys which overwrite other keys, and prefers keys which are lower in the hierarchy;
+      * logs warnings if there are two conflicting keys which don't have an overwriting relationship.
+      */
+     protected static void buildConfigKeys(Class<? extends BrooklynObject> clazz, AbstractBrooklynObject optionalInstance, 
+             Map<String, FieldAndValue<ConfigKey<?>>> configKeys) {
+         ListMultimap<String,FieldAndValue<ConfigKey<?>>> configKeysAll = 
+                 ArrayListMultimap.<String, FieldAndValue<ConfigKey<?>>>create();
+         
+         for (Field f : FlagUtils.getAllFields(clazz)) {
+             boolean isConfigKey = ConfigKey.class.isAssignableFrom(f.getType());
+             if (!isConfigKey) {
+                 if (!HasConfigKey.class.isAssignableFrom(f.getType())) {
+                     // neither ConfigKey nor HasConfigKey
+                     continue;
+                 }
+             }
+             if (!Modifier.isStatic(f.getModifiers())) {
+                 // require it to be static or we have an instance
+                 LOG.warn("Discouraged use of non-static config key "+f+" defined in " + (optionalInstance!=null ? optionalInstance : clazz));
+                 if (optionalInstance==null) continue;
+             }
+             try {
+                 ConfigKey<?> k = isConfigKey ? (ConfigKey<?>) f.get(optionalInstance) : 
+                     ((HasConfigKey<?>)f.get(optionalInstance)).getConfigKey();
+                 
+                 if (k==null) {
+                     LOG.warn("no value defined for config key field (skipping): "+f);
+                 } else {
+                     configKeysAll.put(k.getName(), new FieldAndValue<ConfigKey<?>>(f, k));
+                 }
+                 
+             } catch (IllegalAccessException e) {
+                 LOG.warn("cannot access config key (skipping): "+f);
+             }
+         }
+         LinkedHashSet<String> keys = new LinkedHashSet<String>(configKeysAll.keys());
+         for (String kn: keys) {
+             List<FieldAndValue<ConfigKey<?>>> kk = Lists.newArrayList(configKeysAll.get(kn));
+             if (kk.size()>1) {
+                 // remove anything which extends another value in the list
+                 for (FieldAndValue<ConfigKey<?>> k: kk) {
+                     ConfigKey<?> key = value(k);
+                     if (key instanceof BasicConfigKeyOverwriting) {                            
+                         ConfigKey<?> parent = ((BasicConfigKeyOverwriting<?>)key).getParentKey();
+                         // find and remove the parent from consideration
+                         for (FieldAndValue<ConfigKey<?>> k2: kk) {
+                             if (value(k2) == parent)
+                                 configKeysAll.remove(kn, k2);
+                         }
+                     }
+                 }
+                 kk = Lists.newArrayList(configKeysAll.get(kn));
+             }
+             // multiple keys, not overwriting; if their values are the same then we don't mind
+             FieldAndValue<ConfigKey<?>> best = null;
+             for (FieldAndValue<ConfigKey<?>> k: kk) {
+                 if (best==null) {
+                     best=k;
+                 } else {
+                     Field lower = Reflections.inferSubbestField(k.field, best.field);
+                     ConfigKey<? extends Object> lowerV = lower==null ? null : lower.equals(k.field) ? k.value : best.value;
+                     if (best.value == k.value) {
+                         // same value doesn't matter which we take (but take lower if there is one)
+                         if (LOG.isTraceEnabled()) 
+                             LOG.trace("multiple definitions for config key {} on {}; same value {}; " +
+                                     "from {} and {}, preferring {}", 
+                                     new Object[] {
+                                     best.value.getName(), optionalInstance!=null ? optionalInstance : clazz,
+                                     best.value.getDefaultValue(),
+                                     k.field, best.field, lower});
+                         best = new FieldAndValue<ConfigKey<?>>(lower!=null ? lower : best.field, best.value);
+                     } else if (lower!=null) {
+                         // different value, but one clearly lower (in type hierarchy)
+                         if (LOG.isTraceEnabled()) 
+                             LOG.trace("multiple definitions for config key {} on {}; " +
+                                     "from {} and {}, preferring lower {}, value {}", 
+                                     new Object[] {
+                                     best.value.getName(), optionalInstance!=null ? optionalInstance : clazz,
+                                     k.field, best.field, lower,
+                                     lowerV.getDefaultValue() });
+                         best = new FieldAndValue<ConfigKey<?>>(lower, lowerV);
+                     } else {
+                         // different value, neither one lower than another in hierarchy
+                         LOG.warn("multiple ambiguous definitions for config key {} on {}; " +
+                                 "from {} and {}, values {} and {}; " +
+                                 "keeping latter (arbitrarily)", 
+                                 new Object[] {
+                                 best.value.getName(), optionalInstance!=null ? optionalInstance : clazz,
+                                 k.field, best.field, 
+                                 k.value.getDefaultValue(), best.value.getDefaultValue() });
+                         // (no change)
+                     }
+                 }
+             }
+             if (best==null) {
+                 // shouldn't happen
+                 LOG.error("Error - no matching config key from "+kk+" in class "+clazz+", even though had config key name "+kn);
+                 continue;
+             } else {
+                 configKeys.put(best.value.getName(), best);
+             }
+         }
+     }
+     
+     protected static class FieldAndValue<V> {
+         public final Field field;
+         public final V value;
+         public FieldAndValue(Field field, V value) {
+             this.field = field;
+             this.value = value;
+         }
+         @Override
+         public String toString() {
+             return Objects.toStringHelper(this).add("field", field).add("value", value).toString();
+         }
+     }
+     
+     protected static <V> V value(FieldAndValue<V> fv) {
+         if (fv==null) return null;
+         return fv.value;
+     }
+     
+     protected static Field field(FieldAndValue<?> fv) {
+         if (fv==null) return null;
+         return fv.field;
+     }
+ 
 -    @SuppressWarnings("unused")
+     protected static <V> Collection<V> value(Collection<FieldAndValue<V>> fvs) {
+         List<V> result = new ArrayList<V>();
+         for (FieldAndValue<V> fv: fvs) result.add(value(fv));
+         return result;
+     }
+ 
+     protected static <K,V> Map<K,V> value(Map<K,FieldAndValue<V>> fvs) {
+         Map<K,V> result = new LinkedHashMap<K,V>();
+         for (K key: fvs.keySet())
+             result.put(key, value(fvs.get(key)));
+         return result;
+     }
+ 
+ }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/enricher/basic/AbstractEnricher.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/enricher/basic/AbstractEnricher.java
index df6a370,8503663..9cba5b9
--- a/core/src/main/java/brooklyn/enricher/basic/AbstractEnricher.java
+++ b/core/src/main/java/brooklyn/enricher/basic/AbstractEnricher.java
@@@ -62,10 -62,6 +62,7 @@@ public abstract class AbstractEnricher 
  
      @Override
      protected void onChanged() {
-         // currently changes simply trigger re-persistence; there is no intermediate listener as we do for EntityChangeListener
-         if (getManagementContext() != null) {
-             getManagementContext().getRebindManager().getChangeListener().onChanged(this);
-         }
+         requestPersist();
      }
 +    
  }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
index 68846aa,ad2043c..97d7b4e
--- a/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
+++ b/core/src/main/java/brooklyn/entity/basic/AbstractEntity.java
@@@ -1349,9 -1285,24 +1308,29 @@@ public abstract class AbstractEntity ex
      }
  
      @Override
 +    protected void onTagsChanged() {
++        super.onTagsChanged();
 +        getManagementSupport().getEntityChangeListener().onTagsChanged();
 +    }
++
+     public Set<Object> getTags() {
+         return getTagSupport().getTags();
+     }
+ 
+     @Override
+     public boolean addTag(Object tag) {
+         return getTagSupport().addTag(tag);
+     }    
+ 
+     @Override
+     public boolean removeTag(Object tag) {
+         return getTagSupport().removeTag(tag);
+     }    
+ 
+     @Override
+     public boolean containsTag(Object tag) {
+         return getTagSupport().containsTag(tag);
+     }    
      
      @Override
      protected void finalize() throws Throwable {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
index 9cc05c3,b5d7c0a..78fa574
--- a/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
+++ b/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
@@@ -231,7 -230,6 +231,7 @@@ public class InternalEntityFactory exte
              if (spec.getDisplayName()!=null)
                  ((AbstractEntity)entity).setDisplayName(spec.getDisplayName());
              
-             ((AbstractEntity)entity).addTags(spec.getTags());
++            entity.getTagSupport().addTags(spec.getTags());
              ((AbstractEntity)entity).configure(MutableMap.copyOf(spec.getFlags()));
              
              for (Map.Entry<ConfigKey<?>, Object> entry : spec.getConfig().entrySet()) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/entity/proxying/InternalLocationFactory.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/entity/proxying/InternalLocationFactory.java
index 6a7b599,b989392..934acce
--- a/core/src/main/java/brooklyn/entity/proxying/InternalLocationFactory.java
+++ b/core/src/main/java/brooklyn/entity/proxying/InternalLocationFactory.java
@@@ -95,11 -102,9 +102,11 @@@ public class InternalLocationFactory ex
              managementContext.prePreManage(loc);
  
              if (spec.getDisplayName()!=null)
 -                ((AbstractLocation)loc).setName(spec.getDisplayName());
 +                ((AbstractLocation)loc).setDisplayName(spec.getDisplayName());
 +            
-             ((AbstractLocation)loc).addTags(spec.getTags());
++            loc.getTagSupport().addTags(spec.getTags());
              
-             if (isNewStyleLocation(clazz)) {
+             if (isNewStyle(clazz)) {
                  ((AbstractLocation)loc).setManagementContext(managementContext);
                  ((AbstractLocation)loc).configure(ConfigBag.newInstance().putAll(spec.getFlags()).putAll(spec.getConfig()).getAllConfig());
              }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
index 76dd312,d52d368..9a84c3b
--- a/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
+++ b/core/src/main/java/brooklyn/entity/proxying/InternalPolicyFactory.java
@@@ -94,9 -105,7 +105,9 @@@ public class InternalPolicyFactory exte
              if (spec.getDisplayName()!=null)
                  ((AbstractPolicy)pol).setDisplayName(spec.getDisplayName());
              
-             ((AbstractPolicy)pol).addTags(spec.getTags());
++            pol.getTagSupport().addTags(spec.getTags());
 +            
-             if (isNewStylePolicy(clazz)) {
+             if (isNewStyle(clazz)) {
                  ((AbstractPolicy)pol).setManagementContext(managementContext);
                  Map<String, Object> config = ConfigBag.newInstance().putAll(spec.getFlags()).putAll(spec.getConfig()).getAllConfig();
                  ((AbstractPolicy)pol).configure(MutableMap.copyOf(config)); // TODO AbstractPolicy.configure modifies the map
@@@ -131,9 -140,7 +142,9 @@@
              if (spec.getDisplayName()!=null)
                  ((AbstractEnricher)enricher).setDisplayName(spec.getDisplayName());
              
-             ((AbstractEnricher)enricher).addTags(spec.getTags());
++            enricher.getTagSupport().addTags(spec.getTags());
 +            
-             if (isNewStyleEnricher(clazz)) {
+             if (isNewStyle(clazz)) {
                  ((AbstractEnricher)enricher).setManagementContext(managementContext);
                  Map<String, Object> config = ConfigBag.newInstance().putAll(spec.getFlags()).putAll(spec.getConfig()).getAllConfig();
                  ((AbstractEnricher)enricher).configure(MutableMap.copyOf(config)); // TODO AbstractEnricher.configure modifies the map

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/location/basic/AbstractLocation.java
index 84259ff,c4b4ee0..4d76849
--- a/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
+++ b/core/src/main/java/brooklyn/location/basic/AbstractLocation.java
@@@ -35,10 -35,8 +35,7 @@@ import org.slf4j.LoggerFactory
  import brooklyn.basic.AbstractBrooklynObject;
  import brooklyn.config.ConfigKey;
  import brooklyn.config.ConfigKey.HasConfigKey;
--import brooklyn.entity.basic.EntityDynamicType;
- import brooklyn.entity.proxying.InternalFactory;
  import brooklyn.entity.rebind.BasicLocationRebindSupport;
- import brooklyn.entity.rebind.RebindManagerImpl;
  import brooklyn.entity.rebind.RebindSupport;
  import brooklyn.entity.trait.Configurable;
  import brooklyn.event.basic.BasicConfigKey;
@@@ -113,7 -104,7 +106,7 @@@ public abstract class AbstractLocation 
  
      private final Map<Class<?>, Object> extensions = Maps.newConcurrentMap();
      
--    private final EntityDynamicType entityType;
++    private final LocationDynamicType locationType;
      
      /**
       * Construct a new instance of an AbstractLocation.
@@@ -141,28 -132,20 +134,18 @@@
       * <li>abbreviatedName
       * </ul>
       */
-     @SuppressWarnings({ "rawtypes", "unchecked" })
 -    public AbstractLocation(Map properties) {
 +    public AbstractLocation(Map<?,?> properties) {
+         super(properties);
          inConstruction = true;
-         _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing();
-         if (!_legacyConstruction && properties!=null && !properties.isEmpty()) {
-             LOG.warn("Forcing use of deprecated old-style location construction for "+getClass().getName()+" because properties were specified ("+properties+")");
-             _legacyConstruction = true;
-         }
          
          // When one calls getConfig(key), we want to use the default value specified on *this* location
--        // if it overrides the default config. The easiest way to look up all our config keys is to 
--        // reuse the code for Entity (and this will become identical when locations become first-class
--        // entities). See {@link #getConfig(ConfigKey)}
--        entityType = new EntityDynamicType((Class)getClass());
++        // if it overrides the default config, by using the type object 
++        locationType = new LocationDynamicType(this);
          
-         if (_legacyConstruction) {
-             LOG.warn("Deprecated use of old-style location construction for "+getClass().getName()+"; instead use LocationManager().createLocation(spec)");
-             if (LOG.isDebugEnabled())
-                 LOG.debug("Source of use of old-style location construction", new Throwable("Source of use of old-style location construction"));
-             
-             configure(properties);
-             
+         if (isLegacyConstruction()) {
+             AbstractBrooklynObject checkWeGetThis = configure(properties);
+             assert this.equals(checkWeGetThis) : this+" configure method does not return itself; returns "+checkWeGetThis+" instead of "+this;
+ 
              boolean deferConstructionChecks = (properties.containsKey("deferConstructionChecks") && TypeCoercions.coerce(properties.get("deferConstructionChecks"), Boolean.class));
              if (!deferConstructionChecks) {
                  FlagUtils.checkRequiredFields(this);
@@@ -218,28 -201,12 +201,14 @@@
          }
      }
  
-     @Override
-     public ManagementContext getManagementContext() {
-         return managementContext;
-     }
-     
      /**
-      * Will set fields from flags. The unused configuration can be found via the 
-      * {@linkplain ConfigBag#getUnusedConfig()}.
-      * This can be overridden for custom initialization but note the following. 
-      * <p>
-      * For new-style locations (i.e. not calling constructor directly, this will
-      * be invoked automatically by brooklyn-core post-construction).
-      * <p>
-      * For legacy location use, this will be invoked by the constructor in this class.
-      * Therefore if over-riding you must *not* rely on field initializers because they 
-      * may not run until *after* this method (this method is invoked by the constructor 
-      * in this class, so initializers in subclasses will not have run when this overridden 
-      * method is invoked.) If you require fields to be initialized you must do that in 
-      * this method with a guard (as in FixedListMachineProvisioningLocation).
 -     * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly
++     * @deprecated since 0.7.0; only used for legacy brooklyn types where constructor is called directly;
++     * see overridden method for more info
       */
 +    @SuppressWarnings("serial")
-     public void configure(Map<?,?> properties) {
+     @Override
+     @Deprecated
 -    public AbstractLocation configure(Map properties) {
++    public AbstractLocation configure(Map<?,?> properties) {
          assertNotYetManaged();
          
          boolean firstTime = !configured.getAndSet(true);
@@@ -406,8 -338,7 +342,8 @@@
          
          // In case this entity class has overridden the given key (e.g. to set default), then retrieve this entity's key
          // TODO when locations become entities, the duplication of this compared to EntityConfigMap.getConfig will disappear.
 -        ConfigKey<T> ownKey = (ConfigKey<T>) elvis(entityType.getConfigKey(key.getName()), key);
 +        @SuppressWarnings("unchecked")
-         ConfigKey<T> ownKey = (ConfigKey<T>) elvis(entityType.getConfigKey(key.getName()), key);
++        ConfigKey<T> ownKey = (ConfigKey<T>) elvis(locationType.getConfigKey(key.getName()), key);
  
          return ownKey.getDefaultValue();
      }
@@@ -450,11 -381,19 +386,20 @@@
      
      @Override
      public <T> T setConfig(ConfigKey<T> key, T value) {
 -        return configBag.put(key, value);
 +        T result = configBag.put(key, value);
 +        onChanged();
 +        return result;
      }
  
+     /**
+      * @since 0.6.0 (?) - use getDisplayName
+      * @deprecated since 0.7.0; use {@link #getDisplayName()}
+      */
+     @Deprecated
+     public void setName(String newName) {
+         setDisplayName(newName);
 -        displayNameAutoGenerated = false;
+     }
+ 
      public void setDisplayName(String newName) {
          name.set(newName);
          displayNameAutoGenerated = false;
@@@ -514,13 -451,10 +459,12 @@@
          }
          
          if (isManaged()) {
-             if (!managementContext.getLocationManager().isManaged(child)) {
-                 // this deprecated call should be replaced with an internal interface call?
-                 managementContext.getLocationManager().manage(child);
 -            Locations.manage(child, getManagementContext());
++            if (!getManagementContext().getLocationManager().isManaged(child)) {
++                Locations.manage(child, getManagementContext());
 +            }
-         } else if (managementContext != null) {
-             if (((LocalLocationManager)managementContext.getLocationManager()).getLocationEvenIfPreManaged(child.getId()) == null) {
-                 ((ManagementContextInternal)managementContext).prePreManage(child);
+         } else if (getManagementContext() != null) {
+             if (((LocalLocationManager)getManagementContext().getLocationManager()).getLocationEvenIfPreManaged(child.getId()) == null) {
+                 ((ManagementContextInternal)getManagementContext()).prePreManage(child);
              }
          }
  
@@@ -542,25 -474,12 +486,20 @@@
              child.setParent(null);
              
              if (isManaged()) {
-                 managementContext.getLocationManager().unmanage(child);
+                 getManagementContext().getLocationManager().unmanage(child);
              }
          }
 +        onChanged();
          return removed;
      }
  
-     @Override
-     protected void onTagsChanged() {
-         onChanged();
-     }
- 
 +    protected void onChanged() {
 +        // currently changes simply trigger re-persistence; there is no intermediate listener as we do for EntityChangeListener
 +        if (getManagementContext() != null) {
 +            getManagementContext().getRebindManager().getChangeListener().onChanged(this);
 +        }
 +    }
 +
      /** Default String representation is simplified name of class, together with selected fields. */
      @Override
      public String toString() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/location/basic/LocationDynamicType.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/location/basic/LocationDynamicType.java
index 0000000,0000000..cc0b304
new file mode 100644
--- /dev/null
+++ b/core/src/main/java/brooklyn/location/basic/LocationDynamicType.java
@@@ -1,0 -1,0 +1,39 @@@
++/*
++ * 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.location.basic;
++
++import brooklyn.basic.BrooklynDynamicType;
++import brooklyn.location.Location;
++import brooklyn.location.LocationType;
++
++public class LocationDynamicType extends BrooklynDynamicType<Location, AbstractLocation> {
++
++    public LocationDynamicType(AbstractLocation location) {
++        super(location);
++    }
++    
++    public LocationType getSnapshot() {
++        return (LocationType) super.getSnapshot();
++    }
++
++    @Override
++    protected LocationTypeSnapshot newSnapshot() {
++        return new LocationTypeSnapshot(name, value(configKeys));
++    }
++}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/location/basic/LocationTypeSnapshot.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/location/basic/LocationTypeSnapshot.java
index 0000000,0000000..9a96e98
new file mode 100644
--- /dev/null
+++ b/core/src/main/java/brooklyn/location/basic/LocationTypeSnapshot.java
@@@ -1,0 -1,0 +1,40 @@@
++/*
++ * 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.location.basic;
++
++import java.util.Map;
++
++import brooklyn.basic.BrooklynTypeSnapshot;
++import brooklyn.config.ConfigKey;
++import brooklyn.policy.EnricherType;
++
++public class LocationTypeSnapshot extends BrooklynTypeSnapshot implements EnricherType {
++    
++    private static final long serialVersionUID = 9150132836104748237L;
++
++    LocationTypeSnapshot(String name, Map<String, ConfigKey<?>> configKeys) {
++        super(name, configKeys);
++    }
++
++    @Override
++    public boolean equals(Object obj) {
++        if (this == obj) return true;
++        return (obj instanceof LocationTypeSnapshot) && super.equals(obj);
++    }
++}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
----------------------------------------------------------------------
diff --cc core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
index a53d9a5,7f0e08f..36d8505
--- a/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
+++ b/core/src/main/java/brooklyn/policy/basic/AbstractEntityAdjunct.java
@@@ -40,8 -39,7 +39,6 @@@ import brooklyn.entity.Entity
  import brooklyn.entity.Group;
  import brooklyn.entity.basic.EntityInternal;
  import brooklyn.entity.basic.EntityLocal;
- import brooklyn.entity.proxying.InternalFactory;
--import brooklyn.entity.rebind.RebindManagerImpl;
  import brooklyn.entity.trait.Configurable;
  import brooklyn.event.AttributeSensor;
  import brooklyn.event.Sensor;
@@@ -407,25 -331,11 +338,17 @@@ public abstract class AbstractEntityAdj
      public boolean isRunning() {
          return !isDestroyed();
      }
 +
 +    @Override
 +    public String getUniqueTag() {
 +        return uniqueTag;
 +    }
      
      @Override
-     public Set<Object> getTags() {
-         if (getUniqueTag()==null) return super.getTags();
-         synchronized (tags) {
-             return ImmutableSet.builder().addAll(tags).add(getUniqueTag()).build();
-         }
-     }
-     
-     @Override
      public String toString() {
 -        return Objects.toStringHelper(getClass())
 +        return Objects.toStringHelper(getClass()).omitNullValues()
                  .add("name", name)
 +                .add("uniqueTag", uniqueTag)
                  .add("running", isRunning())
                  .add("id", getId())
                  .toString();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/main/java/brooklyn/policy/basic/AbstractPolicy.java
----------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/test/java/brooklyn/enricher/basic/BasicEnricherTest.java
----------------------------------------------------------------------
diff --cc core/src/test/java/brooklyn/enricher/basic/BasicEnricherTest.java
index b62b9c9,0000000..1252e36
mode 100644,000000..100644
--- a/core/src/test/java/brooklyn/enricher/basic/BasicEnricherTest.java
+++ b/core/src/test/java/brooklyn/enricher/basic/BasicEnricherTest.java
@@@ -1,100 -1,0 +1,100 @@@
 +/*
 + * 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.enricher.basic;
 +
 +import static org.testng.Assert.assertEquals;
 +
 +import java.util.Map;
 +
 +import org.testng.annotations.Test;
 +
 +import brooklyn.config.ConfigKey;
 +import brooklyn.entity.BrooklynAppUnitTestSupport;
 +import brooklyn.event.basic.BasicConfigKey;
 +import brooklyn.policy.EnricherSpec;
 +import brooklyn.util.collections.MutableSet;
 +import brooklyn.util.flags.SetFromFlag;
 +
 +/**
 + * Test that enricher can be created and accessed, by construction and by spec
 + */
 +public class BasicEnricherTest extends BrooklynAppUnitTestSupport {
 +    
 +    // TODO These tests are a copy of BasicPolicyTest, which is a code smell.
 +    // However, the src/main/java code does not contain as much duplication.
 +    
 +    public static class MyEnricher extends AbstractEnricher {
 +        @SetFromFlag("intKey")
 +        public static final BasicConfigKey<Integer> INT_KEY = new BasicConfigKey<Integer>(Integer.class, "bkey", "b key");
 +        
 +        @SetFromFlag("strKey")
 +        public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key");
 +        public static final ConfigKey<Integer> INT_KEY_WITH_DEFAULT = new BasicConfigKey<Integer>(Integer.class, "ckey", "c key", 1);
 +        public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default");
 +        
 +        MyEnricher(Map<?,?> flags) {
 +            super(flags);
 +        }
 +        
 +        public MyEnricher() {
 +            super();
 +        }
 +    }
 +    
 +    @Test
 +    public void testAddInstance() throws Exception {
 +        MyEnricher enricher = new MyEnricher();
 +        enricher.setDisplayName("Bob");
 +        enricher.setConfig(MyEnricher.STR_KEY, "aval");
 +        enricher.setConfig(MyEnricher.INT_KEY, 2);
 +        app.addEnricher(enricher);
 +        
 +        assertEquals(enricher.getDisplayName(), "Bob");
 +        assertEquals(enricher.getConfig(MyEnricher.STR_KEY), "aval");
 +        assertEquals(enricher.getConfig(MyEnricher.INT_KEY), (Integer)2);
 +    }
 +    
 +    @Test
 +    public void testAddSpec() throws Exception {
 +        MyEnricher enricher = app.addEnricher(EnricherSpec.create(MyEnricher.class)
 +            .displayName("Bob")
 +            .configure(MyEnricher.STR_KEY, "aval").configure(MyEnricher.INT_KEY, 2));
 +        
 +        assertEquals(enricher.getDisplayName(), "Bob");
 +        assertEquals(enricher.getConfig(MyEnricher.STR_KEY), "aval");
 +        assertEquals(enricher.getConfig(MyEnricher.INT_KEY), (Integer)2);
 +    }
 +        
 +    @Test
 +    public void testTagsFromSpec() throws Exception {
 +        MyEnricher enricher = app.addEnricher(EnricherSpec.create(MyEnricher.class).tag(99).uniqueTag("x"));
 +
-         assertEquals(enricher.getTags(), MutableSet.of("x", 99));
++        assertEquals(enricher.getTagSupport().getTags(), MutableSet.of("x", 99));
 +        assertEquals(enricher.getUniqueTag(), "x");
 +    }
 +
 +    @Test
 +    public void testSameUniqueTagEnricherNotAddedTwice() throws Exception {
 +        MyEnricher enricher1 = app.addEnricher(EnricherSpec.create(MyEnricher.class).tag(99).uniqueTag("x"));
 +        MyEnricher enricher2 = app.addEnricher(EnricherSpec.create(MyEnricher.class).tag(94).uniqueTag("x"));
 +        assertEquals(enricher2, enricher1);
 +        assertEquals(app.getEnrichers().size(), 1);
 +    }
 +
 +}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
----------------------------------------------------------------------
diff --cc core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
index e9b3399,999c168..415efa5
--- a/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
+++ b/core/src/test/java/brooklyn/entity/basic/EntitiesTest.java
@@@ -107,26 -107,22 +107,26 @@@ public class EntitiesTest extends Brook
      @Test
      public void testCreateGetContainsAndRemoveTags() throws Exception {
          entity = app.createAndManageChild(EntitySpec.create(TestEntity.class)
 +            .tag(2)
              .addInitializer(EntityInitializers.addingTags("foo")));
          
-         entity.addTag(app);
+         entity.getTagSupport().addTag(app);
          
-         Assert.assertTrue(entity.containsTag(app));
-         Assert.assertTrue(entity.containsTag("foo"));
-         Assert.assertTrue(entity.containsTag(2));
-         Assert.assertFalse(entity.containsTag("bar"));
++        Assert.assertTrue(entity.getTagSupport().containsTag(app));
+         Assert.assertTrue(entity.getTagSupport().containsTag("foo"));
++        Assert.assertTrue(entity.getTagSupport().containsTag(2));
+         Assert.assertFalse(entity.getTagSupport().containsTag("bar"));
          
-         Assert.assertEquals(entity.getTags(), MutableSet.of(app, "foo", 2));
 -        Assert.assertEquals(entity.getTagSupport().getTags(), MutableSet.of(app, "foo"));
++        Assert.assertEquals(entity.getTagSupport().getTags(), MutableSet.of(app, "foo", 2));
          
-         entity.removeTag("foo");
-         Assert.assertFalse(entity.containsTag("foo"));
+         entity.getTagSupport().removeTag("foo");
+         Assert.assertFalse(entity.getTagSupport().containsTag("foo"));
          
-         Assert.assertTrue(entity.containsTag(entity.getParent()));
-         Assert.assertFalse(entity.containsTag(entity));
+         Assert.assertTrue(entity.getTagSupport().containsTag(entity.getParent()));
+         Assert.assertFalse(entity.getTagSupport().containsTag(entity));
          
 +        entity.removeTag(2);
-         Assert.assertEquals(entity.getTags(), MutableSet.of(app));
+         Assert.assertEquals(entity.getTagSupport().getTags(), MutableSet.of(app));
      }
      
  }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/test/java/brooklyn/entity/rebind/RebindEnricherTest.java
----------------------------------------------------------------------
diff --cc core/src/test/java/brooklyn/entity/rebind/RebindEnricherTest.java
index d23e2d2,d8273ca..27e5edc
--- a/core/src/test/java/brooklyn/entity/rebind/RebindEnricherTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/RebindEnricherTest.java
@@@ -177,11 -174,7 +179,11 @@@ public class RebindEnricherTest extend
          newApp = (TestApplication) rebind();
          MyEnricher newEnricher = (MyEnricher) Iterables.getOnlyElement(newApp.getEnrichers());
          
 -        assertEquals(newEnricher.getName(), "My Enricher");
 +        assertEquals(newEnricher.getDisplayName(), "My Enricher");
 +        
 +        assertEquals(newEnricher.getUniqueTag(), "tagU");
-         assertEquals(newEnricher.getTags(), MutableSet.of("tagU", "tag1", "tag2"));
++        assertEquals(newEnricher.getTagSupport().getTags(), MutableSet.of("tagU", "tag1", "tag2"));
 +        
          assertEquals(newEnricher.getConfig(MyEnricher.MY_CONFIG_WITH_SETFROMFLAG_NO_SHORT_NAME), "myVal for with setFromFlag noShortName");
          assertEquals(newEnricher.getConfig(MyEnricher.MY_CONFIG_WITH_SETFROMFLAG_WITH_SHORT_NAME), "myVal for setFromFlag withShortName");
          assertEquals(newEnricher.getConfig(MyEnricher.MY_CONFIG_WITHOUT_SETFROMFLAG), "myVal for witout setFromFlag");

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
----------------------------------------------------------------------
diff --cc core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
index bc4e845,90d700b..e0314c3
--- a/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
+++ b/core/src/test/java/brooklyn/entity/rebind/RebindPolicyTest.java
@@@ -111,14 -110,9 +114,14 @@@ public class RebindPolicyTest extends R
                  .configure(MyPolicy.MY_CONFIG_WITH_SETFROMFLAG_WITH_SHORT_NAME, "myVal for setFromFlag withShortName")
                  .configure(MyPolicy.MY_CONFIG_WITHOUT_SETFROMFLAG, "myVal for witout setFromFlag"));
  
-         newApp = (TestApplication) rebind();
+         newApp = rebind();
          MyPolicy newPolicy = (MyPolicy) Iterables.getOnlyElement(newApp.getPolicies());
          
 +        assertEquals(newPolicy.getDisplayName(), "My Policy");
 +        
 +        assertEquals(newPolicy.getUniqueTag(), "tagU");
-         assertEquals(newPolicy.getTags(), MutableSet.of("tagU", "tag1", "tag2"));
++        assertEquals(newPolicy.getTagSupport().getTags(), MutableSet.of("tagU", "tag1", "tag2"));
 +        
          assertEquals(newPolicy.getConfig(MyPolicy.MY_CONFIG_WITH_SETFROMFLAG_NO_SHORT_NAME), "myVal for with setFromFlag noShortName");
          assertEquals(newPolicy.getConfig(MyPolicy.MY_CONFIG_WITH_SETFROMFLAG_WITH_SHORT_NAME), "myVal for setFromFlag withShortName");
          assertEquals(newPolicy.getConfig(MyPolicy.MY_CONFIG_WITHOUT_SETFROMFLAG), "myVal for witout setFromFlag");

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/test/java/brooklyn/location/basic/AbstractLocationTest.java
----------------------------------------------------------------------
diff --cc core/src/test/java/brooklyn/location/basic/AbstractLocationTest.java
index 97fb633,4993486..d005098
--- a/core/src/test/java/brooklyn/location/basic/AbstractLocationTest.java
+++ b/core/src/test/java/brooklyn/location/basic/AbstractLocationTest.java
@@@ -76,6 -75,6 +76,7 @@@ public class AbstractLocationTest 
      private ConcreteLocation createConcrete(Map<String,?> flags) {
          return createConcrete(null, flags);
      }
++    @SuppressWarnings("deprecation")
      private ConcreteLocation createConcrete(String id, Map<String,?> flags) {
          return mgmt.getLocationManager().createLocation( LocationSpec.create(ConcreteLocation.class).id(id).configure(flags) );
      }
@@@ -173,11 -172,5 +174,11 @@@
          ConcreteLocation loc = createConcrete();
          assertEquals(loc.myfield, "mydefault");
      }
 -    
 +
 +    @Test
 +    public void testLocationTags() throws Exception {
 +        LocationInternal loc = mgmt.getLocationManager().createLocation(LocationSpec.create(ConcreteLocation.class).tag("x"));
-         assertEquals(loc.getTags(), MutableSet.of("x"));
++        assertEquals(loc.getTagSupport().getTags(), MutableSet.of("x"));
 +    }
 +
  }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d4a6328e/core/src/test/java/brooklyn/policy/basic/BasicPolicyTest.java
----------------------------------------------------------------------
diff --cc core/src/test/java/brooklyn/policy/basic/BasicPolicyTest.java
index 1072bc8,0000000..2488aee
mode 100644,000000..100644
--- a/core/src/test/java/brooklyn/policy/basic/BasicPolicyTest.java
+++ b/core/src/test/java/brooklyn/policy/basic/BasicPolicyTest.java
@@@ -1,89 -1,0 +1,89 @@@
 +/*
 + * 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.policy.basic;
 +
 +import static org.testng.Assert.assertEquals;
 +
 +import java.util.Map;
 +
 +import org.testng.annotations.Test;
 +
 +import brooklyn.config.ConfigKey;
 +import brooklyn.entity.BrooklynAppUnitTestSupport;
 +import brooklyn.event.basic.BasicConfigKey;
 +import brooklyn.policy.PolicySpec;
 +import brooklyn.util.collections.MutableSet;
 +import brooklyn.util.flags.SetFromFlag;
 +
 +/**
 + * Test that policy can be created and accessed, by construction and by spec
 + */
 +public class BasicPolicyTest extends BrooklynAppUnitTestSupport {
 +    
 +    public static class MyPolicy extends AbstractPolicy {
 +        @SetFromFlag("intKey")
 +        public static final BasicConfigKey<Integer> INT_KEY = new BasicConfigKey<Integer>(Integer.class, "bkey", "b key");
 +        
 +        @SetFromFlag("strKey")
 +        public static final ConfigKey<String> STR_KEY = new BasicConfigKey<String>(String.class, "akey", "a key");
 +        public static final ConfigKey<Integer> INT_KEY_WITH_DEFAULT = new BasicConfigKey<Integer>(Integer.class, "ckey", "c key", 1);
 +        public static final ConfigKey<String> STR_KEY_WITH_DEFAULT = new BasicConfigKey<String>(String.class, "strKey", "str key", "str key default");
 +        
 +        MyPolicy(Map<?,?> flags) {
 +            super(flags);
 +        }
 +        
 +        public MyPolicy() {
 +            super();
 +        }
 +    }
 +    
 +    @Test
 +    public void testAddInstance() throws Exception {
 +        MyPolicy policy = new MyPolicy();
 +        policy.setDisplayName("Bob");
 +        policy.setConfig(MyPolicy.STR_KEY, "aval");
 +        policy.setConfig(MyPolicy.INT_KEY, 2);
 +        app.addPolicy(policy);
 +        
 +        assertEquals(policy.getDisplayName(), "Bob");
 +        assertEquals(policy.getConfig(MyPolicy.STR_KEY), "aval");
 +        assertEquals(policy.getConfig(MyPolicy.INT_KEY), (Integer)2);
 +    }
 +    
 +    @Test
 +    public void testAddSpec() throws Exception {
 +        MyPolicy policy = app.addPolicy(PolicySpec.create(MyPolicy.class)
 +            .displayName("Bob")
 +            .configure(MyPolicy.STR_KEY, "aval").configure(MyPolicy.INT_KEY, 2));
 +        
 +        assertEquals(policy.getDisplayName(), "Bob");
 +        assertEquals(policy.getConfig(MyPolicy.STR_KEY), "aval");
 +        assertEquals(policy.getConfig(MyPolicy.INT_KEY), (Integer)2);
 +    }
 +        
 +    @Test
 +    public void testTagsFromSpec() throws Exception {
 +        MyPolicy policy = app.addPolicy(PolicySpec.create(MyPolicy.class).tag(99).uniqueTag("x"));
 +
-         assertEquals(policy.getTags(), MutableSet.of("x", 99));
++        assertEquals(policy.getTagSupport().getTags(), MutableSet.of("x", 99));
 +        assertEquals(policy.getUniqueTag(), "x");
 +    }
 +
 +}