Skip to content

Commit 2ab003b

Browse files
committed
Adds the upload asset release api
Here's a first pass at adding this feature. Please tell me if you'd like it implemented a different way. Of course I like it as is :P but some things to consider * Content-Type Currently I set the content type automatically based on the name you pass in. This is a little magic as it assumes there's both a `filePath` and `name` field. It could be required to be set by the user as in github.releases.uploadAsset({ owner: owner, id: releaseId, headers: { "content-type": "application/zip", } ... Similarly the size could be manually set there. But, since, based on the current API there's no way for us to upload the file (we don't get the request object back, and we can't just pass the data (it's arguably too big. Could be 100s of meg, more than fits in node's memory space), it seems like we should lookup the size and stream the file as it does now? * Timeout Wasn't sure what to do about that. Timeouts don't seem appropriate for uploading giant files or at least can't easily be a global setting. Anyway, if you don't like this patch hopefully you can suggest a way you'd prefer it.
1 parent f841677 commit 2ab003b

File tree

6 files changed

+184
-59
lines changed

6 files changed

+184
-59
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ github.authorization.create({
147147
* Search: 100%
148148
* Markdown: 100%
149149
* Rate Limit: 100%
150-
* Releases: 90%
150+
* Releases: 100%
151151
* Gitignore: 100%
152152
* Meta: 100%
153153
* Emojis: 100%

api/v3.0.0/releases.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,49 @@ var releases = module.exports = {
325325
});
326326
};
327327

328+
/** section: github
329+
* releases#uploadAsset(msg, callback) -> null
330+
* - msg (Object): Object that contains the parameters and their values to be sent to the server.
331+
* - callback (Function): function to call when the request is finished with an error as first argument and result data as second argument.
332+
*
333+
* ##### Params on the `msg` object:
334+
*
335+
* - headers (Object): Optional. Key/ value pair of request headers to pass along with the HTTP request. Valid headers are: 'If-Modified-Since', 'If-None-Match', 'Cookie', 'User-Agent', 'Accept', 'X-GitHub-OTP'.
336+
* - owner (String): Required.
337+
* - id (Number): Required.
338+
* - repo (String): Required.
339+
* - name (String): Required. the file name of the asset
340+
**/
341+
this.uploadAsset = function(msg, block, callback) {
342+
var self = this;
343+
this.client.httpSend(msg, block, function(err, res) {
344+
if (err)
345+
return self.sendError(err, null, msg, callback);
346+
347+
var ret;
348+
try {
349+
ret = res.data && JSON.parse(res.data);
350+
}
351+
catch (ex) {
352+
if (callback)
353+
callback(new error.InternalServerError(ex.message), res);
354+
return;
355+
}
356+
357+
if (!ret)
358+
ret = {};
359+
if (!ret.meta)
360+
ret.meta = {};
361+
["x-ratelimit-limit", "x-ratelimit-remaining", "x-ratelimit-reset", "x-oauth-scopes", "link", "location", "last-modified", "etag", "status"].forEach(function(header) {
362+
if (res.headers[header])
363+
ret.meta[header] = res.headers[header];
364+
});
365+
366+
if (callback)
367+
callback(null, ret);
368+
});
369+
};
370+
328371
/** section: github
329372
* releases#editAsset(msg, callback) -> null
330373
* - msg (Object): Object that contains the parameters and their values to be sent to the server.

api/v3.0.0/releasesTest.js

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
var Assert = require("assert");
1313
var Client = require("./../../index");
14+
var fs = require("fs");
15+
var mime = require("mime");
1416

1517
describe("[releases]", function() {
1618
var client;
@@ -20,6 +22,8 @@ describe("[releases]", function() {
2022
var repo = "test";
2123
var haveWriteAccess = true; // set to false if the authenticated person below does not have write access to the repo above
2224
var releaseIdWithAsset = 393621; // Some release id from the repo above that has at least 1 asset.
25+
var filePathToUpload = __filename;
26+
var fileSizeToUpload = fs.statSync(filePathToUpload).size;
2327

2428
var releaseId; // release id found when listing releases. Used for get release
2529
var newReleaseId; // release id created when creating release, used for edit and delete release
@@ -190,22 +194,48 @@ describe("[releases]", function() {
190194
);
191195
});
192196

197+
it("should successfully execute POST /repos/:owner/:repo/releases/:id/assets (uploadAsset)", function(next) {
198+
var name = "somenameornot.zip";
199+
client.releases.uploadAsset(
200+
{
201+
owner: owner,
202+
id: releaseIdWithAsset,
203+
repo: repo,
204+
name: name,
205+
filePath: filePathToUpload
206+
},
207+
function(err, res) {
208+
Assert.equal(err, null);
209+
Assert.equal(res.content_type, mime.lookup(name)); // matches extension of name, not filePath
210+
Assert.equal(res.state, "uploaded");
211+
Assert.equal(res.size, fileSizeToUpload);
212+
Assert.equal(res.name, name);
213+
newAssetId = res.id;
214+
next();
215+
}
216+
);
217+
});
218+
193219
it("should successfully execute PATCH /repos/:owner/:repo/releases/assets/:id (editAsset)", function(next) {
194220
if (!newAssetId) {
195221
next();
196222
return;
197223
}
224+
var newName = "somenewname.zip";
198225
client.releases.editAsset(
199226
{
200227
owner: owner,
201-
id: "Number",
228+
id: newAssetId,
202229
repo: repo,
203-
name: "String",
204-
label: "String"
230+
name: newName,
231+
label: "foo"
205232
},
206233
function(err, res) {
207234
Assert.equal(err, null);
208-
// other assertions go here
235+
Assert.equal(res.state, "uploaded");
236+
Assert.equal(res.size, fileSizeToUpload);
237+
Assert.equal(res.name, newName);
238+
Assert.equal(res.label, "foo");
209239
next();
210240
}
211241
);
@@ -219,7 +249,7 @@ describe("[releases]", function() {
219249
client.releases.deleteAsset(
220250
{
221251
owner: owner,
222-
id: "Number",
252+
id: newAssetId,
223253
repo: repo
224254
},
225255
function(err, res) {

api/v3.0.0/routes.json

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3452,6 +3452,37 @@
34523452
"$repo": null
34533453
}
34543454
},
3455+
"upload-asset": {
3456+
"url": "/repos/:owner/:repo/releases/:id/assets",
3457+
"method": "POST",
3458+
"host": "uploads.github.com",
3459+
"hasFileBody": true,
3460+
"timeout": 0,
3461+
"params": {
3462+
"owner": {
3463+
"type": "String",
3464+
"required": true,
3465+
"validation": "",
3466+
"invalidmsg": "",
3467+
"description": ""
3468+
},
3469+
"id": {
3470+
"type": "Number",
3471+
"required": true,
3472+
"validation": "",
3473+
"invalidmsg": "",
3474+
"description": ""
3475+
},
3476+
"$repo": null,
3477+
"name": {
3478+
"type": "String",
3479+
"required": true,
3480+
"validation": "",
3481+
"invalidmsg": "",
3482+
"description": "the file name of the asset"
3483+
}
3484+
}
3485+
},
34553486
"edit-asset": {
34563487
"url": "/repos/:owner/:repo/releases/assets/:id",
34573488
"method": "PATCH",

index.js

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"use strict";
22

33
var error = require("./error");
4+
var fs = require("fs");
5+
var mime = require("mime");
46
var Util = require("./util");
57
var Url = require("url");
68

@@ -645,7 +647,8 @@ var Client = module.exports = function(config) {
645647
this.httpSend = function(msg, block, callback) {
646648
var self = this;
647649
var method = block.method.toLowerCase();
648-
var hasBody = ("head|get|delete".indexOf(method) === -1);
650+
var hasFileBody = block.hasFileBody;
651+
var hasBody = !hasFileBody && ("head|get|delete".indexOf(method) === -1);
649652
var format = hasBody && this.constants.requestFormat
650653
? this.constants.requestFormat
651654
: "query";
@@ -655,9 +658,8 @@ var Client = module.exports = function(config) {
655658

656659
var path = url;
657660
var protocol = this.config.protocol || this.constants.protocol || "http";
658-
var host = this.config.host || this.constants.host;
661+
var host = block.host || this.config.host || this.constants.host;
659662
var port = this.config.port || this.constants.port || (protocol == "https" ? 443 : 80);
660-
661663
var proxyUrl;
662664
if (this.config.proxy !== undefined) {
663665
proxyUrl = this.config.proxy;
@@ -722,6 +724,14 @@ var Client = module.exports = function(config) {
722724
}
723725
}
724726

727+
function callCallback(err, result) {
728+
if (callback) {
729+
var cb = callback;
730+
callback = undefined;
731+
cb(err, result);
732+
}
733+
}
734+
725735
function addCustomHeaders(customHeaders) {
726736
Object.keys(customHeaders).forEach(function(header) {
727737
var headerLC = header.toLowerCase();
@@ -752,66 +762,74 @@ var Client = module.exports = function(config) {
752762
if (this.debug)
753763
console.log("REQUEST: ", options);
754764

755-
var callbackCalled = false
756-
757-
var req = require(protocol).request(options, function(res) {
758-
if (self.debug) {
759-
console.log("STATUS: " + res.statusCode);
760-
console.log("HEADERS: " + JSON.stringify(res.headers));
761-
}
762-
res.setEncoding("utf8");
763-
var data = "";
764-
res.on("data", function(chunk) {
765-
data += chunk;
766-
});
767-
res.on("error", function(err) {
768-
if (!callbackCalled) {
769-
callbackCalled = true;
770-
callback(err);
765+
function httpSendRequest() {
766+
var req = require(protocol).request(options, function(res) {
767+
if (self.debug) {
768+
console.log("STATUS: " + res.statusCode);
769+
console.log("HEADERS: " + JSON.stringify(res.headers));
771770
}
771+
res.setEncoding("utf8");
772+
var data = "";
773+
res.on("data", function(chunk) {
774+
data += chunk;
775+
});
776+
res.on("error", function(err) {
777+
callCallback(err);
778+
});
779+
res.on("end", function() {
780+
if (res.statusCode >= 400 && res.statusCode < 600 || res.statusCode < 10) {
781+
callCallback(new error.HttpError(data, res.statusCode));
782+
} else {
783+
res.data = data;
784+
callCallback(null, res);
785+
}
786+
});
772787
});
773-
res.on("end", function() {
774-
if (callbackCalled)
775-
return;
776788

777-
callbackCalled = true;
778-
if (res.statusCode >= 400 && res.statusCode < 600 || res.statusCode < 10) {
779-
callback(new error.HttpError(data, res.statusCode));
780-
} else {
781-
res.data = data;
782-
callback(null, res);
783-
}
789+
var timeout = (block.timeout !== undefined) ? block.timeout : self.config.timeout;
790+
if (timeout) {
791+
req.setTimeout(timeout);
792+
}
793+
794+
req.on("error", function(e) {
795+
if (self.debug)
796+
console.log("problem with request: " + e.message);
797+
callCallback(e.message);
784798
});
785-
});
786799

787-
if (this.config.timeout) {
788-
req.setTimeout(this.config.timeout);
789-
}
800+
req.on("timeout", function() {
801+
if (self.debug)
802+
console.log("problem with request: timed out");
803+
callCallback(new error.GatewayTimeout());
804+
});
790805

791-
req.on("error", function(e) {
792-
if (self.debug)
793-
console.log("problem with request: " + e.message);
794-
if (!callbackCalled) {
795-
callbackCalled = true;
796-
callback(e.message);
806+
// write data to request body
807+
if (hasBody && query.length) {
808+
if (self.debug)
809+
console.log("REQUEST BODY: " + query + "\n");
810+
req.write(query + "\n");
797811
}
798-
});
799812

800-
req.on("timeout", function() {
801-
if (self.debug)
802-
console.log("problem with request: timed out");
803-
if (!callbackCalled) {
804-
callbackCalled = true;
805-
callback(new error.GatewayTimeout());
813+
if (block.hasFileBody) {
814+
var stream = fs.createReadStream(msg.filePath);
815+
stream.pipe(req);
816+
} else {
817+
req.end();
806818
}
807-
});
819+
};
808820

809-
// write data to request body
810-
if (hasBody && query.length) {
811-
if (self.debug)
812-
console.log("REQUEST BODY: " + query + "\n");
813-
req.write(query + "\n");
821+
if (hasFileBody) {
822+
fs.stat(msg.filePath, function(err, stat) {
823+
if (err) {
824+
callCallback(err);
825+
} else {
826+
headers["content-length"] = stat.size;
827+
headers["content-type"] = mime.lookup(msg.name);
828+
httpSendRequest();
829+
}
830+
});
831+
} else {
832+
httpSendRequest();
814833
}
815-
req.end();
816834
};
817835
}).call(Client.prototype);

package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
"engine" : {
1616
"node": ">=0.4.0"
1717
},
18+
"dependencies": {
19+
"mime": "^1.2.11"
20+
},
1821
"devDependencies": {
1922
"oauth": "~0.9.7",
2023
"optimist": "~0.6.0",

0 commit comments

Comments
 (0)