API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Dependenc...
* Additional tag attributes supported by 'service_collector' only:
* - call: The method name to call on the consumer service. Defaults to
* 'addHandler'. The called method receives two arguments:
* - The handler instance as first argument.
* - Optionally the handler's priority as second argument, if the method
* accepts a second parameter and its name is "priority". In any case,
* all handlers registered at compile time are sorted already.
*
* Example (YAML):
* @code
* tags:
* - { name: service_collector, tag: breadcrumb_builder, call: addBuilder }
* - { name: service_id_collector, tag: theme_negotiator }
* @endcode
*
* Supported handler tag attributes:
* - priority: An integer denoting the priority of the handler. Defaults to 0.
The above is incomplete and a little misleading.
Compare with the actual code:
foreach ($params as $pos => $param) {
if ($param->getClass()) {
$interface = $param->getClass();
}
elseif ($param->getName() === 'id') {
$id_pos = $pos;
}
elseif ($param->getName() === 'priority') {
$priority_pos = $pos;
}
else {
$extra_params[$param->getName()] = $pos;
}
}
...
// Keep track of other tagged handlers arguments.
foreach ($extra_params as $name => $pos) {
$extra_arguments[$id][$pos] = isset($attributes[0][$name]) ? $attributes[0][$name] : $params[$pos]->getDefaultValue();
}
We can see the following:
1. The handler instance must be the first parameter that has a class typehint. Apart from that, its position is not relevant. The docs though say:
> The handler instance as first argument
EDIT: nope, scratch that. Looks like a bug though, as the code has:
$arguments[$interface_pos] = new Reference($id);
just that $interface_pos never gets set to anything other than 0.
2. The priority parameter can be in any position, so long as it is named 'priority'.
3. There can be an 'id' parameter, which gets passed the service ID if present.
4. There can in fact be any number of other parameters, which will be matched up with tag attributes on the collected services, so for example:
* @code
* tags:
* - { name: breadcrumb_builder, cake: chocolate }
will work with:
public function addHandler(MyHandlerInterface $handler, $cake) {
}
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !922
Comments
Comment #2
joachim commentedOops, handler param DOES have to be first. At least for now.
Comment #3
shubhangi1995Comment #4
shubhangi1995There is a patch with changed documentation please review:
Comment #5
shubhangi1995Comment #7
shubhangi1995Comment #8
vacho commentedI solve the problems with standars in patch.
For colaborate with @shubhangi1995 (congrats for contrib to drupal)
Comment #9
joachim commentedThanks for making a patch.
However, the text the patch is adding doesn't really make much sense, and needs work for grammar too, and doesn't seem to cover all the points I mentioned. Additionally, the formatting needs cleaning up, but rather than keep rolling patches I suggest we work on text in comments first, and make a patch once something's finalized.
Comment #11
wolffereast commentedI took a crack at updating the text to match what appears to be occurring:
The processServiceCollectorPass sets the $interface based on the last param that returns a value for getClass():
if ($param->getClass())
It then throws an error if $interface is not set, so the requirement for the Error to throw is not related to the position of the $interface.
Then later it sets the interface_position of the arguments array to a Reference to the $id, which is the key of the array returned by the $container->findTaggedServiceIds. That key, according to the docs, is the tagged service.
foreach ($container->findTaggedServiceIds($tag) as $id => $attributes) {
$arguments[$interface_pos] = new Reference($id);
Then the $priority_pos and $id_pos are set based on their passed order.
As mentioned the $interface_position is always 0, so it is technically possible to accidentally overwrite the Reference($id) in $arguments[0] if either the priority or id are sent through in position 0. Definitely feels like a bug. Along the same lines, the array += operation for the additional arguments could result in a lost argument if one is sent through in position 0 as the $arguments[0] item will take precedence over the $extra_arguments[0].
@joachim should the issue with the interface position be a part of this ticket, or does that feel out of scope?
Comment #12
joachim commented> @joachim should the issue with the interface position be a part of this ticket, or does that feel out of scope?
This is a documentation only issue, so yes, the bug in the code is out of scope. We can file a follow up once this is issue is fixed.
The text is a very good start, but it's not yet saying anything about the method parameter names being matched up to the tag property names.
Comment #13
joachim commentedHow about this:
- The handler instance must be the first method parameter, and it must have a type hint.
- If the method has a parameter named $id, in any position, it will receive the service ID when called.
- If the method has a parameter named $priority, in any position, it will receive the value of the tag's 'priority' attribute.
- Any other method parameters whose names match the name of an attribute of the tag will receive the value of that tag attribute. The order of the method parameters and the order of the service tag attributes do not need to match.
Comment #14
joachim commentedSetting to NR for review of the text.
Comment #19
guilhermevp commentedComment #20
longwave@joachim's suggested text in #13 looks OK to me, let's go with that.
Comment #22
guilhermevp commentedThanks @longwave!
Sending MR, please review!
Comment #23
guilhermevp commentedThanks @joachim, updated merge request!
Comment #24
longwaveThanks for working on this. The updated docs read well to me and the description matches my understanding of the code in
::processServiceCollectorPass().Comment #25
longwaveComment #26
joachim commented> Given $priority is really no different to any other parameters, do we need to call it out with its own bullet point?
I guess I wrote it like that because I was working through the code's functionality. But it makes sense because 'priority' is invented by the service collector system, whereas other tags are arbitrary and would be invented by implementations.
Comment #27
alexpottCommitted and pushed 1e4bd68bb6 to 9.3.x and 37edc2d7ab to 9.2.x. Thanks!