Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
271 changes: 271 additions & 0 deletions src/Fetch/Concerns/ManagesConnectionPool.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,271 @@
<?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.
*
* Note: The connection pool and DNS cache are stored as static properties
* to enable connection sharing across all ClientHandler instances. This is
* intentional to maximize connection reuse. However, be aware that:
* - Configuration changes affect all handlers globally
* - In multi-threaded environments, proper synchronization may be needed
* - Use resetPool() to isolate test environments
*/
trait ManagesConnectionPool
{
/**
* The connection pool instance (shared across all handlers).
*/
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 (shared across all handlers).
*/
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;

/**
* 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());
}
}

/**
* 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)) {
$this->poolingEnabled = $config;

return $this;
}

$this->poolingEnabled = true;

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

// Always initialize DNS cache with configuration TTL (uses default if not specified)
$poolConfig = self::$connectionPool->getConfig();
self::$dnsCache = new DnsCache($poolConfig->getDnsCacheTtl());

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;
}

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

return $this->http2Config->getCurlOptions();
}

/**
* Resolve a hostname using the DNS cache.
*
* Returns null if DNS cache is not configured or if DNS resolution fails.
* This method silently catches exceptions to allow fallback behavior.
*
* @param string $hostname The hostname to resolve
* @return string|null The resolved IP address, or null if not available
*/
protected function resolveHostname(string $hostname): ?string
{
if (self::$dnsCache === null) {
return null;
}

try {
return self::$dnsCache->resolveFirst($hostname);
} catch (\Throwable) {
return null;
}
}
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 @@ -8,6 +8,7 @@
use Fetch\Concerns\HandlesMocking;
use Fetch\Concerns\HandlesUris;
use Fetch\Concerns\ManagesCache;
use Fetch\Concerns\ManagesConnectionPool;
use Fetch\Concerns\ManagesDebugAndProfiling;
use Fetch\Concerns\ManagesPromises;
use Fetch\Concerns\ManagesRetries;
Expand All @@ -31,6 +32,7 @@ class ClientHandler implements ClientHandlerInterface
HandlesMocking,
HandlesUris,
ManagesCache,
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 @@ -595,4 +595,87 @@ public function getCache(): ?\Fetch\Cache\CacheInterface;
* Check if caching is enabled.
*/
public function isCacheEnabled(): bool;

/**
* 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