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

[GitHub] incubator-brooklyn pull request: add entitlements to EffectorResou...

GitHub user andreaturli opened a pull request:

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

    add entitlements to EffectorResource

    

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

    $ git pull https://github.com/andreaturli/incubator-brooklyn feature/effector-entitlements

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

    https://github.com/apache/incubator-brooklyn/pull/67.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 #67
    
----
commit 511e190c0aa81bda1003cc63db89a248b8d907fe
Author: Andrea Turli <an...@gmail.com>
Date:   2014-07-14T13:40:24Z

    add entitlements to EffectorResource

----


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

[GitHub] incubator-brooklyn pull request: add entitlements to EffectorResou...

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

    https://github.com/apache/incubator-brooklyn/pull/67#discussion_r14985040
  
    --- Diff: usage/rest-server/src/main/java/brooklyn/rest/resources/EffectorResource.java ---
    @@ -43,57 +48,61 @@
     import brooklyn.util.guava.Maybe;
     import brooklyn.util.time.Time;
     
    -import com.google.common.base.Function;
    -import com.google.common.collect.FluentIterable;
    -
     public class EffectorResource extends AbstractBrooklynRestResource implements EffectorApi {
     
         private static final Logger log = LoggerFactory.getLogger(EffectorResource.class);
     
    -   @Override
    -  public List<EffectorSummary> list(final String application, final String entityToken) {
    -       final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    -       return FluentIterable.from(entity.getEntityType().getEffectors())
    -            .transform(new Function<Effector<?>, EffectorSummary>() {
    -                @Override
    -                public EffectorSummary apply(Effector<?> effector) {
    -                    return EffectorTransformer.effectorSummary(entity, effector);
    -                }
    -            })
    -           .toSortedList(SummaryComparators.nameComparator());
    -   }
    +    @Override
    +    public List<EffectorSummary> list(final String application, final String entityToken) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +        return FluentIterable
    +                .from(entity.getEntityType().getEffectors())
    +                .filter(new Predicate<Effector<?>>() {
    +                    @Override
    +                    public boolean apply(@Nullable Effector<?> input) {
    +                        return Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.INVOKE_EFFECTOR,
    +                                Entitlements.EntityAndItem.of(entity, input.getName()));
    +                    }
    +                })
    +                .transform(new Function<Effector<?>, EffectorSummary>() {
    +                    @Override
    +                    public EffectorSummary apply(Effector<?> effector) {
    +                        return EffectorTransformer.effectorSummary(entity, effector);
    +                    }
    +                })
    +                .toSortedList(SummaryComparators.nameComparator());
    +    }
     
         @Override
    -  public Response invoke(String application, String entityToken, String effectorName,
    -      String timeout, Map<String, String> parameters
    -  ) {
    -      final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +    public Response invoke(String application, String entityToken, String effectorName,
    +                           String timeout, Map<String, String> parameters) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    --- End diff --
    
    @aledsage @ahgittin I think you are right, I've added the check.


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

[GitHub] incubator-brooklyn pull request: add entitlements to EffectorResou...

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/67#discussion_r14908392
  
    --- Diff: usage/rest-server/src/main/java/brooklyn/rest/resources/EffectorResource.java ---
    @@ -43,57 +48,61 @@
     import brooklyn.util.guava.Maybe;
     import brooklyn.util.time.Time;
     
    -import com.google.common.base.Function;
    -import com.google.common.collect.FluentIterable;
    -
     public class EffectorResource extends AbstractBrooklynRestResource implements EffectorApi {
     
         private static final Logger log = LoggerFactory.getLogger(EffectorResource.class);
     
    -   @Override
    -  public List<EffectorSummary> list(final String application, final String entityToken) {
    -       final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    -       return FluentIterable.from(entity.getEntityType().getEffectors())
    -            .transform(new Function<Effector<?>, EffectorSummary>() {
    -                @Override
    -                public EffectorSummary apply(Effector<?> effector) {
    -                    return EffectorTransformer.effectorSummary(entity, effector);
    -                }
    -            })
    -           .toSortedList(SummaryComparators.nameComparator());
    -   }
    +    @Override
    +    public List<EffectorSummary> list(final String application, final String entityToken) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +        return FluentIterable
    +                .from(entity.getEntityType().getEffectors())
    +                .filter(new Predicate<Effector<?>>() {
    +                    @Override
    +                    public boolean apply(@Nullable Effector<?> input) {
    +                        return Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.INVOKE_EFFECTOR,
    +                                Entitlements.EntityAndItem.of(entity, input.getName()));
    +                    }
    +                })
    +                .transform(new Function<Effector<?>, EffectorSummary>() {
    +                    @Override
    +                    public EffectorSummary apply(Effector<?> effector) {
    +                        return EffectorTransformer.effectorSummary(entity, effector);
    +                    }
    +                })
    +                .toSortedList(SummaryComparators.nameComparator());
    +    }
     
         @Override
    -  public Response invoke(String application, String entityToken, String effectorName,
    -      String timeout, Map<String, String> parameters
    -  ) {
    -      final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +    public Response invoke(String application, String entityToken, String effectorName,
    --- End diff --
    
    I can't see where invoke checks `Entitlements.isEntitled` - is that missing?


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

[GitHub] incubator-brooklyn pull request: add entitlements to EffectorResou...

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

    https://github.com/apache/incubator-brooklyn/pull/67#issuecomment-49196197
  
    looks good with this, 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: add entitlements to EffectorResou...

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/67#discussion_r14981957
  
    --- Diff: usage/rest-server/src/main/java/brooklyn/rest/resources/EffectorResource.java ---
    @@ -43,57 +48,61 @@
     import brooklyn.util.guava.Maybe;
     import brooklyn.util.time.Time;
     
    -import com.google.common.base.Function;
    -import com.google.common.collect.FluentIterable;
    -
     public class EffectorResource extends AbstractBrooklynRestResource implements EffectorApi {
     
         private static final Logger log = LoggerFactory.getLogger(EffectorResource.class);
     
    -   @Override
    -  public List<EffectorSummary> list(final String application, final String entityToken) {
    -       final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    -       return FluentIterable.from(entity.getEntityType().getEffectors())
    -            .transform(new Function<Effector<?>, EffectorSummary>() {
    -                @Override
    -                public EffectorSummary apply(Effector<?> effector) {
    -                    return EffectorTransformer.effectorSummary(entity, effector);
    -                }
    -            })
    -           .toSortedList(SummaryComparators.nameComparator());
    -   }
    +    @Override
    +    public List<EffectorSummary> list(final String application, final String entityToken) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +        return FluentIterable
    +                .from(entity.getEntityType().getEffectors())
    +                .filter(new Predicate<Effector<?>>() {
    +                    @Override
    +                    public boolean apply(@Nullable Effector<?> input) {
    +                        return Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.INVOKE_EFFECTOR,
    +                                Entitlements.EntityAndItem.of(entity, input.getName()));
    +                    }
    +                })
    +                .transform(new Function<Effector<?>, EffectorSummary>() {
    +                    @Override
    +                    public EffectorSummary apply(Effector<?> effector) {
    +                        return EffectorTransformer.effectorSummary(entity, effector);
    +                    }
    +                })
    +                .toSortedList(SummaryComparators.nameComparator());
    +    }
     
         @Override
    -  public Response invoke(String application, String entityToken, String effectorName,
    -      String timeout, Map<String, String> parameters
    -  ) {
    -      final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +    public Response invoke(String application, String entityToken, String effectorName,
    +                           String timeout, Map<String, String> parameters) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    --- End diff --
    
    `getEntity` will check whether the **entity** is visible, but not whether we're entitled to invoke this **effector**, no?


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

[GitHub] incubator-brooklyn pull request: add entitlements to EffectorResou...

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

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


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

[GitHub] incubator-brooklyn pull request: add entitlements to EffectorResou...

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

    https://github.com/apache/incubator-brooklyn/pull/67#discussion_r14928989
  
    --- Diff: usage/rest-server/src/main/java/brooklyn/rest/resources/EffectorResource.java ---
    @@ -43,57 +48,61 @@
     import brooklyn.util.guava.Maybe;
     import brooklyn.util.time.Time;
     
    -import com.google.common.base.Function;
    -import com.google.common.collect.FluentIterable;
    -
     public class EffectorResource extends AbstractBrooklynRestResource implements EffectorApi {
     
         private static final Logger log = LoggerFactory.getLogger(EffectorResource.class);
     
    -   @Override
    -  public List<EffectorSummary> list(final String application, final String entityToken) {
    -       final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    -       return FluentIterable.from(entity.getEntityType().getEffectors())
    -            .transform(new Function<Effector<?>, EffectorSummary>() {
    -                @Override
    -                public EffectorSummary apply(Effector<?> effector) {
    -                    return EffectorTransformer.effectorSummary(entity, effector);
    -                }
    -            })
    -           .toSortedList(SummaryComparators.nameComparator());
    -   }
    +    @Override
    +    public List<EffectorSummary> list(final String application, final String entityToken) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +        return FluentIterable
    +                .from(entity.getEntityType().getEffectors())
    +                .filter(new Predicate<Effector<?>>() {
    +                    @Override
    +                    public boolean apply(@Nullable Effector<?> input) {
    +                        return Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.INVOKE_EFFECTOR,
    +                                Entitlements.EntityAndItem.of(entity, input.getName()));
    +                    }
    +                })
    +                .transform(new Function<Effector<?>, EffectorSummary>() {
    +                    @Override
    +                    public EffectorSummary apply(Effector<?> effector) {
    +                        return EffectorTransformer.effectorSummary(entity, effector);
    +                    }
    +                })
    +                .toSortedList(SummaryComparators.nameComparator());
    +    }
     
         @Override
    -  public Response invoke(String application, String entityToken, String effectorName,
    -      String timeout, Map<String, String> parameters
    -  ) {
    -      final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    +    public Response invoke(String application, String entityToken, String effectorName,
    +                           String timeout, Map<String, String> parameters) {
    +        final EntityLocal entity = brooklyn().getEntity(application, entityToken);
    --- End diff --
    
    @aledsage `BrooklynRestResourceUtils.getEntity` already uses internally `Entitlements.isEntitled` so no need to add it also to that method, correct?


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