Skip to content

Conversation

@norkunas
Copy link
Member

@norkunas norkunas commented Dec 11, 2025

Summary by CodeRabbit

  • Refactor
    • Updated HTTP client discovery mechanism across all provider factories to use PSR-18 compliant client resolution, improving compatibility and standardization while maintaining backward compatibility with no changes to public APIs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Walkthrough

This PR updates HTTP client discovery across all geolocation provider factories by systematically replacing HttpClientDiscovery with Psr18ClientDiscovery, including updated imports and method calls throughout the codebase.

Changes

Cohort / File(s) Summary
HTTP Client Discovery Replacement
src/ProviderFactory/AlgoliaFactory.php, src/ProviderFactory/ArcGISOnlineFactory.php, src/ProviderFactory/BingMapsFactory.php, src/ProviderFactory/FreeGeoIpFactory.php, src/ProviderFactory/GeoPluginFactory.php, src/ProviderFactory/GeonamesFactory.php, src/ProviderFactory/GoogleMapsFactory.php, src/ProviderFactory/GoogleMapsPlacesFactory.php, src/ProviderFactory/HereFactory.php, src/ProviderFactory/HostIpFactory.php, src/ProviderFactory/IpInfoDbFactory.php, src/ProviderFactory/IpInfoFactory.php, src/ProviderFactory/IpstackFactory.php, src/ProviderFactory/LocationIQFactory.php, src/ProviderFactory/MapQuestFactory.php, src/ProviderFactory/MapboxFactory.php, src/ProviderFactory/MaxMindFactory.php, src/ProviderFactory/NominatimFactory.php, src/ProviderFactory/OpenCageFactory.php, src/ProviderFactory/OpenRouteServiceFactory.php, src/ProviderFactory/PickPointFactory.php, src/ProviderFactory/TomTomFactory.php, src/ProviderFactory/YandexFactory.php
Replaced Http\Discovery\HttpClientDiscovery with Http\Discovery\Psr18ClientDiscovery in all provider factory files. Updated imports and changed all discovery method calls from HttpClientDiscovery::find() to Psr18ClientDiscovery::find().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verification points: Confirm all 23 factory files have consistently applied the discovery class swap without missing any call sites
  • Potential concern: Validate that Psr18ClientDiscovery is compatible and behaves identically to HttpClientDiscovery in all supported environments and configurations

Poem

🐰 Hop, hop, through factories we go,
Swapping clients high and low,
PSR-18 now leads the way,
Discovery shines bright today!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change across all modified files: replacing deprecated HttpClientDiscovery with Psr18ClientDiscovery throughout the provider factories.
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

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f165c53 and 3155b85.

📒 Files selected for processing (23)
  • src/ProviderFactory/AlgoliaFactory.php (2 hunks)
  • src/ProviderFactory/ArcGISOnlineFactory.php (2 hunks)
  • src/ProviderFactory/BingMapsFactory.php (2 hunks)
  • src/ProviderFactory/FreeGeoIpFactory.php (2 hunks)
  • src/ProviderFactory/GeoPluginFactory.php (2 hunks)
  • src/ProviderFactory/GeonamesFactory.php (2 hunks)
  • src/ProviderFactory/GoogleMapsFactory.php (2 hunks)
  • src/ProviderFactory/GoogleMapsPlacesFactory.php (2 hunks)
  • src/ProviderFactory/HereFactory.php (2 hunks)
  • src/ProviderFactory/HostIpFactory.php (2 hunks)
  • src/ProviderFactory/IpInfoDbFactory.php (2 hunks)
  • src/ProviderFactory/IpInfoFactory.php (2 hunks)
  • src/ProviderFactory/IpstackFactory.php (2 hunks)
  • src/ProviderFactory/LocationIQFactory.php (2 hunks)
  • src/ProviderFactory/MapQuestFactory.php (2 hunks)
  • src/ProviderFactory/MapboxFactory.php (2 hunks)
  • src/ProviderFactory/MaxMindFactory.php (2 hunks)
  • src/ProviderFactory/NominatimFactory.php (2 hunks)
  • src/ProviderFactory/OpenCageFactory.php (2 hunks)
  • src/ProviderFactory/OpenRouteServiceFactory.php (2 hunks)
  • src/ProviderFactory/PickPointFactory.php (2 hunks)
  • src/ProviderFactory/TomTomFactory.php (2 hunks)
  • src/ProviderFactory/YandexFactory.php (2 hunks)
