Make migration stream more efficient

lib/migration-stream.js

@@ -6,48 +6,9 @@ var fs = require('fs');
 module.exports = MigrationStream;
 module.exports.migrate = migrate;
 
-function migrate(tile, callback) {
-  var vtile = new mapnik.VectorTile(tile.z, tile.x, tile.y);
-  vtile.setData(tile.buffer, {upgrade:true}, function(err) {
-    if (err) {
-        err.code = 'EINVALID';
-        return callback(err);
-    }
-    vtile.getData({compression:'gzip'},function(err, data) {
-      if (err) return callback(err);
-      tile.buffer = data;
-      return callback(null, tile);
-    });
-  });
-}
-
-function checkForErrors(tile) {
-  var info = mapnik.VectorTile.info(tile.buffer);
-  var err = null;
-
-  if (info.errors) {
-    if (info.tile_errors) {
-      err = new Error(info.tile_errors[0].replace(' message', ''));
-    } else if (info.layers) {
-      for (var i = 0; i < info.layers.length; i++) {
-        var layer = info.layers[i];
-        if (layer.errors) {
-          err = new Error(layer.errors[0].replace(' message', ''));
-          break;
-        }
-      }
-    } else {
-      err = new Error('Invalid data');
-    }
-    err.code = 'EINVALID';
-  }
-
-  return err;
-}
-
 function MigrationStream() {
   var migrationStream = new stream.Transform({ objectMode: true });
-  var v1TileDataLogged = false; 
+  var v1TileDataLogged = false;
 
   migrationStream._transform = function(tile, enc, callback) {
     if (!tile.buffer) {
@@ -66,34 +27,71 @@ function MigrationStream() {
     var v2 = info.layers.every(function(layer) {
         return layer.version === 2;
     });
-    
+
     if (v2 === true) {
       // Check for errors on the v2 tile
-      var err = checkForErrors(tile);
+      var err = checkForErrors(info);
       if (err) return callback(err);
 
       migrationStream.push(tile);
       return callback();
-    }else{
-      if(!v1TileDataLogged && process.env.LOG_V1_TILES){
-        fs.writeFileSync('v1-stats.json', JSON.stringify({'tileVersion':'1'}));
+    } else {
+      if (!v1TileDataLogged && process.env.LOG_V1_TILES) {
+        fs.writeFileSync('v1-stats.json', JSON.stringify({ 'tileVersion': '1' }));
         v1TileDataLogged = true;
-      }; 
+      };
     }
 
 
-
-    migrate(tile, function(err, tile) {
+    migrate(tile, function(err, new_tile) {
       if (err) return callback(err);
 
       // Check for errors on newly-minted v2 tile
-      var err = checkForErrors(tile);
+      var err = checkForErrors(mapnik.VectorTile.info(new_tile.buffer));
       if (err) return callback(err);
 
-      migrationStream.push(tile);
+      migrationStream.push(new_tile);
       return callback();
     });
   };
 
   return migrationStream;
 }
+
+function migrate(tile, callback) {
+  var vtile = new mapnik.VectorTile(tile.z, tile.x, tile.y);
+  vtile.setData(tile.buffer, {upgrade:true}, function(err) {
+    if (err) {
+        err.code = 'EINVALID';
+        return callback(err);
+    }
+    vtile.getData({compression:'gzip'},function(err, data) {
+      if (err) return callback(err);
+      tile.buffer = data;
+      return callback(null, tile);
+    });
+  });
+}
+
+function checkForErrors(info) {
+  var err = null;
+
+  if (info.errors) {
+    if (info.tile_errors) {
+      err = new Error(info.tile_errors[0].replace(' message', ''));
+    } else if (info.layers) {
+      for (var i = 0; i < info.layers.length; i++) {
+        var layer = info.layers[i];
+        if (layer.errors) {
+          err = new Error(layer.errors[0].replace(' message', ''));
+          break;
+        }
+      }
+    } else {
+      err = new Error('Invalid data');
+    }
+    err.code = 'EINVALID';
+  }
+
+  return err;
+}

test/copy.tilelive.test.js

@@ -1,6 +1,7 @@
 var test = require('tape');
 var path = require('path');
 var mapnik = require('mapnik');
+var mapnikVT = mapnik.VectorTile; // required for spying
 var crypto = require('crypto');
 var tileliveCopy = require('../lib/tilelivecopy');
 var tilelive = require('@mapbox/tilelive');
@@ -93,31 +94,6 @@ test('copy mbtiles without v1 tile logging', function(t) {
   });
 });
 
-test('copy mbtiles with v1 tile logging', function(t) {
-  process.env.LOG_V1_TILES = true;
-  var fixture = path.resolve(__dirname, 'fixtures', 'valid.mbtiles');
-  var src = 'mbtiles://' + fixture;
-  var dst = dsturi('valid.mbtiles');
-  sinon.spy(tilelive, 'copy');
-
-  tileliveCopy(src, dst, {}, function(err) {
-    t.ifError(err, 'copied');
-    tileCount(dst, function(err, count) {
-      t.equal(tilelive.copy.getCall(0).args[2].type, 'list', 'uses list scheme for mbtiles');
-      t.equal(tilelive.copy.getCall(0).args[2].retry, undefined, 'passes options.retry to tilelive.copy');
-      tilelive.copy.restore();
-
-      tileVersion(dst, 0, 0, 0, function(err, version) {
-        var path = './v1-stats.json';
-        t.equal(fs.existsSync(path), true);
-        process.env.LOG_V1_TILES = false;
-        fs.unlinkSync(path);
-        t.end();
-      });
-    });
-  });
-});
-
 test('copy retry', function(t) {
   var fixture = path.resolve(__dirname, 'fixtures', 'valid.mbtiles');
   var src = 'mbtiles://' + fixture;
@@ -143,6 +119,7 @@ test('copy v2 mbtiles', function(t) {
   var dst = dsturi('valid-v2.mbtiles');
   sinon.spy(tilelive, 'copy');
   sinon.spy(migrationStream, 'migrate');
+  sinon.spy(mapnikVT, 'info');
 
   tileliveCopy(src, dst, {}, function(err) {
     t.ifError(err, 'copied');
@@ -153,6 +130,9 @@ test('copy v2 mbtiles', function(t) {
       t.equal(tilelive.copy.getCall(0).args[2].retry, undefined, 'passes options.retry to tilelive.copy');
       tilelive.copy.restore();
 
+      t.equal(mapnikVT.info.callCount, count, 'called mapnik info as many times as there are tiles (should only be once per v2 tile)');
+      mapnikVT.info.restore();
+
       t.equal(migrationStream.migrate.notCalled, true, 'doesn\t migrate a v2 mbtiles file');
       migrationStream.migrate.restore();
 
@@ -165,7 +145,6 @@ test('copy v2 mbtiles', function(t) {
   });
 });
 
-
 test('copy omnivore', function(t) {
   var fixture = path.resolve(__dirname, 'fixtures', 'valid.geojson');
   var src = 'omnivore://' + fixture;