From b79e4b206b49290f969df9b5543af4b22808d2ad Mon Sep 17 00:00:00 2001 From: Volodymyr Panivko Date: Mon, 11 May 2026 21:15:38 +0200 Subject: [PATCH] THRIFT-5980: Add native method types to PHP Transport hierarchy Client: php Migrate the public methods on lib/php/lib/Transport/ from PHPDoc @param/@return annotations to native parameter and return types. This completes the typing trajectory for the Transport subtree started by THRIFT-5976 (properties), THRIFT-5977 (ctor promotion), THRIFT-5978 (strict_types) and THRIFT-5979 (Server + Factory method types). Library: * TTransport abstract: isOpen(): bool, open(): void, close(): void, read(int): string, readAll(int): string, write(string): void, flush(): void. * TSocket / TSSLSocket / TSocketPool: open/close/read/write/flush typed; setSendTimeout/setRecvTimeout/setDebug/setNumRetries/etc typed; getHost /getPort typed; setHandle typed return. * TBufferedTransport / TFramedTransport / TMemoryBuffer / TNullTransport / TPhpStream: all method overrides typed to match the abstract. * TFramedTransport::write keeps the legacy `?int $len = null` optional parameter so existing internal callers in TBinaryProtocol / TCompactProtocol still type-check; the non-write path now delegates with just the buffer. * TCurlClient / THttpClient: read/write/flush/close/open typed; timeout setters typed as ?float (int widens automatically); addHeaders typed array. Full NaN / +Infinity / -Infinity round-trip for the JSON protocols: * TJSONProtocol now writes all three IEEE-754 sentinels as the conventional quoted tokens ("NaN", "Infinity", "-Infinity") matching the Java and C++ TJSONProtocol implementations; the read side gains a "-Infinity" branch so the round-trip is symmetric. Token literals are hoisted to TOKEN_NAN / TOKEN_POS_INFINITY / TOKEN_NEG_INFINITY consts on TJSONProtocol following the existing NAME_* convention, so writer and reader can never drift. * TSimpleJSONProtocol reuses TJSONProtocol's token constants and emits the same three tokens for its write-only path. * The previously corrupted `{"dbl":}` / `{"thing":}` output for non- finite doubles (locked into integration tests with #TODO markers) is fixed; integration tests now assert the correct round-trippable strings, plus a -Infinity case is added on both read and write paths. Side fixes surfaced by typed transports: * TSocket::setSendTimeout/setRecvTimeout switched to intdiv() so the typed int property assignment is exact (floor returns float). * TCurlClient::$timeout / $connectionTimeout and THttpClient::$timeout tightened from float|int|null to ?float (int widens automatically). * TBufferedTransport / TFramedTransport: stale "wBuf_" comments referring to the old C++-style property name dropped. Tests: * TBinaryProtocolTest / TCompactProtocolTest / TBufferedTransportTest / TFramedTransportTest: dropped `->willReturn(N)` from mocks of the now-void write/open/close/flush. Behavior unchanged; PHPUnit refuses to mock returning a value for void methods. * TFramedTransportTest::testWrite mock with('12345', 5) updated to with('12345') after the non-write path now passes only the buffer. * TCurlClientTest timeout data provider switched to float literals (10.0 instead of 10) to match the now-typed ?float properties. * TJSONProtocolTest gains round-trip cases for NaN, +Infinity, and -Infinity through writeAndReadScalarDataProvider. Generated-by: Claude Opus 4.7 --- lib/php/lib/Protocol/TJSONProtocol.php | 25 +++++- lib/php/lib/Protocol/TSimpleJSONProtocol.php | 17 +++- lib/php/lib/Transport/TBufferedTransport.php | 51 +++--------- lib/php/lib/Transport/TCurlClient.php | 83 ++++++------------- lib/php/lib/Transport/TFramedTransport.php | 40 ++++----- lib/php/lib/Transport/THttpClient.php | 50 +++-------- lib/php/lib/Transport/TMemoryBuffer.php | 16 ++-- lib/php/lib/Transport/TNullTransport.php | 10 +-- lib/php/lib/Transport/TPhpStream.php | 14 ++-- lib/php/lib/Transport/TSSLSocket.php | 2 +- lib/php/lib/Transport/TSocket.php | 70 ++++------------ lib/php/lib/Transport/TSocketPool.php | 39 ++------- lib/php/lib/Transport/TTransport.php | 37 ++------- .../Lib/Protocol/TJSONProtocolTest.php | 20 ++++- .../Lib/Protocol/TSimpleJSONProtocolTest.php | 6 +- .../Unit/Lib/Protocol/TBinaryProtocolTest.php | 30 +++---- .../Lib/Protocol/TCompactProtocolTest.php | 3 +- .../Unit/Lib/Protocol/TJSONProtocolTest.php | 18 ++++ .../Lib/Transport/TBufferedTransportTest.php | 15 ++-- .../Unit/Lib/Transport/TCurlClientTest.php | 8 +- .../Lib/Transport/TFramedTransportTest.php | 12 +-- 21 files changed, 212 insertions(+), 354 deletions(-) diff --git a/lib/php/lib/Protocol/TJSONProtocol.php b/lib/php/lib/Protocol/TJSONProtocol.php index 1ac6b4474d..4e33b547d3 100644 --- a/lib/php/lib/Protocol/TJSONProtocol.php +++ b/lib/php/lib/Protocol/TJSONProtocol.php @@ -78,6 +78,11 @@ class TJSONProtocol extends TProtocol public const NAME_SET = "set"; public const NAME_UUID = "uid"; + /** Quoted tokens used to round-trip non-finite doubles through the JSON protocol. */ + public const TOKEN_NAN = "NaN"; + public const TOKEN_POS_INFINITY = "Infinity"; + public const TOKEN_NEG_INFINITY = "-Infinity"; + private function getTypeNameForTypeID($typeID) { switch ($typeID) { @@ -232,15 +237,25 @@ private function writeJSONInteger(int $num) } } - private function writeJSONDouble($num) + private function writeJSONDouble(float $num): void { $this->context->write(); + if (is_nan($num)) { + $this->trans->write(self::QUOTE . self::TOKEN_NAN . self::QUOTE); + return; + } + + if (is_infinite($num)) { + $token = $num > 0 ? self::TOKEN_POS_INFINITY : self::TOKEN_NEG_INFINITY; + $this->trans->write(self::QUOTE . $token . self::QUOTE); + return; + } + if ($this->context->escapeNum()) { $this->trans->write(self::QUOTE); } - #TODO add compatibility with NAN and INF $this->trans->write(json_encode($num)); if ($this->context->escapeNum()) { @@ -398,10 +413,12 @@ private function readJSONDouble() if (substr($this->reader->peek(), 0, 1) == self::QUOTE) { $arr = $this->readJSONString(true); - if ($arr == "NaN") { + if ($arr === self::TOKEN_NAN) { return NAN; - } elseif ($arr == "Infinity") { + } elseif ($arr === self::TOKEN_POS_INFINITY) { return INF; + } elseif ($arr === self::TOKEN_NEG_INFINITY) { + return -INF; } elseif (!$this->context->escapeNum()) { throw new TProtocolException( "Numeric data unexpectedly quoted " . $arr, diff --git a/lib/php/lib/Protocol/TSimpleJSONProtocol.php b/lib/php/lib/Protocol/TSimpleJSONProtocol.php index a89a7a4701..d230ce52e8 100644 --- a/lib/php/lib/Protocol/TSimpleJSONProtocol.php +++ b/lib/php/lib/Protocol/TSimpleJSONProtocol.php @@ -108,18 +108,27 @@ private function writeJSONInteger(int $num) } } - private function writeJSONDouble($num) + private function writeJSONDouble(float $num): void { $isMapKey = $this->writeContext->isMapKey(); - $this->writeContext->write(); + if (is_nan($num)) { + $this->trans->write(self::QUOTE . TJSONProtocol::TOKEN_NAN . self::QUOTE); + return; + } + + if (is_infinite($num)) { + $token = $num > 0 ? TJSONProtocol::TOKEN_POS_INFINITY : TJSONProtocol::TOKEN_NEG_INFINITY; + $this->trans->write(self::QUOTE . $token . self::QUOTE); + return; + } + if ($isMapKey) { $this->trans->write(self::QUOTE); } - #TODO add compatibility with NAN and INF - $this->trans->write(json_encode((float)$num)); + $this->trans->write(json_encode($num)); if ($isMapKey) { $this->trans->write(self::QUOTE); diff --git a/lib/php/lib/Transport/TBufferedTransport.php b/lib/php/lib/Transport/TBufferedTransport.php index 654b500359..dd365bd89b 100644 --- a/lib/php/lib/Transport/TBufferedTransport.php +++ b/lib/php/lib/Transport/TBufferedTransport.php @@ -56,27 +56,22 @@ public function __construct( ) { } - public function isOpen() + public function isOpen(): bool { return $this->transport->isOpen(); } - /** - * @inheritdoc - * - * @throws TTransportException - */ - public function open() + public function open(): void { $this->transport->open(); } - public function close() + public function close(): void { $this->transport->close(); } - public function putBack($data) + public function putBack(string $data): void { if (strlen($this->rBuf) === 0) { $this->rBuf = $data; @@ -93,10 +88,8 @@ public function putBack($data) * * Therefore, use the readAll method of the wrapped transport inside * the buffered readAll. - * - * @throws TTransportException */ - public function readAll($len) + public function readAll(int $len): string { $have = strlen($this->rBuf); if ($have == 0) { @@ -116,14 +109,7 @@ public function readAll($len) return $data; } - /** - * @inheritdoc - * - * @param int $len - * @return string - * @throws TTransportException - */ - public function read($len) + public function read(int $len): string { if (strlen($this->rBuf) === 0) { $this->rBuf = $this->transport->read($this->rBufSize); @@ -142,39 +128,26 @@ public function read($len) return $ret; } - /** - * @inheritdoc - * - * @param string $buf - * @throws TTransportException - */ - public function write($buf) + public function write(string $buf): void { $this->wBuf .= $buf; if (strlen($this->wBuf) >= $this->wBufSize) { $out = $this->wBuf; - // Note that we clear the internal wBuf_ prior to the underlying write - // to ensure we're in a sane state (i.e. internal buffer cleaned) - // if the underlying write throws up an exception + // Clear the buffer before writing so we stay in a sane state + // even if the underlying transport throws. $this->wBuf = ''; $this->transport->write($out); } } - /** - * @inheritdoc - * - * @throws TTransportException - */ - public function flush() + public function flush(): void { if (strlen($this->wBuf) > 0) { $out = $this->wBuf; - // Note that we clear the internal wBuf_ prior to the underlying write - // to ensure we're in a sane state (i.e. internal buffer cleaned) - // if the underlying write throws up an exception + // Clear the buffer before writing so we stay in a sane state + // even if the underlying transport throws. $this->wBuf = ''; $this->transport->write($out); } diff --git a/lib/php/lib/Transport/TCurlClient.php b/lib/php/lib/Transport/TCurlClient.php index cfc6af99ac..428472e8b4 100644 --- a/lib/php/lib/Transport/TCurlClient.php +++ b/lib/php/lib/Transport/TCurlClient.php @@ -53,18 +53,14 @@ class TCurlClient extends TTransport protected string|false|null $response = null; /** - * Read timeout - * - * @var float|int|null + * Read timeout in seconds. */ - protected $timeout = null; + protected ?float $timeout = null; /** - * Connection timeout - * - * @var float|int|null + * Connection timeout in seconds. */ - protected $connectionTimeout = null; + protected ?float $connectionTimeout = null; /** * http headers @@ -85,81 +81,54 @@ public function __construct( $this->uri = ($uri === '' || str_starts_with($uri, '/')) ? $uri : '/' . $uri; } - /** - * Set read timeout - * - * @param float $timeout - */ - public function setTimeoutSecs($timeout) + public function setTimeoutSecs(?float $timeout): void { $this->timeout = $timeout; } - /** - * Set connection timeout - * - * @param float $connectionTimeout - */ - public function setConnectionTimeoutSecs($connectionTimeout) + public function setConnectionTimeoutSecs(?float $connectionTimeout): void { $this->connectionTimeout = $connectionTimeout; } - /** - * Whether this transport is open. - * - * @return boolean true if open - */ - public function isOpen() + public function isOpen(): bool { return true; } - /** - * Open the transport for reading/writing - * - * @throws TTransportException if cannot open - */ - public function open() + public function open(): void { } - /** - * Close the transport. - */ - public function close() + public function close(): void { $this->request = ''; $this->response = null; } /** - * Read some data into the array. - * - * @param int $len How much to read - * @return string The data that has been read * @throws TTransportException if cannot read any more data */ - public function read($len) + public function read(int $len): string { - if ($len >= strlen($this->response)) { - return $this->response; - } else { - $ret = substr($this->response, 0, $len); - $this->response = substr($this->response, $len); - - return $ret; + $response = (string) $this->response; + if ($len >= strlen($response)) { + return $response; } + + $ret = substr($response, 0, $len); + $this->response = substr($response, $len); + + return $ret; } /** * Guarantees that the full amount of data is read. Since TCurlClient gets entire payload at * once, parent readAll cannot be used. * - * @return string The data, of exact length * @throws TTransportException if cannot read data */ - public function readAll($len) + public function readAll(int $len): string { $data = $this->read($len); @@ -171,12 +140,9 @@ public function readAll($len) } /** - * Writes some data into the pending buffer - * - * @param string $buf The data to write * @throws TTransportException if writing fails */ - public function write($buf) + public function write(string $buf): void { $this->request .= $buf; } @@ -186,7 +152,7 @@ public function write($buf) * * @throws TTransportException if a writing error occurs */ - public function flush() + public function flush(): void { if (!self::$curlHandle) { register_shutdown_function(['Thrift\\Transport\\TCurlClient', 'closeCurlHandle']); @@ -254,7 +220,7 @@ public function flush() } } - public static function closeCurlHandle() + public static function closeCurlHandle(): void { try { if (self::$curlHandle) { @@ -269,7 +235,10 @@ public static function closeCurlHandle() } } - public function addHeaders($headers) + /** + * @param array $headers + */ + public function addHeaders(array $headers): void { $this->headers = array_merge($this->headers, $headers); } diff --git a/lib/php/lib/Transport/TFramedTransport.php b/lib/php/lib/Transport/TFramedTransport.php index 7b9f7f2a1c..e94975f9ed 100644 --- a/lib/php/lib/Transport/TFramedTransport.php +++ b/lib/php/lib/Transport/TFramedTransport.php @@ -50,17 +50,17 @@ public function __construct( ) { } - public function isOpen() + public function isOpen(): bool { return $this->transport->isOpen(); } - public function open() + public function open(): void { $this->transport->open(); } - public function close() + public function close(): void { $this->transport->close(); } @@ -68,10 +68,8 @@ public function close() /** * Reads from the buffer. When more data is required reads another entire * chunk and serves future reads out of that. - * - * @param int $len How much data */ - public function read($len) + public function read(int $len): string { if (!$this->read) { return $this->transport->read($len); @@ -95,12 +93,7 @@ public function read($len) return $out; } - /** - * Put previously read data back into the buffer - * - * @param string $data data to return - */ - public function putBack($data) + public function putBack(string $data): void { if (strlen($this->rBuf) === 0) { $this->rBuf = $data; @@ -112,7 +105,7 @@ public function putBack($data) /** * Reads a chunk of data into the internal read buffer. */ - private function readFrame() + private function readFrame(): void { $buf = $this->transport->readAll(4); $val = unpack('N', $buf); @@ -122,15 +115,14 @@ private function readFrame() } /** - * Writes some data to the pending output buffer. - * - * @param string $buf The data - * @param int $len Limit of bytes to write + * Writes some data to the pending output buffer. When $len is provided, + * truncates $buf to at most $len bytes. */ - public function write($buf, $len = null) + public function write(string $buf, ?int $len = null): void { if (!$this->write) { - return $this->transport->write($buf, $len); + $this->transport->write($buf); + return; } if ($len !== null && $len < strlen($buf)) { @@ -143,18 +135,18 @@ public function write($buf, $len = null) * Writes the output buffer to the stream in the format of a 4-byte length * followed by the actual data. */ - public function flush() + public function flush(): void { if (!$this->write || strlen($this->wBuf) == 0) { - return $this->transport->flush(); + $this->transport->flush(); + return; } $out = pack('N', strlen($this->wBuf)); $out .= $this->wBuf; - // Note that we clear the internal wBuf_ prior to the underlying write - // to ensure we're in a sane state (i.e. internal buffer cleaned) - // if the underlying write throws up an exception + // Clear the buffer before writing so we stay in a sane state + // even if the underlying transport throws. $this->wBuf = ''; $this->transport->write($out); $this->transport->flush(); diff --git a/lib/php/lib/Transport/THttpClient.php b/lib/php/lib/Transport/THttpClient.php index 5184efa01a..04d6f21319 100644 --- a/lib/php/lib/Transport/THttpClient.php +++ b/lib/php/lib/Transport/THttpClient.php @@ -52,11 +52,9 @@ class THttpClient extends TTransport protected $handle = null; /** - * Read timeout - * - * @var float|int|null + * Read timeout in seconds. */ - protected $timeout = null; + protected ?float $timeout = null; /** * http headers @@ -80,39 +78,21 @@ public function __construct( $this->uri = ($uri === '' || str_starts_with($uri, '/')) ? $uri : '/' . $uri; } - /** - * Set read timeout - * - * @param float $timeout - */ - public function setTimeoutSecs($timeout) + public function setTimeoutSecs(?float $timeout): void { $this->timeout = $timeout; } - /** - * Whether this transport is open. - * - * @return boolean true if open - */ - public function isOpen() + public function isOpen(): bool { return true; } - /** - * Open the transport for reading/writing - * - * @throws TTransportException if cannot open - */ - public function open() + public function open(): void { } - /** - * Close the transport. - */ - public function close() + public function close(): void { if ($this->handle) { @fclose($this->handle); @@ -121,13 +101,9 @@ public function close() } /** - * Read some data into the array. - * - * @param int $len How much to read - * @return string The data that has been read * @throws TTransportException if cannot read any more data */ - public function read($len) + public function read(int $len): string { $data = @fread($this->handle, $len); if ($data === false || $data === '') { @@ -151,12 +127,9 @@ public function read($len) } /** - * Writes some data into the pending buffer - * - * @param string $buf The data to write * @throws TTransportException if writing fails */ - public function write($buf) + public function write(string $buf): void { $this->buf .= $buf; } @@ -166,7 +139,7 @@ public function write($buf) * * @throws TTransportException if a writing error occurs */ - public function flush() + public function flush(): void { // God, PHP really has some esoteric ways of doing simple things. $host = $this->host . ($this->port != 80 ? ':' . $this->port : ''); @@ -216,7 +189,10 @@ public function flush() } } - public function addHeaders($headers) + /** + * @param array $headers + */ + public function addHeaders(array $headers): void { $this->headers = array_merge($this->headers, $headers); } diff --git a/lib/php/lib/Transport/TMemoryBuffer.php b/lib/php/lib/Transport/TMemoryBuffer.php index 3bf348707e..203b5b3873 100644 --- a/lib/php/lib/Transport/TMemoryBuffer.php +++ b/lib/php/lib/Transport/TMemoryBuffer.php @@ -44,25 +44,25 @@ public function __construct(protected string $buf = '') { } - public function isOpen() + public function isOpen(): bool { return true; } - public function open() + public function open(): void { } - public function close() + public function close(): void { } - public function write($buf) + public function write(string $buf): void { $this->buf .= $buf; } - public function read($len) + public function read(int $len): string { $bufLength = strlen($this->buf); @@ -87,17 +87,17 @@ public function read($len) return $ret; } - public function getBuffer() + public function getBuffer(): string { return $this->buf; } - public function available() + public function available(): int { return strlen($this->buf); } - public function putBack($data) + public function putBack(string $data): void { $this->buf = $data . $this->buf; } diff --git a/lib/php/lib/Transport/TNullTransport.php b/lib/php/lib/Transport/TNullTransport.php index 835cb9fe44..13cbef5698 100644 --- a/lib/php/lib/Transport/TNullTransport.php +++ b/lib/php/lib/Transport/TNullTransport.php @@ -35,25 +35,25 @@ */ class TNullTransport extends TTransport { - public function isOpen() + public function isOpen(): bool { return true; } - public function open() + public function open(): void { } - public function close() + public function close(): void { } - public function read($len) + public function read(int $len): string { throw new TTransportException("Can't read from TNullTransport."); } - public function write($buf) + public function write(string $buf): void { } } diff --git a/lib/php/lib/Transport/TPhpStream.php b/lib/php/lib/Transport/TPhpStream.php index 836790ee04..cc2e170382 100644 --- a/lib/php/lib/Transport/TPhpStream.php +++ b/lib/php/lib/Transport/TPhpStream.php @@ -54,7 +54,7 @@ public function __construct(int $mode) $this->write = (bool) ($mode & self::MODE_W); } - public function open() + public function open(): void { if ($this->read) { $this->inStream = @fopen($this->inStreamName(), 'r'); @@ -70,7 +70,7 @@ public function open() } } - public function close() + public function close(): void { if ($this->read) { @fclose($this->inStream); @@ -82,14 +82,14 @@ public function close() } } - public function isOpen() + public function isOpen(): bool { return (!$this->read || is_resource($this->inStream)) && (!$this->write || is_resource($this->outStream)); } - public function read($len) + public function read(int $len): string { $data = @fread($this->inStream, $len); if ($data === false || $data === '') { @@ -99,7 +99,7 @@ public function read($len) return $data; } - public function write($buf) + public function write(string $buf): void { while (strlen($buf) > 0) { $got = @fwrite($this->outStream, $buf); @@ -112,12 +112,12 @@ public function write($buf) } } - public function flush() + public function flush(): void { @fflush($this->outStream); } - private function inStreamName() + private function inStreamName(): string { if (php_sapi_name() == 'cli') { return 'php://stdin'; diff --git a/lib/php/lib/Transport/TSSLSocket.php b/lib/php/lib/Transport/TSSLSocket.php index cddb3c3cda..74ea6f68c9 100644 --- a/lib/php/lib/Transport/TSSLSocket.php +++ b/lib/php/lib/Transport/TSSLSocket.php @@ -60,7 +60,7 @@ public function __construct( /** * Connects the socket. */ - public function open() + public function open(): void { if ($this->isOpen()) { throw new TTransportException('Socket already connected', TTransportException::ALREADY_OPEN); diff --git a/lib/php/lib/Transport/TSocket.php b/lib/php/lib/Transport/TSocket.php index fdfaeadae4..5b3cbf9c86 100644 --- a/lib/php/lib/Transport/TSocket.php +++ b/lib/php/lib/Transport/TSocket.php @@ -99,74 +99,49 @@ public function __construct( /** * @param resource $handle - * @return void */ - public function setHandle($handle) + public function setHandle($handle): void { $this->handle = $handle; stream_set_blocking($this->handle, false); } /** - * Sets the send timeout. - * * @param int $timeout Timeout in milliseconds. */ - public function setSendTimeout($timeout) + public function setSendTimeout(int $timeout): void { - $this->sendTimeoutSec = floor($timeout / 1000); + $this->sendTimeoutSec = intdiv($timeout, 1000); $this->sendTimeoutUsec = ($timeout - ($this->sendTimeoutSec * 1000)) * 1000; } /** - * Sets the receive timeout. - * * @param int $timeout Timeout in milliseconds. */ - public function setRecvTimeout($timeout) + public function setRecvTimeout(int $timeout): void { - $this->recvTimeoutSec = floor($timeout / 1000); + $this->recvTimeoutSec = intdiv($timeout, 1000); $this->recvTimeoutUsec = ($timeout - ($this->recvTimeoutSec * 1000)) * 1000; } - /** - * Sets debugging output on or off - * - * @param bool $debug - */ - public function setDebug($debug) + public function setDebug(bool $debug): void { $this->debug = $debug; } - /** - * Get the host that this socket is connected to - * - * @return string host - */ - public function getHost() + public function getHost(): string { return $this->host; } - /** - * Get the remote port that this socket is connected to - * - * @return int port - */ - public function getPort() + public function getPort(): int { return $this->port; } - /** - * Tests whether this is open - * - * @return bool true if the socket is open - */ - public function isOpen() + public function isOpen(): bool { return is_resource($this->handle); } @@ -174,7 +149,7 @@ public function isOpen() /** * Connects the socket. */ - public function open() + public function open(): void { if ($this->isOpen()) { throw new TTransportException('Socket already connected', TTransportException::ALREADY_OPEN); @@ -225,10 +200,7 @@ public function open() } } - /** - * Closes the socket. - */ - public function close() + public function close(): void { @fclose($this->handle); $this->handle = null; @@ -239,11 +211,8 @@ public function close() * * This method will not wait for all the requested data, it will return as * soon as any data is received. - * - * @param int $len Maximum number of bytes to read. - * @return string Binary data */ - public function read($len) + public function read(int $len): string { $null = null; $read = [$this->handle]; @@ -276,10 +245,8 @@ public function read($len) /** * Write to the socket. - * - * @param string $buf The data to write */ - public function write($buf) + public function write(string $buf): void { $null = null; $write = [$this->handle]; @@ -321,16 +288,11 @@ public function write($buf) } /** - * Flush output to the socket. - * * Since read(), readAll() and write() operate on the sockets directly, - * this is a no-op - * - * If you wish to have flushable buffering behaviour, wrap this TSocket - * in a TBufferedTransport. + * this is a no-op. Wrap this TSocket in a TBufferedTransport if you + * need flushable buffering. */ - public function flush() + public function flush(): void { - // no-op } } diff --git a/lib/php/lib/Transport/TSocketPool.php b/lib/php/lib/Transport/TSocketPool.php index 81f3dcb276..534f3401eb 100644 --- a/lib/php/lib/Transport/TSocketPool.php +++ b/lib/php/lib/Transport/TSocketPool.php @@ -112,57 +112,32 @@ public function __construct( * @param string $host hostname or IP * @param int $port port */ - public function addServer(string $host, int $port) + public function addServer(string $host, int $port): void { $this->servers[] = ['host' => $host, 'port' => $port]; } - /** - * Sets how many time to keep retrying a host in the connect function. - * - * @param int $numRetries - */ - public function setNumRetries($numRetries) + public function setNumRetries(int $numRetries): void { $this->numRetries = $numRetries; } - /** - * Sets how long to wait until retrying a host if it was marked down - * - * @param int $numRetries - */ - public function setRetryInterval($retryInterval) + public function setRetryInterval(int $retryInterval): void { $this->retryInterval = $retryInterval; } - /** - * Sets how many time to keep retrying a host before marking it as down. - * - * @param int $numRetries - */ - public function setMaxConsecutiveFailures($maxConsecutiveFailures) + public function setMaxConsecutiveFailures(int $maxConsecutiveFailures): void { $this->maxConsecutiveFailures = $maxConsecutiveFailures; } - /** - * Turns randomization in connect order on or off. - * - * @param bool $randomize - */ - public function setRandomize($randomize) + public function setRandomize(bool $randomize): void { $this->randomize = $randomize; } - /** - * Whether to always try the last server. - * - * @param bool $alwaysTryLast - */ - public function setAlwaysTryLast($alwaysTryLast) + public function setAlwaysTryLast(bool $alwaysTryLast): void { $this->alwaysTryLast = $alwaysTryLast; } @@ -171,7 +146,7 @@ public function setAlwaysTryLast($alwaysTryLast) * Connects the socket by iterating through all the servers in the pool * and trying to find one that works. */ - public function open() + public function open(): void { // Check if we want order randomization if ($this->randomize) { diff --git a/lib/php/lib/Transport/TTransport.php b/lib/php/lib/Transport/TTransport.php index da1386ce88..fdeeb7f37d 100644 --- a/lib/php/lib/Transport/TTransport.php +++ b/lib/php/lib/Transport/TTransport.php @@ -34,46 +34,28 @@ */ abstract class TTransport { - /** - * Whether this transport is open. - * - * @return boolean true if open - */ - abstract public function isOpen(); + abstract public function isOpen(): bool; /** - * Open the transport for reading/writing - * * @throws TTransportException if cannot open */ - abstract public function open(); + abstract public function open(): void; - /** - * Close the transport. - */ - abstract public function close(); + abstract public function close(): void; /** - * Read some data into the array. - * - * @param int $len How much to read - * @return string The data that has been read * @throws TTransportException if cannot read any more data */ - abstract public function read($len); + abstract public function read(int $len): string; /** * Guarantees that the full amount of data is read. * - * @return string The data, of exact length * @throws TTransportException if cannot read data */ - public function readAll($len) + public function readAll(int $len): string { - // return $this->read($len); - $data = ''; - $got = 0; while (($got = strlen($data)) < $len) { $data .= $this->read($len - $got); } @@ -82,19 +64,14 @@ public function readAll($len) } /** - * Writes the given data out. - * - * @param string $buf The data to write * @throws TTransportException if writing fails */ - abstract public function write($buf); + abstract public function write(string $buf): void; /** - * Flushes any pending data out of a buffer - * * @throws TTransportException if a writing error occurs */ - public function flush() + public function flush(): void { } } diff --git a/lib/php/test/Integration/Lib/Protocol/TJSONProtocolTest.php b/lib/php/test/Integration/Lib/Protocol/TJSONProtocolTest.php index b3633e09ab..22989089d3 100644 --- a/lib/php/test/Integration/Lib/Protocol/TJSONProtocolTest.php +++ b/lib/php/test/Integration/Lib/Protocol/TJSONProtocolTest.php @@ -147,21 +147,26 @@ public static function writeDataProvider() ], 'expected' => '{"1":{"dbl":3.1415926535898}}', ]; - #TODO Should be fixed in future yield 'double Nan' => [ 'argsClassName' => \Basic\ThriftTest\ThriftTest_testDouble_args::class, 'argsValues' => [ 'thing' => NAN, ], - 'expected' => '{"1":{"dbl":}}', + 'expected' => '{"1":{"dbl":"NaN"}}', ]; - #TODO Should be fixed in future yield 'double Infinity' => [ 'argsClassName' => \Basic\ThriftTest\ThriftTest_testDouble_args::class, 'argsValues' => [ 'thing' => INF, ], - 'expected' => '{"1":{"dbl":}}', + 'expected' => '{"1":{"dbl":"Infinity"}}', + ]; + yield 'double -Infinity' => [ + 'argsClassName' => \Basic\ThriftTest\ThriftTest_testDouble_args::class, + 'argsValues' => [ + 'thing' => -INF, + ], + 'expected' => '{"1":{"dbl":"-Infinity"}}', ]; yield 'byte' => [ 'argsClassName' => \Basic\ThriftTest\ThriftTest_testByte_args::class, @@ -418,6 +423,13 @@ public static function readDataProvider() 'expectedValue' => INF, 'expectedResult' => 1, ]; + yield 'double -Infinity' => [ + 'buffer' => '{"1":{"dbl":"-Infinity"}}', + 'argsClassName' => \Basic\ThriftTest\ThriftTest_testDouble_args::class, + 'argsPropertyName' => 'thing', + 'expectedValue' => -INF, + 'expectedResult' => 1, + ]; yield 'byte' => [ 'buffer' => '{"1":{"i8":1}}', 'argsClassName' => \Basic\ThriftTest\ThriftTest_testByte_args::class, diff --git a/lib/php/test/Integration/Lib/Protocol/TSimpleJSONProtocolTest.php b/lib/php/test/Integration/Lib/Protocol/TSimpleJSONProtocolTest.php index ff6894bdfb..f654fcb59e 100644 --- a/lib/php/test/Integration/Lib/Protocol/TSimpleJSONProtocolTest.php +++ b/lib/php/test/Integration/Lib/Protocol/TSimpleJSONProtocolTest.php @@ -148,21 +148,19 @@ public static function writeDataProvider() ], 'expected' => '{"thing":3.1415926535898}', ]; - #TODO Should be fixed in future yield 'double Nan' => [ 'argsClassName' => \Basic\ThriftTest\ThriftTest_testDouble_args::class, 'argsValues' => [ 'thing' => NAN, ], - 'expected' => '{"thing":}', + 'expected' => '{"thing":"NaN"}', ]; - #TODO Should be fixed in future yield 'double Infinity' => [ 'argsClassName' => \Basic\ThriftTest\ThriftTest_testDouble_args::class, 'argsValues' => [ 'thing' => INF, ], - 'expected' => '{"thing":}', + 'expected' => '{"thing":"Infinity"}', ]; yield 'byte' => [ 'argsClassName' => \Basic\ThriftTest\ThriftTest_testByte_args::class, diff --git a/lib/php/test/Unit/Lib/Protocol/TBinaryProtocolTest.php b/lib/php/test/Unit/Lib/Protocol/TBinaryProtocolTest.php index 51100400dd..65d4f2f2d4 100644 --- a/lib/php/test/Unit/Lib/Protocol/TBinaryProtocolTest.php +++ b/lib/php/test/Unit/Lib/Protocol/TBinaryProtocolTest.php @@ -190,8 +190,7 @@ public function testWriteFieldStop() $transport ->expects($this->once()) ->method('write') - ->with(pack('c', TType::STOP), 1) #writeByte - ->willReturn(1); + ->with(pack('c', TType::STOP), 1); #writeByte $this->assertEquals(1, $protocol->writeFieldStop()); } @@ -330,8 +329,7 @@ public function testWriteBool() $transport ->expects($this->once()) ->method('write') - ->with(pack('c', (int)$value), 1) #writeByte - ->willReturn(1); + ->with(pack('c', (int)$value), 1); #writeByte $this->assertEquals(1, $protocol->writeBool($value)); } @@ -345,8 +343,7 @@ public function testWriteByte() $transport ->expects($this->once()) ->method('write') - ->with(pack('c', $value), 1) #writeByte - ->willReturn(1); + ->with(pack('c', $value), 1); #writeByte $this->assertEquals(1, $protocol->writeByte($value)); } @@ -360,8 +357,7 @@ public function testWriteI16() $transport ->expects($this->once()) ->method('write') - ->with(pack('n', $value), 2) #writeI16 - ->willReturn(2); + ->with(pack('n', $value), 2); #writeI16 $this->assertEquals(2, $protocol->writeI16($value)); } @@ -375,8 +371,7 @@ public function testWriteI32() $transport ->expects($this->once()) ->method('write') - ->with(pack('N', $value), 4) #writeI32 - ->willReturn(4); + ->with(pack('N', $value), 4); #writeI32 $this->assertEquals(4, $protocol->writeI32($value)); } @@ -413,8 +408,7 @@ public function testWriteI64For32BitArchitecture() $transport ->expects($this->once()) ->method('write') - ->with(pack('N2', $hi, $lo), 8) #writeI64 - ->willReturn(4); + ->with(pack('N2', $hi, $lo), 8); #writeI64 $this->assertEquals(8, $protocol->writeI64($value)); } @@ -434,8 +428,7 @@ public function testWriteI64For64BitArchitecture() $transport ->expects($this->once()) ->method('write') - ->with(pack('N2', $hi, $lo), 8) #writeI64 - ->willReturn(8); + ->with(pack('N2', $hi, $lo), 8); #writeI64 $this->assertEquals(8, $protocol->writeI64($value)); } @@ -449,8 +442,7 @@ public function testWriteDouble() $transport ->expects($this->once()) ->method('write') - ->with(strrev(pack('d', $value)), 8) #writeDouble - ->willReturn(8); + ->with(strrev(pack('d', $value)), 8); #writeDouble $this->assertEquals(8, $protocol->writeDouble($value)); } @@ -495,8 +487,7 @@ public function testWriteUuid() $transport ->expects($this->once()) ->method('write') - ->with(hex2bin('0123456789abcdef0123456789abcdef'), 16) - ->willReturn(16); + ->with(hex2bin('0123456789abcdef0123456789abcdef'), 16); $this->assertEquals(16, $protocol->writeUuid($uuid)); } @@ -985,8 +976,7 @@ public function testReadI64For32BitArchitecture( $transport ->expects($this->once()) ->method('write') - ->with(pack('N2', $hi, $lo), 8) #writeI64 - ->willReturn(4); + ->with(pack('N2', $hi, $lo), 8); #writeI64 $this->assertEquals(8, $protocol->readI64($value)); $this->assertEquals($expectedValue, $value); diff --git a/lib/php/test/Unit/Lib/Protocol/TCompactProtocolTest.php b/lib/php/test/Unit/Lib/Protocol/TCompactProtocolTest.php index 35a8bf975f..ed5b120128 100644 --- a/lib/php/test/Unit/Lib/Protocol/TCompactProtocolTest.php +++ b/lib/php/test/Unit/Lib/Protocol/TCompactProtocolTest.php @@ -860,8 +860,7 @@ public function testWriteUuid() $transport ->expects($this->once()) ->method('write') - ->with(hex2bin('0123456789abcdef0123456789abcdef'), 16) - ->willReturn(16); + ->with(hex2bin('0123456789abcdef0123456789abcdef'), 16); $this->assertSame(16, $protocol->writeUuid($uuid)); } diff --git a/lib/php/test/Unit/Lib/Protocol/TJSONProtocolTest.php b/lib/php/test/Unit/Lib/Protocol/TJSONProtocolTest.php index 7c1948d99f..e176c4ab27 100644 --- a/lib/php/test/Unit/Lib/Protocol/TJSONProtocolTest.php +++ b/lib/php/test/Unit/Lib/Protocol/TJSONProtocolTest.php @@ -284,6 +284,24 @@ public static function writeAndReadScalarDataProvider() 'value' => 1.0e-10, 'readMethod' => 'readDouble', ]; + yield 'double NaN round-trips via "NaN" token' => [ + 'fieldType' => TType::DOUBLE, + 'writeMethod' => 'writeDouble', + 'value' => NAN, + 'readMethod' => 'readDouble', + ]; + yield 'double +Infinity round-trips via "Infinity" token' => [ + 'fieldType' => TType::DOUBLE, + 'writeMethod' => 'writeDouble', + 'value' => INF, + 'readMethod' => 'readDouble', + ]; + yield 'double -Infinity round-trips via "-Infinity" token' => [ + 'fieldType' => TType::DOUBLE, + 'writeMethod' => 'writeDouble', + 'value' => -INF, + 'readMethod' => 'readDouble', + ]; yield 'string empty' => [ 'fieldType' => TType::STRING, 'writeMethod' => 'writeString', diff --git a/lib/php/test/Unit/Lib/Transport/TBufferedTransportTest.php b/lib/php/test/Unit/Lib/Transport/TBufferedTransportTest.php index 78c94594ff..414affec2e 100644 --- a/lib/php/test/Unit/Lib/Transport/TBufferedTransportTest.php +++ b/lib/php/test/Unit/Lib/Transport/TBufferedTransportTest.php @@ -53,8 +53,7 @@ public function testOpen() $transport ->expects($this->once()) - ->method('open') - ->willReturn(null); + ->method('open'); $this->assertNull($bufferedTransport->open()); } @@ -66,8 +65,7 @@ public function testClose() $transport ->expects($this->once()) - ->method('close') - ->willReturn(null); + ->method('close'); $this->assertNull($bufferedTransport->close()); } @@ -209,8 +207,7 @@ public function testWrite( $transport ->expects($this->exactly($bufferedTransportCall)) ->method('write') - ->with($writeData) - ->willReturn(null); + ->with($writeData); $this->assertNull($bufferedTransport->write($writeData)); @@ -244,13 +241,11 @@ public function testFlush( $transport ->expects(!empty($writeBuffer) ? $this->once() : $this->never()) ->method('write') - ->with($writeBuffer) - ->willReturn(null); + ->with($writeBuffer); $transport ->expects($this->once()) - ->method('flush') - ->willReturn(null); + ->method('flush'); $this->assertNull($bufferedTransport->flush()); diff --git a/lib/php/test/Unit/Lib/Transport/TCurlClientTest.php b/lib/php/test/Unit/Lib/Transport/TCurlClientTest.php index e0b4709cfa..e9ab823d56 100644 --- a/lib/php/test/Unit/Lib/Transport/TCurlClientTest.php +++ b/lib/php/test/Unit/Lib/Transport/TCurlClientTest.php @@ -324,8 +324,8 @@ public static function flushDataProvider() yield 'timeout' => array_merge( $default, [ - 'timeout' => 10, - 'connectionTimeout' => 10, + 'timeout' => 10.0, + 'connectionTimeout' => 10.0, 'curlSetOptCalls' => [ [Assert::anything(), CURLOPT_RETURNTRANSFER, true], [Assert::anything(), CURLOPT_USERAGENT, 'PHP/TCurlClient'], @@ -341,8 +341,8 @@ public static function flushDataProvider() 'Content-Length: ' . strlen($request), ], ], - [Assert::anything(), CURLOPT_TIMEOUT, 10], - [Assert::anything(), CURLOPT_CONNECTTIMEOUT, 10], + [Assert::anything(), CURLOPT_TIMEOUT, 10.0], + [Assert::anything(), CURLOPT_CONNECTTIMEOUT, 10.0], [Assert::anything(), CURLOPT_POSTFIELDS, $request], [Assert::anything(), CURLOPT_URL, 'http://localhost'], ], diff --git a/lib/php/test/Unit/Lib/Transport/TFramedTransportTest.php b/lib/php/test/Unit/Lib/Transport/TFramedTransportTest.php index 91042c4e67..8d127c8431 100644 --- a/lib/php/test/Unit/Lib/Transport/TFramedTransportTest.php +++ b/lib/php/test/Unit/Lib/Transport/TFramedTransportTest.php @@ -54,8 +54,7 @@ public function testOpen() $transport ->expects($this->once()) - ->method('open') - ->willReturn(null); + ->method('open'); $this->assertNull($framedTransport->open()); } @@ -67,8 +66,7 @@ public function testClose() $transport ->expects($this->once()) - ->method('close') - ->willReturn(null); + ->method('close'); $this->assertNull($framedTransport->close()); } @@ -169,8 +167,7 @@ public function testWrite( $transport ->expects($writeAllowed ? $this->never() : $this->once()) ->method('write') - ->with('12345', 5) - ->willReturn(5); + ->with('12345'); $framedTransport->write($writeData, $writeLength); @@ -216,8 +213,7 @@ public function testFlush( $transport ->expects($writeAllowed && !empty($writeBuffer) ? $this->once() : $this->never()) ->method('write') - ->with($lowLevelTransportWrite) - ->willReturn(null); + ->with($lowLevelTransportWrite); $this->assertNull($framedTransport->flush()); }