Merge pull request 'Vier fehlschlagende Tests behoben + composer.lock einchecken' (#6) from fds2610-bot/monitor:main into main

Reviewed-on: https://codeberg.org/fds2610/monitor/pulls/6
This commit is contained in:
fds2610 2026-07-27 20:32:01 +02:00
commit 68a57b2e1f
12 changed files with 1808 additions and 11 deletions

1677
api/composer.lock generated Normal file

File diff suppressed because it is too large Load diff

View file

@ -24,6 +24,7 @@ use Monitor\Api\Repository\KpiRepository;
use Monitor\Api\Repository\LegacyStatusFileReader; use Monitor\Api\Repository\LegacyStatusFileReader;
use Monitor\Api\Repository\SystemRepository; use Monitor\Api\Repository\SystemRepository;
use Monitor\Api\Repository\TicketRepository; use Monitor\Api\Repository\TicketRepository;
use Throwable;
/** /**
* Verdrahtet Router, Controller und Auth fuer alle /api/v1/*-Endpunkte aus * Verdrahtet Router, Controller und Auth fuer alle /api/v1/*-Endpunkte aus
@ -108,6 +109,12 @@ final class Application
return Response::error($e->getMessage(), 404); return Response::error($e->getMessage(), 404);
} catch (InvalidArgumentException $e) { } catch (InvalidArgumentException $e) {
return Response::error($e->getMessage(), 422); 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);
} }
} }
} }

View file

@ -7,6 +7,11 @@ namespace Monitor\Api;
/** /**
* Liest die Konfiguration ausschliesslich aus Umgebungsvariablen (12-factor-Stil), * Liest die Konfiguration ausschliesslich aus Umgebungsvariablen (12-factor-Stil),
* damit die Docker-Compose-Umgebung ohne zusaetzliche Config-Dateien auskommt. * 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 final class Config
{ {
@ -21,7 +26,7 @@ final class Config
private function get(string $key, ?string $default = null): ?string 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 === '') { if ($value === false || $value === null || $value === '') {
return $default; return $default;
} }

View file

@ -100,9 +100,19 @@ final class AgentRepository
return array_map(fn (array $row) => $this->hidingKey($row), $stmt->fetchAll()); return array_map(fn (array $row) => $this->hidingKey($row), $stmt->fetchAll());
} }
/** @return array<string, mixed> */ /**
* 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<string, mixed>
*/
public function updateActivity(int $id, string $state, ?int $currentTicketId): 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( $stmt = $this->db->pdo()->prepare(
'UPDATE agents SET state = :state, current_ticket_id = :ticket_id, last_seen_at = :now WHERE id = :id' '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; 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<string, mixed> $row @return array<string, mixed> */ /** @param array<string, mixed> $row @return array<string, mixed> */
private function hidingKey(array $row): array private function hidingKey(array $row): array
{ {

View file

@ -47,7 +47,19 @@ final class CodebergSyncService
'description' => $issue['body'], 'description' => $issue['body'],
'column' => $column, 'column' => $column,
'type' => $firstLabel['name'] ?? null, '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, '#');
}
} }

View file

@ -45,7 +45,7 @@ final class CurlCodebergClient implements CodebergClient
'labels' => array_map( 'labels' => array_map(
static fn (array $label): array => [ static fn (array $label): array => [
'name' => (string) $label['name'], 'name' => (string) $label['name'],
'color' => '#' . ltrim((string) $label['color'], '#'), 'color' => (string) $label['color'],
], ],
$issue['labels'] ?? [] $issue['labels'] ?? []
), ),

View file

@ -73,6 +73,32 @@ final class ApplicationTest extends TestCase
self::assertCount(1, $listResponse->body()['tickets']); 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 public function testPatchUnknownTicketReturns404(): void
{ {
$registerResponse = $this->app->handle(Request::create( $registerResponse = $this->app->handle(Request::create(

View file

@ -21,6 +21,22 @@ final class ConfigTest extends TestCase
self::assertSame(8, $config->deskCount()); 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 public function testValuesCanBeOverridden(): void
{ {
$config = new Config([ $config = new Config([

View file

@ -8,16 +8,20 @@ use Monitor\Api\Controller\AgentController;
use Monitor\Api\Db\Database; use Monitor\Api\Db\Database;
use Monitor\Api\Http\Request; use Monitor\Api\Http\Request;
use Monitor\Api\Repository\AgentRepository; use Monitor\Api\Repository\AgentRepository;
use Monitor\Api\Repository\TicketRepository;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
final class AgentControllerTest extends TestCase final class AgentControllerTest extends TestCase
{ {
private AgentController $controller; private AgentController $controller;
private AgentRepository $repository; private AgentRepository $repository;
private TicketRepository $tickets;
protected function setUp(): void 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); $this->controller = new AgentController($this->repository);
} }
@ -78,16 +82,17 @@ final class AgentControllerTest extends TestCase
public function testActivityUpdatesOwnState(): void public function testActivityUpdatesOwnState(): void
{ {
$registration = $this->repository->register('Alice', 'entwickler'); $registration = $this->repository->register('Alice', 'entwickler');
$ticketId = (int) $this->tickets->create(['title' => 'Testticket'])['id'];
$response = $this->controller->activity( $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']], ['id' => (string) $registration['agent']['id']],
$registration['agent'] $registration['agent']
); );
self::assertSame(200, $response->status); self::assertSame(200, $response->status);
self::assertSame('working', $response->body()['agent']['state']); 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 public function testIndexListsAgents(): void

View file

@ -8,15 +8,19 @@ use InvalidArgumentException;
use Monitor\Api\Db\Database; use Monitor\Api\Db\Database;
use Monitor\Api\Http\NotFoundException; use Monitor\Api\Http\NotFoundException;
use Monitor\Api\Repository\AgentRepository; use Monitor\Api\Repository\AgentRepository;
use Monitor\Api\Repository\TicketRepository;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
final class AgentRepositoryTest extends TestCase final class AgentRepositoryTest extends TestCase
{ {
private AgentRepository $repository; private AgentRepository $repository;
private TicketRepository $tickets;
protected function setUp(): void 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 public function testRegisterAssignsLowestFreeDesk(): void
@ -72,11 +76,21 @@ final class AgentRepositoryTest extends TestCase
{ {
$result = $this->repository->register('Alice', 'entwickler'); $result = $this->repository->register('Alice', 'entwickler');
$id = (int) $result['agent']['id']; $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('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 public function testUpdateActivityThrowsForUnknownAgent(): void

View file

@ -51,6 +51,22 @@ final class CodebergSyncServiceTest extends TestCase
self::assertCount(1, $done); self::assertCount(1, $done);
self::assertSame('Geschlossenes Issue', $done[0]['title']); self::assertSame('Geschlossenes Issue', $done[0]['title']);
self::assertNull($done[0]['type']); 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 public function testSyncDoesNotResetManuallyMovedTicket(): void

View file

@ -82,7 +82,8 @@ namespace Monitor\Api\Tests\Sync {
self::assertCount(1, $issues); self::assertCount(1, $issues);
self::assertSame(1, $issues[0]['number']); 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]); self::assertStringContainsString('state=open', CurlStub::$requestedUrls[0]);
} }