Repository: incubator-brooklyn Updated Branches: refs/heads/master d2beaa7de -> b08fe4eca
clean up how locations are rendered in the js gui, and expand "delete" there was (is) confusion between catalog name and location displayName and a named location. it deserves a major sort-out, i think treating location defs as catalog items and location instances as entities; and only using the catalog api, not a dedicated locations api (or if there is, it delegates to catalog), also removing all the LocationManager blah. but until then we needed something which avoided confusion where catalog item names for locations aren't rendered, and magic display names from location-metadata were being showed so it looked like nothing was being added, though the location worked. for locations, a catalog item name now does nothing, but the location's name (i.e. human-readable ID aka catalog item ID, not the display name) is shown in the catalog accordion list (the "IdentifierName"), better info (but not the full yaml yet) is shown in the detail, and versions are at least handled without bugs, even if only one version for location is really supported in the gui. also adds delete for entity and apps and policies, and fixes delete for locations. Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/91532f39 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/91532f39 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/91532f39 Branch: refs/heads/master Commit: 91532f39a43bdf6c1d46210c599503c1166c2215 Parents: 3650dfa Author: Alex Heneveld <[email protected]> Authored: Sun Jun 21 12:21:21 2015 -0700 Committer: Alex Heneveld <[email protected]> Committed: Sun Jun 21 14:30:07 2015 -0700 ---------------------------------------------------------------------- .../catalog/internal/BasicBrooklynCatalog.java | 4 ++ .../assets/js/model/catalog-item-summary.js | 7 +-- .../src/main/webapp/assets/js/model/location.js | 6 +++ .../src/main/webapp/assets/js/view/catalog.js | 46 +++++++++++++++++--- .../assets/tpl/catalog/details-entity.html | 5 +++ .../assets/tpl/catalog/details-generic.html | 4 ++ .../assets/tpl/catalog/details-location.html | 22 ++++++---- .../main/java/brooklyn/rest/api/CatalogApi.java | 13 ++++++ .../rest/resources/CatalogResource.java | 5 +++ .../rest/resources/LocationResource.java | 34 +++++++++++++-- 10 files changed, 124 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java index 421794c..31458f5 100644 --- a/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java @@ -355,6 +355,10 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { default: throw new RuntimeException("Only entity, policy & location catalog items are supported. Unsupported catalog item type " + item.getCatalogItemType()); } ((AbstractBrooklynObjectSpec<?, ?>)spec).catalogItemId(item.getId()); + + if (Strings.isBlank( ((AbstractBrooklynObjectSpec<?, ?>)spec).getDisplayName() )) + ((AbstractBrooklynObjectSpec<?, ?>)spec).displayName(item.getDisplayName()); + return spec; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/jsgui/src/main/webapp/assets/js/model/catalog-item-summary.js ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/main/webapp/assets/js/model/catalog-item-summary.js b/usage/jsgui/src/main/webapp/assets/js/model/catalog-item-summary.js index 2d8da58..60d1eda 100644 --- a/usage/jsgui/src/main/webapp/assets/js/model/catalog-item-summary.js +++ b/usage/jsgui/src/main/webapp/assets/js/model/catalog-item-summary.js @@ -19,8 +19,9 @@ define(["underscore", "backbone"], function (_, Backbone) { // NB: THIS IS NOT USED CURRENTLY - // the logic in application-add-wizard.js simply loads and manipulates json - // TODO change that so that it uses this backbone model + collection + // the logic in application-add-wizard.js simply loads and manipulates json; + // logic in catalog.js (view) defines its own local model + // TODO change those so that they use this backbone model + collection, // allowing a way to specify on creation what we are looking up in the catalog -- apps or entities or policies var CatalogItem = {} @@ -40,7 +41,7 @@ define(["underscore", "backbone"], function (_, Backbone) { CatalogItem.Collection = Backbone.Collection.extend({ model:CatalogItem.Model, - url:'/v1/catalog' // TODO is this application or entities or policies? + url:'/v1/catalog' // TODO is this application or entities or policies? (but note THIS IS NOT USED) }) return CatalogItem http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/jsgui/src/main/webapp/assets/js/model/location.js ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/main/webapp/assets/js/model/location.js b/usage/jsgui/src/main/webapp/assets/js/model/location.js index 3bb3db2..4e33860 100644 --- a/usage/jsgui/src/main/webapp/assets/js/model/location.js +++ b/usage/jsgui/src/main/webapp/assets/js/model/location.js @@ -69,6 +69,12 @@ define(["underscore", "backbone"], function (_, Backbone) { name = this.get('name') if (name!=null && name.length>0) return name return this.get('spec') + }, + getIdentifierName: function() { + var name = null; + name = this.get('name') + if (name!=null && name.length>0) return name + return this.get('spec') } }) http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/jsgui/src/main/webapp/assets/js/view/catalog.js ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/main/webapp/assets/js/view/catalog.js b/usage/jsgui/src/main/webapp/assets/js/view/catalog.js index 68b3c6f..51d8a0a 100644 --- a/usage/jsgui/src/main/webapp/assets/js/view/catalog.js +++ b/usage/jsgui/src/main/webapp/assets/js/view/catalog.js @@ -113,9 +113,16 @@ define([ // Could use wait flag to block removal of model from collection // until server confirms deletion and success handler to perform // removal. Useful if delete fails for e.g. lack of entitlement. - this.activeModel.destroy(); - var displayName = $(event.currentTarget).data("name"); - this.renderEmpty(displayName ? "Deleted " + displayName : ""); + var that = this; + var displayName = $(event.currentTarget).data("name") || "item"; + this.activeModel.destroy({ + success: function() { + that.renderEmpty("Deleted " + displayName); + }, + error: function(info) { + that.renderEmpty("Unable to permanently delete " + displayName+". Deletion is temporary, client-side only."); + } + }); } }); @@ -259,6 +266,11 @@ define([ } var Catalog = Backbone.Collection.extend({ + modelX: Backbone.Model.extend({ + url: function() { + return "/v1/catalog/" + this.name + "/" + this.id + "?allVersions=true"; + } + }), initialize: function(models, options) { this.name = options["name"]; if (!this.name) { @@ -267,7 +279,12 @@ define([ //this.model is a constructor so it shouldn't be _.bind'ed to this //It actually works when a browser provided .bind is used, but the //fallback implementation doesn't support it. - var model = this.model; + var that = this; + var model = this.model.extend({ + url: function() { + return "/v1/catalog/" + that.name + "/" + this.id.split(":").join("/"); + } + }); _.bindAll(this); this.model = model; }, @@ -466,7 +483,7 @@ define([ autoOpen: this.options.kind == "locations", entryTemplateArgs: function (location, index) { return { - type: location.getPrettyName(), + type: location.getIdentifierName(), id: location.getLinkByName("self") }; } @@ -536,13 +553,28 @@ define([ .then(function() { var model = accordion.collection.get(id); if (!model) { - // caller probably passed the wrong kind (in case of entity v app, the caller might try both) - } else { + // if a version is supplied, try it without a version - needed for locations, navigating after deletion + if (id && id.split(":").length>1) { + model = accordion.collection.get( id.split(":")[0] ); + } + } + if (!model) { + // if an ID is supplied without a version, look for first matching version (should be newest) + if (id && id.split(":").length==1 && accordion.collection.models) { + model = _.find(accordion.collection.models, function(m) { + return m && m.id && m.id.startsWith(id+":"); + }); + } + } + // TODO could look in collection for any starting with ID + if (model) { Backbone.history.navigate("/v1/catalog/"+kind+"/"+id); activeDetailsView = kind; accordion.activeCid = model.cid; accordion.options.onItemSelected(kind, model); accordion.show(); + } else { + // catalog item not found, or not found yet (it might be reloaded and another callback will try again) } }); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-entity.html ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-entity.html b/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-entity.html index f123913..2e43fe5 100644 --- a/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-entity.html +++ b/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-entity.html @@ -16,8 +16,13 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> + <div class="catalog-details"> + <div class="float-right"> + <button data-name="<%= model.id %>" class="btn btn-danger delete">Delete</button> + </div> + <% if (model.get("name") != model.get("symbolicName")) { %> <h2><%- model.get("name") %></h2> <p><%- model.getVersionedAttr("symbolicName") %></p> http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-generic.html ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-generic.html b/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-generic.html index f57500e..49a29a0 100644 --- a/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-generic.html +++ b/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-generic.html @@ -19,6 +19,10 @@ under the License. <div class="catalog-details"> + <div class="float-right"> + <button data-name="<%= model.id %>" class="btn btn-danger delete">Delete</button> + </div> + <% if (model.get("name") !== undefined) { %> <h2><%= model.get("name") %></h2> <% } else if (model.get("type") !== undefined) { %> http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-location.html ---------------------------------------------------------------------- diff --git a/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-location.html b/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-location.html index 27fe3e2..03dd596 100644 --- a/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-location.html +++ b/usage/jsgui/src/main/webapp/assets/tpl/catalog/details-location.html @@ -16,26 +16,30 @@ KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> + <div class="catalog-details"> - <h3><%- model.getPrettyName() %></h3> + <h3><%- model.getIdentifierName() %></h3> <div class="float-right"> - <button data-name="<%= model.getPrettyName() %>" class="btn btn-danger delete">Delete</button> + <button data-name="<%= model.getIdentifierName() %>" class="btn btn-danger delete">Delete</button> </div> -<% if (!model.get("config") || _.isEmpty(model.get("config"))) { %> - <em>No special configuration</em> -<% } else { %> - <br/> <table> - <tr><td><strong>ID:</strong> </td><td><%- model.get("id") || "" %></td></tr> - <tr><td><strong>Name:</strong> </td><td><%- model.get("name") || "" %></td></tr> - <tr><td><strong>Spec:</strong> </td><td><%- model.get("spec") || "" %></td></tr> + <tr><td><strong>Display Name:</strong> </td><td><%- model.getPrettyName() || "" %></td></tr> + <tr><td><strong>Reference Name:</strong> </td><td><%- model.get("name") || "" %></td></tr> + <tr><td><strong>Internal ID:</strong> </td><td><%- model.get("id") || "" %></td></tr> + <tr><td><strong>Implementation Spec:</strong> </td><td><%- model.get("spec") || "" %></td></tr> </table> <br/> + +<% if (!model.get("config") || _.isEmpty(model.get("config"))) { %> + <!-- either no config or it comes from a yaml plan and config not yet available; + TODO need to use the /v1/catalog/location API not the /v1/locations/ API --> +<% } else { %> + <table class="table table-striped table-condensed nonDatatables"> <thead><tr> <th>Configuration Key</th> http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java ---------------------------------------------------------------------- diff --git a/usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java b/usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java index d96de81..f757310 100644 --- a/usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java +++ b/usage/rest-api/src/main/java/brooklyn/rest/api/CatalogApi.java @@ -94,6 +94,19 @@ public interface CatalogApi { @PathParam("entityId") String entityId) throws Exception; @DELETE + @Path("/applications/{applicationId}/{version}") + @ApiOperation(value = "Deletes a specific version of an application's definition from the catalog") + @ApiErrors(value = { + @ApiError(code = 404, reason = "Entity not found") + }) + public void deleteApplication( + @ApiParam(name = "applicationId", value = "The ID of the application or template to delete", required = true) + @PathParam("applicationId") String entityId, + + @ApiParam(name = "version", value = "The version identifier of the application or template to delete", required = true) + @PathParam("version") String version) throws Exception; + + @DELETE @Path("/entities/{entityId}/{version}") @ApiOperation(value = "Deletes a specific version of an entity's definition from the catalog") @ApiErrors(value = { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java index 1e1bc28..344e939 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java @@ -160,6 +160,11 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat } @Override + public void deleteApplication(String applicationId, String version) throws Exception { + deleteEntity(applicationId, version); + } + + @Override public void deleteEntity(String entityId, String version) throws Exception { if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.MODIFY_CATALOG_ITEM, StringAndArgument.of(entityId+(Strings.isBlank(version) ? "" : ":"+version), "delete"))) { throw WebResourceUtils.unauthorized("User '%s' is not authorized to modify catalog", http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/91532f39/usage/rest-server/src/main/java/brooklyn/rest/resources/LocationResource.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/LocationResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/LocationResource.java index 74185d7..5909182 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/resources/LocationResource.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/LocationResource.java @@ -19,6 +19,7 @@ package brooklyn.rest.resources; import java.net.URI; +import java.util.Comparator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -29,13 +30,14 @@ import javax.ws.rs.core.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import brooklyn.catalog.CatalogItem; +import brooklyn.catalog.internal.CatalogUtils; import brooklyn.location.Location; import brooklyn.location.LocationDefinition; import brooklyn.location.basic.LocationConfigKeys; import brooklyn.rest.api.LocationApi; import brooklyn.rest.domain.LocationSpec; import brooklyn.rest.domain.LocationSummary; -import brooklyn.rest.domain.SummaryComparators; import brooklyn.rest.filter.HaHotStateRequired; import brooklyn.rest.transform.LocationTransformer; import brooklyn.rest.transform.LocationTransformer.LocationDetailLevel; @@ -43,6 +45,8 @@ import brooklyn.rest.util.EntityLocationUtils; import brooklyn.rest.util.WebResourceUtils; import brooklyn.util.collections.MutableMap; import brooklyn.util.exceptions.Exceptions; +import brooklyn.util.text.NaturalOrderComparator; +import brooklyn.util.text.Strings; import com.google.common.base.Function; import com.google.common.base.Joiner; @@ -82,7 +86,22 @@ public class LocationResource extends AbstractBrooklynRestResource implements Lo return FluentIterable.from(brooklyn().getLocationRegistry().getDefinedLocations().values()) .transform(transformer) .filter(LocationSummary.class) - .toSortedList(SummaryComparators.displayNameComparator()); + .toSortedList(nameOrSpecComparator()); + } + + private static NaturalOrderComparator COMPARATOR = new NaturalOrderComparator(); + private static Comparator<LocationSummary> nameOrSpecComparator() { + return new Comparator<LocationSummary>() { + @Override + public int compare(LocationSummary o1, LocationSummary o2) { + return COMPARATOR.compare(getNameOrSpec(o1).toLowerCase(), getNameOrSpec(o2).toLowerCase()); + } + }; + } + private static String getNameOrSpec(LocationSummary o) { + if (Strings.isNonBlank(o.getName())) return o.getName(); + if (Strings.isNonBlank(o.getSpec())) return o.getSpec(); + return o.getId(); } // this is here to support the web GUI's circles @@ -152,6 +171,15 @@ public class LocationResource extends AbstractBrooklynRestResource implements Lo @Override @Deprecated public void delete(String locationId) { - brooklyn().getCatalog().deleteCatalogItem(locationId); + // TODO make all locations be part of the catalog, then flip the JS GUI to use catalog api + if (deleteAllVersions(locationId)>0) return; + throw WebResourceUtils.notFound("No catalog item location matching %s; only catalog item locations can be deleted", locationId); + } + + private int deleteAllVersions(String locationId) { + CatalogItem<?, ?> item = CatalogUtils.getCatalogItemOptionalVersion(mgmt(), locationId); + if (item==null) return 0; + brooklyn().getCatalog().deleteCatalogItem(item.getSymbolicName(), item.getVersion()); + return 1 + deleteAllVersions(locationId); } }
