Add adapter pattern with Curl and Swoole HTTP client implementations

[lohanidamodar]

Summary

This PR introduces an adapter pattern to the Fetch library, allowing multiple HTTP client implementations. Two concrete adapters are provided: a Curl adapter (using PHP's cURL extension) and a Swoole adapter (using Swoole's coroutine HTTP client).

Key Changes

• Added Adapter Interface (src/Adapter.php): Defines the contract for HTTP adapters with a send() method

• Curl Adapter (src/Adapter/Curl.php): Implements HTTP requests using PHP's cURL extension with support for:

  • SSL/TLS configuration (certificates, verification, CA bundles)
  • Proxy support with authentication
  • TCP keep-alive settings
  • Custom buffer sizes and HTTP versions
  • Streaming chunks via callback

• Swoole Adapter (src/Adapter/Swoole.php): Implements HTTP requests using Swoole's coroutine HTTP client with support for:

  • Connection pooling and reuse via keep-alive
  • SSL/TLS configuration with hostname verification
  • WebSocket support (masking and compression)
  • Socket buffer and package size configuration
  • Redirect handling with sensitive header filtering
  • Streaming chunks via callback

• Options Classes: Created configuration classes for each adapter:

  • src/Options/Request.php: Common request options (timeout, redirects, user agent)
  • src/Options/Curl.php: Curl-specific configuration
  • src/Options/Swoole.php: Swoole-specific configuration

• Client Updates (src/Client.php):

  • Added constructor accepting an optional Adapter instance (defaults to Curl)
  • Added removeHeader() and clearHeaders() methods
  • Updated fetch() method to support string bodies in addition to arrays
  • Fixed setJsonEncodeFlags() to properly combine flags using bitwise OR
  • Reduced default connectTimeout from 60s to 5s

• Response Improvements (src/Response.php):

  • Enhanced text() method to handle null, string, scalar, and object types
  • Improved json() method with better type handling and error messages
  • Added proper null checks and type conversions

• Test Coverage:

  • Added tests/Adapter/CurlTest.php: Comprehensive tests for Curl adapter
  • Added tests/Adapter/SwooleTest.php: Comprehensive tests for Swoole adapter (skipped if extension unavailable)
  • Updated tests/ClientTest.php: Added tests for default adapter, custom adapter, and Swoole adapter
  • Updated existing tests to use stricter assertions (assertSame instead of assertEquals)

• Infrastructure:

  • Added Dockerfile and docker-compose.yml for containerized testing
  • Updated GitHub Actions workflow to use Docker for testing
  • Updated phpstan.neon to include Swoole IDE helper for static analysis
  • Updated composer.json to require PHP 8.1+ and added phpstan dev dependency

Notable Implementation Details

• Both adapters handle HTTP redirects with configurable limits and sensitive header filtering (authorization, cookie, proxy-authorization, host)
• The Swoole adapter manages a client pool keyed by host:port:ssl combination for connection reuse
• Request options are passed as immutable objects to adapters, allowing for clean separation of concerns
• Chunk callbacks receive Chunk objects with data, size, timestamp, and index information
• The Curl adapter uses a persistent handle that is reset between requests for efficiency

https://claude.ai/code/session_01YNnPHeCYQYWibVJ3m6uKht

[greptile-apps]

Greptile Summary

This PR introduces a well-designed adapter pattern for the Fetch library, providing Curl and Swoole HTTP client implementations backed by clean options value objects and an updated Client class. The infrastructure work (Docker-based CI, PHP 8.1+ requirement, PHPStan at max level) is solid, and the Curl adapter is largely correct. However, the Swoole adapter has three P1 issues that should be addressed before merging:

• Shared client pool is not coroutine-safe – $this->clients is keyed only by host:port:ssl. Two concurrent coroutines targeting the same host share one CoClient, so setMethod(), setHeaders(), and configureBody() calls interleave, producing requests with corrupted state.
• 302/303 redirects re-send the original body – RFC 7231 requires the body to be dropped and the method changed to GET for 302/303 responses. The current code always calls $client->setMethod($method) and configureBody(...) on cross-origin redirects, causing a POST body to be forwarded to the redirect target.
• chunkCallback contract differs from the Curl adapter – In Curl, when a callback is supplied the response body in the returned Response is empty; in Swoole it is always populated. Swoole also fires the callback for each intermediate redirect body rather than only for the final response. Both points violate the Liskov Substitution principle for the shared Adapter interface.

Confidence Score: 3/5

Not safe to merge until the three P1 Swoole issues (concurrency, redirect body, streaming contract) are fixed.

Three P1 findings in the new Swoole adapter represent real defects: incorrect HTTP semantics on redirects, potential request corruption under concurrent coroutine usage, and a broken contract with the Curl adapter for streaming. The Curl adapter and all other changed files are in good shape, but the Swoole implementation needs targeted fixes before production use.

