Skip to content

Commit 0eed7f1

Browse files
abnegateabnegate
authored
Merge pull request #524 from utopia-php/fix-create-documents-nulls
Fix different column for nulls bug in create documents
2 parents a32163e + 5694fa7 commit 0eed7f1

File tree

6 files changed

+151
-23
lines changed

6 files changed

+151
-23
lines changed

‎composer.lock‎

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Database/Adapter/MariaDB.php‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,31 @@ public function createDocuments(string $collection, array $documents): array
980980
try {
981981
$name = $this->filter($collection);
982982

983+
$attributeKeys = Database::INTERNAL_ATTRIBUTE_KEYS;
984+
985+
$hasInternalId = null;
986+
foreach ($documents as $document) {
987+
$attributes = $document->getAttributes();
988+
$attributeKeys = array_merge($attributeKeys, array_keys($attributes));
989+
990+
if ($hasInternalId === null) {
991+
$hasInternalId = !empty($document->getInternalId());
992+
} elseif ($hasInternalId == empty($document->getInternalId())) {
993+
throw new DatabaseException('All documents must have an internalId if one is set');
994+
}
995+
}
996+
$attributeKeys = array_unique($attributeKeys);
997+
998+
if ($this->sharedTables) {
999+
$attributeKeys[] = '_tenant';
1000+
}
1001+
1002+
$columns = [];
1003+
foreach ($attributeKeys as $key => $attribute) {
1004+
$columns[$key] = "`{$this->filter($attribute)}`";
1005+
}
1006+
$columns = '(' . \implode(', ', $columns) . ')';
1007+
9831008
$bindIndex = 0;
9841009
$batchKeys = [];
9851010
$bindValues = [];
@@ -998,6 +1023,7 @@ public function createDocuments(string $collection, array $documents): array
9981023

9991024
if (! empty($document->getInternalId())) {
10001025
$attributes['_id'] = $document->getInternalId();
1026+
$attributeKeys[] = '_id';
10011027
} else {
10021028
$documentIds[] = $document->getId();
10031029
}
@@ -1006,16 +1032,10 @@ public function createDocuments(string $collection, array $documents): array
10061032
$attributes['_tenant'] = $this->tenant;
10071033
}
10081034

1009-
$columns = [];
1010-
foreach (\array_keys($attributes) as $key => $attribute) {
1011-
$columns[$key] = "`{$this->filter($attribute)}`";
1012-
}
1013-
1014-
$columns = '(' . \implode(', ', $columns) . ')';
1015-
10161035
$bindKeys = [];
10171036

