Deprecate Drupal ProxyBuilder in favor of Symfony lazy services [#3396928]

To check if your lazy service works you can check the interface of the received object:
dump(class_implements($service));
// the output should include "Symfony\Component\VarExporter\LazyObjectInterface"

With this MR checked out and after a container rebuild:

> class_implements(\Drupal::service('cron'));
= [
    "Drupal\Core\CronInterface" => "Drupal\Core\CronInterface",
  ]

ie. I don't think the ghost objects are working. I suspect our container and/or dumper need to add support for them, somewhere.

Comment #7

spokje

Dutch

commented 27 October 2023 at 12:43

Status	File	Size
new	do-not-try-this-at-home.patch	944 bytes
new	LazyStateGhost.jpg	358.33 KB

@longwave: Hmm, at the very least #6 points out that this MR is missing test coverage.

ie. I don't think the ghost objects are working. I suspect our container and/or dumper need to add support for them, somewhere.

I'm a tad more optimistic.

The attached (incredibly nasty hacky do-not-try-this-at-home) test patch on top of this MR shows that the following service-IDs implement Symfony\Component\VarExporter\LazyObjectInterface:

1) Drupal\Tests\Core\LazyService\LazyServiceTest::testBuildComplexMethod
page_cache_response_policy
config.installer
cron
module_installer
content_uninstall_validator
required_module_uninstall_validator
module_required_by_themes_uninstall_validator
database_driver_uninstall_validator
plugin.cache_clearer
paramconverter.menu_link
lock.persistent
router.dumper
router.builder
paramconverter.configentity_admin
bare_html_page_renderer
batch.storage
file.mime_type.guesser
file.mime_type.guesser.extension
Drupal\Core\PageCache\ResponsePolicyInterface
Drupal\Core\Config\ConfigInstallerInterface
Drupal\Core\CronInterface
Drupal\Core\Extension\ModuleInstallerInterface
Drupal\Core\Plugin\CachedDiscoveryClearerInterface
Drupal\Core\Routing\MatcherDumperInterface
Drupal\Core\Routing\RouteBuilderInterface
Drupal\Core\Render\BareHtmlPageRendererInterface
Drupal\Core\Batch\BatchStorageInterface

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-''
+'certainly not equal'

That list matches on first glance what I expected.
Can the difference between your and my findings be explained by looking at different times in the Container lifecycle maybe?

As a sidenote: I also didn't believe this to work, but putting breakpoints in tests using the lazy services indeed shows the presence of LazyObjectState and [ActualClassName]Ghost[hashID] classes. (See https://www.drupal.org/files/issues/2023-10-27/LazyStateGhost.jpg)
Of course, that's no proof at all.

Comment #8

longwave

he/him

English

commented 27 October 2023 at 13:03

In kernel tests, the container is a \Drupal\Core\DependencyInjection\ContainerBuilder - you can see this by dumping $container::class in that sample test. This is a thin layer over Symfony's ContainerBuilder which does support lazy services.

However at runtime the container is a \Drupal\Core\DependencyInjection\Container which is not based on Symfony at all, it only implements their interface. The ContainerBuilder is dumped and stored in cache by the rebuild process, and reloaded from there as a normal Container on subsequent page loads.

IIRC we were bitten by this before when adding service deprecations: #3215611: Service deprecations are only triggered on container build,not ::get()

#2529516: Decouple tests from relying that $container is a container builder would make this distinction better for kernel tests.

Comment #9

longwave

he/him

English

commented 27 October 2023 at 13:06

Not that this should put us off; the fact the kernel tests are passing here are a good sign that these lazy services are working, just they are no longer lazy at runtime or in functional tests. But if we add support for them to the dumper/container then they should be usable at runtime too!

Comment #10

longwave

he/him

English

commented 27 October 2023 at 13:07

Comment #11

spokje

Dutch

commented 27 October 2023 at 13:24

Assigned:	Unassigned	» spokje
Status:	Needs review	» Needs work

Thanks @longwave, that's the reason I always danced around the container bit, never got a firm grasp of it.

The explanation helped, still got hope this is going to work and shouldn't be that hard.
(It seems the actual container is fed the drupal proxy classes instead of the actual services, let's see if that can be altered quickly)
#FamousLastWords
#OnHisWayToTheDrawingBoard

Final question (for now):

With this MR checked out and after a container rebuild:

> class_implements(\Drupal::service('cron'));
= [
"Drupal\Core\CronInterface" => "Drupal\Core\CronInterface",
]

How, what, and where did you do that class_implements?

Also un-parenting, since this isn't directly SF6.4 related.

Comment #12

longwave

he/him

English

commented 27 October 2023 at 13:33

I use DDEV for all Drupal dev work; drush provides a shell that bootstraps Drupal so you can run PHP code interactively:

$ ddev drush cr
 [success] Cache rebuild complete.

$ ddev drush php
Psy Shell v0.11.9 (PHP 8.1.23 — cli) by Justin Hileman
Drupal 10 (Drupal 11.0-dev)
> class_implements(\Drupal::service('cron'));
= [
    "Drupal\Core\CronInterface" => "Drupal\Core\CronInterface",
  ]

Comment #13

spokje

Dutch

commented 27 October 2023 at 13:37

*hat is tipped*
Much obliged, kind sir.

Comment #14

joachim commented 27 October 2023 at 15:29

> In kernel tests, the container is a \Drupal\Core\DependencyInjection\ContainerBuilder - you can see this by dumping $container::class in that sample test. This is a thin layer over Symfony's ContainerBuilder which does support lazy services.

Aside: is that documented somewhere?