From b2ac761cc2e875564164babf5f26c5dfa5fa449e Mon Sep 17 00:00:00 2001 From: Bastian Lederer Date: Mon, 31 Aug 2026 07:58:27 +0200 Subject: [PATCH] Rework `Incidents` and `Incident` api Add 3 separate factories to `Incidents`. `matchAll()` and `getAll()` aaccept an iterable where each element is a set of tags, all incidents matching any of the tag sets are yielded. In the case of `getAll()` exact matches are required, `matchAll()` allows partial matches as well. `get()` expects the full tags of a single object, and creates an `Incident` from a query instead of a `Model`. The query is run lazily, if it does not return an `Inciednt` an `IncidentNotFoundException` is thrown. --- .../Exception/IncidentNotFoundException.php | 12 + .../Notifications/Integrations/Incident.php | 205 +++++- .../Notifications/Integrations/Incidents.php | 202 ++++-- .../Integrations/IncidentTest.php | 664 +++++++++++++----- .../Integrations/IncidentsTest.php | 363 +++++----- 5 files changed, 997 insertions(+), 449 deletions(-) create mode 100644 library/Notifications/Integrations/Exception/IncidentNotFoundException.php diff --git a/library/Notifications/Integrations/Exception/IncidentNotFoundException.php b/library/Notifications/Integrations/Exception/IncidentNotFoundException.php new file mode 100644 index 000000000..7852aa2b0 --- /dev/null +++ b/library/Notifications/Integrations/Exception/IncidentNotFoundException.php @@ -0,0 +1,12 @@ + +// SPDX-License-Identifier: GPL-3.0-or-later + +namespace Icinga\Module\Notifications\Integrations\Exception; + +use Exception; + +class IncidentNotFoundException extends Exception +{ +} diff --git a/library/Notifications/Integrations/Incident.php b/library/Notifications/Integrations/Incident.php index 7d2675c8c..926f47274 100644 --- a/library/Notifications/Integrations/Incident.php +++ b/library/Notifications/Integrations/Incident.php @@ -8,35 +8,116 @@ use DateTime; use Generator; use Icinga\Module\Notifications\Common\EntityManager; +use Icinga\Module\Notifications\Integrations\Exception\IncidentNotFoundException; use Icinga\Module\Notifications\Model\Contact; use Icinga\Module\Notifications\Model\Incident as IncidentModel; use Icinga\Module\Notifications\Model\IncidentContact; use Icinga\Module\Notifications\Model\IncidentHistory; +use Icinga\User; use InvalidArgumentException; +use ipl\Orm\Query; use ipl\Sql\Connection; +use ipl\Sql\Expression; use ipl\Stdlib\Filter; +use LogicException; /** * Manage an incident's recipients and read its state */ class Incident { - /** @var IncidentModel The managed incident */ - private IncidentModel $incident; + /** @var ?IncidentModel The managed incident, null if it wasn't fetched yet */ + private ?IncidentModel $incident = null; + + /** @var ?Query The query to lazy load the incident */ + private ?Query $query = null; /** @var Connection The database connection to use */ private Connection $db; + private function __construct() + { + } + /** - * Create a new wrapper for the given model + * Create an instance from a query that should return one incident * - * @param IncidentModel $incident - * @param Connection $db The connection to read and persist through + * If the query does not return an incident, calling any function on the created instance throws an + * {@see IncidentNotFoundException} + * + * @param Query $query + * + * @return static */ - public function __construct(IncidentModel $incident, Connection $db) + public static function fromQuery(Query $query): static { - $this->incident = $incident; - $this->db = $db; + $incident = new static(); + $incident->query = $query; + $incident->db = $query->getDb(); + + return $incident; + } + + /** + * Create an instance from an {@see IncidentModel} + * + * Instances created with this factory will never throw an {@see IncidentNotFoundException} + * + * @param IncidentModel $model + * @param Connection $db + * + * @return static + */ + public static function fromModel(IncidentModel $model, Connection $db): static + { + $incident = new static(); + $incident->incident = $model; + $incident->db = $db; + + return $incident; + } + + /** + * Get the given user's role for the incident, null if the user has no role, throws if no matching incident exists + * + * @param User $user + * + * @return 'manager'|'subscriber'|'recipient'|null + * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result + */ + public function getRole(User $user): ?string + { + if ($this->incident === null) { + $incidentContactTable = (new IncidentContact())->getTableName(); + $contactTable = (new Contact())->getTableName(); + $query = $this->consumeQuery() + ->withColumns(['role' => + new Expression( + "(SELECT ic.role FROM $incidentContactTable AS ic" + . " JOIN $contactTable AS c ON ic.contact_id = c.id" + . " WHERE c.username = ? AND c.deleted = 'n' AND ic.incident_id = %s)", + ['id'], + $user->getUsername() + ) + ]); + + $this->incident = $query->first(); + if ($this->incident === null) { + throw new IncidentNotFoundException('No matching incident was found'); + } + + return $this->incident->role; + } else { + return IncidentContact::on($this->db) + ->columns('role') + ->filter(Filter::all( + Filter::equal('incident_id', $this->incident()->id), + Filter::equal('contact.username', $user->getUsername()) + )) + ->first() + ?->role; + } } /** @@ -48,6 +129,7 @@ public function __construct(IncidentModel $incident, Connection $db) * * @return $this * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result * @throws InvalidArgumentException If no contact with that username exists */ public function addManager(string $username): static @@ -66,6 +148,7 @@ public function addManager(string $username): static * * @return $this * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result * @throws InvalidArgumentException If no contact with that username exists */ public function addSubscriber(string $username): static @@ -84,6 +167,7 @@ public function addSubscriber(string $username): static * * @return $this * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result * @throws InvalidArgumentException If no contact with that username exists */ public function removeManager(string $username): static @@ -102,6 +186,7 @@ public function removeManager(string $username): static * * @return $this * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result * @throws InvalidArgumentException If no contact with that username exists */ public function removeSubscriber(string $username): static @@ -122,53 +207,65 @@ public function removeSubscriber(string $username): static /** * Yield each active subscriber of the incident * - * @return Generator + * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result */ - public function getSubscribers(): Generator + public function getSubscribers(): array { - foreach ($this->resolveRecipients(['manager', 'subscriber']) as $recipient) { - yield [ - 'name' => $recipient['name'], - 'username' => $recipient['username'], - 'role' => $recipient['role'], - 'roleChangedAt' => $recipient['roleChangedAt'], - ]; - } + return array_map( + function ($recipient) { + return [ + 'name' => $recipient['name'], + 'username' => $recipient['username'], + 'role' => $recipient['role'], + 'roleChangedAt' => $recipient['roleChangedAt'] + ]; + }, + $this->resolveRecipients(['manager', 'subscriber']) + ); } /** * Yield each configured recipient of the incident * - * @return Generator + * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result */ - public function getRecipients(): Generator + public function getRecipients(): array { - foreach ($this->resolveRecipients(['recipient']) as $recipient) { - yield [ - 'type' => $recipient['type'], - 'name' => $recipient['name'], - 'username' => $recipient['username'], - 'roleChangedAt' => $recipient['roleChangedAt'] - ]; - } + return array_map( + function ($recipient) { + return [ + 'type' => $recipient['type'], + 'name' => $recipient['name'], + 'username' => $recipient['username'], + 'roleChangedAt' => $recipient['roleChangedAt'] + ]; + }, + $this->resolveRecipients(['recipient']) + ); } /** * Get whether the incident is muted * * @return bool + * + * @throws IncidentNotFoundException If the query passed to {@see static::fromQuery()} has no result */ public function isMuted(): bool { - return $this->incident->mute_reason !== null; + return $this->incident()->mute_reason !== null; } /** @@ -202,7 +299,7 @@ private function existingContact(int $contactId): ?IncidentContact /** @var ?IncidentContact $entry */ $entry = IncidentContact::on($this->db) ->filter(Filter::all( - Filter::equal('incident_id', $this->incident->id), + Filter::equal('incident_id', $this->incident()->id), Filter::equal('contact_id', $contactId) )) ->first() @@ -231,7 +328,7 @@ private function resolveRecipients(array $roles): array ->with(['contact', 'contactgroup', 'schedule']) ->filter( Filter::all( - Filter::equal('incident_id', $this->incident->id), + Filter::equal('incident_id', $this->incident()->id), Filter::equal('role', $roles) ) ); @@ -296,7 +393,7 @@ private function assignRole(string $username, string $role, array $noopRoles): s (new EntityManager($this->db))->save($existing); } else { $incidentContact = (new IncidentContact())->setNew(); - $incidentContact->incident_id = $this->incident->id; + $incidentContact->incident_id = $this->incident()->id; $incidentContact->contact_id = $contact->id; $incidentContact->role = $role; (new EntityManager($this->db))->save($incidentContact); @@ -319,7 +416,7 @@ private function assignRole(string $username, string $role, array $noopRoles): s private function addRoleChangedHistory(int $contactId, ?string $oldRole, ?string $newRole): void { $history = (new IncidentHistory())->setNew(); - $history->incident_id = $this->incident->id; + $history->incident_id = $this->incident()->id; $history->contact_id = $contactId; $history->type = 'recipient_role_changed'; $history->old_recipient_role = $oldRole; @@ -327,4 +424,46 @@ private function addRoleChangedHistory(int $contactId, ?string $oldRole, ?string $history->time = new DateTime(); (new EntityManager($this->db))->save($history); } + + /** + * Fetch the incident lazily and return it + * + * @return IncidentModel + * + * @throws IncidentNotFoundException + */ + private function incident(): IncidentModel + { + if ($this->incident === null) { + $this->incident = $this->consumeQuery()->first(); + } + + if ($this->incident === null) { + throw new IncidentNotFoundException('No matching incident was found'); + } + + return $this->incident; + } + + /** + * Single use getter for the query to lazy load the incident + * + * @return Query + * + * @throws LogicException If the query has already been consumed + */ + private function consumeQuery(): Query + { + if ($this->query === null) { + throw new LogicException( + 'Cannot fetch the incident again, the query has already been consumed.' + . 'An earlier call probably failed with an IncidentNotFoundException.' + ); + } + + $query = $this->query; + $this->query = null; + + return $query; + } } diff --git a/library/Notifications/Integrations/Incidents.php b/library/Notifications/Integrations/Incidents.php index c8e4460d2..3513f29d4 100644 --- a/library/Notifications/Integrations/Incidents.php +++ b/library/Notifications/Integrations/Incidents.php @@ -8,14 +8,16 @@ use Countable; use Generator; use Icinga\Module\Notifications\Common\Database; +use Icinga\Module\Notifications\Integrations\Exception\IncidentNotFoundException; use Icinga\Module\Notifications\Model\Incident as IncidentModel; use Icinga\Module\Notifications\Repository\ContactRepository; use Icinga\User; +use InvalidArgumentException; use ipl\Orm\Query; use ipl\Orm\ResultSet; use ipl\Sql\Connection; -use ipl\Sql\Expression; use ipl\Stdlib\Filter; +use ipl\Stdlib\Filter\Any; use IteratorAggregate; /** @@ -23,109 +25,105 @@ */ class Incidents implements IteratorAggregate, Countable { - /** @var array Required tags keyed by name; a null value requires the tag's absence */ - protected array $tags; - - /** @var Connection The database connection to read from */ - protected Connection $db; - - /** @var ?ResultSet The executed query result */ + /** @var ?ResultSet The executed query result */ private ?ResultSet $results = null; + /** @var Query The query to get the matching incidents */ + private Query $query; + /** - * Create new Incidents + * Create new Incidents from the given query * - * Matches the incidents of every object that carries each tag given with a value and lacks each tag - * given as null. Tags not listed are unconstrained. Matching is performed by tag alone, independent of the source. - * - * Examples: - * ['host' => 'icinga2'] — host icinga2 and all of its services - * ['host' => 'icinga2', 'service' => 'ssh'] — only the ssh service on icinga2 - * ['host' => 'icinga2', 'service' => null] — only the host icinga2, none of its services + * The query is used as is, if it is intended to be used otherwise it should be cloned before passing it. * - * @param array $tags Required tags keyed by name, a null value requires the tag's absence - * @param Connection $db The database connection to read from + * @param Query $query */ - public function __construct(array $tags, Connection $db) + public function __construct(Query $query) { - $this->tags = $tags; - $this->db = $db; + $this->query = $query; } /** - * Create new Incidents for the given tags, reading through the default database connection + * Get the incident of the object matching the given set of id tags, reading through the default database connection * - * @param array $tags Required tags keyed by name, a null value requires the tag's absence + * If no matching incident is found, calling any function on the returned instance throws an + * {@see IncidentNotFoundException} * - * @return static + * @param array $tags The full set of id tags + * + * @return Incident + * + * @throws InvalidArgumentException If $tags is empty */ - public static function find(array $tags): static + public static function get(array $tags): Incident { - return new static($tags, Database::get()); + return Incident::fromQuery( + static::openIncidents(Database::get())->filter(static::tagSetFilterExactMatches([$tags])) + ); } /** - * Get whether the given user can manage incidents + * Get the matching incident for each given set of id tags * - * @param User $user + * @param iterable> $tagSets * - * @return bool + * @return static + * + * @throws InvalidArgumentException If $tagSets or one of its elements is empty */ - public static function canManage(User $user): bool + public static function getAll(iterable $tagSets): static { - return (new ContactRepository(Database::get()))->findByUsername($user->getUsername()) !== null; + return new static(static::openIncidents(Database::get())->filter(static::tagSetFilterExactMatches($tagSets))); } /** - * Get whether the given user can subscribe to incidents + * Get the incidents of all objects matching any of the given tag sets * - * @param User $user + * An absent tag matches any value. * - * @return bool + * Using null as a value requires the tag to be absent. + * + * If all given sets are guaranteed to contain the full id tags of each object use {@see static::getAll()} instead! + * + * Examples: + * [['host' => 'icinga2']] — host icinga2 and all of its services + * [['host' => 'icinga2', 'service' => 'ssh']] — only the ssh service on icinga2 + * [['host' => 'icinga2', 'service' => null]] — only the host icinga2, none of its services + * [['host' => 'a'], ['host' => 'b']] — the hosts a and b and all of their services + * + * @param iterable> $tagSets Tag sets + * + * @return static + * + * @throws InvalidArgumentException If $tagSets or one of its elements is empty */ - public static function canSubscribe(User $user): bool + public static function matchAll(iterable $tagSets): static { - return (new ContactRepository(Database::get()))->findByUsername($user->getUsername()) !== null; + return new static(static::openIncidents(Database::get())->filter(static::tagSetFilterPartialMatches($tagSets))); } /** - * Get whether the object has at least one incident + * Get whether the given user can manage incidents + * + * @param User $user * * @return bool */ - public function hasIncident(): bool + public static function canManage(User $user): bool { - if ($this->results !== null) { - return $this->incidents()->hasResult(); - } - - return $this->buildQuery() - ->columns([new Expression('1')]) - ->first() !== null; + return (new ContactRepository(Database::get()))->findByUsername($user->getUsername()) !== null; } /** - * Get the given user's current incident roles - * - * Returns a generator that yields {@see Incident} as key and the user's role as string. - * The role can either be 'manager', 'recipient' or 'subscriber'. + * Get whether the given user can subscribe to incidents * * @param User $user * - * @return Generator - * @phpstan-return Generator + * @return bool */ - public function getRoles(User $user): Generator + public static function canSubscribe(User $user): bool { - $filter = Filter::equal('incident_contact.contact.username', $user->getUsername()); - $filter->metaData()->set('forceOptimization', false); - - $incidents = $this->buildQuery() - ->withColumns(['role' => 'incident_contact.role']) - ->filter($filter); - foreach ($incidents as $incident) { - yield new Incident($incident, $this->db) => $incident->role; - } + return (new ContactRepository(Database::get()))->findByUsername($user->getUsername()) !== null; } /** @@ -137,7 +135,7 @@ public function getRoles(User $user): Generator public function getIterator(): Generator { foreach ($this->incidents() as $incident) { - yield new Incident($incident, $this->db); + yield Incident::fromModel($incident, $this->query->getDb()); } } @@ -152,24 +150,86 @@ public function count(): int private function incidents(): ResultSet { if ($this->results === null) { - $this->results = $this->buildQuery()->execute(); + $this->results = $this->query->execute(); } return $this->results; } - private function buildQuery(): Query + /** + * @return Query + */ + private static function openIncidents(Connection $db): Query { - $query = IncidentModel::on($this->db)->filter(Filter::unlike('recovered_at', '*')); + return IncidentModel::on($db)->filter(Filter::unlike('recovered_at', '*')); + } - foreach ($this->tags as $tag => $value) { - if ($value === null) { - $query->filter(Filter::unlike("incident.object.tag.$tag", '*')); - } else { - $query->filter(Filter::equal("incident.object.tag.$tag", $value)); + /** + * @param iterable> $tagSets + */ + private static function tagSetFilterExactMatches(iterable $tagSets): Any + { + $tagSetFilter = new Any(); + foreach ($tagSets as $tags) { + if (empty($tags)) { + throw new InvalidArgumentException('A set of id tags must not be empty'); } + + $tagSetFilter->add(Filter::equal('object_id', static::objectIdFor($tags))); + } + + if ($tagSetFilter->isEmpty()) { + throw new InvalidArgumentException('At least one set of id tags is required'); + } + + return $tagSetFilter; + } + + /** + * @param iterable> $tagSets + */ + private static function tagSetFilterPartialMatches(iterable $tagSets): Any + { + $tagSetFilter = new Any(); + foreach ($tagSets as $tags) { + if (empty($tags)) { + throw new InvalidArgumentException('A set of id tags must not be empty'); + } + + $tagFilter = Filter::all(); + foreach ($tags as $tag => $value) { + if ($value === null) { + $tagFilter->add(Filter::unlike("incident.object.tag.$tag", '*')); + } else { + $tagFilter->add(Filter::equal("incident.object.tag.$tag", $value)); + } + } + + $tagSetFilter->add($tagFilter); + } + + if ($tagSetFilter->isEmpty()) { + throw new InvalidArgumentException('At least one set of id tags is required'); + } + + return $tagSetFilter; + } + + /** + * Get the object id for the given id tags + * + * @param array $tags + * + * @return string + */ + private static function objectIdFor(array $tags): string + { + ksort($tags, SORT_STRING); + $data = ''; + foreach ($tags as $tag => $value) { + $data .= $tag . chr(0) . $value . chr(0); } - return $query; + return hash('sha256', $data); } } diff --git a/test/php/library/Notifications/Integrations/IncidentTest.php b/test/php/library/Notifications/Integrations/IncidentTest.php index 2fb70ed67..15bce8961 100644 --- a/test/php/library/Notifications/Integrations/IncidentTest.php +++ b/test/php/library/Notifications/Integrations/IncidentTest.php @@ -6,13 +6,19 @@ namespace Tests\Icinga\Module\Notifications\Integrations; use DateTime; +use Icinga\Module\Notifications\Integrations\Exception\IncidentNotFoundException; use Icinga\Module\Notifications\Integrations\Incident; use Icinga\Module\Notifications\Model\Incident as IncidentModel; +use Icinga\Module\Notifications\Test\DbTestBackends; +use Icinga\User; use InvalidArgumentException; +use ipl\Sql\Adapter\Pgsql; +use ipl\Sql\Connection; +use ipl\Sql\Test\SharedDatabases\TransactionIsolation; use ipl\Stdlib\Filter; use PDO; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; -use Tests\Icinga\Module\Notifications\Lib\EntityManager\RecordingConnection; /** * Contract of the integration-facing {@see Incident}: it is identified by usernames (never Contact @@ -25,44 +31,61 @@ * group or schedule — and yield a uniform shape carrying a `type` discriminator, the display `name` and a * nullable `username`. Subscribers additionally carry their `role` and the `roleChangedAt` time their * current role was last changed; deleted contact groups and schedules are omitted from both. + * + * The incident itself is either handed over as a model or resolved lazily from a query, which is why the + * role tests run against both {@see Incident::fromModel()} and {@see Incident::fromQuery()} — each resolves + * a role by different means. Only the latter can fail to find an incident, and if it does, every operation + * reports it the same way, by throwing an {@see IncidentNotFoundException}. + * + * Every test runs against real databases — once for MySQL and once for PostgreSQL (see {@see DbTestBackends} / + * `#[DataProvider('sharedDatabases')]`), each within its own transaction which is rolled back afterwards. The + * rows an incident consists of are seeded by the test itself, everything it merely requires to exist by + * {@see self::initializeNotificationsDb()}. */ +#[TransactionIsolation] class IncidentTest extends TestCase { - /** @var int Millisecond timestamp seeded into `incident_contact.changed_at` when a test does not care about it */ - private const SEEDED_CHANGED_AT = 1700000000000; + use DbTestBackends; + + /** @var int Id of the channel every contact refers to */ + private const CHANNEL_ID = 1; - private RecordingConnection $db; + /** @var int Millisecond timestamp every seeded `incident_contact` row is stamped with */ + private const ROLE_CHANGED_AT = 1700000000000; + + /** @var Connection The database of the current test, set by every test */ + private Connection $db; /** - * Set up an in-memory SQLite database with the tables these operations touch. The integration - * under test reads usernames and persists through the connection it is handed, so no global state - * is involved. + * Seed the channel every contact refers to and the object every incident belongs to + * + * Neither is seeded per test, as no test changes them and none of them cares about the object beyond that + * it exists. This runs before a test's transaction starts, so both survive its rollback. */ - protected function setUp(): void - { - $this->db = new RecordingConnection(['db' => 'sqlite', 'dbname' => ':memory:']); - $this->db->exec( - 'CREATE TABLE incident (id INTEGER PRIMARY KEY AUTOINCREMENT,' - . ' object_id BLOB, started_at INTEGER, recovered_at INTEGER, severity VARCHAR, mute_reason VARCHAR,' - . ' message VARCHAR);' - . 'CREATE TABLE contact (id INTEGER PRIMARY KEY AUTOINCREMENT, full_name VARCHAR, username VARCHAR,' - . ' default_channel_id INTEGER, changed_at INTEGER, deleted VARCHAR, external_uuid VARCHAR);' - . 'CREATE TABLE contactgroup (id INTEGER PRIMARY KEY AUTOINCREMENT, name VARCHAR, changed_at INTEGER,' - . ' deleted VARCHAR, external_uuid VARCHAR);' - . 'CREATE TABLE schedule (id INTEGER PRIMARY KEY AUTOINCREMENT, name VARCHAR, changed_at INTEGER,' - . ' timezone VARCHAR, deleted VARCHAR);' - . 'CREATE TABLE incident_contact (incident_id INTEGER NOT NULL, contact_id INTEGER,' - . ' contactgroup_id INTEGER, schedule_id INTEGER, role VARCHAR, changed_at INTEGER);' - . 'CREATE TABLE incident_history (id INTEGER PRIMARY KEY AUTOINCREMENT, incident_id INTEGER NOT NULL,' - . ' rule_id INTEGER, rule_escalation_id INTEGER, time INTEGER, type VARCHAR, contact_id INTEGER,' - . ' schedule_id INTEGER, contactgroup_id INTEGER, channel_id INTEGER, new_severity VARCHAR,' - . ' old_severity VARCHAR, new_recipient_role VARCHAR, old_recipient_role VARCHAR, message VARCHAR,' - . ' notification_state VARCHAR, sent_at INTEGER);' - ); + protected static function initializeNotificationsDb(Connection $db): void + { + $db->insert('available_channel_type', [ + 'type' => 'email', 'name' => 'Email', 'version' => '1', 'author' => 'Test', 'config_attrs' => '' + ]); + $db->insert('channel', [ + 'id' => self::CHANNEL_ID, + 'external_uuid' => static::transformUUIDForDB($db, '00000000-0000-0000-0000-0000000000c1'), + 'name' => 'Test', + 'type' => 'email', + 'changed_at' => (int) (new DateTime())->format('Uv') + ]); + + $db->insert('object', [ + 'id' => self::objectId($db), + 'name' => 'test' + ]); } - public function testAddManagerAddsTheContactAsManagerByUsername(): void + #[DataProvider('sharedDatabases')] + public function testAddManagerAddsTheContactAsManagerByUsername(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('uname'); @@ -80,8 +103,11 @@ public function testAddManagerAddsTheContactAsManagerByUsername(): void ); } - public function testAddManagerThrowsForAnUnknownUsername(): void + #[DataProvider('sharedDatabases')] + public function testAddManagerThrowsForAnUnknownUsername(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->expectException(InvalidArgumentException::class); @@ -89,8 +115,11 @@ public function testAddManagerThrowsForAnUnknownUsername(): void $this->incident($id)->addManager('ghost'); } - public function testRemoveManagerDemotesTheManagerToSubscriber(): void + #[DataProvider('sharedDatabases')] + public function testRemoveManagerDemotesTheManagerToSubscriber(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $contactId = $this->seedContact('uname'); $this->seedIncidentContact($id, $contactId, 'manager'); @@ -109,8 +138,11 @@ public function testRemoveManagerDemotesTheManagerToSubscriber(): void ); } - public function testAddSubscriberAddsTheContactAsSubscriberByUsername(): void + #[DataProvider('sharedDatabases')] + public function testAddSubscriberAddsTheContactAsSubscriberByUsername(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('uname'); @@ -128,8 +160,11 @@ public function testAddSubscriberAddsTheContactAsSubscriberByUsername(): void ); } - public function testRemoveSubscriberDeletesTheSubscriberEntry(): void + #[DataProvider('sharedDatabases')] + public function testRemoveSubscriberDeletesTheSubscriberEntry(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $contactId = $this->seedContact('uname'); $this->seedIncidentContact($id, $contactId, 'subscriber'); @@ -145,8 +180,11 @@ public function testRemoveSubscriberDeletesTheSubscriberEntry(): void ); } - public function testGetSubscribersExcludesConfiguredRecipients(): void + #[DataProvider('sharedDatabases')] + public function testGetSubscribersExcludesConfiguredRecipients(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('alice'), 'manager'); $this->seedIncidentContact($id, $this->seedContact('bob'), 'recipient'); @@ -157,21 +195,11 @@ public function testGetSubscribersExcludesConfiguredRecipients(): void ); } - public function testGetSubscribersIgnoresRowsWithoutARecipientReference(): void + #[DataProvider('sharedDatabases')] + public function testGetSubscribersOmitsDeletedRecipients(Connection $db): void { - $id = $this->seedIncident(); - $this->seedIncidentContact($id, $this->seedContact('alice'), 'manager'); - // Neither contact, contact group nor schedule is referenced; such a row yields nothing. - $this->seedIncidentContact($id, null, 'subscriber'); - - $this->assertSame( - [['name' => 'Alice Example', 'username' => 'alice', 'role' => 'manager']], - $this->withoutRoleChangedAt($this->incident($id)->getSubscribers()) - ); - } + $this->db = $db; - public function testGetSubscribersOmitsDeletedRecipients(): void - { $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('alice'), 'subscriber'); $this->seedIncidentContact($id, $this->seedContact('gone-contact', deleted: true), 'subscriber'); @@ -194,17 +222,19 @@ public function testGetSubscribersOmitsDeletedRecipients(): void ); } - public function testGetSubscribersResolvesRoleChangedAtFromTheContactsChangedAt(): void + #[DataProvider('sharedDatabases')] + public function testGetSubscribersResolvesRoleChangedAtFromTheRecipientsChangedAt(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); - $alice = $this->seedContact('alice'); - $this->seedIncidentContact($id, $alice, 'manager', changedAt: 1700000000000); + $this->seedIncidentContact($id, $this->seedContact('alice'), 'manager'); $subscribers = iterator_to_array($this->incident($id)->getSubscribers(), false); $this->assertCount(1, $subscribers); $this->assertInstanceOf(DateTime::class, $subscribers[0]['roleChangedAt']); - $this->assertSame(1700000000, $subscribers[0]['roleChangedAt']->getTimestamp()); + $this->assertSame(intdiv(self::ROLE_CHANGED_AT, 1000), $subscribers[0]['roleChangedAt']->getTimestamp()); unset($subscribers[0]['roleChangedAt']); $this->assertSame( @@ -214,25 +244,36 @@ public function testGetSubscribersResolvesRoleChangedAtFromTheContactsChangedAt( ); } - public function testGetRecipientsYieldsConfiguredRecipientsOfEachType(): void + #[DataProvider('sharedDatabases')] + public function testGetRecipientsYieldsConfiguredRecipientsOfEachType(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('alice'), 'recipient'); $this->seedIncidentContact($id, null, 'recipient', contactgroupId: $this->seedContactgroup('windows-admins')); $this->seedIncidentContact($id, null, 'recipient', scheduleId: $this->seedSchedule('On-Call')); + $recipients = $this->withoutRoleChangedAt($this->incident($id)->getRecipients()); + + // The reader does not guarantee an order, so it is normalised here instead of relying on the row order + usort($recipients, fn(array $a, array $b): int => [$a['type'], $a['name']] <=> [$b['type'], $b['name']]); + $this->assertSame( [ ['type' => 'contact', 'name' => 'Alice Example', 'username' => 'alice'], ['type' => 'contactgroup', 'name' => 'windows-admins', 'username' => null], ['type' => 'schedule', 'name' => 'On-Call', 'username' => null], ], - $this->withoutRoleChangedAt($this->sortedByTypeAndName($this->incident($id)->getRecipients())) + $recipients ); } - public function testGetRecipientsExcludesActiveSubscribers(): void + #[DataProvider('sharedDatabases')] + public function testGetRecipientsExcludesActiveSubscribers(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('alice'), 'manager'); $this->seedIncidentContact($id, $this->seedContact('bob'), 'subscriber'); @@ -244,8 +285,11 @@ public function testGetRecipientsExcludesActiveSubscribers(): void ); } - public function testGetRecipientsOmitsDeletedRecipients(): void + #[DataProvider('sharedDatabases')] + public function testGetRecipientsOmitsDeletedRecipients(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('alice'), 'recipient'); $this->seedIncidentContact($id, $this->seedContact('gone-contact', deleted: true), 'recipient'); @@ -268,17 +312,251 @@ public function testGetRecipientsOmitsDeletedRecipients(): void ); } - public function testIsMutedReflectsTheMuteReason(): void + #[DataProvider('sharedDatabases')] + public function testIsMutedReflectsTheMuteReason(Connection $db): void { - $muted = $this->seedIncident('down for maintenance'); + $this->db = $db; + + $muted = $this->seedIncident(muteReason: 'down for maintenance'); $notMuted = $this->seedIncident(); $this->assertTrue($this->incident($muted)->isMuted()); $this->assertFalse($this->incident($notMuted)->isMuted()); } - public function testAddManagerWritesRoleChangedHistory(): void + #[DataProvider('sharedDatabases')] + public function testGetRoleReturnsTheContactsRole(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $this->seedIncidentContact($id, $this->seedContact('boss'), 'manager'); + $this->seedIncidentContact($id, $this->seedContact('sub'), 'subscriber'); + $this->seedIncidentContact($id, $this->seedContact('rcpt'), 'recipient'); + + $incident = $this->incident($id); + + $this->assertSame('manager', $incident->getRole(new User('boss'))); + $this->assertSame('subscriber', $incident->getRole(new User('sub'))); + $this->assertSame('recipient', $incident->getRole(new User('rcpt'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleReturnsNullForAContactWithoutARole(Connection $db): void { + $this->db = $db; + + $id = $this->seedIncident(); + $this->seedContact('uname'); + + $this->assertNull($this->incident($id)->getRole(new User('uname'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleReturnsNullForAnUnknownUsername(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + + $this->assertNull($this->incident($id)->getRole(new User('ghost'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleOmitsDeletedContacts(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $this->seedIncidentContact($id, $this->seedContact('uname', deleted: true), 'manager'); + + $this->assertNull($this->incident($id)->getRole(new User('uname'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleIgnoresRolesOfOtherIncidents(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $otherId = $this->seedIncident(); + $this->seedIncidentContact($otherId, $this->seedContact('uname'), 'manager'); + + $this->assertNull($this->incident($id)->getRole(new User('uname'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleReturnsTheContactsRoleWhenTheIncidentIsFetchedLazily(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $this->seedIncidentContact($id, $this->seedContact('boss'), 'manager'); + $this->seedIncidentContact($id, $this->seedContact('sub'), 'subscriber'); + $this->seedContact('nobody'); + + $incident = $this->incidentFromQuery($id); + + $this->assertSame('manager', $incident->getRole(new User('boss'))); + $this->assertSame('subscriber', $incident->getRole(new User('sub'))); + $this->assertNull($incident->getRole(new User('nobody'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleOmitsDeletedContactsWhenTheIncidentIsFetchedLazily(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $this->seedIncidentContact($id, $this->seedContact('uname', deleted: true), 'manager'); + + $this->assertNull($this->incidentFromQuery($id)->getRole(new User('uname'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleIgnoresRolesOfOtherIncidentsWhenTheIncidentIsFetchedLazily(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $otherId = $this->seedIncident(); + $this->seedIncidentContact($otherId, $this->seedContact('uname'), 'manager'); + + $this->assertNull($this->incidentFromQuery($id)->getRole(new User('uname'))); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->getRole(new User('uname')); + } + + #[DataProvider('sharedDatabases')] + public function testIsMutedThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->isMuted(); + } + + #[DataProvider('sharedDatabases')] + public function testGetSubscribersThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->getSubscribers(); + } + + #[DataProvider('sharedDatabases')] + public function testGetRecipientsThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->getRecipients(); + } + + #[DataProvider('sharedDatabases')] + public function testAddManagerThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->seedContact('uname'); // Or the username is reported as unknown instead + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->addManager('uname'); + } + + #[DataProvider('sharedDatabases')] + public function testAddSubscriberThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->seedContact('uname'); + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->addSubscriber('uname'); + } + + #[DataProvider('sharedDatabases')] + public function testRemoveManagerThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->seedContact('uname'); + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->removeManager('uname'); + } + + #[DataProvider('sharedDatabases')] + public function testRemoveSubscriberThrowsWithoutAMatchingIncident(Connection $db): void + { + $this->db = $db; + + $this->seedContact('uname'); + + $this->expectException(IncidentNotFoundException::class); + + $this->incidentFromQuery(0)->removeSubscriber('uname'); + } + + #[DataProvider('sharedDatabases')] + public function testTheIncidentIsFetchedLazilyAndOnlyOnce(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $incident = $this->incidentFromQuery($id); + + // Had the instance fetched the incident upon creation, it would still answer with the row as it was then + $this->db->update('incident', ['mute_reason' => 'down for maintenance'], ['id = ?' => $id]); + + $this->assertTrue($incident->isMuted(), 'The incident was fetched before it was used'); + + // And now that it has been fetched, that very row is what it keeps answering with + $this->db->update('incident', ['mute_reason' => null], ['id = ?' => $id]); + + $this->assertTrue($incident->isMuted(), 'The incident was fetched again instead of being reused'); + } + + #[DataProvider('sharedDatabases')] + public function testGetRoleFetchesTheIncidentAlongWithTheRoleAndOnlyOnce(Connection $db): void + { + $this->db = $db; + + $id = $this->seedIncident(); + $this->seedIncidentContact($id, $this->seedContact('boss'), 'manager'); + $this->seedIncidentContact($id, $this->seedContact('sub'), 'subscriber'); + + $incident = $this->incidentFromQuery($id); + + $this->assertSame('manager', $incident->getRole(new User('boss'))); + + $this->db->update('incident', ['mute_reason' => 'down for maintenance'], ['id = ?' => $id]); + + // Only the first call fetches the incident, the role of any other user is resolved by a separate query + $this->assertSame('subscriber', $incident->getRole(new User('sub')), 'The second role was not resolved'); + $this->assertFalse($incident->isMuted(), 'The incident was fetched again instead of being reused'); + } + + #[DataProvider('sharedDatabases')] + public function testAddManagerWritesRoleChangedHistory(Connection $db): void + { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('uname'); @@ -290,8 +568,11 @@ public function testAddManagerWritesRoleChangedHistory(): void ); } - public function testRemoveManagerWritesRoleChangedHistory(): void + #[DataProvider('sharedDatabases')] + public function testRemoveManagerWritesRoleChangedHistory(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'manager'); @@ -303,8 +584,11 @@ public function testRemoveManagerWritesRoleChangedHistory(): void ); } - public function testAddSubscriberWritesRoleChangedHistory(): void + #[DataProvider('sharedDatabases')] + public function testAddSubscriberWritesRoleChangedHistory(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('uname'); @@ -316,8 +600,11 @@ public function testAddSubscriberWritesRoleChangedHistory(): void ); } - public function testRemoveSubscriberWritesRoleChangedHistory(): void + #[DataProvider('sharedDatabases')] + public function testRemoveSubscriberWritesRoleChangedHistory(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'subscriber'); @@ -329,8 +616,11 @@ public function testRemoveSubscriberWritesRoleChangedHistory(): void ); } - public function testChainedRoleChangesEachWriteAHistoryRow(): void + #[DataProvider('sharedDatabases')] + public function testChainedRoleChangesEachWriteAHistoryRow(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('alice'); $this->seedContact('bob'); @@ -348,8 +638,11 @@ public function testChainedRoleChangesEachWriteAHistoryRow(): void ); } - public function testASecondWriteDoesNotDuplicateHistoryFromAnEarlierWrite(): void + #[DataProvider('sharedDatabases')] + public function testASecondWriteDoesNotDuplicateHistoryFromAnEarlierWrite(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('alice'); $this->seedContact('bob'); @@ -367,8 +660,11 @@ public function testASecondWriteDoesNotDuplicateHistoryFromAnEarlierWrite(): voi ); } - public function testAddSubscriberDoesNotDemoteAnExistingManager(): void + #[DataProvider('sharedDatabases')] + public function testAddSubscriberDoesNotDemoteAnExistingManager(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'manager'); @@ -378,8 +674,11 @@ public function testAddSubscriberDoesNotDemoteAnExistingManager(): void $this->assertSame([], $this->storedRoleHistory()); } - public function testAddManagerPromotesAnExistingSubscriberInPlace(): void + #[DataProvider('sharedDatabases')] + public function testAddManagerPromotesAnExistingSubscriberInPlace(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'subscriber'); @@ -392,8 +691,11 @@ public function testAddManagerPromotesAnExistingSubscriberInPlace(): void ); } - public function testAddManagerOnAnExistingManagerIsANoop(): void + #[DataProvider('sharedDatabases')] + public function testAddManagerOnAnExistingManagerIsANoop(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'manager'); @@ -403,8 +705,11 @@ public function testAddManagerOnAnExistingManagerIsANoop(): void $this->assertSame([], $this->storedRoleHistory(), 'A no-op records no role change'); } - public function testAddSubscriberOnAnExistingSubscriberIsANoop(): void + #[DataProvider('sharedDatabases')] + public function testAddSubscriberOnAnExistingSubscriberIsANoop(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'subscriber'); @@ -414,8 +719,11 @@ public function testAddSubscriberOnAnExistingSubscriberIsANoop(): void $this->assertSame([], $this->storedRoleHistory(), 'A no-op records no role change'); } - public function testRemoveManagerOfANonManagerIsANoop(): void + #[DataProvider('sharedDatabases')] + public function testRemoveManagerOfANonManagerIsANoop(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'subscriber'); @@ -426,8 +734,11 @@ public function testRemoveManagerOfANonManagerIsANoop(): void $this->assertSame([], $this->storedRoleHistory()); } - public function testRemoveManagerWithoutAnEntryIsANoop(): void + #[DataProvider('sharedDatabases')] + public function testRemoveManagerWithoutAnEntryIsANoop(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('uname'); @@ -437,8 +748,11 @@ public function testRemoveManagerWithoutAnEntryIsANoop(): void $this->assertSame([], $this->storedRoleHistory()); } - public function testRemoveSubscriberOfANonSubscriberIsANoop(): void + #[DataProvider('sharedDatabases')] + public function testRemoveSubscriberOfANonSubscriberIsANoop(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedIncidentContact($id, $this->seedContact('uname'), 'manager'); @@ -449,8 +763,11 @@ public function testRemoveSubscriberOfANonSubscriberIsANoop(): void $this->assertSame([], $this->storedRoleHistory()); } - public function testRemoveSubscriberWithoutAnEntryIsANoop(): void + #[DataProvider('sharedDatabases')] + public function testRemoveSubscriberWithoutAnEntryIsANoop(Connection $db): void { + $this->db = $db; + $id = $this->seedIncident(); $this->seedContact('uname'); @@ -460,81 +777,6 @@ public function testRemoveSubscriberWithoutAnEntryIsANoop(): void $this->assertSame([], $this->storedRoleHistory()); } - /** - * Insert an incident and return its generated id. - */ - private function seedIncident(?string $muteReason = null): int - { - $this->db->insert('incident', ['severity' => 'crit', 'mute_reason' => $muteReason]); - - return (int) $this->db->lastInsertId(); - } - - /** - * Insert a contact with the given username and return its generated id. - * - * The full name defaults to a distinct value derived from the username (e.g. "Alice Example" for - * "alice"), so the readers' name/username pairing can be asserted unambiguously. - */ - private function seedContact(string $username, bool $deleted = false): int - { - $fullName = ucfirst($username) . ' Example'; - - $this->db->insert( - 'contact', - ['full_name' => $fullName, 'username' => $username, 'deleted' => $deleted ? 'y' : 'n'] - ); - - return (int) $this->db->lastInsertId(); - } - - /** - * Insert a contact group with the given name and return its generated id. - */ - private function seedContactgroup(string $name, bool $deleted = false): int - { - $this->db->insert('contactgroup', ['name' => $name, 'deleted' => $deleted ? 'y' : 'n']); - - return (int) $this->db->lastInsertId(); - } - - /** - * Insert a schedule with the given name and return its generated id. - */ - private function seedSchedule(string $name, bool $deleted = false): int - { - $this->db->insert('schedule', ['name' => $name, 'deleted' => $deleted ? 'y' : 'n']); - - return (int) $this->db->lastInsertId(); - } - - /** - * Insert an `incident_contact` row referencing exactly one recipient. - * - * Exactly one of $contactId, $contactgroupId or $scheduleId is expected to be set; the others stay - * null, mirroring the polymorphic recipient key the daemon writes. - */ - private function seedIncidentContact( - int $incidentId, - ?int $contactId, - string $role, - ?int $contactgroupId = null, - ?int $scheduleId = null, - int $changedAt = self::SEEDED_CHANGED_AT - ): void { - $this->db->insert( - 'incident_contact', - [ - 'incident_id' => $incidentId, - 'contact_id' => $contactId, - 'contactgroup_id' => $contactgroupId, - 'schedule_id' => $scheduleId, - 'role' => $role, - 'changed_at' => $changedAt - ] - ); - } - /** * Wrap the seeded incident in the integration object under test. * @@ -547,25 +789,19 @@ private function incident(int $id): Incident ->filter(Filter::equal('id', $id)) ->first(); - return new Incident($model, $this->db); + return Incident::fromModel($model, $this->db); } /** - * Collect the given recipients into a list ordered by type and name. + * Wrap the incident with the given id in an instance that resolves it lazily. * - * The readers do not guarantee an order, so tests asserting more than one recipient normalise it here - * instead of relying on the database's row order. + * The incident does not have to exist, which is how the tests reach the missing incident cases. * - * @param iterable $recipients - * - * @return list> + * @param int $id */ - private function sortedByTypeAndName(iterable $recipients): array + private function incidentFromQuery(int $id): Incident { - $list = iterator_to_array($recipients, false); - usort($list, fn(array $a, array $b): int => [$a['type'] ?? '', $a['name']] <=> [$b['type'] ?? '', $b['name']]); - - return $list; + return Incident::fromQuery(IncidentModel::on($this->db)->filter(Filter::equal('id', $id))); } /** @@ -620,4 +856,116 @@ private function storedRoleHistory(): array . ' WHERE h.type = \'recipient_role_changed\' ORDER BY h.id' )->fetchAll(PDO::FETCH_ASSOC); } + + /** + * Insert an open incident and return its generated id + * + * @param ?string $muteReason The reason the incident is muted, null leaves it unmuted + */ + private function seedIncident(?string $muteReason = null): int + { + $this->db->insert('incident', [ + 'object_id' => self::objectId($this->db), + 'severity' => 'crit', + 'started_at' => (int) (new DateTime())->format('Uv'), + 'mute_reason' => $muteReason + ]); + + return (int) $this->db->lastInsertId(); + } + + /** + * Insert a contact with the given username and return its generated id + * + * The full name is derived from the username (e.g. "Alice Example" for "alice"), so the readers' + * name/username pairing can be asserted unambiguously. + */ + private function seedContact(string $username, bool $deleted = false): int + { + $this->db->insert('contact', [ + 'external_uuid' => static::transformUUIDForDB( + $this->db, + sprintf('00000000-0000-0000-0000-%012x', crc32($username)) + ), + 'full_name' => ucfirst($username) . ' Example', + 'username' => $username, + 'default_channel_id' => self::CHANNEL_ID, + 'changed_at' => (int) (new DateTime())->format('Uv'), + 'deleted' => $deleted ? 'y' : 'n' + ]); + + return (int) $this->db->lastInsertId(); + } + + /** + * Insert a contact group with the given name and return its generated id + */ + private function seedContactgroup(string $name, bool $deleted = false): int + { + $this->db->insert('contactgroup', [ + 'external_uuid' => static::transformUUIDForDB( + $this->db, + sprintf('00000000-0000-0000-0001-%012x', crc32($name)) + ), + 'name' => $name, + 'changed_at' => (int) (new DateTime())->format('Uv'), + 'deleted' => $deleted ? 'y' : 'n' + ]); + + return (int) $this->db->lastInsertId(); + } + + /** + * Insert a schedule with the given name and return its generated id + */ + private function seedSchedule(string $name, bool $deleted = false): int + { + $this->db->insert('schedule', [ + 'name' => $name, + 'timezone' => 'Europe/Berlin', + 'changed_at' => (int) (new DateTime())->format('Uv'), + 'deleted' => $deleted ? 'y' : 'n' + ]); + + return (int) $this->db->lastInsertId(); + } + + /** + * Insert an `incident_contact` row referencing exactly one recipient + * + * Exactly one of $contactId, $contactgroupId or $scheduleId is expected to be set; the others stay + * null, mirroring the polymorphic recipient key the daemon writes. The database enforces this. + * + * @param string $role One of `recipient`, `subscriber` or `manager` + */ + private function seedIncidentContact( + int $incidentId, + ?int $contactId, + string $role, + ?int $contactgroupId = null, + ?int $scheduleId = null + ): void { + $this->db->insert('incident_contact', [ + 'incident_id' => $incidentId, + 'contact_id' => $contactId, + 'contactgroup_id' => $contactgroupId, + 'schedule_id' => $scheduleId, + 'role' => $role, + 'changed_at' => self::ROLE_CHANGED_AT + ]); + } + + /** + * Get the id of the object every incident belongs to + * + * These tests don't care about the object, they only require one to exist, hence its fixed id. It is + * returned in the representation the current database expects for a binary literal, as the tests seed + * the tables directly, i.e. without the ORM's Binary behavior in between. + */ + private static function objectId(Connection $db): string + { + $id = str_repeat('7e', 32); // The column requires a SHA256, i.e. exactly 32 bytes + + return $db->getAdapter() instanceof Pgsql ? "\\x$id" : hex2bin($id); + } } diff --git a/test/php/library/Notifications/Integrations/IncidentsTest.php b/test/php/library/Notifications/Integrations/IncidentsTest.php index e75956017..0c5ecda90 100644 --- a/test/php/library/Notifications/Integrations/IncidentsTest.php +++ b/test/php/library/Notifications/Integrations/IncidentsTest.php @@ -10,15 +10,18 @@ use Icinga\Module\Notifications\Integrations\Incidents; use Icinga\Module\Notifications\Model\Incident as IncidentModel; use Icinga\Module\Notifications\Test\DbTestBackends; -use Icinga\User; +use InvalidArgumentException; use ipl\Orm\Query; use ipl\Sql\Adapter\Pgsql; use ipl\Sql\Connection; use ipl\Sql\Test\SharedDatabases\TransactionIsolation; use ipl\Sql\Test\TestConnection; +use ipl\Stdlib\Filter\All; +use ipl\Stdlib\Filter\Any; use ipl\Stdlib\Filter\Chain; use ipl\Stdlib\Filter\Condition; use ipl\Stdlib\Filter\Equal; +use ipl\Stdlib\Filter\Rule; use ipl\Stdlib\Filter\Unlike; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -30,10 +33,7 @@ * * The tests that execute a query run against real databases — once for MySQL and once for PostgreSQL (see * {@see DbTestBackends} / `#[DataProvider('sharedDatabases')]`). Each of them runs inside its own transaction which is - * rolled back afterwards, so the incidents, objects it seeds don't leak into the next test. The - * prerequisite channel a contact requires is seeded once per driver in {@see self::initializeNotificationsDb()} and - * its id captured into {@see self::$channelId} (the id can't be assumed as rolled-back transactions still advance - * the auto-increment). + * rolled back afterwards, so the incidents and objects it seeds don't leak into the next test. * * The tests that only assert on the filter {@see Incidents} constructs don't need a database at all and use a * {@see TestConnection} instead. @@ -43,52 +43,28 @@ class IncidentsTest extends TestCase { use DbTestBackends; - /** @var int Id of the channel seeded per driver (auto-increment drifts across rolled-back transactions) */ - private static int $channelId; - /** * Object ids seeded into the current test's database, keyed by object id * - * @var array> + * @var array */ private array $seededObjects = []; /** @var Connection The database of the current test, set by every test using the shared databases */ private Connection $db; - protected static function initializeNotificationsDb(Connection $db): void - { - $db->insert('available_channel_type', [ - 'type' => 'email', 'name' => 'Email', 'version' => '1', 'author' => 'Test', 'config_attrs' => '' - ]); - $db->insert('channel', [ - 'external_uuid' => static::transformUUIDForDB($db, '00000000-0000-0000-0000-0000000000c1'), - 'name' => 'Test', - 'type' => 'email', - 'changed_at' => (int) (new DateTime())->format('Uv') - ]); - - self::$channelId = (int) $db->lastInsertId(); - } - /** - * Reset the set of seeded object ids so each test starts with a fresh, empty database + * Nothing has to exist before a test's transaction starts, every test seeds what it requires itself */ - protected function setUp(): void + protected static function initializeNotificationsDb(Connection $db): void { - $this->seededObjects = []; - - // The trait's own setUp() is not used as this class defines one, so the previous test's transaction - // has to be rolled back here - $this->rollbackChanges(); } public function testBuildsAnEqualFilterForEachTagWithAValue(): void { $conditions = $this->conditions($this->builtFilter(['host' => 'icinga2', 'service' => 'http'])); - $this->assertCount(3, $conditions); - $this->assertContains([Unlike::class, 'recovered_at', '*'], $conditions); + $this->assertCount(2, $conditions); $this->assertContains([Equal::class, 'incident.object.tag.host', 'icinga2'], $conditions); $this->assertContains([Equal::class, 'incident.object.tag.service', 'http'], $conditions); } @@ -97,124 +73,54 @@ public function testBuildsAnAbsenceFilterForTagsGivenAsNull(): void { $conditions = $this->conditions($this->builtFilter(['host' => 'icinga2', 'service' => null])); - $this->assertCount(3, $conditions); + $this->assertCount(2, $conditions); $this->assertContains([Equal::class, 'incident.object.tag.host', 'icinga2'], $conditions); // A null value requires the tag's absence, expressed as "no value matches the wildcard". $this->assertContains([Unlike::class, 'incident.object.tag.service', '*'], $conditions); } - public function testAlwaysFiltersOutRecoveredIncidents(): void - { - // Even without any tags the query is constrained to open incidents (recovered_at IS NULL). - $this->assertSame([[Unlike::class, 'recovered_at', '*']], $this->conditions($this->builtFilter([]))); - } - - #[DataProvider('sharedDatabases')] - public function testGetIterator(Connection $db): void - { - $this->db = $db; - - $this->seedIncident(['host' => 'a']); - $this->seedIncident(['host' => 'b']); - - $incidents = iterator_to_array(new Incidents([], $db), false); - - $this->assertCount(2, $incidents); - foreach ($incidents as $incident) { - $this->assertInstanceOf(Incident::class, $incident); - } - } - - #[DataProvider('sharedDatabases')] - public function testHasIncident(Connection $db): void - { - $this->db = $db; - - $this->assertFalse((new Incidents([], $db))->hasIncident()); - - $this->seedIncident(['host' => 'a']); - - $this->assertTrue((new Incidents([], $db))->hasIncident()); - } - - #[DataProvider('sharedDatabases')] - public function testGetRolesYieldsTheUsersRoleForEachIncidentTheyAreInvolvedIn(Connection $db): void + public function testObjectIdFor(): void { - $this->db = $db; - - $managed = $this->seedIncident(['host' => 'a']); - $subscribed = $this->seedIncident(['host' => 'b']); - $foreign = $this->seedIncident(['host' => 'c']); - - $jdoe = $this->seedContact('jdoe'); - $this->seedRole($managed, $jdoe, 'manager'); - $this->seedRole($subscribed, $jdoe, 'subscriber'); - $this->seedRole($foreign, $this->seedContact('jane'), 'recipient'); - - $roles = []; - foreach ((new Incidents([], $db))->getRoles(new User('jdoe')) as $incident => $role) { - $roles[$this->incidentId($incident)] = $role; - } - - ksort($roles); + $tags1 = ['foo' => 'bar', 'baz' => 'qux']; + $tags2 = ['baz' => 'qux', 'foo' => 'bar']; - $expected = [$managed => 'manager', $subscribed => 'subscriber']; - ksort($expected); + $id1 = self::invokeStatic('objectIdFor', $tags1); + $id2 = self::invokeStatic('objectIdFor', $tags2); - $this->assertSame($expected, $roles, 'getRoles() did not yield the user\'s role for each of their incidents'); + $this->assertSame($id1, $id2, 'The order of tags must not have an effect on the object id'); + // object id from the daemon's object_test.go + $this->assertSame('4bfe8b2596005172c9db4d2b4b400a12b478b87a793ed9577e9d2d165fd07e7a', $id1); } - #[DataProvider('sharedDatabases')] - public function testGetRolesIgnoresClosedIncidents(Connection $db): void + public function testRejectsAnEmptyTagSet(): void { - $this->db = $db; - - $recovered = $this->seedIncident(['host' => 'a'], 1_700_000_000_000); - $this->seedRole($recovered, $this->seedContact('jdoe'), 'manager'); - - $this->assertSame([], iterator_to_array((new Incidents([], $db))->getRoles(new User('jdoe')))); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('A set of id tags must not be empty'); + $this->conditions($this->builtFilter([])); } - #[DataProvider('sharedDatabases')] - public function testGetRolesYieldsNothingForAUserWithoutIncidents(Connection $db): void + public function testRejectsAnEmptySet(): void { - $this->db = $db; + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('At least one set of id tags is required'); - $this->seedRole($this->seedIncident(['host' => 'a']), $this->seedContact('jane'), 'manager'); - - $this->assertSame([], iterator_to_array((new Incidents([], $db))->getRoles(new User('jdoe')))); + $this->query(new TestConnection(), []); } #[DataProvider('sharedDatabases')] - public function testGetRolesYieldsOnlyTheRoleForTheGivenUser(Connection $db): void - { - $this->db = $db; - - $incidentId = $this->seedIncident(['host' => 'a']); - $this->seedRole($incidentId, $this->seedContact('jane'), 'manager'); - $this->seedRole($incidentId, $this->seedContact('jdoe'), 'subscriber'); - $this->seedRole($incidentId, $this->seedContact('bob'), 'recipient'); - - $roles = []; - foreach ((new Incidents(['host' => 'a'], $db))->getRoles(new User('jdoe')) as $incident => $role) { - $roles[] = [$this->incidentId($incident), $role]; - } - - $this->assertSame([[$incidentId, 'subscriber']], $roles); - } - - #[DataProvider('sharedDatabases')] - public function testIteratingAfterHasIncidentYieldsAllMatchesFromTheSameInstance(Connection $db): void + public function testGetIterator(Connection $db): void { $this->db = $db; $this->seedIncident(['host' => 'a']); $this->seedIncident(['host' => 'b']); - $incidents = new Incidents([], $db); + $incidents = iterator_to_array($this->incidents($db, [['host' => 'a'], ['host' => 'b']]), false); - $this->assertTrue($incidents->hasIncident()); - $this->assertCount(2, iterator_to_array($incidents, false)); + $this->assertCount(2, $incidents); + foreach ($incidents as $incident) { + $this->assertInstanceOf(Incident::class, $incident); + } } #[DataProvider('sharedDatabases')] @@ -224,7 +130,7 @@ public function testCount(Connection $db): void $this->seedIncident(['host' => 'a']); - $this->assertEquals(1, (new Incidents([], $db))->count()); + $this->assertEquals(1, $this->incidents($db, [['host' => 'a']])->count()); } #[DataProvider('sharedDatabases')] @@ -237,7 +143,7 @@ public function testExcludesClosedIncidents(Connection $db): void $this->seedIncident(['host' => 'a']); $this->seedIncident(['host' => 'b'], 1_700_000_000_000); - $incidents = new Incidents([], $db); + $incidents = $this->incidents($db, [['host' => 'a'], ['host' => 'b']]); $this->assertEquals(1, $incidents->count()); $this->assertCount(1, iterator_to_array($incidents, false)); @@ -255,25 +161,77 @@ public function testMatchesTheIncidentsOfObjectsCarryingAllGivenTags(Connection // The host's incident and the one of its service, matched by the tag they have in common $this->assertSame( [$host, $service], - $this->incidentIds(new Incidents(['host' => 'a'], $db)), + $this->incidentIds($this->incidents($db, [['host' => 'a']])), 'Not all incidents of the objects carrying the given tag were matched' ); // Only the host's incident, as a null value requires the tag's absence $this->assertSame( [$host], - $this->incidentIds(new Incidents(['host' => 'a', 'service' => null], $db)), + $this->incidentIds($this->incidents($db, [['host' => 'a', 'service' => null]])), 'A tag given as null did not exclude the objects carrying it' ); // Only the service's incident, as its object is the only one carrying both tags $this->assertSame( [$service], - $this->incidentIds(new Incidents(['host' => 'a', 'service' => 'http'], $db)), + $this->incidentIds($this->incidents($db, [['host' => 'a', 'service' => 'http']])), 'Not only the incidents of the object carrying all given tags were matched' ); } + #[DataProvider('sharedDatabases')] + public function testMatchesTheIncidentOfTheObjectCarryingExactlyTheGivenTags(Connection $db): void + { + $this->db = $db; + + $host = $this->seedIncident(['host' => 'a']); + $service = $this->seedIncident(['host' => 'a', 'service' => 'http']); + + $this->assertSame( + [$host], + $this->incidentIds($this->incidents($db, [['host' => 'a']], true)), + "The incident of the object carrying exactly the host's tags was not matched" + ); + + $this->assertSame( + [$service], + $this->incidentIds($this->incidents($db, [['host' => 'a', 'service' => 'http']], true)), + "The incident of the object carrying exactly the service's tags was not matched" + ); + } + + #[DataProvider('sharedDatabases')] + public function testMatchesNoIncidentOfObjectsCarryingTagsBesidesTheGivenOnes(Connection $db): void + { + $this->db = $db; + + $this->seedIncident(['host' => 'a', 'service' => 'http']); + + $this->assertSame( + [], + $this->incidentIds($this->incidents($db, [['host' => 'a']], true)), + 'The incident of an object carrying tags besides the given ones was matched' + ); + } + + #[DataProvider('sharedDatabases')] + public function testMatchesTheIncidentsOfObjectsSatisfyingAnyGivenTagSet(Connection $db): void + { + $this->db = $db; + + $host = $this->seedIncident(['host' => 'a']); + $service = $this->seedIncident(['host' => 'b', 'service' => 'http']); + $this->seedIncident(['host' => 'c']); + + // An object only has to satisfy any of the tag sets, each of which identifies it completely + $this->assertSame( + [$host, $service], + $this->incidentIds($this->incidents($db, [['host' => 'a'], ['host' => 'b', 'service' => 'http']], true)), + 'Not the incidents of the objects satisfying any of the given tag sets were matched' + ); + } + /** * Build the query {@see Incidents} would run for the given tags and return its filter * @@ -285,30 +243,97 @@ public function testMatchesTheIncidentsOfObjectsCarryingAllGivenTags(Connection */ private function builtFilter(array $tags): Chain { - $incidents = new Incidents($tags, new TestConnection()); + return $this->query(new TestConnection(), [$tags])->getFilter(); + } + + /** + * Create an instance matching the incidents of the given tag sets, reading through the given database + * + * @param iterable> $tagSets + * @param bool $exactMatches Whether an object must not have tags besides those given in a set, + * as {@see Incidents::getAll()} requires it + */ + private function incidents(Connection $db, iterable $tagSets, bool $exactMatches = false): Incidents + { + return new Incidents($this->query($db, $tagSets, $exactMatches)); + } - /** @var Query $query */ - $query = (new ReflectionMethod($incidents, 'buildQuery'))->invoke($incidents); + /** + * Assemble the query the factories of {@see Incidents} would pass to its constructor + * + * The constructor only accepts a query, which the factories build with the class' own helpers while + * reading through the singleton. The tests use those helpers as well, as what they assemble is what + * the assertions are about, but pass the database explicitly. + * + * @param iterable> $tagSets + * @param bool $exactMatches Whether an object must not have tags besides those given in a set + * + * @return Query + */ + private function query(Connection $db, iterable $tagSets, bool $exactMatches = false): Query + { + return self::invokeStatic('openIncidents', $db)->filter(self::invokeStatic( + $exactMatches ? 'tagSetFilterExactMatches' : 'tagSetFilterPartialMatches', + $tagSets + )); + } - return $query->getFilter(); + /** + * Call the given private static method of {@see Incidents} with the given arguments + */ + private static function invokeStatic(string $method, mixed ...$args): mixed + { + return (new ReflectionMethod(Incidents::class, $method))->invoke(null, ...$args); } /** - * Reduce a filter chain to a list of [operator class, column, value] triples for order-independent assertions + * Get the conditions of the tag set filters of the given query filter + * + * Asserts that the filter is an `All` of the recovered_at condition and an `Any` with one `All` per tag set, + * then returns the conditions of those sets as [operator class, column, value] triples for order-independent + * assertions. The recovered_at condition is not among them, it belongs to no tag set and is asserted here. + * + * @param int $tagSets The number of tag sets the filter is expected to consist of * * @return list, mixed}> */ - private function conditions(Chain $chain): array + private function conditions(Chain $filter, int $tagSets = 1): array { + $rules = iterator_to_array($filter, false); + + $this->assertInstanceOf(All::class, $filter); + $this->assertCount(2, $rules, 'The filter is not an All of the recovered_at condition and the tag sets'); + $this->assertSame([Unlike::class, 'recovered_at', '*'], $this->triple($rules[0])); + $this->assertInstanceOf(Any::class, $rules[1], 'The tag sets are not combined with OR'); + + $tagSetFilters = iterator_to_array($rules[1], false); + + $this->assertCount($tagSets, $tagSetFilters, 'Not every tag set got its own filter'); + $conditions = []; - foreach ($chain as $rule) { - $this->assertInstanceOf(Condition::class, $rule); - $conditions[] = [$rule::class, $rule->getColumn(), $rule->getValue()]; + foreach ($tagSetFilters as $tagSetFilter) { + $this->assertInstanceOf(All::class, $tagSetFilter, "A tag set's conditions are not combined with AND"); + + foreach ($tagSetFilter as $rule) { + $conditions[] = $this->triple($rule); + } } return $conditions; } + /** + * Reduce the given rule to an [operator class, column, value] triple + * + * @return array{class-string, string|array, mixed} + */ + private function triple(Rule $rule): array + { + $this->assertInstanceOf(Condition::class, $rule); + + return [$rule::class, $rule->getColumn(), $rule->getValue()]; + } + /** * Get the ids of the incidents the given instance yields, sorted for order-independent assertions * @@ -340,8 +365,9 @@ private function incidentId(Incident $incident): int } /** - * Insert an incident for the object identified by the given tags, creating the - * object and object_id_tag rows it relates to so the joins in {@see Incidents::buildQuery()} resolve. + * Insert an incident for the object identified by the given tags, creating the object and + * object_id_tag rows it relates to so the incident's reference and the joins in the query of + * {@see self::query()} resolve. * * @param array $tags * @param ?int $recoveredAt Recovery time in milliseconds; null leaves the incident open @@ -367,45 +393,10 @@ private function seedIncident(array $tags, ?int $recoveredAt = null): int } /** - * Insert a contact with the given username and return its id - */ - private function seedContact(string $username): int - { - $this->db->insert('contact', [ - 'external_uuid' => static::transformUUIDForDB( - $this->db, - sprintf('00000000-0000-0000-0000-%012x', crc32($username)), - ), - 'full_name' => $username, - 'username' => $username, - 'default_channel_id' => self::$channelId, - 'changed_at' => (int) (new DateTime())->format('Uv') - ]); - - return (int) $this->db->lastInsertId(); - } - - /** - * Make the given contact a recipient, subscriber or manager of the given incident + * Get the id of the object identified by the given tags * - * @param string $role One of `recipient`, `subscriber` or `manager` - */ - private function seedRole(int $incidentId, int $contactId, string $role): void - { - $this->db->insert('incident_contact', [ - 'incident_id' => $incidentId, - 'contact_id' => $contactId, - 'role' => $role, - 'changed_at' => (int) (new DateTime())->format('Uv') - ]); - } - - /** - * Derive a stable, unique object id for the given tags - * - * Stands in for the daemon's object hashing: the exact algorithm is irrelevant to these tests, it - * only has to be deterministic and collision-free for distinct tags so - * the seeded incident and object_id_tag rows share one id while distinct objects differ. + * The id is derived exactly the way {@see Incidents} derives the ids it looks incidents up by, as + * only then do the seeded rows match what the queries under test search for. * * The id is returned in the representation the current database expects for a binary literal, as * the tests seed the tables directly, i.e. without the ORM's Binary behavior in between. @@ -414,15 +405,13 @@ private function seedRole(int $incidentId, int $contactId, string $role): void */ private function objectId(array $tags): string { - ksort($tags); - - $hash = hash('sha256', serialize($tags)); + $id = self::invokeStatic('objectIdFor', $tags); if ($this->db->getAdapter() instanceof Pgsql) { - return sprintf('\\x%s', $hash); + return sprintf('\\x%s', $id); } - return hex2bin($hash); + return hex2bin($id); } /** @@ -440,8 +429,8 @@ private function seedObject(array $tags, string $objectId): void $this->seededObjects[$objectId] = true; $this->db->insert('object', [ - 'id' => $objectId, - 'name' => implode(', ', $tags) + 'id' => $objectId, + 'name' => implode(', ', $tags) ]); foreach ($tags as $tag => $value) {