Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5c61f33
Initial plan
Copilot Nov 28, 2025
c29537f
Add middleware/interceptor system for request/response transformation
Copilot Nov 28, 2025
3d07422
Fix code review comments: improve docblock and JSON validation
Copilot Nov 28, 2025
621c234
Update src/Fetch/Interfaces/ClientHandler.php
Thavarshan Nov 29, 2025
4665e4d
Update src/Fetch/Concerns/SupportsMiddleware.php
Thavarshan Nov 29, 2025
9b25fa1
Update src/Fetch/Concerns/SupportsMiddleware.php
Thavarshan Nov 29, 2025
eba3dd5
Initial plan
Copilot Nov 28, 2025
d9122a1
Add connection pooling and HTTP/2 support implementation
Copilot Nov 28, 2025
5892023
Fix code review issues: separate curl multi options, fix URI in excep…
Copilot Nov 28, 2025
9428c55
Simplify metrics incrementing, improve curl options comment
Copilot Nov 28, 2025
95539e4
Fix code style issues: proper namespace imports and class element ord…
Copilot Nov 28, 2025
7ab3d2b
Update src/Fetch/Concerns/ManagesConnectionPool.php
Thavarshan Nov 28, 2025
6ef63be
Update src/Fetch/Concerns/ManagesConnectionPool.php
Thavarshan Nov 28, 2025
25061d0
Update src/Fetch/Pool/DnsCache.php
Thavarshan Nov 28, 2025
798891b
Update src/Fetch/Concerns/ManagesConnectionPool.php
Thavarshan Nov 28, 2025
56baf59
Update src/Fetch/Pool/HostConnectionPool.php
Thavarshan Nov 28, 2025
af63d60
Update src/Fetch/Concerns/ManagesConnectionPool.php
Thavarshan Nov 28, 2025
b094bb5
Update src/Fetch/Concerns/ManagesConnectionPool.php
Thavarshan Nov 28, 2025
5a167c4
Update src/Fetch/Pool/ConnectionPool.php
Thavarshan Nov 28, 2025
7e8c750
Address code review feedback: improve documentation and fix issues
Copilot Nov 28, 2025
b477660
Initial plan for RFC 7234 HTTP caching support
Copilot Nov 28, 2025
74d4128
Implement RFC 7234 HTTP caching support with memory and file backends
Copilot Nov 28, 2025
a556117
Fix code review issues: improve method signatures and remove deprecat…
Copilot Nov 28, 2025
e08995e
Fix code style issues: reorder class elements and remove superfluous …
Copilot Nov 28, 2025
e956c1a
Update src/Fetch/Cache/CacheKeyGenerator.php
Thavarshan Nov 28, 2025
4feb84c
Update src/Fetch/Cache/CacheInterface.php
Thavarshan Nov 28, 2025
06de2aa
Update src/Fetch/Cache/FileCache.php
Thavarshan Nov 28, 2025
bd31a7b
Update src/Fetch/Cache/FileCache.php
Thavarshan Nov 28, 2025
446203c
Update tests/Unit/CacheTest.php
Thavarshan Nov 28, 2025
dc4cdeb
Update src/Fetch/Concerns/ManagesCache.php
Thavarshan Nov 28, 2025
2a4042b
Fix TTL behavior consistency and use JSON serialization for security
Copilot Nov 28, 2025
8a233c7
Refactor cache interface and related tests; remove unused set method …
Thavarshan Nov 28, 2025
c0efed2
Address code review feedback: status codes consistency, race conditio…
Copilot Nov 28, 2025
e30c597
Refactor executeSyncRequestWithCache method to improve caching suppor…
Thavarshan Nov 28, 2025
51aa72c
Refactor cache interface and implementations to return boolean for se…
Thavarshan Nov 28, 2025
9bc7a75
Refactor middleware class definitions for improved readability
Thavarshan Nov 29, 2025
d971e7b
Address code review comments: improve stream handling, add validation…
Copilot Nov 29, 2025
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
87 changes: 86 additions & 1 deletion src/Fetch/Concerns/PerformsHttpRequests.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use GuzzleHttp\Exception\RequestException as GuzzleRequestException;
use GuzzleHttp\Psr7\Request as GuzzleRequest;
use Matrix\Exceptions\AsyncException;
use Psr\Http\Message\RequestInterface;
use React\Promise\PromiseInterface;

