Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
262 changes: 262 additions & 0 deletions src/Fetch/Concerns/ManagesConnectionPool.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
<?php

declare(strict_types=1);

namespace Fetch\Concerns;

use Fetch\Interfaces\ClientHandler;
use Fetch\Pool\ConnectionPool;
use Fetch\Pool\DnsCache;
use Fetch\Pool\Http2Configuration;
use Fetch\Pool\PoolConfiguration;

/**
* Trait for managing connection pooling and HTTP/2 support.
*/
trait ManagesConnectionPool
{
/**
* The connection pool instance.
*/
protected static ?ConnectionPool $connectionPool = null;
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using static properties for connection pool and DNS cache creates a shared state across all ClientHandler instances. This could lead to race conditions in multi-threaded environments (e.g., when using parallel processing or async operations) and makes it difficult to isolate connections per handler. Consider making these instance-level properties or adding proper synchronization mechanisms.

Copilot uses AI. Check for mistakes.

/**
* The DNS cache instance.
*/
protected static ?DnsCache $dnsCache = null;
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the connection pool, the static DNS cache creates shared state across all ClientHandler instances which could lead to race conditions and unexpected behavior when multiple handlers are used concurrently. Consider making this an instance property instead.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +33
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using static properties for $connectionPool and $dnsCache creates shared global state across all ClientHandler instances. This can lead to unexpected behavior when:

  1. Multiple handlers are created with different pool configurations (the last one wins)
  2. Tests run in parallel and share state
  3. Different parts of an application need different pooling strategies

While this may be intentional for connection sharing, it should be clearly documented that these are global singletons. Consider whether instance-level pools would be more appropriate, or add documentation warning about this shared state behavior.

Copilot uses AI. Check for mistakes.

/**
* The HTTP/2 configuration.
*/
protected ?Http2Configuration $http2Config = null;

/**
* Whether connection pooling is enabled for this handler.
*/
protected bool $poolingEnabled = false;

/**
* Configure connection pooling for this handler.
*
* @param array<string, mixed>|bool $config Pool configuration or boolean to enable/disable
* @return $this
*/
public function withConnectionPool(array|bool $config = true): ClientHandler
{
if (is_bool($config)) {
Comment thread
Thavarshan marked this conversation as resolved.
$this->poolingEnabled = $config;

return $this;
}

$this->poolingEnabled = true;

// Initialize or update the global pool
self::$connectionPool = ConnectionPool::fromArray($config);

// Initialize DNS cache if TTL is specified
if (isset($config['dns_cache_ttl'])) {
self::$dnsCache = new DnsCache((int) $config['dns_cache_ttl']);
}

return $this;
}

/**
* Configure HTTP/2 for this handler.
*
* @param array<string, mixed>|bool $config HTTP/2 configuration or boolean to enable/disable
* @return $this
*/
public function withHttp2(array|bool $config = true): ClientHandler
{
if (is_bool($config)) {
$this->http2Config = new Http2Configuration(enabled: $config);
} else {
$this->http2Config = Http2Configuration::fromArray($config);
}

// Apply HTTP/2 curl options to the handler options
if ($this->http2Config->isEnabled()) {
$curlOptions = $this->http2Config->getCurlOptions();
if (! empty($curlOptions)) {
$existingCurl = $this->options['curl'] ?? [];
// Use + operator to preserve integer keys (CURL constants)
// and give priority to existing options over defaults
$this->options['curl'] = $existingCurl + $curlOptions;
Comment on lines +103 to +106
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states 'give priority to existing options over defaults', but with the + operator, if the same key exists in both arrays, the value from the left operand ($existingCurl) is kept and the right operand ($curlOptions) is ignored. This means HTTP/2 options from $curlOptions won't override existing curl options. If this is intentional, the comment is correct, but if HTTP/2 options should take precedence, use array_merge() or reverse the operands.

Copilot uses AI. Check for mistakes.
}

// Set HTTP version in options
$this->options['version'] = 2.0;
}

return $this;
}

/**
* Get the connection pool instance.
*
* @return ConnectionPool|null The pool instance or null if not configured
*/
public function getConnectionPool(): ?ConnectionPool
{
return self::$connectionPool;
}

/**
* Get the DNS cache instance.
*
* @return DnsCache|null The DNS cache or null if not configured
*/
public function getDnsCache(): ?DnsCache
{
return self::$dnsCache;
}

/**
* Get the HTTP/2 configuration.
*
* @return Http2Configuration|null The HTTP/2 config or null if not configured
*/
public function getHttp2Config(): ?Http2Configuration
{
return $this->http2Config;
}

/**
* Check if connection pooling is enabled.
*/
public function isPoolingEnabled(): bool
{
return $this->poolingEnabled && self::$connectionPool !== null && self::$connectionPool->isEnabled();
}

/**
* Check if HTTP/2 is enabled.
*/
public function isHttp2Enabled(): bool
{
return $this->http2Config !== null && $this->http2Config->isEnabled();
}

/**
* Get connection pool statistics.
*
* @return array<string, mixed>
*/
public function getPoolStats(): array
{
if (self::$connectionPool === null) {
return ['enabled' => false];
}

return self::$connectionPool->getStats();
}

/**
* Get DNS cache statistics.
*
* @return array<string, mixed>
*/
public function getDnsCacheStats(): array
{
if (self::$dnsCache === null) {
return ['enabled' => false];
}

return array_merge(['enabled' => true], self::$dnsCache->getStats());
}

/**
* Clear the DNS cache.
*
* @param string|null $hostname Specific hostname to clear, or null for all
* @return $this
*/
public function clearDnsCache(?string $hostname = null): ClientHandler
{
if (self::$dnsCache !== null) {
self::$dnsCache->clear($hostname);
}

return $this;
}

/**
* Close all pooled connections.
*
* @return $this
*/
public function closeAllConnections(): ClientHandler
{
if (self::$connectionPool !== null) {
self::$connectionPool->closeAll();
}

return $this;
}

/**
* Reset the global connection pool and DNS cache.
*
* @return $this
*/
public function resetPool(): ClientHandler
{
if (self::$connectionPool !== null) {
self::$connectionPool->closeAll();
}
self::$connectionPool = null;
self::$dnsCache = null;
$this->poolingEnabled = false;

return $this;
}

Comment thread
Thavarshan marked this conversation as resolved.
/**
* Initialize connection pooling with default configuration.
*
* @param PoolConfiguration|null $config Optional pool configuration
*/
protected static function initializePool(?PoolConfiguration $config = null): void
{
if (self::$connectionPool === null) {
$poolConfig = $config ?? new PoolConfiguration;
self::$connectionPool = new ConnectionPool($poolConfig);
self::$dnsCache = new DnsCache($poolConfig->getDnsCacheTtl());
}
}

/**
* Get cURL options for HTTP/2 support.
*
* @return array<int, mixed>
*/
protected function getHttp2CurlOptions(): array
{
if ($this->http2Config === null) {
return [];
}

return $this->http2Config->getCurlOptions();
}
Comment thread
Thavarshan marked this conversation as resolved.

/**
* Resolve a hostname using the DNS cache.
*
* @param string $hostname The hostname to resolve
* @return string|null The resolved IP address or null
*/
protected function resolveHostname(string $hostname): ?string
{
if (self::$dnsCache === null) {
return null;
}

try {
return self::$dnsCache->resolveFirst($hostname);
} catch (\Throwable) {
return null;
}
}
Comment on lines +259 to +270
Copy link

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The resolveHostname method silently catches and ignores all exceptions, returning null. Consider adding a PHPDoc comment explaining when null is returned (no DNS cache or DNS resolution failure) to help consumers of this method understand the behavior.

Copilot uses AI. Check for mistakes.
Comment thread
Thavarshan marked this conversation as resolved.
Comment thread
Thavarshan marked this conversation as resolved.
}
2 changes: 2 additions & 0 deletions src/Fetch/Http/ClientHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Fetch\Concerns\ConfiguresRequests;
use Fetch\Concerns\HandlesMocking;
use Fetch\Concerns\HandlesUris;
use Fetch\Concerns\ManagesConnectionPool;
use Fetch\Concerns\ManagesDebugAndProfiling;
use Fetch\Concerns\ManagesPromises;
use Fetch\Concerns\ManagesRetries;
Expand All @@ -29,6 +30,7 @@ class ClientHandler implements ClientHandlerInterface
use ConfiguresRequests,
HandlesMocking,
HandlesUris,
ManagesConnectionPool,
ManagesDebugAndProfiling,
ManagesPromises,
ManagesRetries,
Expand Down
83 changes: 83 additions & 0 deletions src/Fetch/Interfaces/ClientHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -573,4 +573,87 @@ public function getDebugOptions(): array;
* Get the last debug info from the most recent request.
*/
public function getLastDebugInfo(): ?\Fetch\Support\DebugInfo;

