Skip to content
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

ref: Make getScope private #776

Merged
merged 5 commits into from
Feb 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# CHANGELOG

## Unreleased

- ...
- Updated .gitattributes to reduce package footprint (#770)
- Use multibyte functions to handle unicode paths (#774)
- Remove `Hub::getScope()` to deny direct access to `Scope` instances (#776)

## 2.0.0-beta2 (2019-02-11)
- Rename `SentryAuth` class to `SentryAuthentication` (#742)
Expand Down
18 changes: 10 additions & 8 deletions src/State/Hub.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,6 @@ public function getClient(): ?ClientInterface
return $this->getStackTop()->getClient();
}

/**
* {@inheritdoc}
*/
public function getScope(): Scope
{
return $this->getStackTop()->getScope();
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -242,6 +234,16 @@ public function getIntegration(string $className): ?IntegrationInterface
return null;
}

/**
* Gets the scope bound to the top of the stack.
*
* @return Scope
*/
private function getScope(): Scope
{
return $this->getStackTop()->getScope();
}

/**
* Gets the topmost client/layer pair in the stack.
*
Expand Down
7 changes: 0 additions & 7 deletions src/State/HubInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ interface HubInterface
*/
public function getClient(): ?ClientInterface;

/**
* Gets the scope bound to the top of the stack.
*
* @return Scope
*/
public function getScope(): Scope;

/**
* Gets the ID of the last captured event.
*
Expand Down
9 changes: 7 additions & 2 deletions tests/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,13 @@ public function testHandlingExceptionThrowingAnException(): void

$client = new Client(new Options(), $transport, $this->createEventFactory());

Hub::getCurrent()->bindClient($client);
$client->captureException($this->createCarelessExceptionWithStacktrace(), Hub::getCurrent()->getScope());
$hub = Hub::getCurrent();
$hub->bindClient($client);

$method = new \ReflectionMethod($hub, 'getScope');
$method->setAccessible(true);

$client->captureException($this->createCarelessExceptionWithStacktrace(), $method->invoke($hub));
}

/**
Expand Down
7 changes: 6 additions & 1 deletion tests/SdkTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,12 @@ public function testAddBreadcrumb(): void

addBreadcrumb($breadcrumb);

$this->assertSame([$breadcrumb], Hub::getCurrent()->getScope()->getBreadcrumbs());
$hub = Hub::getCurrent();

$method = new \ReflectionMethod($hub, 'getScope');
$method->setAccessible(true);

$this->assertSame([$breadcrumb], $method->invoke($hub)->getBreadcrumbs());
}

public function testWithScope(): void
Expand Down
41 changes: 25 additions & 16 deletions tests/State/HubTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Sentry\ClientInterface;
use Sentry\Severity;
use Sentry\State\Hub;
use Sentry\State\HubInterface;
use Sentry\State\Scope;

final class HubTest extends TestCase
Expand All @@ -19,7 +20,7 @@ public function testConstructorCreatesScopeAutomatically(): void
{
$hub = new Hub(null, null);

$this->assertNotNull($hub->getScope());
$this->assertNotNull($this->getScope($hub));
}

public function testGetClient(): void
Expand All @@ -36,7 +37,7 @@ public function testGetScope(): void
$scope = new Scope();
$hub = new Hub($this->createMock(ClientInterface::class), $scope);

$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, $this->getScope($hub));
}

public function testGetLastEventId(): void
Expand All @@ -57,14 +58,14 @@ public function testPushScope(): void
{
$hub = new Hub($this->createMock(ClientInterface::class));

$scope1 = $hub->getScope();
$scope1 = $this->getScope($hub);
$client1 = $hub->getClient();

$scope2 = $hub->pushScope();
$client2 = $hub->getClient();

$this->assertNotSame($scope1, $scope2);
$this->assertSame($scope2, $hub->getScope());
$this->assertSame($scope2, $this->getScope($hub));
$this->assertSame($client1, $client2);
$this->assertSame($client1, $hub->getClient());
}
Expand All @@ -73,22 +74,22 @@ public function testPopScope(): void
{
$hub = new Hub($this->createMock(ClientInterface::class));

$scope1 = $hub->getScope();
$scope1 = $this->getScope($hub);
$client = $hub->getClient();

$scope2 = $hub->pushScope();

$this->assertSame($scope2, $hub->getScope());
$this->assertSame($scope2, $this->getScope($hub));
$this->assertSame($client, $hub->getClient());

$this->assertTrue($hub->popScope());

$this->assertSame($scope1, $hub->getScope());
$this->assertSame($scope1, $this->getScope($hub));
$this->assertSame($client, $hub->getClient());

$this->assertFalse($hub->popScope());

$this->assertSame($scope1, $hub->getScope());
$this->assertSame($scope1, $this->getScope($hub));
$this->assertSame($client, $hub->getClient());
}

Expand All @@ -97,7 +98,7 @@ public function testWithScope(): void
$scope = new Scope();
$hub = new Hub($this->createMock(ClientInterface::class), $scope);

$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, $this->getScope($hub));

$callbackInvoked = false;

Expand All @@ -116,7 +117,7 @@ public function testWithScope(): void
}

$this->assertTrue($callbackInvoked);
$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, $this->getScope($hub));
}

public function testConfigureScope(): void
Expand All @@ -134,7 +135,7 @@ public function testConfigureScope(): void
});

$this->assertTrue($callbackInvoked);
$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, $this->getScope($hub));
}

public function testBindClient(): void
Expand Down Expand Up @@ -217,7 +218,7 @@ public function testAddBreadcrumb(): void

$hub->addBreadcrumb($breadcrumb);

$this->assertSame([$breadcrumb], $hub->getScope()->getBreadcrumbs());
$this->assertSame([$breadcrumb], $this->getScope($hub)->getBreadcrumbs());
}

public function testAddBreadcrumbDoesNothingIfMaxBreadcrumbsLimitIsZero(): void
Expand All @@ -227,14 +228,14 @@ public function testAddBreadcrumbDoesNothingIfMaxBreadcrumbsLimitIsZero(): void

$hub->addBreadcrumb(new Breadcrumb(Breadcrumb::LEVEL_ERROR, Breadcrumb::TYPE_ERROR, 'error_reporting'));

$this->assertEmpty($hub->getScope()->getBreadcrumbs());
$this->assertEmpty($this->getScope($hub)->getBreadcrumbs());
}

public function testAddBreadcrumbRespectsMaxBreadcrumbsLimit(): void
{
$client = ClientBuilder::create(['max_breadcrumbs' => 2])->getClient();
$hub = new Hub($client);
$scope = $hub->getScope();
$scope = $this->getScope($hub);

$breadcrumb1 = new Breadcrumb(Breadcrumb::LEVEL_WARNING, Breadcrumb::TYPE_ERROR, 'error_reporting', 'foo');
$breadcrumb2 = new Breadcrumb(Breadcrumb::LEVEL_WARNING, Breadcrumb::TYPE_ERROR, 'error_reporting', 'bar');
Expand All @@ -260,7 +261,7 @@ public function testAddBreadcrumbDoesNothingWhenBeforeBreadcrumbCallbackReturnsN

$hub->addBreadcrumb(new Breadcrumb(Breadcrumb::LEVEL_ERROR, Breadcrumb::TYPE_ERROR, 'error_reporting'));

$this->assertEmpty($hub->getScope()->getBreadcrumbs());
$this->assertEmpty($this->getScope($hub)->getBreadcrumbs());
}

public function testAddBreadcrumbStoresBreadcrumbReturnedByBeforeBreadcrumbCallback(): void
Expand All @@ -276,7 +277,7 @@ public function testAddBreadcrumbStoresBreadcrumbReturnedByBeforeBreadcrumbCallb

$hub->addBreadcrumb($breadcrumb1);

$this->assertSame([$breadcrumb2], $hub->getScope()->getBreadcrumbs());
$this->assertSame([$breadcrumb2], $this->getScope($hub)->getBreadcrumbs());
}

public function testCaptureEvent(): void
Expand All @@ -292,4 +293,12 @@ public function testCaptureEvent(): void

$this->assertEquals('2b867534eead412cbdb882fd5d441690', $hub->captureEvent(['message' => 'test']));
}

private function getScope(HubInterface $hub): Scope
{
$method = new \ReflectionMethod($hub, 'getScope');
$method->setAccessible(true);

return $method->invoke($hub);
}
}