diff --git a/lib/base-apis.js b/lib/base-apis.js index c53d4043ff3..b926e50a38a 100644 --- a/lib/base-apis.js +++ b/lib/base-apis.js @@ -574,13 +574,38 @@ MatrixBaseApis.prototype.setRoomDirectoryVisibility = /** * Upload a file to the media repository on the home server. - * @param {File} file object - * @param {module:client.callback} callback Optional. - * @return {module:client.Promise} Resolves: TODO - * @return {module:http-api.MatrixError} Rejects: with an error response. + * + * @param {object} file The object to upload. On a browser, something that + * can be sent to XMLHttpRequest.send (typically a File). Under node.js, + * a a Buffer, String or ReadStream. + * + * @param {object} opts options object + * + * @param {string=} opts.name Name to give the file on the server. Defaults + * to file.name. + * + * @param {string=} opts.type Content-type for the upload. Defaults to + * file.type, or applicaton/octet-stream. + * + * @param {boolean=} opts.rawResponse Return the raw body, rather than + * parsing the JSON. Defaults to false (except on node.js, where it + * defaults to true for backwards compatibility). + * + * @param {boolean=} opts.onlyContentUri Just return the content URI, + * rather than the whole body. Defaults to false (except on browsers, + * where it defaults to true for backwards compatibility). Ignored if + * opts.rawResponse is true. + * + * @param {Function=} opts.callback Deprecated. Optional. The callback to + * invoke on success/failure. See the promise return values for more + * information. + * + * @return {module:client.Promise} Resolves to response object, as + * determined by this.opts.onlyData, opts.rawResponse, and + * opts.onlyContentUri. Rejects with an error (usually a MatrixError). */ -MatrixBaseApis.prototype.uploadContent = function(file, callback) { - return this._http.uploadContent(file, callback); +MatrixBaseApis.prototype.uploadContent = function(file, opts) { + return this._http.uploadContent(file, opts); }; /** diff --git a/lib/http-api.js b/lib/http-api.js index 3309f383953..172b89c80b3 100644 --- a/lib/http-api.js +++ b/lib/http-api.js @@ -63,9 +63,11 @@ module.exports.PREFIX_MEDIA_R0 = "/_matrix/media/r0"; * requests. This function must look like function(opts, callback){ ... }. * @param {string} opts.prefix Required. The matrix client prefix to use, e.g. * '/_matrix/client/r0'. See PREFIX_R0 and PREFIX_UNSTABLE for constants. - * @param {bool} opts.onlyData True to return only the 'data' component of the - * response (e.g. the parsed HTTP body). If false, requests will return status - * codes and headers in addition to data. Default: false. + * + * @param {bool=} opts.onlyData True to return only the 'data' component of the + * response (e.g. the parsed HTTP body). If false, requests will return an + * object with the properties code, headers and data. + * * @param {string} opts.accessToken The access_token to send with requests. Can be * null to not send an access token. * @param {Object} opts.extraParams Optional. Extra query parameters to send on @@ -99,20 +101,87 @@ module.exports.MatrixHttpApi.prototype = { /** * Upload content to the Home Server - * @param {File} file A File object (in a browser) or in Node, - an object with properties: - name: The file's name - stream: A read stream - * @param {Function} callback Optional. The callback to invoke on - * success/failure. See the promise return values for more information. - * @return {module:client.Promise} Resolves to {data: {Object}, + * + * @param {object} file The object to upload. On a browser, something that + * can be sent to XMLHttpRequest.send (typically a File). Under node.js, + * a a Buffer, String or ReadStream. + * + * @param {object} opts options object + * + * @param {string=} opts.name Name to give the file on the server. Defaults + * to file.name. + * + * @param {string=} opts.type Content-type for the upload. Defaults to + * file.type, or applicaton/octet-stream. + * + * @param {boolean=} opts.rawResponse Return the raw body, rather than + * parsing the JSON. Defaults to false (except on node.js, where it + * defaults to true for backwards compatibility). + * + * @param {boolean=} opts.onlyContentUri Just return the content URI, + * rather than the whole body. Defaults to false (except on browsers, + * where it defaults to true for backwards compatibility). Ignored if + * opts.rawResponse is true. + * + * @param {Function=} opts.callback Deprecated. Optional. The callback to + * invoke on success/failure. See the promise return values for more + * information. + * + * @return {module:client.Promise} Resolves to response object, as + * determined by this.opts.onlyData, opts.rawResponse, and + * opts.onlyContentUri. Rejects with an error (usually a MatrixError). */ - uploadContent: function(file, callback) { - if (callback !== undefined && !utils.isFunction(callback)) { - throw Error( - "Expected callback to be a function but got " + typeof callback - ); + uploadContent: function(file, opts) { + if (utils.isFunction(opts)) { + // opts used to be callback + opts = { + callback: opts, + }; + } else if (opts === undefined) { + opts = {}; + } + + // if the file doesn't have a mime type, use a default since + // the HS errors if we don't supply one. + var contentType = opts.type || file.type || 'application/octet-stream'; + var fileName = opts.name || file.name; + + // we used to recommend setting file.stream to the thing to upload on + // nodejs. + var body = file.stream ? file.stream : file; + + // backwards-compatibility hacks where we used to do different things + // between browser and node. + var rawResponse = opts.rawResponse; + if (rawResponse === undefined) { + if (global.XMLHttpRequest) { + rawResponse = false; + } else { + console.warn( + "Returning the raw JSON from uploadContent(). Future " + + "versions of the js-sdk will change this default, to " + + "return the parsed object. Set opts.rawResponse=false " + + "to change this behaviour now." + ); + rawResponse = true; + } } + + var onlyContentUri = opts.onlyContentUri; + if (!rawResponse && onlyContentUri === undefined) { + if (global.XMLHttpRequest) { + console.warn( + "Returning only the content-uri from uploadContent(). " + + "Future versions of the js-sdk will change this " + + "default, to return the whole response object. Set " + + "opts.onlyContentUri=false to change this behaviour now." + ); + onlyContentUri = true; + } else { + onlyContentUri = false; + } + } + // browser-request doesn't support File objects because it deep-copies // the options using JSON.parse(JSON.stringify(options)). Instead of // loading the whole file into memory as a string and letting @@ -123,11 +192,30 @@ module.exports.MatrixHttpApi.prototype = { var upload = { loaded: 0, total: 0 }; var promise; + + // XMLHttpRequest doesn't parse JSON for us. request normally does, but + // we're setting opts.json=false so that it doesn't JSON-encode the + // request, which also means it doesn't JSON-decode the response. Either + // way, we have to JSON-parse the response ourselves. + var bodyParser = null; + if (!rawResponse) { + bodyParser = function(rawBody) { + var body = JSON.parse(rawBody); + if (onlyContentUri) { + body = body.content_uri; + if (body === undefined) { + throw Error('Bad response'); + } + } + return body; + }; + } + if (global.XMLHttpRequest) { var defer = q.defer(); var xhr = new global.XMLHttpRequest(); upload.xhr = xhr; - var cb = requestCallback(defer, callback, this.opts.onlyData); + var cb = requestCallback(defer, opts.callback, this.opts.onlyData); var timeout_fn = function() { xhr.abort(); @@ -142,23 +230,21 @@ module.exports.MatrixHttpApi.prototype = { switch (xhr.readyState) { case global.XMLHttpRequest.DONE: callbacks.clearTimeout(xhr.timeout_timer); - var err; - if (!xhr.responseText) { - err = new Error('No response body.'); + var resp; + try { + if (!xhr.responseText) { + throw new Error('No response body.'); + } + resp = xhr.responseText; + if (bodyParser) { + resp = bodyParser(resp); + } + } catch (err) { err.http_status = xhr.status; cb(err); return; } - - var resp = JSON.parse(xhr.responseText); - if (resp.content_uri === undefined) { - err = Error('Bad response'); - err.http_status = xhr.status; - cb(err); - return; - } - - cb(undefined, xhr, resp.content_uri); + cb(undefined, xhr, resp); break; } }; @@ -171,30 +257,26 @@ module.exports.MatrixHttpApi.prototype = { }); var url = this.opts.baseUrl + "/_matrix/media/v1/upload"; url += "?access_token=" + encodeURIComponent(this.opts.accessToken); - url += "&filename=" + encodeURIComponent(file.name); + url += "&filename=" + encodeURIComponent(fileName); xhr.open("POST", url); - if (file.type) { - xhr.setRequestHeader("Content-Type", file.type); - } else { - // if the file doesn't have a mime type, use a default since - // the HS errors if we don't supply one. - xhr.setRequestHeader("Content-Type", 'application/octet-stream'); - } - xhr.send(file); + xhr.setRequestHeader("Content-Type", contentType); + xhr.send(body); promise = defer.promise; // dirty hack (as per _request) to allow the upload to be cancelled. promise.abort = xhr.abort.bind(xhr); } else { var queryParams = { - filename: file.name, + filename: fileName, }; + promise = this.authedRequest( - callback, "POST", "/upload", queryParams, file.stream, { + opts.callback, "POST", "/upload", queryParams, body, { prefix: "/_matrix/media/v1", - headers: {"Content-Type": file.type}, + headers: {"Content-Type": contentType}, json: false, + bodyParser: bodyParser, } ); } @@ -495,6 +577,32 @@ module.exports.MatrixHttpApi.prototype = { return this.opts.baseUrl + prefix + path + queryString; }, + /** + * @private + * + * @param {function} callback + * @param {string} method + * @param {string} uri + * @param {object} queryParams + * @param {object|string} data + * @param {object=} opts + * + * @param {boolean=true} opts.json Json-encode data before sending, and + * decode response on receipt. (We will still json-decode error + * responses, even if this is false.) + * + * @param {object=} opts.headers extra request headers + * + * @param {number=} opts.localTimeoutMs client-side timeout for the + * request. No timeout if undefined. + * + * @param {function=} opts.bodyParser function to parse the body of the + * response before passing it to the promise and callback. + * + * @return {module:client.Promise} a promise which resolves to either the + * response object (if this.opts.onlyData is truthy), or the parsed + * body. Rejects + */ _request: function(callback, method, uri, queryParams, data, opts) { if (callback !== undefined && !utils.isFunction(callback)) { throw Error( @@ -510,6 +618,9 @@ module.exports.MatrixHttpApi.prototype = { queryParams[key] = this.opts.extraParams[key]; } } + + var json = opts.json === undefined ? true : opts.json; + var defer = q.defer(); var timeoutId; @@ -540,7 +651,7 @@ module.exports.MatrixHttpApi.prototype = { withCredentials: false, qs: queryParams, body: data, - json: opts.json === undefined ? true : opts.json, + json: json, timeout: localTimeoutMs, headers: opts.headers || {}, _matrix_opts: this.opts @@ -552,7 +663,15 @@ module.exports.MatrixHttpApi.prototype = { return; // already rejected promise } } - var handlerFn = requestCallback(defer, callback, self.opts.onlyData); + + // if json is falsy, we won't parse any error response, so need + // to do so before turning it into a MatrixError + var parseErrorJson = !json; + var handlerFn = requestCallback( + defer, callback, self.opts.onlyData, + parseErrorJson, + opts.bodyParser + ); handlerFn(err, response, body); } ); @@ -579,14 +698,34 @@ module.exports.MatrixHttpApi.prototype = { * * If onlyData is true, the defer/callback is invoked with the body of the * response, otherwise the result code. + * + * If parseErrorJson is true, we will JSON.parse the body if we get a 4xx error. + * */ -var requestCallback = function(defer, userDefinedCallback, onlyData) { +var requestCallback = function( + defer, userDefinedCallback, onlyData, + parseErrorJson, bodyParser +) { userDefinedCallback = userDefinedCallback || function() {}; return function(err, response, body) { - if (!err && response.statusCode >= 400) { - err = new module.exports.MatrixError(body); - err.httpStatus = response.statusCode; + if (!err) { + try { + if (response.statusCode >= 400) { + if (parseErrorJson) { + // we won't have json-decoded the response. + body = JSON.parse(body); + } + err = new module.exports.MatrixError(body); + } else if (bodyParser) { + body = bodyParser(body); + } + } catch (e) { + err = e; + } + if (err) { + err.httpStatus = response.statusCode; + } } if (err) { diff --git a/spec/integ/matrix-client-methods.spec.js b/spec/integ/matrix-client-methods.spec.js index 4cc5f8e7487..f3b941cd69d 100644 --- a/spec/integ/matrix-client-methods.spec.js +++ b/spec/integ/matrix-client-methods.spec.js @@ -43,17 +43,13 @@ describe("MatrixClient", function() { httpBackend.when( "POST", "/_matrix/media/v1/upload" ).check(function(req) { - console.log("Request", req); - expect(req.data).toEqual(buf); expect(req.queryParams.filename).toEqual("hi.txt"); expect(req.queryParams.access_token).toEqual(accessToken); expect(req.headers["Content-Type"]).toEqual("text/plain"); expect(req.opts.json).toBeFalsy(); expect(req.opts.timeout).toBe(undefined); - }).respond(200, { - "content_uri": "uri" - }); + }).respond(200, "content"); var prom = client.uploadContent({ stream: buf, @@ -69,8 +65,8 @@ describe("MatrixClient", function() { expect(uploads[0].loaded).toEqual(0); prom.then(function(response) { - console.log("Response", response); - expect(response.content_uri).toEqual("uri"); + // for backwards compatibility, we return the raw JSON + expect(response).toEqual("content"); var uploads = client.getCurrentUploads(); expect(uploads.length).toEqual(0); @@ -79,6 +75,53 @@ describe("MatrixClient", function() { httpBackend.flush(); }); + it("should parse the response if rawResponse=false", function(done) { + httpBackend.when( + "POST", "/_matrix/media/v1/upload" + ).check(function(req) { + expect(req.opts.json).toBeFalsy(); + }).respond(200, JSON.stringify({ "content_uri": "uri" })); + + client.uploadContent({ + stream: buf, + name: "hi.txt", + type: "text/plain", + }, { + rawResponse: false, + }).then(function(response) { + expect(response.content_uri).toEqual("uri"); + }).catch(utils.failTest).done(done); + + httpBackend.flush(); + }); + + it("should parse errors into a MatrixError", function(done) { + // opts.json is false, so request returns unparsed json. + httpBackend.when( + "POST", "/_matrix/media/v1/upload" + ).check(function(req) { + expect(req.data).toEqual(buf); + expect(req.opts.json).toBeFalsy(); + }).respond(400, JSON.stringify({ + "errcode": "M_SNAFU", + "error": "broken", + })); + + client.uploadContent({ + stream: buf, + name: "hi.txt", + type: "text/plain", + }).then(function(response) { + throw Error("request not failed"); + }, function(error) { + expect(error.httpStatus).toEqual(400); + expect(error.errcode).toEqual("M_SNAFU"); + expect(error.message).toEqual("broken"); + }).catch(utils.failTest).done(done); + + httpBackend.flush(); + }); + it("should return a promise which can be cancelled", function(done) { var prom = client.uploadContent({ stream: buf,