Merge pull request #30 from pryv/bugfix/parentid-zero-length

add parentId min length validation + related DB migration

Author: kebetsi

components/api-server/src/schema/stream.js

@@ -25,7 +25,8 @@ module.exports = function (action, ignoreChildren, refToStreamSchema) {
         minLength: 1
       },
       'parentId': {
-        type: ['string', 'null']
+        type: ['string', 'null'],
+        minLength: 1
       },
       'singleActivity': {
         type: 'boolean'

components/api-server/test/streams.test.js

@@ -282,6 +282,21 @@ describe('streams', function () {
       );
     });
 
+    // Test added to verify fix of issue#29
+    it('must return an error if the new stream\'s parentId ' +
+      'is the empty string', function (done) {
+      var data = {
+        name: 'zero-length parentId string Stream',
+        parentId: ''
+      };
+      request.post(basePath).send(data).end(function (res) {
+        validation.checkError(res, {
+          status: 400,
+          id: ErrorIds.InvalidParametersFormat
+        }, done);
+      });
+    });
+
     it('must slugify the new stream\'s predefined id', function (done) {
       var data = {
         id: 'pas encodé de bleu!',

components/storage/src/migrations/0.7.1.js

@@ -0,0 +1,72 @@
+var async = require('async'),
+  toString = require('components/utils').toString;
+
+/**
+ * v0.7.1:
+ *
+ * - Fixes streams with parentId='' to parentId=null
+ */
+module.exports = function (context, callback) {
+  context.database.getCollection({name: 'users'}, function (err, usersCol) {
+    if (err) { return callback(err); }
+
+    usersCol.find({}).toArray(function (err, users) {
+      if (err) { return callback(err); }
+
+      async.forEachSeries(users, migrateUser, function (err) {
+        if (err) { return callback(err); }
+
+        context.logInfo('Data version is now 0.7.1');
+        callback();
+      });
+    });
+  });
+
+  function migrateUser(user, callback) {
+    context.logInfo('Migrating user ' + toString.user(user) + '...');
+    async.series([
+      function updateStreamsStructure(stepDone) {
+        context.database.getCollection({name: user._id + '.streams'}, function (err, streamsCol) {
+          if (err) {
+            context.logError(err, 'retrieving streams collection');
+            return stepDone(err);
+          }
+
+          var streamsCursor = streamsCol.find(),
+            completed = false;
+          async.until(function () { return completed; }, migrateStreams,
+            context.stepCallbackFn('migrating events structure', stepDone));
+
+          function migrateStreams(streamDone) {
+            streamsCursor.nextObject(function (err, stream) {
+              if (err) { return setImmediate(streamDone.bind(null, err)); }
+              if (! stream) {
+                completed = true;
+                return setImmediate(streamDone);
+              }
+
+              if (stream.parentId !== '') {
+                return setImmediate(streamDone);
+              }
+
+              var update = {
+                $set: {
+                  parentId: null
+                }
+              };
+
+              streamsCol.update({_id: stream._id}, update, streamDone);
+            });
+          }
+        });
+      }
+    ], function (err) {
+      if (err) {
+        context.logError(err, 'migrating user');
+        return callback(err);
+      }
+      context.logInfo('Successfully migrated user ' + toString.user(user) + '.');
+      callback();
+    });
+  }
+};

components/storage/src/migrations/index.js

@@ -9,5 +9,6 @@ module.exports = {
   '0.3.0': require('./0.3.0.js'),
   '0.4.0': require('./0.4.0.js'),
   '0.5.0': require('./0.5.0.js'),
-  '0.7.0': require('./0.7.0.js')
+  '0.7.0': require('./0.7.0.js'),
+  '0.7.1': require('./0.7.1.js')
 };

components/storage/test/Versions.test.js

@@ -151,7 +151,7 @@ describe('Versions', function () {
     });
   });
 
-  it('must handle data migration from v0.5.0 to v0.7.0', function (done) {
+  it.skip('must handle data migration from v0.5.0 to v0.7.0', function (done) {
     var versions = getVersions('0.7.0'),
         userId = 'u_0',
         usersStorage = helpers.dependencies.storage.users;
@@ -174,6 +174,31 @@ describe('Versions', function () {
     });
   });
 
+  it('must handle data migration from v0.7.0 to v0.7.1', function (done) {
+    var versions = getVersions('0.7.1'),
+        user = {id: 'ciya1zox20000ebotsvzyl8cx'};
+
+    async.series({
+      restore: testData.restoreFromDump.bind(null, '0.7.0', mongoFolder),
+      migrate: versions.migrateIfNeeded.bind(versions),
+      streams: storage.user.streams.findAll.bind(storage.user.streams, user, {}),
+      version: versions.getCurrent.bind(versions)
+    }, function (err, results) {
+      should.not.exist(err);
+
+      results.streams.forEach(function (stream) {
+        if (stream.parentId !== null) {
+          stream.parentId.should.not.eql('');
+        }
+      });
+
+      results.version._id.should.eql('0.7.1');
+      should.exist(results.version.migrationCompleted);
+      done();
+    });
+
+  });
+
   function getVersions(/* migration1Id, migration2Id, ... */) {
     var pickArgs = [].slice.call(arguments);
     pickArgs.unshift(migrations);

components/test-helpers/src/data/dumps/0.7.0/db/pryv-node/ciya1zox20000ebotsvzyl8cx.accesses.bson

@@ -0,0 +1 @@
+{ "indexes" : [ { "v" : 1, "key" : { "_id" : 1 }, "name" : "_id_", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.accesses" }, { "v" : 1, "unique" : true, "key" : { "token" : 1 }, "name" : "token_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.accesses", "sparse" : true }, { "v" : 1, "unique" : true, "key" : { "name" : 1, "type" : 1, "deviceName" : 1 }, "name" : "name_1_type_1_deviceName_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.accesses", "sparse" : true }, { "v" : 1, "key" : { "deleted" : 1 }, "name" : "deleted_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.accesses", "expireAfterSeconds" : 94608000 } ] }

components/test-helpers/src/data/dumps/0.7.0/db/pryv-node/ciya1zox20000ebotsvzyl8cx.accesses.metadata.json

@@ -0,0 +1 @@
+{ "indexes" : [ { "v" : 1, "key" : { "_id" : 1 }, "name" : "_id_", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.streams" }, { "v" : 1, "key" : { "name" : 1 }, "name" : "name_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.streams" }, { "v" : 1, "unique" : true, "key" : { "name" : 1, "parentId" : 1 }, "name" : "name_1_parentId_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.streams", "sparse" : true }, { "v" : 1, "key" : { "trashed" : 1 }, "name" : "trashed_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.streams" }, { "v" : 1, "key" : { "deleted" : 1 }, "name" : "deleted_1", "ns" : "pryv-node.ciya1zox20000ebotsvzyl8cx.streams", "expireAfterSeconds" : 31536000 } ] }