Skip to content

Implement ability for users to register custom callbacks via service definition tags. #36

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Implement ability for users to register custom callbacks via service definition tags. #36

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0768c79
implement custom callbacks and refactor the user setting code into a …
Mar 24, 2017
fc6bf1e
use legacy phpunit test classes for php 5.5.9
Jun 15, 2017
849a2af
CS requests
Jun 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions BugsnagBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Bugsnag\BugsnagBundle;

use Bugsnag\BugsnagBundle\DependencyInjection\Compiler\CallbackRegisteringPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Bundle\Bundle;

class BugsnagBundle extends Bundle
Expand All @@ -12,4 +14,16 @@ class BugsnagBundle extends Bundle
* @return string
*/
const VERSION = '1.0.0';

/**
* {@inheritdoc}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please just copy over the description rather than inherit. Makes it easy for a human to read the code and know what it does straight away.

Copy link
Author

@samdjstevens samdjstevens Apr 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description here would literally be "Builds the bundle." - is this really helpful? Anyone who isn't already familiar with Symfony, and what this file is for, would not really be looking at it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better description would be "Setup the callback registering pass." or something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping.

*
* @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
*
* @return void
*/
public function build(ContainerBuilder $container)
{
$container->addCompilerPass(new CallbackRegisteringPass());
}
}
75 changes: 75 additions & 0 deletions Callbacks/UserSettingCallback.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

namespace Bugsnag\BugsnagBundle\Callbacks;

use Bugsnag\Report;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class UserSettingCallback
{
/**
* The token resolver.
*
* @var \Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface|null
*/
protected $tokens;

/**
* The auth checker.
*
* @var \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface|null
*/
protected $checker;

/**
* @var bool
*/
protected $setUser;

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a description to this doc, similar to the others in the project.

* @param null|\Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface $tokens
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put null on the right of the other type. :)

* @param null|\Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface $checker
* @param bool $setUser
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return void

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping.

public function __construct(
TokenStorageInterface $tokens = null,
AuthorizationCheckerInterface $checker = null,
$setUser = true
) {
$this->tokens = $tokens;
$this->checker = $checker;
$this->setUser = $setUser;
}

/**
* @param \Bugsnag\Report $report
*
* @return void
*/
public function registerCallback(Report $report)
{
// If told to not set the user, or the security services were not passed in
// (not registered in the container), then exit early
if (!$this->setUser || is_null($this->tokens) || is_null($this->checker)) {
return;
}

$token = $this->tokens->getToken();

if (!$token || !$this->checker->isGranted('IS_AUTHENTICATED_REMEMBERED')) {
return;
}

$user = $token->getUser();

if ($user instanceof UserInterface) {
$bugsnagUser = ['id' => $user->getUsername()];
} else {
$bugsnagUser = ['id' => (string) $user];
}

$report->setUser($bugsnagUser);
}
}
98 changes: 16 additions & 82 deletions DependencyInjection/ClientFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,8 @@

use Bugsnag\BugsnagBundle\BugsnagBundle;
use Bugsnag\BugsnagBundle\Request\SymfonyResolver;
use Bugsnag\Callbacks\CustomUser;
use Bugsnag\Client;
use Bugsnag\Configuration as Config;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\User\UserInterface;

