Skip to content

Force ID as int#11398

Merged
abnegate merged 4 commits into1.8.xfrom
fix-cast
Feb 25, 2026
Merged

Force ID as int#11398
abnegate merged 4 commits into1.8.xfrom
fix-cast

Conversation

@abnegate
Copy link
Copy Markdown
Member

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a43e949 and 7dc50b4.

📒 Files selected for processing (1)
  • src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php

📝 Walkthrough

Walkthrough

The PR changes ID/sequence handling across the codebase: OpenAPI3 and Swagger2 formatters now map the id rule to type integer with a default int32 format. Document and Row models cast numeric $sequence values to integers; Row only sets $sequence for non-empty numeric documents. A new console variable supportForIntegerIds is added to Console variables, its model, and HTTP payload. Tests add a Scope helper to read this flag and conditionally assert $sequence as int or string based on its value.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description contains only the contributor template with uncompleted placeholders and no actual implementation details, rationale, or explanation of the changes made. Complete the PR description by explaining why IDs are being forced to integers, how this affects the codebase, and providing a test plan to verify the changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Force ID as int' directly and clearly describes the main change across all modified files—converting ID handling from string to integer type throughout the SDK specifications and response models.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-cast

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 25, 2026

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libecpg 18.1-r0 CVE-2026-2004 HIGH
libecpg 18.1-r0 CVE-2026-2005 HIGH
libecpg 18.1-r0 CVE-2026-2006 HIGH
libecpg 18.1-r0 CVE-2026-2007 HIGH
libecpg-dev 18.1-r0 CVE-2026-2004 HIGH
libecpg-dev 18.1-r0 CVE-2026-2005 HIGH
libecpg-dev 18.1-r0 CVE-2026-2006 HIGH
libecpg-dev 18.1-r0 CVE-2026-2007 HIGH
libpng 1.6.54-r0 CVE-2026-25646 HIGH
libpng-dev 1.6.54-r0 CVE-2026-25646 HIGH
libpq 18.1-r0 CVE-2026-2004 HIGH
libpq 18.1-r0 CVE-2026-2005 HIGH
libpq 18.1-r0 CVE-2026-2006 HIGH
libpq 18.1-r0 CVE-2026-2007 HIGH
libpq-dev 18.1-r0 CVE-2026-2004 HIGH
libpq-dev 18.1-r0 CVE-2026-2005 HIGH
libpq-dev 18.1-r0 CVE-2026-2006 HIGH
libpq-dev 18.1-r0 CVE-2026-2007 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2004 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2005 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2006 HIGH
postgresql18-dev 18.1-r0 CVE-2026-2007 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/Appwrite/Utopia/Response/Model/Document.php (1)

39-45: ⚠️ Potential issue | 🟡 Minor

Same default/example string-vs-integer mismatch as in Row.php.

'default' => '' and 'example' => '1' should be 0 and 1 (integers) respectively to match the TYPE_ID → integer mapping and the runtime cast introduced on line 87.

🛠️ Proposed fix
             ->addRule('$sequence', [
                 'type' => self::TYPE_ID,
                 'description' => 'Document sequence ID.',
-                'default' => '',
-                'example' => '1',
+                'default' => 0,
+                'example' => 1,
                 'readOnly' => true,
             ])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Appwrite/Utopia/Response/Model/Document.php` around lines 39 - 45, The
$sequence rule in the Document model uses TYPE_ID (an integer type) but sets
'default' => '' and 'example' => '1', causing a string-vs-integer mismatch;
update the addRule entry for '$sequence' in the Document class to use integer
values ('default' => 0 and 'example' => 1) so they match TYPE_ID and the runtime
cast logic that handles sequence IDs.
src/Appwrite/Utopia/Response/Model/Row.php (1)

39-45: ⚠️ Potential issue | 🟡 Minor

default and example values are strings but the field is now integer-typed.

TYPE_ID now maps to integer in both OpenAPI3 and Swagger2 specs, and the filter() method casts $sequence to int. The 'default' => '' and 'example' => '1' (string) values are inconsistent with the actual integer type, which can cause SDK generators to emit incorrect example/default types.

🛠️ Proposed fix
             ->addRule('$sequence', [
                 'type' => self::TYPE_ID,
                 'description' => 'Row sequence ID.',
-                'default' => '',
-                'example' => '1',
+                'default' => 0,
+                'example' => 1,
                 'readOnly' => true,
             ])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Appwrite/Utopia/Response/Model/Row.php` around lines 39 - 45, The default
