Hey Gagan:
First, I think that this CL now includes a bunch of stuff that has since
been obviated, inc. all the Accel code and StringEncoding helper method.
For the Domain balancing work itself, I remain confused why we should
inherit from AbsoluteReferencePathVisitor. Domain balancing is a
_property_ of a ProxyUriManager, not a special rewriter.
The main rationale I could see in having a separate rewriter is to count
the number of links in the document to be balanced, and then to generate
proxy Uris for them all at once. Yet ProxyUriManager already does that -
it gets the full batch of Uris to proxy in the given List.
Given this, why not just write a DomainBalancingProxyUriManager? This
class needn't even implement its own "base" proxying algorithm. As noted
in a previous comment, you could simply @Inject into it a "base"
ProxyUriManager, and then have DomainBalancingProxyUriManager
intelligently substitute a stable, balanced prefix for the %balance%
token in the resulting URI.
In this way, users can choose to opt into balancing or not simply
through configuration. Where an installation may have done:
bind(ProxyUriManager.class).to(MyProxyUriManager.class);
now they do:
bind(ProxyUriManager.class).annotatedWith(Names.named("shindig.proxy.balancing.base-manager")).to(MyProxyUriManager.class)
bind(ProxyUriManager.class).to(DomainBalancingProxyUriManager.class);
...and update config/container.js's gadgets.uri.proxy.host to include
%balance% somewhere in it.
Code could be as simple as:
class DomainBalancingProxyUriManager implements ProxyUriManager {
@Inject
DomainBalancingProxyUriManager(@Name("shindig.proxy.balancing.base-manager")
ProxyUriManager delegate) { ... }
@Override
public List<ProxyUri> make(List<Uri> uris...) {
List<ProxyUri> proxied = delegate.make(uris...);
// Modify the list here w/ your own balancing alg.
}
}
Thoughts?
Best,
John
On 2010/06/24 01:40:47, gagan.goku wrote:
Please review.
http://codereview.appspot.com/1674041/show