use function Matrix\Support\async;
Expand Down Expand Up @@ -187,14 +188,98 @@ public function sendRequest(
$handler->logRequest($methodStr, $fullUri, $guzzleOptions);
}

// Send the request (async or sync)
// Check if middleware is available and should be used
if (method_exists($handler, 'hasMiddleware') && $handler->hasMiddleware()) {
return $handler->executeWithMiddleware($methodStr, $fullUri, $guzzleOptions, $startTime);
}

// Send the request (async or sync) without middleware
if ($handler->isAsync) {
return $handler->executeAsyncRequest($methodStr, $fullUri, $guzzleOptions);
} else {
return $handler->executeSyncRequest($methodStr, $fullUri, $guzzleOptions, $startTime);
}
}

/**
* Execute the request through the middleware pipeline.
*
* @param string $method The HTTP method
* @param string $uri The full URI
* @param array<string, mixed> $options The Guzzle options
* @param float $startTime The request start time
* @return ResponseInterface|PromiseInterface The response or promise
*/
protected function executeWithMiddleware(
string $method,
string $uri,
array $options,
float $startTime,
): ResponseInterface|PromiseInterface {
// Create a PSR-7 request from the current state
$body = null;
if (isset($options['body'])) {
$body = $options['body'];
} elseif (isset($options['json'])) {
$body = json_encode($options['json']);
}

$psrRequest = new GuzzleRequest(
$method,
$uri,
$options['headers'] ?? [],
$body
);

// Define the core handler that will be called after all middleware
$coreHandler = function (RequestInterface $request) use ($options, $startTime): ResponseInterface|PromiseInterface {
// Extract method and URI from the (potentially modified) request
$method = $request->getMethod();
$uri = (string) $request->getUri();

// Merge any headers from the modified request back into options
$headers = [];
foreach ($request->getHeaders() as $name => $values) {
$headers[$name] = implode(', ', $values);
}
$options['headers'] = array_merge($options['headers'] ?? [], $headers);

// Handle body from modified request
$body = $request->getBody();
if ($body->getSize() > 0) {
$body->rewind();
$bodyContents = $body->getContents();
$body->rewind();
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 code calls $body->rewind() multiple times (lines 250, 252) without checking if the stream is seekable. Non-seekable streams (like php://input in some contexts) will throw an exception when rewind() is called.

Consider checking if the stream is seekable before rewinding:

$body = $request->getBody();
if ($body->getSize() > 0) {
    if ($body->isSeekable()) {
        $body->rewind();
    }
    $bodyContents = $body->getContents();
    if ($body->isSeekable()) {
        $body->rewind();
    }
    
    // ... rest of the logic
}

Alternatively, since you're converting the body to a string anyway, you might not need to rewind it back after reading.

Suggested change
$body->rewind();
$bodyContents = $body->getContents();
$body->rewind();
if ($body->isSeekable()) {
$body->rewind();
}
$bodyContents = $body->getContents();
if ($body->isSeekable()) {
$body->rewind();
}

Copilot uses AI. Check for mistakes.

// Check if it's JSON
$contentType = $request->getHeaderLine('Content-Type');
if (str_contains($contentType, 'application/json')) {
$decoded = json_decode($bodyContents, true);
if (json_last_error() === JSON_ERROR_NONE) {
$options['json'] = $decoded;
unset($options['body']);
} else {
$options['body'] = $bodyContents;
unset($options['json']);
}
} else {
$options['body'] = $bodyContents;
unset($options['json']);
}
}
Comment on lines +313 to +340
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 body size check using $body->getSize() > 0 may not work reliably for all stream types. Some streams (like php://input or certain custom streams) return null for getSize(), which would fail this check even when the body contains data.

Consider checking if the body is readable and has content:

if ($body->isReadable() && ($body->getSize() === null || $body->getSize() > 0)) {
    // Handle body...
}

Or alternatively, always try to read the body and check if it's non-empty:

$body->rewind();
$bodyContents = $body->getContents();
$body->rewind();

if (!empty($bodyContents)) {
    // Check if it's JSON...
}

Copilot uses AI. Check for mistakes.

// Execute the actual request
if ($this->isAsync) {
return $this->executeAsyncRequest($method, $uri, $options);
} else {
return $this->executeSyncRequest($method, $uri, $options, $startTime);
}
Comment on lines +297 to +347
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 $options variable is being captured by the closure and then modified within it (lines 245, 259-260, 262-263, 266-267). Since $options is captured by value, these modifications will affect the variable within the closure scope, potentially causing the original options to be merged with modified options multiple times if the middleware pipeline is executed repeatedly or if there are multiple modifications.

Consider using a copy of the options array for modifications:

$coreHandler = function (RequestInterface $request) use ($options, $startTime): ResponseInterface|PromiseInterface {
    // Work with a copy to avoid side effects
    $modifiedOptions = $options;
    
    // ... rest of the logic using $modifiedOptions instead of $options
    
    if ($this->isAsync) {
        return $this->executeAsyncRequest($method, $uri, $modifiedOptions);
    } else {
        return $this->executeSyncRequest($method, $uri, $modifiedOptions, $startTime);
    }
};

Copilot uses AI. Check for mistakes.
};

// Execute through the middleware pipeline
return $this->getMiddlewarePipeline()->handle($psrRequest, $coreHandler);
}

/**
* Sends an HTTP request with the specified parameters.
*
Expand Down
125 changes: 125 additions & 0 deletions src/Fetch/Concerns/SupportsMiddleware.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
<?php

declare(strict_types=1);

namespace Fetch\Concerns;

use Fetch\Http\MiddlewarePipeline;
use Fetch\Interfaces\ClientHandler;
use Fetch\Interfaces\MiddlewareInterface;

/**
* Trait for adding middleware support to HTTP clients.
*
* This trait provides methods for managing and executing middleware
* that can transform requests and responses.
*/
trait SupportsMiddleware
{
/**
* The middleware pipeline instance.
*/
protected ?MiddlewarePipeline $middlewarePipeline = 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.

When the handler is cloned on line 159 of sendRequest(), the $middlewarePipeline property (which is an object reference) will be shallow-copied. This means both the original handler and the cloned handler will share the same middleware pipeline instance. If middleware is added to one handler, it will affect the other.

This could lead to unexpected behavior where middleware added to a cloned handler affects the original handler. Consider implementing a __clone() method in the ClientHandler class or the trait:

public function __clone(): void
{
    if ($this->middlewarePipeline !== null) {
        $middleware = $this->middlewarePipeline->getMiddleware();
        $this->middlewarePipeline = new MiddlewarePipeline($middleware);
    }
}

Copilot uses AI. Check for mistakes.

/**
* Set multiple middleware at once.
*
Comment thread
Thavarshan marked this conversation as resolved.
Outdated
* @param array<MiddlewareInterface|array{middleware: MiddlewareInterface, priority: int}> $middleware
* @return $this
*/
public function middleware(array $middleware): ClientHandler
{
$this->middlewarePipeline = new MiddlewarePipeline($middleware);

Comment on lines +35 to +36
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 middleware() method creates a new MiddlewarePipeline instance, which replaces any existing pipeline and discards previously added middleware. This could lead to unexpected behavior if middleware was added using addMiddleware() or prependMiddleware() before calling middleware().

Consider either:

  1. Adding to the existing pipeline instead of replacing it:
public function middleware(array $middleware): ClientHandler
{
    $pipeline = $this->getMiddlewarePipeline();
    $pipeline->clear(); // Clear existing if that's the intent
    
    foreach ($middleware as $item) {
        if ($item instanceof MiddlewareInterface) {
            $pipeline->add($item, 0);
        } elseif (is_array($item) && isset($item['middleware'], $item['priority'])) {
            $pipeline->add($item['middleware'], $item['priority']);
        }
    }
    
    return $this;
}
  1. Or documenting this behavior clearly in the PHPDoc that calling middleware() replaces all existing middleware.
Suggested change
$this->middlewarePipeline = new MiddlewarePipeline($middleware);
$pipeline = $this->getMiddlewarePipeline();
$pipeline->clear();
foreach ($middleware as $item) {
if ($item instanceof MiddlewareInterface) {
$pipeline->add($item, 0);
} elseif (is_array($item) && isset($item['middleware'], $item['priority'])) {
$pipeline->add($item['middleware'], $item['priority']);
}
}

Copilot uses AI. Check for mistakes.
return $this;
}

/**
* Add a single middleware to the pipeline.
*
* @param MiddlewareInterface $middleware The middleware to add
* @param int $priority Higher priority middleware runs first (default: 0)
* @return $this
*/
public function addMiddleware(MiddlewareInterface $middleware, int $priority = 0): ClientHandler
{
$this->getMiddlewarePipeline()->add($middleware, $priority);

return $this;
}

/**
* Prepend middleware to run first (with highest priority).
*
* @param MiddlewareInterface $middleware The middleware to prepend
* @return $this
*/
public function prependMiddleware(MiddlewareInterface $middleware): ClientHandler
{
$this->getMiddlewarePipeline()->prepend($middleware);

return $this;
}

/**
* Get the middleware pipeline instance.
*/
public function getMiddlewarePipeline(): MiddlewarePipeline
{
if ($this->middlewarePipeline === null) {
$this->middlewarePipeline = new MiddlewarePipeline;
}

return $this->middlewarePipeline;
}

/**
* Check if any middleware is registered.
*/
public function hasMiddleware(): bool
{
return $this->middlewarePipeline !== null && ! $this->middlewarePipeline->isEmpty();
}

/**
* Clear all middleware from the pipeline.
*
* @return $this
*/
public function clearMiddleware(): ClientHandler
{
if ($this->middlewarePipeline !== null) {
$this->middlewarePipeline->clear();
}

return $this;
}

/**
* Conditionally add middleware.
*
* @param bool $condition The condition to check
* @param callable $callback Callback that receives $this and should add middleware
* @return $this
Comment on lines +101 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.

[nitpick] The PHPDoc comment for the when() method states "Callback that receives $this and should add middleware", but it doesn't specify what the callback should return. The callback's return value is currently ignored. Consider clarifying this in the documentation:

/**
 * Conditionally add middleware.
 *
 * @param  bool  $condition  The condition to check
 * @param  callable(static): void  $callback  Callback that receives $this for adding middleware
 * @return $this
 */

This makes it clearer that the callback doesn't need to return anything and receives the handler instance as a parameter.

Copilot uses AI. Check for mistakes.
*/
public function when(bool $condition, callable $callback): ClientHandler
{
if ($condition) {
$callback($this);
}

return $this;
}

/**
* Conditionally add middleware (inverse of when).
*
* @param bool $condition The condition to check
* @param callable $callback Callback that receives $this and should add middleware
Comment thread
Thavarshan marked this conversation as resolved.
Outdated
* @return $this
*/
public function unless(bool $condition, callable $callback): ClientHandler
{
return $this->when(! $condition, $callback);
}
}
4 changes: 3 additions & 1 deletion src/Fetch/Http/ClientHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Fetch\Concerns\ManagesPromises;
use Fetch\Concerns\ManagesRetries;
use Fetch\Concerns\PerformsHttpRequests;
use Fetch\Concerns\SupportsMiddleware;
use Fetch\Enum\ContentType;
use Fetch\Enum\Method;
use Fetch\Interfaces\ClientHandler as ClientHandlerInterface;
Expand All @@ -32,7 +33,8 @@ class ClientHandler implements ClientHandlerInterface
ManagesDebugAndProfiling,
ManagesPromises,
ManagesRetries,
PerformsHttpRequests;
PerformsHttpRequests,
SupportsMiddleware;

/**
* Default options for the request.
Expand Down
Loading
Loading