refactor to register the callabcks directly on the bugsnag service, rather than through the factory

Author: Sam Stevens

DependencyInjection/ClientFactory.php

@@ -114,13 +114,6 @@ class ClientFactory
      */
     protected $filters;
 
-    /**
-     * Custom callbacks to register on the client.
-     *
-     * @var callable[]
-     */
-    protected $customCallbacks = [];
-
     /**
      * Create a new client factory instance.
      *
@@ -176,26 +169,6 @@ public function __construct(
         $this->filters = $filters;
     }
 
-    /**
-     * Add a custom callback to be registered on the client.
-     *
-     * @param callable|array $callable
-     */
-    public function addCallback($callable)
-    {
-        // If the parameter is not a callable (if the method name does not exist on the class for example),
-        // then throw an exception informing the user
-        if (!is_callable($callable)) {
-            throw new \RuntimeException(sprintf(
-                'Could not add Bugsnag callback. Class %s does not contain the "%s" method.',
-                get_class($callable[0]),
-                $callable[1]
-            ));
-        }
-
-        $this->customCallbacks[] = $callable;
-    }
-
     /**
      * Make a new client instance.
      *
@@ -237,10 +210,6 @@ public function make()
             $client->setFilters($this->filters);
         }
 
-        foreach ($this->customCallbacks as $callable) {
-            $client->registerCallback($callable);
-        }
-
         return $client;
     }
 

DependencyInjection/Compiler/CallbackRegisteringPass.php

@@ -8,31 +8,31 @@
 
 class CallbackRegisteringPass implements CompilerPassInterface
 {
-    const FACTORY_SERVICE_NAME = 'bugsnag.factory';
+    const BUGSNAG_SERVICE_NAME = 'bugsnag';
     const TAG_NAME = 'bugsnag.callback';
 
     /**
      * @param \Symfony\Component\DependencyInjection\ContainerBuilder $container
      */
     public function process(ContainerBuilder $container)
     {
-        if (!$container->has(self::FACTORY_SERVICE_NAME)) {
+        if (!$container->has(self::BUGSNAG_SERVICE_NAME)) {
             return;
         }
 
-        // Get the Bugsnag factory service
-        $bugsnagFactory = $container->findDefinition(self::FACTORY_SERVICE_NAME);
+        // Get the Bugsnag service
+        $bugsnag = $container->findDefinition(self::BUGSNAG_SERVICE_NAME);
 
         // Get all services tagged as a callback
         $callbackServices = $container->findTaggedServiceIds(self::TAG_NAME);
 
-        // Add each callback to the factory service via an addCallback call
+        // 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';
-                $bugsnagFactory->addMethodCall('addCallback', [[new Reference($id), $method]]);
+                $bugsnag->addMethodCall('registerCallback', [[new Reference($id), $method]]);
             }
         }
     }

Tests/DependencyInjection/Compiler/CallbackRegisteringPassTest.php

@@ -2,8 +2,8 @@
 
 namespace Bugsnag\BugsnagBundle\Tests\DependencyInjection\Compiler;
 
-use Bugsnag\BugsnagBundle\DependencyInjection\ClientFactory;
 use Bugsnag\BugsnagBundle\DependencyInjection\Compiler\CallbackRegisteringPass;
+use Bugsnag\Client;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
@@ -15,8 +15,8 @@ public function testTaggedCallbacksAreAddedToFactoryServiceDefinition()
     {
         $containerBuilder = new ContainerBuilder();
 
-        $factory = new Definition(ClientFactory::class);
-        $containerBuilder->setDefinition(CallbackRegisteringPass::FACTORY_SERVICE_NAME, $factory);
+        $bugsnag = new Definition(Client::class);
+        $containerBuilder->setDefinition(CallbackRegisteringPass::BUGSNAG_SERVICE_NAME, $bugsnag);
 
         $taggedCallbackOne = new Definition();
         $taggedCallbackOne->addTag(CallbackRegisteringPass::TAG_NAME);
@@ -29,10 +29,10 @@ public function testTaggedCallbacksAreAddedToFactoryServiceDefinition()
         $pass = new CallbackRegisteringPass();
         $pass->process($containerBuilder);
 
-        $this->assertSame(2, count($factory->getMethodCalls()));
-        $this->assertSame('addCallback', $factory->getMethodCalls()[0][0]);
-        $this->assertEquals([new Reference('callback_1'), 'registerCallback'], $factory->getMethodCalls()[0][1][0]);
-        $this->assertSame('addCallback', $factory->getMethodCalls()[1][0]);
-        $this->assertEquals([new Reference('callback_2'), 'customMethod'], $factory->getMethodCalls()[1][1][0]);
+        $this->assertSame(2, count($bugsnag->getMethodCalls()));
+        $this->assertSame('registerCallback', $bugsnag->getMethodCalls()[0][0]);
+        $this->assertEquals([new Reference('callback_1'), 'registerCallback'], $bugsnag->getMethodCalls()[0][1][0]);
+        $this->assertSame('registerCallback', $bugsnag->getMethodCalls()[1][0]);
+        $this->assertEquals([new Reference('callback_2'), 'customMethod'], $bugsnag->getMethodCalls()[1][1][0]);
     }
 }