Indexing embedded document in different fields fails due duplicate name

[Steveb-p]

Hello,

I'll go right to the point. When embedding a document class in two different fields and attempting to set an index on that document class, index cannot be created due to name conflict:

Trying to create an index with same name test with different key spec { property2.property_sub: 1 } vs existing spec { property1.property_sub: 1 }

Here are two classes that allow to reproduce the error.
Main class goes as follows:

<?php

namespace AppBundle\Document\Test;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
 * @ODM\Document
 */
class TestMain
{
    /**
     * @ODM\Id
     */
    private $id;

    /**
     * @ODM\EmbedMany(targetDocument="TestSub")
     */
    private $property1;

    /**
     * @ODM\EmbedMany(targetDocument="TestSub")
     */
    private $property2;
}

Embedded class:

<?php

namespace AppBundle\Document\Test;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
 * @ODM\Document
 * @ODM\Index(keys = {"property_sub" = "asc"}, name = "test")
 */
class TestSub
{
    /**
     * @ODM\Id
     */
    private $id;

    /**
     * @ODM\Field(type="string")
     */
    private $property_sub;

    /**
     * @ODM\Field(type="string")
     */
    private $property_sub2;
}

I've tested this a little, and issue goes away if:

• name attribute for index is not used
• document is embedded just once

In my case, I'm bound to use name because document is embedded deep in main document tree - otherwise I receive error regarding maximum index name length, which is already nicely mentioned in the docs.

As far as I'm concerned, I will probably create indexes manually, since I'm about to use a document in a "standalone" collection or embedded depending on specific application state, so specifying indexes in metadata won't really work for me (I actually do not want those deep indexes, only on main collection to guarantee uniqueness of specific data sets).
However, I believe it is not an expected behaviour, so I'd like to share my case.

Thanks in advance, and thanks for a great library :)

[malarzm]

@JakeFalcor thanks for the report! I'm not sure yet we can do for collision of index names, I'll try to give it some thought.

Offhand, you shouldn't use same class for being first class document and an embedded one at the same time, correct structure would be having a mapped superclass which is extended by two different classes (then you could add index annotation to class annotated as @Document which would ultimately solve your problem). I'm not sure why we are allowing this at all but I remember it had a lot of side effects back in time and wouldn't consider it safe.

thanks for a great library :)

Glad you're enjoying it ;)

[Steveb-p]

thanks for the report! I'm not sure yet we can do for collision of index names, I'll try to give it some thought.

I was wondering myself how it can be approached. As I said, automatic generation works fine most of the time. However, if name is specified, then class can be embedded just once.
Just pre/postfixing name doesn't seem like a good idea, since it won't be apparent why index name is changed despite being explicitly set.
I'm not really sure, since I'm far from being anywhere close expert at writing annotation/configuration files and mechanism behind them, but maybe a callback function would be acceptable? Something that will allow one to decide themselves how to approach this? Is there anything against it?

Offhand, you shouldn't use same class for being first class document and an embedded one at the same time, correct structure would be having a mapped superclass which is extended by two different classes (then you could add index annotation to class annotated as @document which would ultimately solve your problem). I'm not sure why we are allowing this at all but I remember it had a lot of side effects back in time and wouldn't consider it safe.

I was trying not to create additional classes just to transform one into another, but it probably will be the case. Without it - I'm not 100% yet - it seems that such "embedded" yet declared as "documents" objects are saved in both places: embedded and in collection. I think they even do update accordingly, which is not exactly what my intention is. I'm trying to create "snapshots" and save them alongside documents and this "no-second-class" approach is probably biting me right now :)
I'll try to confirm it. Thanks for fast response.

[alcaeus]

I'm wondering what we could do to prevent this. One option would be to deprecate the name property for indexes in embedded documents and rely on autogenerated index names. Another option would be to manually check indexes and their names when mapping embedded relationships within ClassMetadataFactory and throwing a MappingException when an embedded document class is used for more than one field and it contains an index with a name option. The downside is that we'd have to check the full document tree downwards as the document class might be reused at a different level. What do you think @malarzm?

[Steveb-p]

Just reminding in discussion that in case of deprecating name property, there is also this issue with index names being generated too long when deep inside the tree.

[malarzm]

On a more aggresive note we could forbid defining indexes in the embedded documents (and ignoring them if they'd come from mapped superclass). That would be less convienient but will elude both duplicated names when embedded doc is reused and too long autogenerated names.

[alcaeus]

I don't like that approach as much - I remember me using that feature in quite a few places.

[malarzm]

Just trying to approach the problem from different perspective as I can't think of any good solution... Offhand I think I was using indexes on embedded documents too :)

[The-Don-Himself]

@malarzm just got caught by it as you predicted

namespace name generated from index name "........." is too long (127 byte max)

the "......." was really long I believe because I'm using the 'blending-orm-and-mongodb-odm' feature within an embedded doc with an index.

A double-edged sword this, because for some use cases, embedded indexes makes sense.

[malarzm]

Actually how about we generate indexes for embedded document BUT only one (max two) levels deep from document?

[The-Don-Himself]

@ALL Happy New Year!

I've been thinking, would it be possible to allow users to map an 'alias' of sorts for the autogenerated index name. Unfortunately don't know too much of Mongo or/and Doctrine internals say for sure but I would propose the first 3 letters of each 'nested' index name.

[malarzm]

@The-Don-Himself could you post an example? I'm not sure if I'm getting this right

[The-Don-Himself]

I'm not sure if I can come up with an example per se but let me quote the @Steveb-p statetement

In my case, I'm bound to use name because document is embedded deep in main document tree - otherwise I receive error regarding maximum index name length, which is already nicely mentioned in the docs.

But my understanding is that if you name the index you're very likely to cause a naming collision as you said here

@JakeFalcor thanks for the report! I'm not sure yet we can do for collision of index names, I'll try to give it some thought.

I'm thinking that to still allow deeply nested indices but (try) avoid naming collisions we could introduce aliases as in @Steveb-p example the index name property2 could be aliased to prop2 or better still the first two and last 2 characters so 'pry2'.

Sort of like an 'index naming strategy' to try and shorten the index names. Does this make sense?

[malarzm]

Sort of like an 'index naming strategy' to try and shorten the index names. Does this make sense?

Unfortunately this increases chances of name collision, fields created_at and crafted_at would both be shortened to crat. But maybe this would be good enough to be an opt-in feature?

[The-Don-Himself]

Yes, it will increase the chances of a collision vis-à-vis longer names, and the example you gave above showcases it really well, but I think if documented then devs can be able to weigh the best option for them, whether a 2, 3, 4 character shortening strategy could be of use. It certainly isn't the best solution, but at least it can try gives us the best of both worlds - nested indices with a lesser chance of collisions.

[Steveb-p]

I do believe that "normal" index creation name relies on mongo default behaviour, so it would be really nice if it would just be done in database itself :) Since it's not how real world works...

I'd like to suggest a "hash" approach, where index name would be generated using selected hashing mechanism based on some data available in ODM for determining final index name. This would be similar to indexes created by MySQL Doctrine adapter. It would cause names to lose their semantic meaning, but name collision would be practically non-existent - and, after all, getIndexes mongo method does return all the necessary data to establish what indexes are present and what keys they cover, so name is somewhat secondary?

So, to sum up, is some kind of "generator" strategy for indexes acceptable? If so, what key points of ODM should be looked into if I'd like to add it?