src/Adapter/Swoole.php requires attention for all three P1 issues.

Important Files Changed

Filename	Overview
src/Adapter/Swoole.php	New Swoole HTTP adapter with three P1 issues: shared client pool is not coroutine-safe, 302/303 redirects incorrectly preserve the request body, and streaming callback behaviour is inconsistent with the Curl adapter.
src/Adapter/Curl.php	New Curl adapter wrapping a reusable cURL handle; logic is solid with one minor style issue (missing space after colon in header formatting).
src/Adapter.php	Clean interface definition for the adapter pattern; well-documented with appropriate type hints.
src/Client.php	Client updated to accept an optional Adapter; adds removeHeader/clearHeaders helpers and fixes setJsonEncodeFlags to use bitwise OR; looks correct.
src/Response.php	Enhanced text()/json() methods with null and type guards; straightforward and correct.
src/Options/Curl.php	Well-structured immutable value object for Curl adapter options with sensible defaults.
src/Options/Swoole.php	Well-structured immutable value object for Swoole adapter options with sensible defaults.
src/Options/Request.php	Shared request options (timeout, redirects, user-agent) used by both adapters; clean and correct.
Dockerfile	Multi-stage Dockerfile using PHP 8.4 base image; correctly copies source and exposes the test server.
.github/workflows/tests.yml	CI pipeline migrated to Docker-based testing with GitHub Actions cache for image layers; straightforward update.

Comments Outside Diff (2)

1. src/Adapter/Swoole.php, line 694-711 (link)

   Request body unconditionally re-sent on 302/303 redirects

   RFC 7231 §6.4.2 and §6.4.3 specify that after a 302 or 303 response the user agent must reissue the request as a GET with no message body. The current code always calls $client->setMethod($method) (preserving the original POST/PUT) and $this->configureBody(...) on every cross-origin redirect, regardless of the redirect status code.

   For example, a POST /submit that receives a 302 → /done will incorrectly POST /done with the original body instead of GET /done with no body. This differs from both the Curl adapter (which delegates to CURLOPT_FOLLOWLOCATION and converts POST→GET for 301/302) and from browser/client expectations.

   A fix should check the redirect status code and clear the method/body for 301/302/303:

   if (in_array($statusCode, [301, 302, 303])) {
       $client->setMethod('GET');
       // do not call configureBody
   } else {
       $client->setMethod($method);
       $this->configureBody($client, $body, $headers);
   }

2. src/Adapter/Swoole.php, line 668-680 (link)

   Streaming behaviour is inconsistent with the Curl adapter

   Both adapters implement the same Adapter interface and both accept a $chunkCallback. However their contracts differ in two observable ways:

   1. Body in Response: In the Curl adapter, when $chunkCallback is set, data is only delivered to the callback and $responseBody stays '', so $response->text() returns ''. In the Swoole adapter, $responseBody = $currentResponseBody is always executed (line 679), so $response->text() returns the full body even when a callback was provided. Code that works with one adapter will behave differently on the other.

   2. Intermediate redirect bodies: Because the chunk callback check happens before the redirect check, $chunkCallback is invoked for every redirect response body (which are typically tiny HTML pages), not just the final response. The Curl adapter (via CURLOPT_FOLLOWLOCATION) only triggers the write function for the final response body.

   Both points mean that switching adapters produces different observable behaviour, violating the Liskov Substitution principle for the Adapter interface. The Swoole implementation should mirror the Curl contract: skip appending to $responseBody when $chunkCallback is set, and only call the callback for the final response (after redirect resolution).

_{Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[greptile-apps]

Shared client pool is not coroutine-safe

$this->clients is a single shared pool on the adapter instance. When two concurrent coroutines both target the same host:port:ssl key, they get the same CoClient object. One coroutine's setMethod(), setHeaders(), and configureBody() calls then overwrite the other's configuration before execute() is called, resulting in requests being sent with the wrong method, wrong headers, or the wrong body.

A minimal safe fix is to never reuse a client that is currently executing a request. One approach: remove the client from the pool when it starts executing and re-add it (or a new one) when the response is received. Alternatively, restrict the pool to be per-coroutine (e.g. via Coroutine::getCid() as part of the cache key), or drop the pool entirely and create a new CoClient per request.

[greptile-apps]

Missing space after colon in request header formatting

The formatted header is Key:Value but the standard HTTP/1.1 header field syntax (RFC 7230 §3.2) uses Key: Value (OWS after the colon). While most servers tolerate the compact form, some origin servers and proxies are strict and will reject or misparse headers without the space.

Suggested change

	$formattedHeaders = array_map(function ($key, $value) {
	return $key . ':' . $value;
	}, array_keys($headers), $headers);
	$formattedHeaders = array_map(function ($key, $value) {
	return $key . ': ' . $value;
	}, array_keys($headers), $headers);