tests/e2e/Services/Databases/Legacy/DatabasesBase.php (2)

7871-7871: Strengthen the 400 assertions with a minimal reason check.

Assert that an error message is present so failures are easier to triage without over‑coupling to exact text.

-        $this->assertEquals(400, $point['headers']['status-code']);
+        $this->assertEquals(400, $point['headers']['status-code']);
+        $this->assertNotEmpty($point['body']['message'] ?? null);

-        $this->assertEquals(400, $line['headers']['status-code']);
+        $this->assertEquals(400, $line['headers']['status-code']);
+        $this->assertNotEmpty($line['body']['message'] ?? null);

-        $this->assertEquals(400, $poly['headers']['status-code']);
+        $this->assertEquals(400, $poly['headers']['status-code']);
+        $this->assertNotEmpty($poly['body']['message'] ?? null);

Also applies to: 7894-7894, 7917-7917

---

7929-7930: Clean up created resources to avoid cross‑test leakage.

This test creates a standalone database/collection; deleting them keeps e2e runs tidy.

-        $this->assertEquals(202, $poly['headers']['status-code']);
+        $this->assertEquals(202, $poly['headers']['status-code']);
+
+        // Cleanup to avoid leaking DB/collection across e2e runs
+        $this->client->call(Client::METHOD_DELETE, '/databases/' . $databaseId . '/collections/' . $colId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));
+        $this->client->call(Client::METHOD_DELETE, '/databases/' . $databaseId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));

src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Attributes/Action.php (2)

381-383: Preserve original StructureException as the previous throwable.

Pass $e as previous to retain stack context.

-        } catch (StructureException $e) {
-            throw new Exception($this->getInvalidStructureException(), $e->getMessage());
+        } catch (StructureException $e) {
+            throw new Exception($this->getInvalidStructureException(), $e->getMessage(), null, $e);
         }

---

392-397: Guard undefined index ‘twoWay’.

Use null-coalescing to avoid notices when options['twoWay'] isn’t set.

-        if ($type === Database::VAR_RELATIONSHIP && $options['twoWay']) {
+        if ($type === Database::VAR_RELATIONSHIP && ($options['twoWay'] ?? false)) {

tests/e2e/Services/Databases/TablesDB/DatabasesBase.php (1)

8831-8959: Add minimal assertions and cleanup to harden the new spatial-on-existing-data test.

• Consider asserting response body shape (e.g., presence of “message”) for 400s to reduce false positives.
• Add explicit cleanup to avoid test data leakage across runs.

Apply this diff to add cleanup at the end of the test:

         $this->assertEquals(202, $poly['headers']['status-code']);
+
+        // Cleanup to prevent residue affecting subsequent tests
+        $this->client->call(Client::METHOD_DELETE, '/tablesdb/' . $databaseId . '/tables/' . $tableId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));
+        $this->client->call(Client::METHOD_DELETE, '/tablesdb/' . $databaseId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));
     }

Optionally, after creating the optional spatial columns, fetch one back to assert required=false and default=null once the worker completes (mirrors patterns used elsewhere in this file).

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

tests/e2e/Services/Databases/TablesDB/DatabasesBase.php (1)

9022-9059: Non-null spatial defaults covered — resolves earlier question

This block validates defaults other than null for point/line/polygon as previously discussed.

tests/e2e/Services/Databases/TablesDB/DatabasesBase.php (2)

8831-8959: Add cleanup and (optionally) assert error type to harden the test

• Missing teardown leaves a database/table around; add cleanup to avoid resource buildup and test interference.
• Optional: also assert the error type to verify we hit the pre-validation path (keep message assertion out if unstable).

Apply this diff near the end of the test:

         $this->assertEquals(202, $poly['headers']['status-code']);
+        
+        // Cleanup
+        $this->client->call(Client::METHOD_DELETE, '/tablesdb/' . $databaseId . '/tables/' . $tableId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));
+        $this->client->call(Client::METHOD_DELETE, '/tablesdb/' . $databaseId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));

---

8961-9059: Verify default propagation on new rows and add teardown

The test creates optional spatial columns with non-null defaults but doesn’t verify that a new row receives those defaults. Add a quick insert/assert plus cleanup.

Apply this diff after creating the polygon column:

         $this->assertEquals(202, $poly['headers']['status-code']);
+
+        // Wait for worker to apply columns
+        sleep(2);
+
+        // Insert a new row and assert defaults are applied
+        $new = $this->client->call(Client::METHOD_POST, '/tablesdb/' . $databaseId . '/tables/' . $tableId . '/rows', array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+        ], $this->getHeaders()), [
+            'rowId' => ID::unique(),
+            'data' => ['description' => 'after-defaults'],
+            'permissions' => [
+                Permission::read(Role::user($this->getUser()['$id'])),
+                Permission::update(Role::user($this->getUser()['$id'])),
+                Permission::delete(Role::user($this->getUser()['$id'])),
+            ]
+        ]);
+        $this->assertEquals(201, $new['headers']['status-code']);
+        $this->assertEquals([0.0, 0.0], $new['body']['loc']);
+        $this->assertEquals([[0.0, 0.0], [1.0, 1.0]], $new['body']['route']);
+        $this->assertEquals([[[0.0, 0.0], [1.0, 0.0], [1.0, 1.0], [0.0, 1.0], [0.0, 0.0]]], $new['body']['area']);
+
+        // Cleanup
+        $this->client->call(Client::METHOD_DELETE, '/tablesdb/' . $databaseId . '/tables/' . $tableId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));
+        $this->client->call(Client::METHOD_DELETE, '/tablesdb/' . $databaseId, array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]));

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

