> On 2011-08-25 13:15:21, Ryan Baxter wrote: > > The code itself is fine with one small nit. > > > > Just for my understanding, I assume there is a reason why you need to > > create the container object before you have the metadata you want to > > cache. How are you generating the metadata, or is this metadata you have > > saved off somewhere and your just trying to populate the cache again with > > this information?
Thanks for the review Ryan! I agree about changing the parameter name, but I guess as long as we're at it the function name should change as well. I'll upload a new patch which renames the function/parameter to preloadCaches/preloadData. Regarding your question about our use case for this -- we're implementing a full standalone Java based gadget/widget container supporting OpenSocial gadgets, W3C widgets, ... Since gadget metadata doesn't change very often, and we have a full Java web application running where we can cache stuff, we're implementing a server side cache of gadget metadata and pushing it down to the client along with the rest of the response for a given page. The way that cached metadata is pushed down is as a property of a generic JavaScript widget object which is in an array of mixed type "widgets". Each widget instance could represent an OpenSocial gadget, a W3C widget, ... Our rendering process is then to create an instance of the Common container with no preloaded metadata/tokens (since we haven't enumerated our array of widgets yet), then go ahead and enumerate the widgets array, and then for any OpenSocial type gadget we find, we delegate the rendering off to common container -- which means calling the common container function to preload the metadata/tokens for the gadget instance and then navigating the gadget site. Another reason for pushing metadata down from our container is that we anticipate use cases where the container and shindig will run on different DNS domains, which means the XHR to fetch metadata from the client to shindig would throw a security exception (cross domain) and we don't want to have to proxy the calls from the client to shindig via our container. More detail on that can be found in the comments on another JIRA/review I have open (comments are in the JIRA): https://issues.apache.org/jira/browse/SHINDIG-1580 https://reviews.apache.org/r/1564/ Henry has already added some comments on the JIRA ticket for that one. If he or anyone else could please take another look at that one too I'd greatly appreciate it. Thanks! - Jesse ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1563/#review1639 ----------------------------------------------------------- On 2011-08-17 18:13:23, Jesse Ciancetta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1563/ > ----------------------------------------------------------- > > (Updated 2011-08-17 18:13:23) > > > Review request for shindig. > > > Summary > ------- > > Common container currently supports a one-time bulk preload of gadget > metadata and security tokens via the opt_config parameter passed to the > Container constructor, however it would also be useful to have a way to > incrementally preload this data as well. > > We're currently doing this in Apache Rave by calling the private > container.preloadFromConfig_ function for each of our gadgets before calling > container.navigateGadget which seems to work just fine, but we obviously > don't want to continue to rely on a private function. > > I'm attaching a patch to make the preloadFromConfig_ function public -- if > this looks good to everyone I would be happy to go ahead and submit a patch > against the container specification for this change as well. > > > This addresses bugs RAVE-172 and SHINDIG-1579. > https://issues.apache.org/jira/browse/RAVE-172 > https://issues.apache.org/jira/browse/SHINDIG-1579 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1157893 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/feature.xml > 1137021 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container/container_test.js > 1086837 > > Diff: https://reviews.apache.org/r/1563/diff > > > Testing > ------- > > Additional unit tests added as well as testing in the browser using sample > common container. > > > Thanks, > > Jesse > >
