From 80d4806bc2828bb1437a42e48320e59282c5e86d Mon Sep 17 00:00:00 2001 From: Aled Sage Date: Thu, 15 Nov 2018 09:57:21 +0000 Subject: [PATCH] =?UTF-8?q?Subtype=20setting=20config=20val:=20use=20as=20?= =?UTF-8?q?key=E2=80=99s=20default=20val?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise if another blueprint uses this subtype, it does not see the config val for that key. Worst case, it says the blueprint is invalid because the config key has no value. --- .../BrooklynComponentTemplateResolver.java | 2 + .../BrooklynEntityDecorationResolver.java | 31 ++++++ .../brooklyn/ConfigParametersYamlTest.java | 58 ++++++++++ .../catalog/CatalogYamlEntityTest.java | 105 ++++++++++++++++++ 4 files changed, 196 insertions(+) diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java index b4233a1186..6bd13405fd 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java @@ -265,6 +265,8 @@ private void decorateSpec(EntitySpec spec, Set enc new BrooklynEntityDecorationResolver.TagsResolver(yamlLoader).decorate(spec, attrs, encounteredRegisteredTypeIds); configureEntityConfig(spec, encounteredRegisteredTypeIds); + + new BrooklynEntityDecorationResolver.SpecParameterResolver(yamlLoader).decorateDefaultVals(spec, attrs, encounteredRegisteredTypeIds); } @SuppressWarnings({ "unchecked", "rawtypes" }) diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java index 181fa67ae2..00d48f681c 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynEntityDecorationResolver.java @@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkArgument; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -35,6 +36,8 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.camp.brooklyn.BrooklynCampReservedKeys; import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynYamlTypeInstantiator.InstantiatorFromKey; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.mgmt.BrooklynTags; import org.apache.brooklyn.core.objs.BasicSpecParameter; @@ -221,6 +224,34 @@ public void decorate(EntitySpec entitySpec, ConfigBag attrs, Set enco BasicSpecParameter.initializeSpecWithExplicitParameters(entitySpec, explicitParams, instantiator.loader); } + public void decorateDefaultVals(EntitySpec entitySpec, ConfigBag attrs, Set encounteredRegisteredTypeIds) { + // If entitySpec has explicit config values, those override the 'defaults' in the SpecParameter. + // Therefore in our type, we should report these as the 'defaults'. + // This is particularly important for subtypes that set values for inherited config keys. + // e.g. MySuperType declares required config 'maxSize', and MySubType sets a value for that in its + // `brooklyn.config` section. A user of `MySubType` in the UI would be forced to also set a value + // for 'maxSize' (because 'required' and no default) if we didn't do the code below. + + List> params = entitySpec.getParameters(); + Map, Object> configs = entitySpec.getConfig(); + + List> decoratedParameters = new ArrayList<>(); + for (SpecParameter param : params) { + SpecParameter decoratedParam; + if (configs.containsKey(param.getConfigKey())) { + Object explicitConfigVal = configs.get(param.getConfigKey()); + ConfigKey newConfigKey = ConfigKeys.newConfigKeyWithDefault((ConfigKey)param.getConfigKey(), explicitConfigVal); + decoratedParam = new BasicSpecParameter(param.getLabel(), param.isPinned(), newConfigKey, param.getSensor()); + } else { + decoratedParam = param; + } + decoratedParameters.add(decoratedParam); + + } + // but now we need to filter non-reinheritable, and merge where params are extended/overridden + entitySpec.parametersReplace(decoratedParameters); + } + @Override protected List> buildListOfTheseDecorationsFromIterable(Iterable value) { // returns definitions, used only by #decorate method above diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java index 5db4601154..87be1a45dc 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java @@ -44,6 +44,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.config.BasicConfigInheritance; +import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.config.ConfigPredicates; import org.apache.brooklyn.core.config.ConstraintViolationException; @@ -57,6 +58,7 @@ import org.apache.brooklyn.entity.software.base.EmptySoftwareProcess; import org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess; import org.apache.brooklyn.entity.stock.BasicApplication; +import org.apache.brooklyn.entity.stock.BasicEntity; import org.apache.brooklyn.location.ssh.SshMachineLocation; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableList; @@ -1275,6 +1277,62 @@ public void testConfigParameterPinnedOrder() throws Exception { } } + @Test + public void testRequiredParameterSetInConfigBlockTreatedAsDefault() throws Exception { + String version = TEST_VERSION; + String nameOfItem = "mySuperType"; + ConfigKey configKey = ConfigKeys.newStringConfigKey("myconfig"); + + String valInConfigBlock = "myValInSubType"; + String valInDeploy = "myValInDeploy"; + + addCatalogItems( + "brooklyn.catalog:", + " id: " + nameOfItem, + " version: " + version, + " itemType: entity", + " item:", + " type: " + BasicEntity.class.getName(), + " brooklyn.parameters:", + " - name: " + configKey.getName(), + " type: String", + " constraints:", + " - required", + " brooklyn.config:", + " " + configKey.getName() + ": " + valInConfigBlock); + + // Entity has a config value set in catalog item + { + Entity app = createAndStartApplication( + "services:", + "- type: " + nameOfItem + ":" + version); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(configKey), valInConfigBlock); + } + + // Config value overridden at deploy-time + { + Entity app = createAndStartApplication( + "services:", + "- type: " + nameOfItem + ":" + version, + " brooklyn.config:", + " myconfig: " + valInDeploy); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(configKey), valInDeploy); + } + + // Catalog item has a value for the config; this should be shown as the default + { + RegisteredType subType = mgmt().getTypeRegistry().get(nameOfItem, version); + EntitySpec spec = mgmt().getTypeRegistry().createSpec(subType, null, EntitySpec.class); + List> parameters = spec.getParameters(); + SpecParameter param = Iterables.find(parameters, (p) -> p.getConfigKey().equals(configKey)); + + assertEquals(param.getConfigKey().getDefaultValue(), valInConfigBlock); + assertEquals(param.getConfigKey().getConstraint(), new ConfigConstraints.RequiredPredicate()); + } + } + @ImplementedBy(TestEntityWithPinnedConfigImpl.class) public static interface TestEntityWithPinnedConfig extends Entity { diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java index cacbf40834..5d2654d201 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java @@ -30,6 +30,7 @@ import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.objs.Identifiable; +import org.apache.brooklyn.api.objs.SpecParameter; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; import org.apache.brooklyn.api.typereg.RegisteredType; @@ -37,7 +38,10 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.catalog.internal.CatalogUtils; +import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.config.ConstraintViolationException; +import org.apache.brooklyn.core.entity.Dumper; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.core.test.entity.TestEntityImpl; import org.apache.brooklyn.core.typereg.RegisteredTypes; @@ -760,6 +764,107 @@ protected void doTestReplacementFailureLeavesPreviousIntact(boolean includeBundl Assert.assertNull(item3, "Type should have been deleted"); } + @Test + public void testRequiredParameterSetInSubTypeNoLongerRequired() throws Exception { + String version = TEST_VERSION; + String nameOfSuperType = "mySuperType"; + String nameOfSubType = "mySubType"; + ConfigKey configKey = ConfigKeys.newStringConfigKey("myconfig"); + + String valInSubType = "myValInSubType"; + String valInDeploy = "myValInDeploy"; + + addCatalogItems( + "brooklyn.catalog:", + " id: " + nameOfSuperType, + " version: " + version, + " itemType: entity", + " item:", + " type: " + BasicEntity.class.getName(), + " brooklyn.parameters:", + " - name: " + configKey.getName(), + " type: String", + " constraints:", + " - required"); + + addCatalogItems( + "brooklyn.catalog:", + " id: " + nameOfSubType, + " version: " + version, + " itemType: entity", + " item:", + " type: " + nameOfSuperType + ":" + version, + " brooklyn.config:", + " " + configKey.getName() + ": " + valInSubType); + + // No value for 'required' config; should fail + try { + Entity app = createAndStartApplication( + "services:", + "- type: " + nameOfSuperType + ":" + version); + Dumper.dumpInfo(app); + Asserts.shouldHaveFailedPreviously(); + } catch (ConstraintViolationException e) { + // success - config has no value + } + + // If explicitly set config, then deploy should work + { + Entity app = createAndStartApplication( + "services:", + "- type: " + nameOfSuperType + ":" + version, + " brooklyn.config:", + " myconfig: " + valInDeploy); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(configKey), valInDeploy); + } + + // Subtype has a config value set, but that should be overridden if explicitly value is passed + { + Entity app = createAndStartApplication( + "services:", + "- type: " + nameOfSubType + ":" + version, + " brooklyn.config:", + " myconfig: " + valInDeploy); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(configKey), valInDeploy); + } + + // Subtype has a config value set, so 'required' constraint is satisfied + { + Entity app = createAndStartApplication( + "services:", + "- type: " + nameOfSubType + ":" + version); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + assertEquals(entity.config().get(configKey), valInSubType); + } + + // UI uses spec-parameter's metadata to validate inputs. + // Check that reports constraints / defaults correctly. + + // Supertype has no default value, and constraint is 'required' + { + RegisteredType superType = mgmt().getTypeRegistry().get(nameOfSuperType, version); + EntitySpec spec = mgmt().getTypeRegistry().createSpec(superType, null, EntitySpec.class); + List> parameters = spec.getParameters(); + SpecParameter param = Iterables.find(parameters, (p) -> p.getConfigKey().equals(configKey)); + + assertEquals(param.getConfigKey().getDefaultValue(), null); + assertEquals(param.getConfigKey().getConstraint(), new ConfigConstraints.RequiredPredicate()); + } + + // Subtype has a value; this should be shown as the default + { + RegisteredType subType = mgmt().getTypeRegistry().get(nameOfSubType, version); + EntitySpec spec = mgmt().getTypeRegistry().createSpec(subType, null, EntitySpec.class); + List> parameters = spec.getParameters(); + SpecParameter param = Iterables.find(parameters, (p) -> p.getConfigKey().equals(configKey)); + + assertEquals(param.getConfigKey().getDefaultValue(), valInSubType); + assertEquals(param.getConfigKey().getConstraint(), new ConfigConstraints.RequiredPredicate()); + } + } + private void registerAndLaunchAndAssertSimpleEntity(String symbolicName, String serviceType) throws Exception { registerAndLaunchAndAssertSimpleEntity(symbolicName, serviceType, serviceType); }