Skip to content

Add reusable GeoIP client#7

Open
premtsd-code wants to merge 2 commits into
appwrite:mainfrom
premtsd-code:feat-geoip-client
Open

Add reusable GeoIP client#7
premtsd-code wants to merge 2 commits into
appwrite:mainfrom
premtsd-code:feat-geoip-client

Conversation

@premtsd-code

Copy link
Copy Markdown

Adds a reusable GeoIP HTTP client for consumers that need to call the Geo service.\n\nChanges:\n- Add Appwrite\Geo\GeoIp with get() and getCountryCode() methods.\n- Add unit coverage for successful lookup, missing country, HTTP errors, fetch exceptions, and disabled endpoint/secret behavior.\n- Add a unit PHPUnit suite.\n- Widen utopia-php/fetch support so consumers already on newer fetch versions can install the package.

@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a reusable GeoIp HTTP client (Appwrite\Geo\GeoIp) and a GeoRecord value object for consuming the Geo service, along with a PHPUnit unit suite and a version bump of utopia-php/fetch from 0.5.* to ^1.1.

  • GeoIp::get() fetches IP data and returns a typed GeoRecord; getCountryCode() wraps it for the common case. Both short-circuit when endpoint or secret are empty.
  • GeoRecord is a clean readonly value object with guarded accessors for countryCode and arbitrary fields.
  • Unit tests cover success, missing country, HTTP errors, invalid JSON, fetch exceptions, and the disabled-endpoint/secret path — but no test reuses a single GeoIp instance across multiple get() calls.

Confidence Score: 4/5

Safe to merge after fixing the Authorization header accumulation in GeoIp::get().

Every call to get() appends a fresh Authorization header to the shared $this->client instance. A GeoIp object reused for multiple lookups will send duplicate bearer tokens that grow with each call, causing failures on the second and subsequent requests.

src/Geo/GeoIp.php — the Authorization header setup in get() should move to the constructor.

Important Files Changed

Filename Overview
src/Geo/GeoIp.php New GeoIp HTTP client; adds Authorization header to shared client on every get() call, causing header accumulation when the instance is reused.
src/Geo/GeoRecord.php New value object for GeoIP response data; clean readonly class with well-guarded accessors.
tests/Unit/GeoIpTest.php New unit test suite covering success, missing country, HTTP errors, invalid JSON, fetch exceptions, and disabled endpoint/secret; no test covers multi-call reuse on a single GeoIp instance, so the Authorization accumulation bug is invisible.
composer.json utopia-php/fetch widened from 0.5.* to ^1.1; lock file updated to 1.1.2; no test:unit script added alongside existing test:e2e.
phpunit.xml Adds a new 'unit' test suite pointing at tests/Unit; straightforward change.

Reviews (4): Last reviewed commit: "Return Geo records from GeoIP client" | Re-trigger Greptile

Comment thread tests/Unit/GeoIpTest.php
Comment thread composer.json Outdated
"require": {
"php": ">=8.3.0",
"utopia-php/fetch": "0.5.*",
"utopia-php/fetch": "0.5.* || ^1.1",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 ^1.1 range is untested

The lock file and CI run against 0.5.1. The ^1.1 range is added so downstream consumers on 1.x can install this package, but neither GeoIp.php nor GeoIpTest.php has been validated against 1.x — specifically the constructor signature of Utopia\Fetch\Response used in the test mocks and the constants (CONTENT_TYPE_APPLICATION_JSON, METHOD_GET) used in the main class. If 1.x introduced API changes, consumers who land on 1.x could get silent failures. Running CI with a second composer update targeting ^1.1 would close this gap.

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