Merge pull request #478 from utopia-php/fix-index-validation

Duplicate attributes
utopia-php · Nov 12, 2024 · e43f8ee · e43f8ee
2 parents 562f52c + b0126cc
commit e43f8ee
Show file tree

Hide file tree

Showing 10 changed files with 227 additions and 34 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -1,8 +1,5 @@
 FROM composer:2.0 AS composer
 
-ARG DEBUG=false
-ENV DEBUG=$DEBUG
-
 WORKDIR /usr/local/src/
 
 COPY composer.lock /usr/local/src/

diff --git a/composer.lock b/composer.lock
diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php
@@ -1015,4 +1015,11 @@ abstract public function increaseDocumentAttribute(string $collection, string $i
      * @return string
      */
     abstract public function getConnectionId(): string;
+
+    /**
+     * Get List of internal index keys names
+     *
+     * @return array<string>
+     */
+    abstract public function getInternalIndexesKeys(): array;
 }
diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php
@@ -2425,4 +2425,9 @@ public function getConnectionId(): string
         $stmt = $this->getPDO()->query("SELECT CONNECTION_ID();");
         return $stmt->fetchColumn();
     }
+
+    public function getInternalIndexesKeys(): array
+    {
+        return ['primary', '_created_at', '_updated_at', '_tenant_id'];
+    }
 }
diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php
@@ -1937,4 +1937,9 @@ public function getConnectionId(): string
     {
         return '0';
     }
+
+    public function getInternalIndexesKeys(): array
+    {
+        return [];
+    }
 }
diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php
@@ -1129,4 +1129,8 @@ public function getLikeOperator(): string
         return 'LIKE';
     }
 
+    public function getInternalIndexesKeys(): array
+    {
+        return [];
+    }
 }
diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -334,6 +334,8 @@ class Database
 
     protected int $maxQueryValues = 100;
 
+    protected bool $migrating = false;
+
     /**
      * Stack of collection IDs when creating or updating related documents
      * @var array<string>
@@ -933,6 +935,18 @@ public function setPreserveDates(bool $preserve): static
         return $this;
     }
 
+    public function setMigrating(bool $migrating): self
+    {
+        $this->migrating = $migrating;
+
+        return $this;
+    }
+
+    public function isMigrating(): bool
+    {
+        return $this->migrating;
+    }
+
     public function withPreserveDates(callable $callback): mixed
     {
         $previous = $this->preserveDates;
@@ -1167,7 +1181,8 @@ public function createCollection(string $id, array $attributes = [], array $inde
         if ($this->validate) {
             $validator = new IndexValidator(
                 $attributes,
-                $this->adapter->getMaxIndexLength()
+                $this->adapter->getMaxIndexLength(),
+                $this->adapter->getInternalIndexesKeys()
             );
             foreach ($indexes as $index) {
                 if (!$validator->isValid($index)) {
@@ -1512,10 +1527,17 @@ public function createAttribute(string $collection, string $id, string $type, in
             $this->validateDefaultTypes($type, $default);
         }
 
-        $created = $this->adapter->createAttribute($collection->getId(), $id, $type, $size, $signed, $array);
+        try {
+            $created = $this->adapter->createAttribute($collection->getId(), $id, $type, $size, $signed, $array);
 
-        if (!$created) {
-            throw new DatabaseException('Failed to create attribute');
+            if (!$created) {
+                throw new DatabaseException('Failed to create attribute');
+            }
+        } catch (DuplicateException $e) {
+            // HACK: Metadata should still be updated, can be removed when null tenant collections are supported.
+            if (!$this->adapter->getSharedTables() || !$this->isMigrating()) {
+                throw $e;
+            }
         }
 
         if ($collection->getId() !== self::METADATA) {
@@ -2732,7 +2754,8 @@ public function createIndex(string $collection, string $id, string $type, array
         if ($this->validate) {
             $validator = new IndexValidator(
                 $collection->getAttribute('attributes', []),
-                $this->adapter->getMaxIndexLength()
+                $this->adapter->getMaxIndexLength(),
+                $this->adapter->getInternalIndexesKeys()
             );
             if (!$validator->isValid($index)) {
                 throw new DatabaseException($validator->getDescription());
@@ -2747,7 +2770,8 @@ public function createIndex(string $collection, string $id, string $type, array
             }
         } catch (DuplicateException $e) {
             // HACK: Metadata should still be updated, can be removed when null tenant collections are supported.
-            if (!$this->adapter->getSharedTables()) {
+
+            if (!$this->adapter->getSharedTables() || !$this->isMigrating()) {
                 throw $e;
             }
         }

diff --git a/src/Database/Validator/Index.php b/src/Database/Validator/Index.php
@@ -17,14 +17,21 @@ class Index extends Validator
      */
     protected array $attributes;
 
+    /**
+     * @var array<string> $reservedKeys
+     */
+    protected array $reservedKeys;
+
     /**
      * @param array<Document> $attributes
      * @param int $maxLength
+     * @param array<string> $reservedKeys
      * @throws DatabaseException
      */
-    public function __construct(array $attributes, int $maxLength)
+    public function __construct(array $attributes, int $maxLength, array $reservedKeys = [])
     {
         $this->maxLength = $maxLength;
+        $this->reservedKeys = $reservedKeys;
 
         foreach ($attributes as $attribute) {
             $key = \strtolower($attribute->getAttribute('key', $attribute->getAttribute('$id')));
@@ -80,15 +87,15 @@ public function checkEmptyIndexAttributes(Document $index): bool
     public function checkDuplicatedAttributes(Document $index): bool
     {
         $attributes = $index->getAttribute('attributes', []);
-        $orders = $index->getAttribute('orders', []);
         $stack = [];
-        foreach ($attributes as $key => $attribute) {
-            $direction = $orders[$key] ?? 'ASC';
-            $value = \strtolower($attribute . '|' . $direction);
+        foreach ($attributes as $attribute) {
+            $value = \strtolower($attribute);
+
             if (\in_array($value, $stack)) {
                 $this->message = 'Duplicate attributes provided';
                 return false;
             }
+
             $stack[] = $value;
         }
         return true;
@@ -209,6 +216,24 @@ public function checkIndexLength(Document $index): bool
         return true;
     }
 
+    /**
+     * @param Document $index
+     * @return bool
+     */
+    public function checkReservedNames(Document $index): bool
+    {
+        $key = $index->getAttribute('key', $index->getAttribute('$id'));
+
+        foreach ($this->reservedKeys as $reserved) {
+            if (\strtolower($key) === \strtolower($reserved)) {
+                $this->message = 'Index key name is reserved';
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     /**
      * Is valid.
      *
@@ -243,6 +268,10 @@ public function isValid($value): bool
             return false;
         }
 
+        if (!$this->checkReservedNames($value)) {
+            return false;
+        }
+
         return true;
     }