class ClientFactory
{
Expand All @@ -20,20 +16,6 @@ class ClientFactory
*/
protected $resolver;

/**
* The token resolver.
*
* @var \Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface|null
*/
protected $tokens;

/**
* The auth checker.
*
* @var \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface|null
*/
protected $checker;

/**
* The api key.
*
Expand All @@ -55,13 +37,6 @@ class ClientFactory
*/
protected $callbacks;

/**
* User detection enabled.
*
* @var bool
*/
protected $user;

/**
* The type.
*
Expand Down Expand Up @@ -149,36 +124,30 @@ class ClientFactory
/**
* Create a new client factory instance.
*
* @param \Bugsnag\BugsnagBundle\Request\SymfonyResolver $resolver
* @param \Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface|null $tokens
* @param \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface|null $checker
* @param string|null $key
* @param string|null $endpoint
* @param bool $callbacks
* @param bool $user
* @param string|null $type
* @param string|null $version
* @param bool $batch
* @param string|null $hostname
* @param bool $code
* @param string|null $strip
* @param string|null $project
* @param string|null $root
* @param string|null $env
* @param string|null $stage
* @param string[]|null $stages
* @param string[]|null $filters
* @param \Bugsnag\BugsnagBundle\Request\SymfonyResolver $resolver
* @param string|null $key
* @param string|null $endpoint
* @param bool $callbacks
* @param string|null $type
* @param string|null $version
* @param bool $batch
* @param string|null $hostname
* @param bool $code
* @param string|null $strip
* @param string|null $project
* @param string|null $root
* @param string|null $env
* @param string|null $stage
* @param string[]|null $stages
* @param string[]|null $filters
*
* @return void
*/
public function __construct(
SymfonyResolver $resolver,
TokenStorageInterface $tokens = null,
AuthorizationCheckerInterface $checker = null,
$key = null,
$endpoint = null,
$callbacks = true,
$user = true,
$type = null,
$version = true,
$batch = null,
Expand All @@ -193,12 +162,9 @@ public function __construct(
array $filters = null
) {
$this->resolver = $resolver;
$this->tokens = $tokens;
$this->checker = $checker;
$this->key = $key;
$this->endpoint = $endpoint;
$this->callbacks = $callbacks;
$this->user = $user;
$this->type = $type;
$this->version = $version;
$this->batch = $batch;
Expand Down Expand Up @@ -228,10 +194,6 @@ public function make()
$client->registerDefaultCallbacks();
}

if ($this->tokens && $this->checker && $this->user) {
$this->setupUserDetection($client, $this->tokens, $this->checker);
}

$this->setupPaths($client, $this->strip, $this->project, $this->root);

$client->setReleaseStage($this->stage ?: ($this->env === 'prod' ? 'production' : $this->env));
Expand Down Expand Up @@ -261,34 +223,6 @@ public function make()
return $client;
}

/**
* Setup user detection.
*
* @param \Bugsnag\Client $client
* @param \Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface $tokens
* @param \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface $checker
*
* @return void
*/
protected function setupUserDetection(Client $client, TokenStorageInterface $tokens, AuthorizationCheckerInterface $checker)
{
$client->registerCallback(new CustomUser(function () use ($tokens, $checker) {
$token = $tokens->getToken();

if (!$token || !$checker->isGranted('IS_AUTHENTICATED_REMEMBERED')) {
return;
}

$user = $token->getUser();

if ($user instanceof UserInterface) {
return ['id' => $user->getUsername()];
}

return ['id' => (string) $user];
}));
}

/**
* Setup the client paths.
*
Expand Down
41 changes: 41 additions & 0 deletions DependencyInjection/Compiler/CallbackRegisteringPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Bugsnag\BugsnagBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;

class CallbackRegisteringPass implements CompilerPassInterface
{
const BUGSNAG_SERVICE_NAME = 'bugsnag';
const TAG_NAME = 'bugsnag.callback';

/**
* @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
*
* @return void
*/
public function process(ContainerBuilder $container)
{
if (!$container->has(self::BUGSNAG_SERVICE_NAME)) {
return;
}

// Get the Bugsnag service
$bugsnag = $container->findDefinition(self::BUGSNAG_SERVICE_NAME);

// Get all services tagged as a callback
$callbackServices = $container->findTaggedServiceIds(self::TAG_NAME);

// Register each callback on the bugsnag service
foreach ($callbackServices as $id => $tags) {
foreach ($tags as $attributes) {
// Get the method name to call on the service from the tag definition,
// defaulting to registerCallback
$method = isset($attributes['method']) ? $attributes['method'] : 'registerCallback';
$bugsnag->addMethodCall('registerCallback', [[new Reference($id), $method]]);
}
}
}
}
12 changes: 9 additions & 3 deletions Resources/config/services.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@ services:
class: '%bugsnag.factory%'
arguments:
- '@bugsnag.resolver'
- '@?security.token_storage'
- '@?security.authorization_checker'
- '%bugsnag.api_key%'
- '%bugsnag.endpoint%'
- '%bugsnag.callbacks%'
- '%bugsnag.user%'
- '%bugsnag.app_type%'
- '%bugsnag.app_version%'
- '%bugsnag.batch_sending%'
Expand Down Expand Up @@ -39,3 +36,12 @@ services:
- { name: kernel.event_listener, event: kernel.request, method: onKernelRequest, priority: 256 }
- { name: kernel.event_listener, event: kernel.exception, method: onKernelException, priority: 128 }
- { name: kernel.event_listener, event: console.exception, method: onConsoleException, priority: 128 }

bugsnag.callbacks.user_setting:
class: Bugsnag\BugsnagBundle\Callbacks\UserSettingCallback
arguments:
- '@?security.token_storage'
- '@?security.authorization_checker'
- '%bugsnag.user%'
tags:
- { name: 'bugsnag.callback' }
Loading