Skip to content

Commit 3b32515

Browse files
lohanidamodarclaude
lohanidamodar
and
claude
committed
Address PR review: immutable Metric DTO, remove useFinal option, cleanup
- Refactor Metric class from mutable ArrayObject to immutable DTO using constructor property promotion with readonly properties (review #5) - Add Metric::fromArray() factory for constructing from associative arrays - Store extra dimensions (path, method, status, etc.) in extras map - Remove setUseFinal option; always use FINAL in ClickHouse queries (review #6) - Remove unused Metric import from ClickHouse adapter - Update all Metric tests to use new constructor and fromArray() factory Review items already addressed in prior commits: projectId->tenant (#1), region removal (#2), no purgeExpired on facade (#3), no HTTP log methods on Metric (#4), composer test script (#7). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d15407b commit 3b32515

4 files changed

Lines changed: 218 additions & 77 deletions

File tree

‎src/Usage/Adapter/ClickHouse.php‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use Utopia\Query\Query;
77
use Utopia\Fetch\Client;
88
use Utopia\Usage\Adapter;
9-
use Utopia\Usage\Metric;
109

1110
/**
1211
* ClickHouse Adapter
@@ -35,8 +34,6 @@ class ClickHouse extends Adapter
3534
private string $password;
3635
private bool $secure = false;
3736
private Client $client;
38-
private bool $useFinal = true;
39-
4037
protected int|string|null $tenant = null;
4138
protected bool $sharedTables = false;
4239
protected string $namespace = '';
@@ -72,12 +69,6 @@ public function getName(): string
7269
return 'ClickHouse';
7370
}
7471

75-
public function setUseFinal(bool $useFinal): self
76-
{
77-
$this->useFinal = $useFinal;
78-
return $this;
79-
}
80-
8172
public function setDatabase(string $database): self
8273
{
8374
$this->database = $database;
@@ -536,12 +527,12 @@ private function gaugesTable(): string
536527

537528
private function eventsTableRef(): string
538529
{
539-
return $this->eventsTable() . ($this->useFinal ? ' FINAL' : '');
530+
return $this->eventsTable() . ' FINAL';
540531
}
541532

542533
private function gaugesTableRef(): string
543534
{
544-
return $this->gaugesTable() . ($this->useFinal ? ' FINAL' : '');
535+
return $this->gaugesTable() . ' FINAL';
545536
}
546537

547538
// ─── Internal: Row builders ──────────────────────────────────────

‎src/Usage/Metric.php‎

Lines changed: 99 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,90 +2,152 @@
22

33
namespace Utopia\Usage;
44

5-
use ArrayObject;
6-
75
/**
8-
* Usage Metric (generic result row from event/gauge queries)
6+
* Usage Metric — immutable value object for event/gauge query results.
97
*
10-
* @extends ArrayObject<string, mixed>
8+
* Uses constructor property promotion with readonly properties for type safety.
9+
* Additional dimensions are stored in an extras map accessible via getAttribute().
1110
*/
12-
class Metric extends ArrayObject
11+
class Metric
1312
{
14-
public function __construct(array $input = [])
13+
/**
14+
* @param array<string, mixed> $extras Additional dimension values (path, method, status, userAgent, etc.)
15+
*/
16+
public function __construct(
17+
public readonly string $id = '',
18+
public readonly string $metric = '',
19+
public readonly int|float $value = 0,
20+
public readonly ?string $time = null,
21+
public readonly string $resource = '',
22+
public readonly string $resourceId = '',
23+
public readonly int|string|null $tenant = null,
24+
public readonly array $extras = [],
25+
) {
26+
}
27+
28+
/**
29+
* Create a Metric from an associative array (e.g. a ClickHouse row).
30+
*
31+
* @param array<string, mixed> $data
32+
*/
33+
public static function fromArray(array $data): self
1534
{
16-
parent::__construct($input);
35+
$id = (string) ($data['$id'] ?? $data['id'] ?? '');
36+
$metric = (string) ($data['metric'] ?? '');
37+
38+
$rawValue = $data['value'] ?? 0;
39+
if (is_int($rawValue) || is_float($rawValue)) {
40+
$value = $rawValue;
41+
} elseif (is_numeric($rawValue)) {
42+
$value = $rawValue + 0;
43+
} else {
44+
$value = 0;
45+
}
46+
47+
$time = isset($data['time']) && is_string($data['time']) ? $data['time'] : null;
48+
$resource = (string) ($data['resource'] ?? '');
49+
$resourceId = (string) ($data['resourceId'] ?? '');
50+
51+
$rawTenant = $data['tenant'] ?? null;
52+
if ($rawTenant === null) {
53+
$tenant = null;
54+
} elseif (is_int($rawTenant)) {
55+
$tenant = $rawTenant;
56+
} elseif (is_numeric($rawTenant)) {
57+
$tenant = $rawTenant + 0;
58+
} else {
59+
$tenant = (string) $rawTenant;
60+
}
61+
62+
// Everything else goes into extras
63+
$knownKeys = ['$id', 'id', 'metric', 'value', 'time', 'resource', 'resourceId', 'tenant'];
64+
$extras = array_diff_key($data, array_flip($knownKeys));
65+
66+
return new self(
67+
id: $id,
68+
metric: $metric,
69+
value: $value,
70+
time: $time,
71+
resource: $resource,
72+
resourceId: $resourceId,
73+
tenant: $tenant,
74+
extras: $extras,
75+
);
1776
}
1877

1978
public function getId(): string
2079
{
21-
return (string) ($this['$id'] ?? $this['id'] ?? '');
80+
return $this->id;
2281
}
2382

2483
public function getMetric(): string
2584
{
26-
return (string) ($this['metric'] ?? '');
85+
return $this->metric;
2786
}
2887

2988
public function getValue(int $default = 0): int|float
3089
{
31-
$v = $this['value'] ?? $default;
32-
if (is_int($v) || is_float($v)) {
33-
return $v;
34-
}
35-
return is_numeric($v) ? +$v : $default;
90+
return $this->value !== 0 ? $this->value : $default;
3691
}
3792

3893
public function getTime(): ?string
3994
{
40-
return isset($this['time']) && is_string($this['time']) ? $this['time'] : null;
95+
return $this->time;
4196
}
4297

4398
public function getResource(): string
4499
{
45-
return (string) ($this['resource'] ?? '');
100+
return $this->resource;
46101
}
47102

48103
public function getResourceId(): string
49104
{
50-
return (string) ($this['resourceId'] ?? '');
105+
return $this->resourceId;
51106
}
52107

53108
public function getTenant(): int|string|null
54109
{
55-
$t = $this['tenant'] ?? null;
56-
if ($t === null) {
57-
return null;
58-
}
59-
if (is_int($t)) {
60-
return $t;
61-
}
62-
if (is_numeric($t)) {
63-
return $t + 0;
64-
}
65-
return (string) $t;
110+
return $this->tenant;
66111
}
67112

68113
public function getAttribute(string $name, mixed $default = null): mixed
69114
{
70-
return $this[$name] ?? $default;
71-
}
72-
73-
public function setAttribute(string $key, mixed $value): static
74-
{
75-
$this[$key] = $value;
76-
return $this;
115+
return match ($name) {
116+
'id', '$id' => $this->id,
117+
'metric' => $this->metric,
118+
'value' => $this->value,
119+
'time' => $this->time,
120+
'resource' => $this->resource,
121+
'resourceId' => $this->resourceId,
122+
'tenant' => $this->tenant,
123+
default => $this->extras[$name] ?? $default,
124+
};
77125
}
78126

79127
public function hasAttribute(string $name): bool
80128
{
81-
return isset($this[$name]);
129+
return match ($name) {
130+
'id', '$id', 'metric', 'value', 'time', 'resource', 'resourceId', 'tenant' => true,
131+
default => array_key_exists($name, $this->extras),
132+
};
82133
}
83134

84135
/**
85136
* @return array<string, mixed>
86137
*/
87138
public function toArray(): array
88139
{
89-
return $this->getArrayCopy();
140+
return array_merge(
141+
[
142+
'id' => $this->id,
143+
'metric' => $this->metric,
144+
'value' => $this->value,
145+
'time' => $this->time,
146+
'resource' => $this->resource,
147+
'resourceId' => $this->resourceId,
148+
'tenant' => $this->tenant,
149+
],
150+
$this->extras,
151+
);
90152
}
91153
}

0 commit comments

Comments
 (0)