You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by alasdairhodge <gi...@git.apache.org> on 2016/08/05 14:55:06 UTC
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
GitHub user alasdairhodge opened a pull request:
https://github.com/apache/brooklyn-server/pull/291
Conveniences to make groups easier to configure in YAML
* Extends `$brooklyn:object()` syntax to support parameterised constructors and static factory methods
* Adds variants of `EntityPredicates.config()` and `attribute()` that accept strings
* Adds variants of `EntityFunctions.config()` and `attribute()` that accept strings
* Adds variants of `EntityFunctions.config()` and `attribute()` that accept format specifier
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/alasdairhodge/brooklyn-server yaml-group-enhancements
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/291.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #291
----
commit 33647865f41649f48ce38cbda41a8cd4d9b0e94b
Author: Alasdair Hodge <gi...@alasdairhodge.co.uk>
Date: 2016-08-05T13:25:48Z
Convenience functions to access attributes + config by name.
commit f3f39938d75aaa4ebb1132952493a186399ad30f
Author: Alasdair Hodge <gi...@alasdairhodge.co.uk>
Date: 2016-08-05T13:26:20Z
Convenience predicates to test attributes + config by name.
commit 26af0a214f4ecfd593e821bf77bac5d9b8c7d116
Author: Alasdair Hodge <gi...@alasdairhodge.co.uk>
Date: 2016-08-05T13:26:55Z
Improved $brooklyn:object() DSL.
- Support parameterised constructors
- Support static factory methods
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/291#discussion_r74521077
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java ---
@@ -275,6 +288,57 @@ public void testBrooklynObjectWithFunction() throws Exception {
Assert.assertTrue(testObjectObject instanceof SimpleTestPojo, "Expected a SimpleTestPojo: "+testObjectObject);
}
+ @Test
+ public void testBrooklynObjectWithParameterisedConstructor() throws Exception {
--- End diff --
+1
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/291#discussion_r74489795
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java ---
@@ -327,25 +337,74 @@ public FormatString(String pattern, Object[] args) {
private static final long serialVersionUID = 8878388748085419L;
- private String typeName;
- private Class<?> type;
- private Map<String,Object> fields, config;
+ private final String typeName;
+ private final String factoryMethodName;
+ private final Class<?> type;
+ private final List<Object> constructorArgs, factoryMethodArgs;
+ private final Map<String, Object> fields, config;
+
+ public DslObject(
+ String typeName,
+ List<Object> constructorArgs,
+ Map<String, Object> fields,
+ Map<String, Object> config) {
--- End diff --
I'd use `Map<String, ?>` if we can get away with it - makes it easier for the caller.
But this is pretty internal stuff, so I don't really care.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #291: Conveniences to make groups easier to configure ...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on the issue:
https://github.com/apache/brooklyn-server/pull/291
Just one other thing: do we have a naming convention for config keys? Don't want to start a religious war over the camelCase `factoryMethod` ;o)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #291: Conveniences to make groups easier to configure ...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on the issue:
https://github.com/apache/brooklyn-server/pull/291
Thanks, Aled. Suggest improving test coverage and tidying up the bizarre coersion stuff in a subsequent PR.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/291
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/291#discussion_r74521683
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java ---
@@ -327,25 +337,74 @@ public FormatString(String pattern, Object[] args) {
private static final long serialVersionUID = 8878388748085419L;
- private String typeName;
- private Class<?> type;
- private Map<String,Object> fields, config;
+ private final String typeName;
+ private final String factoryMethodName;
+ private final Class<?> type;
+ private final List<Object> constructorArgs, factoryMethodArgs;
+ private final Map<String, Object> fields, config;
+
+ public DslObject(
+ String typeName,
+ List<Object> constructorArgs,
+ Map<String, Object> fields,
+ Map<String, Object> config) {
--- End diff --
I originally attempted this throughout, but it caused issues with the `Map.put` calls on lines 435, 437, 443, 445, as well as the call to `Reflections.invokeMethodFromArgs` on line 483.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/291#discussion_r74491248
--- Diff: camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java ---
@@ -384,24 +445,48 @@ public Object apply(List<Object> input) {
config.put(name, ((DeferredSupplier<?>) value).get());
}
}
- return create(type, fields, config);
+ if (factoryMethodName == null) {
+ return create(clazz, constructorArgs, fields, config);
+ } else {
+ return create(clazz, factoryMethodName, factoryMethodArgs, fields, config);
+ }
}
}, tasks);
}
- public static <T> T create(Class<T> type, Map<String,?> fields, Map<String,?> config) {
+ public static <T> T create(Class<T> type, List<?> constructorArgs, Map<String,?> fields, Map<String,?> config) {
try {
T bean;
try {
bean = (T) TypeCoercions.coerce(fields, type);
} catch (ClassCoercionException ex) {
- bean = Reflections.invokeConstructorFromArgs(type).get();
+ bean = Reflections.invokeConstructorFromArgs(type, constructorArgs.toArray()).get();
+ BeanUtils.populate(bean, fields);
+ }
+ if (bean instanceof Configurable && config.size() > 0) {
+ ConfigBag configBag = ConfigBag.newInstance(config);
+ FlagUtils.setFieldsFromFlags(bean, configBag);
+ FlagUtils.setAllConfigKeys((Configurable) bean, configBag, true);
+ }
+ return bean;
+ } catch (Exception e) {
+ throw Exceptions.propagate(e);
+ }
+ }
+
+ public static Object create(Class<?> type, String factoryMethodName, List<Object> factoryMethodArgs, Map<String,?> fields, Map<String,?> config) {
+ try {
+ Object bean;
+ try {
+ bean = TypeCoercions.coerce(fields, type);
--- End diff --
I don't follow this line. Why are we trying to coerce the fields (which is a map) to the expected object type?
Do we expect to support someone using `$brooklyn:object: { type: Map, object.fields: { a: b } }` to instantiate a map with contents `{a: b}`?!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #291: Conveniences to make groups easier to configure ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/291
@alasdairhodge looks good. Just one comment that I'm interested in before this is merged: what is the purpose of calling `TypeCoercions.coerce(fields, type)`?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #291: Conveniences to make groups easier to con...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/291#discussion_r74491894
--- Diff: camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ObjectsYamlTest.java ---
@@ -275,6 +288,57 @@ public void testBrooklynObjectWithFunction() throws Exception {
Assert.assertTrue(testObjectObject instanceof SimpleTestPojo, "Expected a SimpleTestPojo: "+testObjectObject);
}
+ @Test
+ public void testBrooklynObjectWithParameterisedConstructor() throws Exception {
--- End diff --
Looks good. Would also be nice to have a test that used a deferred supplier. But given there are no existing tests in `ObjectsYamlTest` that do that, I see why you didn't either!
We can do that in a separate PR.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #291: Conveniences to make groups easier to configure ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/291
@alasdairhodge we don't have a consistent convention for config key names. We have three approaches being used:
* dot-separated - normally used as a hierarchy, but sometimes dots used in the same way as camelCase (but it shouldn't be)
* underscores
* camelCase
Agree would be nice to have consistency! I'm fine with your camelCase.
Merging now.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---