fix: remove checksum, add tests (#58)

screwdriver-cd · Oct 8, 2018 · 6e4f747 · 6e4f747
1 parent c4966b9
commit 6e4f747
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 165 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);