Skip to content

Commit

Permalink
Use parallel http requests (#79)
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored Jan 25, 2024
1 parent 61d2050 commit a197314
Show file tree
Hide file tree
Showing 12 changed files with 192 additions and 94 deletions.
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
.editorconfig export-ignore
.php-cs-fixer.php export-ignore
phpstan.neon export-ignore
baseline-7.4.neon export-ignore
ignore-by-php-version.neon.php export-ignore
6 changes: 6 additions & 0 deletions baseline-7.4.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
parameters:
ignoreErrors:
-
message: "#^Strict comparison using \\=\\=\\= between null and string will always evaluate to false\\.$#"
count: 1
path: src/utils/HttpClient.php
12 changes: 12 additions & 0 deletions ignore-by-php-version.neon.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php declare(strict_types = 1);

$includes = [];
if (PHP_VERSION_ID < 80000) {
$includes[] = __DIR__ . '/baseline-7.4.neon';
}

$config = [];
$config['includes'] = $includes;
$config['parameters']['phpVersion'] = PHP_VERSION_ID;

return $config;
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
includes:
- extension.neon
- ignore-by-php-version.neon.php

parameters:
level: 8
Expand Down
2 changes: 1 addition & 1 deletion src/TodoByTicketCollector.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function processNode(Node $node, Scope $scope)
$ticketKey = $match['ticketKey'][0];
$todoText = trim($match['comment'][0]);

// collectors do not support serializing objects
// collectors do not support serializing objects, pass a string instead.
$json = json_encode($comment);
if (false === $json) {
throw new RuntimeException('Failed to encode comment as JSON: ' . json_last_error_msg());
Expand Down
40 changes: 34 additions & 6 deletions src/TodoByTicketRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
use PHPStan\Analyser\Scope;
use PHPStan\Node\CollectedDataNode;
use PHPStan\Rules\Rule;
use RuntimeException;
use staabm\PHPStanTodoBy\utils\ExpiredCommentErrorBuilder;
use staabm\PHPStanTodoBy\utils\ticket\TicketRuleConfiguration;

use function array_key_exists;
use function in_array;
use function strlen;

Expand All @@ -36,20 +38,46 @@ public function processNode(Node $node, Scope $scope): array
{
$collectorData = $node->get(TodoByTicketCollector::class);

$ticketKeys = [];
foreach ($collectorData as $collected) {
foreach ($collected as $tickets) {
foreach ($tickets as [$json, $ticketKey, $todoText, $wholeMatchStartOffset, $line]) {
if ([] !== $this->configuration->getKeyPrefixes() && !$this->hasPrefix($ticketKey)) {
continue;
}
if ('' === $ticketKey) {
continue;
}
// de-duplicate keys
$ticketKeys[$ticketKey] = true;
}
}
}

if ([] === $ticketKeys) {
return [];
}

$keyToTicketStatus = $this->configuration->getFetcher()->fetchTicketStatus(
array_keys($ticketKeys)
);

$errors = [];
foreach ($collectorData as $file => $declarations) {
foreach ($declarations as $tickets) {
foreach ($collectorData as $file => $collected) {
foreach ($collected as $tickets) {
foreach ($tickets as [$json, $ticketKey, $todoText, $wholeMatchStartOffset, $line]) {
$comment = $this->commentFromJson($json);
if ([] !== $this->configuration->getKeyPrefixes() && !$this->hasPrefix($ticketKey)) {
continue;
}

$ticketStatus = $this->configuration->getFetcher()->fetchTicketStatus($ticketKey);
if (!array_key_exists($ticketKey, $keyToTicketStatus)) {
throw new RuntimeException("Missing ticket-status for key $ticketKey");
}
$ticketStatus = $keyToTicketStatus[$ticketKey];

if (null === $ticketStatus) {
$errors[] = $this->errorBuilder->buildFileError(
$comment,
$this->commentFromJson($json),
"Ticket $ticketKey doesn't exist or provided credentials do not allow for viewing it.",
null,
$wholeMatchStartOffset,
Expand All @@ -71,7 +99,7 @@ public function processNode(Node $node, Scope $scope): array
}

$errors[] = $this->errorBuilder->buildFileError(
$comment,
$this->commentFromJson($json),
$errorMessage,
null,
$wholeMatchStartOffset,
Expand Down
72 changes: 53 additions & 19 deletions src/utils/HttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,69 @@
final class HttpClient
{
/**
* @param non-empty-array<non-empty-string> $urls
* @param list<string> $headers
* @return array{int, string}
*
* @return non-empty-array<non-empty-string, array{int, string}>
*/
public function get(string $url, array $headers): array
public function getMulti(array $urls, array $headers): array
{
$curl = curl_init($url);
if (!$curl) {
throw new RuntimeException('Could not initialize cURL connection');
}
$mh = curl_multi_init();

// see https://stackoverflow.com/a/27776164/1597388
curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, 3);
curl_setopt($curl, CURLOPT_TIMEOUT, 10);
$handles = [];

curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);
foreach ($urls as $url) {
$curl = curl_init($url);
if (!$curl) {
throw new RuntimeException('Could not initialize cURL connection');
}

if ([] !== $headers) {
curl_setopt($curl, CURLOPT_HTTPHEADER, $headers);
}
// see https://stackoverflow.com/a/27776164/1597388
curl_setopt($curl, CURLOPT_CONNECTTIMEOUT, 3);
curl_setopt($curl, CURLOPT_TIMEOUT, 10);

curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($curl, CURLOPT_RETURNTRANSFER, true);

$response = curl_exec($curl);
if ([] !== $headers) {
curl_setopt($curl, CURLOPT_HTTPHEADER, $headers);
}

$handles[$url] = $curl;
}

if (!is_string($response)) {
throw new RuntimeException("Could not fetch url $url");
foreach ($handles as $handle) {
curl_multi_add_handle($mh, $handle);
}

$responseCode = curl_getinfo($curl, CURLINFO_RESPONSE_CODE);
do {
$status = curl_multi_exec($mh, $active);
if ($active) {
// Wait a short time for more activity
curl_multi_select($mh);
}
} while ($active && CURLM_OK == $status);

$result = [];
foreach ($handles as $url => $handle) {
$response = curl_multi_getcontent($handle);
$errno = curl_multi_errno($mh);

if ($errno || !is_string($response)) {
$message = curl_multi_strerror($errno);
if (null === $message) {
$message = "Could not fetch url $url";
}
throw new RuntimeException($message);
}

$responseCode = curl_getinfo($handle, CURLINFO_RESPONSE_CODE);
$result[$url] = [$responseCode, $response];

curl_multi_remove_handle($mh, $handle);
}
curl_multi_close($mh);

return [$responseCode, $response];
return $result;
}
}
53 changes: 27 additions & 26 deletions src/utils/ticket/GitHubTicketStatusFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ final class GitHubTicketStatusFetcher implements TicketStatusFetcher
private string $defaultRepo;
private ?string $accessToken;

/**
* @var array<string, ?string>
*/
private array $cache;

private HttpClient $httpClient;

public function __construct(string $defaultOwner, string $defaultRepo, ?string $credentials, ?string $credentialsFilePath, HttpClient $httpClient)
Expand All @@ -36,25 +31,24 @@ public function __construct(string $defaultOwner, string $defaultRepo, ?string $
$this->defaultRepo = $defaultRepo;
$this->accessToken = CredentialsHelper::getCredentials($credentials, $credentialsFilePath);

$this->cache = [];
$this->httpClient = $httpClient;
}

public function fetchTicketStatus(string $ticketKey): ?string
public function fetchTicketStatus(array $ticketKeys): array
{
$ticketUrls = [];

$keyRegex = self::KEY_REGEX;
preg_match_all("/$keyRegex/ix", $ticketKey, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);
foreach ($ticketKeys as $ticketKey) {
preg_match_all("/$keyRegex/ix", $ticketKey, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER);

$owner = $matches[0]['githubOwner'][0] ?: $this->defaultOwner;
$repo = $matches[0]['githubRepo'][0] ?: $this->defaultRepo;
$number = $matches[0]['githubNumber'][0];
$cacheKey = "$owner/$repo#$number";
$owner = $matches[0]['githubOwner'][0] ?: $this->defaultOwner;
$repo = $matches[0]['githubRepo'][0] ?: $this->defaultRepo;
$number = $matches[0]['githubNumber'][0];

if (array_key_exists($cacheKey, $this->cache)) {
return $this->cache[$cacheKey];
$ticketUrls[$ticketKey] = "https://api.github.com/repos/$owner/$repo/issues/$number";
}

$url = "https://api.github.com/repos/$owner/$repo/issues/$number";
$apiVersion = self::API_VERSION;

$headers = [
Expand All @@ -67,23 +61,30 @@ public function fetchTicketStatus(string $ticketKey): ?string
$headers[] = "Authorization: Bearer $this->accessToken";
}

[$responseCode, $response] = $this->httpClient->get($url, $headers);
$responses = $this->httpClient->getMulti($ticketUrls, $headers);

if (404 === $responseCode) {
return null;
}
$results = [];
$urlsToKeys = array_flip($ticketUrls);
foreach ($responses as $url => [$responseCode, $response]) {
if (404 === $responseCode) {
$results[$url] = null;
continue;
}

if (200 !== $responseCode) {
throw new RuntimeException("Could not fetch ticket's status from GitHub with $url");
}
if (200 !== $responseCode) {
throw new RuntimeException("Could not fetch ticket's status from GitHub with $url");
}

$data = json_decode($response, true);
$data = json_decode($response, true);
if (!is_array($data) || !array_key_exists('state', $data) || !is_string($data['state'])) {
throw new RuntimeException("GitHub returned invalid response body with $url");
}

if (!is_array($data) || !array_key_exists('state', $data) || !is_string($data['state'])) {
throw new RuntimeException("GitHub returned invalid response body with $url");
$ticketKey = $urlsToKeys[$url];
$results[$ticketKey] = $data['state'];
}

return $this->cache[$cacheKey] = $data['state'];
return $results;
}

public static function getKeyPattern(): string
Expand Down
42 changes: 22 additions & 20 deletions src/utils/ticket/JiraTicketStatusFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ final class JiraTicketStatusFetcher implements TicketStatusFetcher
private string $host;
private ?string $authorizationHeader;

/**
* @var array<string, ?string>
*/
private array $cache;

private HttpClient $httpClient;

public function __construct(string $host, ?string $credentials, ?string $credentialsFilePath, HttpClient $httpClient)
Expand All @@ -31,39 +26,46 @@ public function __construct(string $host, ?string $credentials, ?string $credent
$this->host = $host;
$this->authorizationHeader = $credentials ? self::createAuthorizationHeader($credentials) : null;

$this->cache = [];
$this->httpClient = $httpClient;
}

public function fetchTicketStatus(string $ticketKey): ?string
public function fetchTicketStatus(array $ticketKeys): array
{
if (array_key_exists($ticketKey, $this->cache)) {
return $this->cache[$ticketKey];
}
$ticketUrls = [];

$apiVersion = self::API_VERSION;
foreach ($ticketKeys as $ticketKey) {
$ticketUrls[$ticketKey] = "{$this->host}/rest/api/$apiVersion/issue/$ticketKey?expand=status";
}

$url = "{$this->host}/rest/api/$apiVersion/issue/$ticketKey?expand=status";
$headers = [];
if (null !== $this->authorizationHeader) {
$headers = [
"Authorization: $this->authorizationHeader",
];
}

[$responseCode, $response] = $this->httpClient->get($url, $headers);
$responses = $this->httpClient->getMulti($ticketUrls, $headers);

if (404 === $responseCode) {
return null;
}
$results = [];
$urlsToKeys = array_flip($ticketUrls);
foreach ($responses as $url => [$responseCode, $response]) {
if (404 === $responseCode) {
$results[$url] = null;
continue;
}

if (200 !== $responseCode) {
throw new RuntimeException("Could not fetch ticket's status from Jira with url $url");
}
if (200 !== $responseCode) {
throw new RuntimeException("Could not fetch ticket's status from Jira with url $url");
}

$data = self::decodeAndValidateResponse($response);

$data = self::decodeAndValidateResponse($response);
$ticketKey = $urlsToKeys[$url];
$results[$ticketKey] = $data['fields']['status']['name'];
}

return $this->cache[$ticketKey] = $data['fields']['status']['name'];
return $results;
}

public static function getKeyPattern(): string
Expand Down
8 changes: 6 additions & 2 deletions src/utils/ticket/TicketStatusFetcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
/** @internal */
interface TicketStatusFetcher
{
/** @return string|null Status name or null if ticket doesn't exist */
public function fetchTicketStatus(string $ticketKey): ?string;
/**
* @param non-empty-list<non-empty-string> $ticketKeys
*
* @return array<non-empty-string, null|string> Map using the ticket-key as key and Status name or null if ticket doesn't exist as value
*/
public function fetchTicketStatus(array $ticketKeys): array;

/** @return non-empty-string */
public static function getKeyPattern(): string;
Expand Down
Loading

0 comments on commit a197314

Please sign in to comment.