-
Notifications
You must be signed in to change notification settings - Fork 1
Refactored to profiler service #3
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
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,88 +8,65 @@ | |
| namespace Link0\ProfilerBundle\EventListener; | ||
|
|
||
| use Link0\Profiler\PersistenceHandler; | ||
| use Link0\Profiler\Profiler; | ||
| use Symfony\Component\Console\Event\ConsoleTerminateEvent; | ||
| use Symfony\Component\HttpKernel\Event\GetResponseEvent; | ||
| use Symfony\Component\Console\Event\ConsoleCommandEvent; | ||
| use Symfony\Component\HttpKernel\TerminableInterface; | ||
| use Link0\ProfilerBundle\Service\ProfilingService; | ||
|
|
||
| /** | ||
| * Class ProfilingEventListener | ||
| * | ||
| * Hooks into the symfony event system, and starts and stops the profiler on certain events | ||
| * | ||
| * Start: | ||
| * - console.command | ||
| * - kernel.request | ||
| * Stop: | ||
| * - console.terminate | ||
| * - kernel.terminate | ||
| * You can configure this event listener in your services.yml in the following format | ||
| * | ||
| * services: | ||
| * kernel.listener.link0profilerlistener: | ||
| * class: Link0\ProfilerBundle\EventListener\ProfilingEventListener | ||
| * param: [@profiling_service] | ||
| * tags: | ||
| * - { name: kernel.event_listener, event: console.command, method: start } | ||
| * - { name: kernel.event_listener, event: kernel.request, method: start } | ||
| * - { name: kernel.event_listener, event: console.terminate, method: stop } | ||
| * - { name: kernel.event_listener, event: kernel.terminate, method: stop } | ||
| * | ||
| * @package Link0\ProfilerBundle\EventListener | ||
| */ | ||
| class ProfilingEventListener | ||
| final class ProfilingEventListener | ||
| { | ||
| /** | ||
| * @var \Link0\Profiler\Profiler | ||
|
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. I don't think you need the leading backslash here. |
||
| */ | ||
| private $profiler; | ||
|
|
||
| /** | ||
| * A persistence handler should be fed, since null will disable persisting profiles | ||
| * | ||
| * @param PersistenceHandler $handler | ||
| */ | ||
| public function __construct(PersistenceHandler $handler = null) | ||
| { | ||
| $this->profiler = new Profiler($handler); | ||
| } | ||
|
|
||
| /** | ||
| * @return Profiler | ||
| */ | ||
| public function getProfiler() | ||
| { | ||
| return $this->profiler; | ||
| } | ||
| private $profilingService; | ||
|
|
||
| /** | ||
| * @event console.command | ||
| * @param ConsoleCommandEvent $event | ||
| * @param ProfilingService $profilingService | ||
| */ | ||
| public function onConsoleCommand(ConsoleCommandEvent $event) | ||
| public function __construct(ProfilingService $profilingService) | ||
| { | ||
| $this->getProfiler()->start(); | ||
| $this->profilingService = $profilingService; | ||
| } | ||
|
|
||
| /** | ||
| * @event kernel.request | ||
| * @param GetResponseEvent $event | ||
| * @return ProfilingService | ||
| */ | ||
| public function onKernelRequest(GetResponseEvent $event) | ||
| public function getProfilingService() | ||
|
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. Is this getter necessary? Are the consumers of this class going to need to do anything with the |
||
| { | ||
| if (!$event->isMasterRequest()) { | ||
| return; | ||
| } | ||
|
|
||
| $this->getProfiler()->start(); | ||
| return $this->profilingService; | ||
| } | ||
|
|
||
| /** | ||
| * @event console.terminate | ||
| * @param ConsoleTerminateEvent $event | ||
| * Starts the profiler | ||
| */ | ||
| public function onConsoleTerminate(ConsoleTerminateEvent $event) | ||
| public function start() | ||
| { | ||
| $this->getProfiler()->stop(); | ||
| $this->getProfilingService()->start(); | ||
|
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. When working with internal variables, I use |
||
| } | ||
|
|
||
| /** | ||
| * @event kernel.terminate | ||
| * @param TerminableInterface $event | ||
| * Stops the profiler | ||
| * | ||
| * @return \Link0\Profiler\Profile | ||
| */ | ||
| public function onKernelTerminate(TerminableInterface $event) | ||
| public function stop() | ||
| { | ||
| $this->getProfiler()->stop(); | ||
| return $this->getProfilingService()->stop(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| services: | ||
| kernel.listener.link0profilerlistener: | ||
| class: Link0\ProfilerBundle\EventListener\ProfilingEventListener | ||
| param: [@profiling_service] | ||
| tags: | ||
| - { name: kernel.event_listener, event: console.command, method: onConsoleCommand } | ||
| - { name: kernel.event_listener, event: console.command, method: start } | ||
| - { name: kernel.event_listener, event: kernel.request, method: start } | ||
| - { name: kernel.event_listener, event: console.terminate, method: stop } | ||
| - { name: kernel.event_listener, event: kernel.terminate, method: stop } | ||
| profiling_service: | ||
| class: Link0\ProfilerBundle\Service\ProfilingService |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * ProfilingEventListener.php | ||
| * | ||
| * @author Dennis de Greef <github@link0.net> | ||
| */ | ||
| namespace Link0\ProfilerBundle\Service; | ||
|
|
||
| use Link0\Profiler\PersistenceService; | ||
|
|
||
| /** | ||
| * Class ProfilingService | ||
| * | ||
| * @package Link0\ProfilerBundle\Service | ||
| */ | ||
| final class ProfilingService | ||
| { | ||
| /** | ||
| * @var \Link0\Profiler\PersistenceService | ||
|
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. Don't need leading backslash |
||
| */ | ||
| private $persistenceService; | ||
|
|
||
| /** | ||
| * @var \Link0\Profiler\Profiler | ||
| */ | ||
| private $profiler; | ||
|
|
||
| /** | ||
| * @param PersistenceService $persistenceService | ||
| */ | ||
| public function __construct(PersistenceService $persistenceService = null) | ||
| { | ||
| $this->persistenceService = $persistenceService; | ||
| } | ||
|
|
||
| /** | ||
| * Starts the profiler | ||
| */ | ||
| public function start() | ||
| { | ||
| $this->profiler->start(); | ||
|
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. I guess this gives an error? I don't see where this profiler object comes from. Shouldn't it be injected in the constructor? |
||
| } | ||
|
|
||
| /** | ||
| * @return \Link0\Profiler\Profile | ||
| */ | ||
| public function stop() | ||
| { | ||
| return $this->profiler->stop(); | ||
| } | ||
| } | ||
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.
is this documentation needed in here? Since it just duplicates code that's available in your bundle and thus decreases maintainability.