and example for the '$sequence' rule are strings but the field is integer-typed
(TYPE_ID) and cast to int in the filter() method; update the addRule entry for
'$sequence' to use integer values (e.g., 'default' => 0 and 'example' => 1) so
OpenAPI/Swagger generators see numeric types and examples consistent with
TYPE_ID and the filter() behavior, leaving 'readOnly' and other keys unchanged.
♻️ Duplicate comments (1)
src/Appwrite/SDK/Specification/Format/OpenAPI3.php (1)

845-847: Breaking-change concern already flagged in Swagger2.php applies here equally.

TYPE_IDinteger in the OpenAPI3 component schemas is the same breaking contract change. Marking to avoid a duplicate comment here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php` around lines 845 - 847,
The switch branch mapping TYPE_ID (the case 'id' in OpenAPI3.php) currently
returns 'integer', which duplicates the breaking contract change flagged in
Swagger2; change the mapping back to the non-breaking type (e.g., 'string' or
the previously used representation for TYPE_ID) so component schemas keep the
original contract. Update the case 'id' branch in the OpenAPI3 specification
generator (where TYPE_ID is handled) to emit the original type/format used
before the breaking change and ensure any related schema generation logic
references the same representation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php`:
- Around line 845-847: The OpenAPI3.php switch handling the 'id' type sets $type
= 'integer' but leaves $format null, causing sequence schemas to lack a format;
update the 'id' case (in the same switch where $type and $format are set) to
also assign $format = $rule['format'] ?? 'int32' (mirroring the fix in
Swagger2.php) so that the integer schema for $sequence includes the proper
format.

In `@src/Appwrite/SDK/Specification/Format/Swagger2.php`:
- Around line 825-827: Update the 'id' switch case in Swagger2.php to explicitly
set $format (same pattern as the 'integer' case) so ID fields emit a format;
specifically assign $format = $rule['format'] ?? 'int32' (or 'int64' if your IDs
can exceed 2,147,483,647) next to where $type = 'integer' is set, ensuring the
variables $type, $format and $rule are used consistently when serializing the
schema.

---

Outside diff comments:
In `@src/Appwrite/Utopia/Response/Model/Document.php`:
- Around line 39-45: The $sequence rule in the Document model uses TYPE_ID (an
integer type) but sets 'default' => '' and 'example' => '1', causing a
string-vs-integer mismatch; update the addRule entry for '$sequence' in the
Document class to use integer values ('default' => 0 and 'example' => 1) so they
match TYPE_ID and the runtime cast logic that handles sequence IDs.

In `@src/Appwrite/Utopia/Response/Model/Row.php`:
- Around line 39-45: The default and example for the '$sequence' rule are
strings but the field is integer-typed (TYPE_ID) and cast to int in the filter()
method; update the addRule entry for '$sequence' to use integer values (e.g.,
'default' => 0 and 'example' => 1) so OpenAPI/Swagger generators see numeric
types and examples consistent with TYPE_ID and the filter() behavior, leaving
'readOnly' and other keys unchanged.

---