1018-
foreach ($attributes as $value) {
1037+
foreach ($attributeKeys as $key) {
1038+
$value = $attributes[$key] ?? null;
10191039
if (\is_array($value)) {
10201040
$value = \json_encode($value);
10211041
}

‎src/Database/Adapter/Mongo.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,18 @@ public function createDocuments(string $collection, array $documents): array
757757
$name = $this->getNamespace() . '_' . $this->filter($collection);
758758

759759
$records = [];
760+
$hasInternalId = null;
761+
$documents = array_map(fn ($doc) => clone $doc, $documents);
762+
760763
foreach ($documents as $document) {
764+
$internalId = $document->getInternalId();
765+
766+
if ($hasInternalId === null) {
767+
$hasInternalId = !empty($internalId);
768+
} elseif ($hasInternalId == empty($internalId)) {
769+
throw new DatabaseException('All documents must have an internalId if one is set');
770+
}
771+
761772
$document->removeAttribute('$internalId');
762773

763774
if ($this->sharedTables) {
@@ -767,6 +778,10 @@ public function createDocuments(string $collection, array $documents): array
767778
$record = $this->replaceChars('$', '_', (array)$document);
768779
$record = $this->timeToMongo($record);
769780

781+
if (!empty($internalId)) {
782+
$record['_id'] = $internalId;
783+
}
784+
770785
$records[] = $this->removeNullKeys($record);
771786
}
772787

‎src/Database/Adapter/Postgres.php‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,31 @@ public function createDocuments(string $collection, array $documents): array
10381038

10391039
try {
10401040
$name = $this->filter($collection);
1041+
$attributeKeys = Database::INTERNAL_ATTRIBUTE_KEYS;
1042+
1043+
$hasInternalId = null;
1044+
foreach ($documents as $document) {
1045+
$attributes = $document->getAttributes();
1046+
$attributeKeys = array_merge($attributeKeys, array_keys($attributes));
1047+
1048+
if ($hasInternalId === null) {
1049+
$hasInternalId = !empty($document->getInternalId());
1050+
} elseif ($hasInternalId == empty($document->getInternalId())) {
1051+
throw new DatabaseException('All documents must have an internalId if one is set');
1052+
}
1053+
}
1054+
$attributeKeys = array_unique($attributeKeys);
1055+
1056+
if ($this->sharedTables) {
1057+
$attributeKeys[] = '_tenant';
1058+
}
1059+
1060+
$columns = [];
1061+
foreach ($attributeKeys as $key => $attribute) {
1062+
$columns[$key] = "\"{$this->filter($attribute)}\"";
1063+
}
1064+
$columns = '(' . \implode(', ', $columns) . ')';
1065+
10411066
$internalIds = [];
10421067

10431068
$bindIndex = 0;
@@ -1055,22 +1080,17 @@ public function createDocuments(string $collection, array $documents): array
10551080
if (!empty($document->getInternalId())) {
10561081
$internalIds[$document->getId()] = true;
10571082
$attributes['_id'] = $document->getInternalId();
1083+
$attributeKeys[] = '_id';
10581084
}
10591085

10601086
if ($this->sharedTables) {
10611087
$attributes['_tenant'] = $this->tenant;
10621088
}
10631089

1064-
$columns = [];
1065-
foreach (\array_keys($attributes) as $key => $attribute) {
1066-
$columns[$key] = "\"{$this->filter($attribute)}\"";
1067-
}
1068-
1069-
$columns = '(' . \implode(', ', $columns) . ')';
1070-
10711090
$bindKeys = [];
10721091

1073-
foreach ($attributes as $value) {
1092+
foreach ($attributeKeys as $key) {
1093+
$value = $attributes[$key] ?? null;
10741094
if (\is_array($value)) {
10751095
$value = \json_encode($value);
10761096
}

‎src/Database/Database.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,13 @@ class Database
222222
],
223223
];
224224

225+
public const INTERNAL_ATTRIBUTE_KEYS = [
226+
'_uid',
227+
'_createdAt',
228+
'_updatedAt',
229+
'_permissions',
230+
];
231+
225232
public const INTERNAL_INDEXES = [
226233
'_id',
227234
'_uid',

‎tests/e2e/Adapter/Base.php‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2238,6 +2238,72 @@ public function testCreateDocuments(): array
22382238
return $documents;
22392239
}
22402240

2241+
public function testCreateDocumentsWithDifferentAttributes(): void
2242+
{
2243+
$collection = 'testDiffAttributes';
2244+
2245+
static::getDatabase()->createCollection($collection);
2246+
2247+
$this->assertEquals(true, static::getDatabase()->createAttribute($collection, 'string', Database::VAR_STRING, 128, true));
2248+
$this->assertEquals(true, static::getDatabase()->createAttribute($collection, 'integer', Database::VAR_INTEGER, 0, false));
2249+
$this->assertEquals(true, static::getDatabase()->createAttribute($collection, 'bigint', Database::VAR_INTEGER, 8, false));
2250+
$this->assertEquals(true, static::getDatabase()->createAttribute($collection, 'string_default', Database::VAR_STRING, 128, false, 'default'));
2251+
2252+
$documents = [
2253+
new Document([
2254+
'$id' => 'first',
2255+
'string' => 'text📝',
2256+
'integer' => 5,
2257+
'string_default' => 'not_default',
2258+
]),
2259+
new Document([
2260+
'$id' => 'second',
2261+
'string' => 'text📝',
2262+
]),
2263+
];
2264+
2265+
$documents = static::getDatabase()->createDocuments($collection, $documents);
2266+
2267+
$this->assertEquals(2, count($documents));
2268+
2269+
$this->assertEquals('text📝', $documents[0]->getAttribute('string'));
2270+
$this->assertEquals(5, $documents[0]->getAttribute('integer'));
2271+
$this->assertEquals('not_default', $documents[0]->getAttribute('string_default'));
2272+
$this->assertEquals('text📝', $documents[1]->getAttribute('string'));
2273+
$this->assertNull($documents[1]->getAttribute('integer'));
2274+
$this->assertEquals('default', $documents[1]->getAttribute('string_default'));
2275+
2276+
/**
2277+
* Expect fail, mix of internalId and no internalId
2278+
*/
2279+
$documents = [
2280+
new Document([
2281+
'$id' => 'third',
2282+
'$internalId' => 'third',
2283+
'string' => 'text📝',
2284+
]),
2285+
new Document([
2286+
'$id' => 'fourth',
2287+
'string' => 'text📝',
2288+
]),
2289+
];
2290+
2291+
try {
2292+
static::getDatabase()->createDocuments($collection, $documents);
2293+
$this->fail('Failed to throw exception');
2294+
} catch (DatabaseException $e) {
2295+
}
2296+
2297+
$documents = array_reverse($documents);
2298+
try {
2299+
static::getDatabase()->createDocuments($collection, $documents);
2300+
$this->fail('Failed to throw exception');
2301+
} catch (DatabaseException $e) {
2302+
}
2303+
2304+
static::getDatabase()->deleteCollection($collection);
2305+
}
2306+
22412307
public function testCreateOrUpdateDocuments(): void
22422308
{
22432309
if (!static::getDatabase()->getAdapter()->getSupportForUpserts()) {

0 commit comments

Comments
 (0)