diff --git a/config/zammad.php b/config/zammad.php index f70cd72..48f3c87 100644 --- a/config/zammad.php +++ b/config/zammad.php @@ -3,8 +3,10 @@ return [ /** Zammad API Base URL - * Full URL to your Zammad instance including the API prefix, e.g.: - * https://zammad.example.com/api/v1 + * Full URL to your Zammad instance, e.g.: + * https://zammad.example.com + * + * The API prefix (`/api/v1`) is appended automatically. */ 'url' => env('ZAMMAD_URL', 'http://127.0.0.1:8098'), diff --git a/src/Bridge/LaravelServiceProvider.php b/src/Bridge/LaravelServiceProvider.php index 4267a93..90f3c0e 100644 --- a/src/Bridge/LaravelServiceProvider.php +++ b/src/Bridge/LaravelServiceProvider.php @@ -40,7 +40,7 @@ * - Set your credentials in `.env`: * * ```env - * ZAMMAD_URL=https://zammad.example.com/api/v1 + * ZAMMAD_URL=https://zammad.example.com * ZAMMAD_TOKEN=your-api-token * ``` * @@ -69,7 +69,7 @@ * * 1. `config/zammad.php` values (after `vendor:publish`) * 2. `ZAMMAD_URL` / `ZAMMAD_TOKEN` environment variables (`.env`) - * 3. Built-in defaults (`http://127.0.0.1:8098/api/v1`, empty token) + * 3. Built-in defaults (`http://127.0.0.1:8098`, empty token) * * @see ZammadClient::withToken() */ diff --git a/src/Bridge/SymfonyBundle.php b/src/Bridge/SymfonyBundle.php index 2fb1e66..8f4a138 100644 --- a/src/Bridge/SymfonyBundle.php +++ b/src/Bridge/SymfonyBundle.php @@ -42,7 +42,7 @@ * - Set the environment variables (`.env` or `.env.local`): * * ```env - * ZAMMAD_URL=https://zammad.example.com/api/v1 + * ZAMMAD_URL=https://zammad.example.com * ZAMMAD_TOKEN=your-api-token * ``` * @@ -73,6 +73,11 @@ final class SymfonyBundle extends Bundle public function getContainerExtension(): ?ExtensionInterface { return new class implements ExtensionInterface { + /** + * Registers the configured Zammad client with the container. + * + * @param array> $configs + */ public function load(array $configs, ContainerBuilder $container): void { $resolved = []; @@ -80,7 +85,7 @@ public function load(array $configs, ContainerBuilder $container): void $resolved = array_merge($resolved, $config); } - $url = $resolved['url'] ?? (string) ($_ENV['ZAMMAD_URL'] ?? 'http://127.0.0.1:8098/api/v1'); + $url = $resolved['url'] ?? (string) ($_ENV['ZAMMAD_URL'] ?? 'http://127.0.0.1:8098'); $token = $resolved['token'] ?? (string) ($_ENV['ZAMMAD_TOKEN'] ?? ''); $client = new ZammadClient( diff --git a/src/Core/Transport/RequestHandler.php b/src/Core/Transport/RequestHandler.php index e84791b..65a4b28 100644 --- a/src/Core/Transport/RequestHandler.php +++ b/src/Core/Transport/RequestHandler.php @@ -14,6 +14,7 @@ use Psr\Log\NullLogger; use ZammadAPIClient\Core\Contracts\RequestHandlerInterface; use ZammadAPIClient\Exceptions\AuthenticationException; +use ZammadAPIClient\Exceptions\BadRequestException; use ZammadAPIClient\Exceptions\ForbiddenException; use ZammadAPIClient\Exceptions\NetworkException; use ZammadAPIClient\Exceptions\NotFoundException; @@ -38,6 +39,8 @@ */ final class RequestHandler implements RequestHandlerInterface { + public const API_VERSION = 'v1'; + private ClientInterface $httpClient; private RequestFactoryInterface $requestFactory; private StreamFactoryInterface $streamFactory; @@ -48,7 +51,7 @@ final class RequestHandler implements RequestHandlerInterface /** * @param ClientInterface $httpClient PSR-18 client (any implementation). * @param RequestFactoryInterface $factory PSR-17 factory; must also implement {@see StreamFactoryInterface}. - * @param string $baseUrl Base URL incl. API prefix. + * @param string $baseUrl Base URL; the API prefix (`/api/v1`) is appended if missing. * @param LoggerInterface $logger PSR-3 logger; defaults to NullLogger. * @param int $maxRetries Max retries on HTTP 429 (0 = disable). */ @@ -69,10 +72,21 @@ public function __construct( : $httpClient; $this->requestFactory = $factory; $this->streamFactory = $factory; - $this->baseUrl = $baseUrl; + $this->baseUrl = self::normalizeBaseUrl($baseUrl); $this->logger = $logger; } + /** + * Ensures the base URL ends with the Zammad API prefix (`/api/v1`). + * + * A trailing `/api` or `/api/vN` is stripped first so the prefix is never + * duplicated and the version is always the one this client targets. + */ + private static function normalizeBaseUrl(string $url): string + { + return preg_replace('#/api(?:/v\d+)?$#', '', rtrim($url, '/')) . '/api/' . self::API_VERSION; + } + /** * Returns the raw PSR-7 response from the most recent request, or null if * no request has been made yet. @@ -135,7 +149,8 @@ public function getRaw(string $uri, array $query = [], array $headers = []): str $uri .= '?' . http_build_query($query); } - $options = !empty($headers) ? ['headers' => $headers] : []; + $headers += ['Accept' => '*/*']; + $options = ['headers' => $headers]; return (string) $this->dispatch('GET', $uri, $options)->getBody(); } @@ -248,35 +263,65 @@ private function dispatch(string $method, string $uri, array $options): Response throw $this->mapError($status, $uri, $response); } + /** + * Maps an unsuccessful HTTP response to its corresponding domain exception. + */ private function mapError(int $status, string $uri, ResponseInterface $response): ZammadException { + $raw = (string) $response->getBody(); + return match (true) { - $status === 401 => new AuthenticationException('Invalid credentials'), - $status === 403 => new ForbiddenException("Access denied: {$uri}"), - $status === 404 => new NotFoundException("Resource not found: {$uri}"), - $status === 422 => $this->validationError($response), + $status === 400 => new BadRequestException($this->extractErrorMessage($raw) ?? 'Bad request'), + $status === 401 => new AuthenticationException($this->extractErrorMessage($raw) ?? 'Invalid credentials'), + $status === 403 => new ForbiddenException($this->extractErrorMessage($raw) ?? "Access denied: {$uri}"), + $status === 404 => new NotFoundException($this->extractErrorMessage($raw) ?? "Resource not found: {$uri}"), + $status === 422 => $this->validationError($raw), $status === 429 => new RateLimitException( 'Too many requests', (int) ($response->getHeaderLine('Retry-After') ?: 60), ), - $status >= 500 => new ServerErrorException("Server error: {$status}"), - default => new NetworkException("Unexpected status: {$status}"), + $status >= 500 => new ServerErrorException($this->extractErrorMessage($raw) ?? "Server error: {$status}"), + default => new NetworkException($this->extractErrorMessage($raw) ?? "Unexpected status: {$status}"), }; } - private function validationError(ResponseInterface $response): ValidationException + /** + * Builds a validation exception from a raw HTTP 422 response body. + */ + private function validationError(string $raw): ValidationException { - $raw = (string) $response->getBody(); + return new ValidationException( + $this->extractErrorMessage($raw) ?? $this->extractValidationMessage($raw), + $this->extractValidationErrors($this->decodeLenient($raw)), + ); + } + + /** + * Extracts a human-readable message from an error response body. + * + * Prefers the JSON `error`/`error_human` fields; falls back to the raw + * body for non-HTML text responses. Returns null when no usable message + * can be extracted (callers then supply a generic fallback). + */ + private function extractErrorMessage(string $raw): ?string + { + if ($raw === '') { + return null; + } + $body = $this->decodeLenient($raw); - $message = is_string($body['error'] ?? null) - ? $body['error'] - : $this->extractValidationMessage($raw); + $message = $body['error'] ?? $body['error_human'] ?? null; + if (is_string($message) && $message !== '') { + return $message; + } - return new ValidationException( - $message, - $this->extractValidationErrors($body), - ); + $trimmed = trim($raw); + if ($trimmed === '' || str_starts_with($trimmed, '<')) { + return null; + } + + return substr($trimmed, 0, 200); } /** @@ -285,7 +330,7 @@ private function validationError(ResponseInterface $response): ValidationExcepti */ private function extractValidationErrors(array $body): array { - $details = $body['details'] ?? $body['error_details'] ?? null; + $details = $body['details'] ?? $body['error_details'] ?? $body['errors'] ?? null; return is_array($details) ? $details : []; } diff --git a/src/Exceptions/BadRequestException.php b/src/Exceptions/BadRequestException.php new file mode 100644 index 0000000..3911034 --- /dev/null +++ b/src/Exceptions/BadRequestException.php @@ -0,0 +1,28 @@ +config ?? new ConnectionConfig(); - $url = self::normalizeUrl($this->url); - $httpClient = new GuzzleClient([ 'headers' => [ 'User-Agent' => self::USER_AGENT, @@ -68,20 +69,9 @@ public function createHandler(): RequestHandlerInterface return new RequestHandler( $httpClient, new HttpFactory(), - $url, + $this->url, logger: $config->logger ?? new NullLogger(), maxRetries: $config->maxRetries, ); } - - private static function normalizeUrl(string $url): string - { - $url = rtrim($url, '/'); - - if (!str_contains($url, '/api/')) { - $url .= '/api/v1'; - } - - return $url; - } } diff --git a/test/Unit/Core/RequestHandlerTest.php b/test/Unit/Core/RequestHandlerTest.php index 94c1318..98ad0a1 100644 --- a/test/Unit/Core/RequestHandlerTest.php +++ b/test/Unit/Core/RequestHandlerTest.php @@ -16,6 +16,7 @@ use Psr\Http\Message\ResponseInterface; use ZammadAPIClient\Core\Transport\RequestHandler; use ZammadAPIClient\Exceptions\AuthenticationException; +use ZammadAPIClient\Exceptions\BadRequestException; use ZammadAPIClient\Exceptions\ForbiddenException; use ZammadAPIClient\Exceptions\NetworkException; use ZammadAPIClient\Exceptions\NotFoundException; @@ -128,6 +129,9 @@ public function testUnauthorizedMapsToTypedException(): void $this->handler->get('tickets'); } + /** + * Verifies that HTTP 403 responses map to the forbidden exception. + */ public function testForbiddenMapsToTypedException(): void { $this->httpClient->response = new Response(403, [], ''); @@ -136,6 +140,73 @@ public function testForbiddenMapsToTypedException(): void $this->handler->get('tickets'); } + /** + * Verifies that HTTP 400 responses preserve the API error message. + */ + public function testBadRequestMapsToBadRequestException(): void + { + $this->httpClient->response = new Response(400, [], (string) json_encode(['error' => 'invalid filter'])); + + try { + $this->handler->get('tickets'); + self::fail('Expected BadRequestException'); + } catch (BadRequestException $e) { + self::assertSame('invalid filter', $e->getMessage()); + } + } + + /** + * Verifies that server exceptions preserve the API error message. + */ + public function testServerErrorIncludesBodyMessage(): void + { + $this->httpClient->response = new Response(500, [], (string) json_encode(['error' => 'boom'])); + + try { + $this->handler->get('tickets'); + self::fail('Expected ServerErrorException'); + } catch (ServerErrorException $e) { + self::assertSame('boom', $e->getMessage()); + } + } + + /** + * Verifies that validation errors use the human-readable error field. + */ + public function testValidationExceptionReadsErrorHuman(): void + { + $this->httpClient->response = new Response(422, [], (string) json_encode(['error_human' => 'human readable'])); + + try { + $this->handler->post('tickets', ['x' => 1]); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + self::assertSame('human readable', $e->getMessage()); + } + } + + /** + * Verifies that validation details are extracted from the errors field. + */ + public function testValidationExceptionExtractsErrorsKey(): void + { + $this->httpClient->response = new Response( + 422, + [], + (string) json_encode(['error' => 'bad', 'errors' => ['title' => 'required']]), + ); + + try { + $this->handler->post('tickets', ['x' => 1]); + self::fail('Expected ValidationException'); + } catch (ValidationException $e) { + self::assertSame(['title' => 'required'], $e->errors); + } + } + + /** + * Verifies that raw requests return the response body without decoding it. + */ public function testGetRawReturnsUndecodedBody(): void { $binary = "PNG\x00\x01binary-not-json"; @@ -144,6 +215,46 @@ public function testGetRawReturnsUndecodedBody(): void self::assertSame($binary, $this->handler->getRaw('ticket_attachment/1/2/3')); } + /** + * Verifies that raw requests accept responses of any content type. + */ + public function testGetRawSendsWildcardAccept(): void + { + $this->httpClient->response = new Response(200, [], 'binary'); + + $this->handler->getRaw('ticket_attachment/1/2/3'); + + self::assertNotNull($this->httpClient->lastRequest); + self::assertSame('*/*', $this->httpClient->lastRequest->getHeaderLine('Accept')); + } + + /** + * Verifies that supported base URL forms resolve to the v1 API path. + */ + public function testNormalizesBaseUrlToApiV1(): void + { + $this->httpClient->response = new Response(200, [], '{}'); + + $cases = [ + 'https://zammad.example' => 'https://zammad.example/api/v1/tickets', + 'https://zammad.example/' => 'https://zammad.example/api/v1/tickets', + 'https://zammad.example/api' => 'https://zammad.example/api/v1/tickets', + 'https://zammad.example/api/' => 'https://zammad.example/api/v1/tickets', + 'https://zammad.example/api/v1' => 'https://zammad.example/api/v1/tickets', + 'https://zammad.example/api/v2' => 'https://zammad.example/api/v1/tickets', + ]; + + foreach ($cases as $baseUrl => $expected) { + $handler = new RequestHandler($this->httpClient, $this->httpFactory, $baseUrl, maxRetries: 0); + $handler->get('tickets'); + + self::assertSame($expected, (string) $this->httpClient->lastRequest->getUri(), "URL: {$baseUrl}"); + } + } + + /** + * Verifies that a successful response with invalid JSON is rejected. + */ public function testNonJsonBodyOn200ThrowsNetworkException(): void { $this->httpClient->response = new Response(200, [], 'proxy error'); @@ -212,12 +323,19 @@ public function testConstructorRejectsFactoryWithoutStreamFactory(): void ); } + /** + * Verifies that PSR client failures are wrapped as network exceptions. + */ public function testDispatchCatchesClientException(): void { $httpClient = new class implements ClientInterface { + /** + * Simulates a PSR client transport failure. + */ public function sendRequest(RequestInterface $request): ResponseInterface { - throw new class extends \RuntimeException implements ClientExceptionInterface {}; + throw new class extends \RuntimeException implements ClientExceptionInterface { + }; } };