Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong Manager returned for similar Namespaces #404

Open
friedrichroell opened this issue Nov 4, 2024 · 3 comments
Open

Wrong Manager returned for similar Namespaces #404

friedrichroell opened this issue Nov 4, 2024 · 3 comments

Comments

@friedrichroell
Copy link

Bug Report

Q A
Version 4.0.0

Summary

The wrong Manager can be selected if configured Managers have similar namespaces.

Current behavior

The wrong Manager is returned when two or more Managers with similar namespaces are configured.

Expected behavior

The correct Manager should be selected.

How to reproduce

Configure Manager A with the namespace:

App\Entity

and Manager B with:

App\EntityFoo


MappingDriverChain::isTransient will select A for a B-configured Manager because:

str_starts_with($className, $namespace) returns true for both.

A solution could be to find the best match rather than selecting the first driver that matches the namespace prefix.

Solution proposal

Find the best namespace overlap:

public function isTransient(string $className): bool
{
    $driver = $this->findBestMatchingDriver($className) ?? $this->defaultDriver;

    return $driver ? $driver->isTransient($className) : true;
}

private function findBestMatchingDriver(string $className): ?MappingDriver
{
    $bestDriver = null;
    $maxOverlap = 0;

    foreach ($this->drivers as $namespace => $driver) {
        if ( ! str_starts_with($className, $namespace)) {
            continue;
        }
        
        $currentOverlap = similar_text($className, $namespace);
        
        if ($currentOverlap > $maxOverlap) {
            $maxOverlap = $currentOverlap;
            $bestDriver = $driver;
        }
    }

    return $bestDriver;
}

I could create a pull request with tests if this approach is heading in the right direction. A huge thank you to all contributors!

@greg0ire
Copy link
Member

greg0ire commented Nov 4, 2024

Wouldn't doing something similar as

if (! str_starts_with($className, $prefix . '\\')) {
achieve the same result?

Also, does this affect 4.0.0 only, or should it be contributed to 3.x as a bugfix?

@friedrichroell
Copy link
Author

friedrichroell commented Nov 4, 2024

What about this case? (although this might be a misconfiguration in the first place)

Class: App\Entity\Foo\Bar

Namespace Driver A
App\Entity
Namespace Driver B
App\Entity\Foo

It will match both even with an appended backslash

--

This should be contributed to 3.x as well

@greg0ire
Copy link
Member

greg0ire commented Nov 4, 2024

This case does not look realistic… I mean who would nest namespaces?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants