brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tbouron <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #885: Icons for adjuncts, and descriptions
Date Wed, 08 Nov 2017 10:41:10 GMT
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/885#discussion_r149632259
  
    --- Diff: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/transform/AdjunctTransformer.java
---
    @@ -45,17 +45,24 @@
      */
     public class AdjunctTransformer {
     
    -    public static AdjunctSummary adjunctSummary(Entity entity, EntityAdjunct adjunct,
UriBuilder ub) {
    -        return embellish(new AdjunctSummary(adjunct), entity, adjunct, ub);
    +    public static AdjunctSummary adjunctSummary(Entity entity, EntityAdjunct adjunct,
UriBuilder ub, BrooklynRestResourceUtils brooklynU) {
    +        return embellish(new AdjunctSummary(adjunct), entity, adjunct, ub, brooklynU);
         }
     
         @SuppressWarnings("unchecked")
    -    private static <T extends AdjunctSummary> T embellish(T adjunctSummary, Entity
entity, EntityAdjunct adjunct, UriBuilder ub) {
    +    private static <T extends AdjunctSummary> T embellish(T adjunctSummary, Entity
entity, EntityAdjunct adjunct, UriBuilder ub, BrooklynRestResourceUtils brooklynU) {
    +        
    +        if (adjunctSummary.getIconUrl()!=null) {
    +            if (brooklynU.isUrlServerSideAndSafe(adjunctSummary.getIconUrl()))
    +                // route to server if it is a server-side url
    +                adjunctSummary.iconUrl(adjunctUri(entity, adjunct, ub)+"/icon");
    +        }
    +
             return (T) adjunctSummary.state(inferStatus(adjunct)).links( buildLinks(entity,
adjunct, ub, adjunctSummary instanceof AdjunctDetail) );
         }
     
    -    public static AdjunctDetail adjunctDetail(BrooklynRestResourceUtils utils, Entity
entity, EntityAdjunct adjunct, UriBuilder ub) {
    -        AdjunctDetail result = embellish(new AdjunctDetail(adjunct), entity, adjunct,
ub);
    +    public static AdjunctDetail adjunctDetail(Entity entity, EntityAdjunct adjunct, UriBuilder
ub, BrooklynRestResourceUtils utils) {
    --- End diff --
    
    My question is more: do we really need this change? It just shuffles the arguments, there
is no benefits compare to the hassle of potential breaking things and informing users. I don't
think it is worth the effort. 
    
    If we are going forward with this change, it should be added to the list of breaking changes
in the release notes.


---

Mime
View raw message