Skip to content

Commit

Permalink
Merge pull request #56 from burcu/paramfix
Browse files Browse the repository at this point in the history
Requests should not take query params as the first parameter
  • Loading branch information
Burcu Dogan committed Jul 14, 2013
2 parents d826c3a + d50e9db commit ae95b4a
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 34 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ var request1 =
client.plus.people.get({ userId: '+BurcuDogan' });

var request2 =
client.urlshortener.url.insert(null, { longUrl: 'http://google.com' });
client.urlshortener.url.insert({ longUrl: 'http://google.com' });

// Create from client service using the raw action name
var request3 = client.urlshortener.newRequest(
Expand Down
14 changes: 7 additions & 7 deletions examples/multiple.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ googleapis
var req2 = client.plus.people.get({ userId: '+BurcuDogan' });

client
.newBatchRequest()
.add(req1)
.add(req2)
.withApiKey(API_KEY)
.execute(function(err, results) {
console.log(err, results);
});
.newBatchRequest()
.add(req1)
.add(req2)
.withApiKey(API_KEY)
.execute(function(err, results) {
console.log(err, results);
});
});
15 changes: 10 additions & 5 deletions examples/urlshortener.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ googleapis

// a single request
client
.urlshortener
.newRequest('urlshortener.url.get', { shortUrl: 'http://goo.gl/DdUKX' })
.execute(printResult);
.urlshortener
.newRequest('urlshortener.url.get', { shortUrl: 'http://goo.gl/DdUKX' })
.execute(printResult);

// request builders
client.urlshortener.url.get({ shortUrl: 'http://goo.gl/DdUKX' })
.execute(printResult);
client.urlshortener.url
.get({ shortUrl: 'http://goo.gl/DdUKX' })
.execute(printResult);

client.urlshortener.url
.insert({ longUrl: 'http://somelongurl.com' })
.execute(printResult);
});
46 changes: 30 additions & 16 deletions lib/requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,31 @@ var util = require('util');
var querystring = require('querystring');
var DefaultTransporter = require('./transporters.js');

/**
* Moves resource parameters to params.resource object.
*/
var moveResourceParams = function(apiMeta, methodName, params) {
if (!apiMeta || !apiMeta.methods || !apiMeta.methods[methodName]) {
// nothing to do here, dont have enough
// info about the parameters
return params;
}

var methodMeta = apiMeta.methods[methodName],
existing = params,
queryParameters = {};

for (var parameterName in methodMeta.parameters || {}) {
if (existing[parameterName]) {
queryParameters[parameterName] = existing[parameterName];
delete existing[parameterName];
}
}

params = queryParameters;
params.resource = existing;
return params;
};

function BaseRequest(apiMeta) {
this.transporter = new DefaultTransporter();
Expand Down Expand Up @@ -129,14 +154,12 @@ BaseRequest.prototype.handleResponse = function(opt_fn) {
* @param {object} apiMeta Schema returned by Discovery API.
* @param {string} methodName Method name.
* @param {?object} params Parameters.
* @param {object=} opt_resource Optional resource.
*/
function Request(apiMeta, methodName, params, opt_resource) {
function Request(apiMeta, methodName, params) {
Request.super_.call(this);
this.apiMeta = apiMeta;
this.methodName = methodName;
this.params = params;
this.resource = opt_resource;
this.params = params || {};
}

/**
Expand All @@ -153,14 +176,10 @@ Request.prototype.generatePayload = function() {
jsonrpc: BaseRequest.JSONRPC_VERSION,
id: 0,
method: this.methodName,
params: this.params || {},
params: moveResourceParams(this.apiMeta, this.methodName, this.params),
apiVersion: this.apiMeta.version
};

if (this.resource) {
request.params.resource = this.resource;
}

return [request];
};

Expand Down Expand Up @@ -214,16 +233,11 @@ BatchRequest.prototype.add = function(request) {
* @return {Array.<object>} Generated payload.
*/
BatchRequest.prototype.generatePayload = function() {

var payload = [];
for (var i = 0; i < this.requests_.length; i++) {
var request = this.requests_[i];
request.params = request.params || {};

if (request.resource) {
request.params.resource = request.resource;
}

request.params =
moveResourceParams(this.apiMeta, request.method, request.params);
payload.push({
jsonrpc: BaseRequest.JSONRPC_VERSION,
id: i,
Expand Down
24 changes: 19 additions & 5 deletions tests/test.requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ describe('Requests', function() {
assert.equal(-1, generatedUrl.indexOf('?'));
});

it('should generate a valid JSON-RPC payload for single' +
it('should generate a valid JSON-RPC payload for single ' +
'requests', function(done) {
var gapis = new googleapis.GoogleApis();
gapis.Transporter = urlshortenerDiscoveryTransporter;
gapis
.discover('urlshortener', 'v1')
.execute(function(err, client) {
var obj = { longUrl: 'http://someurl...' };
var request = client.urlshortener.url.insert(null, obj);
var request = client.urlshortener.url.insert(obj);
var payload = request.generatePayload();
var first = payload[0];

Expand Down Expand Up @@ -89,11 +89,25 @@ describe('Requests', function() {
gapis
.discover('urlshortener', 'v1')
.execute(function(err, client) {
var params = { test: 'a' };
var request = client.urlshortener.url.list(params);
var params = { shortUrl: 'a' };
var request = client.urlshortener.url.get(params);
var payload = request.generatePayload();
var first = payload[0];
assert.equal(first.params, params);
assert.equal(first.params.shortUrl, 'a');
done();
});
});

it('should move resource parameters under resource key', function(done) {
var gapis = new googleapis.GoogleApis();
gapis
.discover('drive', 'v2')
.execute(function(err, client) {
var req = client.drive.files.update({ fileId: 'fileId', title: 'Test' }),
payload = req.generatePayload(),
first = payload[0];
assert.equal(first.params.fileId, 'fileId');
assert.equal(first.params.resource.title, 'Test');
done();
});
});
Expand Down

0 comments on commit ae95b4a

Please sign in to comment.