Skip to content

Conversation

lkrms
Copy link
Collaborator

@lkrms lkrms commented Apr 3, 2025

No description provided.

lkrms added 8 commits April 2, 2025 13:35
- Consolidate header-related methods used across multiple interfaces via
  new `HasHeaders` and `HasHttpHeaders` interfaces
- Add `getHeaderLines()` alongside existing `getHeaderLine()` methods
- `HttpMessageInterface`
- `HttpMultipartStreamInterface`
- `HttpMultipartStreamPartInterface`
- `HttpRequestInterface`
- `HttpResponseInterface`
- `HttpServerRequestInterface`
- `HttpStreamInterface`
- Rename `FormDataFlag` to `HasFormDataFlag` and access its constants
  via implementations
- Rename `MimeType` to `HasMediaType`, add `TYPE_` prefix to the names
  of its constants and access them via implementations
- Rename `HttpRequestMethod` to `HasRequestMethod`, add `METHOD_` prefix
  to the names of its constants and access them via implementations
- Curler: Rename `Curler::METHOD_HAS_BODY` to `REQUEST_METHOD_HAS_BODY`
  (to prevent conflict with other `METHOD_*` constants)
- Rename `HttpHeader` to `HasHeader`, add `HEADER_` prefix to the names
  of its constants and access them via implementations
- Add `HEADERS_` prefix to the names of `HttpHeaderGroup` constants and
  move them to `HasHeader` before removing `HttpHeaderGroup`
- Remove nullability from `parse()` parameter `$component`
- Rename methods:
  - `toParts()` -> `getComponents()`
  - `isReference()` -> `isRelativeReference()`
- Rename `Uri` constants:
  - `EXPAND_EMPTY_PATH` -> `NORMALISE_EMPTY_PATH`
  - `COLLAPSE_MULTIPLE_SLASHES` -> `NORMALISE_MULTIPLE_SLASHES`
- Remove unused `Uri` methods:
  - `fromParts()`
  - `unparse()`
  - `resolveReference()`
- Remove `Uri` method `removeDotSegments()` from API
- In `Uri::parse()`, replicate `parse_url()` behaviour when URI and
  component are both invalid (i.e. return `false`)
- Move `Uri::isAuthorityForm()` to `HttpUtil` and rename it to
  `requestTargetIsAuthorityForm()`
Copy link

codecov bot commented Apr 3, 2025

Codecov Report

Attention: Patch coverage is 83.97337% with 313 lines in your changes missing coverage. Please review.

Project coverage is 73.79%. Comparing base (1d30318) to head (815cf63).
Report is 37 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Toolkit/Http/Server/Server.php 75.50% 61 Missing ⚠️
src/Toolkit/Curler/Curler.php 66.94% 40 Missing ⚠️
src/Toolkit/Http/HttpUtil.php 80.32% 24 Missing ⚠️
src/Toolkit/Collection/HasItems.php 82.30% 20 Missing ⚠️
src/Toolkit/Cli/CliUtil.php 0.00% 19 Missing ⚠️
src/Toolkit/Http/Message/ServerRequest.php 70.17% 17 Missing ⚠️
src/Toolkit/Http/Message/ServerRequestUpload.php 33.33% 16 Missing ⚠️
src/Toolkit/Sli/Command/AbstractCommand.php 0.00% 16 Missing ⚠️
src/Toolkit/Http/Internal/FormDataEncoder.php 88.97% 14 Missing ⚠️
src/Toolkit/Http/OAuth2/OAuth2Client.php 0.00% 11 Missing ⚠️
... and 19 more
Additional details and impacted files
Components Coverage Δ
Cache 100.00% <ø> (ø)
Cli 76.45% <0.00%> (-0.99%) ⬇️
Collections 84.79% <88.79%> (-1.17%) ⬇️
Console 85.55% <81.81%> (-1.46%) ⬇️
Container 75.78% <ø> (ø)
Contracts ∅ <ø> (∅)
Core 81.87% <ø> (-0.04%) ⬇️
Curler 64.46% <66.90%> (+0.56%) ⬆️
Db 0.00% <ø> (ø)
Http 81.05% <88.28%> (+1.91%) ⬆️
Iterators 82.31% <ø> (ø)
PHPDoc 91.20% <ø> (ø)
PHPStan 99.32% <ø> (ø)
Polyfills 100.00% <ø> (ø)
Sli 40.66% <0.00%> (+0.27%) ⬆️
Sync 66.36% <35.48%> (+0.13%) ⬆️
Testing 77.16% <ø> (ø)
Utils 93.41% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lkrms added 8 commits April 4, 2025 11:26
- Fix issue where `InvalidArgumentTypeException` is thrown with an
  ambiguous `$type` in some `Http` classes

