Geoff, I like 4. but it might be too much work, for what it's worth - that's why 1. seems like a good alternative. If there's no good technical reason to have HttpAsserts in utils, then +1 for (1).
Svet. > On 16.11.2015 г., at 17:48, Geoff Macartney > <[email protected]> wrote: > > > Hi Brooklyners, > > As part of #994 [1], I moved HttpTool out of core into brooklyn-utils-common > (leaving a deprecated HttpTools in core). This was done on the grounds that > it is a tool that operates on HTTP, does not depend on brooklyn-core, and > could find wider use outside core. > > What I didn’t reckon with is that my change broke downstream builds, because > the deprecated HttpTool in core included return types from the new package > (o.a.b.util.http), while its users are expecting the old packages. To fix > this it’s not enough merely to restore the original HttpTool in core[2], as > the HttpToolResponse from the tool ‘leaks’ into other classes, such as the > response from org.apache.brooklyn.feed.http.HttpFeed#getPoller: > > protected Poller<HttpToolResponse> getPoller() > > and various methods in HttpValueFunctions, e.g. > > public static Function<HttpToolResponse, Boolean> containsHeader(String > header) > > This seems worth asking the community about. What would you recommend here > as a course of action? > > The options seem to include: > > 1. Moving the HttpTool and associated classes back into core. This would > make the code backward compatible again but does leave the tool inside core > Brooklyn, when it would be nice to have it as a util (especially when it does > not in fact depend on anything in core). > 2. Removing the deprecated classes in core, leaving the tool in > brooklyn-utils-common, but changing the *package* to what the downstreams > expect - org.apache.brooklyn.utils.core.http (note the “core” even though > this would not be in brooklyn-core). While it might seem wrong to have a > util package with “core” in its name, this is really a matter of > interpretation - it could be a “core util”. On the other hand I understand > there were changes made in Brooklyn recently to avoid this sort of > overlapping package name, because of OSGi, so maybe this is not a possibility. > 3. Deprecating the methods in HttpFeed and others that use the old > HttpToolResponse, and providing alternatives that use the new one. I haven’t > tried this but would be worried that it would result in an avalanche of > deprecation. > 4. A middle ground, which requires a bit of work: > > * Leave HttpTool in utils > * extend it in core, with methods which return HttpToolResponse specialized > to return the core’s package > (org.apache.brooklyn.util.core.http.HttpToolResponse). > * make HttpToolResponse a delegate to the utils version > * Leave all code using HttpTool to use the deprecated version > > This last alternative would require a bit of rework in some places, at least > HttpPollConfig would need to change to something that didn’t rely on the old > HttpToolResponse, which in turn would mean you’d probably want to update > HttpFeed to support the old HttpPollConfig (but deprecated) and the newer > alternative. > > There may well be other approaches that we could take. I invite your > suggestions as to the best approach. > > best regards > Geoff > > [1] https://github.com/apache/incubator-brooklyn/pull/994 > [2] https://github.com/apache/incubator-brooklyn/pull/1032 > <https://github.com/apache/incubator-brooklyn/pull/1032> > > ———————————————————— > Gnu PGP key - http://is.gd/TTTTuI > >
