Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/885#discussion_r149639562
--- 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 --
it isn't a breaking change as there has been no release since it was
introduced. changes are common within a release as new things evolve. this
makes the method consistent with others and puts salient fields first, context
fields after.
i'm pretty sure nothing external is using any of these transformers and
almost certain not this very new one. i'm pretty sure you're talking a very
low theoretical-only risk and as we haven't done a release we're not breaking
any guarantees we offer.
release notes are for breaking changes since previous release so would be
very odd to put it there. especially as it would be trivial to leave the old
method in place deprecated instead -- that would be the sensible fix -- but for
the reasons above don't even think that's worth it.
i'll take this all back if you can point me to anything that might actually
be impacted however!
---
