diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b401ca6..1b5f8ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,42 +11,39 @@ permissions: contents: read jobs: - resolve-php-version: - name: Resolve PHP version + resolve-tooling-image: + name: Resolve tooling image runs-on: ubuntu-latest timeout-minutes: 5 outputs: - php-version: ${{ steps.config.outputs.php-version }} + php-image: ${{ steps.config.outputs.php-image }} steps: - name: Checkout uses: actions/checkout@v7 - - name: Resolve PHP version from composer.json + - name: Resolve tooling image from the Makefile id: config - run: | - version=$(jq -r '.require.php' composer.json | grep -oP '\d+\.\d+' | head -1) - echo "php-version=$version" >> "$GITHUB_OUTPUT" + run: echo "php-image=$(make show-image)" >> "$GITHUB_OUTPUT" build: name: Build - needs: resolve-php-version + needs: resolve-tooling-image runs-on: ubuntu-latest timeout-minutes: 15 + env: + image: ${{ needs.resolve-tooling-image.outputs.php-image }} + workspace: /var/www/html steps: - name: Checkout uses: actions/checkout@v7 - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - tools: composer:2 - php-version: ${{ needs.resolve-php-version.outputs.php-version }} - - name: Validate composer.json - run: composer validate --no-interaction + run: docker run --rm -v "${PWD}":${{ env.workspace }} ${{ env.image }} composer validate --no-interaction - name: Install dependencies - run: composer install --no-progress --optimize-autoloader --prefer-dist --no-interaction + run: > + docker run --rm -v "${PWD}":${{ env.workspace }} ${{ env.image }} + composer install --no-progress --optimize-autoloader --prefer-dist --no-interaction - name: Upload vendor and composer.lock as artifact uses: actions/upload-artifact@v7 @@ -58,19 +55,13 @@ jobs: auto-review: name: Auto review - needs: [resolve-php-version, build] + needs: [resolve-tooling-image, build] runs-on: ubuntu-latest timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@v7 - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - tools: composer:2 - php-version: ${{ needs.resolve-php-version.outputs.php-version }} - - name: Download vendor artifact from build uses: actions/download-artifact@v8 with: @@ -78,23 +69,17 @@ jobs: path: . - name: Run review - run: composer review + run: make review tests: name: Tests - needs: [resolve-php-version, auto-review] + needs: [resolve-tooling-image, auto-review] runs-on: ubuntu-latest timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@v7 - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - tools: composer:2 - php-version: ${{ needs.resolve-php-version.outputs.php-version }} - - name: Download vendor artifact from build uses: actions/download-artifact@v8 with: @@ -102,4 +87,4 @@ jobs: path: . - name: Run tests - run: composer tests + run: make tests diff --git a/Makefile b/Makefile index 90ab50d..6915324 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,12 @@ endif TTY := $(shell [ -t 0 ] && echo -it) -DOCKER_RUN = docker run ${PLATFORM} --rm ${TTY} --net=host -v ${PWD}:/app -w /app gustavofreze/php:8.5-alpine +PHP_VERSION := $(shell sed -n 's/.*"php": *"^\([0-9]*\.[0-9]*\)".*/\1/p' composer.json) +IMAGE_VERSION := 1.0.0 +PHP_IMAGE := gustavofreze/php:${PHP_VERSION}-cli-${IMAGE_VERSION} +WORKSPACE := /var/www/html + +DOCKER_RUN = docker run ${PLATFORM} --rm ${TTY} --net=host -v ${PWD}:${WORKSPACE} ${PHP_IMAGE} RESET := \033[0m GREEN := \033[0;32m @@ -44,6 +49,10 @@ show-reports: ## Open coverage and mutation reports in the browser show-outdated: ## Show outdated direct dependencies @${DOCKER_RUN} composer outdated --direct +.PHONY: show-image +show-image: ## Show the pinned PHP tooling image + @echo ${PHP_IMAGE} + .PHONY: clean clean: ## Remove dependencies and generated artifacts @sudo chown -R ${USER}:${USER} ${PWD} @@ -66,7 +75,7 @@ help: ## Display this help message | awk 'BEGIN {FS = ":.*?## "}; {printf "$(YELLOW)%-25s$(RESET) %s\n", $$1, $$2}' @echo "" @echo "$$(printf '$(GREEN)')Reports$$(printf '$(RESET)')" - @grep -E '^(show-reports|show-outdated):.*?## .*$$' $(MAKEFILE_LIST) \ + @grep -E '^(show-reports|show-outdated|show-image):.*?## .*$$' $(MAKEFILE_LIST) \ | awk 'BEGIN {FS = ":.*?## "}; {printf "$(YELLOW)%-25s$(RESET) %s\n", $$1, $$2}' @echo "" @echo "$$(printf '$(GREEN)')Cleanup$$(printf '$(RESET)')" diff --git a/README.md b/README.md index c3e31cb..7894ddb 100644 --- a/README.md +++ b/README.md @@ -153,7 +153,9 @@ use TinyBlocks\Http\Server\Response; Response::from(body: ['status' => 'accepted'], code: Code::ACCEPTED); ``` -Attach additional headers via varargs of `Headerable`: +Attach additional headers via varargs of `Headerable`. They add to the `application/json` default rather than replacing +it, so a response carrying a `Link` or a `Cache-Control` header still declares its media type. Passing a `ContentType` +is what changes the media type, and it replaces the default instead of appending a second one: ```php write(), code: $code, - headers: ResponseHeaders::fromOrDefault(...$headers), + headers: ResponseHeaders::fromWithDefaultContentType(...$headers), protocolVersion: ProtocolVersion::default(), customReasonPhrase: null ); @@ -38,7 +38,7 @@ public static function createWithoutBody(Code $code, Headerable ...$headers): Re return new InternalResponse( body: StreamFactory::fromEmptyBody()->write(), code: $code, - headers: ResponseHeaders::fromOrDefault(...$headers), + headers: ResponseHeaders::fromWithDefaultContentType(...$headers), protocolVersion: ProtocolVersion::default(), customReasonPhrase: null ); diff --git a/src/Internal/Server/Response/ResponseHeaders.php b/src/Internal/Server/Response/ResponseHeaders.php index 7bbbca3..c63d1d2 100644 --- a/src/Internal/Server/Response/ResponseHeaders.php +++ b/src/Internal/Server/Response/ResponseHeaders.php @@ -10,6 +10,8 @@ final readonly class ResponseHeaders { + private const string CONTENT_TYPE = 'Content-Type'; + private function __construct(private array $headers) { } @@ -24,19 +26,23 @@ private static function mergeInto(Headerable $header, array $merged): array return $merged; } - public static function fromOrDefault(Headerable ...$headers): ResponseHeaders + public static function fromWithDefaultContentType(Headerable ...$headers): ResponseHeaders { - if (empty($headers)) { - return new ResponseHeaders(headers: ContentType::applicationJson(charset: Charset::UTF_8)->toArray()); - } - $merged = []; foreach ($headers as $header) { $merged = ResponseHeaders::mergeInto(header: $header, merged: $merged); } - return new ResponseHeaders(headers: $merged); + $provided = new ResponseHeaders(headers: $merged); + + if ($provided->hasHeader(name: ResponseHeaders::CONTENT_TYPE)) { + return $provided; + } + + $contentType = ContentType::applicationJson(charset: Charset::UTF_8); + + return new ResponseHeaders(headers: ResponseHeaders::mergeInto(header: $contentType, merged: $merged)); } private function findKey(string $name): ?string diff --git a/tests/Unit/Server/HeadersTest.php b/tests/Unit/Server/HeadersTest.php index 007c55a..90b52fb 100644 --- a/tests/Unit/Server/HeadersTest.php +++ b/tests/Unit/Server/HeadersTest.php @@ -289,7 +289,11 @@ public function testNoContentWhenCacheControlWithEveryDirectiveGivenThenHeaderRe self::assertSame($expected, $actual->getHeaderLine('Cache-Control')); self::assertSame([$expected], $actual->getHeader('Cache-Control')); - self::assertSame($cacheControl->toArray(), $actual->getHeaders()); + + /** @And the default Content-Type sits beside it, because a caller header adds rather than replaces */ + $expectedHeaders = [...$cacheControl->toArray(), 'Content-Type' => ['application/json; charset=utf-8']]; + + self::assertSame($expectedHeaders, $actual->getHeaders()); } public function testWithHeaderWhenChainedWithDistinctKeysThenBothPresentAlongsideDefault(): void diff --git a/tests/Unit/Server/ResponseTest.php b/tests/Unit/Server/ResponseTest.php index 572bf53..ae7abc9 100644 --- a/tests/Unit/Server/ResponseTest.php +++ b/tests/Unit/Server/ResponseTest.php @@ -17,8 +17,12 @@ use Test\TinyBlocks\Http\Models\Product; use Test\TinyBlocks\Http\Models\Products; use Test\TinyBlocks\Http\Models\Status; +use TinyBlocks\Http\Charset; use TinyBlocks\Http\Code; +use TinyBlocks\Http\ContentType; use TinyBlocks\Http\Exceptions\BodyTypeIsUnsupported; +use TinyBlocks\Http\Link; +use TinyBlocks\Http\LinkRelation; use TinyBlocks\Http\Server\Response; final class ResponseTest extends TestCase @@ -823,4 +827,44 @@ public static function responseFromProvider(): array ] ]; } + + public function testOkWhenUnrelatedHeaderGivenThenKeepsDefaultContentType(): void + { + /** @Given a header that says nothing about the media type */ + $link = Link::to(uri: '/dragons?page=2', relation: LinkRelation::NEXT); + + /** @When the response is created with that header alongside a body */ + $actual = Response::ok(['name' => 'Hydra'], $link); + + /** @Then the header is carried */ + self::assertSame(['; rel="next"'], $actual->getHeader('Link')); + + /** @And the default Content-Type survives, because the body is still JSON */ + self::assertSame(['application/json; charset=utf-8'], $actual->getHeader('Content-Type')); + } + + public function testOkWhenContentTypeGivenThenReplacesTheDefault(): void + { + /** @Given a media type the caller chose */ + $contentType = ContentType::textPlain(charset: Charset::UTF_8); + + /** @When the response is created with it */ + $actual = Response::ok('Hydra', $contentType); + + /** @Then the caller wins and no second media type is appended */ + self::assertSame(['text/plain; charset=utf-8'], $actual->getHeader('Content-Type')); + } + + public function testNoContentWhenUnrelatedHeaderGivenThenKeepsDefaultContentType(): void + { + /** @Given a header that says nothing about the media type */ + $link = Link::to(uri: '/dragons?page=2', relation: LinkRelation::NEXT); + + /** @When a bodiless response is created with that header */ + $actual = Response::noContent($link); + + /** @Then the header is carried and the default Content-Type still applies */ + self::assertSame(['; rel="next"'], $actual->getHeader('Link')); + self::assertSame(['application/json; charset=utf-8'], $actual->getHeader('Content-Type')); + } }