----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/974/#review937 -----------------------------------------------------------
Ship it! LGTM - Ryan On 2011-06-30 13:09:14, Stanton Sievers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/974/ > ----------------------------------------------------------- > > (Updated 2011-06-30 13:09:14) > > > Review request for shindig, johnfargo and Michael Hermanto. > > > Summary > ------- > > This change is to accommodate synchronous rpc handlers that may be registered > with the common container via > osapi.container.Container.prototype.rpcRegister. Per the documentation in > rpc.js#process, an rpc handler should return a result value that will be > included as part of a synchronous callback made by the rpc service. If no > result is provided it is assumed that the rpc handler will perform the > callback. > > Currently rpcRegister in the common container does not allow for the > returning of any results, thus forcing the rpc handler to perform the > callback even in synchronous cases. > > For convenience, here is the documentation in rpc.js#process: > // If there is a callback for this service, attach a callback function > // to the rpc context object for asynchronous rpc services. > // > // Synchronous rpc request handlers should simply ignore it and return a > // value as usual. > // Asynchronous rpc request handlers, on the other hand, should pass its > // result to this callback function and not return a value on exit. > // > // For example, the following rpc handler passes the first parameter back > // to its rpc client with a one-second delay. > // > // function asyncRpcHandler(param) { > // var me = this; > // setTimeout(function() { > // me.callback(param); > // }, 1000); > // } > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1140140 > > Diff: https://reviews.apache.org/r/974/diff > > > Testing > ------- > > Minimal testing performed. We noticed this problem during some feature > development in which a gadgets.rpc.call to the feature required a callback > and the rpc handler in the container did not explicitly invoke the callback. > This change fixed that problem. > > > Thanks, > > Stanton > >
