diff --git a/api/src/Application.php b/api/src/Application.php index 153056a..749788d 100644 --- a/api/src/Application.php +++ b/api/src/Application.php @@ -24,6 +24,7 @@ use Monitor\Api\Repository\KpiRepository; use Monitor\Api\Repository\LegacyStatusFileReader; use Monitor\Api\Repository\SystemRepository; use Monitor\Api\Repository\TicketRepository; +use Throwable; /** * Verdrahtet Router, Controller und Auth fuer alle /api/v1/*-Endpunkte aus @@ -108,6 +109,12 @@ final class Application return Response::error($e->getMessage(), 404); } catch (InvalidArgumentException $e) { return Response::error($e->getMessage(), 422); + } catch (Throwable $e) { + // Unerwartete Fehler (z.B. PDOException) duerfen nie als PHP-Fatal-Error mit + // Stacktrace und Dateipfaden im Response-Body landen - Details nur ins Log. + error_log(sprintf('[monitor-api] %s: %s in %s:%d', $e::class, $e->getMessage(), $e->getFile(), $e->getLine())); + + return Response::error('Interner Serverfehler.', 500); } } } diff --git a/api/src/Config.php b/api/src/Config.php index 72e6267..e150f3a 100644 --- a/api/src/Config.php +++ b/api/src/Config.php @@ -7,6 +7,11 @@ namespace Monitor\Api; /** * Liest die Konfiguration ausschliesslich aus Umgebungsvariablen (12-factor-Stil), * damit die Docker-Compose-Umgebung ohne zusaetzliche Config-Dateien auskommt. + * + * Die Umgebung wird ausschliesslich in fromEnvironment() eingesammelt; get() greift nie + * selbst auf getenv() zu. Damit ist eine mit explizitem Array gebaute Config in Tests + * vollstaendig gekapselt und uebernimmt keine Werte (z.B. CODEBERG_TOKEN) aus der + * Umgebung des laufenden Prozesses. */ final class Config { @@ -21,7 +26,7 @@ final class Config private function get(string $key, ?string $default = null): ?string { - $value = $this->env[$key] ?? getenv($key); + $value = $this->env[$key] ?? null; if ($value === false || $value === null || $value === '') { return $default; } diff --git a/api/src/Repository/AgentRepository.php b/api/src/Repository/AgentRepository.php index f060482..ed4658a 100644 --- a/api/src/Repository/AgentRepository.php +++ b/api/src/Repository/AgentRepository.php @@ -100,9 +100,19 @@ final class AgentRepository return array_map(fn (array $row) => $this->hidingKey($row), $stmt->fetchAll()); } - /** @return array */ + /** + * Ein unbekanntes Ticket wuerde sonst erst in der FOREIGN-KEY-Constraint auflaufen und + * als ungefangene PDOException beim Client landen - deshalb hier pruefen und als + * regulaeren Validierungsfehler (422) melden. + * + * @return array + */ public function updateActivity(int $id, string $state, ?int $currentTicketId): array { + if ($currentTicketId !== null && !$this->ticketExists($currentTicketId)) { + throw new InvalidArgumentException('Unbekanntes Ticket: ' . $currentTicketId); + } + $stmt = $this->db->pdo()->prepare( 'UPDATE agents SET state = :state, current_ticket_id = :ticket_id, last_seen_at = :now WHERE id = :id' ); @@ -121,6 +131,14 @@ final class AgentRepository return $agent; } + private function ticketExists(int $ticketId): bool + { + $stmt = $this->db->pdo()->prepare('SELECT 1 FROM tickets WHERE id = :id'); + $stmt->execute(['id' => $ticketId]); + + return $stmt->fetchColumn() !== false; + } + /** @param array $row @return array */ private function hidingKey(array $row): array { diff --git a/api/src/Sync/CodebergSyncService.php b/api/src/Sync/CodebergSyncService.php index 822e9c7..f4b9b3f 100644 --- a/api/src/Sync/CodebergSyncService.php +++ b/api/src/Sync/CodebergSyncService.php @@ -47,7 +47,19 @@ final class CodebergSyncService 'description' => $issue['body'], 'column' => $column, 'type' => $firstLabel['name'] ?? null, - 'color' => $firstLabel['color'] ?? null, + 'color' => self::normalizeColor($firstLabel['color'] ?? null), ]; } + + /** + * Codeberg liefert Label-Farben mal mit, mal ohne fuehrendes "#". Die Normalisierung + * gehoert hierher und nicht in den Client, damit jede CodebergClient-Implementierung + * dieselbe CSS-taugliche Farbe im Ticket ablegt. + */ + private static function normalizeColor(?string $color): ?string + { + $color = trim((string) $color); + + return $color === '' ? null : '#' . ltrim($color, '#'); + } } diff --git a/api/src/Sync/CurlCodebergClient.php b/api/src/Sync/CurlCodebergClient.php index b264840..808bf38 100644 --- a/api/src/Sync/CurlCodebergClient.php +++ b/api/src/Sync/CurlCodebergClient.php @@ -45,7 +45,7 @@ final class CurlCodebergClient implements CodebergClient 'labels' => array_map( static fn (array $label): array => [ 'name' => (string) $label['name'], - 'color' => '#' . ltrim((string) $label['color'], '#'), + 'color' => (string) $label['color'], ], $issue['labels'] ?? [] ), diff --git a/api/tests/Unit/ApplicationTest.php b/api/tests/Unit/ApplicationTest.php index 02924b4..fac4dc1 100644 --- a/api/tests/Unit/ApplicationTest.php +++ b/api/tests/Unit/ApplicationTest.php @@ -73,6 +73,32 @@ final class ApplicationTest extends TestCase self::assertCount(1, $listResponse->body()['tickets']); } + /** + * Regression: Ein unbekanntes Ticket lief bis in die FOREIGN-KEY-Constraint und kam als + * ungefangene PDOException samt Stacktrace beim Client an. + */ + public function testActivityWithUnknownTicketReturns422WithoutInternals(): void + { + $registerResponse = $this->app->handle(Request::create( + 'POST', + '/api/v1/agents', + body: ['display_name' => 'Alice', 'role' => 'entwickler'], + headers: ['X-Admin-Key' => 'admin-secret'] + )); + $apiKey = $registerResponse->body()['api_key']; + $agentId = $registerResponse->body()['agent']['id']; + + $response = $this->app->handle(Request::create( + 'POST', + '/api/v1/agents/' . $agentId . '/activity', + body: ['state' => 'working', 'current_ticket' => 999], + headers: ['X-Api-Key' => $apiKey] + )); + + self::assertSame(422, $response->status); + self::assertStringNotContainsString('PDOException', json_encode($response->body())); + } + public function testPatchUnknownTicketReturns404(): void { $registerResponse = $this->app->handle(Request::create( diff --git a/api/tests/Unit/ConfigTest.php b/api/tests/Unit/ConfigTest.php index c7c9f9f..cf85e4e 100644 --- a/api/tests/Unit/ConfigTest.php +++ b/api/tests/Unit/ConfigTest.php @@ -21,6 +21,22 @@ final class ConfigTest extends TestCase self::assertSame(8, $config->deskCount()); } + /** + * Regression: get() griff frueher selbst auf getenv() zurueck, wodurch ein in der + * Umgebung gesetzter CODEBERG_TOKEN in eine explizit gebaute Config (und damit in + * Testausgaben) durchschlug. + */ + public function testExplicitEnvArrayDoesNotFallBackToProcessEnvironment(): void + { + putenv('CODEBERG_TOKEN=darf-nicht-durchsickern'); + + try { + self::assertNull((new Config([]))->codebergToken()); + } finally { + putenv('CODEBERG_TOKEN'); + } + } + public function testValuesCanBeOverridden(): void { $config = new Config([ diff --git a/api/tests/Unit/Controller/AgentControllerTest.php b/api/tests/Unit/Controller/AgentControllerTest.php index 5cf9ca8..60ca743 100644 --- a/api/tests/Unit/Controller/AgentControllerTest.php +++ b/api/tests/Unit/Controller/AgentControllerTest.php @@ -8,16 +8,20 @@ use Monitor\Api\Controller\AgentController; use Monitor\Api\Db\Database; use Monitor\Api\Http\Request; use Monitor\Api\Repository\AgentRepository; +use Monitor\Api\Repository\TicketRepository; use PHPUnit\Framework\TestCase; final class AgentControllerTest extends TestCase { private AgentController $controller; private AgentRepository $repository; + private TicketRepository $tickets; protected function setUp(): void { - $this->repository = new AgentRepository(Database::inMemory()); + $db = Database::inMemory(); + $this->repository = new AgentRepository($db); + $this->tickets = new TicketRepository($db); $this->controller = new AgentController($this->repository); } @@ -78,16 +82,17 @@ final class AgentControllerTest extends TestCase public function testActivityUpdatesOwnState(): void { $registration = $this->repository->register('Alice', 'entwickler'); + $ticketId = (int) $this->tickets->create(['title' => 'Testticket'])['id']; $response = $this->controller->activity( - Request::create('POST', '/x', body: ['state' => 'working', 'current_ticket' => 7]), + Request::create('POST', '/x', body: ['state' => 'working', 'current_ticket' => $ticketId]), ['id' => (string) $registration['agent']['id']], $registration['agent'] ); self::assertSame(200, $response->status); self::assertSame('working', $response->body()['agent']['state']); - self::assertSame(7, $response->body()['agent']['current_ticket_id']); + self::assertSame($ticketId, $response->body()['agent']['current_ticket_id']); } public function testIndexListsAgents(): void diff --git a/api/tests/Unit/Repository/AgentRepositoryTest.php b/api/tests/Unit/Repository/AgentRepositoryTest.php index 4fdaccc..f56e8de 100644 --- a/api/tests/Unit/Repository/AgentRepositoryTest.php +++ b/api/tests/Unit/Repository/AgentRepositoryTest.php @@ -8,15 +8,19 @@ use InvalidArgumentException; use Monitor\Api\Db\Database; use Monitor\Api\Http\NotFoundException; use Monitor\Api\Repository\AgentRepository; +use Monitor\Api\Repository\TicketRepository; use PHPUnit\Framework\TestCase; final class AgentRepositoryTest extends TestCase { private AgentRepository $repository; + private TicketRepository $tickets; protected function setUp(): void { - $this->repository = new AgentRepository(Database::inMemory(), deskCount: 2); + $db = Database::inMemory(); + $this->repository = new AgentRepository($db, deskCount: 2); + $this->tickets = new TicketRepository($db); } public function testRegisterAssignsLowestFreeDesk(): void @@ -72,11 +76,21 @@ final class AgentRepositoryTest extends TestCase { $result = $this->repository->register('Alice', 'entwickler'); $id = (int) $result['agent']['id']; + $ticketId = (int) $this->tickets->create(['title' => 'Testticket'])['id']; - $updated = $this->repository->updateActivity($id, 'working', 42); + $updated = $this->repository->updateActivity($id, 'working', $ticketId); self::assertSame('working', $updated['state']); - self::assertSame(42, $updated['current_ticket_id']); + self::assertSame($ticketId, $updated['current_ticket_id']); + } + + public function testUpdateActivityRejectsUnknownTicket(): void + { + $result = $this->repository->register('Alice', 'entwickler'); + + $this->expectException(InvalidArgumentException::class); + + $this->repository->updateActivity((int) $result['agent']['id'], 'working', 999); } public function testUpdateActivityThrowsForUnknownAgent(): void diff --git a/api/tests/Unit/Sync/CodebergSyncServiceTest.php b/api/tests/Unit/Sync/CodebergSyncServiceTest.php index 4b8fbf2..1411db2 100644 --- a/api/tests/Unit/Sync/CodebergSyncServiceTest.php +++ b/api/tests/Unit/Sync/CodebergSyncServiceTest.php @@ -51,6 +51,22 @@ final class CodebergSyncServiceTest extends TestCase self::assertCount(1, $done); self::assertSame('Geschlossenes Issue', $done[0]['title']); self::assertNull($done[0]['type']); + self::assertNull($done[0]['color']); + } + + public function testSyncNormalizesLabelColorRegardlessOfLeadingHash(): void + { + $tickets = new TicketRepository(Database::inMemory()); + $client = new FakeCodebergClient([ + 'open' => [ + ['number' => 1, 'title' => 'Mit Raute', 'body' => null, 'labels' => [['name' => 'bug', 'color' => '#00ff00']]], + ], + 'closed' => [], + ]); + + (new CodebergSyncService($client, $tickets, 'fds2610', 'monitor'))->sync(); + + self::assertSame('#00ff00', $tickets->all('backlog')[0]['color']); } public function testSyncDoesNotResetManuallyMovedTicket(): void diff --git a/api/tests/Unit/Sync/CurlCodebergClientTest.php b/api/tests/Unit/Sync/CurlCodebergClientTest.php index d0a5cd0..e478987 100644 --- a/api/tests/Unit/Sync/CurlCodebergClientTest.php +++ b/api/tests/Unit/Sync/CurlCodebergClientTest.php @@ -82,7 +82,8 @@ namespace Monitor\Api\Tests\Sync { self::assertCount(1, $issues); self::assertSame(1, $issues[0]['number']); - self::assertSame('#ff0000', $issues[0]['labels'][0]['color']); + // Der Client reicht die Label-Farbe roh durch; das "#" setzt CodebergSyncService. + self::assertSame('ff0000', $issues[0]['labels'][0]['color']); self::assertStringContainsString('state=open', CurlStub::$requestedUrls[0]); }