Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/229#discussion_r69521020
--- Diff:
core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---
@@ -384,5 +387,73 @@ public static WeightedParameter
getFieldConfig(ConfigKey<?> config, Field config
}
}
+ /**
+ * Returns true if the {@link SpecParameter parameter name} is the
same as
+ * that on the specified paramater.
+ */
+ protected static Predicate<SpecParameter<?>> sameName(final
SpecParameter<?> param) {
+ return new Predicate<SpecParameter<?>>() {
+ @Override
+ public boolean apply(SpecParameter<?> input) {
+ return
input.getConfigKey().getName().equals(param.getConfigKey().getName());
+ }
+ };
+ }
+
+ /**
+ * Returns true if the {@link SpecParameter#getLabel() label} is the
same as
+ * the specified string.
+ */
+ public static Predicate<SpecParameter<?>> labelEqualTo(final String
label) {
+ return new Predicate<SpecParameter<?>>() {
+ @Override
+ public boolean apply(SpecParameter<?> input) {
+ return input.getLabel().equals(label);
+ }
+ };
+ }
+
+ /**
+ * Returns true if the {@link ConfigKey#getName() config key name} is
the same
+ * as the specified string.
+ */
+ public static Predicate<SpecParameter<?>> nameEqualTo(final String
name) {
+ return new Predicate<SpecParameter<?>>() {
+ @Override
+ public boolean apply(SpecParameter<?> input) {
+ return input.getConfigKey().getName().equals(name);
+ }
+ };
+ }
+
+ /**
+ * Adds the given list of {@link SpecParameter parameters} to the
provided
+ * {@link AbstractBrooklynObjectSpec spec}. Inherits parent parameters
except
+ * where there is an existing {@link ConfigKey config key}.
+ * <p>
+ * Implemented by explicitly replacing the existing parameters with an
+ * updated list.
+ *
+ * @see EntitySpec#parameters(List)
+ */
+ public static void addParameters(AbstractBrooklynObjectSpec<?, ?>
spec, List<? extends SpecParameter<?>> explicitParams,
BrooklynClassLoadingContext loader) {
+ if (explicitParams.size() > 0) {
+ List<SpecParameter<?>> currentParams =
MutableList.copyOf(spec.getParameters());
+ Map<ConfigKey<?>, ?> currentConfig = spec.getConfig();
+ for (final SpecParameter<?> param : explicitParams) {
+ Optional<ConfigKey<?>> configExists =
Iterables.tryFind(currentConfig.keySet(),
ConfigPredicates.nameEqualTo(param.getConfigKey().getName()));
+ if (!configExists.isPresent()) {
+ Optional<SpecParameter<?>> paramExists =
Iterables.tryFind(currentParams, sameName(param));
+ if (paramExists.isPresent()) {
+ currentParams.remove(paramExists.get());
--- End diff --
@grkvlt I wondered about the same pattern as is used for config (see
`BrooklynComponentTemplateResolver.decorateSpec`). Not sure if the
`configureEntityConfig` does anything significantly different from what you
have though, just in a different place.
My gut high-level feel was that you'd always overwrite the inherited
SpecParameter if there was one with the same name (irrespective of whether an
additional config value is defined). Then when we look up the config value,
we'd figure out the right ConfigKey (see `EntityConfigMap.getConfig`, and how
it looks up "ownKey" - that should return the config key produced from our
SpecParameter). Then we will use that as the default.
The area that this PR does not seem to address (in the test cases at least,
as far as I can see) is the second kind of inheritance: an entity inheriting
config from its parent. What should this do? Should we find the `SpecParameter`
of our parent entity, and use that default? Or are you leaving that for a
separate PR?
But I really don't like the code in there (or our camp parsing in general -
hard to know exactly what to do to improve it though).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
