Skip to content

Commit a2d91ae

Browse files
ThavarshanThavarshan
committed
Refactor and enhance unit tests for HTTP request handling and retry mechanisms
- Simplified the ManagesRetriesTest by using a mock class for testing retry logic. - Consolidated tests for retryable status codes and exceptions into a single test method. - Improved backoff delay calculations in retry tests. - Updated PerformsHttpRequestsTest to utilize GuzzleHttp client mocks for more accurate request testing. - Added tests for various HTTP methods (GET, POST, PUT, DELETE, PATCH, OPTIONS) with appropriate assertions. - Enhanced exception handling tests to verify error messages and retry behavior. - Removed unnecessary reflection methods and streamlined test setup.
1 parent b985c19 commit a2d91ae

19 files changed

Lines changed: 1925 additions & 2607 deletions

‎README.md‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
[![Fetch PHP](./assets/Banner.png)](https://github.com/Thavarshan/fetch-php)
2-
31
# Fetch PHP
42

53
[![Latest Version on Packagist](https://img.shields.io/packagist/v/jerome/fetch-php.svg)](https://packagist.org/packages/jerome/fetch-php)
@@ -207,9 +205,12 @@ try {
207205

208206
```php
209207
// Set up an async request
210-
$promise = fetch_client()
211-
->async()
212-
->get('https://api.example.com/users');
208+
// Get the handler for async operations
209+
$handler = fetch_client()->getHandler();
210+
$handler->async();
211+
212+
// Make the async request
213+
$promise = $handler->get('https://api.example.com/users');
213214

214215
// Handle the result with callbacks
215216
$promise->then(
@@ -431,9 +432,10 @@ try {
431432
}
432433

433434
// Asynchronous error handling
434-
fetch_client()
435-
->async()
436-
->get('https://api.example.com/nonexistent')
435+
$handler = fetch_client()->getHandler();
436+
$handler->async();
437+
438+
$promise = $handler->get('https://api.example.com/nonexistent')
437439
->then(function ($response) {
438440
if ($response->isSuccess()) {
439441
return $response->json();

‎src/Fetch/Concerns/ConfiguresRequests.php‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use Fetch\Interfaces\ClientHandler;
99
use GuzzleHttp\Cookie\CookieJarInterface;
1010
use InvalidArgumentException;
11-
use ValueError;
1211

1312
trait ConfiguresRequests
1413
{
@@ -171,7 +170,7 @@ public function withHeader(string $header, mixed $value): ClientHandler
171170
public function withBody(array|string $body, string|ContentType $contentType = ContentType::JSON): ClientHandler
172171
{
173172
// Convert string content type to enum if necessary
174-
$contentTypeEnum = $this->normalizeContentType($contentType);
173+
$contentTypeEnum = ContentType::normalizeContentType($contentType);
175174
$contentTypeValue = $contentTypeEnum instanceof ContentType
176175
? $contentTypeEnum->value
177176
: $contentTypeEnum;
@@ -389,7 +388,7 @@ protected function configureRequestBody(mixed $body = null, string|ContentType $
389388
}
390389

391390
// Normalize content type
392-
$contentTypeEnum = $this->normalizeContentType($contentType);
391+
$contentTypeEnum = ContentType::normalizeContentType($contentType);
393392

394393
if (is_array($body)) {
395394
match ($contentTypeEnum) {
@@ -404,24 +403,4 @@ protected function configureRequestBody(mixed $body = null, string|ContentType $
404403

405404
$this->withBody($body, $contentType);
406405
}
407-
408-
/**
409-
* Normalize a content type to an enum if possible.
410-
*
411-
* @param string|ContentType $contentType The content type
412-
* @return string|ContentType The normalized content type
413-
*/
414-
protected function normalizeContentType(string|ContentType $contentType): string|ContentType
415-
{
416-
if ($contentType instanceof ContentType) {
417-
return $contentType;
418-
}
419-
420-
try {
421-
return ContentType::tryFromString($contentType, ContentType::JSON);
422-
} catch (ValueError $e) {
423-
// If it's not a valid enum value, keep it as a string
424-
return $contentType;
425-
}
426-
}
427406
}

‎src/Fetch/Concerns/HandlesUris.php‎

Lines changed: 85 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,25 @@ trait HandlesUris
1818
*/
1919
protected function buildFullUri(string $uri): string
2020
{
21+
// Get base URI and query parameters from options
2122
$baseUri = $this->options['base_uri'] ?? '';
2223
$queryParams = $this->options['query'] ?? [];
2324

24-
// Early return if URI is empty
25-
if (empty($uri) && empty($baseUri)) {
26-
throw new InvalidArgumentException('URI cannot be empty');
25+
// Normalize URIs before processing
26+
$uri = $this->normalizeUri($uri);
27+
if (! empty($baseUri)) {
28+
$baseUri = $this->normalizeUri($baseUri);
2729
}
2830

29-
// Handle absolute URLs
30-
if ($this->isAbsoluteUrl($uri)) {
31-
return $this->appendQueryParameters($uri, $queryParams);
32-
}
31+
// Validate inputs
32+
$this->validateUriInputs($uri, $baseUri);
3333

34-
// For relative URLs, require a base URI
35-
if (empty($baseUri)) {
36-
throw new InvalidArgumentException(
37-
"Relative URI '{$uri}' cannot be used without a base URI. ".
38-
'Set a base URI using the baseUri() method.'
39-
);
40-
}
34+
// Build the final URI
35+
$fullUri = $this->isAbsoluteUrl($uri)
36+
? $uri
37+
: $this->joinUriPaths($baseUri, $uri);
4138

42-
// Handle relative URLs with base URI
43-
$fullUri = $this->combineBaseWithRelativeUri($baseUri, $uri);
44-
45-
// Append query parameters if they exist
39+
// Add query parameters if any
4640
return $this->appendQueryParameters($fullUri, $queryParams);
4741
}
4842

@@ -60,6 +54,35 @@ protected function getFullUri(): string
6054
return $this->buildFullUri($uri);
6155
}
6256

57+
/**
58+
* Validate URI and base URI inputs.
59+
*
60+
* @param string $uri The URI or path
61+
* @param string $baseUri The base URI
62+
*
63+
* @throws InvalidArgumentException If validation fails
64+
*/
65+
protected function validateUriInputs(string $uri, string $baseUri): void
66+
{
67+
// Check if we have any URI to work with
68+
if (empty($uri) && empty($baseUri)) {
69+
throw new InvalidArgumentException('URI cannot be empty');
70+
}
71+
72+
// For relative URIs, ensure we have a base URI
73+
if (! $this->isAbsoluteUrl($uri) && empty($baseUri)) {
74+
throw new InvalidArgumentException(
75+
"Relative URI '{$uri}' cannot be used without a base URI. ".
76+
'Set a base URI using the baseUri() method.'
77+
);
78+
}
79+
80+
// Ensure base URI is valid if provided
81+
if (! empty($baseUri) && ! $this->isAbsoluteUrl($baseUri)) {
82+
throw new InvalidArgumentException("Invalid base URI: {$baseUri}");
83+
}
84+
}
85+
6386
/**
6487
* Check if a URI is an absolute URL.
6588
*
@@ -72,22 +95,15 @@ protected function isAbsoluteUrl(string $uri): bool
7295
}
7396

7497
/**
75-
* Combine a base URI with a relative URI.
98+
* Join base URI with a path properly.
7699
*
77100
* @param string $baseUri The base URI
78-
* @param string $relativeUri The relative URI
101+
* @param string $path The path to append
79102
* @return string The combined URI
80103
*/
81-
protected function combineBaseWithRelativeUri(string $baseUri, string $relativeUri): string
104+
protected function joinUriPaths(string $baseUri, string $path): string
82105
{
83-
// Ensure base URI is valid if not empty
84-
if (! empty($baseUri) && ! $this->isAbsoluteUrl($baseUri)) {
85-
throw new InvalidArgumentException("Invalid base URI: {$baseUri}");
86-
}
87-
88-
// Remove trailing slash from base URI and leading slash from relative URI
89-
// Then combine them with a forward slash
90-
return rtrim($baseUri, '/').'/'.ltrim($relativeUri, '/');
106+
return rtrim($baseUri, '/').'/'.ltrim($path, '/');
91107
}
92108

93109
/**
@@ -103,41 +119,66 @@ protected function appendQueryParameters(string $uri, array $queryParams): strin
103119
return $uri;
104120
}
105121

106-
// Check if the URI has a fragment
107-
$fragments = explode('#', $uri, 2);
108-
$baseUri = $fragments[0];
109-
$fragment = isset($fragments[1]) ? '#'.$fragments[1] : '';
122+
// Split URI to preserve any fragment
123+
[$baseUri, $fragment] = $this->splitUriFragment($uri);
110124

111-
// Parse the URI to determine if it already has query parameters
112-
$parsedUrl = parse_url($baseUri);
113-
114-
// Determine the separator based on whether the URI already has query parameters
115-
$separator = isset($parsedUrl['query']) && ! empty($parsedUrl['query']) ? '&' : '?';
125+
// Determine the separator based on URI structure
126+
$separator = $this->getQuerySeparator($baseUri);
116127

117128
// Build the query string
118129
$queryString = http_build_query($queryParams);
119130

131+
// Combine everything
132+
return $baseUri.$separator.$queryString.$fragment;
133+
}
134+
135+
/**
136+
* Split a URI into its base and fragment parts.
137+
*
138+
* @param string $uri The URI to split
139+
* @return array{0: string, 1: string} [baseUri, fragment]
140+
*/
141+
protected function splitUriFragment(string $uri): array
142+
{
143+
$fragments = explode('#', $uri, 2);
144+
$baseUri = $fragments[0];
145+
$fragment = isset($fragments[1]) ? '#'.$fragments[1] : '';
146+
147+
return [$baseUri, $fragment];
148+
}
149+
150+
/**
151+
* Determine the appropriate query string separator for a URI.
152+
*
153+
* @param string $uri The URI
154+
* @return string The separator ('?' or '&' or '')
155+
*/
156+
protected function getQuerySeparator(string $uri): string
157+
{
120158
// Handle special case: URI already ends with a question mark
121-
if (str_ends_with($baseUri, '?')) {
122-
return $baseUri.$queryString.$fragment;
159+
if (str_ends_with($uri, '?')) {
160+
return '';
123161
}
124162

125-
// Append the query string to the URI before any fragment
126-
return $baseUri.$separator.$queryString.$fragment;
163+
// Check if the URI already has query parameters
164+
$parsedUrl = parse_url($uri);
165+
166+
return isset($parsedUrl['query']) && ! empty($parsedUrl['query']) ? '&' : '?';
127167
}
128168

129169
/**
130-
* Normalize a URI by ensuring it has the correct format.
170+
* Normalize a URI by removing redundant slashes.
131171
*
132172
* @param string $uri The URI to normalize
133173
* @return string The normalized URI
134174
*/
135175
protected function normalizeUri(string $uri): string
136176
{
137-
// Remove multiple consecutive slashes (except in the scheme)
177+
// Extract scheme if present (e.g., http://)
138178
if (preg_match('~^(https?://)~i', $uri, $matches)) {
139179
$scheme = $matches[1];
140180
$rest = substr($uri, strlen($scheme));
181+
// Normalize consecutive slashes in the path
141182
$rest = preg_replace('~//+~', '/', $rest);
142183

143184
return $scheme.$rest;

‎src/Fetch/Concerns/ManagesPromises.php‎

Lines changed: 8 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Fetch\Interfaces\ClientHandler;
88
use InvalidArgumentException;
9+
use Matrix\Exceptions\TimeoutException;
910
use React\Promise\PromiseInterface;
1011
use RuntimeException;
1112
use Throwable;
@@ -197,7 +198,7 @@ public function resolve(mixed $value): PromiseInterface
197198
*/
198199
public function reject(mixed $reason): PromiseInterface
199200
{
200-
return reject($reason);
201+
return reject(is_string($reason) ? new RuntimeException($reason) : $reason);
201202
}
202203

203204
/**
@@ -242,45 +243,13 @@ public function map(array $items, callable $callback, int $concurrency = 5): Pro
242243
*/
243244
protected function awaitWithTimeout(PromiseInterface $promise, float $timeout): mixed
244245
{
245-
$timeoutMicro = (int) ($timeout * 1000000);
246-
$result = null;
247-
$isResolved = false;
248-
$error = null;
249-
250-
// Create a promise that will resolve or reject after the timeout
251-
$timeoutPromise = $this->createTimeoutPromise($timeout);
252-
253-
// Race the original promise against the timeout
254-
$racePromise = race([$promise, $timeoutPromise]);
255-
256-
// Set up the callback for when the promise resolves
257-
$promise->then(
258-
function ($value) use (&$result, &$isResolved) {
259-
$result = $value;
260-
$isResolved = true;
261-
},
262-
function ($reason) use (&$error) {
263-
$error = $reason;
264-
}
265-
);
266-
267-
// Wait for either the promise to resolve or the timeout to occur
268-
await($racePromise);
269-
270-
// If we have an error, throw it
271-
if ($error !== null) {
272-
if ($error instanceof Throwable) {
273-
throw $error;
274-
}
275-
throw new RuntimeException('Promise rejected: '.(string) $error);
276-
}
277-
278-
// If we didn't resolve, it means we timed out
279-
if (! $isResolved) {
280-
throw new RuntimeException("Promise timed out after {$timeout} seconds");
246+
try {
247+
// Use Matrix's built-in timeout function
248+
return await(timeout($promise, $timeout));
249+
} catch (TimeoutException $e) {
250+
// Convert to RuntimeException for consistency with your API
251+
throw new RuntimeException("Promise timed out after {$timeout} seconds", 0, $e);
281252
}
282-
283-
return $result;
284253
}
285254

286255
/**

0 commit comments

Comments
 (0)