Skip to content

Commit fccb120

Browse files
weaverryanweaverryan
committed
Making the StatusApi services NOT aware of their repository
Instead, now we just pass in whatever Repository we want to take the action against.
1 parent e05949d commit fccb120

File tree

11 files changed

+74
-87
lines changed

11 files changed

+74
-87
lines changed

‎app/config/github.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ services:
33
app.subscriber.issue_subscriber:
44
class: AppBundle\Issues\IssueListener
55
arguments:
6-
- '@app.status_api.default'
6+
- '@app.status_api'
77

88
parameters:
99
# point to the main symfony repositories

‎app/config/services.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ services:
2424

2525
app.github.cached_labels_api:
2626
class: AppBundle\Issues\GitHub\CachedLabelsApi
27-
arguments: ['@app.github.labels_api', '%default_repository%']
27+
arguments: ['@app.github.labels_api']
2828

29-
app.status_api.default:
29+
app.status_api:
3030
class: AppBundle\Issues\GitHub\GitHubStatusApi
31-
arguments: ['@app.github.cached_labels_api', '%default_repository%']
31+
arguments: ['@app.github.cached_labels_api']
3232

3333
app.github.request_handler:
3434
class: AppBundle\Issues\GitHubRequestHandler

‎app/config/services_test.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ imports:
33
- { resource: services.yml }
44

55
services:
6-
app.status_api.default:
6+
app.status_api:
77
class: AppBundle\Issues\NullStatusApi

‎src/AppBundle/Controller/DefaultController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class DefaultController extends Controller
1313
public function homepageAction()
1414
{
1515
return $this->render('default/homepage.html.twig', [
16-
'needsReviewUrl' => $this->get('app.status_api.default')->getNeedsReviewUrl(),
16+
'needsReviewUrl' => $this->get('app.status_api')->getNeedsReviewUrl(),
1717
]);
1818
}
1919
}

‎src/AppBundle/Event/GitHubEvent.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace AppBundle\Event;
44

5+
use AppBundle\Repository\Repository;
56
use Symfony\Component\EventDispatcher\Event;
67

