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); }