You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by grkvlt <gi...@git.apache.org> on 2014/09/21 00:28:07 UTC
[GitHub] incubator-brooklyn pull request: Add type coercion for parameteriz...
GitHub user grkvlt opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/182
Add type coercion for parameterized collections and their entries
Coerce individual entries of collections separately for simpler `ConfigKey` definitions and easier usage in YAML files.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/grkvlt/incubator-brooklyn enhancement/collection-entry-coercion
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/182.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 #182
----
commit c56e8df70c7132458af2049db00d4d4df4a01b62
Author: Andrew Kennedy <gr...@apache.org>
Date: 2014-09-20T22:25:13Z
Add type coercion for parameterized collections and their entries
----
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#discussion_r18120509
--- Diff: usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java ---
@@ -93,53 +114,141 @@ public static Object sensor(String clazzName, String sensorName) {
Map<String,Sensor<?>> sensors = EntityDynamicType.findSensors((Class)clazz, null);
sensor = sensors.get(sensorName);
}
- if (sensor == null)
+ if (sensor == null) {
throw new IllegalArgumentException("Sensor " + sensorName + " not found on class " + clazzName);
+ }
return sensor;
} catch (ClassNotFoundException e) {
throw Exceptions.propagate(e);
}
}
- // --- build complex things
-
+ // Build complex things
+
public static EntitySpecConfiguration entitySpec(Map<String, Object> arguments) {
return new EntitySpecConfiguration(arguments);
}
-
- // --- string manipulation
- /** return a literal string -- ie skip parsing */
+ /**
+ * Return an instance of the specified class with its fields set according
+ * to the {@link Map} or a {@link BrooklynDslDeferredSupplier} if the arguments are not
+ * yet fully resolved.
+ */
+ public static Object object(Map<String, Object> arguments) {
+ try {
+ ConfigBag config = ConfigBag.newInstance(arguments);
+ String typeName = BrooklynYamlTypeInstantiator.InstantiatorFromKey.extractTypeName("object", config).orNull();
+ Map<String,Object> fields = (Map<String, Object>) config.getStringKey("object.fields");
+ Class<?> type = Class.forName(typeName);
+ if (DslUtils.resolved(fields.values())) {
+ String json = gson.get().toJson(fields);
+ return gson.get().fromJson(json, type);
+ } else {
+ return new DslObject(type, fields);
+ }
+ } catch (Exception e) {
+ throw Exceptions.propagate(e);
+ }
+ }
+
+ // String manipulation
+
+ /** Return the expression as a literal string without any further parsing. */
public static Object literal(Object expression) {
return expression;
}
- /** returns a DslParsedObject<String> OR a String if it is fully resolved */
+ /**
+ * Returns a formatted string or a {@link BrooklynDslDeferredSupplier} if the arguments
+ * are not yet fully resolved.
+ */
public static Object formatString(final String pattern, final Object ...args) {
if (DslUtils.resolved(args)) {
// if all args are resolved, apply the format string now
return String.format(pattern, args);
}
- return new FormatString(pattern, args);
+ return new DslFormatString(pattern, args);
}
- protected static final class FormatString extends BrooklynDslDeferredSupplier<String> {
--- End diff --
@ahgittin not sure I understand where these classes could actually be persisted?
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#discussion_r18120480
--- Diff: software/webapp/src/main/java/brooklyn/entity/proxy/ProxySslConfig.java ---
@@ -19,64 +19,17 @@
package brooklyn.entity.proxy;
import java.io.Serializable;
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicBoolean;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import brooklyn.util.collections.MutableMap;
-import brooklyn.util.flags.TypeCoercions;
-
-import com.google.common.base.Function;
-import com.google.common.base.Joiner;
import com.google.common.base.Objects;
public class ProxySslConfig implements Serializable {
private static final long serialVersionUID = -2692754611458939617L;
- private static final Logger LOG = LoggerFactory.getLogger(ProxySslConfig.class);
- private static final AtomicBoolean initialized = new AtomicBoolean(false);
-
- /** Setup type coercion. */
- @SuppressWarnings("rawtypes")
- public static void init() {
- if (initialized.getAndSet(true)) return;
-
- TypeCoercions.registerAdapter(Map.class, ProxySslConfig.class, new Function<Map, ProxySslConfig>() {
--- End diff --
No, I added it while trying to make some SSL applications work, so I'm going to change that code when this is merged.
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#discussion_r18169132
--- Diff: usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java ---
@@ -93,53 +111,164 @@ public static Object sensor(String clazzName, String sensorName) {
Map<String,Sensor<?>> sensors = EntityDynamicType.findSensors((Class)clazz, null);
sensor = sensors.get(sensorName);
}
- if (sensor == null)
+ if (sensor == null) {
throw new IllegalArgumentException("Sensor " + sensorName + " not found on class " + clazzName);
+ }
return sensor;
} catch (ClassNotFoundException e) {
throw Exceptions.propagate(e);
}
}
- // --- build complex things
-
+ // Build complex things
+
public static EntitySpecConfiguration entitySpec(Map<String, Object> arguments) {
return new EntitySpecConfiguration(arguments);
}
-
- // --- string manipulation
- /** return a literal string -- ie skip parsing */
+ /**
+ * Return an instance of the specified class with its fields set according
+ * to the {@link Map} or a {@link BrooklynDslDeferredSupplier} if the arguments are not
+ * yet fully resolved.
+ */
+ public static Object object(Map<String, Object> arguments) {
+ ConfigBag config = ConfigBag.newInstance(arguments);
+ String typeName = BrooklynYamlTypeInstantiator.InstantiatorFromKey.extractTypeName("object", config).orNull();
+ Map<String,Object> fields = Maybe.fromNullable((Map<String, Object>) config.getStringKey("object.fields"))
+ .or(MutableMap.<String, Object>of());
+ try {
+ Class<?> type = Class.forName(typeName);
--- End diff --
Is this the right class loader?
@grkvlt says "we can't get access to the right class loader, unfortunately; see code for sensor".
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#issuecomment-56561611
I know I said I wasn't sure about this but I've changed my mind.
I don't want to go too far down this slippery slope but this seems pretty safe and very useful. And certainly there are some important fixes here (coercing to `Set` where `List` was wanted!). So merging.
One thought, perhaps the returned coerced collection/map objects should be `MutableList.asUnmodifiable` (since they are now likely to be copies we do not want the user to modify them in error!) ?
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#issuecomment-57058319
Removed `ImmutableSet` (and `Map` and `List`) coercions as many entities want to modify collections, and also fixed a bug caused by `PortRange` being an `Iterable<Integer>` which means I had to switch coercion class checks back to `value instanceof Collection` again.
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#issuecomment-56608534
@ahgittin @aledsage I've added code for `$brooklyn:object` DSL functionality here as well, see `ObjectsYamlTest` for the syntax
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#discussion_r18076946
--- Diff: core/src/main/java/brooklyn/management/ManagementContextInjectable.java ---
@@ -16,16 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
-package brooklyn.rest.util;
+package brooklyn.management;
import brooklyn.management.ManagementContext;
-/** provides a way for management context to be injected directly.
- * normally it is discovered automatically by the ServletContext,
- * but in the jersey InMemory test container it is not, so we must inject it manually.
+/**
+ * Provides a way for the {@link ManagementContext} to be injected directly.
*/
public interface ManagementContextInjectable {
--- End diff --
Not likely to be a problem here but remember that where anything might be persisted, let's keep old classes and deprecate them, e.g. old `MCI` class still exists, extending the new `MCI`
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#discussion_r18077137
--- Diff: software/webapp/src/main/java/brooklyn/entity/proxy/ProxySslConfig.java ---
@@ -19,64 +19,17 @@
package brooklyn.entity.proxy;
import java.io.Serializable;
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicBoolean;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import brooklyn.util.collections.MutableMap;
-import brooklyn.util.flags.TypeCoercions;
-
-import com.google.common.base.Function;
-import com.google.common.base.Joiner;
import com.google.common.base.Objects;
public class ProxySslConfig implements Serializable {
private static final long serialVersionUID = -2692754611458939617L;
- private static final Logger LOG = LoggerFactory.getLogger(ProxySslConfig.class);
- private static final AtomicBoolean initialized = new AtomicBoolean(false);
-
- /** Setup type coercion. */
- @SuppressWarnings("rawtypes")
- public static void init() {
- if (initialized.getAndSet(true)) return;
-
- TypeCoercions.registerAdapter(Map.class, ProxySslConfig.class, new Function<Map, ProxySslConfig>() {
--- End diff --
is this going to be a compatibility issue? is anyone using this existing coercion?
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#issuecomment-57038090
@ahgittin I think the compatibility issue should be resolved.
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#issuecomment-56929949
looks good. some compatibility issues, and one high-level issue:
i don't like the inconsistency between `objectType` and `object.fields`. could we go with just `type` and `fields` ? since we are in a `$brooklyn:object` context i think this won't be ambiguous.
another option would be to use `@type` which is unambiguously not a java field name, and assume everything else is a java field, so we could avoid the `fields` sub-map altogether. (in the future we could have `@strategy` if we want to specify a specific deseriailization strategy, or could have `@construct: [ constructorParam1, constructorParam2 ]` ... wdyt? @aledsage ?
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#discussion_r18077162
--- Diff: usage/camp/src/main/java/io/brooklyn/camp/brooklyn/spi/dsl/methods/BrooklynDslCommon.java ---
@@ -93,53 +114,141 @@ public static Object sensor(String clazzName, String sensorName) {
Map<String,Sensor<?>> sensors = EntityDynamicType.findSensors((Class)clazz, null);
sensor = sensors.get(sensorName);
}
- if (sensor == null)
+ if (sensor == null) {
throw new IllegalArgumentException("Sensor " + sensorName + " not found on class " + clazzName);
+ }
return sensor;
} catch (ClassNotFoundException e) {
throw Exceptions.propagate(e);
}
}
- // --- build complex things
-
+ // Build complex things
+
public static EntitySpecConfiguration entitySpec(Map<String, Object> arguments) {
return new EntitySpecConfiguration(arguments);
}
-
- // --- string manipulation
- /** return a literal string -- ie skip parsing */
+ /**
+ * Return an instance of the specified class with its fields set according
+ * to the {@link Map} or a {@link BrooklynDslDeferredSupplier} if the arguments are not
+ * yet fully resolved.
+ */
+ public static Object object(Map<String, Object> arguments) {
+ try {
+ ConfigBag config = ConfigBag.newInstance(arguments);
+ String typeName = BrooklynYamlTypeInstantiator.InstantiatorFromKey.extractTypeName("object", config).orNull();
+ Map<String,Object> fields = (Map<String, Object>) config.getStringKey("object.fields");
+ Class<?> type = Class.forName(typeName);
+ if (DslUtils.resolved(fields.values())) {
+ String json = gson.get().toJson(fields);
+ return gson.get().fromJson(json, type);
+ } else {
+ return new DslObject(type, fields);
+ }
+ } catch (Exception e) {
+ throw Exceptions.propagate(e);
+ }
+ }
+
+ // String manipulation
+
+ /** Return the expression as a literal string without any further parsing. */
public static Object literal(Object expression) {
return expression;
}
- /** returns a DslParsedObject<String> OR a String if it is fully resolved */
+ /**
+ * Returns a formatted string or a {@link BrooklynDslDeferredSupplier} if the arguments
+ * are not yet fully resolved.
+ */
public static Object formatString(final String pattern, final Object ...args) {
if (DslUtils.resolved(args)) {
// if all args are resolved, apply the format string now
return String.format(pattern, args);
}
- return new FormatString(pattern, args);
+ return new DslFormatString(pattern, args);
}
- protected static final class FormatString extends BrooklynDslDeferredSupplier<String> {
--- End diff --
This will break persistence. Can we keep the old class as a subclass of the new class?
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/182
---
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] incubator-brooklyn pull request: Add type coercion for parameteriz...
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/182#issuecomment-57001478
@ahgittin I wanted to use `objectType` (or `type` actually) and `object.fields` for symmetry with Entity and Enricher/Policy basically. I was also thinking that we could eventually support `brooklyn.config` with `ConfigKey`s as well as the map of fields.
---
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.
---