Download & Extend

Symfony update exposes fatal error during installation due to 'access_manager' service depending on 'request'

Project:Drupal core
Version:8.x-dev
Component:routing system
Category:bug report
Priority:major
Assigned:Unassigned
Status:needs work
Issue tags:WSCCI

Issue Summary

Spin-off from #1894002: Update vendor libraries and pin them to specific versions in composer.json.

That issue is trying to update to Symfony 2.2-BETA2. That Symfony update includes a fix to ContainerBuilder to throw an error upon trying to create an uninstantiated synthetic service. Throwing an error in that case is the correct action, and the pre-beta2 version of ContainerBuilder in Drupal HEAD is wrong to not be throwing that error.

That fix, however, exposes a Drupal bug, as demonstrated by the test failures caused by this patch.

The bug is that the 'access_manager' service depends on 'request'. However, during installation, drupal_flush_all_caches() is called which calls drupal_container()->get('router.builder')->rebuild(); which dispatches events listened to by the 'access_subscriber', which depends on 'access_manager'. However, since this is during installation, there is no 'request'.

Possible solutions:
1. Fix 'access_manager' to not have a hard dependency on 'request'. Its methods that get called during the router rebuild events don't need the request for anything. Only its other methods do.
2. Make the installer set a mock 'request' service instance in the DIC.

I think the first one would be cleaner. Feedback?

AttachmentSizeStatusTest resultOperations
access_manager_request_dependency.patch910 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details | Re-test

Comments

#1

Status:needs review» needs work

The last submitted patch, access_manager_request_dependency.patch, failed testing.

#2

thanks for debugging this @effulgentsia! tagging appropriately.

#3

I agree. The access checker can take the request as a parameter at runtime, eliminating the dependency. That's probably my fault in the first place. :-)

#4

Status:needs work» needs review

Nice and simple.

AttachmentSizeStatusTest resultOperations
1907750-access-request-dependency.patch3.13 KBIdlePASSED: [[SimpleTest]]: [MySQL] 49,076 pass(es).View details | Re-test

#5

Follow up patch after #1894002: Update vendor libraries and pin them to specific versions in composer.json was committed. This patch reverts the hunk which temporarily removed the setSynthetic(TRUE) on the request service.

AttachmentSizeStatusTest resultOperations
1907750-access-request-dependency-5.patch3.71 KBIdleFAILED: [[SimpleTest]]: [MySQL] 48,342 pass(es), 0 fail(s), and 79 exception(s).View details | Re-test

#6

Status:needs review» needs work

The last submitted patch, 1907750-access-request-dependency-5.patch, failed testing.

#7

"You have requested a synthetic service ("request"). The DIC does not know how to construct this service." that's the error I was getting at #1894002: Update vendor libraries and pin them to specific versions in composer.json before I removed the synthetic flag from the request service. Back then I was getting it straight up in the installer. Now it seems to install ok on the testbot and also on my machine (tested earlier before uploading this patch). So looks like there might be some other services depending on 'request' besides 'access_manager' ?

nobody click here