Allow API key to be set by environment variable, bounds checks (#66)

* Allow API key to be set by env var

* Reject out of bounds coordinates

Co-authored-by: Samuel Scully <[email protected]>

Author: sbscully

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):
     """

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,,'
         ]

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)

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'