First batch. I need to noodle on the bindig/dispatch JS code some more.
http://codereview.appspot.com/50047/diff/1/43 File features/src/main/javascript/features/osapi/batch.js (right): http://codereview.appspot.com/50047/diff/1/43#newcode1 Line 1: /* In general this code needs more thorough documentation. http://codereview.appspot.com/50047/diff/1/43#newcode24 Line 24: * Currently, since makeRequest is proxied, these requests are handled separately to is this comment valid? makeRequest is'nt part of the api. http://codereview.appspot.com/50047/diff/1/43#newcode30 Line 30: * Latch that waits for the batch of json requests and each makeRequest call to finish before comment cleanup for makeRequest, switch to osapi.http. Probably best to make generic and describe how calls are partitioned across end-points. http://codereview.appspot.com/50047/diff/1/43#newcode70 Line 70: finishedJsonRequest : finishNonProxiedRequest, are both of these needed with the transition to osapi.http http://codereview.appspot.com/50047/diff/1/43#newcode95 Line 95: var executeJsonRequests = function(jsonRequests, json, getDataFromResult, countDownLatch) { parameter documentation would be helpful http://codereview.appspot.com/50047/diff/1/43#newcode111 Line 111: var executeProxiedRequests = function(proxiedRequests, countDownLatch) { not needed with elimination of makeRequest? http://codereview.appspot.com/50047/diff/1/43#newcode144 Line 144: if (request.isMakeRequest) { remove makeRequest specific piece. http://codereview.appspot.com/50047/diff/1/44 File features/src/main/javascript/features/osapi/jsonrequest.js (right): http://codereview.appspot.com/50047/diff/1/44#newcode22 Line 22: rpcUrl: The url of the opensocial data endpoint. opensocial -> JSON-RPC http://codereview.appspot.com/50047/diff/1/44#newcode30 Line 30: * The bare mechanism for creating json opensocial requests. opensocial -> JSON-RPC http://codereview.appspot.com/50047/diff/1/44#newcode85 Line 85: // for (var i = 0; i < result.data.length; i++) { remove if unused http://codereview.appspot.com/50047/diff/1/44#newcode101 Line 101: var defaultUserId = '@viewer'; remove? http://codereview.appspot.com/50047/diff/1/44#newcode126 Line 126: var ensureUserId = function(options) { remove? http://codereview.appspot.com/50047/diff/1/44#newcode135 Line 135: var ensureGroupId = function(options) { remove? http://codereview.appspot.com/50047/diff/1/44#newcode143 Line 143: ensureUserId(params); remove? http://codereview.appspot.com/50047/diff/1/44#newcode144 Line 144: ensureGroupId(params); remove? http://codereview.appspot.com/50047/diff/1/38 File features/src/main/javascript/features/osapi/makerequest.js (right): http://codereview.appspot.com/50047/diff/1/38#newcode23 Line 23: osapi.makeRequest = function() { remove? http://codereview.appspot.com/50047/diff/1/40 File features/src/main/javascript/features/osapi/osapi.js (right): http://codereview.appspot.com/50047/diff/1/40#newcode53 Line 53: var requiredConfig = { not sure this serves a purpose? its hidden in a closure http://codereview.appspot.com/50047/diff/1/39 File features/src/main/javascript/features/osapi/peoplehelpers.js (right): http://codereview.appspot.com/50047/diff/1/39#newcode20 Line 20: osapi.people = osapi.people || {}; shouldnt do this, code should be loaded after init and if osapi.people doesnt exist neither do the helpers. http://codereview.appspot.com/50047/diff/1/39#newcode50 Line 50: options.userId = "@viewer"; technically you dont need to set these as they are the protocol defaults. http://codereview.appspot.com/50047/diff/1/41 File features/src/main/javascript/features/osapi/util.js (right): http://codereview.appspot.com/50047/diff/1/41#newcode40 Line 40: if (httpError == "Error 501") { might be better to express as a map http://codereview.appspot.com/50047/diff/1/18 File features/src/test/javascript/features/osapi/activitiestest.js (right): http://codereview.appspot.com/50047/diff/1/18#newcode36 Line 36: "system" : { "listMethods" : "" }, this seems weird. can we use arrays here instead. http://codereview.appspot.com/50047/diff/1/9 File java/common/src/main/java/org/apache/shindig/protocol/RpcServiceFetcher.java (right): http://codereview.appspot.com/50047/diff/1/9#newcode91 Line 91: return endpointServices; make immutable before returning http://codereview.appspot.com/50047/diff/1/9#newcode110 Line 110: URL url = new URL(endpoint+"?method="+SYSTEM_LIST_METHODS_METHOD); Any reason not to bind and use the HttpFetcher? http://codereview.appspot.com/50047/diff/1/3 File java/server/src/test/resources/endtoend/osapi/activitiesTest.xml (right): http://codereview.appspot.com/50047/diff/1/3#newcode37 Line 37: osapi.activities.get({ userId : 'canonical', groupId : '@self'}).execute(receivedData); groupid not needed. http://codereview.appspot.com/50047