🔇 Additional comments (22)
src/ProviderFactory/OpenCageFactory.php (1)

17-32: Consistent client resolution with new discovery

Client resolution now prefers http_client, then deprecated httplug_client, then the injected client, with discovery as last resort; this is consistent with other factories and keeps BC.

src/ProviderFactory/YandexFactory.php (1)

17-32: Discovery swap preserves existing configuration behaviour

The new discovery call is correctly wired into the same http_clienthttplug_client$this->httpClient fallback chain, so configuration semantics are preserved.

src/ProviderFactory/MapQuestFactory.php (1)

17-32: HTTP client discovery update is aligned with other factories

The factory now prefers http_client over deprecated httplug_client, with $this->httpClient and discovery as fallbacks, matching the pattern used elsewhere.

src/ProviderFactory/GoogleMapsPlacesFactory.php (1)

17-32: Places factory client resolution remains correct with new discovery

The precedence of http_client / httplug_client / $this->httpClient is unchanged, with only the discovery mechanism swapped, so the change looks safe.

src/ProviderFactory/PickPointFactory.php (1)

17-32: PickPoint factory aligned with shared discovery pattern

The client lookup chain is consistent with other providers and simply swaps in the PSR‑18 discovery, so this looks good.

src/ProviderFactory/MapboxFactory.php (1)

15-30: Mapbox factory correctly migrated to PSR‑18 discovery

The HTTP client resolution honours http_client first, then httplug_client, then $this->httpClient, with PSR‑18 discovery as fallback, matching other factories.

src/ProviderFactory/IpInfoDbFactory.php (1)

17-32: Fallback chain and discovery migration verified as correct

The code properly implements the fallback precedence: http_client over deprecated httplug_client, then $this->httpClient, then Psr18ClientDiscovery::find(). The OptionsResolver correctly configures both options with httplug_client marked as deprecated. No HttpClientDiscovery references remain in the codebase—the migration to Psr18ClientDiscovery is complete and consistent across all factory classes.

src/ProviderFactory/LocationIQFactory.php (1)

15-33: Psr18ClientDiscovery migration looks correct and preserves client precedence

Line 15 and Line 30 cleanly swap in Psr18ClientDiscovery while keeping the same precedence chain (http_clienthttplug_client$this->httpClient → discovery). No functional regressions are evident within this factory; it aligns with the rest of the bundle’s PSR‑18 shift.

Please ensure the LocationIQ-related tests (or integration checks) pass with your current php-http/discovery and geocoder-php/locationiq-provider versions to confirm discovery wiring at runtime.

src/ProviderFactory/OpenRouteServiceFactory.php (1)

17-35: Consistent PSR‑18 discovery update for OpenRouteService

The switch to Psr18ClientDiscovery on Line 17 and Line 32 is consistent with other factories, and the fallback order for $httpClient remains unchanged. Provider construction (new OpenRouteService($httpClient, $config['api_key'])) is unaffected.

Please run the OpenRouteService-related tests to confirm that PSR‑18 discovery works with the currently installed provider and discovery versions.

src/ProviderFactory/GeoPluginFactory.php (1)

17-35: GeoPlugin factory change is minimal and aligned with PSR‑18 usage

The change to Psr18ClientDiscovery (Line 17, Line 32) is a straightforward replacement and keeps the same client resolution order. No additional behavior changes are introduced.

Please confirm GeoPlugin-related tests still pass so we know PSR‑18 discovery and the provider’s client expectations are in sync.

src/ProviderFactory/NominatimFactory.php (1)

17-35: Nominatim HTTP client discovery updated without altering behavior

