From b89bef6e892367318a39aa1285a5aea702f8622b Mon Sep 17 00:00:00 2001 From: Emily Date: Thu, 9 Aug 2018 14:49:52 -0700 Subject: [PATCH] refactor to single bucket --- app/templates/file/index.js | 2 +- server/config.js | 21 ++++++++++------ server/storage/fs.js | 2 +- server/storage/index.js | 46 +++++++++++++++-------------------- server/storage/s3.js | 4 +-- test/backend/s3-tests.js | 20 +++++++-------- test/backend/storage-tests.js | 42 ++++++++++++++++---------------- 7 files changed, 68 insertions(+), 69 deletions(-) diff --git a/app/templates/file/index.js b/app/templates/file/index.js index 2e9a69ca..dcf47510 100644 --- a/app/templates/file/index.js +++ b/app/templates/file/index.js @@ -30,7 +30,7 @@ module.exports = function(file, state) { ${bytes(file.size)} · ${state.translate('downloadCount', { num: `${number(totalDownloads)} / ${number(downloadLimit)}` - })} + })} · ${remainingTime}

diff --git a/server/config.js b/server/config.js index 6d037721..73985155 100644 --- a/server/config.js +++ b/server/config.js @@ -4,19 +4,24 @@ const path = require('path'); const { randomBytes } = require('crypto'); const conf = convict({ - s3_buckets: { - format: Array, - default: [], - env: 'S3_BUCKETS' + s3_bucket: { + format: String, + default: '', + env: 'S3_BUCKET' }, - num_of_buckets: { + num_of_prefixes: { format: Number, - default: 3, - env: 'NUM_OF_BUCKETS' + default: 5, + env: 'NUM_OF_PREFIXES' + }, + expire_prefixes: { + format: Array, + default: ['5minutes', '1hour', '1day', '1week', '2weeks'], + env: 'EXPIRE_PREFIXES' }, expire_times_seconds: { format: Array, - default: [86400, 604800, 1209600], + default: [300, 3600, 86400, 604800, 1209600], env: 'EXPIRE_TIMES_SECONDS' }, default_expire_seconds: { diff --git a/server/storage/fs.js b/server/storage/fs.js index 69e92bd9..aa6da744 100644 --- a/server/storage/fs.js +++ b/server/storage/fs.js @@ -6,7 +6,7 @@ const mkdirp = require('mkdirp'); const stat = promisify(fs.stat); class FSStorage { - constructor(config, index, log) { + constructor(config, log) { this.log = log; this.dir = config.file_dir; mkdirp.sync(this.dir); diff --git a/server/storage/index.js b/server/storage/index.js index 479aad4c..d3c6c866 100644 --- a/server/storage/index.js +++ b/server/storage/index.js @@ -5,15 +5,10 @@ const createRedisClient = require('./redis'); class DB { constructor(config) { - const Storage = - config.s3_buckets.length > 0 ? require('./s3') : require('./fs'); + const Storage = config.s3_bucket ? require('./s3') : require('./fs'); this.log = mozlog('send.storage'); - this.storage = []; - - for (let i = 0; i < config.num_of_buckets; i++) { - this.storage.push(new Storage(config, i, this.log)); - } + this.storage = new Storage(config, this.log); this.redis = createRedisClient(config); this.redis.on('error', err => { @@ -26,32 +21,33 @@ class DB { return Math.ceil(result) * 1000; } - async getBucket(id) { - return this.redis.hgetAsync(id, 'bucket'); + async getPrefixedId(id) { + const prefix = await this.redis.hgetAsync(id, 'prefix'); + return `${prefix}-${id}`; } async length(id) { - const bucket = await this.redis.hgetAsync(id, 'bucket'); - return this.storage[bucket].length(id); + const filePath = await this.getPrefixedId(id); + return this.storage.length(filePath); } async get(id) { - const bucket = await this.redis.hgetAsync(id, 'bucket'); - return this.storage[bucket].getStream(id); + const filePath = await this.getPrefixedId(id); + return this.storage.getStream(filePath); } async set(id, file, meta, expireSeconds = config.default_expire_seconds) { - const bucketTimes = config.expire_times_seconds; - let bucket = 0; - while (bucket < config.num_of_buckets - 1) { - if (expireSeconds <= bucketTimes[bucket]) { + const expireTimes = config.expire_times_seconds; + let i; + for (i = 0; i < expireTimes.length - 1; i++) { + if (expireSeconds <= expireTimes[i]) { break; } - bucket++; } - - await this.storage[bucket].set(id, file); - this.redis.hset(id, 'bucket', bucket); + const prefix = config.expire_prefixes[i]; + const filePath = `${prefix}-${id}`; + await this.storage.set(filePath, file); + this.redis.hset(id, 'prefix', prefix); this.redis.hmset(id, meta); this.redis.expire(id, expireSeconds); } @@ -61,16 +57,14 @@ class DB { } async del(id) { - const bucket = await this.redis.hgetAsync(id, 'bucket'); + const filePath = await this.getPrefixedId(id); + this.storage.del(filePath); this.redis.del(id); - this.storage[bucket].del(id); } async ping() { await this.redis.pingAsync(); - for (const bucket of this.storage) { - bucket.ping(); - } + await this.storage.ping(); } async metadata(id) { diff --git a/server/storage/s3.js b/server/storage/s3.js index 08d6133f..bb2b0100 100644 --- a/server/storage/s3.js +++ b/server/storage/s3.js @@ -2,8 +2,8 @@ const AWS = require('aws-sdk'); const s3 = new AWS.S3(); class S3Storage { - constructor(config, index, log) { - this.bucket = config.s3_buckets[index]; + constructor(config, log) { + this.bucket = config.s3_bucket; this.log = log; } diff --git a/test/backend/s3-tests.js b/test/backend/s3-tests.js index d4afd8b3..997b7c34 100644 --- a/test/backend/s3-tests.js +++ b/test/backend/s3-tests.js @@ -32,8 +32,8 @@ const S3Storage = proxyquire('../../server/storage/s3', { }); describe('S3Storage', function() { - it('uses config.s3_buckets', function() { - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + it('uses config.s3_bucket', function() { + const s = new S3Storage({ s3_bucket: 'foo' }); assert.equal(s.bucket, 'foo'); }); @@ -42,7 +42,7 @@ describe('S3Storage', function() { s3Stub.headObject = sinon .stub() .returns(resolvedPromise({ ContentLength: 123 })); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); const len = await s.length('x'); assert.equal(len, 123); sinon.assert.calledWithMatch(s3Stub.headObject, { @@ -54,7 +54,7 @@ describe('S3Storage', function() { it('throws when id not found', async function() { const err = new Error(); s3Stub.headObject = sinon.stub().returns(rejectedPromise(err)); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); try { await s.length('x'); assert.fail(); @@ -70,7 +70,7 @@ describe('S3Storage', function() { s3Stub.getObject = sinon .stub() .returns({ createReadStream: () => stream }); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); const result = s.getStream('x'); assert.equal(result, stream); sinon.assert.calledWithMatch(s3Stub.getObject, { @@ -84,7 +84,7 @@ describe('S3Storage', function() { it('calls s3.upload', async function() { const file = { on: sinon.stub() }; s3Stub.upload = sinon.stub().returns(resolvedPromise()); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); await s.set('x', file); sinon.assert.calledWithMatch(s3Stub.upload, { Bucket: 'foo', @@ -103,7 +103,7 @@ describe('S3Storage', function() { promise: () => Promise.reject(err), abort }); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); try { await s.set('x', file); assert.fail(); @@ -119,7 +119,7 @@ describe('S3Storage', function() { }; const err = new Error(); s3Stub.upload = sinon.stub().returns(rejectedPromise(err)); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); try { await s.set('x', file); assert.fail(); @@ -132,7 +132,7 @@ describe('S3Storage', function() { describe('del', function() { it('calls s3.deleteObject', async function() { s3Stub.deleteObject = sinon.stub().returns(resolvedPromise(true)); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); const result = await s.del('x'); assert.equal(result, true); sinon.assert.calledWithMatch(s3Stub.deleteObject, { @@ -145,7 +145,7 @@ describe('S3Storage', function() { describe('ping', function() { it('calls s3.headBucket', async function() { s3Stub.headBucket = sinon.stub().returns(resolvedPromise(true)); - const s = new S3Storage({ s3_buckets: ['foo', 'bar', 'baz'] }, 0); + const s = new S3Storage({ s3_bucket: 'foo' }); const result = await s.ping(); assert.equal(result, true); sinon.assert.calledWithMatch(s3Stub.headBucket, { Bucket: 'foo' }); diff --git a/test/backend/storage-tests.js b/test/backend/storage-tests.js index e92d7a34..d7bfbb76 100644 --- a/test/backend/storage-tests.js +++ b/test/backend/storage-tests.js @@ -21,10 +21,11 @@ class MockStorage { } const config = { - default_expire_seconds: 10, - num_of_buckets: 3, - expire_times_seconds: [86400, 604800, 1209600], - s3_buckets: ['foo', 'bar', 'baz'], + s3_bucket: 'foo', + default_expire_seconds: 20, + num_of_prefixes: 3, + expire_prefixes: ['ten', 'twenty', 'thirty'], + expire_times_seconds: [10, 20, 30], env: 'development', redis_host: 'localhost' }; @@ -48,7 +49,6 @@ describe('Storage', function() { describe('length', function() { it('returns the file size', async function() { - await storage.set('x', null); const len = await storage.length('x'); assert.equal(len, 12); }); @@ -56,7 +56,6 @@ describe('Storage', function() { describe('get', function() { it('returns a stream', async function() { - await storage.set('x', null); const s = await storage.get('x'); assert.equal(s, stream); }); @@ -71,30 +70,31 @@ describe('Storage', function() { assert.equal(Math.ceil(s), seconds); }); - it('puts into right bucket based on expire time', async function() { - for (let i = 0; i < config.num_of_buckets; i++) { - await storage.set( - 'x', - null, - { foo: 'bar' }, - config.expire_times_seconds[i] - ); - const bucket = await storage.getBucket('x'); - assert.equal(bucket, i); - await storage.del('x'); - } + it('adds right prefix based on expire time', async function() { + await storage.set('x', null, { foo: 'bar' }, 10); + const path_x = await storage.getPrefixedId('x'); + assert.equal(path_x, 'ten-x'); + await storage.del('x'); + + await storage.set('y', null, { foo: 'bar' }, 11); + const path_y = await storage.getPrefixedId('y'); + assert.equal(path_y, 'twenty-y'); + await storage.del('y'); + + await storage.set('z', null, { foo: 'bar' }, 33); + const path_z = await storage.getPrefixedId('z'); + assert.equal(path_z, 'thirty-z'); + await storage.del('z'); }); it('sets metadata', async function() { const m = { foo: 'bar' }; await storage.set('x', null, m); const meta = await storage.redis.hgetallAsync('x'); - delete meta.bucket; + delete meta.prefix; await storage.del('x'); assert.deepEqual(meta, m); }); - - //it('throws when storage fails'); }); describe('setField', function() {