Le ven. 12 juil. 2024 à 01:40, Benjamin Außenhofer <kont...@beberlei.de> a écrit :
> > > Am 11.07.2024, 20:31:44 schrieb Tim Düsterhus <t...@bastelstu.be>: > >> Hi >> >> On 7/11/24 10:32, Nicolas Grekas wrote: >> >> > Many things are already possible in userland. That does not always mean >> >> > that the cost-benefit ratio is appropriate for inclusion in core. I get >> >> > behind the two examples in the “About Lazy-Loading Strategies” section, >> >> > but I'm afraid I still can't wrap my head why I would want an object >> >> > that makes itself lazy in its own constructor: I have not yet seen a >> >> > real-world example. >> >> > >> >> >> Keeping this capability for userland is not an option for me as it would >> >> mostly defeat my goal, which is to get rid of any userland code on this >> >> topic (and is achieved by the RFC). >> >> >> Here is a real-world example: >> >> >> https://github.com/doctrine/DoctrineBundle/blob/2.12.x/src/Repository/LazyServiceEntityRepository.php >> >> >> This class currently uses a poor-man's implementation of lazy objects and >> >> would greatly benefit from resetAsLazyGhost(). >> >> >> >> Sorry, I was probably a little unclear with my question. I was not >> specifically asking if anyone did that, because I am fairly sure that >> everything possible has been done before. >> >> I was interested in learning why I would want to promote a >> "LazyServiceEntityRepository" instead of the user of my library just >> making the "ServiceEntityRepository" lazy themselves. >> >> I understand that historically making the "ServiceEntityRepository" lazy >> yourself would have been very complicated, but the new RFC makes this >> super easy. >> >> So based on my understanding the "LazyServiceEntityRepository" >> (c|sh)ould be deprecated with the reason that PHP 8.4 provides all the >> necessary tools to do it yourself, no? That would also match your goal >> of getting rid of userland code on this topic. >> >> To me this is what the language evolution should do: Enable users to do >> things that previously needed to be provided by userland libraries, >> because they were complicated and fragile, not enabling userland >> libraries to simplify things that they should not need to provide in the >> first place because the language already provides it. >> > > I agree with Tim here, the Doctrine ORM EntityRepository plus Symfony > Service Entity Repository extension are not a necessary real world case > that would require this RFC to include a way for classes to make > themselves lazy. > > I took the liberty at rewriting the code of DefaultRepositoryFactory > (Doctrine code itself) and ContainerRepositoryFactory in a way to make the > repositories lazy without needing resetAsLazy, just > $reflector->createLazyProxy. In case of the second the > LazyServiceEntityRepository class could be deleted. > > https://gist.github.com/beberlei/80d7a3219b6a2a392956af18e613f86a > Thanks for the example. Here is the quick diff: Before: return $this->managedRepositories[$repositoryHash] = new $repositoryClassName($entityManager, $metadata); After: $reflector = new ReflectionClass($repositoryClassName); return $this->managedRepositories[$repositoryHash] = $reflector->newLazyProxy(static fn () => new $repositoryClassName($entityManager, $metadata); But in the case of repositories registered as services, this code path won't be reached for the class we're talking about (we'll take the "$container->has()" code path). Yet we need to account for the history of such a piece of code: the service repository class I shared uses laziness internally because this was the most effective way to handle circular references seamlessly without breaking any of the currently working consumer apps. With refactoring, we can fix all design issues in all softwares. Yet that's too theoretical and this is missing the point IMHO. What we want to provide is a complete coverage of the lazy object domain. Simplifying too much can lead to an incomplete solution that'd be too restrictive. Please let me know if this is not how it works or can work or if my > reasoning is flawed. > > Unless you have no way of getting to the „new $object“ in the code, there > is always a way to just use newLazy*. And when a library does not expose > new $object to you to override, then that is an architectural choice (and > maybe flaw that you have to accept). > I still think not having the reset* methods would greatly simplify this RFC > and would allow to force more constraints, have less footguns. > > For example we could simplify the API of newLazyProxy to not receive a > $factory that can arbitrarily create and get objects from somewhere, but > also initializer and always force the lazy object to be an instance created > by newInstanceWithoutConstructor. > > You said in a previous mail about reset*() > > From a technical pov, this is just a different flavor of the same code >> infrastructure, so this is pretty aligned with the rest of the proposed >> API. >> > > We are not specifically considering the technical POV, but even more > importantly the user facing API. And this just adds to the surface of the > API a lot of things that are pushing only a 1-5% edge case. > I share this angle: figuring out the minimal API to cover the domain. Yet I showed reset methods proved useful already (see also my example about refreshable fixtures) - not only as "just tools", but more importantly from a theoretical aspect as a useful API to cover all situations that userland will encounter (and does already). If we don't have them, I know I will have to maintain a library that already provides this resetting capability because there are valid use cases for them. That'd be a failure to address the needs of userland on the topic to me. Cheers, Nicolas
