Add support for loaded features in the GadgetsHandler API

Support for the loadedFeatures parameter in the API, which specifies a list of already-present features in JS requests.

[zhoresh, 2011-02-15 00:56:10]

look good, just couple of nits.

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
(right):

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:113:
LOADED_FEATURES("loadedFeatures"),
Add to UriCommon.Param

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/JsUriManager.java
(right):

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/JsUriManager.java:153:
return ImmutableList.copyOf(in);
Why changed? The original code will support nulls in collection, so be sure in
all cases null is not provided

[jtarrio, 2011-02-15 01:36:53]

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
(right):

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:113:
LOADED_FEATURES("loadedFeatures"),
On 2011/02/15 00:56:10, zhoresh wrote:
> Add to UriCommon.Param

UriCommon.Param already has the equivalent parameter, ALREADY_HAVE("loaded")
(whose name is less than descriptive, to be frank.)

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/JsUriManager.java
(right):

http://codereview.appspot.com/4160052/diff/1/java/gadgets/src/main/java/org/a...
java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/JsUriManager.java:153:
return ImmutableList.copyOf(in);
On 2011/02/15 00:56:10, zhoresh wrote:
> Why changed? The original code will support nulls in collection, so be sure in
> all cases null is not provided

I did it because Collections.UnmodifiableCollection does not implement equals()
and hashCode(), so the unit tests fail catastrofically even when they should
succeed.

I'll look for an alternative...

[jtarrio, 2011-02-15 21:01:49]

Second patch

[jtarrio, 2011-02-15 23:24:10]

Third patch, with some renaming

[zhoresh, 2011-02-16 00:43:25]

LGTM++

http://codereview.appspot.com/4160052/diff/11001/java/gadgets/src/test/java/o...
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java
(right):

http://codereview.appspot.com/4160052/diff/11001/java/gadgets/src/test/java/o...
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java:189:
assertEquals("another:onemore",
jsUri.getQueryParameter(Param.LOADED_LIBS.getKey()));
Shouldn't we make it smarter to just drop "another"?
Not critical, just an idea for future improvement.

[jtarrio, 2011-02-16 01:39:51]

http://codereview.appspot.com/4160052/diff/11001/java/gadgets/src/test/java/o...
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java
(right):

http://codereview.appspot.com/4160052/diff/11001/java/gadgets/src/test/java/o...
java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultJsUriManagerTest.java:189:
assertEquals("another:onemore",
jsUri.getQueryParameter(Param.LOADED_LIBS.getKey()));
On 2011/02/16 00:43:25, zhoresh wrote:
> Shouldn't we make it smarter to just drop "another"?
> Not critical, just an idea for future improvement.

No, because we also want to exclude "another"'s dependencies :)