Introduce ConcatUriManager

Introduces interface ConcatUriManager, the intent of which is to be used in Concat-based rewriters *and* in ConcatProxyServlet. It provides a well-defined API between URI generation and consumption.

In addition to the interface definition, DefaultConcatUriManager is introduced. It provides a base implementation of ConcatUriManager that is conceptually consistent with all the other UriManager implementations provided. In particular:
* Defines its own ContainerConfig "gadgets.uri.concat" namespace with "host" and "path" values used for Host: and path assignment, respectively.
* Provides a symmetric makeUri/validateUri pair
* Provides a Versioner interface, in this case supporting batching, for versioning URLs or batches of same. No default Versioner implementation is provided in this CL.

Neither the interface nor implementation are used in this CL, as the intent is to keep CLs as small as possible. A follow-up CL will change ConcatProxyServlet to use the new interface, along with attendant rewriter changes.

[zhoresh, 2010-02-23 22:52:14]

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

[johnfargo, 2010-02-24 01:35:07]

Accommodates zhoresh@'s first round of comments.

[johnfargo, 2010-02-24 01:35:23]

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#newcode84
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:84:
public static final class ConcatUri {
On 2010/02/23 22:52:14, zhoresh wrote:
> add class javadoc that explain snippet

Done.

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 {
Yikes, good catch. Added to this CL.

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#newcode73
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:73:
// Above params are common for all generated Uris.
On 2010/02/23 22:52:14, zhoresh wrote:
> Since it is common, it sounds like a conversion that can be extracted to a
> utility class or a base class for all UriManagers

By this I mean that these params are common for all ConcatUris, so I pass the
base Uri into makeConcatUri (this sort of is the utility method).

That said, at least one other UriManager (ProxyUriManager) is quite similar...
perhaps a possibility to factor out common logic exists there.

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;
On 2010/02/23 22:52:14, zhoresh wrote:
> Maybe iterator on versions instead of index

Done.

[johnfargo, 2010-02-27 00:24:49]

Improving symmetry of make/validate APIs. Ridding API of Gadget context object.
Other updates.

[zhoresh, 2010-03-01 19:33:49]

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

http://codereview.appspot.com/217091/diff/2001/3001#newcode68
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:68:
* @param Gadget context object for the request
gadget param is gone

http://codereview.appspot.com/217091/diff/2001/3001#newcode133
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:133:
ValidatedUri validateConcatUri(Uri uri);
I would rename it to parse instead of validate

http://codereview.appspot.com/217091/diff/2001/3006
File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUri.java
(right):

http://codereview.appspot.com/217091/diff/2001/3006#newcode30
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUri.java:30:
public static class Context {
I would rather see Context and Validate as one class.
It make it clearer for symetry of convertion to and from url.
Also easy to override

[johnfargo, 2010-03-02 01:12:52]

Symmetric APIs. For concat this isn't 100% true (@see ConcatData) but is much
closer.

[zhoresh, 2010-03-02 16:39:20]

LGTM, Thanks!
Just a minor suggestion.

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

http://codereview.appspot.com/217091/diff/3011/2006#newcode160
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:160:
List<String> version(List<List<Uri>> resourceUris, String container);
Since this is a specific versioner, you might save some code by passing in
ConcatUri instead of Uri (for both version and validate)

[johnfargo, 2010-03-03 18:58:01]

Thanks Ziv -- code committed. I'll be doing some cleanup passes after this
raft of CLs and will look into the ConcatUri vs. Uri replacement.

On Tue, Mar 2, 2010 at 8:39 AM, <zhoresh@gmail.com> wrote:

> LGTM, Thanks!
> Just a minor suggestion.
>
>
> http://codereview.appspot.com/217091/diff/3011/2006
>
> File
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java
> (right):
>
> http://codereview.appspot.com/217091/diff/3011/2006#newcode160
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ConcatUriManager.java:160:
> List<String> version(List<List<Uri>> resourceUris, String container);
> Since this is a specific versioner, you might save some code by passing
> in ConcatUri instead of Uri (for both version and validate)
>
>
> http://codereview.appspot.com/217091/show
>