Permalink
Browse files

Improve prefixed container identification

Previously, if 'test' was the prefix, a container named 'tester0' would
be seen as belonging to the prefix group because it starts with 'test'.
This identification has been improved to only select those containers
named <prefix><int>.
  • Loading branch information...
1 parent 2709dc0 commit 78d5705f191815a5173bd532a4d69dc37f8c3a63 @rossj committed Nov 5, 2012
Showing with 136 additions and 54 deletions.
  1. +42 −34 lib/rackit.js
  2. +94 −20 test/rackit.test.js
View
@@ -373,7 +373,39 @@ Rackit.prototype._createContainer = function (sName, cb) {
}
cb();
- }], cb);
+ }],
+ cb);
+};
+
+/**
+ * This method searches through the container cache for those that match the prefix
+ * @return an array containing container names that match the prefix, in sorted order
+ * @private
+ */
+Rackit.prototype._getPrefixedContainers = function() {
+ var o1 = this;
+
+ var reg = new RegExp('^' + o1.options.prefix + '\\d+$');
+ var asContainers = [];
+ var idx = o1.aContainers.length;
+
+ while (idx--) {
+ var sContainer = o1.aContainers[idx].name;
+
+ // Check that the container name matches
+ if (sContainer.match(reg))
+ asContainers.push(sContainer);
+ }
+
+ // Sort the container array by numerical index
+ var reg = /\d+$/;
+ asContainers.sort(function(a, b) {
+ a = parseInt(a.match(reg)[0]);
+ b = parseInt(b.match(reg)[0]);
+ return a-b;
+ });
+
+ return asContainers;
};
/**
@@ -385,40 +417,25 @@ Rackit.prototype._createContainer = function (sName, cb) {
Rackit.prototype._getContainer = function (cb) {
var o1 = this;
- // Search through the containers we have for the highest numbered, prefixed container
- // TODO: Cache the highest numbered container
- var idx = o1.aContainers.length;
- var container;
- var highestContainerNum = -1;
- var containerNum;
- while (idx--) {
- container = o1.aContainers[idx];
- // Check that the container name matches
- if (container.name.lastIndexOf(o1.options.prefix, 0) === 0) {
- // The prefix matches, get the number
- containerNum = container.name.substring(o1.options.prefix.length);
- if (!isNaN(containerNum)) {
- highestContainerNum = Math.max(highestContainerNum, containerNum);
- }
- }
- }
+ var asContainers = o1._getPrefixedContainers();
// Check that we found a prefixed container
var name;
- if (highestContainerNum < 0) {
+ if (!asContainers.length) {
+ // If no existing containers, create one!
name = o1.options.prefix + '0';
return o1._createContainer(name, function (err) {
cb(err, name);
});
}
- // We have found a prefixed container.. get it
- name = o1.options.prefix + highestContainerNum;
- container = o1.hContainers[name];
+ // We have containers. Get the most recent one.
+ name = asContainers.pop();
// Check if the container is full
- if (container.count >= 50000) {
- name = o1.options.prefix + (highestContainerNum + 1);
+ if (o1.hContainers[name].count >= 50000) {
+ // The container is full, create the next one
+ name = o1.options.prefix + (parseInt(name.match(/\d+$/)[0]) + 1);
return o1._createContainer(name, function (err) {
cb(err, name);
});
@@ -578,16 +595,7 @@ Rackit.prototype.list = function (cb) {
var o1 = this;
var aCloudpaths = [];
- var asContainers = [];
-
- var idx = o1.aContainers.length;
- while (idx--) {
- var container = o1.aContainers[idx];
- // Check that the container name matches
- if (container.name.lastIndexOf(o1.options.prefix, 0) === 0) {
- asContainers.push(container.name);
- }
- }
+ var asContainers = o1._getPrefixedContainers();
// List the objects for each container in parallel
async.forEach(
View
@@ -146,6 +146,11 @@ var superNock = {
content_type : 'application\/octet-stream',
last_modified : '2012-1-01T00:00:0.0'
}]
+ },
+ {
+ name : 'multiplemultiple0',
+ count : 0,
+ bytes : 2000
}
],
@@ -178,6 +183,24 @@ var superNock = {
cdn_streaming_uri : 'https://c2.r2.stream.cf1.rackcdn.com'
}
],
+ // Gets an array of containers matching a prefix
+ getPrefixedContainers : function (prefix) {
+ var container, containers = [];
+ var i = this.aContainers.length;
+ var reg = new RegExp('^' + prefix + '\\d+$');
+
+ while (i--) {
+ container = this.aContainers[i];
+
+ // If the container doesn't have the prefix, skip it
+ if (!container.name.match(reg))
+ continue;
+
+ containers.push(container);
+ }
+
+ return containers;
+ },
CDN : function () {
var path = url.parse(mockOptions.cdn).pathname + '?format=json';
var scope = nock(mockOptions.cdn)
@@ -234,12 +257,14 @@ var superNock = {
return this;
},
list : function (prefix, limit) {
- var container, basepath, path, i, count, j, objects
+ var containers = this.getPrefixedContainers(prefix);
+ var i = containers.length;
+ var container, basepath, path, count, j, objects;
var scope = nock(mockOptions.storage);
// There may be more than one container with this prefix, and the client will be requesting from all
- for (i = 0; i < this.aContainers.length; i++) {
- container = this.aContainers[i];
+ while (i--) {
+ container = containers[i];
// Skip containers that don't have the given prefix
if (container.name.indexOf(prefix) !== 0)
@@ -287,6 +312,7 @@ var superNock = {
};
describe('Rackit', function () {
+
describe('Constructor', function () {
it('should have default options', function () {
var rackit = new Rackit();
@@ -398,6 +424,67 @@ describe('Rackit', function () {
});
+ describe('#_getPrefixedContainers', function() {
+ var rackit;
+
+ // Start off with a new, initialized rackit
+ before(function (cb) {
+ superNock.typicalResponse();
+ rackit = new Rackit({
+ user : rackitOptions.user,
+ key : rackitOptions.key
+ });
+ rackit.init(function (err) {
+ superNock.allDone();
+ cb(err);
+ });
+ });
+
+ it('should return an empty array if no prefixed containers have been made', function() {
+ // Hack some data into Rackit
+ rackit.options.prefix = 'nonexistent';
+ rackit.aContainers = [{
+ name: 'existent'
+ }];
+
+ rackit._getPrefixedContainers().should.have.length(0);
+ });
+
+ it('should return a sorted array of prefixed containers', function() {
+ // Hack some data into Rackit
+ rackit.options.prefix = 'existent';
+ rackit.aContainers = [{
+ name: 'blah0'
+ }, {
+ name: 'existent2'
+ }, {
+ name: 'existent3'
+ }, {
+ name: 'existent0'
+ }];
+
+ rackit._getPrefixedContainers().should.eql(['existent0', 'existent2', 'existent3']);
+ });
+
+ it('should not include containers with a matching sub-prefix', function() {
+ // Hack some data into Rackit
+ rackit.options.prefix = 'existent';
+ rackit.aContainers = [{
+ name: 'blah0'
+ }, {
+ name: 'existent2'
+ }, {
+ name: 'existent3'
+ }, {
+ name: 'existent0'
+ }, {
+ name: 'existenter0'
+ }];
+
+ rackit._getPrefixedContainers().should.eql(['existent0', 'existent2', 'existent3']);
+ });
+ });
+
describe('#add', function () {
var rackit;
@@ -905,20 +992,7 @@ describe('Rackit', function () {
});
});
- // Gets an array of containers for a prefix
- function getContainers (prefix) {
- var i, containers = [];
-
- for (i = 0; i < superNock.aContainers.length; i++) {
- // If the container doesn't have the prefix, skip it
- if (superNock.aContainers[i].name.indexOf(prefix) !== 0)
- continue;
-
- containers.push(superNock.aContainers[i]);
- }
- return containers;
- }
// Gets all of the object cloudpaths belonging to the given containers. This function gets the objects
// from the mock (the "actual" data store) for validation of what Rackit gives
@@ -973,7 +1047,7 @@ describe('Rackit', function () {
var prefix = 'empty';
// Assert the test conditions (no files)
- getObjects(getContainers(prefix)).length.should.equal(0);
+ getObjects(superNock.getPrefixedContainers(prefix)).length.should.equal(0);
assertList(prefix, 10000, [], cb);
});
@@ -982,7 +1056,7 @@ describe('Rackit', function () {
var prefix = 'single';
// Assert the test conditions (one container)
- var containers = getContainers(prefix);
+ var containers = superNock.getPrefixedContainers(prefix);
containers.length.should.equal(1);
// Find the actual container objects to validate
@@ -999,7 +1073,7 @@ describe('Rackit', function () {
var prefix = 'single';
// Assert the test conditions (one container)
- var containers = getContainers(prefix);
+ var containers = superNock.getPrefixedContainers(prefix);
containers.length.should.equal(1);
// Find the actual container objects to validate
@@ -1017,7 +1091,7 @@ describe('Rackit', function () {
var listLimit = 1;
// Assert the test conditions (multiple containers, above limit)
- var containers = getContainers(prefix);
+ var containers = superNock.getPrefixedContainers(prefix);
containers.length.should.be.above(1);
// Assert that one of the containers (first one) is above the list limit

0 comments on commit 78d5705

Please sign in to comment.