chore: absorb audit into packages/audit - #13955
Conversation
Update database
Update database
Feat activity adapter
Revert "Feat activity adapter"
Allow passing queries
…feat-pass-queries
Remove parameterised queries
Track latest database
Allow userId nulls
Update lock
chore(audit)!: upgrade to utopia-php/database 7.x
Allow validators 0.4 in dependent packages
Allow validators 0.5 in dependent packages
refactor(audit): migrate the ClickHouse adapter to utopia-php/query 0.6
chore: require validators 0.6
chore: stop committing package lock files
chore(audit): require Validators ^1.0
Point the mirror redirect workflow at appwrite/appwrite, refresh the README banner and drop the dead Travis badge.
Autoload Utopia\Audit from packages/audit and replace utopia-php/audit, so the vendored 4.0.3 copy leaves the lock. utopia-php/fetch is hoisted to the root until the next commit moves the ClickHouse adapter to client. Step A: flat src/, Utopia\Audit\Tests\ over tests/, the MariaDB and ClickHouse suites under tests/E2E/, a standalone level-max phpstan.neon with a 5-finding baseline, and a standalone rector.php.
utopia-php/fetch is archived. The ClickHouse adapter now sends PSR-7 requests through utopia-php/client with the same 30 s timeout, 5 s connect timeout, followed redirects and reused connection, the same auth and database headers, multipart parameter bodies and urlencoded JSONEachRow bodies, and the same status and error handling. The constructor takes an optional PSR-18 client so the request building is unit tested with a recording fake. Nothing requires utopia-php/fetch any more, so it leaves the root require and the lock.
ChiragAgg5k
added
the
absorb
🟢 Tier S · Ready to merge
Absorbs Utopia Audit into Latest changes: The new commits import Platform's source and tests, add its mirror and package tooling, switch root Composer autoloading and replacement to the local package, and record its PHPStan baseline in the monorepo RFC.
📂 Walkthrough · 9
Reviewed the commits since |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| $sql = $table->createIfNotExists()->query; | ||
|
|
||
| $this->assertStringContainsString('CREATE TABLE IF NOT EXISTS `default`.`audits`', $sql); | ||
| $this->assertStringContainsString('`id` String', $sql); | ||
| $this->assertStringContainsString('`actorId` Nullable(String)', $sql); |
There was a problem hiding this comment.
ClickHouse::setup() would not fail these tests, while a harmless SQL-formatting change could. The repository requires tests of observable behavior rather than assertions that mirror source code or configuration. The exact COUNT SQL assertion in tests/Adapter/ClickHouseTest.php and reflection-based column-definition assertions in the E2E test follow the same pattern. This repository requirement must be satisfied before merging.
Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/audit/tests/Adapter/ClickHouseSqlSnapshotTest.php
Line: 100-104
Comment:
**Tests mirror SQL implementation** The new tests rebuild query-builder calls without invoking the audit adapter, then assert exact SQL text. As the test's own comment notes, a wrong argument in `ClickHouse::setup()` would not fail these tests, while a harmless SQL-formatting change could. The repository requires tests of observable behavior rather than assertions that mirror source code or configuration. The exact COUNT SQL assertion in `tests/Adapter/ClickHouseTest.php` and reflection-based column-definition assertions in the E2E test follow the same pattern. This repository requirement must be satisfied before merging.
**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| - MYSQL_ROOT_PASSWORD=password | ||
| ports: | ||
| - "13307:3306" | ||
| healthcheck: | ||
| test: ["CMD", "sh", "-c", "mysqladmin ping -h localhost -u root -p$$MYSQL_ROOT_PASSWORD"] |
There was a problem hiding this comment.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/audit/docker-compose.yml
Line: 5-9
Comment:
**Test databases exposed beyond localhost** If the E2E services run on a network-reachable host, these port mappings expose MariaDB root and the ClickHouse default user through host interfaces beyond localhost, using passwords fixed in this file. The tests connect locally, so restricting the published ports to loopback would preserve test access without exposing those accounts to other hosts. **How this was verified:** Both services publish host ports without a loopback address and configure known account passwords.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (! $database->exists('utopiaTests')) { | ||
| $database->create(); | ||
| $this->audit->setup(); | ||
| } |
There was a problem hiding this comment.
utopiaTests already exists without its audit collection, this branch skips Audit::setup(). The shared fixture then immediately cleans up and writes logs, so the E2E suite fails instead of creating the collection it needs. Run the collection setup even when the database already exists.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/audit/tests/E2E/Adapter/DatabaseTest.php
Line: 39-42
Comment:
**Existing database skips audit setup** If `utopiaTests` already exists without its audit collection, this branch skips `Audit::setup()`. The shared fixture then immediately cleans up and writes logs, so the E2E suite fails instead of creating the collection it needs. Run the collection setup even when the database already exists.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
|
||
| ## System requirements | ||
|
|
||
| Utopia Framework requires PHP 8.0 or later. We recommend using the latest PHP version whenever possible. |
There was a problem hiding this comment.
| Utopia Framework requires PHP 8.0 or later. We recommend using the latest PHP version whenever possible. | |
| Utopia Audit requires PHP 8.5 or later. We recommend using the latest PHP version whenever possible. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/audit/README.md
Line: 459
Comment:
**PHP requirement is misstated** This says PHP 8.0 is supported, but the package's Composer manifest requires PHP 8.5 or later. Readers on PHP 8.0–8.4 who follow the installation instructions cannot install it.
```suggestion
Utopia Audit requires PHP 8.5 or later. We recommend using the latest PHP version whenever possible.
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| // Store a single log entry and return a Log object | ||
| } | ||
|
|
||
| public function createBatch(array $logs): array |
There was a problem hiding this comment.
createBatch(array $logs): array, but the abstract adapter requires a bool return. Copying this example creates an incompatible method override, so the custom adapter cannot be loaded.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/audit/README.md
Line: 357
Comment:
**Custom adapter example cannot load** The example declares `createBatch(array $logs): array`, but the abstract adapter requires a `bool` return. Copying this example creates an incompatible method override, so the custom adapter cannot be loaded.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after)
|
# Conflicts: # composer.lock
![hansi-codes[bot]](https://avatars.githubusercontent.com/in/5046327?s=60&v=4){kind=small}
Syncing with main regenerated the lock from main's copy, which still lists fetch; nothing requires it once audit uses client.
| } | ||
|
|
||
| // ClickHouse identifiers: alphanumeric, underscores, cannot start with number | ||
| if (!preg_match('/^[a-zA-Z_]\w*$/', $identifier)) { |
There was a problem hiding this comment.
preg_match() to validate identifiers. The repository prohibits adding regular expressions and calls for string or character checks, or an existing validator, instead. This requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/audit/src/Adapter/ClickHouse.php
Line: 220
Comment:
**Prohibited regex in identifier validation** The imported ClickHouse adapter uses `preg_match()` to validate identifiers. The repository prohibits adding regular expressions and calls for string or character checks, or an existing validator, instead. This requirement must be satisfied before merging.
**Context Used:** AGENTS.md ([source](https://github.com/appwrite/appwrite/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
utopia-php/monorepo#206 prepared audit 5.0.0 for utopia-php/database 8: the adapters read attributes and indexes as typed Attribute and Index models, and the legacy TYPE_* constants come from Method. Main absorbed audit into packages/audit (#13955) and the monorepo closed #206 in favour of a change here, so this applies #206's candidate (mirror head f8422f9cbf) to the package, line for line, with its tests. Main's utopia-php/client transport stays; the train only touched the schema code around it. getAttribute() now declares ?Attribute, so its not-found branch returns null explicitly, and two baseline entries go with the errors the port removes. The package requires database ^8.0 with dev stability until 8.0.0 is tagged, because bin/monorepo check and test resolve it from Packagist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Moves
utopia-php/auditintopackages/audit, so Appwrite loads it directly. Wave 4 of #13828, followingrfc/monorepo.md. The mirror head is the4.0.3release Appwrite already locks (bc9fa153), so there is no version gap.Add 'packages/audit/' from commit 'bc9fa153…'chore(audit): mirror plumbingmirror.ymlpoints at this repository'smirror-redirect.yml; README banner refreshed; dead Travis badge dropped. No banned filesrefactor: load audit from packages/replaceentries; dropsutopia-php/audit: ^4.0; hoistsutopia-php/fetchfor one commit; step A (below); PHPStan baseline; RFC phase 8 bulletrefactor(audit): replace utopia-php/fetch with clientstyle(audit): strip trailing whitespace from the READMEgit diff --checkStep A
src/Audit/*,Utopia\Audit\→src/Auditsrc/*,Utopia\Audit\→src/tests/Audit/{QueryTest,Adapter/ClickHouseSqlSnapshotTest},Utopia\Tests\tests/*,Utopia\Audit\Tests\tests/Audit/{AuditBase,Adapter/ClickHouseTest,Adapter/DatabaseTest}tests/E2E/,Utopia\Audit\Tests\E2E\(MariaDB and ClickHouse from the package compose file)phpunit.xmltestsminustests/E2E; e2e istests/E2Ephpstan.neon../../phpstan.neonsrcandtestsrector.phpwithPhpSets()+typeDeclarationsApart from commit 4,
src/is the release byte for byte except the path move and Pint (fn (spacing, empty constructor bodies, andsimplified_null_returnturningSQL::getAttribute()'sreturn null;intoreturn;, which is the same value). Rector changes nothing, sorector.phpneeds no skips. Nothing new to hoist:databaseis in the rootrequire, andquery,validators,clientandpsr7are inpackages/.fetch → client
Adapter\ClickHousewas the onlyUtopia\Fetchuser. It now sends PSR-7 requests throughUtopia\Client.new Client()held on the adapter, one reused cURL handlenew Client(new CurlAdapter())->withConnectionReuse()setTimeout(30_000)withTimeout(30)withFollowRedirects(), max 50 (see below)addHeader('X-ClickHouse-User' / 'X-ClickHouse-Key')on the clientaddHeader('X-ClickHouse-Database')before each queryfetch(…, method: GET)for/pingRequestFactory::createRequest(Method::GET, …)RequestFactory::multipart(Method::POST, …, $fields)application/x-www-form-urlencodedRequestFactory::body(…, ContentType::FORM_URLENCODED)getBody()(string) getBody()Utopia\Fetch\Exception(an\Exception) caught and wrappedUtopia\Client\Exception\*(also\Exception), caught and wrapped the same wayStatus handling is unchanged: non-200 throws
ClickHouse query failed with HTTP {status}: {body}, wrapped asClickHouse query execution failed: …;ping()returnsfalseon any throwable or non-200.I compared both clients against a header-echo server: same method, URL, auth headers, multipart fields, raw body and content type, and the same
Expect: 100-continuebehaviour on bodies over 1 MB. Remaining differences:Accept-EncodingAccept-Encodingadvertised, decoded transparently (ClickHouse only compresses withenable_http_compression=1)ping()after a queryX-ClickHouse-Database/pingignores them)The 5-hop cap does not carry over because
withFollowRedirects(maxHops:)is only onpackages/clienthead (#13910), not in the releasedclient0.5.1 that the package's registry-resolved job installs. The package requiresutopia-php/client: ^0.5; once 0.6.0 is out the cap can come back withmaxHops: 5.The constructor gains an optional trailing
?ClientInterface $client = null(as the agents adapters do), so the request building is unit tested with a named recording fake (tests/Adapter/Client.php): ping, https, multipart parameters, JSONEachRow insert, error status, and network errors. No reflection.Lock
utopia-php/auditutopia-php/fetchNothing else in the lock changes. With audit gone nothing in Appwrite's dependency graph requires
utopia-php/fetch, which completes the fetch replacement from #13893. The one remainingUtopia\Fetchuser in the tree ispackages/emails/import.php, a dev-only list importer that installsutopia-php/fetchthroughpackages/emails' ownrequire-dev; it is untouched here.Validation
bin/monorepo validatebin/monorepo check auditbin/monorepo test audit(registry-resolved:client0.5.1,psr70.2.2), unit tierbin/monorepo test audit, e2e tierQuery::contains()phpunit --testsuite packagesinappwrite/appwrite:2.3.0agentsDiffChecktests that need a git checkout the container cannot see from a worktree, andclient'sSwooleCoroutinereused-connection test (Connection reset by peer); this PR changes neither packageUtopia\AuditUtopia\Audit\Adapter\ClickHouse→packages/audit/src/Adapter/ClickHouse.php;vendor/utopia-php/auditandvendor/utopia-php/fetchgonecomposer validate --no-check-publishutopia-php/platformexact-constraint warning only)bin/monorepo split audit --dry-run635db709, fast-forwards from the mirror headbc9fa153git diff --checkgit grep 'Utopia\\Fetch'Appwrite\Utopia\Fetch\BodyMultipartandpackages/emails/import.phpBaseline: 5 findings. 2 in
src:Log::getData()returns the decodedmixedarray against itsarray<string, mixed>docblock, and Pint'sreturn;in the untypedSQL::getAttribute(). 3 in the e2e tests: batch fixtures typed as plain arrays againstlogBatch()'s event shape. Listed under phase 8 in the RFC.Merge and follow-up
Merge with a merge commit. A squash drops the
git-subtree-*annotation and orphans the package from its mirror.The mirror already has the canonical
mainruleset (id18922505: no deletion, no force push, PRs required; split app4016286always bypasses), so the absorb ran with--skip-ruleset. It also has classic branch protection onmain(enforce_adminson, 1 approving review, no required checks) whose PR-review bypass allowances include the split app. Left as is.After merge:
Splitpushes toutopia-php/auditpackages/auditfromutopia-php/monorepoclient0.6.0maxHops: 5emailspackages/emails/import.phpoffutopia-php/fetchNo open issues. Nothing is ported here. Anything still wanted gets re-opened against
packages/audit/src/(paths lost theAudit/segment).🤖 Generated with Claude Code