From 6e4f747df3462bcd64afb7822da2e639c4ef654e Mon Sep 17 00:00:00 2001 From: Dao Lam Date: Mon, 8 Oct 2018 15:23:33 -0700 Subject: [PATCH] fix: remove checksum, add tests (#58) --- helpers/aws.js | 29 ---------- plugins/caches.js | 17 +----- test/helpers/aws.test.js | 41 -------------- test/plugins/caches.test.js | 103 +++++++++--------------------------- 4 files changed, 25 insertions(+), 165 deletions(-) diff --git a/helpers/aws.js b/helpers/aws.js index 919e1ee..f4597f0 100644 --- a/helpers/aws.js +++ b/helpers/aws.js @@ -1,7 +1,6 @@ 'use strict'; const AWS = require('aws-sdk'); -const crypto = require('crypto'); class AwsClient { /** @@ -59,34 +58,6 @@ class AwsClient { }); }); } - - /** - * Compare the checksum of local cache with the one in S3 - * @method compareChecksum - * @param {String} localCache content of localFile - * @param {String} cacheKey cache key - * @param {Function} callback callback function - */ - compareChecksum(localCache, cacheKey, callback) { - const params = { - Bucket: this.bucket, - Key: cacheKey - }; - const localmd5 = crypto.createHash('md5').update(localCache).digest('hex'); - - return this.client.headObject(params, (err, data) => { - if (err) { - return callback(err); - } - const remotemd5 = data.Metadata.md5; - - if (localmd5 !== remotemd5) { - return callback(null, false); - } - - return callback(null, true); - }); - } } module.exports = AwsClient; diff --git a/plugins/caches.js b/plugins/caches.js index 3664350..a12241e 100644 --- a/plugins/caches.js +++ b/plugins/caches.js @@ -143,22 +143,7 @@ exports.plugin = { + `headers ${JSON.stringify(contents.h)}`); try { - if (!awsClient) { - await cache.set(cacheKey, value, 0); - } else { - await awsClient.compareChecksum(value.c, - `caches/${cacheKey}`, async (err, areEqual) => { - if (err) { - console.log('Failed to compare checksums: ', err); - } - - if (!areEqual) { - await cache.set(cacheKey, value, 0); - } else { - console.log('Cache has not changed, not setting cache.'); - } - }); - } + await cache.set(cacheKey, value, 0); } catch (err) { request.log([cacheName, 'error'], `Failed to store in cache: ${err}`); diff --git a/test/helpers/aws.test.js b/test/helpers/aws.test.js index 7965c33..7553c29 100644 --- a/test/helpers/aws.test.js +++ b/test/helpers/aws.test.js @@ -105,45 +105,4 @@ describe('aws helper test', () => { done(); }); }); - - it('returns true if checksum are the same', (done) => { - const headParam = { - Bucket: testBucket, - Key: cacheKey - }; - - return awsClient.compareChecksum(localCache, cacheKey, (err, checksum) => { - assert.calledWith(clientMock.prototype.headObject, headParam); - assert.isNull(err); - assert.isTrue(checksum); - done(); - }); - }); - - it('returns false if checksum are not the same', (done) => { - const headParam = { - Bucket: testBucket, - Key: cacheKey - }; - - const newLocalCache = 'A DIFFERENT FILE'; - - return awsClient.compareChecksum(newLocalCache, cacheKey, (err, checksum) => { - assert.calledWith(clientMock.prototype.headObject, headParam); - assert.isNull(err); - assert.isFalse(checksum); - done(); - }); - }); - - it('returns err if fails to get headObject', (done) => { - const err = new Error('failed to get headObject'); - - clientMock.prototype.headObject = sinon.stub().yieldsAsync(err); - - return awsClient.compareChecksum(localCache, cacheKey, (e) => { - assert.deepEqual(e, err); - done(); - }); - }); }); diff --git a/test/plugins/caches.test.js b/test/plugins/caches.test.js index dfd32b3..326ba8f 100644 --- a/test/plugins/caches.test.js +++ b/test/plugins/caches.test.js @@ -30,8 +30,7 @@ describe('events plugin test', () => { }; awsClientMock = sinon.stub().returns({ - updateLastModified: sinon.stub().yields(null), - compareChecksum: sinon.stub().yields(null, false) + updateLastModified: sinon.stub().yields(null) }); mockery.registerMock('../helpers/aws', awsClientMock); @@ -95,6 +94,21 @@ describe('events plugin test', () => { }) )); + it('returns 403 if credentials is not valid', () => ( + server.inject({ + headers: { + 'x-foo': 'bar' + }, + credentials: { + eventId: 5555, + scope: ['build'] + }, + url: `/caches/events/${mockEventID}/foo` + }).then((response) => { + assert.equal(response.statusCode, 403); + }) + )); + describe('caching is not setup right', () => { let badServer; @@ -251,83 +265,6 @@ describe('events plugin test', () => { assert.equal(getResponse.result, 'THIS IS A TEST'); }); }); - - describe('local file and remote file are the same', () => { - let cacheConfigMock; - let cacheAwsClientMock; - let cachePlugin; - let cacheServer; - - beforeEach(() => { - mockery.deregisterAll(); - mockery.resetCache(); - - cacheConfigMock = { - get: sinon.stub().returns({ - plugin: 's3' - }) - }; - - cacheAwsClientMock = sinon.stub().returns({ - updateLastModified: sinon.stub().yields(null), - compareChecksum: sinon.stub().yields(null, true) - }); - - mockery.registerMock('../helpers/aws', cacheAwsClientMock); - mockery.registerMock('config', cacheConfigMock); - - // eslint-disable-next-line global-require - cachePlugin = require('../../plugins/caches'); - - cacheServer = Hapi.server({ - cache: { - engine: catmemory, - maxByteSize: 512, - allowMixedContent: true - }, - port: 1235 - }); - - cacheServer.auth.scheme('custom', () => ({ - authenticate: (request, h) => h.authenticated() - })); - cacheServer.auth.strategy('token', 'custom'); - cacheServer.auth.strategy('session', 'custom'); - - return cacheServer.register({ - plugin: cachePlugin, - options: { - expiresInSec: '100', - maxByteSize: '5368709120' - } }) - .then(() => cacheServer.start()); - }); - - afterEach(() => { - cacheServer = null; - mockery.deregisterAll(); - mockery.resetCache(); - }); - - it('does not save cache if checksums are equal', async () => { - options.url = `/caches/events/${mockEventID}/foo`; - - options.headers['content-type'] = 'application/x-ndjson'; - const putResponse = await cacheServer.inject(options); - - assert.equal(putResponse.statusCode, 202); - - return cacheServer.inject({ - url: `/caches/events/${mockEventID}/foo`, - credentials: { - eventId: mockEventID, - scope: ['build'] - } - }).then((getResponse) => { - assert.equal(getResponse.statusCode, 404); - }); - }); - }); }); describe('DELETE /caches/events/:id/:cacheName', () => { @@ -383,6 +320,14 @@ describe('events plugin test', () => { }); })); + it('returns 403 if credentials is not valid', () => { + deleteOptions.credentials.eventId = 5555; + + return server.inject(deleteOptions).then((response) => { + assert.equal(response.statusCode, 403); + }); + }); + it('deletes an event cache', () => server.inject(putOptions).then((postResponse) => { assert.equal(postResponse.statusCode, 202);