diff --git a/opencage/geocoder.py b/opencage/geocoder.py index ec7aa69..0800716 100644 --- a/opencage/geocoder.py +++ b/opencage/geocoder.py @@ -32,15 +32,17 @@ class InvalidInputError(OpenCageGeocodeError): """ There was a problem with the input you provided. - :var bad_value: The value that caused the problem + :var message: Error message describing the bad input. + :var bad_value: The value that caused the problem. """ - def __init__(self, bad_value): + def __init__(self, message, bad_value=None): super().__init__() + self.message = message self.bad_value = bad_value def __unicode__(self): - return "Input must be a unicode string, not " + repr(self.bad_value)[:100] + return self.message __str__ = __unicode__ @@ -135,13 +137,19 @@ class OpenCageGeocode: def __init__( self, - key, + key=None, protocol='https', domain=DEFAULT_DOMAIN, sslcontext=None, user_agent_comment=None): """Constructor.""" - self.key = key + self.key = key if key is not None else os.environ.get('OPENCAGE_API_KEY') + + if self.key is None: + raise ValueError( + "API key not provided. " + "Either pass a 'key' parameter or set the OPENCAGE_API_KEY environment variable." + ) if protocol and protocol not in ('http', 'https'): protocol = 'https' @@ -243,7 +251,11 @@ def reverse_geocode(self, lat, lng, **kwargs): :raises RateLimitExceededError: if you have exceeded the number of queries you can make. : Exception says when you can try again :raises UnknownError: if something goes wrong with the OpenCage API + :raises InvalidInputError: if the latitude or longitude is out of bounds """ + + self._validate_lat_lng(lat, lng) + return self.geocode(_query_for_reverse_geocoding(lat, lng), **kwargs) async def reverse_geocode_async(self, lat, lng, **kwargs): @@ -258,7 +270,11 @@ async def reverse_geocode_async(self, lat, lng, **kwargs): :rtype: dict :raises RateLimitExceededError: if exceeded number of queries you can make. You can try again :raises UnknownError: if something goes wrong with the OpenCage API + :raises InvalidInputError: if the latitude or longitude is out of bounds """ + + self._validate_lat_lng(lat, lng) + return await self.geocode_async(_query_for_reverse_geocoding(lat, lng), **kwargs) @backoff.on_exception( @@ -340,12 +356,33 @@ async def _opencage_async_request(self, params): def _parse_request(self, query, params): if not isinstance(query, str): - raise InvalidInputError(bad_value=query) + error_message = "Input must be a unicode string, not " + repr(query)[:100] + raise InvalidInputError(error_message, bad_value=query) data = {'q': query, 'key': self.key} data.update(params) # Add user parameters return data + def _validate_lat_lng(self, lat, lng): + """ + Validate latitude and longitude values. + + Raises InvalidInputError if the values are out of bounds. + """ + try: + lat_float = float(lat) + if not -90 <= lat_float <= 90: + raise InvalidInputError(f"Latitude must be a number between -90 and 90, not {lat}", bad_value=lat) + except ValueError: + raise InvalidInputError(f"Latitude must be a number between -90 and 90, not {lat}", bad_value=lat) + + try: + lng_float = float(lng) + if not -180 <= lng_float <= 180: + raise InvalidInputError(f"Longitude must be a number between -180 and 180, not {lng}", bad_value=lng) + except ValueError: + raise InvalidInputError(f"Longitude must be a number between -180 and 180, not {lng}", bad_value=lng) + def _query_for_reverse_geocoding(lat, lng): """ diff --git a/test/cli/test_cli_run.py b/test/cli/test_cli_run.py index fa0fc2b..89fbb27 100644 --- a/test/cli/test_cli_run.py +++ b/test/cli/test_cli_run.py @@ -96,7 +96,7 @@ def test_input_errors(capfd): _, err = capfd.readouterr() # assert err == '' - assert err.count("\n") == 6 + assert err.count("\n") == 7 assert "Line 1 - Missing input column 2 in ['50.101010']" in err assert "Line 1 - Expected two comma-separated values for reverse geocoding, got ['50.101010']" in err assert "Line 3 - Empty line" in err @@ -109,7 +109,7 @@ def test_input_errors(capfd): length=4, lines=[ '50.101010,,', - '-100,60.1,de,48153', + '-100,60.1,,', ',,', 'a,b,,' ] diff --git a/test/test_error_invalid_input.py b/test/test_error_invalid_input.py index 8eb8fa0..fcaa5c2 100644 --- a/test/test_error_invalid_input.py +++ b/test/test_error_invalid_input.py @@ -34,3 +34,36 @@ def test_must_be_unicode_string(): geocoder.geocode(latin1_string) assert str(excinfo.value) == f"Input must be a unicode string, not {latin1_string!r}" assert excinfo.value.bad_value == latin1_string + + +@responses.activate +def test_reject_out_of_bounds_coordinates(): + """Test that reverse geocoding rejects out-of-bounds latitude and longitude values.""" + responses.add( + responses.GET, + geocoder.url, + body='{"results":{}}', + status=200 + ) + + # Valid coordinates should work + geocoder.reverse_geocode(45.0, 90.0) + geocoder.reverse_geocode(-45.0, -90.0) + + # Invalid latitude values (outside -90 to 90) + with pytest.raises(InvalidInputError) as excinfo: + geocoder.reverse_geocode(91.0, 45.0) + assert "Latitude must be a number between -90 and 90" in str(excinfo.value) + + with pytest.raises(InvalidInputError) as excinfo: + geocoder.reverse_geocode(-91.0, 45.0) + assert "Latitude must be a number between -90 and 90" in str(excinfo.value) + + # Invalid longitude values (outside -180 to 180) + with pytest.raises(InvalidInputError) as excinfo: + geocoder.reverse_geocode(45.0, 181.0) + assert "Longitude must be a number between -180 and 180" in str(excinfo.value) + + with pytest.raises(InvalidInputError) as excinfo: + geocoder.reverse_geocode(45.0, -181.0) + assert "Longitude must be a number between -180 and 180" in str(excinfo.value) diff --git a/test/test_geocoder_args.py b/test/test_geocoder_args.py new file mode 100644 index 0000000..5cb70f1 --- /dev/null +++ b/test/test_geocoder_args.py @@ -0,0 +1,25 @@ +# encoding: utf-8 + +from opencage.geocoder import OpenCageGeocode + +import os + + +def test_protocol_http(): + """Test that HTTP protocol can be set correctly""" + geocoder = OpenCageGeocode('abcde', protocol='http') + assert geocoder.url == 'http://api.opencagedata.com/geocode/v1/json' + + +def test_api_key_env_var(): + """Test that API key can be set by an environment variable""" + + os.environ['OPENCAGE_API_KEY'] = 'from-env-var' + geocoder = OpenCageGeocode() + assert geocoder.key == 'from-env-var' + + +def test_custom_domain(): + """Test that custom domain can be set""" + geocoder = OpenCageGeocode('abcde', domain='example.com') + assert geocoder.url == 'https://example.com/geocode/v1/json' diff --git a/test/test_setting_protocol.py b/test/test_setting_protocol.py deleted file mode 100644 index 0194833..0000000 --- a/test/test_setting_protocol.py +++ /dev/null @@ -1,8 +0,0 @@ -# encoding: utf-8 - - -from opencage.geocoder import OpenCageGeocode - -# Check if protocol can be set -geocoder = OpenCageGeocode('abcde', 'http') -assert geocoder.url == 'http://api.opencagedata.com/geocode/v1/json'