Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

#101 list objects #103

Closed
wants to merge 2 commits into from

2 participants

@kof

If this one is ok I will also add #listFiles.

@domenic

We need res.on('error', fn) too.

right.

@domenic

It'd be awesome to booleanize and numberize as necessary. E.g.

data.IsTruncated = Boolean(data.IsTruncated);
data.MaxKeys = Number(data.MaxKeys);
data.Contents.forEach(function (object) {
    object.Size = Number(object.Size);
    object.LastModified = new Date(object.LastModified);
});

any Ideas how to make it generic and not too slow?

Hmm, not really. But I think since we know the possible keys returned, just explicitly fixing them on a case-by-case basis is OK.

@domenic

Don't update the version in a pull request, I'll do that when I do a release :)

@domenic

You'll probably want to add these to the multiple-delete test to try and clean up.

I think tests should be independent from each other, so I can just clean them up

Good point.

@domenic

This is a nice test; well done.

@domenic

It's not really res (which I would expect to be a HTTP response object); it's more like data I think.

@domenic

Very nicely done. I left a bunch of little comments but I look forward to merging this. Feel free to amend last commit and force-push.

@kof kof commented on the diff
test/knox.test.js
((6 lines not shown))
+
+ client.putFile(jsonFixture, files[0], function(err, res){
+ client.putFile(jsonFixture, files[1], function(err, res){
+ client.list({prefix: 'list'}, function(err, data){
+ assert.ok(!err);
+ assert.equal(data.Prefix, 'list');
+ assert.strictEqual(data.IsTruncated, true);
+ assert.strictEqual(data.MaxKeys, 1000);
+ assert.equal(data.Contents.length, 2);
+ assert.ok(data.Contents[0].LastModified instanceof Date);
+ assert.equal(typeof data.Contents[0].Size, 'number');
+ assert.deepEqual(
+ Object.keys(data.Contents[0]),
+ ['Key', 'LastModified', 'ETag', 'Size', 'Owner', 'StorageClass']
+ );
+ client.deleteMultiple(files, function(err, res) {
@kof
kof added a note

it will never get to this callback, err == null, res.statusCode == 200 ....

do you have the same issue?

@domenic
domenic added a note

No, it's not hanging at all for me... test passes perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kof

Everything is done except of hanging on ln 354 in knox.test.js

@domenic

I'm ready to merge, since it passes for me and the tests have been flaky for you (which I still really want to fix :-/). Love the normalizeResponse function; very clever. Any last-minute changes you want to make?

@kof

No :)

@domenic domenic closed this in c8a8631
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 10, 2012
  1. @kof

    #101 list objects

    kof authored
  2. @kof

    #101 code review changes

    kof authored
This page is out of date. Refresh to see the latest.
Showing with 131 additions and 3 deletions.
  1. +102 −1 lib/client.js
  2. +4 −2 package.json
  3. +25 −0 test/knox.test.js
View
103 lib/client.js
@@ -16,7 +16,9 @@ var Emitter = require('events').EventEmitter
, url = require('url')
, mime = require('mime')
, fs = require('fs')
- , crypto = require('crypto');
+ , crypto = require('crypto')
+ , xml2js = require('xml2js')
+ , qs = require('querystring');
// The max for multi-object delete, bucket listings, etc.
var BUCKET_OPS_MAX = 1000;
@@ -478,6 +480,105 @@ Client.prototype.deleteMultiple = function(filenames, headers, fn){
};
/**
+ * Possible params for Client#list.
+ *
+ * @type {Object}
+ */
+
+var LIST_PARAMS = {
+ delimiter: true
+ , marker: true
+ ,'max-keys': true
+ , prefix: true
+};
+
+/**
+ * Normalization map for Client#list.
+ *
+ * @type {Object}
+ */
+
+var RESPONSE_NORMALIZATION = {
+ MaxKeys: Number,
+ IsTruncated: Boolean,
+ LastModified: Date,
+ Size: Number
+};
+
+/**
+ * Convert data we get from S3 xml in Client#list, since every primitive
+ * value there is a string.
+ *
+ * @type {Object}
+ */
+
+function normalizeResponse(data) {
+ for (var key in data) {
+ var Constr = RESPONSE_NORMALIZATION[key];
+
+ if (Constr) {
+ if (Constr === Date) {
+ data[key] = new Date(data[key]);
+ } else {
+ data[key] = Constr(data[key]);
+ }
+ } else if (Array.isArray(data[key])) {
+ data[key].forEach(normalizeResponse);
+ }
+ }
+}
+
+/**
+ * List up to 1000 objects at a time, with optional `headers`, `params`
+ * and callback `fn` with a possible exception and the response.
+ *
+ * @param {Object|Function} params
+ * @param {Object|Function} headers
+ * @param {Function} fn
+ * @api public
+ */
+
+Client.prototype.list = function(params, headers, fn){
+ if ('function' == typeof headers) {
+ fn = headers;
+ headers = {};
+ }
+
+ if ('function' == typeof params) {
+ fn = params;
+ params = null;
+ }
+
+ if (params && !LIST_PARAMS[Object.keys(params)[0]]) {
+ headers = params;
+ params = null;
+ }
+
+ var url = params ? '?' + qs.stringify(params) : '';
+
+ this.getFile(url, headers, function(err, res){
+ if (err) return fn(err);
+
+ var xmlStr = '';
+
+ res.on('data', function(chunk){
+ xmlStr += chunk;
+ });
+
+ res.on('end', function(){
+ new xml2js.Parser({explicitArray: false, explicitRoot: false})
+ .parseString(xmlStr, function(err, data){
+ if (err) return fn(err);
+
+ delete data.$;
+ normalizeResponse(data);
+ fn(null, data);
+ });
+ }).on('error', fn);
+ });
+};
+
+/**
* Return a url to the given `filename`.
*
* @param {String} filename
View
6 package.json
@@ -6,7 +6,8 @@
"author": "TJ Holowaychuk <tj@learnboost.com>",
"contributors": [
"TJ Holowaychuk <tj@learnboost.com>",
- "Domenic Denicola <domenic@domenicdenicola.com>"
+ "Domenic Denicola <domenic@domenicdenicola.com>",
+ "Oleg Slobodksoi <oleg008@gmail.com>"
],
"license": "MIT",
"main": "./lib/index.js",
@@ -18,7 +19,8 @@
"url": "https://github.com/LearnBoost/knox/issues"
},
"dependencies": {
- "mime": "*"
+ "mime": "*",
+ "xml2js": "0.2.x"
},
"devDependencies": {
"mocha": "*"
View
25 test/knox.test.js
@@ -334,6 +334,31 @@ module.exports = {
});
},
+ 'test .list()': function(done){
+ var files = ['/list/user1.json', '/list/user2.json'];
+
+ client.putFile(jsonFixture, files[0], function(err, res){
+ client.putFile(jsonFixture, files[1], function(err, res){
+ client.list({prefix: 'list'}, function(err, data){
+ assert.ok(!err);
+ assert.equal(data.Prefix, 'list');
+ assert.strictEqual(data.IsTruncated, true);
+ assert.strictEqual(data.MaxKeys, 1000);
+ assert.equal(data.Contents.length, 2);
+ assert.ok(data.Contents[0].LastModified instanceof Date);
+ assert.equal(typeof data.Contents[0].Size, 'number');
+ assert.deepEqual(
+ Object.keys(data.Contents[0]),
+ ['Key', 'LastModified', 'ETag', 'Size', 'Owner', 'StorageClass']
+ );
+ client.deleteMultiple(files, function(err, res) {
@kof
kof added a note

it will never get to this callback, err == null, res.statusCode == 200 ....

do you have the same issue?

@domenic
domenic added a note

No, it's not hanging at all for me... test passes perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ done();
+ });
+ });
+ });
+ });
+ },
+
'test /?delete': function (done) {
var xml = ['<?xml version="1.0" encoding="UTF-8"?>\n','<Delete>'];
xml.push('<Object><Key>/test/user4.json</Key></Object>');
Something went wrong with that request. Please try again.