Commit b6a2d047 authored by jrw's avatar jrw Committed by Commit bot

Updated XHR API so call sites are more descriptive.

This CL mainly converts from using positional parameters to a single
struct, but there are various other changes as well.

The functions corresponding to different HTTP methods have been
eliminated, and the "doMethod" function has been renamed "start".
Saving one parameter at each call site seemed like a poor tradeoff for
having four functions that differ only in the HTTP method they use.

The core logic of creating an XHR and arranging for the onDone
function to be called has been factored out into a separate function,
startInternal_.

The "parameters" argument has been replaced by separate urlParams and
content arguments.  This eliminates the need to heuristically decide
what to do with the parmameters based on the HTTP method, and it
allows URL parameters and body content to be included in the same
call, a case which occurs in practice in host_controller.js.

There are now three parameters for sepecifying different types of body
content: textContent, formContent, and jsonContent.  They have
different data types, and the formContent and jsonContent parameters
implicitly set the Content-type header to the correct value.

A new parameter, oauthToken, is a convenience for adding a
correctly-formatted Authorization header.  This formatting was
previously duplicated at five seperate call sites.

The string "OAuth" in Authorization headers has been replaced with
"Bearer". The OAuth 2 spec supports the use of "Bearer" but not
"OAuth".

There are no longer any instances where headers are specified
explicitly, but the ability to do so still exists in the API.

Finally, headers and urlParameters with the value null are accepted
but not included in the HTTP request.  This simplifies the logic of
the doRegisterHost function in host_controller.js.

BUG=

Review URL: https://codereview.chromium.org/945033002

