Skip to content

Commit 08ce674

Browse files
abnegateabnegate
committed
(refactor): Collection constructor takes a string id only
Storage hydration is Collection::fromArray. createDocumentInstance uses that for metadata instead of new Collection($row). fromDocument is gone.
1 parent fe47cbf commit 08ce674

6 files changed

Lines changed: 148 additions & 152 deletions

File tree

‎src/Database/Collection.php‎

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,33 +10,20 @@
1010
class Collection extends Document
1111
{
1212
/**
13-
* @param array<string, mixed>|string $id Storage payload, or collection id for the named constructor
1413
* @param array<Attribute> $attributes
1514
* @param array<Index> $indexes
1615
* @param array<string>|null $permissions Null means default create-any; empty means none
1716
* @param array<string, mixed> $metadata
1817
*/
1918
public function __construct(
20-
string|array $id = '',
19+
string $id = '',
2120
string $name = '',
2221
array $attributes = [],
2322
array $indexes = [],
2423
?array $permissions = null,
2524
bool $documentSecurity = true,
2625
public array $metadata = [],
2726
) {
28-
if (\is_array($id)) {
29-
if (\is_array($id['attributes'] ?? null)) {
30-
$id['attributes'] = self::castAttributes($id['attributes']);
31-
}
32-
if (\is_array($id['indexes'] ?? null)) {
33-
$id['indexes'] = self::castIndexes($id['indexes']);
34-
}
35-
parent::__construct($id);
36-
37-
return;
38-
}
39-
4027
$data = [
4128
self::ID => ID::custom($id),
4229
'name' => $name !== '' ? $name : $id,
@@ -51,6 +38,55 @@ public function __construct(
5138
parent::__construct(\array_merge($data, $this->metadata));
5239
}
5340

41+
/**
42+
* @param array<string, mixed> $data
43+
*/
44+
public static function fromArray(array $data): self
45+
{
46+
$id = $data[self::ID] ?? '';
47+
if (! \is_string($id)) {
48+
$id = '';
49+
}
50+
51+
$name = $data['name'] ?? $id;
52+
if (! \is_string($name)) {
53+
$name = $id;
54+
}
55+
56+
$permissions = null;
57+
if (\array_key_exists(self::PERMISSIONS, $data) && \is_array($data[self::PERMISSIONS])) {
58+
$permissions = $data[self::PERMISSIONS];
59+
}
60+
61+
$rawAttributes = $data['attributes'] ?? [];
62+
$rawIndexes = $data['indexes'] ?? [];
63+
64+
$collection = new self(
65+
id: $id,
66+
name: $name,
67+
attributes: \is_array($rawAttributes) ? self::castAttributes($rawAttributes) : [],
68+
indexes: \is_array($rawIndexes) ? self::castIndexes($rawIndexes) : [],
69+
permissions: $permissions,
70+
documentSecurity: (bool) ($data['documentSecurity'] ?? true),
71+
);
72+
73+
if (\is_string($rawAttributes)) {
74+
$collection->setAttribute('attributes', $rawAttributes);
75+
}
76+
if (\is_string($rawIndexes)) {
77+
$collection->setAttribute('indexes', $rawIndexes);
78+
}
79+
80+
foreach ($data as $key => $value) {
81+
if (\in_array($key, [self::ID, 'name', 'attributes', 'indexes', self::PERMISSIONS, 'documentSecurity'], true)) {
82+
continue;
83+
}
84+
$collection->setAttribute($key, $value);
85+
}
86+
87+
return $collection;
88+
}
89+
5490
public function __get(string $name): mixed
5591
{
5692
return match ($name) {
@@ -80,26 +116,11 @@ public function __set(string $name, mixed $value): void
80116
public function __isset(string $name): bool
81117
{
82118
return match ($name) {
83-
'id', 'name', 'attributes', 'indexes', 'documentSecurity', 'metadata' => true,
84-
'permissions' => true,
119+
'id', 'name', 'attributes', 'indexes', 'permissions', 'documentSecurity', 'metadata' => true,
85120
default => $this->offsetExists($name),
86121
};
87122
}
88123

89-
public function toDocument(): Document
90-
{
91-
return $this;
92-
}
93-
94-
public static function fromDocument(Document $document): self
95-
{
96-
if ($document instanceof self) {
97-
return $document;
98-
}
99-
100-
return new self($document->getArrayCopy());
101-
}
102-
103124
/**
104125
* @param mixed $attributes
105126
* @return array<Attribute>

‎src/Database/Database.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2095,7 +2095,7 @@ public static function collectionDefinition(): array
20952095
Attribute::string(key: 'indexes', size: 1_000_000, filters: ['json']),
20962096
Attribute::boolean(key: 'documentSecurity', required: true),
20972097
],
2098-
))->toDocument()->getArrayCopy();
2098+
))->getArrayCopy();
20992099
$data[Document::COLLECTION] = self::METADATA;
21002100
$data['attributes'] = self::documentArrays($data['attributes'] ?? []);
21012101
$data['indexes'] = self::documentArrays($data['indexes'] ?? []);
@@ -2554,7 +2554,14 @@ protected function triggerHooks(Event $event, mixed $data = null): void
25542554
*/
25552555
protected function createDocumentInstance(string $collection, array $data): Document
25562556
{
2557+
if ($data === []) {
2558+
return new Document();
2559+
}
2560+
25572561
$className = $this->documentTypes[$collection] ?? Document::class;
2562+
if ($className === Collection::class) {
2563+
return Collection::fromArray($data);
2564+
}
25582565

25592566
return new $className($data);
25602567
}

‎src/Database/ORM/EntityManager.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ public function syncCollectionFromEntity(string $className): void
164164
}
165165

166166
$currentDoc = $this->db->getCollection($metadata->collection);
167-
$current = \Utopia\Database\Collection::fromDocument($currentDoc);
167+
if (! $currentDoc instanceof \Utopia\Database\Collection) {
168+
throw new \RuntimeException('Collection metadata is not a Collection model');
169+
}
170+
$current = $currentDoc;
168171

169172
$differ = new \Utopia\Database\Schema\Diff();
170173
$diff = $differ->diff($current, $desired);

‎src/Database/Schema/Introspector.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ public function introspectCollection(string $collectionId): Collection
2020
throw new \RuntimeException("Collection '{$collectionId}' not found");
2121
}
2222

23-
return Collection::fromDocument($collectionDoc);
23+
if (! $collectionDoc instanceof Collection) {
24+
throw new \RuntimeException("Collection '{$collectionId}' is not a Collection model");
25+
}
26+
27+
return $collectionDoc;
2428
}
2529

2630
/**
@@ -32,7 +36,9 @@ public function introspectDatabase(): array
3236
$result = [];
3337

3438
foreach ($collections as $doc) {
35-
$result[] = Collection::fromDocument($doc);
39+
if ($doc instanceof Collection) {
40+
$result[] = $doc;
41+
}
3642
}
3743

3844
return $result;

‎tests/unit/CollectionModelTest.php‎

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function testToDocumentProducesCorrectStructure(): void
8383
documentSecurity: true,
8484
);
8585

86-
$doc = $collection->toDocument();
86+
$doc = $collection;
8787

8888
$this->assertInstanceOf(Document::class, $doc);
8989
$this->assertSame('accounts', $doc->getId());
@@ -99,17 +99,15 @@ public function testToDocumentProducesCorrectStructure(): void
9999
public function testToDocumentUsesIdWhenNameEmpty(): void
100100
{
101101
$collection = new Collection(id: 'myCol', name: '');
102-
$doc = $collection->toDocument();
103102

104-
$this->assertSame('myCol', $doc->getAttribute('name'));
103+
$this->assertSame('myCol', $collection->getAttribute('name'));
105104
}
106105

107106
public function testToDocumentPreservesNameWhenSet(): void
108107
{
109108
$collection = new Collection(id: 'myCol', name: 'My Collection');
110-
$doc = $collection->toDocument();
111109

112-
$this->assertSame('My Collection', $doc->getAttribute('name'));
110+
$this->assertSame('My Collection', $collection->getAttribute('name'));
113111
}
114112

115113
public function testFromDocumentRoundtrip(): void
@@ -126,8 +124,7 @@ public function testFromDocumentRoundtrip(): void
126124
documentSecurity: false,
127125
);
128126

129-
$doc = $original->toDocument();
130-
$restored = Collection::fromDocument($doc);
127+
$restored = Collection::fromArray($original->getArrayCopy());
131128

132129
$this->assertSame($original->id, $restored->id);
133130
$this->assertSame($original->name, $restored->name);
@@ -137,14 +134,13 @@ public function testFromDocumentRoundtrip(): void
137134
$this->assertSame($original->attributes[0]->key, $restored->attributes[0]->key);
138135
$this->assertSame($original->indexes[0]->key, $restored->indexes[0]->key);
139136
$this->assertInstanceOf(StringType::class, $restored->attributes[0]);
140-
$this->assertInstanceOf(Attribute::class, $doc->getAttribute('attributes')[0]);
141-
$this->assertInstanceOf(Index::class, $doc->getAttribute('indexes')[0]);
137+
$this->assertInstanceOf(Attribute::class, $original->getAttribute('attributes')[0]);
138+
$this->assertInstanceOf(Index::class, $original->getAttribute('indexes')[0]);
142139
}
143140

144-
public function testFromDocumentWithEmptyDocument(): void
141+
public function testFromArrayWithEmptyPayload(): void
145142
{
146-
$doc = new Document();
147-
$collection = Collection::fromDocument($doc);
143+
$collection = Collection::fromArray([]);
148144

149145
$this->assertSame('', $collection->id);
150146
$this->assertSame('', $collection->name);
@@ -165,11 +161,10 @@ public function testWithMultipleAttributes(): void
165161

166162
$collection = new Collection(id: 'users', attributes: $attrs);
167163

168-
$doc = $collection->toDocument();
169-
$restoredAttrs = $doc->getAttribute('attributes');
164+
$restoredAttrs = $collection->getAttribute('attributes');
170165
$this->assertCount(4, $restoredAttrs);
171166

172-
$restored = Collection::fromDocument($doc);
167+
$restored = Collection::fromArray($collection->getArrayCopy());
173168
$this->assertCount(4, $restored->attributes);
174169
$this->assertSame('name', $restored->attributes[0]->key);
175170
$this->assertSame('active', $restored->attributes[3]->key);
@@ -189,10 +184,9 @@ public function testWithMultipleIndexes(): void
189184

190185
$collection = new Collection(id: 'users', indexes: $indexes);
191186

192-
$doc = $collection->toDocument();
193-
$this->assertCount(3, $doc->getAttribute('indexes'));
187+
$this->assertCount(3, $collection->getAttribute('indexes'));
194188

195-
$restored = Collection::fromDocument($doc);
189+
$restored = Collection::fromArray($collection->getArrayCopy());
196190
$this->assertCount(3, $restored->indexes);
197191
$this->assertSame('idx_compound', $restored->indexes[2]->key);
198192
}
@@ -207,26 +201,23 @@ public function testWithPermissions(): void
207201
];
208202

209203
$collection = new Collection(id: 'posts', permissions: $permissions);
210-
$doc = $collection->toDocument();
211204

212-
$this->assertCount(4, $doc->getPermissions());
213-
$this->assertContains(Permission::read(Role::any()), $doc->getPermissions());
205+
$this->assertCount(4, $collection->getPermissions());
206+
$this->assertContains(Permission::read(Role::any()), $collection->getPermissions());
214207
}
215208

216209
public function testDocumentSecurityTrue(): void
217210
{
218211
$collection = new Collection(id: 'secure', documentSecurity: true);
219-
$doc = $collection->toDocument();
220212

221-
$this->assertTrue($doc->getAttribute('documentSecurity'));
213+
$this->assertTrue($collection->getAttribute('documentSecurity'));
222214
}
223215

224216
public function testDocumentSecurityFalse(): void
225217
{
226218
$collection = new Collection(id: 'insecure', documentSecurity: false);
227-
$doc = $collection->toDocument();
228219

229-
$this->assertFalse($doc->getAttribute('documentSecurity'));
220+
$this->assertFalse($collection->getAttribute('documentSecurity'));
230221
}
231222

232223
public function testFromDocumentPreservesPermissions(): void
@@ -236,16 +227,14 @@ public function testFromDocumentPreservesPermissions(): void
236227
Permission::create(Role::any()),
237228
];
238229

239-
$doc = new Document([
230+
$collection = Collection::fromArray([
240231
'$id' => 'test',
241232
'$permissions' => $permissions,
242233
'name' => 'test',
243234
'attributes' => [],
244235
'indexes' => [],
245236
'documentSecurity' => true,
246237
]);
247-
248-
$collection = Collection::fromDocument($doc);
249238
$this->assertNotNull($collection->permissions);
250239
$this->assertCount(2, $collection->permissions);
251240
}
@@ -255,8 +244,7 @@ public function testAttributeModelsStayModels(): void
255244
$attr = Attribute::string(key: 'title', size: 64);
256245
$collection = new Collection(id: 'articles', attributes: [$attr]);
257246

258-
$doc = $collection->toDocument();
259-
$attributes = $doc->getAttribute('attributes');
247+
$attributes = $collection->getAttribute('attributes');
260248

261249
$this->assertInstanceOf(Attribute::class, $attributes[0]);
262250
$this->assertSame('title', $attributes[0]->key);
@@ -268,8 +256,7 @@ public function testIndexModelsStayModels(): void
268256
$idx = Index::fullText(key: 'idx_test', attributes: ['body']);
269257
$collection = new Collection(id: 'articles', indexes: [$idx]);
270258

271-
$doc = $collection->toDocument();
272-
$indexes = $doc->getAttribute('indexes');
259+
$indexes = $collection->getAttribute('indexes');
273260

274261
$this->assertInstanceOf(Index::class, $indexes[0]);
275262
$this->assertSame('idx_test', $indexes[0]->key);

0 commit comments

Comments
 (0)