You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by mbarrientos <gi...@git.apache.org> on 2014/07/07 13:21:23 UTC

[GitHub] incubator-brooklyn pull request: Adds EntityInitializers for http ...

GitHub user mbarrientos opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/48

    Adds EntityInitializers for http sensors and suport for full JSONPath

    - Adds EntityInitializer for ´HttpRequestSensors´ by defining a ´HttpFeed´ over sensors.
    - Full JSONPath support. Now is possible to specify a JSONPath (as defined here http://goessner.net/articles/JsonPath/ , thanks @ahgittin) by calling `HttpValueFunctions.jsonContentsFromPath()`

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mbarrientos/incubator-brooklyn http-request-sensors

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/48.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 #48
    
----
commit 964c7f965c3e4505bc81f5f3be62c18ae16c9ffd
Author: Miguel Barrientos <mb...@users.noreply.github.com>
Date:   2014-06-30T14:51:55Z

    Merge pull request #2 from apache/master
    
    Sync with parent repository

commit ba6722d9815d50c170ec4958de707d9fec4a81c0
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-02T07:33:30Z

    Added entity initializers for HttpRequestSensors. Polling over a URI and retrieving a single JSON value from the response.

commit 1a77123b258168ffd905c96885d6bd9080907cb6
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-02T07:34:35Z

    Fix deprecated calls on "chain" to "Functionals.chain"

commit 5c406c962e5c714b919cf095ea7aa7394ae742ed
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-04T15:31:46Z

    Fixed commented code and unused imports

commit ee4f15cfc89d6d794dc7389a30978c87f91eb049
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-04T15:38:21Z

    Changed asserts to static imports

commit d8cadb6c0072265961106fc6cf24a6b3289695b8
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-05T00:16:17Z

    Fixed variable names to fit standard notation

commit 802adfa1a42028b23777839c74a92de5c5aca727
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-05T00:18:07Z

    Add support for json-path (https://code.google.com/p/json-path) for full access to json response

commit 8afb7c47df58e1f6cc1830b7559bb5c5f5039339
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-07T09:23:10Z

    Moved test to new package

commit 974da609e1d926150241bb6407aa371d8e286001
Author: Miguel Barrientos <mb...@lcc.uma.es>
Date:   2014-07-07T09:55:27Z

    Refactoring to avoid removing functionality:
    now it supports both full path with JSONPath (through HttpValueFunctions.jsonContentsFromPath) and 1-level json element only.

----


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15049022
  
    --- Diff: software/base/src/test/java/brooklyn/entity/software/http/HttpRequestSensorTest.java ---
    @@ -0,0 +1,55 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.entity.basic.ApplicationBuilder;
    +import brooklyn.entity.basic.Attributes;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.proxying.EntitySpec;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.basic.Sensors;
    +import brooklyn.location.Location;
    +import brooklyn.test.EntityTestUtils;
    +import brooklyn.test.entity.TestApplication;
    +import brooklyn.test.entity.TestEntity;
    +import brooklyn.util.config.ConfigBag;
    +import com.google.common.collect.ImmutableList;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import static org.testng.Assert.assertEquals;
    +
    +public class HttpRequestSensorTest {
    +    final static AttributeSensor<String> SENSOR_STRING = Sensors.newStringSensor("aString", "");
    +    final static AttributeSensor<String> TARGET_TYPE = Sensors.newStringSensor("java.lang.String", "");
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    +        app = ApplicationBuilder.newManagedApp(TestApplication.class);
    +        entity = app.createAndManageChild(EntitySpec.create(TestEntity.class).location(app.newLocalhostProvisioningLocation().obtain()));
    +        app.start(ImmutableList.<Location>of());
    +    }
    +
    +    @AfterMethod(alwaysRun=true)
    +    public void tearDown() throws Exception {
    +        if (app != null) Entities.destroyAll(app.getManagementContext());
    +    }
    +
    +    @Test(groups="Integration")
    +    public void testHttpSensor() throws Exception {
    +        HttpRequestSensor<Integer> sensor = new HttpRequestSensor<Integer>(ConfigBag.newInstance()
    +                .configure(HttpRequestSensor.SENSOR_NAME, SENSOR_STRING.getName())
    +                .configure(HttpRequestSensor.SENSOR_TYPE, TARGET_TYPE.getName())
    +                .configure(HttpRequestSensor.JSON_PATH, "$.myKey")
    +                .configure(HttpRequestSensor.SENSOR_URI, "http://echo.jsontest.com/myKey/myValue"));
    --- End diff --
    
    Don't worry about it here, but for future reference see `brooklyn.event.feed.http.HttpFeedTest`. This uses `BetterMockWebServer` to set up the HTTP responses so that tests can be run without an internet connection. Thus the test can be a unit test rather than an integration test.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15120493
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -53,10 +53,43 @@ public AddSensor(T sensor) {
         public void apply(EntityLocal entity) {
             ((EntityInternal)entity).getMutableEntityType().addSensor(sensor);
         }
    -    
    -    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params) {
    +
    +    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params){
    +        String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    +        return Sensors.newSensor(type, name);
    +    }
    +
    +    public static <T> AttributeSensor<T> newSensor(ConfigBag params) {
             String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    -        return Sensors.newSensor(type, name); 
    +        String className = getFullClassName(params.get(SENSOR_TYPE));
    +        Class<T> type = null;
    +
    +        try {
    +            type = (Class<T>) Class.forName(className);
    +        } catch (ClassNotFoundException e) {
    +            throw new IllegalArgumentException("Invalid target type for sensor "+name+": " + className);
    +        }
    +        return Sensors.newSensor(type, name);
    +    }
    +
    +    private static String getFullClassName(String className) {
    +        if(className.equalsIgnoreCase("string")){
    +            return "java.lang.String";
    --- End diff --
    
    `String.class.getName()` and similar below would be cleaner than these magic strings -- but i'm not going to block this PR for that!  nice to have these conveniences.  might want to move them to a shared utils if we need them elsewhere.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r14704099
  
    --- Diff: software/base/src/main/java/brooklyn/entity/software/http/HttpRequestSensor.java ---
    @@ -0,0 +1,63 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.effector.AddSensor;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.feed.http.HttpFeed;
    +import brooklyn.event.feed.http.HttpPollConfig;
    +import brooklyn.event.feed.http.HttpValueFunctions;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.time.Duration;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Map;
    +
    +/**
    + * Configurable {@link brooklyn.entity.proxying.EntityInitializer} which adds an HTTP sensor feed to retrieve the
    + * <code>JSONObject</code> from a JSON response in order to populate the sensor with the indicated <code>name</code>.
    + */
    +// generics introduced here because we might support a configurable 'targetType` parameter in future, 
    +// with automatic casting (e.g. for ints); this way it remains compatible
    +public final class HttpRequestSensor<T extends String> extends AddSensor<String, AttributeSensor<String>> {
    +
    +    private static final org.slf4j.Logger log = LoggerFactory.getLogger(HttpRequestSensor.class);
    +
    +    public static final ConfigKey<String> JSON_PATH = ConfigKeys.newStringConfigKey("jsonPath");
    +    public static final ConfigKey<String> SENSOR_URI = ConfigKeys.newStringConfigKey("uri");
    +
    +    String jsonPath;
    +    String uri;
    +
    +    public HttpRequestSensor(ConfigBag params) {
    +        super(newSensor(String.class, params));
    +        jsonPath = Preconditions.checkNotNull(params.get(JSON_PATH), JSON_PATH);
    +        uri = Preconditions.checkNotNull(params.get(SENSOR_URI), SENSOR_URI);
    +    }
    +
    +    @Override
    +    public void apply(final EntityLocal entity) {
    +        super.apply(entity);
    +
    +        Duration period = Duration.ONE_SECOND;
    +
    +        HttpPollConfig<String> pollConfig = new HttpPollConfig<String>(sensor)
    +                .checkSuccess(HttpValueFunctions.responseCodeEquals(200))
    +                .onFailureOrException(Functions.constant((String) null))
    +                .onSuccess(HttpValueFunctions.jsonContentsFromPath(jsonPath, String.class));
    +
    +        if (period != null) pollConfig.period(period);
    --- End diff --
    
    Period will always be non null, because set to `Duration.ONE_SECOND` above.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r14703939
  
    --- Diff: core/src/main/java/brooklyn/event/feed/http/JsonFunctions.java ---
    @@ -132,6 +133,22 @@ private JsonFunctions() {} // instead use static utility methods
             };
         }
     
    +    /**
    +     * returns an element from a json object given a full path {@link com.jayway.jsonpath.JsonPath}
    +     */
    +    public static Function<JsonElement,? extends JsonElement> getPath(final String element) {
    +        return new Function<JsonElement, JsonElement>() {
    +            @Override public JsonElement apply(JsonElement input) {
    +                String jsonString = input.toString();
    +                JsonParser jsonParser = new JsonParser();
    +                JsonElement curr = jsonParser.parse(JsonPath.<String>read(jsonString, element));
    --- End diff --
    
    Interesting! I hadn't come across jsonPath before.
    
    A few code comments:
    * JsonPath.read(jsonString, element) will not necessarily return a `String` - e.g. if with test code `{"europe":{"uk":{"edinburgh":{"population":500000,"weather":"wet","lighting":"dark"}},"france":{"population":80000000}}}` then `$.europe.france.population` returns an `Integer`.
    * `JsonPath.<String>read(jsonString, element)` returns null if it could not be found - this causes an NPE in the method `jsonParser.parse(...)`, rather than it getting to your nicer exception below.
    * Would be good to add some unit test cases for this.
    
    For test cases, I played around with the code below in `JsonFunctionsTest`:
    
        @Test
        public void testJsonPath() {
            JsonElement pop = JsonFunctions.getPath("$.europe.france.population").apply( europeMap() );
            Assert.assertEquals( (int)JsonFunctions.cast(Integer.class).apply(pop), 80*1000*1000 );
        }



---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048949
  
    --- Diff: software/base/src/test/java/brooklyn/entity/software/http/HttpRequestSensorTest.java ---
    @@ -0,0 +1,55 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.entity.basic.ApplicationBuilder;
    +import brooklyn.entity.basic.Attributes;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.proxying.EntitySpec;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.basic.Sensors;
    +import brooklyn.location.Location;
    +import brooklyn.test.EntityTestUtils;
    +import brooklyn.test.entity.TestApplication;
    +import brooklyn.test.entity.TestEntity;
    +import brooklyn.util.config.ConfigBag;
    +import com.google.common.collect.ImmutableList;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import static org.testng.Assert.assertEquals;
    +
    +public class HttpRequestSensorTest {
    +    final static AttributeSensor<String> SENSOR_STRING = Sensors.newStringSensor("aString", "");
    +    final static AttributeSensor<String> TARGET_TYPE = Sensors.newStringSensor("java.lang.String", "");
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    +        app = ApplicationBuilder.newManagedApp(TestApplication.class);
    +        entity = app.createAndManageChild(EntitySpec.create(TestEntity.class).location(app.newLocalhostProvisioningLocation().obtain()));
    +        app.start(ImmutableList.<Location>of());
    +    }
    +
    +    @AfterMethod(alwaysRun=true)
    +    public void tearDown() throws Exception {
    +        if (app != null) Entities.destroyAll(app.getManagementContext());
    +    }
    +
    +    @Test(groups="Integration")
    +    public void testHttpSensor() throws Exception {
    +        HttpRequestSensor<Integer> sensor = new HttpRequestSensor<Integer>(ConfigBag.newInstance()
    +                .configure(HttpRequestSensor.SENSOR_NAME, SENSOR_STRING.getName())
    +                .configure(HttpRequestSensor.SENSOR_TYPE, TARGET_TYPE.getName())
    --- End diff --
    
    Does `TARGET_TYPE` need be an `AttributeSensor`? It looks like you just call `getName()`, which means it could just be a string. Or better yet it could use the value of `SENSOR_STRING.getTypeName()`.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048468
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -53,10 +52,23 @@ public AddSensor(T sensor) {
         public void apply(EntityLocal entity) {
             ((EntityInternal)entity).getMutableEntityType().addSensor(sensor);
         }
    -    
    -    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params) {
    +
    +    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params){
             String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    -        return Sensors.newSensor(type, name); 
    +        return Sensors.newSensor(type, name);
    +    }
    +
    +    public static <T> AttributeSensor<T> newSensor(ConfigBag params) {
    +        String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    +        String className = Preconditions.checkNotNull(params.get(SENSOR_TYPE), "target class must be supplied when defining a sensor");
    +        Class<T> type = null;
    +
    +        try {
    +            type = (Class<T>) Class.forName(className);
    +        } catch (ClassNotFoundException e) {
    +            throw new IllegalArgumentException("Invalid target type");
    --- End diff --
    
    In the exception, can you include more information to help those diagnosing problems from log files. e.g. `"Invalid target type for sensor "+name+": "+className`


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15057896
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -53,10 +52,23 @@ public AddSensor(T sensor) {
         public void apply(EntityLocal entity) {
             ((EntityInternal)entity).getMutableEntityType().addSensor(sensor);
         }
    -    
    -    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params) {
    +
    +    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params){
             String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    -        return Sensors.newSensor(type, name); 
    +        return Sensors.newSensor(type, name);
    +    }
    +
    +    public static <T> AttributeSensor<T> newSensor(ConfigBag params) {
    +        String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    +        String className = Preconditions.checkNotNull(params.get(SENSOR_TYPE), "target class must be supplied when defining a sensor");
    +        Class<T> type = null;
    +
    +        try {
    +            type = (Class<T>) Class.forName(className);
    --- End diff --
    
    +1 `Class.forName` good enough for now, and as we develop a way to support OSGi items then we'll improve it


---
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: Adds EntityInitializers for http ...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#issuecomment-48736558
  
    @aledsage json path is baked in to HTTP PATCH method so pretty common
    
    @mbarrientos do you have an ICLA?  we'd love to get this merged soon.  note also there are conflicts now, probably due to insertion of headers - should be easy to fix.


---
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: Adds EntityInitializers for http ...

Posted by mbarrientos <gi...@git.apache.org>.
Github user mbarrientos commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#discussion_r15049245
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -53,10 +52,23 @@ public AddSensor(T sensor) {
         public void apply(EntityLocal entity) {
             ((EntityInternal)entity).getMutableEntityType().addSensor(sensor);
         }
    -    
    -    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params) {
    +
    +    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params){
             String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    -        return Sensors.newSensor(type, name); 
    +        return Sensors.newSensor(type, name);
    +    }
    +
    +    public static <T> AttributeSensor<T> newSensor(ConfigBag params) {
    +        String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    +        String className = Preconditions.checkNotNull(params.get(SENSOR_TYPE), "target class must be supplied when defining a sensor");
    +        Class<T> type = null;
    +
    +        try {
    +            type = (Class<T>) Class.forName(className);
    --- End diff --
    
    You're right, i can also add some type checking for basic types (in the end most of them will be either String, Number or Boolean since it comes from a JSON) similar on how it's done in `JsonFunctions.cast`.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048426
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -53,10 +52,23 @@ public AddSensor(T sensor) {
         public void apply(EntityLocal entity) {
             ((EntityInternal)entity).getMutableEntityType().addSensor(sensor);
         }
    -    
    -    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params) {
    +
    +    public static <T> AttributeSensor<T> newSensor(Class<T> type, ConfigBag params){
             String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    -        return Sensors.newSensor(type, name); 
    +        return Sensors.newSensor(type, name);
    +    }
    +
    +    public static <T> AttributeSensor<T> newSensor(ConfigBag params) {
    +        String name = Preconditions.checkNotNull(params.get(SENSOR_NAME), "name must be supplied when defining a sensor");
    +        String className = Preconditions.checkNotNull(params.get(SENSOR_TYPE), "target class must be supplied when defining a sensor");
    +        Class<T> type = null;
    +
    +        try {
    +            type = (Class<T>) Class.forName(className);
    --- End diff --
    
    Unfortunately this won't work well with the OSGi work that is progressing. The `Class.forName` will use the class loader of the `AddSensor` class, which is fine if there is a flat classpath. However, with OSGi then the entity's classes (including those for custom sensor types) could be in OSGi bundles that are added on-the-fly. This classloading with then fail.
    
    However, this code will work for all the standard types like `String`, `URI`, etc so I'm inclined to include it. If @ahgittin agrees, then could you add some javadoc to this method like:
    
        /**
         * Creates a new sensor. The configuration can include the sensor {@code name} and {@code targetType}.
         * For the targetType, currently this only supports classes on the initial classpath 
         * (e.g. not those in OSGi bundles added at runtime).
         */
        @Beta


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048689
  
    --- Diff: software/base/src/main/java/brooklyn/entity/software/http/HttpRequestSensor.java ---
    @@ -0,0 +1,60 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.effector.AddSensor;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.feed.http.HttpFeed;
    +import brooklyn.event.feed.http.HttpPollConfig;
    +import brooklyn.event.feed.http.HttpValueFunctions;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.time.Duration;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Map;
    +
    +/**
    + * Configurable {@link brooklyn.entity.proxying.EntityInitializer} which adds an HTTP sensor feed to retrieve the
    + * <code>JSONObject</code> from a JSON response in order to populate the sensor with the indicated <code>name</code>.
    + */
    +public final class HttpRequestSensor<T> extends AddSensor<T, AttributeSensor<T>> {
    +
    +    private static final org.slf4j.Logger log = LoggerFactory.getLogger(HttpRequestSensor.class);
    +
    +    public static final ConfigKey<String> JSON_PATH = ConfigKeys.newStringConfigKey("jsonPath");
    +    public static final ConfigKey<String> SENSOR_URI = ConfigKeys.newStringConfigKey("uri");
    +
    +    private String jsonPath;
    --- End diff --
    
    Preference for final fields wherever appropriate; but don't worry about it.
    See http://www.ibm.com/developerworks/library/j-jtp1029/index.html (in particular the section on how great and underused final fields are).


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048208
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -38,12 +38,11 @@
     @Beta
     public class AddSensor<RT,T extends Sensor<RT>> implements EntityInitializer {
         protected final T sensor;
    -    
         public static final ConfigKey<String> SENSOR_NAME = ConfigKeys.newStringConfigKey("name");
         public static final ConfigKey<Duration> SENSOR_PERIOD = ConfigKeys.newConfigKey(Duration.class, "period", "Period, including units e.g. 1m or 5s or 200ms");
         
         // TODO
    -//    public static final ConfigKey<String> SENSOR_TYPE = ConfigKeys.newStringConfigKey("targetType");
    --- End diff --
    
    @ahgittin why was this previously commented out in your original commit? What are implications of uncommenting it? (An example of why I and many others hate commented out code that has no explanation!)


---
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: Adds EntityInitializers for http ...

Posted by mbarrientos <gi...@git.apache.org>.
Github user mbarrientos commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#discussion_r15050360
  
    --- Diff: software/base/src/test/java/brooklyn/entity/software/http/HttpRequestSensorTest.java ---
    @@ -0,0 +1,55 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.entity.basic.ApplicationBuilder;
    +import brooklyn.entity.basic.Attributes;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.proxying.EntitySpec;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.basic.Sensors;
    +import brooklyn.location.Location;
    +import brooklyn.test.EntityTestUtils;
    +import brooklyn.test.entity.TestApplication;
    +import brooklyn.test.entity.TestEntity;
    +import brooklyn.util.config.ConfigBag;
    +import com.google.common.collect.ImmutableList;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import static org.testng.Assert.assertEquals;
    +
    +public class HttpRequestSensorTest {
    +    final static AttributeSensor<String> SENSOR_STRING = Sensors.newStringSensor("aString", "");
    +    final static AttributeSensor<String> TARGET_TYPE = Sensors.newStringSensor("java.lang.String", "");
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    +        app = ApplicationBuilder.newManagedApp(TestApplication.class);
    +        entity = app.createAndManageChild(EntitySpec.create(TestEntity.class).location(app.newLocalhostProvisioningLocation().obtain()));
    +        app.start(ImmutableList.<Location>of());
    +    }
    +
    +    @AfterMethod(alwaysRun=true)
    +    public void tearDown() throws Exception {
    +        if (app != null) Entities.destroyAll(app.getManagementContext());
    +    }
    +
    +    @Test(groups="Integration")
    +    public void testHttpSensor() throws Exception {
    +        HttpRequestSensor<Integer> sensor = new HttpRequestSensor<Integer>(ConfigBag.newInstance()
    +                .configure(HttpRequestSensor.SENSOR_NAME, SENSOR_STRING.getName())
    +                .configure(HttpRequestSensor.SENSOR_TYPE, TARGET_TYPE.getName())
    +                .configure(HttpRequestSensor.JSON_PATH, "$.myKey")
    +                .configure(HttpRequestSensor.SENSOR_URI, "http://echo.jsontest.com/myKey/myValue"));
    --- End diff --
    
    Thanks, i'll note that down =)


---
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: Adds EntityInitializers for http ...

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/48#discussion_r14704065
  
    --- Diff: software/base/src/main/java/brooklyn/entity/software/http/HttpRequestSensor.java ---
    @@ -0,0 +1,63 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.effector.AddSensor;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.feed.http.HttpFeed;
    +import brooklyn.event.feed.http.HttpPollConfig;
    +import brooklyn.event.feed.http.HttpValueFunctions;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.time.Duration;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Map;
    +
    +/**
    + * Configurable {@link brooklyn.entity.proxying.EntityInitializer} which adds an HTTP sensor feed to retrieve the
    + * <code>JSONObject</code> from a JSON response in order to populate the sensor with the indicated <code>name</code>.
    + */
    +// generics introduced here because we might support a configurable 'targetType` parameter in future, 
    +// with automatic casting (e.g. for ints); this way it remains compatible
    +public final class HttpRequestSensor<T extends String> extends AddSensor<String, AttributeSensor<String>> {
    --- End diff --
    
    Is `<T extends String>` necessary? `String` is final, so nothing will ever extend it. You could use <T extends CharSequence>` or just remove the T? Or use a basic `Object` if it might be automatically converted at some point (not sure about that last option 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Adds EntityInitializers for http ...

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/48#discussion_r15058443
  
    --- Diff: core/src/main/java/brooklyn/entity/effector/AddSensor.java ---
    @@ -38,12 +38,11 @@
     @Beta
     public class AddSensor<RT,T extends Sensor<RT>> implements EntityInitializer {
         protected final T sensor;
    -    
         public static final ConfigKey<String> SENSOR_NAME = ConfigKeys.newStringConfigKey("name");
         public static final ConfigKey<Duration> SENSOR_PERIOD = ConfigKeys.newConfigKey(Duration.class, "period", "Period, including units e.g. 1m or 5s or 200ms");
         
         // TODO
    -//    public static final ConfigKey<String> SENSOR_TYPE = ConfigKeys.newStringConfigKey("targetType");
    --- End diff --
    
    should have had a comment:  the idea was to uncomment it when we were supporting it.  previously the only subclass, `SshCommandSensor` only needed to support strings so this was unnecessary, and this commented out line (successfully I'd say!) indicated how to proceed when other types were wanted.
    
    i wonder could we:
    * make `targetType` optional, e.g. defaulting to either `String` or `Object`
    * support convenience names `string`, `object`, `int`, `double`, to make it easier for folks to specify the type
    * rework `SshCommandSensor` to make use of this field, sharing the type coercion routines with the new classes introduced here.  (alternatively, move the `SENSOR_TYPE` field to `HttpRequestSensor`; thinking being, if this is on the superclass, all subclasses should support it.)


---
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: Adds EntityInitializers for http ...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#issuecomment-49452129
  
    great stuff, merging


---
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: Adds EntityInitializers for http ...

Posted by mbarrientos <gi...@git.apache.org>.
Github user mbarrientos commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#issuecomment-48916316
  
    @aledsage thanks for the git tips, they were really useful! 
    
    @ahgittin @aledsage, yes, I already signed the ICLA.
    
    I added support for defining the target type on the YAML, even if it's a bit low-level detail I couldn't find another easy way to not lose that information while parsing from JSON. (I will try to fix the conflicts 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.
---

[GitHub] incubator-brooklyn pull request: Adds EntityInitializers for http ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/48


---
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: Adds EntityInitializers for http ...

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/48#discussion_r14704160
  
    --- Diff: software/base/src/test/java/brooklyn/entity/software/http/HttpRequestSensorTest.java ---
    @@ -0,0 +1,62 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.entity.basic.ApplicationBuilder;
    +import brooklyn.entity.basic.Attributes;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.proxying.EntitySpec;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.basic.Sensors;
    +import brooklyn.location.Location;
    +import brooklyn.test.entity.TestApplication;
    +import brooklyn.test.entity.TestEntity;
    +import brooklyn.util.config.ConfigBag;
    +import com.google.common.collect.ImmutableList;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import static org.testng.Assert.assertTrue;
    +import static org.testng.Assert.assertEquals;
    +import static brooklyn.test.Asserts.succeedsEventually;
    +
    +public class HttpRequestSensorTest {
    +    final static AttributeSensor<String> SENSOR_STRING = Sensors.newStringSensor("aString", "");
    +    final static AttributeSensor<String> SENSOR_JSON_OBJECT = Sensors.newStringSensor("aJSONObject","");
    +    final static AttributeSensor<String> SENSOR_URI = Sensors.newStringSensor("uri","");
    +
    +    private TestApplication app;
    +    private EntityLocal entity;
    +
    +    @BeforeMethod(alwaysRun=true)
    +    public void setUp() throws Exception {
    +        app = ApplicationBuilder.newManagedApp(TestApplication.class);
    +        entity = app.createAndManageChild(EntitySpec.create(TestEntity.class).location(app.newLocalhostProvisioningLocation().obtain()));
    +        app.start(ImmutableList.<Location>of());
    +    }
    +
    +    @AfterMethod(alwaysRun=true)
    +    public void tearDown() throws Exception {
    +        if (app != null) Entities.destroyAll(app.getManagementContext());
    +    }
    +
    +    @Test(groups="Integration")
    +    public void testHttpSensor() throws Exception {
    +        new HttpRequestSensor<String>(ConfigBag.newInstance()
    +                .configure(HttpRequestSensor.SENSOR_NAME, SENSOR_STRING.getName())
    +                .configure(HttpRequestSensor.JSON_PATH, "$.myKey")
    +                .configure(HttpRequestSensor.SENSOR_URI, "http://echo.jsontest.com/myKey/myValue"))
    +            .apply(entity);
    +        entity.setAttribute(Attributes.SERVICE_UP, true);
    +
    +        succeedsEventually(new Runnable() {
    --- End diff --
    
    Can instead use the convenience `EntityTestUtils.assertAttributeEqualsEventually(entity, SENSOR_STRING, "mValue")`


---
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: Adds EntityInitializers for http ...

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/48#discussion_r14703979
  
    --- Diff: core/src/main/java/brooklyn/event/feed/http/JsonFunctions.java ---
    @@ -132,6 +133,22 @@ private JsonFunctions() {} // instead use static utility methods
             };
         }
     
    +    /**
    +     * returns an element from a json object given a full path {@link com.jayway.jsonpath.JsonPath}
    +     */
    +    public static Function<JsonElement,? extends JsonElement> getPath(final String element) {
    +        return new Function<JsonElement, JsonElement>() {
    +            @Override public JsonElement apply(JsonElement input) {
    +                String jsonString = input.toString();
    +                JsonParser jsonParser = new JsonParser();
    +                JsonElement curr = jsonParser.parse(JsonPath.<String>read(jsonString, element));
    --- End diff --
    
    A shame we have to parse the json a second time (rather than being able to use the JsonElement), but I don't see any obvious alternative.


---
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: Adds EntityInitializers for http ...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#issuecomment-48457554
  
    Hi @mbarrientos thanks for contributing!
    
    First, have you signed a CLA with apache? If not, could you fill in and submit this form please to apache - http://www.apache.org/licenses/icla.txt
    
    I hadn't come across jsonPath before - looks very useful. How wide spread is its use, and how much has it become a standard? (@ahgittin any comments on that?)
    
    The new dependencies look small and are apache 2 licensed, so I'm fine with us adding those.
    
    @mbarrientos can you look at adding tests to `JsonFunctionsTest` like those I suggested in the comment, and fixing the couple of problems (NPE and ClassCastException, depending on what is in the json). Then I'd be happy to merge.


---
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: Adds EntityInitializers for http ...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/48#issuecomment-48458263
  
    Also, if you can squash the commits so that history is cleaner that would be nice. But if you feel that is too much hassle then just let me know. (`git rebase -i HEAD~7`).
    
    Note that I generally avoid commits with "merge" in my PRs by always using `git pull --rebase`. This old thread contains some useful git commands, which I've been meaning to turn into docs and/or blog post for a very long time: https://groups.google.com/forum/#!msg/jclouds-dev/iZYY5TyrgA0/vsVzb5HkN9MJ
    
    One last git tip. If you rebase your commits and do `git push -f` then beware of strange git defaults! See http://stackoverflow.com/questions/948354/git-push-default-behavior and http://longair.net/blog/2011/02/27/an-asymmetry-between-git-pull-and-git-push/ for an explanation, and for what you want to do (e.g. something like `git config --global push.default current`).


---
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: Adds EntityInitializers for http ...

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/48#discussion_r14704087
  
    --- Diff: software/base/src/main/java/brooklyn/entity/software/http/HttpRequestSensor.java ---
    @@ -0,0 +1,63 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.effector.AddSensor;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.feed.http.HttpFeed;
    +import brooklyn.event.feed.http.HttpPollConfig;
    +import brooklyn.event.feed.http.HttpValueFunctions;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.time.Duration;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Map;
    +
    +/**
    + * Configurable {@link brooklyn.entity.proxying.EntityInitializer} which adds an HTTP sensor feed to retrieve the
    + * <code>JSONObject</code> from a JSON response in order to populate the sensor with the indicated <code>name</code>.
    + */
    +// generics introduced here because we might support a configurable 'targetType` parameter in future, 
    +// with automatic casting (e.g. for ints); this way it remains compatible
    +public final class HttpRequestSensor<T extends String> extends AddSensor<String, AttributeSensor<String>> {
    +
    +    private static final org.slf4j.Logger log = LoggerFactory.getLogger(HttpRequestSensor.class);
    +
    +    public static final ConfigKey<String> JSON_PATH = ConfigKeys.newStringConfigKey("jsonPath");
    +    public static final ConfigKey<String> SENSOR_URI = ConfigKeys.newStringConfigKey("uri");
    +
    +    String jsonPath;
    --- End diff --
    
    Suggest making these `private final`.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048772
  
    --- Diff: software/base/src/main/java/brooklyn/entity/software/http/HttpRequestSensor.java ---
    @@ -0,0 +1,60 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.effector.AddSensor;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.feed.http.HttpFeed;
    +import brooklyn.event.feed.http.HttpPollConfig;
    +import brooklyn.event.feed.http.HttpValueFunctions;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.time.Duration;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Map;
    +
    +/**
    + * Configurable {@link brooklyn.entity.proxying.EntityInitializer} which adds an HTTP sensor feed to retrieve the
    + * <code>JSONObject</code> from a JSON response in order to populate the sensor with the indicated <code>name</code>.
    + */
    +public final class HttpRequestSensor<T> extends AddSensor<T, AttributeSensor<T>> {
    --- End diff --
    
    This looks really useful.
    
    Over time, given its name of `HttpRequestSensor`, I can imagine us adding support for other things like xquery for XML content, or for comparing the http status code, etc.
    
    Can you mark the class as `@Beta` to give us more scope for such extensions in the future.


---
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: Adds EntityInitializers for http ...

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/48#discussion_r15048602
  
    --- Diff: software/base/src/main/java/brooklyn/entity/software/http/HttpRequestSensor.java ---
    @@ -0,0 +1,60 @@
    +package brooklyn.entity.software.http;
    +
    +import brooklyn.config.ConfigKey;
    +import brooklyn.entity.basic.ConfigKeys;
    +import brooklyn.entity.basic.EntityLocal;
    +import brooklyn.entity.effector.AddSensor;
    +import brooklyn.event.AttributeSensor;
    +import brooklyn.event.feed.http.HttpFeed;
    +import brooklyn.event.feed.http.HttpPollConfig;
    +import brooklyn.event.feed.http.HttpValueFunctions;
    +import brooklyn.util.config.ConfigBag;
    +import brooklyn.util.time.Duration;
    +import com.google.common.base.Functions;
    +import com.google.common.base.Preconditions;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.Map;
    +
    +/**
    + * Configurable {@link brooklyn.entity.proxying.EntityInitializer} which adds an HTTP sensor feed to retrieve the
    + * <code>JSONObject</code> from a JSON response in order to populate the sensor with the indicated <code>name</code>.
    + */
    +public final class HttpRequestSensor<T> extends AddSensor<T, AttributeSensor<T>> {
    +
    +    private static final org.slf4j.Logger log = LoggerFactory.getLogger(HttpRequestSensor.class);
    +
    +    public static final ConfigKey<String> JSON_PATH = ConfigKeys.newStringConfigKey("jsonPath");
    +    public static final ConfigKey<String> SENSOR_URI = ConfigKeys.newStringConfigKey("uri");
    +
    +    private String jsonPath;
    +    private String uri;
    +
    +    public HttpRequestSensor(ConfigBag params) {
    +        super(AddSensor.<T>newSensor(params));
    +        jsonPath = Preconditions.checkNotNull(params.get(JSON_PATH), JSON_PATH);
    +        uri = Preconditions.checkNotNull(params.get(SENSOR_URI), SENSOR_URI);
    +    }
    +
    +    @Override
    +    public void apply(final EntityLocal entity) {
    +        super.apply(entity);
    +
    +        Duration period = Duration.ONE_SECOND;
    +
    +        HttpPollConfig<T> pollConfig = new HttpPollConfig<T>(sensor)
    +                .checkSuccess(HttpValueFunctions.responseCodeEquals(200))
    +                .onFailureOrException(Functions.constant((T) null))
    +                .onSuccess(HttpValueFunctions.<T>jsonContentsFromPath(jsonPath))
    +                .period(period);
    +
    +        HttpFeed.builder().entity(entity)
    +                .baseUri(uri)
    +                .poll(pollConfig)
    +                .build();
    +    }
    +
    +    public HttpRequestSensor(Map<String, String> params) {
    --- End diff --
    
    Minor preference for constructors to all go at the top together; but don't worry about it.


---
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.
---