Lines 17 and 32 correctly replace HttpClientDiscovery with Psr18ClientDiscovery while preserving the existing resolution chain and constructor arguments. This is consistent with the broader migration in the bundle.

Please run the Nominatim tests (especially around custom http_client / httplug_client config) to verify everything behaves as before with PSR‑18 discovery.

src/ProviderFactory/FreeGeoIpFactory.php (1)

17-35: FreeGeoIp now uses PSR‑18 discovery with identical fallback logic

On Line 17 and Line 32, Psr18ClientDiscovery cleanly replaces HttpClientDiscovery, keeping the same $httpClient resolution order and reusing the existing $config['base_url'] wiring.

Please verify FreeGeoIp provider tests or integration flows with your installed geocoder-php/free-geoip-provider and php-http/discovery to ensure runtime compatibility.

src/ProviderFactory/TomTomFactory.php (1)

17-35: TomTom factory’s PSR‑18 client discovery swap is safe and consistent

The updated import and $httpClient assignment (Lines 17 and 32) simply move discovery to Psr18ClientDiscovery without changing how config values override the discovered client. Construction of TomTom is unchanged.

Please rerun the TomTom-related tests or sample configuration to confirm end‑to‑end behavior with PSR‑18 discovery.

src/ProviderFactory/HostIpFactory.php (1)

17-35: HostIp factory correctly moved to Psr18ClientDiscovery

The import and usage of Psr18ClientDiscovery (Lines 17 and 32) are correct and maintain the same ordering for client resolution. No other behavior or configuration semantics are modified.

Please ensure HostIp tests pass under this change to validate that PSR‑18 discovery integrates cleanly with your current HostIp provider version.

src/ProviderFactory/AlgoliaFactory.php (1)

17-35: Algolia factory PSR‑18 discovery change is clean and in line with others

Lines 17 and 32 update the factory to use Psr18ClientDiscovery while preserving the same $httpClient selection order and provider construction. This is consistent with the rest of the bundle’s migration and shouldn’t affect consumers.

Please confirm Algolia-related tests or usage still behave as expected so we know all dependencies are compatible with the new PSR‑18 discovery path.

src/ProviderFactory/HereFactory.php (1)

17-17: LGTM!

The migration from HttpClientDiscovery to Psr18ClientDiscovery is correct. Both return a Psr\Http\Client\ClientInterface implementation, and the fallback chain logic remains intact.

Also applies to: 36-36

src/ProviderFactory/GeonamesFactory.php (1)

17-17: LGTM!

Consistent with the PR-wide migration to Psr18ClientDiscovery.

Also applies to: 32-32

src/ProviderFactory/IpstackFactory.php (1)

17-17: LGTM!

Consistent with the PR-wide migration to Psr18ClientDiscovery.

Also applies to: 32-32

src/ProviderFactory/MaxMindFactory.php (1)

17-17: LGTM!

Consistent with the PR-wide migration to Psr18ClientDiscovery.

Also applies to: 32-32

src/ProviderFactory/ArcGISOnlineFactory.php (1)

17-17: LGTM!

Consistent with the PR-wide migration to Psr18ClientDiscovery.

Also applies to: 32-32

src/ProviderFactory/BingMapsFactory.php (1)

17-17: LGTM!

Consistent with the PR-wide migration to Psr18ClientDiscovery.

Also applies to: 32-32

src/ProviderFactory/IpInfoFactory.php (1)

17-17: LGTM! Consistent with the PR-wide migration to Psr18ClientDiscovery. The changes correctly replace HttpClientDiscovery across all 25 provider factories.

The composer.json constraint "^1.14" for php-http/discovery exceeds the minimum version requirement (1.7.0) when Psr18ClientDiscovery was introduced, so version compatibility is confirmed.

Also applies to: 32-32


Comment @coderabbitai help to get the list of available commands and usage tips.

@norkunas norkunas merged commit 9f3069c into geocoder-php:master Dec 11, 2025
18 of 19 checks passed
@norkunas norkunas deleted the rmdep branch December 11, 2025 12:47
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