Feat: Implement AppwriteTablesDB adapter #105
Conversation
WalkthroughThis pull request adds an Appwrite dependency and a new Appwrite TablesDB adapter for abuse/time-limit tracking. Composer updates constrain utopia-php/database to 3.. and add appwrite/appwrite 18... A new adapter class TablesDB implements per-key, per-time-slot counting and lifecycle setup against Appwrite TablesDB. Tests and CI are updated: a new TablesDB integration test, reduced request thresholds in abuse tests, a .env.example file, CI step to generate .env from secrets, and docker-compose/ .gitignore changes to surface and ignore Appwrite-related environment variables. README includes an example for the new adapter. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/Abuse/Adapters/TimeLimit/AppwriteTablesDB.php (3)
15-35: Remove redundant$countproperty; parentTimeLimitalready defines it.
TimeLimitalready hasprotected ?int $count = null;, so redeclaring it here is unnecessary and risks divergence if the parent type or visibility changes. Rely on the inherited property instead.- protected TablesDB $tablesDB; - protected string $databaseId; - protected ?int $count = null; + protected TablesDB $tablesDB; + protected string $databaseId;
106-129: Consider a small safety improvement inwaitForResourcesReady().The polling loop is fine, but it might be safer to log or expose the last non‑available resource state when the max attempts are exceeded, to aid debugging when schema setup fails in CI or production. Not required for correctness.
290-301: Add a safety cap to thecleanup()delete loop.The
do/while ($response['total'] > 0)loop is reasonable iftotalreflects the number of remaining matching rows, but if delete semantics change ortotalis misinterpreted, this could theoretically spin for a long time. Adding a max-iterations guard (and maybe logging) would make this more robust without changing normal behavior.tests/Abuse/AppwriteTablesDBTest.php (1)
20-23: Consider cleaning up the temporary test database after the suite.Each run creates a new database ID (
abuse-cicd-<uniqid()>) buttearDownAfterClass()is empty, so these databases will accumulate over time in the target Appwrite project. It’s worth deleting the test database at the end of the suite (guarded by try/catch) to avoid resource leakage.When adding cleanup, ensure the API key used has
databases.write/tables.write/rows.writescopes so that database deletion succeeds.Also applies to: 31-32, 40-42
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
composer.json(1 hunks)src/Abuse/Adapters/TimeLimit/AppwriteTablesDB.php(1 hunks)tests/Abuse/AppwriteTablesDBTest.php(1 hunks)tests/Abuse/Base.php(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/Abuse/Base.php (6)
tests/Abuse/AppwriteTablesDBTest.php (1)
getAdapter(35-38)tests/Abuse/RedisClusterTest.php (1)
getAdapter(36-39)tests/Abuse/RedisTest.php (1)
getAdapter(34-37)tests/Abuse/DatabaseTest.php (1)
getAdapter(48-51)src/Abuse/Adapter.php (1)
setParam(35-40)src/Abuse/Abuse.php (1)
Abuse(5-56)
tests/Abuse/AppwriteTablesDBTest.php (3)
src/Abuse/Adapters/TimeLimit.php (2)
TimeLimit(8-105)limit(89-92)src/Abuse/Adapters/TimeLimit/AppwriteTablesDB.php (2)
AppwriteTablesDB(15-307)setup(40-53)tests/Abuse/Base.php (2)
Base(9-99)getAdapter(11-11)
src/Abuse/Adapters/TimeLimit/AppwriteTablesDB.php (2)
src/Abuse/Abuse.php (1)
Abuse(5-56)src/Abuse/Adapters/TimeLimit.php (3)
TimeLimit(8-105)limit(89-92)time(101-104)
🔇 Additional comments (4)
src/Abuse/Adapters/TimeLimit/AppwriteTablesDB.php (2)
161-189:count()/hit()logic is consistent withTimeLimitexpectations.Given
TimeLimit::check()always callscount()beforehit()for the same key/timestamp, the caching via$this->countand the use ofincrementRowColumn()are coherent. The optimistic create‑then‑increment with arow_already_existsretry also matches the TablesDB semantics. No changes needed here.Please just confirm that
incrementRowColumn()is available and behaves atomically in appwrite/appwrite 18.x for TablesDB integer columns.Also applies to: 198-251
65-79: Exception type strings verified as correct.All exception
typestrings referenced inexecuteWithSilentErrorcalls match Appwrite PHP SDK 18.x:
database_already_exists✓table_already_exists✓column_already_exists✓index_already_exists✓row_already_exists✓ (confirmed correct)The code safely masks expected idempotent operation failures without risk of silencing unexpected errors.
tests/Abuse/Base.php (1)
53-87: Lowered fast‑request limits make tests lighter while preserving behavior.Updating the comment, limits, and loop counts to 10 keeps the semantics of the “fast requests” and “limit reset” tests intact while making them cheaper and faster to run. Looks good.
composer.json (1)
25-26: Dependencies verified and properly specified.The Appwrite PHP SDK version 18.x is compatible with Appwrite server 1.8.x, which introduces TablesDB, a major update to Appwrite's structured data handling with relational concepts. The utopia-php/database 3.x series is stable and supports the required PHP 8.0+ requirement. Both version constraints follow semantic versioning correctly and allow safe patch and minor updates within their major versions. The composer.json structure is valid, and dependencies are properly aligned with the TablesDB adapter integration.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/Abuse/Adapters/TimeLimit/Appwrite/TablesDB.php (1)
264-279: FixgetLogs()ordering and response key to match TablesDB API
getLogs()currently usesQuery::orderDesc('')and reads$response['documents'], while other methods in this class (and TablesDB docs) work with$response['rows']. This will break at runtime.You should:
- Sort by a real column (your
timecolumn is a good fit).- Map over
$response['rows']to buildDocumentinstances.public function getLogs(?int $offset = null, ?int $limit = 25): array { $queries = []; - $queries[] = Query::orderDesc(''); + $queries[] = Query::orderDesc('time'); @@ - $response = $this->tablesDB->listRows($this->databaseId, self::TABLE_ID, $queries); - - return \array_map(fn ($document) => new Document($document), $response['documents']); + $response = $this->tablesDB->listRows($this->databaseId, self::TABLE_ID, $queries); + + return \array_map( + static fn (array $row) => new Document($row), + $response['rows'] + ); }This aligns
getLogs()with howlistRows()is used elsewhere in the adapter.
🧹 Nitpick comments (1)
.env.example (1)
1-3: Optionally align .env.example with dotenv-linter (ordering + trailing newline)The file works as-is, but to satisfy the reported
EndingBlankLineandUnorderedKeywarnings you can:
- Add a blank line at the end of the file.
- Reorder keys alphabetically (e.g.
APPWRITE_API_KEY,APPWRITE_ENDPOINT,APPWRITE_PROJECT_ID).-APPWRITE_ENDPOINT= -APPWRITE_PROJECT_ID= -APPWRITE_API_KEY= +APPWRITE_API_KEY= +APPWRITE_ENDPOINT= +APPWRITE_PROJECT_ID= +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.env.example(1 hunks).github/workflows/tests.yml(1 hunks).gitignore(1 hunks)docker-compose.yml(1 hunks)src/Abuse/Adapters/TimeLimit/Appwrite/TablesDB.php(1 hunks)tests/Abuse/Appwrite/TablesDBTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/Abuse/Appwrite/TablesDBTest.php (2)
src/Abuse/Adapters/TimeLimit.php (2)
TimeLimit(8-105)limit(89-92)src/Abuse/Adapters/TimeLimit/Appwrite/TablesDB.php (2)
TablesDB(15-307)setup(40-53)
src/Abuse/Adapters/TimeLimit/Appwrite/TablesDB.php (2)
src/Abuse/Abuse.php (1)
Abuse(5-56)src/Abuse/Adapters/TimeLimit.php (3)
TimeLimit(8-105)limit(89-92)time(101-104)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 3-3: [UnorderedKey] The APPWRITE_API_KEY key should go before the APPWRITE_ENDPOINT key
(UnorderedKey)
🪛 GitHub Actions: CodeQL
tests/Abuse/Appwrite/TablesDBTest.php
[error] 27-29: PHPStan: Parameter #1 $endpoint of Appwrite\Client::setEndpoint() expects string, string|false given; Parameter #1 $value of Appwrite\Client::setProject() expects string, string|false given; Parameter #1 $value of Appwrite\Client::setKey() expects string, string|false given.
⏰ 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). (1)
- GitHub Check: Tests (8.1)
🔇 Additional comments (4)
docker-compose.yml (1)
26-29: APPWRITE env passthrough for tests container looks correctPassing
APPWRITE_ENDPOINT,APPWRITE_PROJECT_ID, andAPPWRITE_API_KEYthrough to thetestsservice aligns with the new.env/CI setup and should make them visible viagetenv()inside the container..gitignore (1)
2-3: Ignoring .env is appropriate for secret-bearing configAdding
.envto.gitignorematches the new CI/env-based configuration and helps avoid accidentally committing Appwrite credentials..github/workflows/tests.yml (1)
20-24: .env generation step wires Appwrite secrets into docker-compose correctlyWriting
APPWRITE_ENDPOINT,APPWRITE_PROJECT_ID, andAPPWRITE_API_KEYfrom GitHub Secrets into.envbeforedocker compose build/upis consistent with the compose + tests setup and should make these values available inside thetestscontainer.Just ensure the three secrets are defined in the repo’s Actions secrets; otherwise
.envwill contain empty values and the Appwrite-based tests will likely fail or need to be skipped.src/Abuse/Adapters/TimeLimit/Appwrite/TablesDB.php (1)
23-25: Remove redundant$countproperty; it's already defined in TimeLimit
TimeLimitalready declaresprotected ?int $count = null;at line 15. While PHP allows child classes to shadow parent properties, this creates unnecessary duplication. The inherited property is already used throughout the class (incount()andhit()methods), so this redeclaration is redundant.Remove it:
- protected TablesDBService $tablesDB; - protected string $databaseId; - protected ?int $count = null; + protected TablesDBService $tablesDB; + protected string $databaseId;
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
9-9: Update dependency claim to reflect optional adapter dependencies.The README states the library is "dependency free" on line 9, but the new Appwrite adapter introduces an optional Appwrite dependency. This creates a documentation inconsistency. The statement should clarify that the core library is dependency-free, while some adapters require optional dependencies.
Consider updating line 9 to something like:
-Although this library is part of the [Utopia Framework](https://github.com/utopia-php/framework) project it is dependency free, and can be used as standalone with any other PHP project or framework. +Although this library is part of the [Utopia Framework](https://github.com/utopia-php/framework) project it has no required dependencies by default, and can be used as standalone with any other PHP project or framework. Some adapters may require optional dependencies.
🧹 Nitpick comments (1)
README.md (1)
71-101: Appwrite TablesDB adapter example is clear and consistent.The example follows the same pattern as the MySQL adapter (setup → create adapter → setup → setParam → check), maintains code clarity with helpful comments, and correctly uses the new adapter API.
One minor suggestion: consider adding a brief comment or note about what
$adapter->setup()does for the Appwrite adapter (e.g., whether it creates collections automatically), similar to how the MySQL section references the schema file. This would help users understand any prerequisites or side effects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)tests/Abuse/Appwrite/TablesDBTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Abuse/Appwrite/TablesDBTest.php
🔇 Additional comments (1)
README.md (1)
24-25: Documentation structure improvement approved.Good use of subsection heading to organize the adapter examples.
Summary by CodeRabbit
New Features
Tests
Chores
Documentation