Re: Introduce ConcatUriManager (issue217091)

[zh...@gmail.com]

http://codereview.appspot.com/217091/diff/1/3
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java
(right):

http://codereview.appspot.com/217091/diff/1/3#newcode73
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:73:
List<ConcatUri> makeConcatUri(Gadget gadget, List<List<Uri>>
resourceUris,
I am not clear why the interface here need list of lists, as
encapsulation go I don't think the manager need that detail.

http://codereview.appspot.com/217091/diff/1/3#newcode84
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:84:
public static final class ConcatUri {
add class javadoc that explain snippet

http://codereview.appspot.com/217091/diff/1/3#newcode103
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:103:
public static final class ValidatedUri extends ProxyUriBase {
Where is ProxyUriBase?

http://codereview.appspot.com/217091/diff/1/4
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java
(right):

http://codereview.appspot.com/217091/diff/1/4#newcode49
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:49:
public DefaultConcatUriManager(ContainerConfig config, Versioner
versioner) {
Don't you need to specify versioner as nullable?

http://codereview.appspot.com/217091/diff/1/4#newcode73
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:73:
// Above params are common for all generated Uris.
Since it is common, it sounds like a conversion that can be extracted to
a utility class or a base class for all UriManagers

http://codereview.appspot.com/217091/diff/1/4#newcode85
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:85:
String version = versions != null ? versions.get(i++) : null;
Maybe iterator on versions instead of index

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