From 0fc16987581538c38fe1bdeea53a505422493a5b Mon Sep 17 00:00:00 2001 From: Levi van Noort <73097785+levivannoort@users.noreply.github.com> Date: Wed, 8 Jul 2026 11:00:24 +0200 Subject: [PATCH 1/2] refactor(clickhouse): make retention a readonly constructor property Replace the getRetention/setRetention accessors with a promoted public readonly ?int constructor property. The positive-days check moves into the constructor body. Immutable-after-construction is coroutine-safe; the fluent setter is dropped. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Audit/Adapter/ClickHouse.php | 39 ++++----------------- tests/Audit/Adapter/ClickHouseTest.php | 47 +++++++++----------------- 2 files changed, 23 insertions(+), 63 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index 1524a5d..c48849c 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -84,15 +84,13 @@ class ClickHouse extends SQL protected bool $asyncCleanup = false; - /** @var int|null Retention in days; when set, setup() applies a TTL on the table. Null disables TTL. */ - private ?int $retention = null; - /** * @param string $host ClickHouse host * @param string $username ClickHouse username (default: 'default') * @param string $password ClickHouse password (default: '') * @param int $port ClickHouse HTTP port (default: 8123) * @param bool $secure Whether to use HTTPS (default: false) + * @param int|null $retention Retention window in days; when set, setup() applies a TTL so rows older than the window are dropped. Null disables TTL. * @throws Exception If validation fails */ public function __construct( @@ -100,11 +98,16 @@ public function __construct( string $username = 'default', string $password = '', int $port = self::DEFAULT_PORT, - bool $secure = false + bool $secure = false, + public readonly ?int $retention = null, ) { $this->validateHost($host); $this->validatePort($port); + if ($retention !== null && $retention < 1) { + throw new Exception('Retention must be a positive number of days'); + } + $this->host = $host; $this->port = $port; $this->username = $username; @@ -363,34 +366,6 @@ public function isAsyncCleanup(): bool return $this->asyncCleanup; } - /** - * Set the retention window in days. When set, setup() applies a TTL so - * rows older than the window are dropped by background merges. Pass null - * to disable (the default). - * - * @param int|null $days - * @return self - * @throws Exception If $days is not positive - */ - public function setRetention(?int $days): self - { - if ($days !== null && $days < 1) { - throw new Exception('Retention must be a positive number of days'); - } - $this->retention = $days; - return $this; - } - - /** - * Get the retention window in days, or null when TTL is disabled. - * - * @return int|null - */ - public function getRetention(): ?int - { - return $this->retention; - } - /** * Override getAttributes to provide extended attributes for ClickHouse. * Includes existing attributes from parent and adds new missing ones. diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index c3c6a40..2f220b1 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -377,71 +377,56 @@ public function testSetSecure(): void } /** - * Test setRetention stores the value and getRetention returns it + * Test retention defaults to null and is stored when passed to the constructor */ - public function testSetRetention(): void + public function testRetention(): void { $adapter = new ClickHouse( host: 'clickhouse', username: 'default', password: 'clickhouse' ); + $this->assertNull($adapter->retention); - $this->assertNull($adapter->getRetention()); - - $result = $adapter->setRetention(30); - $this->assertInstanceOf(ClickHouse::class, $result); - $this->assertEquals(30, $adapter->getRetention()); - } - - /** - * Test setRetention accepts null to disable retention - */ - public function testSetRetentionAcceptsNull(): void - { $adapter = new ClickHouse( host: 'clickhouse', username: 'default', - password: 'clickhouse' + password: 'clickhouse', + retention: 30 ); - - $adapter->setRetention(30); - $adapter->setRetention(null); - $this->assertNull($adapter->getRetention()); + $this->assertEquals(30, $adapter->retention); } /** - * Test setRetention rejects zero days + * Test retention rejects zero days */ - public function testSetRetentionRejectsZero(): void + public function testRetentionRejectsZero(): void { $this->expectException(Exception::class); $this->expectExceptionMessage('Retention must be a positive number of days'); - $adapter = new ClickHouse( + new ClickHouse( host: 'clickhouse', username: 'default', - password: 'clickhouse' + password: 'clickhouse', + retention: 0 ); - - $adapter->setRetention(0); } /** - * Test setRetention rejects negative days + * Test retention rejects negative days */ - public function testSetRetentionRejectsNegative(): void + public function testRetentionRejectsNegative(): void { $this->expectException(Exception::class); $this->expectExceptionMessage('Retention must be a positive number of days'); - $adapter = new ClickHouse( + new ClickHouse( host: 'clickhouse', username: 'default', - password: 'clickhouse' + password: 'clickhouse', + retention: -1 ); - - $adapter->setRetention(-1); } /** From 014555fe5e23932b2874ceca120701e0e4ecdd51 Mon Sep 17 00:00:00 2001 From: Levi van Noort <73097785+levivannoort@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:38:16 +0200 Subject: [PATCH 2/2] feat(clickhouse): make retention settable per table via setRetention() A single adapter instance runs setup() against several tables by mutating its namespace between calls (e.g. a shared `projects` table with a short window and a `console` table with a long one). A readonly, construction-time retention forced the same TTL onto every table. Drop `readonly`, add a fluent `setRetention()` so the window can be set per table before each setup(), and share validation via validateRetention(). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Audit/Adapter/ClickHouse.php | 35 ++++++++++++++++++++++---- tests/Audit/Adapter/ClickHouseTest.php | 22 ++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index c48849c..760c553 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -99,14 +99,11 @@ public function __construct( string $password = '', int $port = self::DEFAULT_PORT, bool $secure = false, - public readonly ?int $retention = null, + public ?int $retention = null, ) { $this->validateHost($host); $this->validatePort($port); - - if ($retention !== null && $retention < 1) { - throw new Exception('Retention must be a positive number of days'); - } + $this->validateRetention($retention); $this->host = $host; $this->port = $port; @@ -356,6 +353,34 @@ public function setAsyncCleanup(bool $asyncCleanup): self return $this; } + /** + * Set the retention window in days applied by setup() as a TTL, or null to + * disable it. Call before setup(); a single adapter can setup() several + * tables (differing namespaces), so retention is settable per table rather + * than fixed at construction. + * + * @param int|null $retention + * @return self + * @throws Exception If retention is not a positive number of days. + */ + public function setRetention(?int $retention): self + { + $this->validateRetention($retention); + $this->retention = $retention; + return $this; + } + + /** + * @param int|null $retention + * @throws Exception If retention is set but not a positive number of days. + */ + private function validateRetention(?int $retention): void + { + if ($retention !== null && $retention < 1) { + throw new Exception('Retention must be a positive number of days'); + } + } + /** * Get whether cleanup() runs asynchronously. * diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index 2f220b1..1efc86e 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -397,6 +397,28 @@ public function testRetention(): void $this->assertEquals(30, $adapter->retention); } + /** + * Test retention can be set and cleared per table via the fluent setter + */ + public function testSetRetention(): void + { + $adapter = new ClickHouse( + host: 'clickhouse', + username: 'default', + password: 'clickhouse' + ); + + $this->assertSame($adapter, $adapter->setRetention(30)); + $this->assertEquals(30, $adapter->retention); + + $adapter->setRetention(null); + $this->assertNull($adapter->retention); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Retention must be a positive number of days'); + $adapter->setRetention(0); + } + /** * Test retention rejects zero days */