Fix embedding documents containing named indexes

[alcaeus]

Q	A
Type	bug
BC Break	no
Fixed issues	fixes #1344

Summary

This fixes an issue when embedding a document containing a named index multiple times. Since the index had a name, this caused an exception with MongoDB since the index would already exist when creating it for the second embed use (see test case). With this change we're prefixing indexes pulled up from embedded documents with the name of the property that's embedding them. While this can cause exceptions when an index name becomes too long, the current solution gives more control to the user.

One option would be to add an indexPrefix option to embedded relationships and using that instead of the field name. Since that would be a new feature, I will not include it in the bugfix for 1.2.x.

[malarzm]

I've just realized we're not mentioning name at all in https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/1.2/reference/annotations-reference.html#index - could you please add it and note that indexes on embedded documents will have a prefix?

[alcaeus]

could you please add it and note that indexes on embedded documents will have a prefix?

Yeah, prefix should be documented regardless of previous docs. I'll add a chapter later today.

[alcaeus]

Documentation added, mind taking another look @malarzm?

[malarzm]

It'd be great to add name to the list of optional attributes, but if you don't feel like it feel free to merge, it was missing before anyway 👍

[Steveb-p]

From what I remember maximum index name length in MongoDB should be 125 characters. It isn't specified in MongoDB docs, but is mentioned multiple times in MongoDB issue tracker (https://jira.mongodb.org/browse/DOCS-1820 for example) and on some other sites. I'd say mention of this limit might be added to that documentation note?

ODM might also check for index name length in the future?

[alcaeus]

From what I remember maximum index name length in MongoDB should be 125 characters. It isn't specified in MongoDB docs

The index name length documentation explains this in more detail:

Fully qualified index names, which includes the namespace and the dot separators (i.e. ..$), cannot be longer than 128 characters.

We can check this at schema creation, but I believe any attempt to truncate index names can just cause issues down the road, so I prefer if the original exception was shown to the user. As mentioned previously, I believe it’s more sensible allowing users to configure this index prefix in future versions than trying to add more magic to the prefix.

[alcaeus]

It'd be great to add name to the list of optional attributes

I somehow completely missed the fact that the name wasn’t in the options. This should indeed be properly documented. 👍

[Steveb-p]

My intention would not be to truncate or otherwise transform the index name, but rather prevent the attempt to create such an index or wrap exception with Doctrine's own one with some helpful message about how it can be resolved, and maybe what field it was about.

But that is wayyy out of scope :) Thank you @alcaeus

[alcaeus]

@malarzm added a link to the indexes chapter to the annotation mapping, as the options are explained in more detail there. I've also added the note about index prefixing there so it won't get lost.

@Steveb-p let's fix the first issue - the reason I'm not really keen on doing too much with the exception is because either way, the user will be presented with a difficult problem: they have to either rename the index, the fields contained in the index path (as they are prefixed to the name), the collection name or even the database name.

Even without our exception, the error message provided by MongoDB will contain all necessary information:

namespace name generated from index name "doctrine_odm_tests.GH1344TooLongIndexName.$embedded1_this_is_a_really_long_name_that_will_cause_problems_for_whoever_tries_to_use_it_whether_in_an_embedded_field_or_not" is too long (127 byte max)

Regardless, I've added a test case to cover this case to make sure we're always giving users at least the index name.