Skip to content

Internal attributes defaults #608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fogelito wants to merge 13 commits into from
Closed

Internal attributes defaults #608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6d5d60e
Check internals
fogelito Jun 12, 2025
173264b
RelationshipTests
fogelito Jun 12, 2025
2ecf187
Merge branch 'main' of github.com:utopia-php/database into internal-a…
fogelito Jun 12, 2025
1d4d7d1
Add attribute for update and create
fogelito Jun 15, 2025
ba87da6
Test return attributes
fogelito Jun 16, 2025
8545413
Remove var_dump
fogelito Jun 16, 2025
32c6884
Remove var_dump
fogelito Jun 16, 2025
ed9c424
tests internals RelationshipTests
fogelito Jun 16, 2025
f71ee21
fix main::$collection
fogelito Jun 16, 2025
22b5f19
Merge branch 'main' of github.com:utopia-php/database into internal-a…
fogelito Jun 16, 2025
8b0bc4c
Fix test non share table
fogelito Jun 17, 2025
b23b8a8
Remove hint
fogelito Jun 17, 2025
42600a6
Merge branch 'main' of github.com:utopia-php/database into internal-a…
fogelito Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 36 additions & 15 deletions src/Database/Adapter/SQL.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,12 @@ public function getDocument(string $collection, string $id, array $queries = [],
unset($document['_permissions']);
}

$document['main::$permissions'] = json_decode($document['main::$permissions'], true);

if ($this->sharedTables) {
$document['main::$tenant'] = $document['main::$tenant'] === null ? null : (int)$document['main::$tenant'];
}

return new Document($document);
}

Expand Down Expand Up @@ -1674,6 +1680,32 @@ public function getTenantQuery(
return "{$condition} ({$alias}{$dot}_tenant IN ({$bindings}) {$orIsNull})";
}

/**
* Get the SQL projection given the selected attributes
*
* @param array<string> $selects
* @return string
* @throws Exception
*/
protected function addHiddenAttribute(array $selects): string
{
$alias = Query::DEFAULT_ALIAS;

$strings = [];

$strings[] = $alias.'._uid as '.$this->quote($alias.'::$id');
$strings[] = $alias.'._id as '.$this->quote($alias.'::$sequence');
$strings[] = $alias.'._permissions as '.$this->quote($alias.'::$permissions');
$strings[] = $alias.'._createdAt as '.$this->quote($alias.'::$createdAt');
$strings[] = $alias.'._updatedAt as '.$this->quote($alias.'::$updatedAt');

if ($this->sharedTables) {
$strings[] = $alias.'._tenant as '.$this->quote($alias.'::$tenant');
}

return ', '.implode(', ', $strings);
}

/**
* Get the SQL projection given the selected attributes
*
Expand All @@ -1685,28 +1717,17 @@ public function getTenantQuery(
protected function getAttributeProjection(array $selections, string $prefix): mixed
{
if (empty($selections) || \in_array('*', $selections)) {
return "{$this->quote($prefix)}.*";
return "{$this->quote($prefix)}.* {$this->addHiddenAttribute($selections)}";
}

$internalKeys = [
'$id',
'$sequence',
'$permissions',
'$createdAt',
'$updatedAt',
];

$selections = \array_diff($selections, [...$internalKeys, '$collection']);

foreach ($internalKeys as $internalKey) {
$selections[] = $this->getInternalKeyForAttribute($internalKey);
}
$selections = array_diff($selections, ['$collection']);

foreach ($selections as &$selection) {
$selection = $this->getInternalKeyForAttribute($selection);
$selection = "{$this->quote($prefix)}.{$this->quote($this->filter($selection))}";
}

return \implode(',', $selections);
return \implode(',', $selections).$this->addHiddenAttribute($selections);
}

protected function getInternalKeyForAttribute(string $attribute): string
Expand Down
95 changes: 81 additions & 14 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,25 @@ public function silent(callable $callback, ?array $listeners = null): mixed
}
}

/**
* Check if attribute is internal
*
* @param string $attribute
* @return bool
*/
public static function isInternalAttribute(string $attribute): bool
{
if (str_contains($attribute, '$')) {
foreach (Database::INTERNAL_ATTRIBUTES as $attr) {
if (str_contains($attribute, $attr['$id'])) {
return true;
}
}
}

return false;
}