tests/e2e/Services/Databases/TablesDB/DatabasesBase.php (2)

8831-8959: Strengthen failure assertions and validate existing-row nulls

• Also assert the error message content on 400 to lock the new API validation.
• Optionally, fetch the pre-existing row to assert the new spatial columns are null.

Example patch:

@@
-        $this->assertEquals(400, $point['headers']['status-code']);
+        $this->assertEquals(400, $point['headers']['status-code']);
+        $this->assertStringContainsString('existing rows', $point['body']['message'] ?? '');
@@
-        $this->assertEquals(400, $line['headers']['status-code']);
+        $this->assertEquals(400, $line['headers']['status-code']);
+        $this->assertStringContainsString('existing rows', $line['body']['message'] ?? '');
@@
-        $this->assertEquals(400, $poly['headers']['status-code']);
+        $this->assertEquals(400, $poly['headers']['status-code']);
+        $this->assertStringContainsString('existing rows', $poly['body']['message'] ?? '');
@@
         $this->assertEquals(202, $poly['headers']['status-code']);
+
+        // Optionally verify existing row gets nulls for new columns
+        $existing = $this->client->call(
+            Client::METHOD_GET,
+            '/tablesdb/' . $databaseId . '/tables/' . $tableId . '/rows/' . $row['body']['$id'],
+            array_merge(['content-type' => 'application/json','x-appwrite-project' => $this->getProject()['$id']], $this->getHeaders())
+        );
+        $this->assertEquals(200, $existing['headers']['status-code']);
+        $this->assertArrayHasKey('loc', $existing['body']);
+        $this->assertArrayHasKey('route', $existing['body']);
+        $this->assertArrayHasKey('area', $existing['body']);
+        $this->assertNull($existing['body']['loc']);
+        $this->assertNull($existing['body']['route']);
+        $this->assertNull($existing['body']['area']);

Optionally add cleanup at the end to drop the created table/database to reduce test-suite residue.

---

8961-9094: Defaults test looks good; add one negative case for “required + default”

Pre-validation should still block required spatial column when rows exist, even if a non-null default is provided. Add a quick 400 assertion for that combo to guard against regressions.

Example:

@@
         $this->assertEquals(201, $row['headers']['status-code']);
@@
+        // Required with non-null default should still be rejected when rows exist
+        $requiredPointWithDefault = $this->client->call(Client::METHOD_POST, '/tablesdb/' . $databaseId . '/tables/' . $tableId . '/columns/point', array_merge([
+            'content-type' => 'application/json',
+            'x-appwrite-project' => $this->getProject()['$id'],
+            'x-appwrite-key' => $this->getProject()['apiKey']
+        ]), [
+            'key' => 'loc_required',
+            'required' => true,
+            'default' => [1.0, 1.0]
+        ]);
+        $this->assertEquals(400, $requiredPointWithDefault['headers']['status-code']);

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Attributes/Action.php (1)

369-381: Tighten spatial “existing rows” pre-check and make message context-aware.

Use strict type check, avoid chaining under unary not, and say “attribute” vs “column”. Also reuse $type/$required locals.

Apply:

-            if (
-                !$dbForProject->getAdapter()->getSupportForSpatialIndexNull() &&
-                \in_array($attribute->getAttribute('type'), Database::SPATIAL_TYPES) &&
-                $attribute->getAttribute('required')
-            ) {
-                $hasData = !Authorization::skip(fn () => $dbForProject
-                    ->findOne('database_' . $db->getSequence() . '_collection_' . $collection->getSequence()))
-                    ->isEmpty();
-
-                if ($hasData) {
-                    throw new StructureException('Failed to add required spatial column: existing rows present. Make the column optional.');
-                }
-            }
+            $isSpatial = \in_array($type, Database::SPATIAL_TYPES, true);
+            if (
+                !$dbForProject->getAdapter()->getSupportForSpatialIndexNull()
+                && $isSpatial
+                && $required
+            ) {
+                $rows = 'database_' . $db->getSequence() . '_collection_' . $collection->getSequence();
+                $existing = Authorization::skip(fn () => $dbForProject->findOne($rows));
+                if ($existing instanceof Document && !$existing->isEmpty()) {
+                    $entity = $this->isCollectionsAPI() ? 'attribute' : 'column';
+                    throw new StructureException("Failed to add required spatial $entity: existing rows present. Make the $entity optional.");
+                }
+            }

src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Attributes/Action.php (1)

433-433: Propagate the message here too for parity.

Carry $e->getMessage() like above.

Apply:

-            } catch (StructureException) {
-                throw new Exception($this->getStructureException());
+            } catch (StructureException $e) {
+                throw new Exception($this->getStructureException(), $e->getMessage());

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro