perf: simplify repository authorized checks by hmacr · Pull Request #11616 · appwrite/appwrite

hmacr · 2026-03-23T16:01:37Z

Closes SER-1170

greptile-apps · 2026-03-23T16:03:14Z

Greptile Summary

This PR is a performance optimization for the VCS repository authorization check. It upgrades utopia-php/vcs from 2.* to 3.* and uses the new hasAccessToAllRepositories() method to avoid an unnecessary getInstallationRepository() API call when the GitHub App installation already has access to all repositories.

Get.php: The authorization logic now short-circuits with hasAccessToAllRepositories() before falling back to the per-repository check via getInstallationRepository(). This is logically correct — if the app has blanket access, no additional API call is needed.
composer.json: Version constraint for utopia-php/vcs bumped from 2.* to 3.*.
composer.lock: Reflects the new version 3.0.1 and adds a transitive dependency on utopia-php/fetch 0.5.*. Notably, the plugin-api-version field was downgraded from 2.9.0 to 2.6.0, suggesting the lockfile was regenerated with a different version of Composer than the base branch.

Confidence Score: 4/5

Safe to merge; the logic change is minimal and correct, with the only minor concern being a Composer version mismatch in the lockfile.
The core logic change is straightforward and correct — skipping an API call when the app already has full access is sound. The main flag is the plugin-api-version downgrade in composer.lock (2.9.0 → 2.6.0), which suggests the lockfile was produced with a different Composer version than CI/the team uses; this does not affect runtime but may cause tooling inconsistencies.
composer.lock — verify the plugin-api-version downgrade is intentional and the lockfile was regenerated with the correct Composer version.

Important Files Changed

Filename	Overview
src/Appwrite/Platform/Modules/VCS/Http/Installations/Repositories/Get.php	Short-circuits the authorized check by calling `hasAccessToAllRepositories()` first; skips the `getInstallationRepository()` API call when the GitHub App has broad access, reducing unnecessary network calls.
composer.json	Bumps `utopia-php/vcs` from `2.` to `3.` to bring in the `hasAccessToAllRepositories()` method.
composer.lock	Lockfile updated to `utopia-php/vcs` 3.0.1 with a new dependency on `utopia-php/fetch 0.5.*`; `plugin-api-version` downgraded from 2.9.0 to 2.6.0, suggesting the lockfile was regenerated with a different Composer version.

_{Reviews (1): Last reviewed commit: "perf: simplify repository authorized che..." | Re-trigger Greptile}

coderabbitai · 2026-03-23T16:04:03Z

📝 Walkthrough

Walkthrough

The pull request updates the utopia-php/vcs dependency constraint in composer.json from 2.* to 3.*. In Get.php, authorization now first checks hasAccessToAllRepositories() and only if that fails attempts an installation-scoped getInstallationRepository() call, treating RepositoryNotFound as unauthorized. In XList.php, the searchRepositories call was changed to pass $owner as the first argument instead of $providerInstallationId.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'perf: simplify repository authorized checks' accurately reflects the main change: refactoring authorization logic in repository-related code to be simpler and more efficient.
Description check	✅ Passed	The description 'Closes SER-1170' is related to the changeset as it references a specific issue ticket that the PR is meant to address.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ser-1170

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 · 2026-03-23T16:11:18Z

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit e3538d0 - 3 flaky tests

Test	Retries	Total Time	Details
`UsageTest::testFunctionsStats`	1	10.13s	Logs
`UsageTest::testPrepareSitesStats`	1	8ms	Logs
`FunctionsScheduleTest::testCreateScheduledAtExecution`	1	131.09s	Logs

Commit bc2c4d0 - 3 flaky tests

Test	Retries	Total Time	Details
`UsageTest::testFunctionsStats`	1	10.21s	Logs
`UsageTest::testPrepareSitesStats`	1	8ms	Logs
`TablesDBCustomClientTest::testNotContains`	1	240.15s	Logs

github-actions · 2026-03-23T16:14:17Z

✨ Benchmark results

Requests per second: 1,750
Requests with 200 status code: 315,115
P99 latency: 0.095919944

⚡ Benchmark Comparison

Metric	This PR	Latest version
RPS	1,750	1,240
200	315,115	223,171
P99	0.095919944	0.180904594

coderabbitai

Actionable comments posted: 1

🤖 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/Platform/Modules/VCS/Http/Installations/Repositories/XList.php`:
- Around line 143-144: The call to searchRepositories() should not receive a
null/empty owner: after calling getOwnerName($providerInstallationId) (in
XList.php), validate the result (e.g., null-coalesced or checked with empty())
and if it's empty throw an Exception with Exception::GENERAL_SERVER_ERROR and
message 'Unable to retrieve repository owner' before calling
$github->searchRepositories($owner, $page, $limit, $search); ensure you
reference getOwnerName and searchRepositories so the guard is added immediately
between those calls.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3f75b6e1-5063-4523-ae43-c64334ff1767

📥 Commits

Reviewing files that changed from the base of the PR and between e3538d0 and bc2c4d0.

📒 Files selected for processing (1)

src/Appwrite/Platform/Modules/VCS/Http/Installations/Repositories/XList.php

* perf: simplify repository authorized checks * search repos

perf: simplify repository authorized checks

e3538d0

greptile-apps bot reviewed Mar 23, 2026

View reviewed changes

Comment thread composer.lock

search repos

bc2c4d0

Meldiron approved these changes Mar 23, 2026

View reviewed changes

coderabbitai bot reviewed Mar 23, 2026

View reviewed changes

Comment thread src/Appwrite/Platform/Modules/VCS/Http/Installations/Repositories/XList.php

hmacr merged commit d53cad2 into 1.9.x Mar 23, 2026
81 of 82 checks passed

lohanidamodar pushed a commit that referenced this pull request Mar 26, 2026

perf: simplify repository authorized checks (#11616)

d06d091

* perf: simplify repository authorized checks * search repos

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: simplify repository authorized checks#11616

perf: simplify repository authorized checks#11616
hmacr merged 2 commits into1.9.xfrom
ser-1170

hmacr commented Mar 23, 2026

Uh oh!

greptile-apps bot commented Mar 23, 2026

Uh oh!

Uh oh!

coderabbitai bot commented Mar 23, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

github-actions bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 23, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hmacr commented Mar 23, 2026

Uh oh!

greptile-apps bot commented Mar 23, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

Uh oh!

coderabbitai bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

github-actions bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔄 PHP-Retry Summary

Uh oh!

github-actions bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Benchmark results

⚡ Benchmark Comparison

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Mar 23, 2026 •

edited

Loading

github-actions bot commented Mar 23, 2026 •

edited

Loading

github-actions bot commented Mar 23, 2026 •

edited

Loading