Skip to content

Conversation

@AronNovak
Copy link

Summary

This PR adds support for Geonames premium accounts by introducing optional token and secure parameters to the Geonames provider.

Changes

  • Added optional $token parameter to the Geonames constructor for premium account authentication
  • Added $secure boolean flag to enable the secure HTTPS endpoint (https://secure.geonames.net)
  • Implemented dynamic base URL selection based on the $secure flag
  • Added appendToken() method to append the token parameter to API requests when provided
  • Updated all API endpoint calls to use the dynamic base URL and token parameter

Benefits

  • Enables use of Geonames premium accounts with token authentication
  • Supports the secure HTTPS endpoint for premium users
  • Maintains full backward compatibility with existing free accounts using username-only authentication
  • No breaking changes to existing code

Testing

  • Tested with premium Geonames account using token authentication and secure endpoint
  • Verified backward compatibility with username-only authentication

Documentation

Premium accounts can now be used as follows:

$provider = new Geonames($client, 'username', 'token', true);
  • username: Your Geonames username
  • token: Your premium account token (optional, defaults to null)
  • secure: Use secure endpoint (optional, defaults to false)

…ovider

- Add optional token parameter to Geonames constructor
- Add secure flag to enable https://secure.geonames.net endpoint
- Support dynamic base URL (api.geonames.org or secure.geonames.net)
- Append token parameter to all API requests when provided
- Maintain backward compatibility with existing username-only authentication
Copy link
Member

@jbelien jbelien left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your PR.

On top of the following comments, here are a few extra remarks:

  • Is secure.geonames.org only available if you provide a token?
    Your PR description seems to say so but the code doesn't reflect that.
  • Please add some tests for the use of $token and $secure.
    You'll have to update phpunit.xml.dist and temporarily set your token (do not commit it), the response using your token will be cached in .cached_responses so we can keep testing it without needing your token.

Thanks.

Comment on lines 80 to 85
// Determine base URL based on secure flag
if ($secure) {
$this->baseUrl = 'https://secure.geonames.net';
} else {
$this->baseUrl = 'http://api.geonames.org';
}
Copy link
Member

Choose a reason for hiding this comment

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

Please keep the use of constants. You seem to have made the existing constants irrelevant.

$this->token = $token;

// Determine base URL based on secure flag
if ($secure) {
Copy link
Member

Choose a reason for hiding this comment

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

Use strict comparison.

Suggested change
if ($secure) {
if ($secure === true) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants