This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 01a5817bda056f70c5477694945333a685a1bdc0 Author: Alex Heneveld <[email protected]> AuthorDate: Mon Oct 24 01:16:51 2022 +0100 fix bug where tags get set as config on rebind --- .../core/mgmt/rebind/dto/MementosGenerators.java | 61 ++++++++++++---------- .../jclouds/RebindJcloudsLocationTest.java | 6 ++- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java index 187c1acf75..9fcfb8a926 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/MementosGenerators.java @@ -205,7 +205,31 @@ public class MementosGenerators { public static LocationMemento newLocationMemento(Location location) { return newLocationMementoBuilder(location).build(); } - + + static Set<String> nonPersistableFlagNames(Object typeInstance, boolean forRemoval) { + MutableSet<String> result = MutableSet.copyOf(MutableMap.<String, Object>builder() + .putAll(typeInstance == null ? null : FlagUtils.getFieldsWithFlagsWithModifiers(typeInstance, Modifier.TRANSIENT)) + .putAll(typeInstance == null ? null : FlagUtils.getFieldsWithFlagsWithModifiers(typeInstance, Modifier.STATIC)) + .filterValues(Predicates.not(Predicates.instanceOf(ConfigKey.class))) + .build() + .keySet()); + if (!forRemoval) { + result + .put("id") + .put("name") + .put("tags") + .put("brooklyn.tags"); + } + return result; + } + + static Map<String,Object> persistableFlagValues(Object typeInstance) { + return MutableMap.copyOf(MutableMap.<String, Object>builder() + .putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(typeInstance, Modifier.STATIC ^ Modifier.TRANSIENT)) + .removeAll(nonPersistableFlagNames(typeInstance, false)) + .build()); + } + /** * @deprecated since 0.7.0; use {@link #newBasicMemento(BrooklynObject)} instead */ @@ -214,21 +238,11 @@ public class MementosGenerators { BasicLocationMemento.Builder builder = BasicLocationMemento.builder(); populateBrooklynObjectMementoBuilder(location, builder); - Set<String> nonPersistableFlagNames = MutableMap.<String,Object>builder() - .putAll(FlagUtils.getFieldsWithFlagsWithModifiers(location, Modifier.TRANSIENT)) - .putAll(FlagUtils.getFieldsWithFlagsWithModifiers(location, Modifier.STATIC)) - .put("id", String.class) - .filterValues(Predicates.not(Predicates.instanceOf(ConfigKey.class))) - .build() - .keySet(); - Map<String, Object> persistableFlags = MutableMap.<String, Object>builder() - .putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(location, Modifier.STATIC ^ Modifier.TRANSIENT)) - .removeAll(nonPersistableFlagNames) - .build(); - ConfigBag persistableConfig = new ConfigBag().copy( ((LocationInternal)location).config().getLocalBag() ).removeAll(nonPersistableFlagNames); + ConfigBag persistableConfig = new ConfigBag().copy( ((LocationInternal)location).config().getLocalBag() ) + .removeAll(nonPersistableFlagNames(location, true)); builder.copyConfig(persistableConfig); - builder.locationConfig.putAll(persistableFlags); + builder.locationConfig.putAll(persistableFlagValues(location)); Location parentLocation = location.getParent(); builder.parent = (parentLocation != null) ? parentLocation.getId() : null; @@ -253,7 +267,6 @@ public class MementosGenerators { // TODO persist config keys as well? Or only support those defined on policy class; // current code will lose the ConfigKey type on rebind for anything not defined on class. // Whereas entities support that. - // TODO Do we need the "nonPersistableFlagNames" that locations use? Map<ConfigKey<?>, Object> config = ((AbstractPolicy)policy).config().getInternalConfigMap().getAllConfigLocalRaw(); for (Map.Entry<ConfigKey<?>, Object> entry : config.entrySet()) { ConfigKey<?> key = checkNotNull(entry.getKey(), "config=%s", config); @@ -263,12 +276,7 @@ public class MementosGenerators { builder.highlights(policy.getHighlights()); - Map<String, Object> persistableFlags = MutableMap.<String, Object>builder() - .putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(policy, Modifier.STATIC ^ Modifier.TRANSIENT)) - .remove("id") - .remove("name") - .build(); - builder.config.putAll(persistableFlags); + builder.config.putAll(persistableFlagValues(policy)); return builder.build(); } @@ -285,7 +293,6 @@ public class MementosGenerators { // TODO persist config keys as well? Or only support those defined on policy class; // current code will lose the ConfigKey type on rebind for anything not defined on class. // Whereas entities support that. - // TODO Do we need the "nonPersistableFlagNames" that locations use? Map<ConfigKey<?>, Object> config = ((AbstractEnricher)enricher).config().getInternalConfigMap().getAllConfigLocalRaw(); for (Map.Entry<ConfigKey<?>, Object> entry : config.entrySet()) { ConfigKey<?> key = checkNotNull(entry.getKey(), "config=%s", config); @@ -293,12 +300,7 @@ public class MementosGenerators { builder.config.put(key.getName(), value); } - Map<String, Object> persistableFlags = MutableMap.<String, Object>builder() - .putAll(FlagUtils.getFieldsWithFlagsExcludingModifiers(enricher, Modifier.STATIC ^ Modifier.TRANSIENT)) - .remove("id") - .remove("name") - .build(); - builder.config.putAll(persistableFlags); + builder.config.putAll(persistableFlagValues(enricher)); return builder.build(); } @@ -315,13 +317,14 @@ public class MementosGenerators { // TODO persist config keys as well? Or only support those defined on policy class; // current code will lose the ConfigKey type on rebind for anything not defined on class. // Whereas entities support that. - // TODO Do we need the "nonPersistableFlagNames" that locations use? Map<ConfigKey<?>, Object> config = ((AbstractFeed)feed).config().getInternalConfigMap().getAllConfigLocalRaw(); for (Map.Entry<ConfigKey<?>, Object> entry : config.entrySet()) { ConfigKey<?> key = checkNotNull(entry.getKey(), "config=%s", config); Object value = configValueToPersistable(entry.getValue(), feed, key.getName()); builder.config.put(key.getName(), value); } + + // feed does not include flags return builder.build(); } diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationTest.java index 9261758804..acf5065c85 100644 --- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationTest.java +++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/RebindJcloudsLocationTest.java @@ -41,9 +41,10 @@ public class RebindJcloudsLocationTest extends RebindTestFixtureWithApp { public void setUp() throws Exception { super.setUp(); origLoc = (JcloudsLocation) origManagementContext.getLocationRegistry().getLocationManaged(LOC_SPEC); + origLoc.tags().addTag("Tag"); } - // Previously, the rebound config contained "id" which was then passed to createTemporarySshMachineLocation, causing + // Previously, the rebound config contained "id" and "tags" which was then passed to createTemporarySshMachineLocation, causing // that to fail (because the LocationSpec should not have had "id" in its config) @Test public void testReboundConfigDoesNotContainId() throws Exception { @@ -55,7 +56,8 @@ public class RebindJcloudsLocationTest extends RebindTestFixtureWithApp { ConfigBag config = ConfigBag.newInstanceCopying(newLocConfig); assertNull(newLocConfig.getStringKey(("id"))); - + assertNull(newLocConfig.getStringKey(("tags"))); + SshMachineLocation tempMachine = newLoc.createTemporarySshMachineLocation( HostAndPort.fromParts("localhost", 1234), LoginCredentials.builder().identity("myuser").password("mypass").noPrivateKey().build(),
