diff --git a/.gitignore b/.gitignore index be70968a..20abcb9c 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ build/** **/*.nef **/*.rw2 env/** +venv/** diff --git a/Readme.md b/Readme.md index 1ae6e514..77622d23 100644 --- a/Readme.md +++ b/Readme.md @@ -434,7 +434,7 @@ When I organize photos I look at the embedded metadata. Here are the details of ## Deprecated Use of MapQuest -I used to use the MapQuest API to help me organize your photos by location. I've deprecated this feature and will remove support from the code base. If you're already using MapQuest, after the removal of MapQuest support I'll automatically fall back to Exiftool which is the new default. Follow [pull request #518](https://github.com/jmathai/elodie/issues/518) to know when it gets removed. +I used to use the MapQuest API to help me organize your photos by location. I've deprecated this feature and will remove support from the code base. If you're already using MapQuest, after the removal of MapQuest support I'll automatically fall back to Exiftool which is the new default. Follow [issue #518](https://github.com/jmathai/elodie/issues/518) to know when it gets removed. ## Questions, comments or concerns? diff --git a/elodie/geolocation.py b/elodie/geolocation.py index 66112184..207127a2 100644 --- a/elodie/geolocation.py +++ b/elodie/geolocation.py @@ -19,6 +19,7 @@ __DEFAULT_LOCATION__ = 'Unknown Location' __PREFER_ENGLISH_NAMES__ = None __EXIFTOOL_AVAILABLE__ = None +__MAPQUEST_PLACEHOLDERS__ = ('', 'your-api-key-goes-here') def coordinates_by_name(name): @@ -185,18 +186,29 @@ def exiftool_place_name(lat, lon): def get_key(): global __KEY__ if __KEY__ is not None: - return __KEY__ + return normalize_key(__KEY__) if constants.mapquest_key is not None: __KEY__ = constants.mapquest_key - return __KEY__ + return normalize_key(__KEY__) config = load_config() if('MapQuest' not in config): return None __KEY__ = config['MapQuest']['key'] - return __KEY__ + return normalize_key(__KEY__) + + +def normalize_key(key): + if key is None: + return None + + key = key.strip() + if key in __MAPQUEST_PLACEHOLDERS__: + return None + + return key def get_prefer_english_names(): global __PREFER_ENGLISH_NAMES__ @@ -214,7 +226,7 @@ def get_prefer_english_names(): if('prefer_english_names' not in config['MapQuest']): return False - __PREFER_ENGLISH_NAMES__ = bool(config['MapQuest']['prefer_english_names']) + __PREFER_ENGLISH_NAMES__ = config.getboolean('MapQuest', 'prefer_english_names') return __PREFER_ENGLISH_NAMES__ def place_name(lat, lon): diff --git a/elodie/tests/conftest.py b/elodie/tests/conftest.py index 0e7bf7c7..6c5428f2 100644 --- a/elodie/tests/conftest.py +++ b/elodie/tests/conftest.py @@ -4,6 +4,9 @@ import shutil import sys import tempfile +import json +from urllib.parse import parse_qs +from urllib.parse import urlparse import pytest # Add the parent directories to sys.path so we can import elodie modules and test helpers @@ -15,6 +18,7 @@ from elodie.external.pyexiftool import ExifTool from elodie.dependencies import get_exiftool from elodie import constants +from elodie import geolocation @pytest.fixture(scope="session", autouse=True) @@ -80,4 +84,120 @@ def setup_test_environment(): try: shutil.rmtree(temporary_application_directory) except OSError: - pass # Directory might already be cleaned up \ No newline at end of file + pass # Directory might already be cleaned up + + +@pytest.fixture(scope="function", autouse=True) +def offline_mapquest(request, monkeypatch): + """Provide deterministic geocoding for suites that historically depended on live MapQuest.""" + if request.module.__name__ not in ( + 'elodie.tests.elodie_test', + 'elodie.tests.filesystem_test', + ): + yield + return + + address_lookup = { + 'new york, ny': {'lat': 40.7128, 'lng': -74.0060, 'city': 'New York', 'state': 'NY', 'country': 'US'}, + 'san francisco, ca': {'lat': 37.7749, 'lng': -122.4194, 'city': 'San Francisco', 'state': 'CA', 'country': 'US'}, + 'sunnyvale, ca': {'lat': 37.37188, 'lng': -122.03751, 'city': 'Sunnyvale', 'state': 'CA', 'country': 'US'}, + 'sunnyvale, california': {'lat': 37.37188, 'lng': -122.03751, 'city': 'Sunnyvale', 'state': 'CA', 'country': 'US'}, + } + reverse_lookup = { + '51.521435,0.162714': {'city': 'Rainham', 'state': 'England', 'country': 'GB'}, + '29.758938,-95.3677': {'city': 'Houston', 'state': 'TX', 'country': 'US'}, + '38.1893,-119.9558': {'city': 'Pinecrest', 'state': 'CA', 'country': 'US'}, + '37.366703,-122.033384': {'city': 'Sunnyvale', 'state': 'CA', 'country': 'US'}, + '37.37188,-122.03751': {'city': 'Sunnyvale', 'state': 'CA', 'country': 'US'}, + '40.7128,-74.006': {'city': 'New York', 'state': 'NY', 'country': 'US'}, + '37.7749,-122.4194': {'city': 'San Francisco', 'state': 'CA', 'country': 'US'}, + } + + def canonical_key(lat, lon): + return '{},{}'.format(round(float(lat), 6), round(float(lon), 6)) + + def make_response(payload): + class FakeResponse(object): + def __init__(self, body): + self._body = body + self.text = json.dumps(body) + + def json(self): + return self._body + + return FakeResponse(payload) + + def make_address_payload(location): + match = address_lookup.get(location.lower()) + if match is None: + return { + 'info': {'statuscode': 0}, + 'results': [{ + 'providedLocation': {'location': location}, + 'locations': [{ + 'source': 'FALLBACK', + 'latLng': {'lat': 0, 'lng': 0}, + }] + }] + } + + return { + 'info': {'statuscode': 0}, + 'results': [{ + 'providedLocation': {'location': location}, + 'locations': [{ + 'adminArea5': match['city'], + 'adminArea5Type': 'City', + 'adminArea3': match['state'], + 'adminArea3Type': 'State', + 'adminArea1': match['country'], + 'adminArea1Type': 'Country', + 'geocodeQuality': 'CITY', + 'latLng': {'lat': match['lat'], 'lng': match['lng']}, + }] + }] + } + + def make_reverse_payload(lat, lon): + match = reverse_lookup.get(canonical_key(lat, lon)) + if match is None: + return {'info': {'statuscode': 400}, 'results': []} + + return { + 'info': {'statuscode': 0}, + 'results': [{ + 'providedLocation': {'latLng': {'lat': float(lat), 'lng': float(lon)}}, + 'locations': [{ + 'adminArea5': match['city'], + 'adminArea5Type': 'City', + 'adminArea3': match['state'], + 'adminArea3Type': 'State', + 'adminArea1': match['country'], + 'adminArea1Type': 'Country', + 'latLng': {'lat': float(lat), 'lng': float(lon)}, + }] + }] + } + + def fake_get(url, headers=None): + parsed = urlparse(url) + params = parse_qs(parsed.query) + path = parsed.path + if path.endswith('/address'): + location = params.get('location', [''])[0] + return make_response(make_address_payload(location)) + + if path.endswith('/reverse'): + lat = params.get('lat', [None])[0] + lon = params.get('lon', [None])[0] + if lat is None or lon is None: + location = params.get('location', [''])[0] + if ',' in location: + lat, lon = location.split(',', 1) + return make_response(make_reverse_payload(lat, lon)) + + return make_response({'info': {'statuscode': 400}, 'results': []}) + + monkeypatch.setattr(geolocation.requests, 'get', fake_get) + + yield diff --git a/elodie/tests/elodie_test.py b/elodie/tests/elodie_test.py index 69d53973..d1ffc15b 100644 --- a/elodie/tests/elodie_test.py +++ b/elodie/tests/elodie_test.py @@ -571,6 +571,9 @@ def test_update_location_on_video(): def test_update_location_with_exiftool_fallback(): """Test update_location uses ExifTool when MapQuest key is not available.""" + if not elodie.geolocation.is_exiftool_available(): + pytest.skip('ExifTool geolocation is not available in this environment') + temporary_folder, folder = helper.create_working_folder() origin = '%s/photo.jpg' % folder diff --git a/elodie/tests/geolocation_test.py b/elodie/tests/geolocation_test.py index 4302ea75..f01901a6 100644 --- a/elodie/tests/geolocation_test.py +++ b/elodie/tests/geolocation_test.py @@ -24,6 +24,12 @@ os.environ['TZ'] = 'GMT' +def require_exiftool_geolocation(): + if not geolocation.is_exiftool_available(): + pytest.skip('ExifTool geolocation is not available in this environment') + + + def test_decimal_to_dms(): for x in range(0, 1000): @@ -86,6 +92,7 @@ def test_dms_string_longitude(): def test_exiftool_coordinates_by_name_sunnyvale(): """Test ExifTool coordinates lookup for Sunnyvale, California.""" + require_exiftool_geolocation() result = geolocation.exiftool_coordinates_by_name("Sunnyvale, California") assert result is not None, "Should find coordinates for Sunnyvale, California" @@ -109,10 +116,11 @@ def test_exiftool_coordinates_unavailable(mock_available): def test_exiftool_is_available(): """Test if ExifTool geolocation is available.""" available = geolocation.is_exiftool_available() - assert available == True, "ExifTool geolocation should be available" + assert isinstance(available, bool), "ExifTool availability should be reported as a boolean" def test_exiftool_place_name_sunnyvale(): """Test ExifTool place name lookup with known Sunnyvale coordinates.""" + require_exiftool_geolocation() lat, lon = 37.3688, -122.0365 result = geolocation.exiftool_place_name(lat, lon) @@ -134,6 +142,7 @@ def test_exiftool_place_name_unavailable(mock_available): @mock.patch('elodie.geolocation.__KEY__', None) def test_coordinates_by_name_fallback_to_exiftool(): """Test that coordinates_by_name falls back to ExifTool when MapQuest key is not available.""" + require_exiftool_geolocation() result = geolocation.coordinates_by_name("Sunnyvale, California") assert result is not None, "Should return coordinates using ExifTool fallback" @@ -141,7 +150,14 @@ def test_coordinates_by_name_fallback_to_exiftool(): assert abs(result['latitude'] - 37.3688) < 0.01, f"Should get correct latitude from ExifTool" assert abs(result['longitude'] - (-122.0365)) < 0.01, f"Should get correct longitude from ExifTool" -def test_reverse_lookup_with_valid_key(): +@mock.patch('requests.get') +@mock.patch('elodie.geolocation.__KEY__', 'configured-key') +def test_reverse_lookup_with_valid_key(mock_get): + mock_get.return_value.json.return_value = { + "info": {"statuscode": 0, "copyright": {"text": "© 2022 MapQuest, Inc.", "imageUrl": "http://api.mqcdn.com/res/mqlogo.gif", "imageAltText": "© 2022 MapQuest, Inc."}, "messages": []}, + "options": {"maxResults": 1, "ignoreLatLngInput": False}, + "results": [{"providedLocation": {"latLng": {"lat": 37.368, "lng": -122.03}}, "locations": [{"street": "312 Old San Francisco Rd", "adminArea6": "Heritage District", "adminArea6Type": "Neighborhood", "adminArea5": "Sunnyvale", "adminArea5Type": "City", "adminArea4": "Santa Clara", "adminArea4Type": "County", "adminArea3": "CA", "adminArea3Type": "State", "adminArea1": "US", "adminArea1Type": "Country", "postalCode": "94086", "geocodeQualityCode": "P1AAA", "geocodeQuality": "POINT", "dragPoint": False, "sideOfStreet": "R", "linkId": "0", "unknownInput": "", "type": "s", "latLng": {"lat": 37.36798, "lng": -122.03018}, "displayLatLng": {"lat": 37.36785, "lng": -122.03021}, "mapUrl": ""}]}] + } res = geolocation.lookup(lat=37.368, lon=-122.03) assert res['address']['city'] == 'Sunnyvale', res @@ -154,7 +170,14 @@ def test_reverse_lookup_with_invalid_key(): res = geolocation.lookup(lat=37.368, lon=-122.03) assert res is None, res -def test_lookup_with_valid_key(): +@mock.patch('requests.get') +@mock.patch('elodie.geolocation.__KEY__', 'configured-key') +def test_lookup_with_valid_key(mock_get): + mock_get.return_value.json.return_value = { + "info": {"statuscode": 0, "copyright": {"text": "© 2022 MapQuest, Inc.", "imageUrl": "http://api.mqcdn.com/res/mqlogo.gif", "imageAltText": "© 2022 MapQuest, Inc."}, "messages": []}, + "options": {"maxResults": -1, "ignoreLatLngInput": False}, + "results": [{"providedLocation": {"location": "Sunnyvale,CA"}, "locations": [{"street": "", "adminArea6": "", "adminArea6Type": "Neighborhood", "adminArea5": "Sunnyvale", "adminArea5Type": "City", "adminArea4": "Santa Clara", "adminArea4Type": "County", "adminArea3": "CA", "adminArea3Type": "State", "adminArea1": "US", "adminArea1Type": "Country", "postalCode": "", "geocodeQualityCode": "A5XAX", "geocodeQuality": "CITY", "dragPoint": False, "sideOfStreet": "N", "linkId": "0", "unknownInput": "", "type": "s", "latLng": {"lat": 37.37188, "lng": -122.03751}, "displayLatLng": {"lat": 37.37188, "lng": -122.03751}, "mapUrl": ""}]}] + } res = geolocation.lookup(location='Sunnyvale, CA') latLng = res['results'][0]['locations'][0]['latLng'] assert latLng['lat'] == 37.37188, latLng @@ -186,7 +209,9 @@ def test_lookup_debug_mapquest_url(): assert 'MapQuest url:' in output, output @mock.patch('elodie.constants.location_db', return_value='%s/location.json-cached' % gettempdir()) -def test_place_name_deprecated_string_cached(mock_location_db): +@mock.patch('elodie.geolocation.lookup', return_value={'address': {'city': 'Sunnyvale'}}) +@mock.patch('elodie.geolocation.__KEY__', 'configured-key') +def test_place_name_deprecated_string_cached(mock_lookup, mock_location_db): # See gh-160 for backwards compatability needed when a string is stored instead of a dict helper.reset_dbs() with open(mock_location_db.return_value, 'w') as f: @@ -223,6 +248,7 @@ def test_place_name_no_default(): @mock.patch('elodie.geolocation.__KEY__', None) def test_place_name_fallback_to_exiftool(): """Test that place_name falls back to ExifTool when MapQuest key is not available.""" + require_exiftool_geolocation() # Test with known coordinates for Sunnyvale lat, lon = 37.3688, -122.0365 result = geolocation.place_name(lat, lon)