/**
* Configure connection pooling for this handler.
*
* @param array<string, mixed>|bool $config Pool configuration or boolean to enable/disable
* @return $this
*/
public function withConnectionPool(array|bool $config = true): self;

/**
* Configure HTTP/2 for this handler.
*
* @param array<string, mixed>|bool $config HTTP/2 configuration or boolean to enable/disable
* @return $this
*/
public function withHttp2(array|bool $config = true): self;

/**
* Get the connection pool instance.
*
* @return \Fetch\Pool\ConnectionPool|null The pool instance or null if not configured
*/
public function getConnectionPool(): ?\Fetch\Pool\ConnectionPool;

/**
* Get the DNS cache instance.
*
* @return \Fetch\Pool\DnsCache|null The DNS cache or null if not configured
*/
public function getDnsCache(): ?\Fetch\Pool\DnsCache;

/**
* Get the HTTP/2 configuration.
*
* @return \Fetch\Pool\Http2Configuration|null The HTTP/2 config or null if not configured
*/
public function getHttp2Config(): ?\Fetch\Pool\Http2Configuration;

/**
* Check if connection pooling is enabled.
*/
public function isPoolingEnabled(): bool;

/**
* Check if HTTP/2 is enabled.
*/
public function isHttp2Enabled(): bool;

/**
* Get connection pool statistics.
*
* @return array<string, mixed>
*/
public function getPoolStats(): array;

/**
* Get DNS cache statistics.
*
* @return array<string, mixed>
*/
public function getDnsCacheStats(): array;

/**
* Clear the DNS cache.
*
* @param string|null $hostname Specific hostname to clear, or null for all
* @return $this
*/
public function clearDnsCache(?string $hostname = null): self;

/**
* Close all pooled connections.
*
* @return $this
*/
public function closeAllConnections(): self;

/**
* Reset the global connection pool and DNS cache.
*
* @return $this
*/
public function resetPool(): self;
}
Loading
Loading