This change allows `Http` interfaces to be given names that would
otherwise conflict with imported PSR-7 interfaces.
- Rename:
  - `HttpMessageInterface` -> `MessageInterface`
  - `HttpRequestInterface` -> `RequestInterface`
  - `HttpServerRequestInterface` -> `ServerRequestInterface`
  - `HttpResponseInterface` -> `ResponseInterface`
  - `HttpStreamInterface` -> `StreamInterface`
  - `HttpMultipartStreamInterface` -> `MultipartStreamInterface`
  - `HttpMultipartStreamPartInterface` -> `StreamPartInterface`
  - `HttpHeadersInterface` -> `HeadersInterface`
  - `HasHttpHeaders` -> `HasInnerHeaders`
    - `getHttpHeaders()` -> `getInnerHeaders()`
  - `AccessTokenInterface` -> `CredentialInterface`
    - `getToken()` -> `getCredential()`
    - `getTokenType()` -> `getAuthenticationScheme()`
- Remove unused `HttpRequestHandlerInterface`
- Curler: Rename `Curler` methods for consistency:
  - `getPublicHttpHeaders()` -> `getPublicHeaders()`
  - `hasAccessToken()` -> `hasCredential()`
  - `withAccessToken()` -> `withCredential()`
- Add `DATA_` prefix to names of constants in `HasFormDataFlag`
- Rename `HasHeader` to `HasHttpHeader` and reinstate separate
  `HasHttpHeaders` interface for `HEADERS_*` constants
- Remove unnecessary `MessageInterface::getHttpPayload()`
- Add templates to simplify `MessageInterface::fromPsr7()` parameter and
  return types
- Update implementations
- Add `MultipartStreamInterface::getParts()` for completeness
- Rename `StreamPartInterface` methods for clarity and consistency:
  - `getFallbackFilename()` -> `getAsciiFilename()`
  - `getContent()` -> `getBody()`
- Review `HttpMultipartStream` (rename pending)
  - Do not remain operational after `close()` or `detach()` are called
- Remove unnecessary templates from interface methods
- Extend `DictionaryInterface` instead of `CollectionInterface`
- Remove `$strict` parameter from `addLine()` (implementation detail)
- Rename methods for consistency:
  - `hasLastLine()` -> `hasEmptyLine()`
  - `append()` -> `addValue()`
  - `canonicalize()` -> `normalise()`
@lkrms lkrms force-pushed the cleanup branch 5 times, most recently from 949e95e to 1fe64e3 Compare April 20, 2025 16:00
lkrms added 5 commits May 7, 2025 15:03
- Rename:
  - `AbstractHttpMessage` -> `AbstractMessage`
  - `AbstractHttpRequest` -> `AbstractRequest`
  - `HasHttpHeaders` -> `HasInnerHeadersTrait`
  - `HttpHeaders` -> `Headers`
  - `HttpFactory` -> `MessageFactory`
  - `HttpMultipartStream` -> `MultipartStream`
  - `HttpMultipartStreamPart` -> `StreamPart`
  - `HttpRequest` -> `Request`
  - `HttpResponse` -> `Response`
  - `HttpServerRequest` -> `ServerRequest`
  - `HttpServerRequestUpload` -> `ServerRequestUpload`
  - `HttpStream` -> `Stream`
  - `HttpServer` -> `Server`
- Move message-related classes to their own namespace
- Add `Server` and `Internal` namespaces
- Don't apply an implicit media type to messages with multipart bodies
- Normalise headers when casting messages to strings
- Fix issue where messages cast to strings may have too many empty lines
  between headers and body
- Move `Stream` methods `copyToString()` and `copyToStream()` to
  `HttpUtil`, renaming them to `getStreamContents()` and `copyStream()`
lkrms added 12 commits May 7, 2025 15:03
- Add optional `TArrayValue` template to `DictionaryInterface` and
  `CollectionInterface` for `Arrayable::toArray()` return type control
