You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2015/08/19 15:47:52 UTC

[GitHub] incubator-brooklyn pull request: Feature: catalog item disabled

GitHub user aledsage opened a pull request:

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

    Feature: catalog item disabled

    As discussed on mailing list.
    
    (I still need to rebase this against master, after all the package renames, so it cannot yet be merged).

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

    $ git pull https://github.com/aledsage/incubator-brooklyn feature/CatalogItem-disabled

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

    https://github.com/apache/incubator-brooklyn/pull/850.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 #850
    
----
commit 4bf72a824a3ec802030d56c021542736723b1ae0
Author: Aled Sage <al...@gmail.com>
Date:   2015-08-19T13:38:40Z

    CatalogPredicates: test + not anonymous classes

commit cc28743fa3059a5d61d53ee1d043f0e927831d79
Author: Aled Sage <al...@gmail.com>
Date:   2015-08-19T13:46:33Z

    Adds CatalogItem.disabled
    
    - BrooklynCatalog filters these out for getDefaultVersion
    - REST’s CatalogResource filters these out for listApplications, 
      listEntities, listLocations and listPolicies.
    - Applications refuse to deploy when referenced catalog item is disabled
    - log.warn when deprecated catalog item is used.

----


---
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: Feature: catalog item disabled

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/850#discussion_r37578037
  
    --- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java ---
    @@ -100,8 +100,15 @@
     
         public void setDeprecated(boolean deprecated);
     
    +    public void setDisabled(boolean disabled);
    +
         /**
    -     * @return True if the item has been deprecated and should not be shown in the catalog
    +     * @return True if the item has been deprecated (i.e. it use is discouraged)
          */
         boolean isDeprecated();
    +    
    +    /**
    +     * @return True if the item has been disabled (i.e. it use is forbidden, except for pre-existing apps)
    --- End diff --
    
    **its** use


---
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: Feature: catalog item disabled

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

    https://github.com/apache/incubator-brooklyn/pull/850#issuecomment-133151885
  
    @ahgittin @ygy @bostko This is ready for review - could you take a look please?


---
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: Feature: catalog item disabled

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/850#discussion_r37578668
  
    --- Diff: usage/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java ---
    @@ -314,12 +314,39 @@ public Response getIcon(
             @ApiParam(name = "version", value = "version identifier of catalog item (application, entity, policy, location)", required=true)
             @PathParam("version") String version);
     
    +    /**
    +     * @deprecated since 0.8.0; use "/entities/{itemId}/deprecated" with payload of true/false
    +     */
    +    @Deprecated
         @POST
         @Path("/entities/{itemId}/deprecated/{deprecated}")
    -    public void setDeprecated(
    +    public void setDeprecatedLegacy(
             @ApiParam(name = "itemId", value = "The ID of the catalog item to be deprecated", required = true)
             @PathParam("itemId") String itemId,
             @ApiParam(name = "deprecated", value = "Whether or not the catalog item is deprecated", required = true)
             @PathParam("deprecated") boolean deprecated);
    +    
    +    @POST
    +    @Consumes({MediaType.APPLICATION_JSON, MediaType.APPLICATION_OCTET_STREAM, MediaType.TEXT_PLAIN})
    +    @ApiErrors(value = {
    +            @ApiError(code = 404, reason = "Undefined catalog item"),
    +    })
    +    @Path("/entities/{itemId}/disabled")
    +    public void setDisabled(
    +        @ApiParam(name = "itemId", value = "The ID of the catalog item to be disabled", required = true)
    +        @PathParam("itemId") String itemId,
    +        @ApiParam(name = "deprecated", value = "Whether or not the catalog item is disabled", required = true)
    --- End diff --
    
    `name=disabled`


---
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: Feature: catalog item disabled

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/850#discussion_r37578250
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/location/CatalogLocationResolver.java ---
    @@ -55,6 +55,12 @@ public void init(ManagementContext managementContext) {
         public Location newLocationFromString(Map locationFlags, String spec, LocationRegistry registry) {
             String id = spec.substring(NAME.length()+1);
             CatalogItem<?, ?> item = CatalogUtils.getCatalogItemOptionalVersion(managementContext, id);
    +        if (item.isDeprecated()) {
    +            log.warn("Use of deprecated catalog item "+item.getSymbolicName()+":"+item.getVersion());
    +        } else if (item.isDisabled()) {
    --- End diff --
    
    need to check disabled first


---
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: Feature: catalog item disabled

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/850#discussion_r37578177
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java ---
    @@ -94,49 +165,119 @@ public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> symbolicName(final Predicate<? super String> filter) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && filter.apply(item.getSymbolicName());
                 }
             };
    +        return new SymbolicNameMatches<T,SpecT>(filter);
    +    }
    +    
    +    private static class SymbolicNameMatches<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final Predicate<? super String> filter;
    +        
    +        public SymbolicNameMatches(Predicate<? super String> filter) {
    +            this.filter = filter;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && filter.apply(item.getSymbolicName());
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> javaType(final Predicate<? super String> filter) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && filter.apply(item.getJavaType());
                 }
             };
    +        return new JavaTypeMatches<T, SpecT>(filter);
    +    }
    +    
    +    private static class JavaTypeMatches<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final Predicate<? super String> filter;
    +        
    +        public JavaTypeMatches(Predicate<? super String> filter) {
    +            this.filter = filter;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && filter.apply(item.getJavaType());
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> xml(final Predicate<? super String> filter) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && filter.apply(item.toXmlString());
                 }
             };
    +        return new XmlMatches<T,SpecT>(filter);
    +    }
    +    
    +    private static class XmlMatches<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final Predicate<? super String> filter;
    +        
    +        public XmlMatches(Predicate<? super String> filter) {
    +            this.filter = filter;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && filter.apply(item.toXmlString());
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> entitledToSee(final ManagementContext mgmt) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    --- End diff --
    
    `@since`


---
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: Feature: catalog item disabled

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

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


---
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: Feature: catalog item disabled

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/850#discussion_r37578101
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java ---
    @@ -39,21 +39,65 @@
     public class CatalogPredicates {
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> isCatalogItemType(final CatalogItemType ciType) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    --- End diff --
    
    `@since 0.8.0` ? 


---
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: Feature: catalog item disabled

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

    https://github.com/apache/incubator-brooklyn/pull/850#issuecomment-134982456
  
    Incorporated comments from @ahgittin (thanks!); build successful - merging now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Feature: catalog item disabled

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/850#discussion_r37959566
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java ---
    @@ -39,21 +39,65 @@
     public class CatalogPredicates {
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> isCatalogItemType(final CatalogItemType ciType) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    --- End diff --
    
    Are you saying to add this to the `private static class` that I added, which this now uses? Not sure how `@since 0.8.0` relates to this given line?


---
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: Feature: catalog item disabled

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/850#discussion_r37578196
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java ---
    @@ -94,49 +165,119 @@ public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> symbolicName(final Predicate<? super String> filter) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && filter.apply(item.getSymbolicName());
                 }
             };
    +        return new SymbolicNameMatches<T,SpecT>(filter);
    +    }
    +    
    +    private static class SymbolicNameMatches<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final Predicate<? super String> filter;
    +        
    +        public SymbolicNameMatches(Predicate<? super String> filter) {
    +            this.filter = filter;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && filter.apply(item.getSymbolicName());
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> javaType(final Predicate<? super String> filter) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && filter.apply(item.getJavaType());
                 }
             };
    +        return new JavaTypeMatches<T, SpecT>(filter);
    +    }
    +    
    +    private static class JavaTypeMatches<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final Predicate<? super String> filter;
    +        
    +        public JavaTypeMatches(Predicate<? super String> filter) {
    +            this.filter = filter;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && filter.apply(item.getJavaType());
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> xml(final Predicate<? super String> filter) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && filter.apply(item.toXmlString());
                 }
             };
    +        return new XmlMatches<T,SpecT>(filter);
    +    }
    +    
    +    private static class XmlMatches<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final Predicate<? super String> filter;
    +        
    +        public XmlMatches(Predicate<? super String> filter) {
    +            this.filter = filter;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && filter.apply(item.toXmlString());
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> entitledToSee(final ManagementContext mgmt) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && 
                         Entitlements.isEntitled(mgmt.getEntitlementManager(), Entitlements.SEE_CATALOG_ITEM, item.getCatalogItemId());
                 }
             };
    +        return new EntitledToSee<T, SpecT>(mgmt);
    +    }
    +    
    +    private static class EntitledToSee<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final ManagementContext mgmt;
    +        
    +        public EntitledToSee(ManagementContext mgmt) {
    +            this.mgmt = mgmt;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && 
    +                    Entitlements.isEntitled(mgmt.getEntitlementManager(), Entitlements.SEE_CATALOG_ITEM, item.getCatalogItemId());
    +        }
         }
      
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> isBestVersion(final ManagementContext mgmt) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    --- End diff --
    
    `@since`


