Problem/Motivation
GroupPurlProvider::create() receives $configuration, $plugin_id and $plugin_definition but doesn't pass them to the constructor, and __construct() never calls PluginBase::__construct(). The plugin ID is never set, so getProviderId(), which returns getPluginId(), returns NULL. Subclasses that inherit create() have the same problem.
PURL's RequestSubscriber::onRequest() stores matched modifiers under the provider ID ($matches[$provider->getProviderId()]). When two providers both return NULL, their matches share the key '', and the second overwrites the first before hook_purl_matched_modifiers_alter() runs.
We hit this with a second provider that extends GroupPurlProvider to handle per-group custom domains. On a group's custom domain, a path starting with another group's alias matched both providers. The path-prefix match silently replaced the domain match, so the wrong group was activated.
Steps to reproduce
- Enable purl and group_purl.
- Run
drush php:eval 'var_dump(\Drupal::service("purl.plugin.provider_manager")->createInstance("group_purl_provider")->getProviderId());' - Expected:
string(19) "group_purl_provider". Actual:NULL.
Proposed resolution
- 3.0.x: Pass the plugin arguments from
create()to the constructor and callparent::__construct(), following the standard plugin pattern. The patch updatesGroupPurlProviderTestfor the new constructor, adds a test thatcreate()sets the plugin ID, and fixes the constructor docblock, which namesGroupDomainPurlProvider. - 2.2.x: Set the three plugin properties on the new instance in
create()and leave the constructor alone. Changing the constructor signature in a minor release would break subclasses that callparent::__construct()with the current arguments.
API changes
3.0.x: GroupPurlProvider::__construct() takes three new leading parameters: $configuration, $plugin_id and $plugin_definition. Subclasses that call parent::__construct() need updating. 2.2.x: none.
| Comment | File | Size | Author |
|---|---|---|---|
| group_purl-provider-plugin-id-3.0.x.patch | 3.79 KB | rbrandon | |
| group_purl-provider-plugin-id-2.2.x.patch | 1.03 KB | rbrandon |
Comments
Comment #3
freelock@rbrandon I know the tests were failing due to fixes needed to be incorporated in Purl -- looks like a couple of those are now fixed, but the big one is #3608029: 3.x: Drupal 11.4 compatibility fixes (purl subrequest visibility + RouteProvider constructor signature) - and now it looks from the most recent run there are new reasons the tests are failing. I'm wondering if you can post a 3.x of Purl with the changes I put there, and then we can clean up the tests here and publish a release?
Comment #4
rbrandon commentedOk, sounds good I saw that they were already failing but didn't realize there was a further regression.