Skip to content

Commit

Permalink
Fix uploadContent for node.js
Browse files Browse the repository at this point in the history
9e89e71 broke uploadContent, making it set 'json=true' on the request, so that
we would try to turn raw content into JSON. It also misguidedly set a
client-side timeout of 30s.

Fix that, and add some tests to check uploadContent works.

In mock-request: distinguish between an expectation (ExpectedRequest)
and an actual request (Request). Add support for checking the headers, and the
request options in general, to Request.
  • Loading branch information
richvdh committed Oct 8, 2016
1 parent 74d6cb8 commit a3d86c0
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 13 deletions.
6 changes: 4 additions & 2 deletions lib/http-api.js
Expand Up @@ -134,6 +134,8 @@ module.exports.MatrixHttpApi.prototype = {
cb(new Error('Timeout'));
};

// set an initial timeout of 30s; we'll advance it each time we get
// a progress notification
xhr.timeout_timer = callbacks.setTimeout(timeout_fn, 30000);

xhr.onreadystatechange = function() {
Expand Down Expand Up @@ -191,8 +193,8 @@ module.exports.MatrixHttpApi.prototype = {
promise = this.authedRequest(
callback, "POST", "/upload", queryParams, file.stream, {
prefix: "/_matrix/media/v1",
localTimeoutMs: 30000,
headers: {"Content-Type": file.type},
json: false,
}
);
}
Expand Down Expand Up @@ -538,7 +540,7 @@ module.exports.MatrixHttpApi.prototype = {
withCredentials: false,
qs: queryParams,
body: data,
json: true,
json: opts.json === undefined ? true : opts.json,
timeout: localTimeoutMs,
headers: opts.headers || {},
_matrix_opts: this.opts
Expand Down
68 changes: 68 additions & 0 deletions spec/integ/matrix-client-methods.spec.js
Expand Up @@ -37,6 +37,74 @@ describe("MatrixClient", function() {
httpBackend.verifyNoOutstandingExpectation();
});

describe("uploadContent", function() {
var buf = new Buffer('hello world');
it("should upload the file", function(done) {
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"
});

var prom = client.uploadContent({
stream: buf,
name: "hi.txt",
type: "text/plain",
});

expect(prom).toBeDefined();

var uploads = client.getCurrentUploads();
expect(uploads.length).toEqual(1);
expect(uploads[0].promise).toBe(prom);
expect(uploads[0].loaded).toEqual(0);

prom.then(function(response) {
console.log("Response", response);
expect(response.content_uri).toEqual("uri");

var uploads = client.getCurrentUploads();
expect(uploads.length).toEqual(0);
}).catch(utils.failTest).done(done);

httpBackend.flush();
});

it("should return a promise which can be cancelled", function(done) {
var prom = client.uploadContent({
stream: buf,
name: "hi.txt",
type: "text/plain",
});

var uploads = client.getCurrentUploads();
expect(uploads.length).toEqual(1);
expect(uploads[0].promise).toBe(prom);
expect(uploads[0].loaded).toEqual(0);

prom.then(function(response) {
throw Error("request not aborted");
}, function(error) {
expect(error).toEqual("aborted");

var uploads = client.getCurrentUploads();
expect(uploads.length).toEqual(0);
}).catch(utils.failTest).done(done);

var r = client.cancelUpload(prom);
expect(r).toBe(true);
});
});

describe("joinRoom", function() {
it("should no-op if you've already joined a room", function() {
var roomId = "!foo:bar";
Expand Down
69 changes: 58 additions & 11 deletions spec/mock-request.js
Expand Up @@ -11,18 +11,17 @@ function HttpBackend() {
var self = this;
// the request function dependency that the SDK needs.
this.requestFn = function(opts, callback) {
var realReq = new Request(opts.method, opts.uri, opts.body, opts.qs);
realReq.callback = callback;
console.log("HTTP backend received request: %s %s", opts.method, opts.uri);
self.requests.push(realReq);
var req = new Request(opts, callback);
console.log("HTTP backend received request: %s", req);
self.requests.push(req);

var abort = function() {
var idx = self.requests.indexOf(realReq);
var idx = self.requests.indexOf(req);
if (idx >= 0) {
console.log("Aborting HTTP request: %s %s", opts.method,
opts.uri);
self.requests.splice(idx, 1);
realReq.callback("aborted");
req.callback("aborted");
}
};

Expand Down Expand Up @@ -161,22 +160,32 @@ HttpBackend.prototype = {
* @return {Request} An expected request.
*/
when: function(method, path, data) {
var pendingReq = new Request(method, path, data);
var pendingReq = new ExpectedRequest(method, path, data);
this.expectedRequests.push(pendingReq);
return pendingReq;
}
};

function Request(method, path, data, queryParams) {
/**
* Represents the expectation of a request.
*
* <p>Includes the conditions to be matched against, the checks to be made,
* and the response to be returned.
*
* @constructor
* @param {string} method
* @param {string} path
* @param {object?} data
*/
function ExpectedRequest(method, path, data) {
this.method = method;
this.path = path;
this.data = data;
this.queryParams = queryParams;
this.callback = null;
this.response = null;
this.checks = [];
}
Request.prototype = {

ExpectedRequest.prototype = {
/**
* Execute a check when this request has been satisfied.
* @param {Function} fn The function to execute.
Expand Down Expand Up @@ -221,6 +230,44 @@ Request.prototype = {
}
};

/**
* Represents a request made by the app.
*
* @constructor
* @param {object} opts opts passed to request()
* @param {function} callback
*/
function Request(opts, callback) {
this.opts = opts;
this.callback = callback;

Object.defineProperty(this, 'method', {
get: function() { return opts.method; }
});

Object.defineProperty(this, 'path', {
get: function() { return opts.uri; }
});

Object.defineProperty(this, 'data', {
get: function() { return opts.body; }
});

Object.defineProperty(this, 'queryParams', {
get: function() { return opts.qs; }
});

Object.defineProperty(this, 'headers', {
get: function() { return opts.headers || {}; }
});
}

Request.prototype = {
toString: function() {
return this.method + " " + this.path;
},
};

/**
* The HttpBackend class.
*/
Expand Down

0 comments on commit a3d86c0

Please sign in to comment.