Skip to content

Commit c9b68e4

Browse files
committed
bug #122 Make sure to fetch all the labels from Github (Nyholm)
This PR was squashed before being merged into the master branch. Discussion ---------- Make sure to fetch all the labels from Github Wohoo. I found a bug. When we asked Github for all the labels on symfony/symfony, we just got the 20 first ones. This PR will fix that. Commits ------- 1e9b6d7 Make sure to fetch all the labels from Github
2 parents ecd39e4 + 1e9b6d7 commit c9b68e4

File tree

5 files changed

+45
-16
lines changed

5 files changed

+45
-16
lines changed

config/packages/github_api.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ services:
55
calls:
66
- ['authenticate', ['%env(GITHUB_TOKEN)%', 'access_token_header']]
77

8+
Github\ResultPager:
9+
arguments: ['@Github\Client']
10+
811
Github\HttpClient\Builder:
912
arguments:
1013
- '@github.httplug_client'

config/services.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ services:
7575

7676
Github\Api\Issue\Labels:
7777
factory: ['@Github\Api\Issue', labels]
78+
calls:
79+
- ['setPerPage', [100]]
7880

7981
Github\Api\Issue\Milestones:
8082
factory: ['@Github\Api\Issue', milestones]

src/Api/Label/GithubLabelApi.php

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Model\Repository;
66
use Github\Api\Issue\Labels;
77
use Github\Exception\RuntimeException;
8+
use Github\ResultPager;
89
use Psr\Log\LoggerInterface;
910
use Symfony\Contracts\Cache\CacheInterface;
1011
use Symfony\Contracts\Cache\ItemInterface;
@@ -14,17 +15,22 @@
1415
*/
1516
class GithubLabelApi implements LabelApi
1617
{
18+
/**
19+
* In memory cache for specific issues.
20+
*
21+
* @var array<array-key, array<array-key, bool>>
22+
*/
23+
private $labelCache = [];
24+
1725
/**
1826
* @var Labels
1927
*/
2028
private $labelsApi;
2129

2230
/**
23-
* In memory cache for specific issues.
24-
*
25-
* @var array<array-key, array<array-key, bool>>
31+
* @var ResultPager
2632
*/
27-
private $labelCache = [];
33+
private $resultPager;
2834

2935
/**
3036
* @var CacheInterface
@@ -36,9 +42,10 @@ class GithubLabelApi implements LabelApi
3642
*/
3743
private $logger;
3844

39-
public function __construct(Labels $labelsApi, CacheInterface $cache, LoggerInterface $logger)
45+
public function __construct(Labels $labelsApi, ResultPager $resultPager, CacheInterface $cache, LoggerInterface $logger)
4046
{
4147
$this->labelsApi = $labelsApi;
48+
$this->resultPager = $resultPager;
4249
$this->cache = $cache;
4350
$this->logger = $logger;
4451
}
@@ -118,14 +125,9 @@ public function addIssueLabels($issueNumber, array $labels, Repository $reposito
118125
*/
119126
public function getAllLabelsForRepository(Repository $repository): array
120127
{
121-
$key = 'labels'.sha1($repository->getFullName());
128+
$allLabels = $this->getAllLabels($repository);
122129

123-
return $this->cache->get($key, function (ItemInterface $item) use ($repository) {
124-
$labels = $this->labelsApi->all($repository->getVendor(), $repository->getName()) ?? [];
125-
$item->expiresAfter(36000);
126-
127-
return array_column($labels, 'name');
128-
});
130+
return array_column($allLabels, 'name');
129131
}
130132

131133
/**
@@ -136,8 +138,8 @@ public function getComponentLabelsForRepository(Repository $repository): array
136138
$key = 'component_labels_'.sha1($repository->getFullName());
137139

138140
return $this->cache->get($key, function (ItemInterface $item) use ($repository) {
139-
$labels = $this->labelsApi->all($repository->getVendor(), $repository->getName()) ?? [];
140-
$item->expiresAfter(36000);
141+
$labels = $this->getAllLabels($repository);
142+
$item->expiresAfter(86400);
141143
$componentLabels = [];
142144
foreach ($labels as $label) {
143145
if ('dddddd' === strtolower($label['color'])) {
@@ -149,6 +151,18 @@ public function getComponentLabelsForRepository(Repository $repository): array
149151
});
150152
}
151153

154+
private function getAllLabels(Repository $repository): array
155+
{
156+
$key = 'labels_'.sha1($repository->getFullName());
157+
158+
return $this->cache->get($key, function (ItemInterface $item) use ($repository) {
159+
$labels = $this->resultPager->fetchAll($this->labelsApi, 'all', [$repository->getVendor(), $repository->getName()]) ?? [];
160+
$item->expiresAfter(604800);
161+
162+
return $labels;
163+
});
164+
}
165+
152166
private function getCacheKey($issueNumber, Repository $repository)
153167
{
154168
return sprintf('%s_%s_%s', $issueNumber, $repository->getVendor(), $repository->getName());

tests/Api/Label/GithubLabelApiTest.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Api\Label\GithubLabelApi;
66
use App\Model\Repository;
77
use Github\Api\Issue\Labels;
8+
use Github\ResultPager;
89
use PHPUnit\Framework\MockObject\MockObject;
910
use PHPUnit\Framework\TestCase;
1011
use Psr\Log\NullLogger;
@@ -36,7 +37,16 @@ protected function setUp()
3637
$this->backendApi = $this->getMockBuilder(Labels::class)
3738
->disableOriginalConstructor()
3839
->getMock();
39-
$this->api = new GithubLabelApi($this->backendApi, new NullAdapter(), new NullLogger());
40+
41+
$resultPager = $this->getMockBuilder(ResultPager::class)
42+
->disableOriginalConstructor()
43+
->setMethods(['fetchAll'])
44+
->getMock();
45+
$resultPager->method('fetchAll')->willReturnCallback(function () {
46+
return $this->backendApi->all('x', 'y');
47+
});
48+
49+
$this->api = new GithubLabelApi($this->backendApi, $resultPager, new NullAdapter(), new NullLogger());
4050
$this->repository = new Repository(
4151
self::USER_NAME,
4252
self::REPO_NAME,

tests/Service/LabelNameExtractorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public function testExtractLabels()
1414
{
1515
$api = new StaticLabelApi();
1616
$extractor = new LabelNameExtractor($api, new NullLogger());
17-
$repo = new Repository('carson-playground', 'symfony');
17+
$repo = new Repository('carsonbot-playground', 'symfony');
1818

1919
$this->assertSame(['Messenger'], $extractor->extractLabels('[Messenger] Foobar', $repo));
2020
$this->assertSame(['Messenger'], $extractor->extractLabels('[messenger] Foobar', $repo));

0 commit comments

Comments
 (0)