Handle immutable configuration (#33)

* Handle immutable configuration

The popular and awesome https://github.com/lorenwest/node-config
makes all configs values immutable.  Good practice!

parse-server-s3-adapter attempts to edit config values.

This fix will respect immutability allowing this adapter
to work with node-config.

* PR comments....

1. move setting NODE_ENV to test command instead of helpers.
2. add a missing ;.  lint coming.....

Author: Arthur Cinader

lib/optionsFromArguments.js

@@ -50,15 +50,16 @@ const optionsFromArguments = function optionsFromArguments(args) {
     }
   } else {
     if (args.length == 1) {
-      options = stringOrOptions || {};
+      Object.assign(options, stringOrOptions);
     } else if (args.length == 2) {
-      options = stringOrOptions;
+      Object.assign(options, stringOrOptions);
       s3overrides = args[1];
       options.bucket = s3overrides.params.Bucket;
-    } else {
+    } else if (args.length > 2) {
       throw new Error('Failed to configure S3Adapter. Arguments don\'t make sense');
     }
   }
+
   options = requiredOrFromEnvironment(options, 'bucket', 'S3_BUCKET');
   options = fromEnvironmentOrDefault(options, 'accessKey', 'S3_ACCESS_KEY', null);
   options = fromEnvironmentOrDefault(options, 'secretKey', 'S3_SECRET_KEY', null);

package.json

@@ -4,7 +4,7 @@
   "description": "AWS S3 adapter for parse-server",
   "main": "index.js",
   "scripts": {
-    "test": "istanbul cover -x **/spec/** jasmine --captureExceptions"
+    "test": "NODE_ENV=test NODE_CONFIG_DIR=./spec/config istanbul cover -x **/spec/** jasmine --captureExceptions"
   },
   "repository": {
     "type": "git",
@@ -26,6 +26,7 @@
   },
   "devDependencies": {
     "codecov": "^1.0.1",
+    "config": "^1.24.0",
     "istanbul": "^0.4.2",
     "jasmine": "^2.4.1",
     "parse-server-conformance-tests": "^1.0.0"

spec/config/test.js

@@ -0,0 +1,18 @@
+module.exports = {
+  accessKey: 'accessKey',
+  secretKey: 'secretKey',
+  insufficientOptions: {
+    accessKey: 'accessKey',
+    secretKey: 'secretKey',
+  },
+  bucket: 'bucket',
+  objectWithBucket: {
+      bucket: 'bucket',
+  },
+  emptyObject: {},
+  paramsObjectWBucket: {
+      params: {
+          Bucket: 'bucket',
+      },
+  },
+};

spec/support/jasmine.json

@@ -1,6 +1,9 @@
 {
   "spec_dir": "spec",
   "spec_files": [
-    "test.spec.js"
-  ]
+    "**/*[sS]pec.js"
+  ],
+  "helpers": [
+        "helpers/**/*.js"
+    ]
 }

spec/test.spec.js

@@ -1,15 +1,20 @@
 'use strict';
-let filesAdapterTests = require('parse-server-conformance-tests').files;
 
-let S3Adapter = require('../index.js');
-let optionsFromArguments = require('../lib/optionsFromArguments');
+const config = require('config');
+const filesAdapterTests = require('parse-server-conformance-tests').files;
+const S3Adapter = require('../index.js');
+const optionsFromArguments = require('../lib/optionsFromArguments');
 
-describe('S3Adapter tests', () => {
+describe('S3Adapter tests', () => {
+  beforeEach(() => {
+    delete process.env.S3_BUCKET;
+    delete process.env.S3_REGION;
+  });
 
   it('should throw when not initialized properly', () => {
     expect(() => {
       var s3 = new S3Adapter();
-    }).toThrow(new Error('Failed to configure S3Adapter. Arguments don\'t make sense'));
+    }).toThrow("S3Adapter requires option 'bucket' or env. variable S3_BUCKET");
 
     expect(() =>  {
       var s3 = new S3Adapter('accessKey', 'secretKey', {});
@@ -34,6 +39,57 @@ describe('S3Adapter tests', () => {
     }).not.toThrow()
   });
 
+  it('should accept environment for required', () => {
+    const TEST_BUCKET = 'testBucket';
+    process.env.S3_BUCKET = TEST_BUCKET;
+    const s3 = new S3Adapter();
+    expect(s3._bucket).toBe(TEST_BUCKET);
+  });
+
+  describe('configured with immutable values', () => {
+    describe('not initialized properly', () => {
+      it('should fail with two string arguments', () => {
+        expect(() => {
+          var s3 = new S3Adapter(config.get('accessKey'), config.get('secretKey'), {});
+        }).toThrow(new Error('Failed to configure S3Adapter. Arguments don\'t make sense'));
+      });
+
+      it('should fail when passed an object without a bucket', () => {
+        expect(() => {
+          var s3 = new S3Adapter(config.get('insufficientOptions'));
+        }).toThrow("S3Adapter requires option 'bucket' or env. variable S3_BUCKET")
+      });
+    });
+
+
+    describe('should not throw when initialized properly', () => {
+      it('should accept a string bucket', () => {
+        expect(() => {
+          var s3 = new S3Adapter(config.get('bucket'));
+        }).not.toThrow()
+      });
+
+      it('should accept an object with a bucket', () => {
+        expect(() =>  {
+          var s3 = new S3Adapter(config.get('objectWithBucket'));
+        }).not.toThrow()
+      });
+
+      it('should accept a second argument of object with a params object with a bucket', () => {
+        expect(() => {
+          var s3 = new S3Adapter(config.get('emptyObject'), config.get('paramsObjectWBucket'));
+        }).not.toThrow()
+      });
+
+      it('should accept environment over default', () => {
+        const TEST_REGION = 'test';
+        process.env.S3_REGION = TEST_REGION;
+        const s3 = new S3Adapter(config.get('bucket'));
+        expect(s3._region).toBe(TEST_REGION);
+      });
+    });
+  });
+
   describe('to find the right arg in the right place', () => {
     it('should accept just bucket as first string arg', () => {
       var args = ['bucket'];