src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Columns/Polygon/Create.php (1)

43-51: Docs path parity with other spatial endpoints

Same note as Point: please ensure the docs page exists or update the reference.

tests/e2e/Services/GraphQL/Base.php (1)

1-3388: Optional: add GraphQL smoke tests for new constants.

Once the cases and fragments are added, consider adding minimal e2e calls using these constants to avoid regression.

Happy to draft a couple of CRUD tests mirroring existing attribute/column tests for one spatial type (e.g., point) as a template for the others.

app/init/database/filters.php (1)

96-99: Explicit no-op for spatial attributes is fine; add intent comment for maintainers

These cases make it explicit that no extra mutations are needed for spatial attributes in this filter. Add a short comment to avoid future “missing logic?” confusion.

Apply this minimal diff:

                 case Database::VAR_STRING:
                     $filters = $attribute->getAttribute('filters', []);
                     $attribute->setAttribute('encrypt', in_array('encrypt', $filters));
                     break;
+                // Spatial attributes: no extra processing required in subQueryAttributes.
                 case Database::VAR_POINT:
                 case Database::VAR_LINESTRING:
                 case Database::VAR_POLYGON:
                     break;

src/Appwrite/Platform/Modules/Databases/Services/Registry/Collections.php (1)

141-152: Spatial attribute actions registered; minor docs consistency follow-up

The action registrations are correct and mirror other attribute types. Note: the corresponding Action classes’ parameter docs for collectionId sometimes mention “create a new table” even under Collections (see Create/Update for Point/Line/Polygon). Consider updating those descriptions to say “collection” for consistency.

I can prepare a small PR patch to normalize those descriptions across the three action classes if helpful.

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

373-379: Avoid undefined index when checking two-way relationships

Accessing $options['twoWay'] and $primaryDocumentOptions['twoWay'] without guarding can raise notices if not set. Use !empty(...) to be safe.

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