Cr-Commit-Position: refs/heads/master@{#318151}
parent e7c6a45b
...@@ -71,6 +71,7 @@ ...@@ -71,6 +71,7 @@
'webapp/unittests/it2me_service_unittest.js', 'webapp/unittests/it2me_service_unittest.js',
'webapp/unittests/l10n_unittest.js', 'webapp/unittests/l10n_unittest.js',
'webapp/unittests/menu_button_unittest.js', 'webapp/unittests/menu_button_unittest.js',
'webapp/unittests/xhr_unittest.js',
'webapp/unittests/xmpp_connection_unittest.js', 'webapp/unittests/xmpp_connection_unittest.js',
'webapp/unittests/xmpp_login_handler_unittest.js', 'webapp/unittests/xmpp_login_handler_unittest.js',
'webapp/unittests/xmpp_stream_parser_unittest.js', 'webapp/unittests/xmpp_stream_parser_unittest.js',
......
...@@ -85,9 +85,11 @@ remoting.DnsBlackholeChecker.prototype.connect = function(server, ...@@ -85,9 +85,11 @@ remoting.DnsBlackholeChecker.prototype.connect = function(server,
this.signalStrategy_.connect(server, username, authToken); this.signalStrategy_.connect(server, username, authToken);
this.xhr_ = this.xhr_ = remoting.xhr.start({
remoting.xhr.get(remoting.DnsBlackholeChecker.URL_TO_REQUEST_, method: 'GET',
this.onHttpRequestDone_.bind(this)); url: remoting.DnsBlackholeChecker.URL_TO_REQUEST_,
onDone: this.onHttpRequestDone_.bind(this)
});
}; };
remoting.DnsBlackholeChecker.prototype.getState = function() { remoting.DnsBlackholeChecker.prototype.getState = function() {
......
...@@ -272,11 +272,6 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone, ...@@ -272,11 +272,6 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone,
*/ */
function doRegisterHost( function doRegisterHost(
hostName, privateKey, publicKey, hostClientId, oauthToken) { hostName, privateKey, publicKey, hostClientId, oauthToken) {
var headers = {
'Authorization': 'OAuth ' + oauthToken,
'Content-type' : 'application/json; charset=UTF-8'
};
var newHostDetails = { data: { var newHostDetails = { data: {
hostId: newHostId, hostId: newHostId,
hostName: hostName, hostName: hostName,
...@@ -286,16 +281,16 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone, ...@@ -286,16 +281,16 @@ remoting.HostController.prototype.start = function(hostPin, consent, onDone,
var registerHostUrl = var registerHostUrl =
remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts'; remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts';
if (hostClientId) { remoting.xhr.start({
registerHostUrl += '?' + remoting.xhr.urlencodeParamHash( method: 'POST',
{ hostClientId: hostClientId }); url: remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts',
} urlParams: {
hostClientId: hostClientId
remoting.xhr.post( },
registerHostUrl, onDone: onRegistered.bind(null, hostName, publicKey, privateKey),
onRegistered.bind(null, hostName, publicKey, privateKey), jsonContent: newHostDetails,
JSON.stringify(newHostDetails), oauthToken: oauthToken
headers); });
} }
/** /**
......
...@@ -28,12 +28,15 @@ remoting.HostListApiImpl = function() { ...@@ -28,12 +28,15 @@ remoting.HostListApiImpl = function() {
remoting.HostListApiImpl.prototype.get = function(onDone, onError) { remoting.HostListApiImpl.prototype.get = function(onDone, onError) {
/** @type {function(XMLHttpRequest):void} */ /** @type {function(XMLHttpRequest):void} */
var parseHostListResponse = var parseHostListResponse =
this.parseHostListResponse_.bind(this, onDone, onError) this.parseHostListResponse_.bind(this, onDone, onError);
/** @param {string} token */ /** @param {string} token */
var onToken = function(token) { var onToken = function(token) {
var headers = { 'Authorization': 'OAuth ' + token }; remoting.xhr.start({
remoting.xhr.get(remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts', method: 'GET',
parseHostListResponse, '', headers); url: remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts',
onDone: parseHostListResponse,
oauthToken: token
});
}; };
remoting.identity.getToken().then(onToken, remoting.Error.handler(onError)); remoting.identity.getToken().then(onToken, remoting.Error.handler(onError));
}; };
...@@ -51,10 +54,6 @@ remoting.HostListApiImpl.prototype.put = ...@@ -51,10 +54,6 @@ remoting.HostListApiImpl.prototype.put =
function(hostId, hostName, hostPublicKey, onDone, onError) { function(hostId, hostName, hostPublicKey, onDone, onError) {
/** @param {string} token */ /** @param {string} token */
var onToken = function(token) { var onToken = function(token) {
var headers = {
'Authorization': 'OAuth ' + token,
'Content-type' : 'application/json; charset=UTF-8'
};
var newHostDetails = { var newHostDetails = {
'data': { 'data': {
'hostId': hostId, 'hostId': hostId,
...@@ -62,11 +61,13 @@ remoting.HostListApiImpl.prototype.put = ...@@ -62,11 +61,13 @@ remoting.HostListApiImpl.prototype.put =
'publicKey': hostPublicKey 'publicKey': hostPublicKey
} }
}; };
remoting.xhr.put( remoting.xhr.start({
remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts/' + hostId, method: 'PUT',
remoting.xhr.defaultResponse(onDone, onError), url: remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts/' + hostId,
JSON.stringify(newHostDetails), onDone: remoting.xhr.defaultResponse(onDone, onError),
headers); jsonContent: newHostDetails,
oauthToken: token
});
}; };
remoting.identity.getToken().then(onToken, remoting.Error.handler(onError)); remoting.identity.getToken().then(onToken, remoting.Error.handler(onError));
}; };
...@@ -81,11 +82,12 @@ remoting.HostListApiImpl.prototype.put = ...@@ -81,11 +82,12 @@ remoting.HostListApiImpl.prototype.put =
remoting.HostListApiImpl.prototype.remove = function(hostId, onDone, onError) { remoting.HostListApiImpl.prototype.remove = function(hostId, onDone, onError) {
/** @param {string} token */ /** @param {string} token */
var onToken = function(token) { var onToken = function(token) {
var headers = { 'Authorization': 'OAuth ' + token }; remoting.xhr.start({
remoting.xhr.remove( method: 'DELETE',
remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts/' + hostId, url: remoting.settings.DIRECTORY_API_BASE_URL + '/@me/hosts/' + hostId,
remoting.xhr.defaultResponse(onDone, onError), onDone: remoting.xhr.defaultResponse(onDone, onError),
'', headers); oauthToken: token
});
}; };
remoting.identity.getToken().then(onToken, remoting.Error.handler(onError)); remoting.identity.getToken().then(onToken, remoting.Error.handler(onError));
}; };
......
...@@ -93,14 +93,17 @@ remoting.OAuth2ApiImpl.prototype.refreshAccessToken = function( ...@@ -93,14 +93,17 @@ remoting.OAuth2ApiImpl.prototype.refreshAccessToken = function(
} }
}; };
var parameters = { remoting.xhr.start({
'client_id': clientId, method: 'POST',
'client_secret': clientSecret, url: this.getOAuth2TokenEndpoint_(),
'refresh_token': refreshToken, onDone: onResponse,
'grant_type': 'refresh_token' formContent: {
}; 'client_id': clientId,
'client_secret': clientSecret,
remoting.xhr.post(this.getOAuth2TokenEndpoint_(), onResponse, parameters); 'refresh_token': refreshToken,
'grant_type': 'refresh_token'
}
});
}; };
/** /**
...@@ -140,14 +143,18 @@ remoting.OAuth2ApiImpl.prototype.exchangeCodeForTokens = function( ...@@ -140,14 +143,18 @@ remoting.OAuth2ApiImpl.prototype.exchangeCodeForTokens = function(
} }
}; };
var parameters = { remoting.xhr.start({
'client_id': clientId, method: 'POST',
'client_secret': clientSecret, url: this.getOAuth2TokenEndpoint_(),
'redirect_uri': redirectUri, onDone: onResponse,
'code': code, formContent: {
'grant_type': 'authorization_code' 'client_id': clientId,
}; 'client_secret': clientSecret,
remoting.xhr.post(this.getOAuth2TokenEndpoint_(), onResponse, parameters); 'redirect_uri': redirectUri,
'code': code,
'grant_type': 'authorization_code'
}
});
}; };
/** /**
...@@ -177,9 +184,12 @@ remoting.OAuth2ApiImpl.prototype.getEmail = function(onDone, onError, token) { ...@@ -177,9 +184,12 @@ remoting.OAuth2ApiImpl.prototype.getEmail = function(onDone, onError, token) {
onError(remoting.Error.fromHttpStatus(xhr.status)); onError(remoting.Error.fromHttpStatus(xhr.status));
} }
}; };
var headers = { 'Authorization': 'OAuth ' + token }; remoting.xhr.start({
remoting.xhr.get(this.getOAuth2ApiUserInfoEndpoint_(), method: 'GET',
onResponse, '', headers); url: this.getOAuth2ApiUserInfoEndpoint_(),
onDone: onResponse,
oauthToken: token
});
}; };
/** /**
...@@ -210,9 +220,12 @@ remoting.OAuth2ApiImpl.prototype.getUserInfo = ...@@ -210,9 +220,12 @@ remoting.OAuth2ApiImpl.prototype.getUserInfo =
onError(remoting.Error.fromHttpStatus(xhr.status)); onError(remoting.Error.fromHttpStatus(xhr.status));
} }
}; };
var headers = { 'Authorization': 'OAuth ' + token }; remoting.xhr.start({
remoting.xhr.get(this.getOAuth2ApiUserInfoEndpoint_(), method: 'GET',
onResponse, '', headers); url: this.getOAuth2ApiUserInfoEndpoint_(),
onDone: onResponse,
oauthToken: token
});
}; };
/** @type {remoting.OAuth2Api} */ /** @type {remoting.OAuth2Api} */
......
...@@ -433,12 +433,13 @@ remoting.SessionConnectorImpl.prototype.onSignalingState_ = function(state) { ...@@ -433,12 +433,13 @@ remoting.SessionConnectorImpl.prototype.onSignalingState_ = function(state) {
remoting.SessionConnectorImpl.prototype.connectIT2MeWithToken_ = remoting.SessionConnectorImpl.prototype.connectIT2MeWithToken_ =
function(hostId, token) { function(hostId, token) {
// Resolve the host id to get the host JID. // Resolve the host id to get the host JID.
this.pendingXhr_ = remoting.xhr.get( this.pendingXhr_ = remoting.xhr.start({
remoting.settings.DIRECTORY_API_BASE_URL + '/support-hosts/' + method: 'GET',
encodeURIComponent(hostId), url: remoting.settings.DIRECTORY_API_BASE_URL + '/support-hosts/' +
this.onIT2MeHostInfo_.bind(this, hostId), encodeURIComponent(hostId),
'', onDone: this.onIT2MeHostInfo_.bind(this),
{ 'Authorization': 'OAuth ' + token }); oauthToken: token
});
}; };
/** /**
......
This diff is collapsed.
...@@ -10,10 +10,15 @@ var onStateChange = null; ...@@ -10,10 +10,15 @@ var onStateChange = null;
var onIncomingStanzaCallback = null; var onIncomingStanzaCallback = null;
var checker = null; var checker = null;
var signalStrategy = null; var signalStrategy = null;
var fakeXhrs;
module('dns_blackhole_checker', { module('dns_blackhole_checker', {
setup: function() { setup: function() {
sinon.stub(remoting.xhr, 'get'); fakeXhrs = [];
sinon.useFakeXMLHttpRequest().onCreate = function(xhr) {
fakeXhrs.push(xhr);
};
onStateChange = sinon.spy(); onStateChange = sinon.spy();
onIncomingStanzaCallback = sinon.spy(); onIncomingStanzaCallback = sinon.spy();
...@@ -29,8 +34,10 @@ module('dns_blackhole_checker', { ...@@ -29,8 +34,10 @@ module('dns_blackhole_checker', {
sinon.assert.calledWith(signalStrategy.connect, 'server', 'username', sinon.assert.calledWith(signalStrategy.connect, 'server', 'username',
'authToken'); 'authToken');
sinon.assert.calledWith(remoting.xhr.get, QUnit.equal(fakeXhrs.length, 1, 'exactly one XHR is issued');
remoting.DnsBlackholeChecker.URL_TO_REQUEST_); QUnit.equal(
fakeXhrs[0].url, remoting.DnsBlackholeChecker.URL_TO_REQUEST_,
'the correct URL is requested');
}, },
teardown: function() { teardown: function() {
base.dispose(checker); base.dispose(checker);
...@@ -40,14 +47,12 @@ module('dns_blackhole_checker', { ...@@ -40,14 +47,12 @@ module('dns_blackhole_checker', {
onStateChange = null; onStateChange = null;
onIncomingStanzaCallback = null; onIncomingStanzaCallback = null;
checker = null; checker = null;
remoting.xhr.get.restore();
}, },
}); });
test('success', test('success',
function() { function() {
remoting.xhr.get.getCall(0).args[1]({status: 200}); fakeXhrs[0].respond(200);
sinon.assert.notCalled(onStateChange); sinon.assert.notCalled(onStateChange);
[ [
...@@ -82,7 +87,7 @@ test('http response after connected', ...@@ -82,7 +87,7 @@ test('http response after connected',
// Verify that DnsBlackholeChecker goes to CONNECTED state after the // Verify that DnsBlackholeChecker goes to CONNECTED state after the
// the HTTP request has succeeded. // the HTTP request has succeeded.
remoting.xhr.get.getCall(0).args[1]({status: 200}); fakeXhrs[0].respond(200);
sinon.assert.calledWith(onStateChange, sinon.assert.calledWith(onStateChange,
remoting.SignalStrategy.State.CONNECTED); remoting.SignalStrategy.State.CONNECTED);
} }
...@@ -90,7 +95,7 @@ test('http response after connected', ...@@ -90,7 +95,7 @@ test('http response after connected',
test('connect failed', test('connect failed',
function() { function() {
remoting.xhr.get.getCall(0).args[1]({status: 200}); fakeXhrs[0].respond(200);
sinon.assert.notCalled(onStateChange); sinon.assert.notCalled(onStateChange);
[ [
...@@ -105,7 +110,7 @@ test('connect failed', ...@@ -105,7 +110,7 @@ test('connect failed',
test('blocked', test('blocked',
function() { function() {
remoting.xhr.get.getCall(0).args[1]({status: 400}); fakeXhrs[0].respond(400);
sinon.assert.calledWith(onStateChange, sinon.assert.calledWith(onStateChange,
remoting.SignalStrategy.State.FAILED); remoting.SignalStrategy.State.FAILED);
equal(checker.getError(), remoting.Error.NOT_AUTHORIZED); equal(checker.getError(), remoting.Error.NOT_AUTHORIZED);
...@@ -143,7 +148,7 @@ test('blocked after connected', ...@@ -143,7 +148,7 @@ test('blocked after connected',
// Verify that DnsBlackholeChecker goes to FAILED state after it gets the // Verify that DnsBlackholeChecker goes to FAILED state after it gets the
// blocked HTTP response. // blocked HTTP response.
remoting.xhr.get.getCall(0).args[1]({status: 400}); fakeXhrs[0].respond(400);
sinon.assert.calledWith(onStateChange, sinon.assert.calledWith(onStateChange,
remoting.SignalStrategy.State.FAILED); remoting.SignalStrategy.State.FAILED);
equal(checker.getError(), remoting.Error.NOT_AUTHORIZED); equal(checker.getError(), remoting.Error.NOT_AUTHORIZED);
......
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
(function() {
'use strict';
module('xhr', {
setup: function() {
},
teardown: function() {
}
});
test('urlencodeParamHash', function() {
QUnit.equal(
remoting.xhr.urlencodeParamHash({}),
'');
QUnit.equal(
remoting.xhr.urlencodeParamHash({'key': 'value'}),
'key=value');
QUnit.equal(
remoting.xhr.urlencodeParamHash({'key /?=&': 'value /?=&'}),
'key%20%2F%3F%3D%26=value%20%2F%3F%3D%26');
QUnit.equal(
remoting.xhr.urlencodeParamHash({'k1': 'v1', 'k2': 'v2'}),
'k1=v1&k2=v2');
});
asyncTest('basic GET', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'GET',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
}
});
QUnit.equal(request.method, 'GET');
QUnit.equal(request.url, 'http://foo.com');
QUnit.equal(request.withCredentials, false);
QUnit.equal(request.requestBody, null);
QUnit.ok(!('Content-type' in request.requestHeaders));
request.respond(200, {}, 'body');
});
asyncTest('GET with param string', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'GET',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
},
urlParams: 'the_param_string'
});
QUnit.equal(request.method, 'GET');
QUnit.equal(request.url, 'http://foo.com?the_param_string');
QUnit.equal(request.withCredentials, false);
QUnit.equal(request.requestBody, null);
QUnit.ok(!('Content-type' in request.requestHeaders));
request.respond(200, {}, 'body');
});
asyncTest('GET with param object', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'GET',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
},
urlParams: {'a': 'b', 'c': 'd'}
});
QUnit.equal(request.method, 'GET');
QUnit.equal(request.url, 'http://foo.com?a=b&c=d');
QUnit.equal(request.withCredentials, false);
QUnit.equal(request.requestBody, null);
QUnit.ok(!('Content-type' in request.requestHeaders));
request.respond(200, {}, 'body');
});
asyncTest('GET with headers', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'GET',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
},
headers: {'Header1': 'headerValue1', 'Header2': 'headerValue2'}
});
QUnit.equal(request.method, 'GET');
QUnit.equal(request.url, 'http://foo.com');
QUnit.equal(request.withCredentials, false);
QUnit.equal(request.requestBody, null);
QUnit.equal(
request.requestHeaders['Header1'],
'headerValue1');
QUnit.equal(
request.requestHeaders['Header2'],
'headerValue2');
QUnit.ok(!('Content-type' in request.requestHeaders));
request.respond(200, {}, 'body');
});
asyncTest('GET with credentials', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'GET',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
},
withCredentials: true
});
QUnit.equal(request.method, 'GET');
QUnit.equal(request.url, 'http://foo.com');
QUnit.equal(request.withCredentials, true);
QUnit.equal(request.requestBody, null);
QUnit.ok(!('Content-type' in request.requestHeaders));
request.respond(200, {}, 'body');
});
asyncTest('POST with text content', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'POST',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
},
textContent: 'the_content_string'
});
QUnit.equal(request.method, 'POST');
QUnit.equal(request.url, 'http://foo.com');
QUnit.equal(request.withCredentials, false);
QUnit.equal(request.requestBody, 'the_content_string');
QUnit.ok(!('Content-type' in request.requestHeaders));
request.respond(200, {}, 'body');
});
asyncTest('POST with form content', function() {
sinon.useFakeXMLHttpRequest();
var request = remoting.xhr.start({
method: 'POST',
url: 'http://foo.com',
onDone: function(xhr) {
QUnit.ok(xhr === request);
QUnit.equal(xhr.status, 200);
QUnit.equal(xhr.responseText, 'body');
QUnit.start();
},
formContent: {'a': 'b', 'c': 'd'}
});
QUnit.equal(request.method, 'POST');
QUnit.equal(request.url, 'http://foo.com');
QUnit.equal(request.withCredentials, false);
QUnit.equal(request.requestBody, 'a=b&c=d');
QUnit.equal(
request.requestHeaders['Content-type'],
'application/x-www-form-urlencoded');
request.respond(200, {}, 'body');
});
asyncTest('defaultResponse 200', function() {
sinon.useFakeXMLHttpRequest();
var onDone = function() {
QUnit.ok(true);
QUnit.start();
};
var onError = function(error) {
QUnit.ok(false);
QUnit.start();
};
var request = remoting.xhr.start({
method: 'POST',
url: 'http://foo.com',
onDone: remoting.xhr.defaultResponse(onDone, onError)
});
request.respond(200);
});
asyncTest('defaultResponse 404', function() {
sinon.useFakeXMLHttpRequest();
var onDone = function() {
QUnit.ok(false);
QUnit.start();
};
var onError = function(error) {
QUnit.ok(true);
QUnit.start();
};
var request = remoting.xhr.start({
method: 'POST',
url: 'http://foo.com',
onDone: remoting.xhr.defaultResponse(onDone, onError)
});
request.respond(404);
});
})();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment