You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2018/06/01 12:48:53 UTC
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/967
add an /applications/details endpoint which corrects problems with /fetch
and adds some new features:
noticed that fetch actually recurses through all children entities which was never the intention and is wasteful at scale. also for usability it would be nice to include tags and all/regex sensors and config values. this adds support for that.
i've deprecated the existing `/fetch` call but left its behaviour unchanged. have also expanded description of `/list` endpoint.
to make this easier i've promoted `getAll()` from SensorSupportInternal. previously we had no way on the public API to list all sensors for an entity.
it is still @Beta.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server more-details-on-applications-endpoint
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/967.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 #967
----
commit db7f06df21f4f273ef349c9e62de206297c4387c
Author: Alex Heneveld <al...@...>
Date: 2018-06-01T11:42:14Z
add an /applications/details endpoint which corrects problems with /fetch and adds new features
noticed that fetch actually recurses through all children entities which was never the intention and is wasteful at scale.
also for usability it would be nice to include tags and all/regex sensors and config values. this adds support for that.
i've deprecated the existing `/fetch` call but left its behaviour unchanged. have also expanded description of `/list` endpoint.
to make this easier i've promoted `getAll()` from SensorSupportInternal. previously we had no way on the public API to list all sensors for an entity.
it is still @Beta.
----
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194659904
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -226,6 +276,60 @@ private EntityDetail addSensors(EntityDetail result, Entity entity, List<Attribu
}
return result;
}
+
+ private EntitySummary addSensorsByGlobs(EntitySummary result, Entity entity, List<String> extraSensorGlobs) {
+ if (extraSensorGlobs!=null && !extraSensorGlobs.isEmpty()) {
+ Object sensorsO = result.getExtraFields().get("sensors");
+ if (sensorsO==null) {
+ sensorsO = MutableMap.of();
+ result.setExtraField("sensors", sensorsO);
+ }
+ if (!(sensorsO instanceof Map)) {
+ throw new IllegalStateException("sensors field in result for "+entity+" should be a Map; found: "+sensorsO);
+ }
+ @SuppressWarnings("unchecked")
+ Map<String,Object> sensors = (Map<String, Object>) sensorsO;
+
+ Map<AttributeSensor<?>, Object> svs = entity.sensors().getAll();
+ svs.entrySet().stream().forEach(kv -> {
+ Object sv = kv.getValue();
+ if (sv!=null) {
+ String name = kv.getKey().getName();
+ if (extraSensorGlobs.stream().anyMatch(sn -> WildcardGlobs.isGlobMatched(sn, name))) {
+ sv = resolving(sv).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve();
+ sensors.put(name, sv);
--- End diff --
another good spot, all sensors and configs should be guarded, done.
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194661659
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -106,7 +112,16 @@
@Context
private UriInfo uriInfo;
- private EntityDetail fromEntity(Entity entity) {
+ private EntitySummary fromEntity(Entity entity, boolean includeTags, int detailDepth, List<String> extraSensorGlobs, List<String> extraConfigGlobs) {
+ if (detailDepth==0) {
+ return new EntitySummary(
+ entity.getId(),
+ entity.getDisplayName(),
+ entity.getEntityType().getName(),
+ entity.getCatalogItemId(),
+ MutableMap.of("self", EntityTransformer.entityUri(entity, ui.getBaseUriBuilder())) );
--- End diff --
good call, agree
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194661758
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -194,16 +218,42 @@ private EntityDetail fromEntity(Entity entity) {
Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
while (entity != null && entity.getParent() != null) {
if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
- entitySummaries.add(addSensors(fromEntity(entity), entity, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(entity, false, -1, null, null), entity, extraSensors));
}
entity = entity.getParent();
}
}
}
return entitySummaries;
}
+
+ @Override
+ public List<EntitySummary> details(String entityIds, boolean includeAllApps, String extraSensorsGlobsS, String extraConfigGlobsS, int depth) {
+ List<String> extraSensorGlobs = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsGlobsS);
+
+ Map<String, EntitySummary> entitySummaries = MutableMap.of();
+ if (includeAllApps) {
+ for (Entity application : mgmt().getApplications()) {
+ entitySummaries.put(application.getId(), fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs));
+ }
+ }
+
+ if (Strings.isNonBlank(entityIds)) {
+ List<String> extraEntities = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(entityIds);
+ for (String entityId: extraEntities) {
+ Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
+ while (entity != null && !entitySummaries.containsKey(entity.getId())) {
+ if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
+ entitySummaries.put(entity.getId(), fromEntity(entity, true, depth, extraSensorGlobs, extraSensorGlobs));
--- End diff --
done
---
[GitHub] brooklyn-server issue #967: add an /applications/details endpoint which corr...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/967
Might be worth adding some unit tests for this
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r192711341
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -194,16 +218,42 @@ private EntityDetail fromEntity(Entity entity) {
Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
while (entity != null && entity.getParent() != null) {
if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
- entitySummaries.add(addSensors(fromEntity(entity), entity, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(entity, false, -1, null, null), entity, extraSensors));
}
entity = entity.getParent();
}
}
}
return entitySummaries;
}
+
+ @Override
+ public List<EntitySummary> details(String entityIds, boolean includeAllApps, String extraSensorsGlobsS, String extraConfigGlobsS, int depth) {
+ List<String> extraSensorGlobs = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsGlobsS);
+
+ Map<String, EntitySummary> entitySummaries = MutableMap.of();
+ if (includeAllApps) {
+ for (Entity application : mgmt().getApplications()) {
+ entitySummaries.put(application.getId(), fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs));
--- End diff --
`fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs))` needs to be
`... extraSensorGlobs, extraConfigGlobs))`.
at the moment when you request sensors you get results including empty `config` object, but when you request config you don't get either sensors or config.
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194659942
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -226,6 +276,60 @@ private EntityDetail addSensors(EntityDetail result, Entity entity, List<Attribu
}
return result;
}
+
+ private EntitySummary addSensorsByGlobs(EntitySummary result, Entity entity, List<String> extraSensorGlobs) {
+ if (extraSensorGlobs!=null && !extraSensorGlobs.isEmpty()) {
+ Object sensorsO = result.getExtraFields().get("sensors");
+ if (sensorsO==null) {
+ sensorsO = MutableMap.of();
+ result.setExtraField("sensors", sensorsO);
+ }
+ if (!(sensorsO instanceof Map)) {
+ throw new IllegalStateException("sensors field in result for "+entity+" should be a Map; found: "+sensorsO);
+ }
+ @SuppressWarnings("unchecked")
+ Map<String,Object> sensors = (Map<String, Object>) sensorsO;
+
+ Map<AttributeSensor<?>, Object> svs = entity.sensors().getAll();
+ svs.entrySet().stream().forEach(kv -> {
+ Object sv = kv.getValue();
+ if (sv!=null) {
+ String name = kv.getKey().getName();
+ if (extraSensorGlobs.stream().anyMatch(sn -> WildcardGlobs.isGlobMatched(sn, name))) {
+ sv = resolving(sv).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve();
+ sensors.put(name, sv);
+ }
+ }
+ });
+ }
+ return result;
+ }
+
+ private EntitySummary addConfigByGlobs(EntitySummary result, Entity entity, List<String> extraConfigGlobs) {
+ if (extraConfigGlobs!=null && !extraConfigGlobs.isEmpty()) {
+ Object configO = result.getExtraFields().get("config");
+ if (configO==null) {
+ configO = MutableMap.of();
+ result.setExtraField("config", configO);
+ }
+ if (!(configO instanceof Map)) {
+ throw new IllegalStateException("config field in result for "+entity+" should be a Map; found: "+configO);
+ }
+ @SuppressWarnings("unchecked")
+ Map<String,Object> configs = (Map<String, Object>) configO;
+
+ List<Predicate<ConfigKey<?>>> extraConfigPreds = MutableList.of();
+ extraConfigGlobs.stream().forEach(g -> { extraConfigPreds.add( ConfigPredicates.nameMatchesGlob(g) ); });
+ entity.config().findKeysDeclared(Predicates.or(extraConfigPreds)).forEach(key -> {
+ Object v = entity.config().get(key);
+ if (v!=null) {
+ v = resolving(v).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve();
+ configs.put(key.getName(), v);
--- End diff --
done
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
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/967#discussion_r192680963
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -106,7 +112,16 @@
@Context
private UriInfo uriInfo;
- private EntityDetail fromEntity(Entity entity) {
+ private EntitySummary fromEntity(Entity entity, boolean includeTags, int detailDepth, List<String> extraSensorGlobs, List<String> extraConfigGlobs) {
+ if (detailDepth==0) {
--- End diff --
If `detailDepth==0`, it will ignore the `extraSensorGlobs` and `extraConfigGlobs`. That doesn't feel intuitive - is it deliberate? I interpreted "with references to children but not their details" as meaning we'd see the entity ids, but we wouldn't have a record for them.
The deliberate behaviour for negative depth is also not intuitive (i.e. will contradict what a maintainer thinks 'depth' means) and subtle.
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r192711842
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -194,16 +218,42 @@ private EntityDetail fromEntity(Entity entity) {
Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
while (entity != null && entity.getParent() != null) {
if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
- entitySummaries.add(addSensors(fromEntity(entity), entity, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(entity, false, -1, null, null), entity, extraSensors));
}
entity = entity.getParent();
}
}
}
return entitySummaries;
}
+
+ @Override
+ public List<EntitySummary> details(String entityIds, boolean includeAllApps, String extraSensorsGlobsS, String extraConfigGlobsS, int depth) {
+ List<String> extraSensorGlobs = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsGlobsS);
+
+ Map<String, EntitySummary> entitySummaries = MutableMap.of();
+ if (includeAllApps) {
+ for (Entity application : mgmt().getApplications()) {
+ entitySummaries.put(application.getId(), fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs));
+ }
+ }
+
+ if (Strings.isNonBlank(entityIds)) {
+ List<String> extraEntities = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(entityIds);
+ for (String entityId: extraEntities) {
+ Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
+ while (entity != null && !entitySummaries.containsKey(entity.getId())) {
+ if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
+ entitySummaries.put(entity.getId(), fromEntity(entity, true, depth, extraSensorGlobs, extraSensorGlobs));
--- End diff --
same as above
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
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/967#discussion_r192686869
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -226,6 +276,60 @@ private EntityDetail addSensors(EntityDetail result, Entity entity, List<Attribu
}
return result;
}
+
+ private EntitySummary addSensorsByGlobs(EntitySummary result, Entity entity, List<String> extraSensorGlobs) {
+ if (extraSensorGlobs!=null && !extraSensorGlobs.isEmpty()) {
+ Object sensorsO = result.getExtraFields().get("sensors");
+ if (sensorsO==null) {
+ sensorsO = MutableMap.of();
+ result.setExtraField("sensors", sensorsO);
+ }
+ if (!(sensorsO instanceof Map)) {
+ throw new IllegalStateException("sensors field in result for "+entity+" should be a Map; found: "+sensorsO);
+ }
+ @SuppressWarnings("unchecked")
+ Map<String,Object> sensors = (Map<String, Object>) sensorsO;
+
+ Map<AttributeSensor<?>, Object> svs = entity.sensors().getAll();
+ svs.entrySet().stream().forEach(kv -> {
+ Object sv = kv.getValue();
+ if (sv!=null) {
+ String name = kv.getKey().getName();
+ if (extraSensorGlobs.stream().anyMatch(sn -> WildcardGlobs.isGlobMatched(sn, name))) {
+ sv = resolving(sv).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve();
+ sensors.put(name, sv);
--- End diff --
Should this be guarded with `Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_SENSOR, new EntityAndItem<String>(entity, name))`
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194661727
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -194,16 +218,42 @@ private EntityDetail fromEntity(Entity entity) {
Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
while (entity != null && entity.getParent() != null) {
if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
- entitySummaries.add(addSensors(fromEntity(entity), entity, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(entity, false, -1, null, null), entity, extraSensors));
}
entity = entity.getParent();
}
}
}
return entitySummaries;
}
+
+ @Override
+ public List<EntitySummary> details(String entityIds, boolean includeAllApps, String extraSensorsGlobsS, String extraConfigGlobsS, int depth) {
+ List<String> extraSensorGlobs = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsGlobsS);
+
+ Map<String, EntitySummary> entitySummaries = MutableMap.of();
+ if (includeAllApps) {
+ for (Entity application : mgmt().getApplications()) {
+ entitySummaries.put(application.getId(), fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs));
--- End diff --
good spot, fixed
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
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/967#discussion_r192681830
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -182,10 +206,10 @@ private EntityDetail fromEntity(Entity entity) {
public List<EntityDetail> fetch(String entityIds, String extraSensorsS) {
List<String> extraSensorNames = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsS);
List<AttributeSensor<?>> extraSensors = extraSensorNames.stream().map((s) -> Sensors.newSensor(Object.class, s)).collect(Collectors.toList());
-
+
List<EntityDetail> entitySummaries = Lists.newArrayList();
for (Entity application : mgmt().getApplications()) {
- entitySummaries.add(addSensors(fromEntity(application), application, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(application, false, -1, null, null), application, extraSensors));
--- End diff --
It looks like this is doing something sneaky (passing in depth `-1`, so the recursion will never terminate early by reaching depth `0`). If I'm reading that right, it really needs to be documented. Someone could easily change the behaviour of `fromEntity` in the future without realising that someone was relying on negative depth to mean infinite!
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194658631
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -194,16 +218,42 @@ private EntityDetail fromEntity(Entity entity) {
Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
while (entity != null && entity.getParent() != null) {
if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
- entitySummaries.add(addSensors(fromEntity(entity), entity, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(entity, false, -1, null, null), entity, extraSensors));
}
entity = entity.getParent();
}
}
}
return entitySummaries;
}
+
+ @Override
+ public List<EntitySummary> details(String entityIds, boolean includeAllApps, String extraSensorsGlobsS, String extraConfigGlobsS, int depth) {
+ List<String> extraSensorGlobs = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsGlobsS);
+
+ Map<String, EntitySummary> entitySummaries = MutableMap.of();
+ if (includeAllApps) {
+ for (Entity application : mgmt().getApplications()) {
+ entitySummaries.put(application.getId(), fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs));
--- End diff --
good spot, done, and for descendants that should also have a guard. both done.
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r192716302
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -106,7 +112,16 @@
@Context
private UriInfo uriInfo;
- private EntityDetail fromEntity(Entity entity) {
+ private EntitySummary fromEntity(Entity entity, boolean includeTags, int detailDepth, List<String> extraSensorGlobs, List<String> extraConfigGlobs) {
+ if (detailDepth==0) {
+ return new EntitySummary(
+ entity.getId(),
+ entity.getDisplayName(),
+ entity.getEntityType().getName(),
+ entity.getCatalogItemId(),
+ MutableMap.of("self", EntityTransformer.entityUri(entity, ui.getBaseUriBuilder())) );
--- End diff --
An observation - not something necessary but I'd suggest let's add the `self` link into the entity in all cases; this would be more consistent and would mean you could always just use the link if you wanted it, rather than have to check whether it is there and calculate it if not. It would just mean an extra parameter in the entity detail constructor at 161 below.
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
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/967#discussion_r192687341
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -226,6 +276,60 @@ private EntityDetail addSensors(EntityDetail result, Entity entity, List<Attribu
}
return result;
}
+
+ private EntitySummary addSensorsByGlobs(EntitySummary result, Entity entity, List<String> extraSensorGlobs) {
+ if (extraSensorGlobs!=null && !extraSensorGlobs.isEmpty()) {
+ Object sensorsO = result.getExtraFields().get("sensors");
+ if (sensorsO==null) {
+ sensorsO = MutableMap.of();
+ result.setExtraField("sensors", sensorsO);
+ }
+ if (!(sensorsO instanceof Map)) {
+ throw new IllegalStateException("sensors field in result for "+entity+" should be a Map; found: "+sensorsO);
+ }
+ @SuppressWarnings("unchecked")
+ Map<String,Object> sensors = (Map<String, Object>) sensorsO;
+
+ Map<AttributeSensor<?>, Object> svs = entity.sensors().getAll();
+ svs.entrySet().stream().forEach(kv -> {
+ Object sv = kv.getValue();
+ if (sv!=null) {
+ String name = kv.getKey().getName();
+ if (extraSensorGlobs.stream().anyMatch(sn -> WildcardGlobs.isGlobMatched(sn, name))) {
+ sv = resolving(sv).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve();
+ sensors.put(name, sv);
+ }
+ }
+ });
+ }
+ return result;
+ }
+
+ private EntitySummary addConfigByGlobs(EntitySummary result, Entity entity, List<String> extraConfigGlobs) {
+ if (extraConfigGlobs!=null && !extraConfigGlobs.isEmpty()) {
+ Object configO = result.getExtraFields().get("config");
+ if (configO==null) {
+ configO = MutableMap.of();
+ result.setExtraField("config", configO);
+ }
+ if (!(configO instanceof Map)) {
+ throw new IllegalStateException("config field in result for "+entity+" should be a Map; found: "+configO);
+ }
+ @SuppressWarnings("unchecked")
+ Map<String,Object> configs = (Map<String, Object>) configO;
+
+ List<Predicate<ConfigKey<?>>> extraConfigPreds = MutableList.of();
+ extraConfigGlobs.stream().forEach(g -> { extraConfigPreds.add( ConfigPredicates.nameMatchesGlob(g) ); });
+ entity.config().findKeysDeclared(Predicates.or(extraConfigPreds)).forEach(key -> {
+ Object v = entity.config().get(key);
+ if (v!=null) {
+ v = resolving(v).preferJson(true).asJerseyOutermostReturnValue(false).raw(true).context(entity).immediately(true).timeout(Duration.ZERO).resolve();
+ configs.put(key.getName(), v);
--- End diff --
Should this be guarded with `Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_CONFIG, new EntityAndItem<String>(entity, key.getName()))`
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194658482
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java ---
@@ -53,12 +53,46 @@
@Consumes(MediaType.APPLICATION_JSON)
public interface ApplicationApi {
+ @GET
+ @Path("/details")
+ @ApiOperation(
+ value = "Get details for all applications and optionally selected additional entity items, "
+ + "including tags, values for selected sensor and config glob patterns, "
+ + "and recursively this info for children, up to a given depth."
+ )
+ public List<EntitySummary> details(
+ @ApiParam(value="Any additional entity ID's to include, as JSON or comma-separated list; ancestors will also be included", required=false)
+ @DefaultValue("")
+ @QueryParam("items") String items,
+ @ApiParam(value="Whether to include all applications in addition to any explicitly requested IDs; "
+ + "default is true so no items need to be listed; "
+ + "set false to return only info for entities whose IDs are listed in `items` and their ancestors", required=false)
+ @DefaultValue("true")
+ @QueryParam("includeAllApps") boolean includeAllApps,
+ @ApiParam(value="Any additional sensors to include, as JSON or comma-separated list, accepting globs (* and ?); "
+ + "current sensor values if present are returned for each entity in a name-value map under the 'sensors' key", required=false)
+ @DefaultValue("")
+ @QueryParam("sensors") String sensors,
+ @ApiParam(value="Any config to include, as JSON or comma-separated list, accepting globs (* and ?); "
+ + "current config values if present are returned for each entity in a name-value map under the 'config' key", required=false)
+ @DefaultValue("")
+ @QueryParam("config") String config,
+ @ApiParam(value="Tree depth to traverse in children for returning detail; "
+ + "default 1 means to have detail for just applications and additional entity IDs explicitly requested, "
+ + "with references to children but not their details", required=false)
+ @DefaultValue("1")
+ @QueryParam("depth") int depth);
+
@GET
@Path("/fetch")
@ApiOperation(
value = "Fetch details for all applications and optionally selected additional entity items, "
- + "optionally also with the values for selected sensors"
+ + "optionally also with the values for selected sensors. "
+ + "Deprecated since 1.0.0. Use the '/details' endpoint with better semantics. "
+ + "(This returns the complete tree which is wasteful and not usually wanted.)"
)
+ @Deprecated
+ /** @deprecated since 1.0.0 use {@link #details(String, String, int)} */
--- End diff --
agree, done
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/967
---
[GitHub] brooklyn-server issue #967: add an /applications/details endpoint which corr...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/967
tests added and all comments addressed except for `depth` as noted (which now has improved docs/comments)
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
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/967#discussion_r192682508
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -194,16 +218,42 @@ private EntityDetail fromEntity(Entity entity) {
Entity entity = mgmt().getEntityManager().getEntity(entityId.trim());
while (entity != null && entity.getParent() != null) {
if (Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.SEE_ENTITY, entity)) {
- entitySummaries.add(addSensors(fromEntity(entity), entity, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(entity, false, -1, null, null), entity, extraSensors));
}
entity = entity.getParent();
}
}
}
return entitySummaries;
}
+
+ @Override
+ public List<EntitySummary> details(String entityIds, boolean includeAllApps, String extraSensorsGlobsS, String extraConfigGlobsS, int depth) {
+ List<String> extraSensorGlobs = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsGlobsS);
+
+ Map<String, EntitySummary> entitySummaries = MutableMap.of();
+ if (includeAllApps) {
+ for (Entity application : mgmt().getApplications()) {
+ entitySummaries.put(application.getId(), fromEntity(application, true, depth, extraSensorGlobs, extraSensorGlobs));
--- End diff --
Should guard this with `Entitlements.isEntitled` for each of the apps.
Are we happy to assume that if you can see an entity, then you should be allowed to see all of that entity's children?
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
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/967#discussion_r192679474
--- Diff: rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/ApplicationApi.java ---
@@ -53,12 +53,46 @@
@Consumes(MediaType.APPLICATION_JSON)
public interface ApplicationApi {
+ @GET
+ @Path("/details")
+ @ApiOperation(
+ value = "Get details for all applications and optionally selected additional entity items, "
+ + "including tags, values for selected sensor and config glob patterns, "
+ + "and recursively this info for children, up to a given depth."
+ )
+ public List<EntitySummary> details(
+ @ApiParam(value="Any additional entity ID's to include, as JSON or comma-separated list; ancestors will also be included", required=false)
+ @DefaultValue("")
+ @QueryParam("items") String items,
+ @ApiParam(value="Whether to include all applications in addition to any explicitly requested IDs; "
+ + "default is true so no items need to be listed; "
+ + "set false to return only info for entities whose IDs are listed in `items` and their ancestors", required=false)
+ @DefaultValue("true")
+ @QueryParam("includeAllApps") boolean includeAllApps,
+ @ApiParam(value="Any additional sensors to include, as JSON or comma-separated list, accepting globs (* and ?); "
+ + "current sensor values if present are returned for each entity in a name-value map under the 'sensors' key", required=false)
+ @DefaultValue("")
+ @QueryParam("sensors") String sensors,
+ @ApiParam(value="Any config to include, as JSON or comma-separated list, accepting globs (* and ?); "
+ + "current config values if present are returned for each entity in a name-value map under the 'config' key", required=false)
+ @DefaultValue("")
+ @QueryParam("config") String config,
+ @ApiParam(value="Tree depth to traverse in children for returning detail; "
+ + "default 1 means to have detail for just applications and additional entity IDs explicitly requested, "
+ + "with references to children but not their details", required=false)
+ @DefaultValue("1")
+ @QueryParam("depth") int depth);
+
@GET
@Path("/fetch")
@ApiOperation(
value = "Fetch details for all applications and optionally selected additional entity items, "
- + "optionally also with the values for selected sensors"
+ + "optionally also with the values for selected sensors. "
+ + "Deprecated since 1.0.0. Use the '/details' endpoint with better semantics. "
+ + "(This returns the complete tree which is wasteful and not usually wanted.)"
)
+ @Deprecated
+ /** @deprecated since 1.0.0 use {@link #details(String, String, int)} */
--- End diff --
Signature of link is wrong: should be {@link #details(String, boolean, String, String, int)}`
---
[GitHub] brooklyn-server issue #967: add an /applications/details endpoint which corr...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/967
Thanks @aledsage @geomacy . Great comments.
I'll add tests and push all changes addressed except depth.
On maxDepth, it's a not uncommon pattern that <0 is infinite, and 0 could to me logically mean _either_ no detail at level 0, or yes detail at level 0 but 0 additional levels. I've gone with the former semantics as strictly more useful (in terms of functionality). The fact that the default is 1 and this is explained I think is sufficient.
Agree both are a little subtle but there is not an elegant way I can see to make it clearer; it involves additional fields with some redundancy.
If you feel strongly happy to go with an alternate API that preserves functionality -- please suggest -- however this as is my preferred.
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194661916
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -106,7 +112,16 @@
@Context
private UriInfo uriInfo;
- private EntityDetail fromEntity(Entity entity) {
+ private EntitySummary fromEntity(Entity entity, boolean includeTags, int detailDepth, List<String> extraSensorGlobs, List<String> extraConfigGlobs) {
+ if (detailDepth==0) {
--- End diff --
see below
---
[GitHub] brooklyn-server pull request #967: add an /applications/details endpoint whi...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/967#discussion_r194661902
--- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java ---
@@ -182,10 +206,10 @@ private EntityDetail fromEntity(Entity entity) {
public List<EntityDetail> fetch(String entityIds, String extraSensorsS) {
List<String> extraSensorNames = JavaStringEscapes.unwrapOptionallyQuotedJavaStringList(extraSensorsS);
List<AttributeSensor<?>> extraSensors = extraSensorNames.stream().map((s) -> Sensors.newSensor(Object.class, s)).collect(Collectors.toList());
-
+
List<EntityDetail> entitySummaries = Lists.newArrayList();
for (Entity application : mgmt().getApplications()) {
- entitySummaries.add(addSensors(fromEntity(application), application, extraSensors));
+ entitySummaries.add(addSensorsByName((EntityDetail)fromEntity(application, false, -1, null, null), application, extraSensors));
--- End diff --
see below
---