Common container: updates

- Push down caching mechanism to service for gadget metadata.
- Now builds full response from partial request/response.
- Allow defaultable view (ie: if requested view does not exist, fallback to view=default).
- More friendly runtime error checking.
- Per-gadget error reporting.

[zhoresh, 2010-09-16 20:54:15]

http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
File features/src/main/javascript/features/container/gadget_site.js (right):

http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
features/src/main/javascript/features/container/gadget_site.js:246: if
(renderParams[shindig.container.RenderParam.ALLOW_DEFAULT_VIEW] && !viewInfo) {
nit: I would reverse order: !viewInfo && renderParams[...]

http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
File features/src/main/javascript/features/container/service.js (left):

http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
features/src/main/javascript/features/container/service.js:57: >>>>>>> more
gjslint cleanups
Nice ...

http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
File features/src/main/javascript/features/container/service.js (right):

http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
features/src/main/javascript/features/container/service.js:76: * Return a
possibly-cached gadgets metadata for gadgets in request.ids, for
Should we have an expiration mechanism to the cache?
Is there a reason to also limit the size of the cache?

[mhermanto, 2010-09-17 04:22:12]

On Thu, Sep 16, 2010 at 1:54 PM, <zhoresh@gmail.com> wrote:

>
>
>
http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
> File features/src/main/javascript/features/container/gadget_site.js
> (right):
>
>
>
http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
> features/src/main/javascript/features/container/gadget_site.js:246: if
> (renderParams[shindig.container.RenderParam.ALLOW_DEFAULT_VIEW] &&
> !viewInfo) {
> nit: I would reverse order: !viewInfo && renderParams[...]
>

Done.


>
>
>
http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
> File features/src/main/javascript/features/container/service.js (left):
>
>
>
http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
> features/src/main/javascript/features/container/service.js:57: >>>>>>>
> more gjslint cleanups
> Nice ...
>

Not sure what this means. It first appears like a merge conflict marker.


>
>
>
http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
> File features/src/main/javascript/features/container/service.js (right):
>
>
>
http://codereview.appspot.com/2226041/diff/5002/features/src/main/javascript/...
> features/src/main/javascript/features/container/service.js:76: * Return
> a possibly-cached gadgets metadata for gadgets in request.ids, for
> Should we have an expiration mechanism to the cache?
> Is there a reason to also limit the size of the cache?


All good ideas --
1) I think the expiration mechanism should be based on renderParam[nocache],
ie: the same nocache=1 passed into /gadgets/ifr should bust cache in all
layers.
2) I agree there should a limit, as it'll potentially eat up all memory. Not
quite clear to me how preloaded gadgets will be evicted (if a user preload,
he/she would expect the gadget preloaded upon all navigation calls). I
suppose we can introduce unload gadgets, or preload gadgets with an error
"no more can be preloaded". Point being, cache for preloaded and
arbitrarily-navigated gadgets are the same cache, and they probably should
be distinguished, so arbitrarily-navigated gadgets can be evicted at a time
when we find most appropriate.
Both noted as TODOs. Thoughts?


>
> http://codereview.appspot.com/2226041/
>

[mhermanto, 2010-09-17 04:24:14]

Update patch to address comments

[zhoresh, 2010-09-20 19:42:37]

lgtm, I thought the cache is new. 

But we should handle expiation which might require a change in the GadgetHandler