-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
improves coverage and API of httpRequest #576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,154 @@ | ||
var httpRequest = require("../src/httpRequest"), | ||
bodyParser = require('body-parser'), | ||
express = require("express"); | ||
|
||
var port = 13371; | ||
var httpRequestServer = "http://localhost:"+port; | ||
|
||
var app = express(); | ||
app.use(bodyParser.json({ 'type': '*/*' })); | ||
app.get("/hello", function(req, res){ | ||
res.json({response: "OK"}); | ||
}); | ||
|
||
app.get("/404", function(req, res){ | ||
res.status(404); | ||
res.send("NO"); | ||
}); | ||
|
||
app.get("/301", function(req, res){ | ||
res.status(301); | ||
res.location("/hello"); | ||
res.send(); | ||
}); | ||
|
||
app.post('/echo', function(req, res){ | ||
res.json(req.body); | ||
}) | ||
|
||
app.listen(13371); | ||
|
||
|
||
describe("httpRequest", () => { | ||
|
||
it("should do /hello", (done) => { | ||
httpRequest({ | ||
url: httpRequestServer+"/hello" | ||
}).then(function(httpResponse){ | ||
expect(httpResponse.status).toBe(200); | ||
expect(httpResponse.buffer).toEqual(new Buffer('{"response":"OK"}')); | ||
expect(httpResponse.text).toEqual('{"response":"OK"}'); | ||
expect(httpResponse.data.response).toEqual("OK"); | ||
done(); | ||
}, function(){ | ||
fail("should not fail"); | ||
done(); | ||
}) | ||
}); | ||
|
||
it("should do /hello with callback and promises", (done) => { | ||
var calls = 0; | ||
httpRequest({ | ||
url: httpRequestServer+"/hello", | ||
success: function() { calls++; }, | ||
error: function() { calls++; } | ||
}).then(function(httpResponse){ | ||
expect(calls).toBe(1); | ||
expect(httpResponse.status).toBe(200); | ||
expect(httpResponse.buffer).toEqual(new Buffer('{"response":"OK"}')); | ||
expect(httpResponse.text).toEqual('{"response":"OK"}'); | ||
expect(httpResponse.data.response).toEqual("OK"); | ||
done(); | ||
}, function(){ | ||
fail("should not fail"); | ||
done(); | ||
}) | ||
}); | ||
|
||
it("should do not follow redirects by default", (done) => { | ||
|
||
httpRequest({ | ||
url: httpRequestServer+"/301" | ||
}).then(function(httpResponse){ | ||
expect(httpResponse.status).toBe(301); | ||
done(); | ||
}, function(){ | ||
fail("should not fail"); | ||
done(); | ||
}) | ||
}); | ||
|
||
it("should follow redirects when set", (done) => { | ||
|
||
httpRequest({ | ||
url: httpRequestServer+"/301", | ||
followRedirects: true | ||
}).then(function(httpResponse){ | ||
expect(httpResponse.status).toBe(200); | ||
expect(httpResponse.buffer).toEqual(new Buffer('{"response":"OK"}')); | ||
expect(httpResponse.text).toEqual('{"response":"OK"}'); | ||
expect(httpResponse.data.response).toEqual("OK"); | ||
done(); | ||
}, function(){ | ||
fail("should not fail"); | ||
done(); | ||
}) | ||
}); | ||
|
||
it("should fail on 404", (done) => { | ||
var calls = 0; | ||
httpRequest({ | ||
url: httpRequestServer+"/404", | ||
success: function() { | ||
calls++; | ||
fail("should not succeed"); | ||
done(); | ||
}, | ||
error: function(httpResponse) { | ||
calls++; | ||
expect(calls).toBe(1); | ||
expect(httpResponse.status).toBe(404); | ||
expect(httpResponse.buffer).toEqual(new Buffer('NO')); | ||
expect(httpResponse.text).toEqual('NO'); | ||
expect(httpResponse.data).toBe(undefined); | ||
done(); | ||
} | ||
}); | ||
}) | ||
|
||
it("should fail on 404", (done) => { | ||
httpRequest({ | ||
url: httpRequestServer+"/404", | ||
}).then(function(httpResponse){ | ||
fail("should not succeed"); | ||
done(); | ||
}, function(httpResponse){ | ||
expect(httpResponse.status).toBe(404); | ||
expect(httpResponse.buffer).toEqual(new Buffer('NO')); | ||
expect(httpResponse.text).toEqual('NO'); | ||
expect(httpResponse.data).toBe(undefined); | ||
done(); | ||
}) | ||
}) | ||
|
||
it("should post on echo", (done) => { | ||
var calls = 0; | ||
httpRequest({ | ||
method: "POST", | ||
url: httpRequestServer+"/echo", | ||
body: { | ||
foo: "bar" | ||
}, | ||
success: function() { calls++; }, | ||
error: function() { calls++; } | ||
}).then(function(httpResponse){ | ||
expect(calls).toBe(1); | ||
expect(httpResponse.status).toBe(200); | ||
expect(httpResponse.data).toEqual({foo: "bar"}); | ||
done(); | ||
}, function(httpResponse){ | ||
fail("should not fail"); | ||
done(); | ||
}) | ||
}) | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,15 @@ module.exports = function(options) { | |
}; | ||
delete options.success; | ||
delete options.error; | ||
if (options.uri && !options.url) { | ||
options.uri = options.url; | ||
delete options.url; | ||
} | ||
delete options.uri; // not supported | ||
if (typeof options.body === 'object') { | ||
options.body = JSON.stringify(options.body); | ||
options.headers = options.headers || {}; | ||
options.headers['Content-Type'] = "application/json"; | ||
} | ||
// set follow redirects to false by default | ||
options.followRedirect = options.followRedirects == true ? true : false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to have snuck in by accident I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to match Parse.Cloud.httpRequest documentation vs request. http://www.parse.com/docs/js/api/classes/Parse.Cloud.HTTPOptions.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I was referring to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right :) |
||
|
||
request(options, (error, response, body) => { | ||
var httpResponse = {}; | ||
httpResponse.status = response.statusCode; | ||
|
@@ -29,12 +31,12 @@ module.exports = function(options) { | |
// Consider <200 && >= 400 as errors | ||
if (error || httpResponse.status <200 || httpResponse.status >=400) { | ||
if (callbacks.error) { | ||
return callbacks.error(httpResponse); | ||
callbacks.error(httpResponse); | ||
} | ||
return promise.reject(httpResponse); | ||
} else { | ||
if (callbacks.success) { | ||
return callbacks.success(httpResponse); | ||
callbacks.success(httpResponse); | ||
} | ||
return promise.resolve(httpResponse); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You sure this isn't going to break anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uri is not supported in Parse.Cloud.httpRequest AFAIK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