Skip to content

Update db#10664

Merged
abnegate merged 6 commits into1.8.xfrom
feat-update-db
Oct 20, 2025
Merged

Update db#10664
abnegate merged 6 commits into1.8.xfrom
feat-update-db

Conversation

@abnegate
Copy link
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
Contributor

coderabbitai bot commented Oct 17, 2025

📝 Walkthrough

Walkthrough

Bumps the utopia-php/database dependency in composer.json from "2." to "3.". Reworks index validation in Create.php to pass collection indexes (not attributes), use the adapter's max index length, and supply expanded adapter capability flags (index array, spatial null/order, vectors, attributes, multiple fulltext, identical indexes, internal keys). Replaces Database::ARRAY_INDEX_LENGTH with Database::MAX_ARRAY_INDEX_LENGTH. Updates tests to expect the new constant and adjusts some expected error message text/casing. No other functional surface changes reported.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Multiple heterogeneous changes: dependency major-version bump, constructor/signature change with many new validation flags, constant rename across runtime and tests, and test expectation edits. Review should cover adapter API compatibility, validator semantics, and test correctness.

Possibly related PRs

Suggested reviewers

  • christyjacob4
  • loks0n

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description contains only the repository's contribution template with placeholder sections and no substantive content. All key sections—"What does this PR do?", "Test Plan", "Related PRs and Issues", and the checklist—remain unfilled with only template boilerplate and contributor guidance links. This provides no information about what the PR actually does, why it was necessary, how it was tested, or what issues it relates to. Fill out the contribution template with actual details about the PR. At minimum, provide a description of what is being changed (the utopia-php/database version bump and related code updates), explain the motivation for these changes, describe the testing performed, and note any related issues or PRs. This information is essential for reviewers to understand the scope and rationale of the changes.
Title Check ❓ Inconclusive The pull request title "Update db" is extremely vague and generic, failing to convey any meaningful information about the changeset. While the title is technically related to the changes (which do involve database updates), it provides no specificity about what aspect of the database is being updated—such as the major version bump to the utopia-php/database dependency, the changes to the IndexValidator implementation, or the test updates. The title does not give a reviewer scanning the history any clear understanding of the primary change. Consider revising the title to be more specific and descriptive. For example, "Upgrade utopia-php/database to version 3 and update IndexValidator" or "Update database dependency and index validation logic" would better communicate the primary changes in this PR and help maintainers understand the scope at a glance.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-update-db

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d23f993 and caa699e.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • tests/e2e/Services/Databases/Legacy/DatabasesBase.php (3 hunks)
  • tests/e2e/Services/Databases/TablesDB/DatabasesBase.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/e2e/Services/Databases/Legacy/DatabasesBase.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Benchmark
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (1)
tests/e2e/Services/Databases/TablesDB/DatabasesBase.php (1)

1284-1284: Assertion now matches runtime message.

Lowercasing “fulltext” brings the expectation in line with the new dependency’s error string. Looks good to me.


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

github-actions bot commented Oct 17, 2025

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
binutils 2.44-r2 CVE-2025-5244 HIGH
binutils 2.44-r2 CVE-2025-5245 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
pcre2 10.43-r1 CVE-2025-58050 CRITICAL
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
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: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 07d55d1 and 11042af.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • composer.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan

@github-actions
Copy link

github-actions bot commented Oct 17, 2025

✨ Benchmark results

  • Requests per second: 1,138
  • Requests with 200 status code: 204,918
  • P99 latency: 0.173091458

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,138 940
200 204,918 169,202
P99 0.173091458 0.225887968

Copy link

@Aglowharmony Aglowharmony left a comment

Choose a reason for hiding this comment

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

need some cleaning

Copy link
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: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11042af and 5686512.

⛔ Files ignored due to path filters (1)
  • composer.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • composer.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan

Copy link
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: 0

🧹 Nitpick comments (1)
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Indexes/Create.php (1)

74-75: If vector indexes are supported, expand type whitelist accordingly.

New adapter flag getSupportForVectors() suggests vector indexes may be available, but 'type' whitelist excludes a vector type. If v3 defines Database::INDEX_VECTOR, expose it here; otherwise users can’t create vector indexes via this endpoint.

If confirmed, consider:

-            ->param('type', null, new WhiteList([Database::INDEX_KEY, Database::INDEX_FULLTEXT, Database::INDEX_UNIQUE, Database::INDEX_SPATIAL]), 'Index type.')
+            ->param('type', null, new WhiteList(
+                (function () {
+                    $types = [Database::INDEX_KEY, Database::INDEX_FULLTEXT, Database::INDEX_UNIQUE, Database::INDEX_SPATIAL];
+                    if (defined('Utopia\\Database\\Database::INDEX_VECTOR')) {
+                        $types[] = constant('Utopia\\Database\\Database::INDEX_VECTOR');
+                    }
+                    return $types;
+                })(),
+                false,
+                Database::VAR_STRING
+            ), 'Index type.')

Use the verification script above to confirm whether INDEX_VECTOR exists in v3.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5686512 and d23f993.

📒 Files selected for processing (2)
  • src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Indexes/Create.php (3 hunks)
  • tests/e2e/Services/Databases/Legacy/DatabasesBase.php (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Health)
  • GitHub Check: E2E Service Test (Account)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (GraphQL)
  • GitHub Check: E2E Service Test (Console)
  • GitHub Check: E2E Service Test (Avatars)
  • GitHub Check: E2E Shared Mode Service Test (Dev Keys) (Shared V2)
  • GitHub Check: E2E Shared Mode Service Test (Dev Keys) (Shared V1)
  • GitHub Check: E2E Shared Mode Service Test (Site Screenshots) (Shared V1)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: E2E Shared Mode Service Test (Site Screenshots) (Shared V2)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: Unit Test
  • GitHub Check: E2E General Test
  • GitHub Check: scan
🔇 Additional comments (3)
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Indexes/Create.php (3)

163-167: Non-functional: comment tweak is fine.

No behavior change; nothing to action.


169-176: Array attributes: length cap set to MAX_ARRAY_INDEX_LENGTH.

Good catch on the constant rename; setting order to null for array attributes remains consistent.

Please confirm Database::MAX_ARRAY_INDEX_LENGTH exists in utopia-php/database v3 to avoid runtime errors.


193-205: Parameter order and system attributes handling verified as correct; review concerns are unfounded.

The IndexValidator v3.0.0 constructor signature matches exactly with the 11 parameters passed in the code. All arguments are in the correct order and types.

Regarding system attributes: the validator's constructor automatically enriches the provided attributes with Database::INTERNAL_ATTRIBUTES (which includes $id, $createdAt, $updatedAt). Passing only $collection->getAttribute('attributes') is the correct approach—the validator handles system fields internally.

The $adapter refactor suggestion is valid but optional; it's a minor style improvement to reduce repeated method calls, not a correctness issue.

@abnegate abnegate merged commit e2d3624 into 1.8.x Oct 20, 2025
155 of 162 checks passed
@abnegate abnegate deleted the feat-update-db branch October 20, 2025 14:11
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.

2 participants