/**
* Get getConnection Id
*
Expand Down Expand Up @@ -3277,7 +3296,7 @@ public function getDocument(string $collection, string $id, array $queries = [],
if ($collection->getId() !== self::METADATA) {
if (!$validator->isValid([
...$collection->getRead(),
...($documentSecurity ? $document->getRead() : [])
...($documentSecurity ? $document->getRead('main::$permissions') : [])
])) {
return new Document();
}
Expand All @@ -3294,17 +3313,17 @@ public function getDocument(string $collection, string $id, array $queries = [],
$queries,
$forUpdate
);

if ($document->isEmpty()) {
return $document;
}

$document->setAttribute('$collection', $collection->getId());
$document->setAttribute('main::$collection', $collection->getId());

if ($collection->getId() !== self::METADATA) {
if (!$validator->isValid([
...$collection->getRead(),
...($documentSecurity ? $document->getRead() : [])
...($documentSecurity ? $document->getRead('main::$permissions') : [])
])) {
return new Document();
}
Expand Down Expand Up @@ -3335,6 +3354,9 @@ public function getDocument(string $collection, string $id, array $queries = [],

$this->trigger(self::EVENT_DOCUMENT_READ, $document);

$document->setAttribute('main::$createdAt', DateTime::formatTz($document->getAttribute('main::$createdAt')));
$document->setAttribute('main::$updatedAt', DateTime::formatTz($document->getAttribute('main::$updatedAt')));

return $document;
}

Expand Down Expand Up @@ -3667,6 +3689,16 @@ public function createDocument(string $collection, Document $document): Document

$this->trigger(self::EVENT_DOCUMENT_CREATE, $document);

$document->setAttribute('main::$id', $document->getId());
$document->setAttribute('main::$sequence', $document->getSequence());
$document->setAttribute('main::$permissions', $document->getPermissions());
$document->setAttribute('main::$createdAt', $document->getCreatedAt());
$document->setAttribute('main::$updatedAt', $document->getUpdatedAt());
$document->setAttribute('main::$collection', $document->getCollection());
if ($this->adapter->getSharedTables()) {
$document->setAttribute('main::$tenant', $document->getTenant());
}

return $document;
}

Expand Down Expand Up @@ -3989,7 +4021,7 @@ private function relateDocuments(

if ($related->isEmpty()) {
// If the related document doesn't exist, create it, inheriting permissions if none are set
if (!isset($relation['$permissions'])) {
if (!isset($relation['$permissions'])) { // todo: Should this be main::$permissions
$relation->setAttribute('$permissions', $document->getPermissions());
}

Expand Down Expand Up @@ -4143,8 +4175,14 @@ public function updateDocument(string $collection, string $id, Document $documen
$relationships[$relationship->getAttribute('key')] = $relationship;
}

$alias = Query::DEFAULT_ALIAS;

// Compare if the document has any changes
foreach ($document as $key => $value) {
if (str_starts_with($key, $alias.'::') && Database::isInternalAttribute($key)){
continue;
}

// Skip the nested documents as they will be checked later in recursions.
if (\array_key_exists($key, $relationships)) {
// No need to compare nested documents more than max depth.
Expand Down Expand Up @@ -4288,6 +4326,16 @@ public function updateDocument(string $collection, string $id, Document $documen

$this->trigger(self::EVENT_DOCUMENT_UPDATE, $document);

$document->setAttribute('main::$id', $document->getId());
$document->setAttribute('main::$sequence', $document->getSequence());
$document->setAttribute('main::$permissions', $document->getPermissions());
$document->setAttribute('main::$createdAt', $document->getCreatedAt());
$document->setAttribute('main::$updatedAt', $document->getUpdatedAt());
$document->setAttribute('main::$collection', $document->getCollection());
if ($this->adapter->getSharedTables()) {
$document->setAttribute('main::$tenant', $document->getTenant());
}

return $document;
}

Expand Down Expand Up @@ -4588,6 +4636,7 @@ private function updateDocumentRelationships(Document $collection, Document $old
$related->getId(),
$related->setAttribute($twoWayKey, $document->getId())
));

break;
case 'object':
if ($value instanceof Document) {
Expand All @@ -4606,7 +4655,7 @@ private function updateDocumentRelationships(Document $collection, Document $old

$this->relationshipWriteStack[] = $relatedCollection->getId();
if ($related->isEmpty()) {
if (!isset($value['$permissions'])) {
if (!isset($value['$permissions'])) {// todo check if should be main::$permissions
$value->setAttribute('$permissions', $document->getAttribute('$permissions'));
}
$related = $this->createDocument(
Expand Down Expand Up @@ -4695,7 +4744,7 @@ private function updateDocumentRelationships(Document $collection, Document $old
);

if ($related->isEmpty()) {
if (!isset($relation['$permissions'])) {
if (!isset($relation['$permissions'])) { // todo check if should be main::$permissions
$relation->setAttribute('$permissions', $document->getAttribute('$permissions'));
}
$this->createDocument(
Expand Down Expand Up @@ -4735,7 +4784,7 @@ private function updateDocumentRelationships(Document $collection, Document $old
);

if ($related->isEmpty()) {
if (!isset($value['$permissions'])) {
if (!isset($value['$permissions'])) { // todo check if should be main::$permissions
$value->setAttribute('$permissions', $document->getAttribute('$permissions'));
}
$this->createDocument(
Expand Down Expand Up @@ -4808,7 +4857,7 @@ private function updateDocumentRelationships(Document $collection, Document $old
$related = $this->getDocument($relatedCollection->getId(), $relation->getId(), [Query::select(['$id'])]);

if ($related->isEmpty()) {
if (!isset($value['$permissions'])) {
if (!isset($value['$permissions'])) {// todo check if should be main::$permissions
$relation->setAttribute('$permissions', $document->getAttribute('$permissions'));
}
$related = $this->createDocument(
Expand Down Expand Up @@ -6085,6 +6134,9 @@ public function find(string $collection, array $queries = [], string $forPermiss

if (!$node->isEmpty()) {
$node->setAttribute('$collection', $collection->getId());
$node->setAttribute('main::$collection', $collection->getId());
$node->setAttribute('main::$createdAt', DateTime::formatTz($node->getAttribute('main::$createdAt')));
$node->setAttribute('main::$updatedAt', DateTime::formatTz($node->getAttribute('main::$updatedAt')));
}
}

Expand Down Expand Up @@ -6298,6 +6350,21 @@ public static function addFilter(string $name, callable $encode, callable $decod
*/
public function encode(Document $collection, Document $document): Document
{
/**
* When iterating over an ArrayObject, foreach uses an internal iterator (like Iterator), and modifying the object during iteration doesn’t affect the iterator immediately.
*/
$keysToRemove = [];

foreach ($document as $key => $value) {
if (strpos($key, '::$') !== false) {
$keysToRemove[] = $key;
}
}

foreach ($keysToRemove as $key) {
unset($document[$key]);
}

$attributes = $collection->getAttribute('attributes', []);

$internalAttributes = \array_filter(Database::INTERNAL_ATTRIBUTES, function ($attribute) {
Expand Down Expand Up @@ -6586,12 +6653,12 @@ private function validateSelections(Document $collection, array $queries): array

$selections = \array_merge($selections, $relationshipSelections);

$selections[] = '$id';
$selections[] = '$sequence';
$selections[] = '$collection';
$selections[] = '$createdAt';
$selections[] = '$updatedAt';
$selections[] = '$permissions';
// $selections[] = '$id';
// $selections[] = '$sequence';
// $selections[] = '$collection';
// $selections[] = '$createdAt';
// $selections[] = '$updatedAt';
// $selections[] = '$permissions';

return \array_values(\array_unique($selections));
}
Expand Down
19 changes: 7 additions & 12 deletions src/Database/Document.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,17 @@ public function getCollection(): string
/**
* @return array<string>
*/
public function getPermissions(): array
public function getPermissions(string $attribute = '$permissions'): array
{
return \array_values(\array_unique($this->getAttribute('$permissions', [])));
return \array_values(\array_unique($this->getAttribute($attribute, [])));
}
Comment on lines +83 to 86

@coderabbitai coderabbitai Bot Jun 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Guard against non-array permission payloads

getPermissions() assumes the resolved attribute is always an array.
If the DB adapter ever returns malformed data (e.g. a string‐encoded JSON, null, etc.), array_unique() will raise a warning and the method will explode.

-    public function getPermissions(string $attribute = '$permissions'): array
-    {
-        return \array_values(\array_unique($this->getAttribute($attribute, [])));
-    }
+    public function getPermissions(string $attribute = '$permissions'): array
+    {
+        $permissions = $this->getAttribute($attribute, []);
+
+        if (!\is_array($permissions)) {
+            // Fail-safe – return an empty list rather than trigger a PHP warning
+            return [];
+        }
+
+        return \array_values(\array_unique($permissions));
+    }

Same defensive check is advisable in the other helpers that delegate to this method.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function getPermissions(string $attribute = '$permissions'): array
{
return \array_values(\array_unique($this->getAttribute('$permissions', [])));
return \array_values(\array_unique($this->getAttribute($attribute, [])));
}
public function getPermissions(string $attribute = '$permissions'): array
{
$permissions = $this->getAttribute($attribute, []);
if (!\is_array($permissions)) {
// Fail-safe – return an empty list rather than trigger a PHP warning
return [];
}
return \array_values(\array_unique($permissions));
}
🤖 Prompt for AI Agents 
In src/Database/Document.php around lines 83 to 86, the getPermissions() method
assumes the attribute is always an array, which can cause warnings if the data
is malformed. Add a defensive check to verify the attribute is an array before
calling array_unique and array_values; if not, return an empty array. Apply
similar checks in other helper methods that call getPermissions() to ensure
robustness against unexpected data types.


/**
* @return array<string>
*/
public function getRead(): array
public function getRead(string $attribute = '$permissions'): array
{
return $this->getPermissionsByType(Database::PERMISSION_READ);
return $this->getPermissionsByType(Database::PERMISSION_READ, $attribute);
}
Comment on lines +91 to 94

@coderabbitai coderabbitai Bot Jun 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Propagate the attribute parameter to the convenience helpers

getRead() (and, by extension, getCreate(), getUpdate(), getDelete()) now call the 2-parameter getPermissionsByType() but still omit the second argument.

If callers pass a custom attribute key to getRead() they’ll silently fall back to '$permissions', which defeats the purpose of the new API.

-    public function getRead(string $attribute = '$permissions'): array
-    {
-        return $this->getPermissionsByType(Database::PERMISSION_READ, $attribute);
-    }
+    public function getRead(string $attribute = '$permissions'): array
+    {
+        return $this->getPermissionsByType(Database::PERMISSION_READ, $attribute);
+    }

Replicate the additional $attribute parameter on getCreate(), getUpdate(), and getDelete() for a consistent public surface.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/Database/Document.php around lines 91 to 94, the getRead() method accepts
an $attribute parameter but does not pass it to getPermissionsByType(), causing
it to always use the default '$permissions'. To fix this, update getRead() to
pass the $attribute parameter to getPermissionsByType(). Also, add the
$attribute parameter to the signatures of getCreate(), getUpdate(), and
getDelete() methods and ensure they pass it to getPermissionsByType() as well,
maintaining a consistent API.


/**
Expand Down Expand Up @@ -132,11 +132,11 @@ public function getWrite(): array
/**
* @return array<string>
*/
public function getPermissionsByType(string $type): array
public function getPermissionsByType(string $type, string $attribute = '$permissions'): array
{
$typePermissions = [];

foreach ($this->getPermissions() as $permission) {
foreach ($this->getPermissions($attribute) as $permission) {
if (!\str_starts_with($permission, $type)) {
continue;
}
Expand Down Expand Up @@ -183,13 +183,8 @@ public function getAttributes(): array
{
$attributes = [];

$internalKeys = \array_map(
fn ($attr) => $attr['$id'],
Database::INTERNAL_ATTRIBUTES
);

foreach ($this as $attribute => $value) {
if (\in_array($attribute, $internalKeys)) {
if (Database::isInternalAttribute($attribute)){
continue;
}

Expand Down
Loading