Prepended backslash in class name reloads the full metadata from scratch

[Tofandel]

Bug Report

Follow up of symfony/symfony#46837

Summary

When extracting class metadata, doctrine copies the original class data and caches it only the first time
But following that copy, calls to addEntityListener are made which are not repercuted on the alias

persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php

Line 180 in 25ec98a

return $this->loadedMetadata[$className] = $this->loadedMetadata[$realClassName];

Solution

The alias should not be copied, but should just be added to an alias map to retrieve the original cached class straight away

[stof]

Another alternative would be to assign the metadata to the alias, but as a PHP reference rather than a copy.

[Tofandel]

Well I think that's already the case because it's an EntityMetaData instance being copied
After looking at it a bit more, it's not this line that copies it, it's actually treated as a completely different class and reloaded, hence the issue
getRealClass doesn't remove this prepended slash

So I think it's just a question of trimming this slash in getRealClass, so that it's treated as an alias instead of reloaded from scratch

[greg0ire]

Normalization seems more like the way to go yes. If I understood correctly, all this is caused because A\Class and \A\Class are equally valid ways to refer to a class. If that's indeed the issue then we should tweak our APIs so that they normalize FQCNs passed as input to just one representation.

[greg0ire]

Also, be careful when talking about aliases because in doctrine/persistence, (short) aliases mean a representation like this: My:Alias. That feature has been deprecated in #205

[Tofandel]

@greg0ire Yes exactly that

I think just this should cover it

    public function resolveClassName(string $className): string
    {
        $className = ltrim($className, '\\');
        \\...
        

Yes sorry I used alias as in class_alias, but that's not really what I'm trying to depict either, so let's completely forget about aliases as they're not relevant to this, it's 100% a FQCN with multiple possible representation issue

[greg0ire]

Would it?

I think that wouldn't be enough to take advantage of this cache here:

persistence/src/Persistence/Mapping/AbstractClassMetadataFactory.php

Lines 164 to 166 in 25ec98a

	if (isset($this->loadedMetadata[$className])) {
	return $this->loadedMetadata[$className];
	}

If you call getMetadataFor with a class name prepended with a backslash, I'd expect this condition to always fail because I would expect loadedMetadata to use normalized class names (meaning without a leading backslash) as keys.

[Tofandel]

It would take advantage of the cache on line 180 just a bit after, otherwise we can ltrim straight away $className first thing in this function

Do you know any other places where class names would be used as keys?

[greg0ire]

we can ltrim straight away $className first thing in this function

That would make a lot more sense to me.

Do you know any other places where class names would be used as keys?

That normalization should also happen in setMetadataFor and hasMetadataFor IMO.