---
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: Feature: catalog item disabled

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/850#discussion_r37578011
  
    --- Diff: api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java ---
    @@ -100,8 +100,15 @@
     
         public void setDeprecated(boolean deprecated);
     
    +    public void setDisabled(boolean disabled);
    +
         /**
    -     * @return True if the item has been deprecated and should not be shown in the catalog
    +     * @return True if the item has been deprecated (i.e. it use is discouraged)
    --- End diff --
    
    **its** use is discouraged


---
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: Feature: catalog item disabled

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

    https://github.com/apache/incubator-brooklyn/pull/850#issuecomment-133165083
  
    looks great.  lots of minor stuff, and haven't tried the tests, but happy to see this merged when you 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: Feature: catalog item disabled

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/850#discussion_r37578140
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/CatalogPredicates.java ---
    @@ -39,21 +39,65 @@
     public class CatalogPredicates {
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> isCatalogItemType(final CatalogItemType ciType) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    +        new Predicate<CatalogItem<T,SpecT>>() {
                 @Override
                 public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
                     return (item != null) && item.getCatalogItemType()==ciType;
                 }
             };
    +        return new CatalogItemTypeEqualTo<T, SpecT>(ciType);
    +    }
    +
    +    private static class CatalogItemTypeEqualTo<T,SpecT> implements Predicate<CatalogItem<T,SpecT>> {
    +        private final CatalogItemType ciType;
    +        
    +        public CatalogItemTypeEqualTo(final CatalogItemType ciType) {
    +            this.ciType = ciType;
    +        }
    +        @Override
    +        public boolean apply(@Nullable CatalogItem<T,SpecT> item) {
    +            return (item != null) && item.getCatalogItemType()==ciType;
    +        }
         }
     
         public static <T,SpecT> Predicate<CatalogItem<T,SpecT>> deprecated(final boolean deprecated) {
    -        return new Predicate<CatalogItem<T,SpecT>>() {
    +        // TODO PERSISTENCE WORKAROUND kept anonymous function in case referenced in persisted state
    --- End diff --
    
    as above, `@since`


---
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: Feature: catalog item disabled

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/850#discussion_r37578972
  
    --- Diff: usage/rest-server/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java ---
    @@ -254,7 +256,34 @@ private void addTestCatalogItem(String catalogItemId, String itemType, String ve
             client().resource("/v1/catalog").post(yaml);
         }
     
    +    /**
    +     * @deprecated since 0.8.0; delete when no longer support legacy REST api way of deprecating catalog items!
    --- End diff --
    
    we can probably weaken our deprecation policy wrt tests :)


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