- Move `toArray()` from `ReadOnlyCollectionTrait` to
  `RecursiveArrayableCollectionTrait`
- Add non-recursive implementation of `toArray()` to
  `ArrayableCollectionTrait`
- Update implementations as needed
- Move `Headers` methods to `HttpUtil` and refactor:
  - `getContentLength()`
  - `getMultipartBoundary()`
  - `getPreferences()`
  - `mergePreferences()`
  - `getRetryAfter()`
- In `addLine()`:
  - Throw an exception on invalid header field syntax or line folding
    even if `$strict` is `false`
  - Throw `LogicException` if called after headers are applied via
    another method
  - Throw `InvalidHeaderException` instead of `InvalidArgumentException`
- In `map()`, return the instance if values are equivalent
- Fix `addLine()` issue where invalid line folding in the first trailer
  may not be detected
- Fix `getHeaderLine()` issue where values with unquoted commas may be
  inadvertently modified
- Use `getLastHeaderValue()` when retrieving `Content-Type` headers
- Use `getOnlyHeaderValue()` when retrieving `Location` headers
- Rename `HttpUtil` methods:
  - `requestTargetIsAuthorityForm()` -> `isAuthorityForm()`
  - `escapeQuotedString()` -> `quoteString()` (and add double quotes)
  - `getNameValueGenerator()` -> `getNameValuePairs()`
- Fix `HttpUtil::getRetryAfter()` issue where negative delay seconds are
  parsed as a timestamp
- Rename:
  - `StreamDetachedException` -> `StreamClosedException`
  - `StreamInvalidRequestException` -> `InvalidStreamRequestException`
  - `UploadedFileException` -> `UploadFailedException`
- Rename `Regex::UUID` to `UUID_V4`
- Add `Regex::UUID` to match all UUIDs and adopt in
  `AbstractSyncProvider::isValidIdentifier()`
- Http: Adopt terminology and behaviour of latest RFCs
  - Especially:
    - \[RFC7230] and \[RFC7231] -> \[RFC9112] and \[RFC9110]
    - \[RFC5987] -> \[RFC8187]
  - Replace "header line" with "field line" where appropriate
  - Add and implement `HeadersInterface` methods `hasBadWhitespace()`
    and `hasObsoleteLineFolding()`
  - In `Headers::addLine()`, don't throw `InvalidHeaderException` when
    bad whitespace is received and `$strict = true`
  - Rearrange `HasHttpRegex`
  - Fix `AbstractRequest` issue where request target type `origin-form`
    may be incorrectly detected when the given URI has an empty path
- Rename:
  - `getProxyTls()` -> `proxyHasTls()`
  - `getProxyBasePath()` -> `getProxyPath()`
  - `getBaseUri()` -> `getUri()` (and return `Uri`, not `string`)
- Remove superfluous `getScheme()`
- Add:
  - `getLocalIpAddress()`
  - `getLocalPort()` (useful when port is dynamically allocated)
- Allow `stop()` to be called when the server is not running
- In `listen()`:
  - Replace `$callback` with `$listener`, which returns a
    `ServerResponse` instead of receiving control variables by reference
  - Keep listening for requests until a response has a return value
    unless a non-negative `$limit` is given
  - Add request target validity checks
  - Respond to invalid requests with "400 Bad Request" and don't throw
    the underlying exception by default
  - Fix issue where large responses might not be written in full
- Throw `LogicException` when:
  - `getProxy*()` is called on an instance with no proxy
  - an assertion fails (instead of `HttpServerException`)
- Rename `HttpServerException` to `ServerException`
- Add and implement `InvalidHeaderException::getStatusCode()`, then use
  it to return "501 Not Implemented" instead of "400 Bad Request" when
  an invalid HTTP method is received
- Normalise exception variable names as `$ex` (across whole toolkit)
- Improve robustness of HTTP tests
  - Simplify newline handling
  - Allow `http-client.php` to make multiple requests per run
  - In `TestUtil::dumpHttpMessage()`, optionally add one or two newlines
    after messages with one or zero trailing newlines respectively
  - Add `--add-newlines` option to `http-server.php` and
    `http-client.php` and rework tests as needed
@lkrms lkrms merged commit ee9034a into main May 9, 2025
15 checks passed
@lkrms lkrms deleted the cleanup branch May 9, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant