Merge pull request #151 from mapbox/not-always-s3

Don't always put data on S3
mapbox · Sep 18, 2015 · d9e8522 · d9e8522
2 parents 4c49142 + 035c90b
commit d9e8522
Show file tree

Hide file tree

Showing 15 changed files with 237 additions and 191 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -0,0 +1,16 @@
+{
+    "rules": {
+        "indent": [2, 4],
+        "quotes": [2, "single"],
+        "no-console": [0]
+    },
+    "env": {
+        "node": true
+    },
+    "globals": {
+        "process": true,
+        "module": true,
+        "require": true
+    },
+    "extends": "eslint:recommended"
+}
diff --git a/index.js b/index.js
@@ -4,14 +4,14 @@ var uniq = require('uniq');
 var queue = require('queue-async');
 var Dyno = require('dyno');
 var AWS = require('aws-sdk');
-var extent = require('geojson-extent');
-var cuid = require('cuid');
 var tilebelt = require('tilebelt');
 var geobuf = require('geobuf');
 var stream = require('stream');
 
 var MAX_GEOMETRY_SIZE = 1024 * 10;  // 10KB
 
+module.exports = Cardboard;
+
 /**
  * Cardboard client generator
  * @param {object} config - a configuration object
@@ -36,7 +36,7 @@ var MAX_GEOMETRY_SIZE = 1024 * 10;  // 10KB
  *   prefix: 'my-cardboard-prefix'
  * });
  */
-var Cardboard = module.exports = function(config) {
+function Cardboard(config) {
     config = config || {};
     config.MAX_GEOMETRY_SIZE = config.MAX_GEOMETRY_SIZE || MAX_GEOMETRY_SIZE;
 
@@ -120,10 +120,10 @@ var Cardboard = module.exports = function(config) {
         catch (err) { return callback(err); }
 
         var q = queue(1);
-        q.defer(config.s3.putObject.bind(config.s3), encoded[1]);
+        if (encoded[1]) q.defer(config.s3.putObject.bind(config.s3), encoded[1]);
         q.defer(config.dyno.putItem, encoded[0]);
         q.await(function(err) {
-            var result = geobuf.geobufToFeature(encoded[1].Body);
+            var result = geobuf.geobufToFeature(encoded[0].val || encoded[1].Body);
             result.id = utils.idFromRecord(encoded[0]);
             callback(err, result);
         });
@@ -164,7 +164,7 @@ var Cardboard = module.exports = function(config) {
     cardboard.del = function(primary, dataset, callback) {
         var key = { dataset: dataset, id: 'id!' + primary };
 
-        config.dyno.deleteItem(key, { expected: { id: 'NOT_NULL'} }, function(err, res) {
+        config.dyno.deleteItem(key, { expected: { id: 'NOT_NULL'} }, function(err) {
             if (err && err.code === 'ConditionalCheckFailedException') return callback(new Error('Feature does not exist'));
             if (err) return callback(err, true);
             else callback();
@@ -271,7 +271,7 @@ var Cardboard = module.exports = function(config) {
 
             keys.push({ dataset: dataset, id: 'metadata!' + dataset });
 
-            config.dyno.deleteItems(keys, function(err, res) {
+            config.dyno.deleteItems(keys, function(err) {
                 callback(err);
             });
         });
@@ -640,4 +640,4 @@ var Cardboard = module.exports = function(config) {
     };
 
     return cardboard;
-};
+}
diff --git a/lib/batch.js b/lib/batch.js
@@ -1,8 +1,8 @@
-var url = require('url');
 var queue = require('queue-async');
 var geobuf = require('geobuf');
 var _ = require('lodash');
 var Dyno = require('dyno');
+var AWS = require('aws-sdk');
 
 module.exports = function(config) {
     if (!config.bucket) throw new Error('No bucket set');
@@ -29,7 +29,6 @@ module.exports = function(config) {
     batch.put = function(collection, dataset, callback) {
         var records = [];
         var geobufs = [];
-        var s3objects = [];
 
         var encoded;
         var q = queue(150);
@@ -39,13 +38,13 @@ module.exports = function(config) {
             catch (err) { return callback(err); }
 
             records.push(encoded[0]);
-            geobufs.push(encoded[1].Body);
-            q.defer(config.s3.putObject.bind(config.s3), encoded[1]);
+            geobufs.push(encoded[0].val || encoded[1].Body);
+            if (encoded[1]) q.defer(config.s3.putObject.bind(config.s3), encoded[1]);
         }
 
         q.awaitAll(function(err) {
             if (err) return callback(err);
-            config.dyno.putItems(records, function(err, items) {
+            config.dyno.putItems(records, function(err) {
                 if (err && err.unprocessed) {
                     var table = Object.keys(err.unprocessed)[0];
                     var unprocessed = err.unprocessed[table].map(function(item) {

diff --git a/lib/metadata.js b/lib/metadata.js
@@ -5,7 +5,9 @@ var SphericalMercator = require('sphericalmercator');
 var merc = new SphericalMercator();
 var _ = require('lodash');
 
-var Metadata = module.exports = function(dyno, dataset) {
+module.exports = Metadata;
+
+function Metadata(dyno, dataset) {
 
     /**
      * A client for interacting with the metadata for a dataset
@@ -225,7 +227,7 @@ var Metadata = module.exports = function(dyno, dataset) {
     };
 
     return metadata;
-};
+}
 
 /**
  * Adds additional information to return which can be calculated from the metadata object
@@ -257,7 +259,7 @@ function prepare(info) {
 function zoomRange(bytes, extent) {
     var maxSize = 500 * 1024;
     var maxzoom = 14;
-    for (z = 22; z >= 0; z--) {
+    for (var z = 22; z >= 0; z--) {
         var bounds = merc.xyz(extent, z, false, 4326);
         var x = (bounds.maxX - bounds.minX) + 1;
         var y = (bounds.maxY - bounds.minY) + 1;
@@ -294,8 +296,7 @@ function isDatabaseRecord(obj) {
         west: 'number',
         south: 'number',
         east: 'number',
-        north: 'number',
-        s3url: 'string'
+        north: 'number'
     };
 
     return Object.keys(schema).reduce(function(isDatabaseRecord, key) {

diff --git a/lib/utils.js b/lib/utils.js
@@ -7,7 +7,9 @@ var cuid = require('cuid');
 var Metadata = require('./metadata');
 var tilebelt = require('tilebelt');
 
-var Utils = module.exports = function(config) {
+module.exports = Utils;
+
+function Utils(config) {
     /**
      * A module containing internal utility functions
      */
@@ -22,11 +24,12 @@ var Utils = module.exports = function(config) {
         var q = queue(100); // Concurrency of S3 requests
 
         dynamoRecords.forEach(function(dynamoRecord) {
-            var val = dynamoRecord.val;
-            var uri = url.parse(dynamoRecord.s3url);
-
             q.defer(function(next) {
+                var val = dynamoRecord.val;
+                var uri = dynamoRecord.s3url ? url.parse(dynamoRecord.s3url) : undefined;
+
                 if (val) return next(null, geobuf.geobufToFeature(val));
+                if (!uri) return next(new Error('No feature data was found for ' + utils.idFromRecord(dynamoRecord)));
 
                 config.s3.getObject({
                     Bucket: uri.host,
@@ -48,9 +51,8 @@ var Utils = module.exports = function(config) {
      * Wraps an array of GeoJSON features in a FeatureCollection
      * @private
      * @param {object[]} records - an array of GeoJSON features
-     * @param {function} callback - a callback function to handle the response
      */
-    utils.featureCollection = function(records, callback) {
+    utils.featureCollection = function(records) {
         return geojsonNormalize({ type: 'FeatureCollection', features: records });
     };
 
@@ -84,12 +86,16 @@ var Utils = module.exports = function(config) {
             west: truncateNum(info.west),
             south: truncateNum(info.south),
             east: truncateNum(info.east),
-            north: truncateNum(info.north),
-            s3url: ['s3:/', config.bucket, s3Key].join('/')
+            north: truncateNum(info.north)
         };
 
-        if (!useS3) item.val = buf;
-        return [item, s3Params];
+        if (useS3) {
+            item.s3url = ['s3:/', config.bucket, s3Key].join('/');
+            return [item, s3Params];
+        } else {
+            item.val = buf;
+            return [item];
+        }
     };
 
     /**
@@ -105,7 +111,7 @@ var Utils = module.exports = function(config) {
     };
 
     return utils;
-};
+}
 
 function truncateNum(num) {
     return Math.round(Math.pow(10, 6) * num) / Math.pow(10, 6);

diff --git a/package.json b/package.json
@@ -4,10 +4,11 @@
   "description": "A library for storing and searching geographic features",
   "main": "index.js",
   "scripts": {
-    "pretest": "jshint index.js lib test && jscs index.js lib test",
+    "pretest": "eslint index.js lib test/*.js",
     "test": "node test/index.js",
     "coverage": "istanbul cover test/index.js && coveralls < ./coverage/lcov.info",
-    "docs": "node ./docs/generate.js"
+    "docs": "node ./docs/generate.js",
+    "posttest": "rm -r ./test/test"
   },
   "bin": {
     "cardboard": "./bin/cardboard.js"
@@ -47,24 +48,12 @@
     "documentation": "^1.4.0",
     "dynalite": "^0.14.0",
     "dynamodb-test": "^0.1.3",
+    "eslint": "^1.4.3",
     "geojson-fixtures": "0.1.0",
     "geojson-random": "^0.2.2",
     "geojson-stream": "0.0.0",
     "istanbul": "^0.3.15",
-    "jscs": "^1.13.1",
-    "jshint": "^2.8.0",
     "mock-aws-s3": "^0.2.1",
     "tape": "^4.0.0"
-  },
-  "jscsConfig": {
-    "preset": "airbnb",
-    "validateIndentation": 4
-  },
-  "jshint": {
-    "node": true,
-    "globalstrict": false,
-    "undef": true,
-    "unused": true,
-    "noarg": true
   }
 }
diff --git a/test/batch.test.js b/test/batch.test.js
@@ -130,7 +130,7 @@ test('[batch] unprocessed delete returns array of ids', function(assert) {
     var data = fixtures.random(1);
     data.features[0].id = 'abc';
 
-    cardboard.batch.put(data, 'default', function(err, collection) {
+    cardboard.batch.put(data, 'default', function(err) {
         if (err) throw err;
 
         mockcardboard.batch.remove(['abc'], 'default', function(err) {

diff --git a/test/bbox_query.js b/test/bbox_query.js
@@ -4,7 +4,6 @@ var Cardboard = require('../');
 
 var s = require('./setup');
 var config = s.config;
-var dyno = s.dyno;
 
 test('setup', s.setup);
 
@@ -43,7 +42,7 @@ test('queries along 0 lat/lon', function(t) {
         q.defer(cardboard.put, f, dataset);
     });
 
-    q.awaitAll(function(err, res) {
+    q.awaitAll(function(err) {
         t.ifError(err, 'inserted');
         runQueries();
     });
@@ -84,12 +83,12 @@ test('query for line crossing 0 lon n of eq', function(t) {
     var dataset = 'line-query';
 
     var feature = {
-            type: 'Feature',
-            properties: {},
-            geometry: {
-                type: 'LineString',
-                coordinates: [[-1, 1], [1, 1]]
-            }};
+        type: 'Feature',
+        properties: {},
+        geometry: {
+            type: 'LineString',
+            coordinates: [[-1, 1], [1, 1]]
+        }};
 
     // all queries should return a single result
     var queries = [
@@ -106,7 +105,7 @@ test('query for line crossing 0 lon n of eq', function(t) {
 
     var q = queue();
     q.defer(cardboard.put, feature, dataset);
-    q.awaitAll(function(err, res) {
+    q.awaitAll(function(err) {
         t.ifError(err, 'inserted');
         runQueries();
     });
@@ -147,12 +146,12 @@ test('query for line crossing 0 lon s of eq', function(t) {
     var dataset = 'line-query';
 
     var feature = {
-            type: 'Feature',
-            properties: {},
-            geometry: {
-                type: 'LineString',
-                coordinates: [[-1, -1], [1, -1]]
-            }};
+        type: 'Feature',
+        properties: {},
+        geometry: {
+            type: 'LineString',
+            coordinates: [[-1, -1], [1, -1]]
+        }};
 
     // all queries should return a single result
     var queries = [
@@ -169,7 +168,7 @@ test('query for line crossing 0 lon s of eq', function(t) {
 
     var q = queue();
     q.defer(cardboard.put, feature, dataset);
-    q.awaitAll(function(err, res) {
+    q.awaitAll(function(err) {
         t.ifError(err, 'inserted');
         runQueries();
     });
@@ -207,21 +206,21 @@ test('query for line near -90 lon n of eq', function(t) {
 
     // tile for this feature: [ 31, 63, 7 ]
     var wanted = {
-            type: 'Feature',
-            properties: {},
-            geometry: {
-                type: 'LineString',
-                coordinates: [[-92, 1], [-91, 1]]
-            }};
+        type: 'Feature',
+        properties: {},
+        geometry: {
+            type: 'LineString',
+            coordinates: [[-92, 1], [-91, 1]]
+        }};
 
     // tile for this feature: [0, 0, 0]
     var unwanted = {
-            type: 'Feature',
-            properties: {},
-            geometry: {
-                type: 'LineString',
-                coordinates: [[-1, 1], [1, 1]]
-            }};
+        type: 'Feature',
+        properties: {},
+        geometry: {
+            type: 'LineString',
+            coordinates: [[-1, 1], [1, 1]]
+        }};
 
     // all queries should return a single result
     var queries = [
@@ -233,7 +232,7 @@ test('query for line near -90 lon n of eq', function(t) {
     var q = queue();
     q.defer(cardboard.put, wanted, dataset);
     q.defer(cardboard.put, unwanted, dataset);
-    q.awaitAll(function(err, res) {
+    q.awaitAll(function(err) {
         t.ifError(err, 'inserted');
         runQueries();
     });
@@ -299,7 +298,7 @@ test('queries along antimeridian (W)', function(t) {
         q.defer(cardboard.put, f, dataset);
     });
 
-    q.awaitAll(function(err, res) {
+    q.awaitAll(function(err) {
         t.ifError(err, 'inserted');
         runQueries();
     });