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 2022/10/24 00:18:55 UTC
[brooklyn-server] 04/04: fix bug where tags get set as config on rebind
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 <al...@cloudsoft.io>
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(),