Skip to content

Commit 89a3854

Browse files
stayallivestayallive
authored
Resolve integrations from container (#239)
* Remove duplicated test checks * Resolve integrations from the Laravel container * Add test for simple instance in configuration * Remove usage of `expectException` * Use make not get to resolve from container * Add helper for retrieving the user configuration * Add changelog entry
1 parent 55506de commit 89a3854

7 files changed

+132
-47
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
- Resolve `integrations` option from the container (#239)
6+
37
## 1.0.2
48

59
- Track Artisan command invocation in breadcrumb (#232)

‎src/Sentry/Laravel/ServiceProvider.php‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Sentry\ClientBuilder;
77
use Illuminate\Log\LogManager;
88
use Laravel\Lumen\Application as Lumen;
9+
use Sentry\Integration\IntegrationInterface;
910
use Illuminate\Foundation\Application as Laravel;
1011
use Illuminate\Support\ServiceProvider as IlluminateServiceProvider;
1112

@@ -51,7 +52,7 @@ public function register(): void
5152

5253
$this->mergeConfigFrom(__DIR__ . '/../../../config/sentry.php', static::$abstract);
5354

54-
$this->configureAndRegisterClient($this->app['config'][static::$abstract]);
55+
$this->configureAndRegisterClient($this->getUserConfig());
5556

5657
if (($logManager = $this->app->make('log')) instanceof LogManager) {
5758
$logManager->extend('sentry', function ($app, array $config) {
@@ -65,7 +66,7 @@ public function register(): void
6566
*/
6667
protected function bindEvents(): void
6768
{
68-
$userConfig = $this->app['config'][static::$abstract];
69+
$userConfig = $this->getUserConfig();
6970

7071
$handler = new EventHandler($this->app->events, $userConfig);
7172

@@ -95,7 +96,7 @@ protected function configureAndRegisterClient(): void
9596
{
9697
$this->app->singleton(static::$abstract, function () {
9798
$basePath = base_path();
98-
$userConfig = $this->app['config'][static::$abstract];
99+
$userConfig = $this->getUserConfig();
99100

100101
// We do not want this setting to hit our main client because it's Laravel specific
101102
unset(
@@ -110,9 +111,11 @@ protected function configureAndRegisterClient(): void
110111
'prefixes' => [$basePath],
111112
'project_root' => $basePath,
112113
'in_app_exclude' => [$basePath . '/vendor'],
113-
'integrations' => [new Integration],
114114
],
115-
$userConfig
115+
$userConfig,
116+
[
117+
'integrations' => $this->getIntegrations(),
118+
]
116119
);
117120

118121
$clientBuilder = ClientBuilder::create($options);
@@ -132,11 +135,45 @@ protected function configureAndRegisterClient(): void
132135
*/
133136
protected function hasDsnSet(): bool
134137
{
135-
$config = $this->app['config'][static::$abstract];
138+
$config = $this->getUserConfig();
136139

137140
return !empty($config['dsn']);
138141
}
139142

143+
/**
144+
* Resolve the integrations from the user configuration with the container.
145+
*
146+
* @return array
147+
*/
148+
private function getIntegrations(): array
149+
{
150+
$integrations = [new Integration];
151+
152+
$userIntegrations = $this->getUserConfig()['integrations'] ?? [];
153+
154+
foreach ($userIntegrations as $userIntegration) {
155+
if ($userIntegration instanceof IntegrationInterface) {
156+
$integrations[] = $userIntegration;
157+
} elseif (\is_string($userIntegration)) {
158+
$integrations[] = $this->app->make($userIntegration);
159+
} else {
160+
throw new \RuntimeException('Sentry integrations should either be a container reference or a instance of `\Sentry\Integration\IntegrationInterface`.');
161+
}
162+
}
163+
164+
return $integrations;
165+
}
166+
167+
/**
168+
* Retrieve the user configuration.
169+
*
170+
* @return array
171+
*/
172+
private function getUserConfig(): array
173+
{
174+
return $this->app['config'][static::$abstract];
175+
}
176+
140177
/**
141178
* Get the services provided by the provider.
142179
*

‎test/Sentry/IntegrationsOptionTest.php‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Tests;
4+
5+
use Sentry\State\Hub;
6+
use Sentry\Integration\IntegrationInterface;
7+
8+
class IntegrationsOptionTest extends SentryLaravelTestCase
9+
{
10+
protected function getEnvironmentSetUp($app)
11+
{
12+
parent::getEnvironmentSetUp($app);
13+
14+
$app->singleton('custom-sentry-integration', static function () {
15+
return new IntegrationsOptionTestIntegrationStub;
16+
});
17+
}
18+
19+
public function testCustomIntegrationIsResolvedFromContainerByAlias()
20+
{
21+
$this->resetApplicationWithConfig([
22+
'sentry.integrations' => [
23+
'custom-sentry-integration',
24+
],
25+
]);
26+
27+
$this->assertNotNull(Hub::getCurrent()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class));
28+
}
29+
30+
public function testCustomIntegrationIsResolvedFromContainerByClass()
31+
{
32+
$this->resetApplicationWithConfig([
33+
'sentry.integrations' => [
34+
IntegrationsOptionTestIntegrationStub::class,
35+
],
36+
]);
37+
38+
$this->assertNotNull(Hub::getCurrent()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class));
39+
}
40+
41+
public function testCustomIntegrationByInstance()
42+
{
43+
$this->resetApplicationWithConfig([
44+
'sentry.integrations' => [
45+
new IntegrationsOptionTestIntegrationStub,
46+
],
47+
]);
48+
49+
$this->assertNotNull(Hub::getCurrent()->getClient()->getIntegration(IntegrationsOptionTestIntegrationStub::class));
50+
}
51+
52+
/**
53+
* @expectedException \ReflectionException
54+
*/
55+
public function testCustomIntegrationThrowsExceptionIfNotResolvable()
56+
{
57+
$this->resetApplicationWithConfig([
58+
'sentry.integrations' => [
59+
'this-will-not-resolve',
60+
],
61+
]);
62+
}
63+
64+
/**
65+
* @expectedException \RuntimeException
66+
*/
67+
public function testIncorrectIntegrationEntryThrowsException()
68+
{
69+
$this->resetApplicationWithConfig([
70+
'sentry.integrations' => [
71+
static function () {
72+
},
73+
],
74+
]);
75+
}
76+
}
77+
78+
class IntegrationsOptionTestIntegrationStub implements IntegrationInterface
79+
{
80+
public function setupOnce(): void
81+
{
82+
}
83+
}

‎test/Sentry/SentryLaravelTestCase.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ abstract class SentryLaravelTestCase extends LaravelTestCase
1616

1717
protected function getEnvironmentSetUp($app)
1818
{
19+
$app['config']->set('sentry.dsn', 'http://publickey:[email protected]/123');
20+
1921
foreach ($this->setupConfig as $key => $value) {
2022
$app['config']->set($key, $value);
2123
}

‎test/Sentry/SqlBindingsInBreadcrumbsTest.php‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,6 @@
66

77
class SqlBindingsInBreadcrumbsTest extends SentryLaravelTestCase
88
{
9-
protected function getEnvironmentSetUp($app)
10-
{
11-
$app['config']->set('sentry.dsn', 'http://publickey:[email protected]/123');
12-
13-
parent::getEnvironmentSetUp($app);
14-
}
15-
16-
public function testIsBound()
17-
{
18-
$this->assertTrue(app()->bound('sentry'));
19-
$this->assertInstanceOf(Hub::class, app('sentry'));
20-
}
21-
22-
/**
23-
* @depends testIsBound
24-
*/
259
public function testSqlBindingsAreRecordedWhenEnabled()
2610
{
2711
$this->resetApplicationWithConfig([
@@ -46,9 +30,6 @@ public function testSqlBindingsAreRecordedWhenEnabled()
4630
$this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']);
4731
}
4832

49-
/**
50-
* @depends testIsBound
51-
*/
5233
public function testSqlBindingsAreRecordedWhenDisabled()
5334
{
5435
$this->resetApplicationWithConfig([

‎test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyDisabledTest.php‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,13 @@ protected function getEnvironmentSetUp($app)
1111
{
1212
parent::getEnvironmentSetUp($app);
1313

14-
$app['config']->set('sentry.dsn', 'http://publickey:[email protected]/123');
15-
1614
$config = $app['config']->all();
1715

1816
$config['sentry']['breadcrumbs.sql_bindings'] = false;
1917

2018
$app['config'] = new Repository($config);
2119
}
2220

23-
public function testIsBound()
24-
{
25-
$this->assertTrue(app()->bound('sentry'));
26-
$this->assertInstanceOf(Hub::class, app('sentry'));
27-
}
28-
29-
/**
30-
* @depends testIsBound
31-
*/
3221
public function testSqlBindingsAreRecordedWhenDisabledByOldConfigKey()
3322
{
3423
$this->assertFalse($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']);

‎test/Sentry/SqlBindingsInBreadcrumbsWithOldConfigKeyEnabledTest.php‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,13 @@ protected function getEnvironmentSetUp($app)
1111
{
1212
parent::getEnvironmentSetUp($app);
1313

14-
$app['config']->set('sentry.dsn', 'http://publickey:[email protected]/123');
15-
1614
$config = $app['config']->all();
1715

1816
$config['sentry']['breadcrumbs.sql_bindings'] = true;
1917

2018
$app['config'] = new Repository($config);
2119
}
2220

23-
public function testIsBound()
24-
{
25-
$this->assertTrue(app()->bound('sentry'));
26-
$this->assertInstanceOf(Hub::class, app('sentry'));
27-
}
28-
29-
/**
30-
* @depends testIsBound
31-
*/
3221
public function testSqlBindingsAreRecordedWhenEnabledByOldConfigKey()
3322
{
3423
$this->assertTrue($this->app['config']->get('sentry')['breadcrumbs.sql_bindings']);

0 commit comments

Comments
 (0)