Skip to content

Commit

Permalink
Merge pull request #304 from utopia-php/fix-5404-check-related-doc-id
Browse files Browse the repository at this point in the history 
add checking of nested docs id in comparison check when updating document
  • Loading branch information
@abnegate
abnegate authored Aug 10, 2023
2 parents 4d55c6e + 9091665 commit 317d6a1
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 43 deletions.
75 changes: 66 additions & 9 deletions src/Database/Database.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2920,25 +2920,83 @@ public function updateDocument(string $collection, string $id, Document $documen
if ($collection->getId() !== self::METADATA) {
$documentSecurity = $collection->getAttribute('documentSecurity', false);

$relationshipKeys = [];
foreach ($relationships as $relationship) {
$relationshipKey = $relationship->getAttribute('key');
$relationshipKeys[$relationshipKey] = $relationshipKey;
$relationships[$relationship->getAttribute('key')] = $relationship;
}

// Compare if the document has any changes
foreach ($document as $key=>$value) {
foreach ($document as $key => $value) {
// Skip the nested documents as they will be checked later in recursions.
if (array_key_exists($key, $relationshipKeys)) {
if (\array_key_exists($key, $relationships)) {
$relationType = (string) $relationships[$key]['options']['relationType'];
$side = (string) $relationships[$key]['options']['side'];

switch($relationType) {
case Database::RELATION_ONE_TO_ONE:
$oldValue = $old->getAttribute($key) instanceof Document
? $old->getAttribute($key)->getId()
: $old->getAttribute($key);

if ((\is_null($value) !== \is_null($oldValue))
|| (\is_string($value) && $value !== $oldValue)
|| ($value instanceof Document && $value->getId() !== $oldValue)) {
$shouldUpdate = true;
}
break;
case Database::RELATION_ONE_TO_MANY:
case Database::RELATION_MANY_TO_ONE:
case Database::RELATION_MANY_TO_MANY:
if (
($relationType === Database::RELATION_MANY_TO_ONE && $side === Database::RELATION_SIDE_PARENT) ||
($relationType === Database::RELATION_ONE_TO_MANY && $side === Database::RELATION_SIDE_CHILD)
) {
$oldValue = $old->getAttribute($key) instanceof Document
? $old->getAttribute($key)->getId()
: $old->getAttribute($key);

if ((\is_null($value) !== \is_null($oldValue))
|| (\is_string($value) && $value !== $oldValue)
|| ($value instanceof Document && $value->getId() !== $oldValue)) {
$shouldUpdate = true;
}
break;
}

if ((\is_null($old->getAttribute($key)) !== \is_null($value))
|| \count($old->getAttribute($key)) !== \count($value)) {
$shouldUpdate = true;
break;
}
foreach ($value as $index => $relation) {
$oldValue = $old->getAttribute($key)[$index] instanceof Document
? $old->getAttribute($key)[$index]->getId()
: $old->getAttribute($key)[$index];

if ((\is_string($relation) && $relation !== $oldValue)
|| ($relation instanceof Document && $relation->getId() !== $oldValue)) {
$shouldUpdate = true;
break;
}
}
break;
}

if ($shouldUpdate) {
break;
}

continue;
}

$oldAttributeValue = $old->getAttribute($key);
$oldValue = $old->getAttribute($key);

// If values are not equal we need to update document.
if ($oldAttributeValue !== $value) {
if ($value !== $oldValue) {
$shouldUpdate = true;
break;
}
}

if ($shouldUpdate && !$validator->isValid([
...$collection->getUpdate(),
...($documentSecurity ? $old->getUpdate() : [])
Expand All @@ -2949,8 +3007,6 @@ public function updateDocument(string $collection, string $id, Document $documen

if ($shouldUpdate) {
$document->setAttribute('$updatedAt', $time);
} else {
$document->setAttribute('$updatedAt', $old->getUpdatedAt());
}

// Check if document was updated after the request timestamp
Expand Down Expand Up @@ -2980,6 +3036,7 @@ public function updateDocument(string $collection, string $id, Document $documen
$document = $this->decode($collection, $document);

$this->purgeRelatedDocuments($collection, $id);

$this->cache->purge('cache-' . $this->getNamespace() . ':' . $collection->getId() . ':' . $id . ':*');

$this->trigger(self::EVENT_DOCUMENT_UPDATE, $document);
Expand Down
63 changes: 29 additions & 34 deletions tests/Database/Base.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3220,15 +3220,14 @@ public function testWritePermissionsUpdateFailure(Document $document): Document
return $document;
}


/**
* @depends testCreateDocument
*/
public function testNoChangeUpdateDocumentWithoutPermission(Document $document): Document
{
Authorization::setRole(Role::any()->toString());

$document = static::getDatabase()->createDocument('documents', new Document([
'$id' => ID::unique(),
'$permissions' => [],
'string' => 'text📝',
'integer' => 5,
'bigint' => 8589934592, // 2^33
Expand All @@ -3237,10 +3236,14 @@ public function testNoChangeUpdateDocumentWithoutPermission(Document $document):
'colors' => ['pink', 'green', 'blue'],
]));

Authorization::cleanRoles();
$updatedDocument = static::getDatabase()->updateDocument('documents', $document->getId(), $document);
$updatedDocument = static::getDatabase()->updateDocument(
'documents',
$document->getId(),
$document
);

// Document should not be updated as there is no change. It should also not throw any authorization exception without any permission because of no change.
// Document should not be updated as there is no change.
// It should also not throw any authorization exception without any permission because of no change.
$this->assertEquals($updatedDocument->getUpdatedAt(), $document->getUpdatedAt());

return $document;
Expand All @@ -3253,53 +3256,45 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void
return;
}

Authorization::cleanRoles();
Authorization::setRole(Role::user('a')->toString());
static::getDatabase()->createCollection('parentRelationTest');
static::getDatabase()->createCollection('childRelationTest');

static::getDatabase()->createCollection('parentRelationTest', [], [], [
Permission::read(Role::user('a')),
Permission::create(Role::user('a')),
]);
static::getDatabase()->createCollection('childRelationTest', [], [], [
Permission::read(Role::user('a')),
Permission::create(Role::user('a')),
]);
static::getDatabase()->createAttribute('parentRelationTest', 'name', Database::VAR_STRING, 255, false);
static::getDatabase()->createAttribute('childRelationTest', 'name', Database::VAR_STRING, 255, false);

static::getDatabase()->createRelationship(
collection: 'parentRelationTest',
relatedCollection: 'childRelationTest',
type: Database::RELATION_ONE_TO_MANY,
id: 'childs'
id: 'children'
);

// Create document with relationship with nested data
$parent = static::getDatabase()->createDocument('parentRelationTest', new Document([
'$id' => 'parent1',
'$permissions' => [
Permission::read(Role::any()),
],
'name' => 'Parent 1',
'childs' => [
'children' => [
[
'$id' => 'child1',
'$permissions' => [
Permission::read(Role::any()),
],
'name' => 'Child 1',
],
],
]));
$this->assertEquals(1, \count($parent['childs']));
$updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', new Document([
'$id' => 'parent1',
'name'=>'Parent 1',
'$collection' => 'parentRelationTest',
'childs' => [
new Document([
'$id' => 'child1',
'$collection' => 'childRelationTest'
]),
]
]));

$this->assertEquals(1, \count($parent['children']));

$parent->setAttribute('children', ['child1']);

$updatedParent = static::getDatabase()->updateDocument('parentRelationTest', 'parent1', $parent);

$this->assertEquals($updatedParent->getUpdatedAt(), $parent->getUpdatedAt());
$this->assertEquals($updatedParent->getAttribute('childs')[0]->getUpdatedAt(), $parent->getAttribute('childs')[0]->getUpdatedAt());
$this->assertEquals($updatedParent->getAttribute('children')[0]->getUpdatedAt(), $parent->getAttribute('children')[0]->getUpdatedAt());

static::getDatabase()->deleteCollection('parentRelationTest');
static::getDatabase()->deleteCollection('childRelationTest');
Expand All @@ -3308,7 +3303,7 @@ public function testNoChangeUpdateDocumentWithRelationWithoutPermission(): void
public function testExceptionAttributeLimit(): void
{
if ($this->getDatabase()->getLimitForAttributes() > 0) {
// load the collection up to the limit
// Load the collection up to the limit
$attributes = [];
for ($i = 0; $i < $this->getDatabase()->getLimitForAttributes(); $i++) {
$attributes[] = new Document([
Expand All @@ -3322,7 +3317,8 @@ public function testExceptionAttributeLimit(): void
'filters' => [],
]);
}
$collection = static::getDatabase()->createCollection('attributeLimit', $attributes);

static::getDatabase()->createCollection('attributeLimit', $attributes);

$this->expectException(LimitException::class);
$this->assertEquals(false, static::getDatabase()->createAttribute('attributeLimit', "breaking", Database::VAR_INTEGER, 0, true));
Expand Down Expand Up @@ -11587,7 +11583,6 @@ public function testCollectionPermissionsRelationshipsUpdateWorks(array $data):

Authorization::cleanRoles();
Authorization::setRole(Role::users()->toString());

static::getDatabase()->updateDocument(
$collection->getId(),
$document->getId(),
Expand Down

0 comments on commit 317d6a1

Please sign in to comment.