Duplicate comments:
In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php`:
- Around line 845-847: The switch branch mapping TYPE_ID (the case 'id' in
OpenAPI3.php) currently returns 'integer', which duplicates the breaking
contract change flagged in Swagger2; change the mapping back to the non-breaking
type (e.g., 'string' or the previously used representation for TYPE_ID) so
component schemas keep the original contract. Update the case 'id' branch in the
OpenAPI3 specification generator (where TYPE_ID is handled) to emit the original
type/format used before the breaking change and ensure any related schema
generation logic references the same representation.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac0aa81 and 62d0319.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • src/Appwrite/SDK/Specification/Format/OpenAPI3.php
  • src/Appwrite/SDK/Specification/Format/Swagger2.php
  • src/Appwrite/Utopia/Response/Model/Document.php
  • src/Appwrite/Utopia/Response/Model/Row.php

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 25, 2026

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit 62d0319 - 2 flaky tests
Test Retries Total Time Details
UsageTest::testDatabaseStatsCollectionsAPI 1 10.21s Logs
FunctionsCustomServerTest::testCreateDeploymentFromCLI 1 407ms Logs
Commit a43e949 - 2 flaky tests
Test Retries Total Time Details
UsageTest::testDatabaseStatsCollectionsAPI 1 10.20s Logs
LegacyCustomServerTest::testOperators 1 240.61s Logs
Commit 7dc50b4 - 8 flaky tests
Test Retries Total Time Details
UsageTest::testDatabaseStatsCollectionsAPI 1 10.18s Logs
LegacyConsoleClientTest::testBulkOperators 1 240.33s Logs
LegacyCustomClientTest::testUpsertDocument 1 240.97s Logs
LegacyCustomServerTest::testSpatialRelationshipManyToOne 1 240.19s Logs
LegacyPermissionsGuestTest::testWriteDocument 1 240.39s Logs
TablesDBConsoleClientTest::testNotEndsWith 1 240.64s Logs
TablesDBCustomServerTest::testSpatialColCreateOnExistingData 1 240.45s Logs
LegacyTransactionsConsoleClientTest::testCreateOperations 1 240.71s Logs

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 25, 2026

✨ Benchmark results

  • Requests per second: 1,831
  • Requests with 200 status code: 329,639
  • P99 latency: 0.088427067

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,831 1,172
200 329,639 210,986
P99 0.088427067 0.177117399

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Appwrite/SDK/Specification/Format/OpenAPI3.php`:
- Around line 845-848: The OpenAPI3 specification generator currently sets IDs
to integer with a default format of 'int32' in the case 'id' branch; update that
fallback (in the switch handling for 'id' inside
src/Appwrite/SDK/Specification/Format/OpenAPI3.php) to use 'int64' instead of
'int32' so sequence/auto-increment IDs are represented as 64-bit integers (i.e.,
change the $rule['format'] ?? 'int32' fallback to $rule['format'] ?? 'int64').

In `@src/Appwrite/Utopia/Response/Model/Document.php`:
- Around line 86-87: The code is setting a default $sequence=0 via
getAttribute('$sequence', 0) which then gets materialized for documents that
never had the field; change the check to only act when the attribute actually
exists (e.g. use $document->hasAttribute('$sequence') or call
getAttribute('$sequence') without a default and check !== null), then if the
attribute exists and is numeric cast and call
$document->setAttribute('$sequence', (int)...). Update the block around the
Document handling (the $document variable / methods getAttribute, hasAttribute,
setAttribute) to avoid using the default 0 when determining whether to set the
attribute.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62d0319 and c91252d.

📒 Files selected for processing (8)
  • src/Appwrite/Platform/Modules/Console/Http/Variables/Get.php
  • src/Appwrite/SDK/Specification/Format/OpenAPI3.php
  • src/Appwrite/SDK/Specification/Format/Swagger2.php
  • src/Appwrite/Utopia/Response/Model/ConsoleVariables.php
  • src/Appwrite/Utopia/Response/Model/Document.php
  • src/Appwrite/Utopia/Response/Model/Row.php
  • tests/e2e/Scopes/Scope.php
  • tests/e2e/Services/Databases/DatabasesBase.php
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/Appwrite/Utopia/Response/Model/Row.php
  • src/Appwrite/SDK/Specification/Format/Swagger2.php

@abnegate abnegate merged commit 32ef15f into 1.8.x Feb 25, 2026
81 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant