Conversion of object query parameters to their identifiers is inconsistent

[stof]

The detection of whether the object is an entity uses $this->_em->getMetadataFactory()->hasMetadataFor(ClassUtils::getClass($value)). But hasMetadataFor checks whether the factory has loaded metadata for the class. So the behavior depends on what happened previously in the request (and can change between a dev environment and the prod as the prod is more likely to use a cache for the DQL parsing logic, which is one of the place which could trigger the loading of metadata when being executed).

I suggest switching to using ! isTransient() instead (potentially keeping the check for loaded metadata first for performance reasons as isTransient does not check the loaded metadata to short-circuit drivers).
what do you think about it ?

[Ocramius]

I think we should modify hasMetadataFor() so that it actually does an internal lookup, and fails silently if the requested class is not mapped. The fact that hasMetadataFor() always checks for current in-memory state is a big annoyance, and it caused a lot of bugs in the past as well.

[stof]

you mean making hasMetadataFor the equivalent of ! isTransient for the returned value ? This would be fine with me, but it could be considered a BC break

[stof]

Btw, this would require changing the interface in doctrine/common too, because this is the documented behavior: https://github.com/doctrine/common/blob/v2.7.1/lib/Doctrine/Common/Persistence/Mapping/ClassMetadataFactory.php#L54

so I'd rather not change it now

[Ocramius]

@stof I can find bugs for basically every usage of hasMetadataFor, so I think making it an alias (and keeping a map of the transient classes internally, for performance) would be a massive improvement.

About BC breaks, it would need a lookup (on github) of who uses that method, and for which purposes.

[Ocramius]

@stof yeah, the interface change would indeed be a breakage, nice spotting :-\

[InputOutput]

I think I just encountered this bug in the wild, using an Entity as parameter in a where condition in DQL without specifying the specific property to use (in this case ID). Worked fine in dev mode, broke after cache warmup in production.