-
-
Notifications
You must be signed in to change notification settings - Fork 229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add priority field to processor tags #455
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,40 +29,65 @@ public function process(ContainerBuilder $container) | |
return; | ||
} | ||
|
||
$processors = []; | ||
|
||
foreach ($container->findTaggedServiceIds('monolog.processor') as $id => $tags) { | ||
foreach ($tags as $tag) { | ||
if (!empty($tag['channel']) && !empty($tag['handler'])) { | ||
throw new \InvalidArgumentException(sprintf('you cannot specify both the "handler" and "channel" attributes for the "monolog.processor" tag on service "%s"', $id)); | ||
if (!isset($tag['priority'])) { | ||
$tag['priority'] = 0; | ||
} | ||
|
||
if (!empty($tag['handler'])) { | ||
$definition = $container->findDefinition(sprintf('monolog.handler.%s', $tag['handler'])); | ||
$parentDef = $definition; | ||
while (!$parentDef->getClass() && $parentDef instanceof ChildDefinition) { | ||
$parentDef = $container->findDefinition($parentDef->getParent()); | ||
} | ||
$class = $container->getParameterBag()->resolveValue($parentDef->getClass()); | ||
if (!method_exists($class, 'pushProcessor')) { | ||
throw new \InvalidArgumentException(sprintf('The "%s" handler does not accept processors', $tag['handler'])); | ||
} | ||
} elseif (!empty($tag['channel'])) { | ||
if ('app' === $tag['channel']) { | ||
$definition = $container->getDefinition('monolog.logger'); | ||
} else { | ||
$definition = $container->getDefinition(sprintf('monolog.logger.%s', $tag['channel'])); | ||
} | ||
} else { | ||
$definition = $container->getDefinition('monolog.logger_prototype'); | ||
} | ||
$processors[] = [ | ||
'id' => $id, | ||
'tag' => $tag, | ||
]; | ||
} | ||
} | ||
|
||
// Sort by priority so that higher-prio processors are added last. | ||
// The effect is the monolog will call the higher-prio processors first | ||
usort( | ||
$processors, | ||
function (array $left, array $right) { | ||
return $left['tag']['priority'] <=> $right['tag']['priority']; | ||
} | ||
Comment on lines
+49
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion, however the PriorityTaggedServiceTrait can´t be used since it will require some changes because the Trait sorts it ascending (as expected). But monologs pushProcessor actually array_shifts instead of adding it on top of the stack, so I need a descending sort. Which is clarified in the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could still use the trait to get the sorted services and reverse the list before adding method calls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Could we resolve this comment and merge this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, I'm sorry for my lack of updates, feel free to take over. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
); | ||
|
||
if (!empty($tag['method'])) { | ||
$processor = [new Reference($id), $tag['method']]; | ||
foreach ($processors as $processor) { | ||
$tag = $processor['tag']; | ||
$id = $processor['id']; | ||
|
||
if (!empty($tag['channel']) && !empty($tag['handler'])) { | ||
throw new \InvalidArgumentException(sprintf('you cannot specify both the "handler" and "channel" attributes for the "monolog.processor" tag on service "%s"', $id)); | ||
} | ||
|
||
if (!empty($tag['handler'])) { | ||
$definition = $container->findDefinition(sprintf('monolog.handler.%s', $tag['handler'])); | ||
$parentDef = $definition; | ||
while (!$parentDef->getClass() && $parentDef instanceof ChildDefinition) { | ||
$parentDef = $container->findDefinition($parentDef->getParent()); | ||
} | ||
$class = $container->getParameterBag()->resolveValue($parentDef->getClass()); | ||
if (!method_exists($class, 'pushProcessor')) { | ||
throw new \InvalidArgumentException(sprintf('The "%s" handler does not accept processors', $tag['handler'])); | ||
} | ||
} elseif (!empty($tag['channel'])) { | ||
if ('app' === $tag['channel']) { | ||
$definition = $container->getDefinition('monolog.logger'); | ||
} else { | ||
// If no method is defined, fallback to use __invoke | ||
$processor = new Reference($id); | ||
$definition = $container->getDefinition(sprintf('monolog.logger.%s', $tag['channel'])); | ||
} | ||
$definition->addMethodCall('pushProcessor', [$processor]); | ||
} else { | ||
$definition = $container->getDefinition('monolog.logger_prototype'); | ||
} | ||
|
||
if (!empty($tag['method'])) { | ||
$processor = [new Reference($id), $tag['method']]; | ||
} else { | ||
// If no method is defined, fallback to use __invoke | ||
$processor = new Reference($id); | ||
} | ||
$definition->addMethodCall('pushProcessor', [$processor]); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the
priority
option also be added to#[AsMonologProcessor]
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should be able to add it without a problem, thanks for the suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: 1797