Fix #222: Mongo::Error::OperationFailure: E11000 duplicate key error index.

[dblock]

This is a proposed fix for #222. The problem is that we try to create records with a blank indexed field that generates blank slugs, which causes the unique index to break. We remove the default value of [] and store a nil slug that works with a sparse: true index.

I'd love a code review from @johnnyshields or @digitalplaywright please.

[Dan-Nolan]

Could you also add in a check to see if to_url resolves to a nil slug? I ran into a problem with this specifically when users were inputting values like '-'. The expression'-'.to_url resolves to an empty string as well, which could also cause this problem.

[dblock]

Good point @Dan-Nolan, that seems to work, see specs in 218d257.

[Dan-Nolan]

Sweet! Thanks for checking that out!

[johnnyshields]

The bigger question is, should we allow _slugs to be blank? I think we should (esp for legacy reasons) by using a sparse index on _slugs so that undefined is ignored for uniqueness purposes.

I think the logic to generate a fallback slug should be handled by the implementer on a case-by-case basis. I'm not convinced that using the model name as a default fallback is an elegant solution.

[dblock]

@johnnyshields You think that sparse: true on the index would work?

[johnnyshields]

@dblock sparse indexes will ignore documents which do not have the _slugs field set, hence are good for incorporating legacy documents. However, if the value of _slugs is nil (i.e. set in db to null) or [] then the unique index error can occur. As long as we're not setting _slugs in the DB for legacy and non-slugged docs then a sparse index would work.

https://docs.mongodb.com/manual/core/index-sparse/

As of MongoDB 3.2 there's a better option called "partial indexes" (sparse index on steroids) where you can exclude values nil and [] from the index as well.

https://docs.mongodb.com/manual/core/index-partial/

[dblock]

@johnnyshields I updated this PR. The problem is that we default slugs to []. If we remove that we can start setting nils. I think it's better, would like your opinion.

[johnnyshields]

@dblock this needs to be tested. my understanding is that sparse indexes consider "nil" (null) as a value if it's explicitly set in the db, so be careful. I've filed an issue with Mongoid that they need a way to distinguish between nil and unset.

[dblock]

@johnnyshields: This is tested here, note that I am creating an index in spec_helper.

[dblock]

I hear you @johnnyshields on 3.2.0, we could default to [] and require that version and make a more special index, but that's a whole other ballgame and will require people to migrate a massive amount of indexes.

The code in Mongoid doesn't set the value to nil, it leaves it out. At least that's what I see.

[dblock]

Ok, I am pretty happy with what's in this PR.

@johnnyshields: do you think I am doing it all wrong or can I merge this?

@Dan-Nolan: the implementation has changed and since we last spoke, check this out.

@BorisBresciani you opened #222, please check this out.

[johnnyshields]

@dblock looks fine to me.

[Dan-Nolan]

@dblock Looks great to me; very well documented too! Handling fallback slugs in a case-by-case basis makes sense, and in our case, we ended up not allowing values to resolve to empty stringex urls. It was bad data for us anyhow.

[dblock]

FYI was released in 5.3.0