You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by zh...@gmail.com on 2010/06/14 23:03:01 UTC

Adding AbsolutePathReferenceRewriter and DomainBalancingUriRe (issue1674041)

http://codereview.appspot.com/1674041/diff/5001/6007
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomainBalancingUriVisitor.java
(right):

http://codereview.appspot.com/1674041/diff/5001/6007#newcode44
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomainBalancingUriVisitor.java:44:
public class DomainBalancingUriVisitor extends
AbsolutePathReferenceVisitor {
Can you move this to separate review?
At this point it doesn't really do much, so maybe explain what you are
trying to do here.

http://codereview.appspot.com/1674041/diff/5001/6007#newcode82
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomainBalancingUriVisitor.java:82:
Map<Node, Uri> nodeToAbsoluteUri = new HashMap<Node, Uri>(nodes.size());
Maybe you can just use lists instead of maps. Unless I miss something, I
think getRewrittenUris just go over the urls in order.

http://codereview.appspot.com/1674041/diff/5001/6007#newcode142
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomainBalancingUriVisitor.java:142:
* GGS or not. Uri's rewritten by other rewriters should not be rewritten
as
please use Shindig instead of GGS

http://codereview.appspot.com/1674041/diff/5001/6007#newcode162
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomainBalancingUriVisitor.java:162:
public void registerAccelHosts(List<String> accelHosts) {
I prefer injected constructor over injected set functions (and don't you
also need to name the string list?)

http://codereview.appspot.com/1674041/diff/5001/6006
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java
(right):

http://codereview.appspot.com/1674041/diff/5001/6006#newcode56
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java:56:
requestUri = new UriBuilder()
Can this be merged with RoxyUriManager? Basically you want similar code
so it can hanle chain-param style the same way.

http://codereview.appspot.com/1674041/diff/5001/6005
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java
(right):

http://codereview.appspot.com/1674041/diff/5001/6005#newcode122
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java:122:
uri.setScheme(config.getString(container, PROXY_SCHEME));
No need to set schema, it is taken from the parent page

http://codereview.appspot.com/1674041/show