fix #174 Drop mongodb roles index "name_1" in _forwardMigrate

[jsamr]

See #174

WARNING : throws an exception if index "name_1" does not exists. Any reason to think this might happen? In that case, might want to surround it with a try catch block and print a warning.

[mitar]

Thanks for the pull request.

I would wrap it into the try/catch, but also verify that the error thrown is really that the index does not exist. Otherwise rethrow the exception.

Also, do the inverse for backwards migration.

[jsamr]

There is unfortunately no neat way to check against this peculiar exception. I have made up this workaround though :

EDITED SNIPPET

    // drop old index, preventing mongo duplicate key 'null' errors
    try {
      Meteor.roles._dropIndex("name_1");
    } catch (e) {
      if (e.name !== 'MongoError') throw e;
      match = e.errmsg.match(/index not found/);
      if (!match) throw e;
    }

But I'm not certain this message doesn't depend on mongodb driver or mongodb release. PS :

• full message is index not found with name [name_1]
• Meteor 1.2.1
• mongodb v3.0.7

[mitar]

See code example here. So maybe do a similar check for MongoError.

And do not worry about warning. Just ignore in that case. We just want to prevent errors later on. We do not really care if index is there. We just do not want to interfere with it later.

Also respect code style.

[jsamr]

Ok I updated the above snippet, hope code style is fitting now. But something is intriguing me : in the portion of code you referred, you used e.err while I had to use e.errmsg (e.err was undefined), any thoughts ?

[mitar]

Interesting. Even in one other code I use e.err. Not sure where errmsg is coming from. You could turn around your regex as I have in this code and then do (e.err || e.errmsg) just to be sure.

You can fix the same also in the other part of the code.

And also make removing/adding indexes for forward migration.

And just work in the pull request, not by editing the code in the comment.

[jsamr]

Yeah well that was to prevent from having 10 commits instead of 2 sensible ones! I'll do the backwards thing during next week, and commit this snippet + err | errmsg changes tonight.

[mitar]

Yeah well that was to prevent from having 10 commits instead of 2 sensible ones!

You can always squash them later.

[jsamr]

Done !

[mitar]

Style is a bit off. When you are contributing to an existing project, always try to match code style of the rest of the code. And in general be mindful of your code style. Even your own code style of code you are contributing is not consistent.

[mitar]

Fixed style in https://github.com/alanning/meteor-roles/commit/76fe6541573e77d1f6cb2ab5875c38894a4a40dc.

Thanks for reporting this and fixing it.

[jsamr]

Thanks mitar for your patience and advices. I will pay more attention to code style next time I contribute.