SonataDoctrineMongoDBAdminBundle
SonataDoctrineMongoDBAdminBundle copied to clipboard
sonata-project
Implement ProxyResolverInterface
Subject
I am targeting this branch, because this feature is backwards compatible.
Implements ODM part of solution #7906 at Sonata Admin for #7905 at Sonata Admin.
Changelog
### Added
- ModelManager now implements ProxyResolverInterface
Phpstan is failing @antonymous.
And what if the class is not a document ?
IMHO, the getRealClass method should work with everyclass
getRealClass(new \DateTime()) => \DateTime (no metada)
getRealClass(new ProductDocument()) => ProductDocument (found in the metadata)
Is there something like a MetadataNotFound exception to catch ?
Phpstan is failing @antonymous.
Fixed.
And what if the class is not a document ?
IMHO, the
getRealClassmethod should work with everyclassgetRealClass(new \DateTime()) => \DateTime (no metada) getRealClass(new ProductDocument()) => ProductDocument (found in the metadata)Is there something like a
MetadataNotFoundexception to catch ?
Before metadata search there is a document manager search. If failed, then RuntimeException will be thrown (exactly as in ORM-version):
$dm = $this->registry->getManagerForClass($class);
if (!$dm instanceof DocumentManager) {
throw new \RuntimeException(sprintf('No document manager defined for class %s', $class));
}
Of cause we could catch that and return class name of the passed object. But should we? Shouldn't model manager be concerned exclusively with "models" and fail hard if some other class provided, because it's very wrong?
What do we do, @VincentLanglet?
Of cause we could catch that and return class name of the passed object. But should we? Shouldn't model manager be concerned exclusively with "models" and fail hard if some other class provided, because it's very wrong?
What do we do, @VincentLanglet?
When I created the interface ProxyResolverInterface, I didn't make it generic https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Model/ProxyResolverInterface.php#L16-L24
If you want to only restrict to the managed object you should do
/**
* @phpstan-template T
*/
interface ProxyResolverInterface
{
/**
* @phpstan-param T|Proxy<T>
* @phpstan-return class-string<T>
*/
public function getRealClass(object $object): string;
}
I didn't like it because to me, if I'm using this method, it's because I don't know what I'm manipulating. That's why I'm calling getRealClass. And since I'm don't really know what I'm manipulating, I can't be sure too it's managed by the modelManager. And then I don't want to get exception anytime I'm passing something which is not managed.
To me it seems smoother to have the following behavior
- Not managed ? Just return getclass
- Managed but it's a proxy ? return the real class
- Managed but it's not a proxy ? return getclass
Why doing less when you can support everything ?
@VincentLanglet I guess I just fail to see for Sonata Admin the value of ProxyResolverInterface as a contract separate from ModelManagerInterface. Anyway, I added the test and the code needed to pass it.
@VincentLanglet I guess I just fail to see for Sonata Admin the value of ProxyResolverInterface as a contract separate from ModelManagerInterface. Anyway, I added the test and the code needed to pass it.
Yeah, I agree we could have make ProxyResolverInterface extending ModelManagerInterface. But since that wasn't really needed... I hope I won't regret it because changing now would be a BC-break ^^'
The test failing https://github.com/sonata-project/SonataDoctrineMongoDBAdminBundle/runs/7846998341?check_suite_focus=true might be a false positive. Let's just remove the extra phpdoc ; and I'll wait for @franmomu to take a look since he works with mongo.
