Fix ExifTool geolocation parsing for custom path templates

elodie/filesystem.py

@@ -403,12 +403,23 @@ def get_dynamic_path(self, part, mask, metadata):
         # Each part has its own custom logic and we evaluate a single part and return
         #  the evaluated string.
         if part in ('custom'):
+            config = load_config()
+            config_directory = self.default_folder_path_definition
+            if 'Directory' in config:
+                config_directory = config['Directory']
             custom_parts = re.findall('(%[a-z_]+)', mask)
             folder = mask
             for i in custom_parts:
+                sub_key = i[1:]
+                # If the sub-key is itself defined under [Directory] (e.g.
+                # location=%country|%"Unknown"), expand to that alias before
+                # dispatching. Without this, location/city/state/country
+                # references nested in a custom template would never see
+                # their alias and would fall through to place_name['default'].
+                sub_mask = config_directory[sub_key] if sub_key in config_directory else i
                 folder = folder.replace(
                     i,
-                    self.get_dynamic_path(i[1:], i, metadata)
+                    self.get_dynamic_path(sub_key, sub_mask, metadata)
                 )
             return folder
         elif part in ('date'):

elodie/geolocation.py

@@ -21,6 +21,12 @@
 __EXIFTOOL_AVAILABLE__ = None
 
 
+def _geolocation_field(data, name):
+    # exiftool returns Geolocation* tags either with the 'ExifTool:' group
+    # prefix (when -G is in effect) or without it. Accept both.
+    return data.get('ExifTool:' + name, data.get(name))
+
+
 def coordinates_by_name(name):
     # Try to get cached location first
     db = Db()
@@ -115,7 +121,9 @@ def is_exiftool_available():
         et = ExifTool()
         # Test if geolocation database is available by doing a simple lookup
         result = et.execute_json(b"-api", b"geolocation=40.7128,-74.0060")  # NYC coordinates
-        __EXIFTOOL_AVAILABLE__ = result and len(result) > 0 and 'ExifTool:GeolocationCity' in result[0]
+        __EXIFTOOL_AVAILABLE__ = bool(
+            result and len(result) > 0 and _geolocation_field(result[0], 'GeolocationCity')
+        )
     except Exception:
         __EXIFTOOL_AVAILABLE__ = False
 
@@ -130,17 +138,18 @@ def exiftool_coordinates_by_name(name):
     try:
         et = ExifTool()
         result = et.execute_json(b"-api", f"geolocation={name}".encode('utf-8'))
-        if result and len(result) > 0 and 'ExifTool:GeolocationPosition' in result[0]:
-            position = result[0]['ExifTool:GeolocationPosition']
-            # Position format is "lat lon"
-            lat, lon = position.split()
-            return {
-                'latitude': float(lat),
-                'longitude': float(lon)
-            }
+        if result and len(result) > 0:
+            position = _geolocation_field(result[0], 'GeolocationPosition')
+            if position:
+                # Position format is "lat lon"
+                lat, lon = position.split()
+                return {
+                    'latitude': float(lat),
+                    'longitude': float(lon)
+                }
     except Exception as e:
         log.error(f"ExifTool geolocation lookup failed: {e}")
-    
+
     return None
 
 
@@ -156,23 +165,24 @@ def exiftool_place_name(lat, lon):
         if result and len(result) > 0:
             data = result[0]
             location_data = {}
-
-            # Build location data following the priority: City, Region, Subregion, Country
-            if 'ExifTool:GeolocationCity' in data and data['ExifTool:GeolocationCity'].strip():
-                location_data['city'] = data['ExifTool:GeolocationCity']
-                if 'default' not in location_data:
-                    location_data['default'] = data['ExifTool:GeolocationCity']
-
-            if 'ExifTool:GeolocationRegion' in data and data['ExifTool:GeolocationRegion'].strip():
-                location_data['state'] = data['ExifTool:GeolocationRegion']
-                if 'default' not in location_data:
-                    location_data['default'] = data['ExifTool:GeolocationRegion']
-
-            if 'ExifTool:GeolocationCountry' in data and data['ExifTool:GeolocationCountry'].strip():
-                location_data['country'] = data['ExifTool:GeolocationCountry']
-                if 'default' not in location_data:
-                    location_data['default'] = data['ExifTool:GeolocationCountry']
-
+
+            city = _geolocation_field(data, 'GeolocationCity')
+            region = _geolocation_field(data, 'GeolocationRegion')
+            country = _geolocation_field(data, 'GeolocationCountry')
+
+            # Build location data; default falls back to most-specific available.
+            if city and city.strip():
+                location_data['city'] = city
+                location_data.setdefault('default', city)
+
+            if region and region.strip():
+                location_data['state'] = region
+                location_data.setdefault('default', region)
+
+            if country and country.strip():
+                location_data['country'] = country
+                location_data.setdefault('default', country)
+
             if location_data:
                 return location_data
 

elodie/tests/filesystem_test.py

@@ -532,6 +532,34 @@ def test_get_folder_path_with_custom_path(mock_get_config_file):
 
     assert path == os.path.join('2015-12-05','US-CA-Sunnyvale'), path
 
+@mock.patch('elodie.config.get_config_file', return_value='%s/config.ini-custom-with-location-alias' % gettempdir())
+@mock.patch('elodie.geolocation.place_name', return_value={
+    'city': 'Sunnyvale',
+    'state': 'California',
+    'country': 'United States',
+    'default': 'Sunnyvale',
+})
+def test_get_folder_path_custom_template_expands_location_alias(mock_place_name, mock_get_config_file):
+    """A %location reference nested inside a %custom template should resolve via
+    the [Directory] location alias rather than falling back to place_name['default']."""
+    with open(mock_get_config_file.return_value, 'w') as f:
+        f.write("""
+[Directory]
+date=%Y-%m
+location=%country
+custom=%date %location
+full_path=%custom
+        """)
+    if hasattr(load_config, 'config'):
+        del load_config.config
+    filesystem = FileSystem()
+    media = Photo(helper.get_file('with-location.jpg'))
+    path = filesystem.get_folder_path(media.get_metadata())
+    if hasattr(load_config, 'config'):
+        del load_config.config
+
+    assert path == '2015-12 United States', path
+
 @mock.patch('elodie.config.get_config_file', return_value='%s/config.ini-fallback' % gettempdir())
 def test_get_folder_path_with_fallback_folder(mock_get_config_file):
     with open(mock_get_config_file.return_value, 'w') as f:

elodie/tests/geolocation_test.py

@@ -127,10 +127,94 @@ def test_exiftool_place_name_sunnyvale():
 def test_exiftool_place_name_unavailable(mock_available):
     """Test ExifTool place name when ExifTool is not available."""
     mock_available.return_value = False
-    
+
     result = geolocation.exiftool_place_name(37.3688, -122.0365)
     assert result is None, "Should return None when ExifTool is unavailable"
 
+
+# Regression tests: exiftool may return Geolocation* tags either with the
+# 'ExifTool:' group prefix (when -G is in effect) or without it. The library
+# must accept both forms; a previous bug accepted only the prefixed form,
+# causing reverse lookups to silently fall back to the city as country.
+
+@mock.patch('elodie.geolocation.is_exiftool_available')
+@mock.patch('elodie.geolocation.ExifTool')
+def test_exiftool_place_name_handles_bare_keys(mock_exiftool_cls, mock_available):
+    mock_available.return_value = True
+    mock_et = mock.MagicMock()
+    mock_et.execute_json.return_value = [{
+        'SourceFile': ' ',
+        'GeolocationCity': 'Bratislava',
+        'GeolocationRegion': 'Bratislava Region',
+        'GeolocationCountry': 'Slovakia',
+        'GeolocationCountryCode': 'SK',
+        'GeolocationPosition': '48.1481 17.1067',
+    }]
+    mock_exiftool_cls.return_value = mock_et
+
+    result = geolocation.exiftool_place_name(48.1486, 17.1077)
+
+    assert result is not None
+    assert result['city'] == 'Bratislava', result
+    assert result['state'] == 'Bratislava Region', result
+    assert result['country'] == 'Slovakia', result
+
+@mock.patch('elodie.geolocation.is_exiftool_available')
+@mock.patch('elodie.geolocation.ExifTool')
+def test_exiftool_place_name_handles_prefixed_keys(mock_exiftool_cls, mock_available):
+    mock_available.return_value = True
+    mock_et = mock.MagicMock()
+    mock_et.execute_json.return_value = [{
+        'SourceFile': ' ',
+        'ExifTool:GeolocationCity': 'Bratislava',
+        'ExifTool:GeolocationRegion': 'Bratislava Region',
+        'ExifTool:GeolocationCountry': 'Slovakia',
+        'ExifTool:GeolocationPosition': '48.1481 17.1067',
+    }]
+    mock_exiftool_cls.return_value = mock_et
+
+    result = geolocation.exiftool_place_name(48.1486, 17.1077)
+
+    assert result is not None
+    assert result['city'] == 'Bratislava', result
+    assert result['state'] == 'Bratislava Region', result
+    assert result['country'] == 'Slovakia', result
+
+@mock.patch('elodie.geolocation.ExifTool')
+def test_exiftool_coordinates_by_name_handles_bare_keys(mock_exiftool_cls):
+    # Reset the cached availability flag so the mocked ExifTool is consulted.
+    geolocation.__EXIFTOOL_AVAILABLE__ = None
+    mock_et = mock.MagicMock()
+    mock_et.execute_json.return_value = [{
+        'SourceFile': ' ',
+        'GeolocationCity': 'Bratislava',
+        'GeolocationCountry': 'Slovakia',
+        'GeolocationPosition': '48.1481 17.1067',
+    }]
+    mock_exiftool_cls.return_value = mock_et
+    try:
+        result = geolocation.exiftool_coordinates_by_name('Bratislava')
+    finally:
+        geolocation.__EXIFTOOL_AVAILABLE__ = None
+
+    assert result is not None
+    assert abs(result['latitude'] - 48.1481) < 0.001, result
+    assert abs(result['longitude'] - 17.1067) < 0.001, result
+
+@mock.patch('elodie.geolocation.ExifTool')
+def test_is_exiftool_available_handles_bare_keys(mock_exiftool_cls):
+    geolocation.__EXIFTOOL_AVAILABLE__ = None
+    mock_et = mock.MagicMock()
+    mock_et.execute_json.return_value = [{
+        'SourceFile': ' ',
+        'GeolocationCity': 'New York',
+    }]
+    mock_exiftool_cls.return_value = mock_et
+    try:
+        assert geolocation.is_exiftool_available() is True
+    finally:
+        geolocation.__EXIFTOOL_AVAILABLE__ = None
+
 @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."""