search

Meteor-Community-Packages / meteor-schema-index

Control some MongoDB indexing with schema options
MIT License
38 stars 17 forks source link

Reindex on change of `unique` or `sparse` #21

Closed SimonSimCity closed 10 months ago

SimonSimCity commented 6 years ago

This fixes #19 and requires #20 to be merged first.

The only downside is, that this script doesn't handle a changed definition from { name: 'foo', unique: true } to { name: 'foo', unique: false }. It will not delete the unique index of this property, neither will it re-create the index without uniqueness required.

Should I change the cod so it removes such an index?

aldeed commented 6 years ago

@SimonSimCity This is good and can be merged after a few things.

  1. Can you rebase on master now that I merged your other PR?
  2. Can you add a few tests to show that this works in different circumstances? I do not think this will work if you have unique: true without index: true and then you remove unique or set to false because it is wrapped in if ('index' in definition || definition.unique === true). The conditional logic will probably have to be tweaked a bit.
  3. As long as you're going to do this for unique and sparse, might as well make it work when you remove index without setting it to false, right? The correct approach to make it all work might be to loop through the existing indices before the schema loop, and if any don't match their schema, drop them. Then do the current loop through the schema to re-add them.
aldeed commented 6 years ago

We would also need to be careful that we don't drop existing indices added outside of this package. When adding them to existingIndexes, you should check first to make sure that index.name starts with c2_. Add a test for this, too.

SimonSimCity commented 6 years ago

Sorry for it taking soo long time for me to get back to you here ...

I've now rebased it on master and tried to run the tests, but wasn't able to. The command, stated in the README.md (npm test:watch or npm test) resulted in an error and calling meteor test-packages ./package/indexing/ - which was my next guess, since this is a meteor package - left me without any tests. Can you please help me in getting this going?

SimonSimCity commented 6 years ago

@aldeed could you please help me to run the tests?