Skip to content

Commit bade5be

Browse files
loks0nclaude
loks0n
and
claude
committed
test: migrate to constructor config + per-row/per-call tenant; fix daily purge
- ClickHouse tests construct with namespace/sharedTables and use a ScopedClickHouse test helper that supplies a default tenant to writes (per-row) and reads (per-call), so the existing call sites exercise the real tenant code paths without threading tenant through each one. - Rewrote the per-row tenant-override test to use the new API directly (addBatch with $tenant on the row, find/purge with an explicit tenant). - Reflection helpers target the parent ClickHouse class so they resolve private members through the test subclass. Also fixes a purge regression surfaced by the migration: with tenant now an explicit query filter, purgeDaily treated a tenant scope as a daily-safe narrowing filter, so a path-only purge fell through to `DELETE FROM daily WHERE tenant=X` and wiped the tenant's unrelated daily rows. Tenant is now excluded from the compatibility / no-op decision and re-attached only to the final scoped delete. 246 tests green, PHPStan max + Pint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 554c12d commit bade5be

5 files changed

Lines changed: 165 additions & 77 deletions

File tree

‎src/Usage/Adapter/ClickHouse.php‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4122,12 +4122,22 @@ private function purgeDaily(array $queries): void
41224122
fn (string $col): bool => $col !== 'value'
41234123
));
41244124
$safeAttributes = array_merge(['time'], $safeAttributes);
4125-
if ($this->sharedTables) {
4126-
$safeAttributes[] = 'tenant';
4125+
4126+
// Tenant scopes the delete but never narrows it: a tenant-only daily
4127+
// delete wipes every metric for that tenant. Keep it out of the
4128+
// compatibility / no-op decision and re-attach it to the final query.
4129+
$tenantQueries = [];
4130+
$narrowingQueries = [];
4131+
foreach ($queries as $query) {
4132+
if ($query->getAttribute() === 'tenant') {
4133+
$tenantQueries[] = $query;
4134+
} else {
4135+
$narrowingQueries[] = $query;
4136+
}
41274137
}
41284138

41294139
$compatible = true;
4130-
foreach ($queries as $query) {
4140+
foreach ($narrowingQueries as $query) {
41314141
$attr = $query->getAttribute();
41324142
if ($attr === '') {
41334143
continue;
@@ -4138,14 +4148,18 @@ private function purgeDaily(array $queries): void
41384148
}
41394149
}
41404150

4141-
$dailyQueries = $compatible
4142-
? $this->translateTimeQueriesToDayBoundaries($queries)
4143-
: $this->buildStaleRollupPurgeQueries($queries, $safeAttributes);
4151+
$dailyNarrowing = $compatible
4152+
? $this->translateTimeQueriesToDayBoundaries($narrowingQueries)
4153+
: $this->buildStaleRollupPurgeQueries($narrowingQueries, $safeAttributes);
41444154

4145-
if (! empty($queries) && empty($dailyQueries)) {
4155+
// A non-empty caller filter that leaves no daily-expressible narrowing
4156+
// predicate must not fall through to a tenant-only (or unbounded) wipe.
4157+
if (! empty($narrowingQueries) && empty($dailyNarrowing)) {
41464158
return;
41474159
}
41484160

4161+
$dailyQueries = array_merge($dailyNarrowing, $tenantQueries);
4162+
41494163
$dailyTable = $this->buildTableReference($this->getEventsDailyTableName());
41504164

41514165
$parsed = $this->parseQueries($dailyQueries, Usage::TYPE_EVENT);

‎tests/Benchmark/BenchmarkBase.php‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use ReflectionClass;
77
use RuntimeException;
88
use Throwable;
9+
use Utopia\Tests\Usage\Adapter\ScopedClickHouse;
910
use Utopia\Usage\Adapter\ClickHouse as ClickHouseAdapter;
1011
use Utopia\Usage\Usage;
1112

@@ -49,20 +50,7 @@ abstract class BenchmarkBase extends TestCase
4950

5051
protected function setUp(): void
5152
{
52-
$host = getenv('CLICKHOUSE_HOST') ?: 'clickhouse';
53-
$username = getenv('CLICKHOUSE_USER') ?: 'default';
54-
$password = getenv('CLICKHOUSE_PASSWORD') ?: 'clickhouse';
55-
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
56-
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
57-
58-
$this->adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
59-
$this->adapter->setNamespace($this->namespace);
60-
$this->adapter->setSharedTables(true);
61-
$this->adapter->setTenant($this->tenant);
62-
63-
if ($database = getenv('CLICKHOUSE_DATABASE')) {
64-
$this->adapter->setDatabase($database);
65-
}
53+
$this->adapter = ScopedClickHouse::fromEnv($this->namespace, true, $this->tenant);
6654

6755
$this->usage = new Usage($this->adapter);
6856
$this->usage->setup();
@@ -89,7 +77,7 @@ protected function seedEventRows(int $rows, ?string $metric = null): void
8977
{
9078
$metric ??= $this->metric;
9179

92-
$reflection = new ReflectionClass($this->adapter);
80+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
9381

9482
$getEvents = $reflection->getMethod('getEventsTableName');
9583
$getEvents->setAccessible(true);
@@ -123,7 +111,7 @@ protected function seedEventRows(int $rows, ?string $metric = null): void
123111
*/
124112
protected function seedGaugeRows(int $rows, string $metric = 'storage'): void
125113
{
126-
$reflection = new ReflectionClass($this->adapter);
114+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
127115

128116
$getGauges = $reflection->getMethod('getGaugesTableName');
129117
$getGauges->setAccessible(true);
@@ -157,7 +145,7 @@ protected function seedGaugeRows(int $rows, string $metric = 'storage'): void
157145
*/
158146
protected function runRawSql(string $sql): void
159147
{
160-
$reflection = new ReflectionClass($this->adapter);
148+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
161149
$query = $reflection->getMethod('query');
162150
$query->setAccessible(true);
163151
$query->invoke($this->adapter, $sql, []);
@@ -247,7 +235,7 @@ protected function captureCHStats(string $queryId): array
247235
. "FORMAT JSON";
248236

249237
try {
250-
$reflection = new ReflectionClass($this->adapter);
238+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
251239
$query = $reflection->getMethod('query');
252240
$query->setAccessible(true);
253241
$raw = $query->invoke($this->adapter, $sql, []);
@@ -286,7 +274,7 @@ protected function captureProjectionsUsed(string $queryId): array
286274
. "ORDER BY event_time DESC LIMIT 1 FORMAT JSON";
287275

288276
try {
289-
$reflection = new ReflectionClass($this->adapter);
277+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
290278
$query = $reflection->getMethod('query');
291279
$query->setAccessible(true);
292280
$raw = $query->invoke($this->adapter, $sql, []);

‎tests/Usage/Adapter/ClickHouseTest.php‎

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ protected function initializeUsage(): void
2222
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
2323

2424

25-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
26-
$adapter->setNamespace('utopia_usage');
27-
$adapter->setTenant('1');
25+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage');
2826

2927
// Optional customization via env vars
3028
if ($database = getenv('CLICKHOUSE_DATABASE')) {
@@ -43,18 +41,15 @@ public function testMetricTenantOverridesAdapterTenantInBatch(): void
4341
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
4442
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
4543

46-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
47-
$adapter->setNamespace('utopia_usage_shared');
48-
$adapter->setSharedTables(true);
49-
$adapter->setTenant('1');
44+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage_shared', true);
5045

5146
if ($database = getenv('CLICKHOUSE_DATABASE')) {
5247
$adapter->setDatabase($database);
5348
}
5449

5550
$usage = new Usage($adapter);
5651
$usage->setup();
57-
$usage->purge();
52+
$usage->purge(tenant: '2');
5853

5954
$metrics = [
6055
[
@@ -67,17 +62,15 @@ public function testMetricTenantOverridesAdapterTenantInBatch(): void
6762

6863
$this->assertTrue($usage->addBatch($metrics, Usage::TYPE_EVENT));
6964

70-
// Switch adapter scope to the metric tenant to verify the row was stored under the override
71-
$adapter->setTenant('2');
72-
65+
// Read scoped to the metric's tenant to verify the row was stored under the per-row override
7366
$results = $usage->find([
7467
\Utopia\Query\Query::equal('metric', ['tenant-override']),
75-
], Usage::TYPE_EVENT);
68+
], Usage::TYPE_EVENT, tenant: '2');
7669

7770
$this->assertCount(1, $results);
7871
$this->assertEquals('2', $results[0]->getTenant());
7972

80-
$usage->purge();
73+
$usage->purge(tenant: '2');
8174
}
8275

8376
/**
@@ -834,9 +827,7 @@ public function testCompression(): void
834827
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
835828
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
836829

837-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
838-
$adapter->setNamespace('utopia_usage_compression_test');
839-
$adapter->setTenant('1');
830+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage_compression_test');
840831

841832
if ($database = getenv('CLICKHOUSE_DATABASE')) {
842833
$adapter->setDatabase($database);
@@ -890,9 +881,7 @@ public function testConnectionPooling(): void
890881
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
891882
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
892883

893-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
894-
$adapter->setNamespace('utopia_usage_pooling_test');
895-
$adapter->setTenant('1');
884+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage_pooling_test');
896885

897886
if ($database = getenv('CLICKHOUSE_DATABASE')) {
898887
$adapter->setDatabase($database);
@@ -1021,9 +1010,7 @@ public function testRetryWithSuccessfulOperations(): void
10211010
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
10221011
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
10231012

1024-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
1025-
$adapter->setNamespace('utopia_usage_retry_test');
1026-
$adapter->setTenant('1');
1013+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage_retry_test');
10271014
$adapter->setMaxRetries(2);
10281015
$adapter->setRetryDelay(50);
10291016

@@ -1055,10 +1042,8 @@ public function testErrorMessagesIncludeContext(): void
10551042
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
10561043
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
10571044

1058-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
1059-
$adapter->setNamespace('utopia_usage_error_test');
1045+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage_error_test');
10601046
$adapter->setDatabase('nonexistent_db_for_testing_errors_12345');
1061-
$adapter->setTenant('1');
10621047
$adapter->setMaxRetries(0); // Disable retries for faster test
10631048

10641049
$usage = new Usage($adapter);
@@ -1089,9 +1074,7 @@ public function testAsyncInsertConfiguration(): void
10891074
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
10901075
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
10911076

1092-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
1093-
$adapter->setNamespace('utopia_usage_async');
1094-
$adapter->setTenant('1');
1077+
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure, 'utopia_usage_async');
10951078

10961079
if ($database = getenv('CLICKHOUSE_DATABASE')) {
10971080
$adapter->setDatabase($database);

‎tests/Usage/Adapter/ClickHouseTestCase.php‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,50 +21,35 @@ abstract class ClickHouseTestCase extends TestCase
2121
*/
2222
protected function makeAdapter(string $namespace, ?string $tenant = '1'): ClickHouseAdapter
2323
{
24-
$host = getenv('CLICKHOUSE_HOST') ?: 'clickhouse';
25-
$username = getenv('CLICKHOUSE_USER') ?: 'default';
26-
$password = getenv('CLICKHOUSE_PASSWORD') ?: 'clickhouse';
27-
$port = (int) (getenv('CLICKHOUSE_PORT') ?: 8123);
28-
$secure = (bool) (getenv('CLICKHOUSE_SECURE') ?: false);
29-
30-
$adapter = new ClickHouseAdapter($host, $username, $password, $port, $secure);
31-
$adapter->setNamespace($namespace);
32-
$adapter->setSharedTables(true);
33-
if ($tenant !== null) {
34-
$adapter->setTenant($tenant);
35-
}
36-
37-
if ($database = getenv('CLICKHOUSE_DATABASE')) {
38-
$adapter->setDatabase($database);
39-
}
40-
41-
return $adapter;
24+
return ScopedClickHouse::fromEnv($namespace, true, $tenant);
4225
}
4326

4427
/**
4528
* Read the adapter's `database` property (private).
4629
*/
4730
protected function databaseName(ClickHouseAdapter $adapter): string
4831
{
49-
$reflection = new ReflectionClass($adapter);
32+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
5033
$prop = $reflection->getProperty('database');
5134
$prop->setAccessible(true);
5235
$value = $prop->getValue($adapter);
36+
5337
return is_string($value) ? $value : '';
5438
}
5539

5640
/**
5741
* Invoke a private accessor on the adapter that returns a table name
5842
* (e.g. getEventsTableName, getDimRollupTableName).
5943
*
60-
* @param array<int, mixed> $args
44+
* @param array<int, mixed> $args
6145
*/
6246
protected function resolveTableName(ClickHouseAdapter $adapter, string $accessor, array $args = []): string
6347
{
64-
$reflection = new ReflectionClass($adapter);
48+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
6549
$method = $reflection->getMethod($accessor);
6650
$method->setAccessible(true);
6751
$raw = $method->invokeArgs($adapter, $args);
52+
6853
return is_string($raw) ? $raw : '';
6954
}
7055

@@ -73,14 +58,15 @@ protected function resolveTableName(ClickHouseAdapter $adapter, string $accessor
7358
* API does internally; needed in tests for setup / cleanup statements
7459
* that don't fit the find / sum / purge contracts.
7560
*
76-
* @param array<string, mixed> $params
61+
* @param array<string, mixed> $params
7762
*/
7863
protected function queryRaw(ClickHouseAdapter $adapter, string $sql, array $params = []): string
7964
{
80-
$reflection = new ReflectionClass($adapter);
65+
$reflection = new ReflectionClass(ClickHouseAdapter::class);
8166
$method = $reflection->getMethod('query');
8267
$method->setAccessible(true);
8368
$raw = $method->invoke($adapter, $sql, $params);
69+
8470
return is_string($raw) ? $raw : '';
8571
}
8672
}

0 commit comments

Comments
 (0)