Skip to content

THRIFT-6010: Add PSR-18 HTTP transport (TPsrHttpClient) for PHP#3492

Open
sveneld wants to merge 1 commit into
apache:masterfrom
sveneld:THRIFT-6010
Open

THRIFT-6010: Add PSR-18 HTTP transport (TPsrHttpClient) for PHP#3492
sveneld wants to merge 1 commit into
apache:masterfrom
sveneld:THRIFT-6010

Conversation

@sveneld
Copy link
Copy Markdown
Contributor

@sveneld sveneld commented May 17, 2026

Summary

Adds Thrift\Transport\TPsrHttpClient — an HTTP transport for Thrift clients backed by any PSR-18 Psr\Http\Client\ClientInterface. Concrete HTTP clients ship under Composer suggest; users pick Guzzle, Symfony HttpClient, or php-http/curl-client and discovery wires them up.

Purely additive: TCurlClient and THttpClient are untouched.

Design

  • Constructor: (string $url, ?ClientInterface $client = null, ?RequestFactoryInterface $requestFactory = null, ?StreamFactoryInterface $streamFactory = null) — URL is a single string; PSR-7 / PSR-17 handle parsing.
  • php-http/discovery auto-discovers client + factories when not explicitly injected; failures surface as TTransportException::NOT_OPEN with an actionable message ("Install one of: guzzle, symfony, curl-client").
  • PSR-18 NetworkExceptionInterfaceNOT_OPEN. Other ClientExceptionInterface subtypes plus \InvalidArgumentException / \RuntimeException from PSR-7 builders → NOT_OPEN / UNKNOWN as appropriate. Non-200 status → UNKNOWN.
  • Connection pooling, TLS, proxies, timeouts → all live on the injected ClientInterface. The transport itself stores no static state.

Composer

  • require: psr/http-client, psr/http-factory, psr/http-message, php-http/discovery (all MIT — ASF Category A).
  • require-dev: nyholm/psr7 for the unit-test suite.
  • suggest: guzzlehttp/guzzle, symfony/http-client, php-http/curl-client.
  • config.allow-plugins: php-http/discovery: true (required by Composer 2.2+).

Tests

New TPsrHttpClientTest — 15 cases using an injected fake ClientInterface and real nyholm/psr7 factories. No phpmock. Covers default + custom headers, URL pass-through (6 variants), empty body, response buffer, status-code rejection, network / client / invalid-URL exception mapping.

CI green on PHP 8.1 – 8.5 plus the existing PHP cross-test job.

Test plan

  • composer phpstan (level 6) clean
  • vendor/bin/phpunit -c lib/php/phpunit.xml --testsuite Unit clean
  • make style clean
  • cross-test (cpp, go, rs, cpp, py, php, nodejs, nodets) pass
  • Smoke test against a real Thrift HTTP server with a Guzzle-backed TPsrHttpClient

Usage

use GuzzleHttp\Client;
use Thrift\Transport\TPsrHttpClient;

$client = new Client(['timeout' => 5.0]);
$transport = new TPsrHttpClient(
    url: 'https://rpc.example.com/thrift',
    client: $client,
);

  • Did you create an Apache Jira ticket? — THRIFT-6010
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"? — yes
  • Did you squash your changes to a single commit? — single commit (728249b23)
  • Did you do your best to avoid breaking changes? — purely additive; no existing files modified beyond composer.json
  • If your change does not involve any code, include [skip ci] — n/a (code change)

Generated-by: Claude Opus 4.7 noreply@anthropic.com

@mergeable mergeable Bot added the php label May 17, 2026
@sveneld sveneld marked this pull request as ready for review May 17, 2026 18:39
Client: php

TPsrHttpClient is a new HTTP transport backed by any PSR-18 ClientInterface,
with PSR-7/PSR-17 factories auto-discovered via php-http/discovery when not
explicitly injected. Concrete clients (Guzzle, Symfony HttpClient, php-http/
curl-client) are listed under composer "suggest".

Existing TCurlClient and THttpClient transports are left untouched.

Generated-by: Claude Opus 4.7 <noreply@anthropic.com>
@sveneld sveneld changed the title THRIFT-6010: Add PSR-18 HTTP transport (TPsrHttpClient) and deprecate TCurlClient THRIFT-6010: Add PSR-18 HTTP transport (TPsrHttpClient) for PHP May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant