Update common container JSON-RPC endpoint for security token refresh

Refactored code. Tests to be added. Sending this out now for an initial design review.

[johnfargo, 2010-07-19 20:29:19]

This strategy looks quite good to me. I'd be sure to sync w/ ZivH, who I
understand is planning to refactor the filtering logic a bit, to avoid undue
merge problems.

[zhoresh, 2010-07-19 21:39:53]

design lgtm, please also update the tests

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

http://codereview.appspot.com/1696052/diff/1/2#newcode104
src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:104:
@Operation(httpMethods = "GET", path="/@metadata.supportedFields")
Change path to "suppertedFields.get" to be consistent with the rest?

[mhermanto, 2010-07-20 07:31:32]

Added tests

[mhermanto, 2010-07-20 07:35:24]

Thanks for the review. New patch uploaded.

On Mon, Jul 19, 2010 at 2:39 PM, <zhoresh@gmail.com> wrote:

> design lgtm, please also update the tests
>

Done.

>
> http://codereview.appspot.com/1696052/diff/1/2
> File
>
> src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
> (right):
>
> http://codereview.appspot.com/1696052/diff/1/2#newcode104
> src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:104:
> @Operation(httpMethods = "GET", path="/@metadata.supportedFields")
> Change path to "suppertedFields.get" to be consistent with the rest?


Done.


>
>
> http://codereview.appspot.com/1696052/show
>

[johnfargo, 2010-07-20 22:42:26]

http://codereview.appspot.com/1696052/diff/6001/7001
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
(right):

http://codereview.appspot.com/1696052/diff/6001/7001#newcode123
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:123:
CompletionService<R> completionService = new
ExecutorCompletionService<R>(executor);
why not just init this in GadgetHandler at the top level?

[fargo, 2010-07-20 23:03:46]

Chatted on IM; since this is:
A) overridable, and...
B) generic-typed "<R>"

This looks good enough to me. Patching and, assuming all goes smoothly,
committing.

--j

On Tue, Jul 20, 2010 at 3:42 PM, <johnfargo@gmail.com> wrote:

>
> http://codereview.appspot.com/1696052/diff/6001/7001
> File
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
> (right):
>
> http://codereview.appspot.com/1696052/diff/6001/7001#newcode123
>
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:123:
> CompletionService<R> completionService = new
> ExecutorCompletionService<R>(executor);
> why not just init this in GadgetHandler at the top level?
>
>
> http://codereview.appspot.com/1696052/show
>

[johnfargo, 2010-07-20 23:08:40]

Committed as r966049. Feel free to close out this review.

On 2010/07/20 22:42:26, johnfargo wrote:
> http://codereview.appspot.com/1696052/diff/6001/7001
> File
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java
> (right):
> 
> http://codereview.appspot.com/1696052/diff/6001/7001#newcode123
>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandler.java:123:
> CompletionService<R> completionService = new
> ExecutorCompletionService<R>(executor);
> why not just init this in GadgetHandler at the top level?