-            if ($primaryDocumentOptions['twoWay']) {
+            if (!empty($primaryDocumentOptions['twoWay'])) {

Also applies to: 574-589

src/Appwrite/Utopia/Response/Model/AttributePoint.php (1)

26-33: Consistency check: default value shape

Default is declared as TYPE_JSON here (good for an array like [x, y]). Ensure downstream serializers/SDKs expect JSON for attribute defaults of spatial types. See ColumnPoint/ColumnLine using TYPE_STRING for defaults; consider aligning types across attributes vs columns for consistency, or document the intentional difference.

src/Appwrite/Utopia/Response/Model/ColumnPoint.php (1)

26-33: Consider using TYPE_JSON for 'default' to match the array shape

The example '[0, 0]' represents JSON array coordinates, but the rule uses TYPE_STRING. If clients should receive a JSON array, prefer TYPE_JSON for stronger typing and parity with AttributePoint/Line/Polygon defaults.

-            ->addRule('default', [
-                'type' => self::TYPE_STRING,
+            ->addRule('default', [
+                'type' => self::TYPE_JSON,
                 'description' => 'Default value for column when not provided. Cannot be set when column is required.',
                 'default' => null,
                 'required' => false,
                 'example' => '[0, 0]'
             ])

src/Appwrite/Utopia/Response/Model/ColumnLine.php (1)

26-33: Prefer TYPE_JSON for line default for consistency and typing

Like ColumnPoint, the default represents a JSON array. Switching to TYPE_JSON would better reflect the payload shape and align with attribute models.

-            ->addRule('default', [
-                'type' => self::TYPE_STRING,
+            ->addRule('default', [
+                'type' => self::TYPE_JSON,
                 'description' => 'Default value for column when not provided. Cannot be set when column is required.',
                 'default' => null,
                 'required' => false,
                 'example' => '[[0, 0], [1, 1]]'
             ])

src/Appwrite/Utopia/Response/Model/ColumnPolygon.php (2)

13-33: Polygon default example should be a fully closed ring (5 coordinates) for clarity

Use a canonical rectangle or square with the first/last positions equal to avoid confusion about ring closure. Current example forms a triangle.

Apply this minimal change:

-                'example' => '[[[0, 0], [0, 10], [10, 10], [0, 0]]]'
+                'example' => '[[[0, 0], [0, 10], [10, 10], [10, 0], [0, 0]]]'

---

26-33: Confirm spatial column default rule type consistency
Verified that ColumnPoint, ColumnLine, and ColumnPolygon all declare their default rule with 'type' => self::TYPE_STRING and no use of TYPE_JSON.

• src/Appwrite/Utopia/Response/Model/ColumnPoint.php: ->addRule('default', ['type' => self::TYPE_STRING, …])
• src/Appwrite/Utopia/Response/Model/ColumnLine.php: ->addRule('default', ['type' => self::TYPE_STRING, …])
• src/Appwrite/Utopia/Response/Model/ColumnPolygon.php: ->addRule('default', ['type' => self::TYPE_STRING, …])

If your API contracts return spatial defaults as native JSON arrays rather than encoded strings, you may optionally refactor these to use self::TYPE_JSON for all three models to reflect the actual data type. Otherwise, keeping them as TYPE_STRING is consistent and acceptable.

src/Appwrite/Platform/Modules/Databases/Services/Registry/Tables.php (1)

105-160: Confirm absence of Update endpoints for spatial columns

• Verified that there are no UpdateLine, UpdatePoint, or UpdatePolygon class definitions under src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Columns/Line, …/Point, or …/Polygon, nor are any such updates registered in registerColumnActions() (lines 105–160).
• If spatial-column updates are intentionally unsupported, please file a backlog ticket or update the API reference/docs to explicitly document that behavior.

src/Appwrite/Utopia/Response/Model/AttributePolygon.php (1)

13-33: Nit: make 'type' a string rule for semantic consistency

Base Attribute defines 'type' as a string; Point/Line currently use JSON for 'type'. To align semantics (and Swagger/SDK typing), consider using TYPE_STRING here (and optionally across geometry models) while keeping 'default' as JSON.

Apply within this hunk:

-        ->addRule('type', [
-            'type' => self::TYPE_JSON,
+        ->addRule('type', [
+            'type' => self::TYPE_STRING,
             'description' => 'Attribute type.',
             'default' => '',
             'example' => 'polygon',
         ])

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

61-66: Confirm stance on default for spatial attributes.

Parameter type is Text(0, 0) and action signature uses ?string $default. If defaults for POINT are intentionally unsupported (which is reasonable), consider:

• Explicitly documenting that defaults are not accepted for spatial attributes.
• Or removing the default parameter entirely (like other types did historically) to prevent misuse.

If defaults should be supported later, the input type will likely need to be JSON (array|null) rather than string to better match AttributePoint’s JSON rules.

tests/e2e/Services/Databases/Legacy/DatabasesCustomServerTest.php (4)

6248-6251: Reduce flakiness: poll instead of fixed sleeps for attribute readiness.

Replace sleep(2) with an assertEventually(...) that waits until attributes are available, similar to prior tests in this file. This will stabilize CI on slower machines.

Apply this diff:

-        // Wait for attributes to be created
-        sleep(2);
+        // Wait for attributes to be created
+        $this->assertEventually(function () use ($databaseId, $collectionId) {
+            $attributes = $this->client->call(
+                Client::METHOD_GET,
+                "/databases/{$databaseId}/collections/{$collectionId}/attributes",
+                array_merge(['content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id']], $this->getHeaders())
+            );
+            return ($attributes['headers']['status-code'] ?? 0) === 200
+                && ($attributes['body']['total'] ?? 0) >= 2; // expecting at least name + location (+ polygon)
+        }, 60000, 500);

---

6286-6295: Avoid brittle geometry assertions; normalize once, then assert.

Current checks branch on whether area[0] is an array. Prefer a tiny normalizer (e.g., wrap a ring if not nested) to keep assertions simple and future-proof.

Apply this diff:

-            // Check polygon structure - it might be stored as an array of arrays
-            if (is_array($document['area'][0])) {
-                $this->assertGreaterThan(1, count($document['area'][0])); // POLYGON has multiple points
-            } else {
-                $this->assertGreaterThan(1, count($document['area'])); // POLYGON has multiple points
-            }
+            // Normalize polygon to rings and assert minimum points
+            $rings = is_array($document['area'][0]) && is_array(($document['area'][0])[0])
+                ? $document['area']          // already rings
+                : [$document['area']];       // single ring
+            $this->assertGreaterThan(1, count($rings[0]), 'POLYGON must contain multiple points');

---

6545-6568: Avoid hitting batch size limits implicitly.

Use the configured APP_LIMIT_DATABASE_BATCH instead of hardcoding 100 to keep the test portable across deployments.

Apply this diff:

-        for ($i = 0; $i < 100; $i++) {
+        $batch = defined('APP_LIMIT_DATABASE_BATCH') ? min(APP_LIMIT_DATABASE_BATCH, 100) : 100;
+        for ($i = 0; $i < $batch; $i++) {

---

6569-6581: Cleanup path is correct; consider asserting cascade effects.

You may assert 204s and optionally verify the database is gone to catch cleanup races.

Apply this diff:

-        $this->client->call(Client::METHOD_DELETE, '/databases/' . $databaseId, array_merge([
+        $deletedDb = $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']
         ]));
+        $this->assertEquals(204, $deletedDb['headers']['status-code']);

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

61-66: Default parameter description vs. validator mismatch.

The description suggests clients can send a default value, but the validator new Text(0, 0) effectively disallows any non-empty payload for default (only null/empty). If defaults for spatial types are intentionally unsupported, adjust the description to say “Defaults are not supported for polygon attributes” (and consider omitting the parameter from the public contract in a future major). If defaults should be supported, switch to an appropriate validator (e.g., JSON) and document the expected shape.

Proposed doc-only tweak:

-            ->param('default', null, new Text(0, 0), 'Default value for attribute when not provided. Cannot be set when attribute is required.', true)
+            ->param('default', null, new Text(0, 0), 'Defaults are not supported for polygon attributes and will be ignored.', true)

---

85-87: Status code override is fine; consider a short note in code.

createAttribute() sets 201 internally, then you override to 202 here for async processing semantics. That’s okay and matches other attribute endpoints, but it’s easy to miss. Consider a short inline comment to explain why we force 202.

         $response
-            ->setStatusCode(SwooleResponse::STATUS_CODE_ACCEPTED)
+            // Return 202 to indicate async attribute provisioning (aligns with legacy attributes API)
+            ->setStatusCode(SwooleResponse::STATUS_CODE_ACCEPTED)
             ->dynamic($attribute, $this->getResponseModel());

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

61-66: Default parameter text should match behavior.

Like Polygon/Point, the Text(0, 0) validator prevents non-empty defaults, while the description implies defaults are usable. Either (a) clarify that defaults aren’t supported for line attributes or (b) adopt a validator that accepts a JSON array shape if defaults should be allowed.

-            ->param('default', null, new Text(0, 0), 'Default value for attribute when not provided. Cannot be set when attribute is required.', true)
+            ->param('default', null, new Text(0, 0), 'Defaults are not supported for line attributes and will be ignored.', true)

---

85-87: Optional: add a note on forcing 202.

Same 201→202 override as other attribute endpoints; a brief comment improves maintainability.

         $response
-            ->setStatusCode(SwooleResponse::STATUS_CODE_ACCEPTED)
+            // Force 202 to indicate async provisioning and maintain API parity with other attributes
+            ->setStatusCode(SwooleResponse::STATUS_CODE_ACCEPTED)
             ->dynamic($attribute, $this->getResponseModel());

tests/e2e/Services/Databases/Legacy/DatabasesCustomClientTest.php (6)

1082-1093: Comment contradicts expected status.

The comment says “handle both 201/200,” but the assertion expects 202. Update the comment to reduce confusion.

-        // Create point attribute - handle both 201 (created) and 200 (already exists)
+        // Create point attribute - endpoint returns 202 Accepted after enqueueing attribute creation

---

1135-1147: Upsert requests: path documentId vs body documentId mismatch.

PUT requests include a documentId in the path and also pass a (different) documentId in the body. Depending on server-side validation, this can be rejected or introduce ambiguity. Prefer either:

• Rely on the path-only (omit documentId from body), or
• Ensure the body documentId matches the path id.

Suggest removing the body documentId for clarity.

Proposed changes (Point upsert as example):

-        $response = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $collectionId . '/documents/' . ID::unique(), array_merge([
+        $upsertId = ID::unique();
+        $response = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $collectionId . '/documents/' . $upsertId, array_merge([
             'content-type' => 'application/json',
             'x-appwrite-project' => $this->getProject()['$id'],
         ], $this->getHeaders()), [
-            'documentId' => ID::unique(),
             'data' => [
                 'name' => 'Upserted Location',
                 'location' => [34.0522, -118.2437] // Los Angeles coordinates
             ]
         ]);

Apply the same pattern to the line and polygon upsert calls in the other tests.

Also applies to: 1270-1283, 1407-1420

---

1177-1312: Line tests are comprehensive; consider adding an invalid-shape test.

Great coverage for CRUD and delete/404 verification. Consider an extra negative test: reject a line with fewer than two points, or with non-numeric coordinates, to exercise validator paths.

---

1314-1447: Polygon tests look good; add ring-closure and winding validation cases if applicable.

You already test a closed ring. If the backend enforces ring closure or winding order, add:

• A polygon without a closing point → expect 400.
• A polygon with an empty ring or non-array members → expect 400.

---

1490-1518: Mixed collection coverage is valuable; consider array-typed spatial attributes.

Endpoints accept the “array” flag; adding one attribute with array=true (e.g., multiple polygons) would strengthen coverage for serialization and validation of arrays of geometries.

---

1164-1168: Reduce flakiness: replace fixed sleeps with a polling helper.

Fixed sleeps can be brittle under load. Consider a small polling helper that repeatedly GETs the collection until the attribute status is “available” (with timeout), then proceeds. This will make the tests more deterministic.

Example helper sketch (outside this file):

private function waitForAttributesReady(string $databaseId, string $collectionId, int $timeoutSec = 20): void
{
    $deadline = time() + $timeoutSec;
    do {
        $res = $this->client->call(Client::METHOD_GET, "/databases/{$databaseId}/collections/{$collectionId}", [
            'content-type' => 'application/json',
            'x-appwrite-project' => $this->getProject()['$id'],
            'x-appwrite-key' => $this->getProject()['apiKey']
        ]);
        $ready = 200 === $res['headers']['status-code']
            && array_reduce($res['body']['attributes'] ?? [], fn($ok, $a) => $ok && ($a['status'] ?? '') === 'available', true);
        if ($ready) return;
        usleep(250_000);
    } while (time() < $deadline);
    $this->fail('Attributes not ready before timeout');
}

Then call this instead of sleep().

Also applies to: 1299-1312, 1435-1447, 1590-1601

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: ItzNotABug
PR: appwrite/appwrite#9693
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php:57-59
Timestamp: 2025-06-19T09:20:03.312Z
Learning: In table-related endpoints (such as `src/Appwrite/Platform/Modules/Databases/Http/Databases/Tables/Update.php`), parameter descriptions should use "table" and "row" terminology instead of "collection" and "document" for clarity and consistency.