78
/**
@@ -17,7 +18,7 @@ class GitHubEvent extends Event
1718
private $repository;
1819
private $maintainers;
1920

20-
public function __construct(array $data, $repository, array $maintainers = array())
21+
public function __construct(array $data, Repository $repository, array $maintainers = array())
2122
{
2223
$this->data = $data;
2324
$this->repository = $repository;
@@ -29,6 +30,9 @@ public function getData()
2930
return $this->data;
3031
}
3132

33+
/**
34+
* @return Repository
35+
*/
3236
public function getRepository()
3337
{
3438
return $this->repository;

‎src/AppBundle/Issues/GitHub/CachedLabelsApi.php‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace AppBundle\Issues\GitHub;
44

5+
use AppBundle\Repository\Repository;
56
use Github\Api\Issue\Labels;
67

78
/**
@@ -19,80 +20,75 @@ class CachedLabelsApi
1920
*/
2021
private $labelCache = [];
2122

22-
/**
23-
* @var string
24-
*/
25-
private $repositoryUsername;
26-
27-
/**
28-
* @var string
29-
*/
30-
private $repositoryName;
31-
32-
public function __construct(Labels $labelsApi, $repository)
23+
public function __construct(Labels $labelsApi)
3324
{
34-
list($repositoryUsername, $repositoryName) = explode('/', $repository);
35-
3625
$this->labelsApi = $labelsApi;
37-
$this->repositoryUsername = $repositoryUsername;
38-
$this->repositoryName = $repositoryName;
3926
}
4027

41-
public function getIssueLabels($issueNumber)
28+
public function getIssueLabels($issueNumber, Repository $repository)
4229
{
43-
if (!isset($this->labelCache[$issueNumber])) {
44-
$this->labelCache[$issueNumber] = [];
30+
$key = $this->getCacheKey($issueNumber, $repository);
31+
if (!isset($this->labelCache[$key])) {
32+
$this->labelCache[$key] = [];
4533

4634
$labelsData = $this->labelsApi->all(
47-
$this->repositoryUsername,
48-
$this->repositoryName,
35+
$repository->getVendor(),
36+
$repository->getName(),
4937
$issueNumber
5038
);
5139

5240
// Load labels, keep only the first status label
5341
foreach ($labelsData as $labelData) {
54-
$this->labelCache[$issueNumber][$labelData['name']] = true;
42+
$this->labelCache[$key][$labelData['name']] = true;
5543
}
5644
}
5745

58-
return array_keys($this->labelCache[$issueNumber]);
46+
return array_keys($this->labelCache[$key]);
5947
}
6048

61-
public function addIssueLabel($issueNumber, $label)
49+
public function addIssueLabel($issueNumber, $label, Repository $repository)
6250
{
63-
if (isset($this->labelCache[$issueNumber][$label])) {
51+
$key = $this->getCacheKey($issueNumber, $repository);
52+
53+
if (isset($this->labelCache[$key][$label])) {
6454
return;
6555
}
6656

6757
$this->labelsApi->add(
68-
$this->repositoryUsername,
69-
$this->repositoryName,
58+
$repository->getVendor(),
59+
$repository->getName(),
7060
$issueNumber,
7161
$label
7262
);
7363

7464
// Update cache if already loaded
75-
if (isset($this->labelCache[$issueNumber])) {
76-
$this->labelCache[$issueNumber][$label] = true;
65+
if (isset($this->labelCache[$key])) {
66+
$this->labelCache[$key][$label] = true;
7767
}
7868
}
7969

80-
public function removeIssueLabel($issueNumber, $label)
70+
public function removeIssueLabel($issueNumber, $label, Repository $repository)
8171
{
82-
if (isset($this->labelCache[$issueNumber]) && !isset($this->labelCache[$issueNumber][$label])) {
72+
$key = $this->getCacheKey($issueNumber, $repository);
73+
if (isset($this->labelCache[$key]) && !isset($this->labelCache[$key][$label])) {
8374
return;
8475
}
8576

8677
$this->labelsApi->remove(
87-
$this->repositoryUsername,
88-
$this->repositoryName,
78+
$repository->getVendor(),
79+
$repository->getName(),
8980
$issueNumber,
9081
$label
9182
);
9283

9384
// Update cache if already loaded
94-
if (isset($this->labelCache[$issueNumber])) {
95-
unset($this->labelCache[$issueNumber][$label]);
85+
if (isset($this->labelCache[$key])) {
86+
unset($this->labelCache[$key][$label]);
9687
}
9788
}
89+
90+
private function getCacheKey($issueNumber, Repository $repository)
91+
{
92+
return sprintf('%s_%s_%s', $issueNumber, $repository->getVendor(), $repository->getName());
93+
}
9894
}

‎src/AppBundle/Issues/GitHub/GitHubStatusApi.php‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use AppBundle\Issues\Status;
66
use AppBundle\Issues\StatusApi;
7+
use AppBundle\Repository\Repository;
78

89
class GitHubStatusApi implements StatusApi
910
{
@@ -21,37 +22,25 @@ class GitHubStatusApi implements StatusApi
2122
*/
2223
private $labelsApi;
2324

24-
/**
25-
* @var string
26-
*/
27-
private $repositoryUsername;
28-
29-
/**
30-
* @var string
31-
*/
32-
private $repositoryName;
33-
34-
public function __construct(CachedLabelsApi $labelsApi, $repository)
25+
public function __construct(CachedLabelsApi $labelsApi)
3526
{
3627
$this->labelsApi = $labelsApi;
3728
$this->labelToStatus = array_flip($this->statusToLabel);
38-
list($vendor, $name) = explode('/', $repository);
39-
$this->repositoryUsername = $vendor;
40-
$this->repositoryName = $name;
4129
}
4230

4331
/**
4432
* @param int $issueNumber The GitHub issue number
4533
* @param string $newStatus A Status::* constant
34+
* @param Repository $repository
4635
*/
47-
public function setIssueStatus($issueNumber, $newStatus)
36+
public function setIssueStatus($issueNumber, $newStatus, Repository $repository)
4837
{
4938
if (!isset($this->statusToLabel[$newStatus])) {
5039
throw new \InvalidArgumentException(sprintf('Invalid status "%s"', $newStatus));
5140
}
5241

5342
$newLabel = $this->statusToLabel[$newStatus];
54-
$currentLabels = $this->labelsApi->getIssueLabels($issueNumber);
43+
$currentLabels = $this->labelsApi->getIssueLabels($issueNumber, $repository);
5544
$addLabel = true;
5645

5746
foreach ($currentLabels as $label) {
@@ -70,18 +59,18 @@ public function setIssueStatus($issueNumber, $newStatus)
7059
}
7160

7261
// Remove other statuses
73-
$this->labelsApi->removeIssueLabel($issueNumber, $label);
62+
$this->labelsApi->removeIssueLabel($issueNumber, $label, $repository);
7463
}
7564

7665
// Ignored if the label is already set
7766
if ($addLabel) {
78-
$this->labelsApi->addIssueLabel($issueNumber, $newLabel);
67+
$this->labelsApi->addIssueLabel($issueNumber, $newLabel, $repository);
7968
}
8069
}
8170

82-
public function getIssueStatus($issueNumber)
71+
public function getIssueStatus($issueNumber, Repository $repository)
8372
{
84-
$currentLabels = $this->labelsApi->getIssueLabels($issueNumber);
73+
$currentLabels = $this->labelsApi->getIssueLabels($issueNumber, $repository);
8574

8675
foreach ($currentLabels as $label) {
8776
if (isset($this->labelToStatus[$label])) {
@@ -93,12 +82,12 @@ public function getIssueStatus($issueNumber)
9382
return;
9483
}
9584

96-
public function getNeedsReviewUrl()
85+
public function getNeedsReviewUrl(Repository $repository)
9786
{
9887
return sprintf(
9988
'https://github.com/%s/%s/labels/%s',
100-
$this->repositoryUsername,
101-
$this->repositoryName,
89+
$repository->getVendor(),
90+
$repository->getName(),
10291
rawurlencode($this->statusToLabel[Status::NEEDS_REVIEW])
10392
);
10493
}

‎src/AppBundle/Issues/GitHubRequestHandler.php‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@
99
use AppBundle\Exception\GitHubInvalidConfigurationException;
1010
use AppBundle\Exception\GitHubPreconditionFailedException;
1111
use AppBundle\Exception\GitHubRuntimeException;
12-
use AppBundle\Issues\GitHub\CachedLabelsApi;
13-
use AppBundle\Issues\GitHub\GitHubStatusApi;
1412
use AppBundle\Repository\Provider\RepositoryProviderInterface;
15-
use AppBundle\Repository\RepositoryRegistry;
1613
use Github\Api\Issue\Labels;
1714
use Symfony\Component\DependencyInjection\ContainerInterface;
1815
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -25,17 +22,12 @@
2522
*/
2623
class GitHubRequestHandler
2724
{
28-
private $labelsApi;
2925
private $dispatcher;
3026
private $repositoryProvider;
31-
/**
32-
* @var ContainerInterface
33-
*/
3427
private $container;
3528

36-
public function __construct(Labels $labelsApi, EventDispatcherInterface $dispatcher, RepositoryProviderInterface $repositoryProvider, ContainerInterface $container)
29+
public function __construct(EventDispatcherInterface $dispatcher, RepositoryProviderInterface $repositoryProvider, ContainerInterface $container)
3730
{
38-
$this->labelsApi = $labelsApi;
3931
$this->dispatcher = $dispatcher;
4032
$this->repositoryProvider = $repositoryProvider;
4133
$this->container = $container;

‎src/AppBundle/Issues/IssueListener.php‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use AppBundle\Event\GitHubEvent;
66
use AppBundle\GitHubEvents;
7+
use AppBundle\Repository\Repository;
78
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
89

910
class IssueListener implements EventSubscriberInterface
@@ -39,6 +40,7 @@ final public function __construct(StatusApi $statusApi)
3940
public function onIssueComment(GitHubEvent $event)
4041
{
4142
$data = $event->getData();
43+
$repository = $event->getRepository();
4244
$newStatus = null;
4345
$issueNumber = $data['issue']['number'];
4446
$user = $data['comment']['user']['login'];
@@ -58,7 +60,7 @@ public function onIssueComment(GitHubEvent $event)
5860
// Second subpattern = first status character
5961
$newStatus = $statuses[strtolower(end($matches[1]))];
6062

61-
$this->setIssueStatus($issueNumber, $newStatus);
63+
$this->setIssueStatus($issueNumber, $newStatus, $repository);
6264
}
6365

6466
$event->setResponseData(array(
@@ -75,13 +77,14 @@ public function onIssueComment(GitHubEvent $event)
7577
public function onPullRequest(GitHubEvent $event)
7678
{
7779
$data = $event->getData();
80+
$repository = $event->getRepository();
7881
if ('opened' !== $action = $data['action']) {
7982
$responseData = array('unsupported_action' => $action);
8083
} else {
8184
$pullRequestNumber = $data['pull_request']['number'];
8285
$newStatus = Status::NEEDS_REVIEW;
8386

84-
$this->setIssueStatus($pullRequestNumber, $newStatus);
87+
$this->setIssueStatus($pullRequestNumber, $newStatus, $repository);
8588

8689
$responseData = array(
8790
'pull_request' => $pullRequestNumber,
@@ -100,6 +103,7 @@ public function onPullRequest(GitHubEvent $event)
100103
public function onIssues(GitHubEvent $event)
101104
{
102105
$data = $event->getData();
106+
$repository = $event->getRepository();
103107
if ('labeled' !== $action = $data['action']) {
104108
$event->setResponseData(array('unsupported_action' => $action));
105109

@@ -108,7 +112,7 @@ public function onIssues(GitHubEvent $event)
108112

109113
$responseData = array('issue' => $issueNumber = $data['issue']['number']);
110114
// Ignore non-bugs or issue which already has a status
111-
if ('bug' !== strtolower($data['label']['name']) || null !== $currentStatus = $this->getIssueStatus($issueNumber)) {
115+
if ('bug' !== strtolower($data['label']['name']) || null !== $currentStatus = $this->getIssueStatus($issueNumber, $repository)) {
112116
$responseData['status_change'] = null;
113117
$event->setResponseData($responseData);
114118

@@ -117,20 +121,20 @@ public function onIssues(GitHubEvent $event)
117121

118122
$newStatus = Status::NEEDS_REVIEW;
119123

120-
$this->setIssueStatus($issueNumber, $newStatus);
124+
$this->setIssueStatus($issueNumber, $newStatus, $repository);
121125
$responseData['status_change'] = $newStatus;
122126

123127
$event->setResponseData($responseData);
124128
}
125129

126-
private function getIssueStatus($issueNumber)
130+
private function getIssueStatus($issueNumber, Repository $repository)
127131
{
128-
return $this->statusApi->getIssueStatus($issueNumber);
132+
return $this->statusApi->getIssueStatus($issueNumber, $repository);
129133
}
130134

131-
private function setIssueStatus($issueNumber, $status)
135+
private function setIssueStatus($issueNumber, $status, Repository $repository)
132136
{
133-
return $this->statusApi->setIssueStatus($issueNumber, $status);
137+
return $this->statusApi->setIssueStatus($issueNumber, $status, $repository);
134138
}
135139

136140
/**

0 commit comments

Comments
 (0)