diff --git a/custom-recipes/forward_geocoding/recipe.json b/custom-recipes/forward_geocoding/recipe.json index ba9fede..0770981 100644 --- a/custom-recipes/forward_geocoding/recipe.json +++ b/custom-recipes/forward_geocoding/recipe.json @@ -200,37 +200,66 @@ "label" : "Authentication", "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'here', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0" }, + { + "name": "use_preset", + "label" : "Use preset", + "type": "BOOLEAN", + "mandatory": false, + "description":"Get the authentication parameters from a shared configuration in DSS.", + "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'here', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0" + }, + { + "name": "api_key_preset", + "label": "Geocoder Api Key Preset", + "type": "PRESET", + "parameterSetId": "geocoder_api_key", + "visibilityCondition": "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0 && model.use_preset == true" + }, + { + "name": "google_geocoder_preset", + "label": "Geocoder Api Key Preset", + "type": "PRESET", + "parameterSetId": "google_geocoder_auth", + "visibilityCondition": "model.provider === 'google' && model.use_preset == true" + }, + { + "name": "here_geocoder_preset", + "label": "Geocoder Api Key Preset", + "type": "PRESET", + "parameterSetId": "here_geocoder_auth", + "visibilityCondition": "model.provider === 'here' && model.use_preset == true" + }, { "name": "api_key", - "type": "STRING", + "type": "PASSWORD", "label" : "API key", - "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0" + "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0 && model.use_preset == false" }, { "name": "here_app_id", "type": "STRING", "label" : "HERE App ID", - "visibilityCondition" : "model.provider === 'here'" + "visibilityCondition" : "model.provider === 'here' && model.use_preset == false" }, { "name": "here_app_code", - "type": "STRING", + "type": "PASSWORD", "label" : "HERE App Code", - "visibilityCondition" : "model.provider === 'here'" + "visibilityCondition" : "model.provider === 'here' && model.use_preset == false" }, { "name": "google_client", "type": "STRING", "label" : "Google client ID", "description": "Only if using a premium plan - optional otherwise", - "visibilityCondition" : "model.provider === 'google'" + "visibilityCondition" : "model.provider === 'google' && model.use_preset == false" }, { "name": "google_client_secret", - "type": "STRING", + "type": "PASSWORD", "label" : "Google secret", "description": "Only if using a premium plan - optional otherwise", - "visibilityCondition" : "model.provider === 'google'" + "visibilityCondition" : "model.provider === 'google' && model.use_preset == false" } ], diff --git a/custom-recipes/reverse_geocoding/recipe.json b/custom-recipes/reverse_geocoding/recipe.json index 5851eb0..a99c4e6 100644 --- a/custom-recipes/reverse_geocoding/recipe.json +++ b/custom-recipes/reverse_geocoding/recipe.json @@ -190,37 +190,66 @@ "label" : "Authentication", "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'here', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0" }, + { + "name": "use_preset", + "label" : "Use preset", + "type": "BOOLEAN", + "mandatory": false, + "description":"Get the authentication parameters from a shared configuration in DSS.", + "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'here', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0" + }, + { + "name": "api_key_preset", + "label": "Geocoder Api Key Preset", + "type": "PRESET", + "parameterSetId": "geocoder_api_key", + "visibilityCondition": "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0 && model.use_preset == true" + }, + { + "name": "google_geocoder_preset", + "label": "Geocoder Api Key Preset", + "type": "PRESET", + "parameterSetId": "google_geocoder_auth", + "visibilityCondition": "model.provider === 'google' && model.use_preset == true" + }, + { + "name": "here_geocoder_preset", + "label": "Geocoder Api Key Preset", + "type": "PRESET", + "parameterSetId": "here_geocoder_auth", + "visibilityCondition": "model.provider === 'here' && model.use_preset == true" + }, { "name": "api_key", - "type": "STRING", + "type": "PASSWORD", "label" : "API key", - "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0" + "visibilityCondition" : "['baidu', 'bing', 'canadapost', 'gaode', 'geonames', 'google', 'locationiq', 'mapbox', 'mapquest', 'mapzen', 'opencage', 'tamu', 'tomtom', 'w3w'].indexOf(model.provider) >= 0 && model.use_preset == false" }, { "name": "here_app_id", "type": "STRING", "label" : "HERE App ID", - "visibilityCondition" : "model.provider === 'here'" + "visibilityCondition" : "model.provider === 'here' && model.use_preset == false" }, { "name": "here_app_code", - "type": "STRING", + "type": "PASSWORD", "label" : "HERE App Code", - "visibilityCondition" : "model.provider === 'here'" + "visibilityCondition" : "model.provider === 'here' && model.use_preset == false" }, { "name": "google_client", "type": "STRING", "label" : "Google client ID", "description": "Only if using a premium plan - optional otherwise", - "visibilityCondition" : "model.provider === 'google'" + "visibilityCondition" : "model.provider === 'google' && model.use_preset == false" }, { "name": "google_client_secret", - "type": "STRING", + "type": "PASSWORD", "label" : "Google secret", "description": "Only if using a premium plan - optional otherwise", - "visibilityCondition" : "model.provider === 'google'" + "visibilityCondition" : "model.provider === 'google' && model.use_preset == false" } ], diff --git a/parameter-sets/geocoder_api_key/parameter-set.json b/parameter-sets/geocoder_api_key/parameter-set.json new file mode 100644 index 0000000..c8c14a9 --- /dev/null +++ b/parameter-sets/geocoder_api_key/parameter-set.json @@ -0,0 +1,20 @@ +{ + "meta" : { + "label": "Geocoder Api Key Preset", + "description": "Authentication key for providers: 'Baidu', 'Bing', 'CanadaPost', 'Gaode', 'Geonames', 'LocationIQ', 'Mapbox', 'MapQuest', 'OpenCage', 'Tamu', 'TomTom', 'What3Words'", + "icon": "icon-key" + }, + "defaultDefinableInline": true, + "defaultDefinableAtProjectLevel": true, + + "pluginParams": [], + + "params": [ + { + "name": "api_key", + "label": "API key", + "type": "PASSWORD", + "mandatory": true + } + ] +} \ No newline at end of file diff --git a/parameter-sets/google_geocoder_auth/parameter-set.json b/parameter-sets/google_geocoder_auth/parameter-set.json new file mode 100644 index 0000000..88d6c61 --- /dev/null +++ b/parameter-sets/google_geocoder_auth/parameter-set.json @@ -0,0 +1,31 @@ +{ + "meta" : { + "label": "Google Geocoder Auth Preset", + "description": "Authentication data for Google provider", + "icon": "icon-map-marker" + }, + "defaultDefinableInline": true, + "defaultDefinableAtProjectLevel": true, + + "pluginParams": [], + + "params": [ + { + "name": "api_key", + "label": "API key", + "type": "PASSWORD" + }, + { + "name": "google_client", + "type": "STRING", + "label" : "Google client ID", + "description": "Only if using a premium plan - optional otherwise" + }, + { + "name": "google_client_secret", + "type": "PASSWORD", + "label" : "Google secret", + "description": "Only if using a premium plan - optional otherwise" + } + ] +} \ No newline at end of file diff --git a/parameter-sets/here_geocoder_auth/parameter-set.json b/parameter-sets/here_geocoder_auth/parameter-set.json new file mode 100644 index 0000000..072d22f --- /dev/null +++ b/parameter-sets/here_geocoder_auth/parameter-set.json @@ -0,0 +1,26 @@ +{ + "meta" : { + "label": "Here Geocoder Auth Preset", + "description": "Authentication data for HERE provider", + "icon": "icon-map-marker" + }, + "defaultDefinableInline": true, + "defaultDefinableAtProjectLevel": true, + + "pluginParams": [], + + "params": [ + { + "name": "here_app_id", + "type": "STRING", + "label" : "HERE App ID", + "mandatory": true + }, + { + "name": "here_app_code", + "type": "PASSWORD", + "label" : "HERE App Code", + "mandatory": true + } + ] +} \ No newline at end of file diff --git a/python-lib/dataframe_forward_geocoding.py b/python-lib/dataframe_forward_geocoding.py index 38d944f..c771572 100644 --- a/python-lib/dataframe_forward_geocoding.py +++ b/python-lib/dataframe_forward_geocoding.py @@ -60,16 +60,26 @@ def get_forward_geocode_function(config): """ Handle authentication mechanism with respect to the chosen geocoding service provider `provider_function` """ - provider_function = getattr(geocoder, config['provider']) + provider = config['provider'] + provider_function = getattr(geocoder, provider) + use_preset = config.get('use_preset', False) + if use_preset: + preset_name = { + 'google': 'google_geocoder_preset', + 'here': 'here_geocoder_preset' + }.get(provider, 'api_key_preset') + authentication_config = config.get(preset_name) or {} + else: + authentication_config = config - if config['provider'] == 'here': - return lambda address: provider_function(address, app_id=config['here_app_id'], app_code=config['here_app_code']) - elif config['provider'] == 'google': - return lambda address: provider_function(address, key=config['api_key'], client=config['google_client'], client_secret=config['google_client_secret']) + if provider == 'here': + return lambda address: provider_function(address, app_id=authentication_config.get('here_app_id', ''), app_code=authentication_config.get('here_app_code', '')) + elif provider == 'google': + return lambda address: provider_function(address, key=authentication_config.get('api_key', ''), client=authentication_config.get('google_client', ''), client_secret=authentication_config.get('google_client_secret', '')) elif config['batch_enabled']: - return lambda addresses: provider_function(addresses, key=config['api_key'], method='batch', timeout=config['batch_timeout']) + return lambda addresses: provider_function(addresses, key=authentication_config.get('api_key', ''), method='batch', timeout=config['batch_timeout']) else: - return lambda address: provider_function(address, key=config['api_key']) + return lambda address: provider_function(address, key=authentication_config.get('api_key', '')) def perform_forward_geocode(df, config, fun, cache): diff --git a/python-lib/dataframe_reverse_geocoding.py b/python-lib/dataframe_reverse_geocoding.py index 2c7f787..ea5c99a 100644 --- a/python-lib/dataframe_reverse_geocoding.py +++ b/python-lib/dataframe_reverse_geocoding.py @@ -55,16 +55,26 @@ def add_reverse_geocode_columns(cache, config, current_df, geocode_function): def get_reverse_geocode_function(config): - provider_function = getattr(geocoder, config['provider']) + provider = config['provider'] + provider_function = getattr(geocoder, provider) + use_preset = config.get('use_preset', False) + if use_preset: + preset_name = { + 'google': 'google_geocoder_preset', + 'here': 'here_geocoder_preset' + }.get(provider, 'api_key_preset') + authentication_config = config.get(preset_name) or {} + else: + authentication_config = config - if config['provider'] == 'here': - return lambda lat, lng: provider_function([lat, lng], method='reverse', app_id=config['here_app_id'], app_code=config['here_app_code']) - elif config['provider'] == 'google': - return lambda lat, lng: provider_function([lat, lng], method='reverse', key=config['api_key'], client=config['google_client'], client_secret=config['google_client_secret']) + if provider == 'here': + return lambda lat, lng: provider_function([lat, lng], method='reverse', app_id=authentication_config.get('here_app_id', ''), app_code=authentication_config.get('here_app_code', '')) + elif provider == 'google': + return lambda lat, lng: provider_function([lat, lng], method='reverse', key=authentication_config.get('api_key', ''), client=authentication_config.get('google_client', ''), client_secret=authentication_config.get('google_client_secret', '')) elif config['batch_enabled']: - return lambda locations: provider_function(locations, method='batch_reverse', key=config['api_key']) + return lambda locations: provider_function(locations, method='batch_reverse', key=authentication_config.get('api_key', '')) else: - return lambda lat, lng: provider_function([lat, lng], method='reverse', key=config['api_key']) + return lambda lat, lng: provider_function([lat, lng], method='reverse', key=authentication_config.get('api_key', '')) def perform_reverse_geocode(df, config, fun, cache): diff --git a/python-lib/dku_io.py b/python-lib/dku_io.py index 338ce4f..8cbc4d6 100644 --- a/python-lib/dku_io.py +++ b/python-lib/dku_io.py @@ -17,7 +17,7 @@ def get_config_forward_geocoding(plugin_config, recipe_config): :return: """ processed_config = {} - for param in ['address_column', 'cache_enabled', 'provider', 'api_key', 'here_app_id', 'here_app_code', 'google_client', 'google_client_secret']: + for param in ['address_column', 'cache_enabled', 'provider', 'api_key', 'api_key_preset', 'google_geocoder_preset', 'here_geocoder_preset', 'use_preset', 'here_app_id', 'here_app_code', 'google_client', 'google_client_secret']: processed_config[param] = recipe_config.get(param, None) if processed_config['provider'] is None: @@ -70,7 +70,7 @@ def get_config_reverse_geocoding(plugin_config, recipe_config): processed_config = {} - for param in ['lat_column', 'lng_column', 'provider', 'cache_enabled', 'api_key', 'here_app_id', 'here_app_code', 'google_client', 'google_client_secret']: + for param in ['lat_column', 'lng_column', 'provider', 'cache_enabled', 'api_key', 'api_key_preset', 'google_geocoder_preset', 'here_geocoder_preset', 'use_preset', 'here_app_id', 'here_app_code', 'google_client', 'google_client_secret']: processed_config[param] = recipe_config.get(param, None) if processed_config['provider'] is None: diff --git a/tests/python/unit/test_authentication_presets.py b/tests/python/unit/test_authentication_presets.py new file mode 100644 index 0000000..279bb99 --- /dev/null +++ b/tests/python/unit/test_authentication_presets.py @@ -0,0 +1,300 @@ +import dataframe_forward_geocoding as forward_geocoding +import dataframe_reverse_geocoding as reverse_geocoding +import pytest +from dku_io import get_config_forward_geocoding, get_config_reverse_geocoding + + +def record_provider_calls(monkeypatch, module, provider: str) -> list: + """Mocks a geocoder library provider and records its calls.""" + calls = [] + + def provider_function(*args, **kwargs) -> None: + """Captures arguments passed to the mocked provider.""" + calls.append((args, kwargs)) + + monkeypatch.setattr(module.geocoder, provider, provider_function) + return calls + + +@pytest.mark.parametrize( + "provider, config, argument, expected_kwargs", + [ + pytest.param( + "opencage", + { + "batch_enabled": False, + "use_preset": True, + "api_key_preset": {"api_key": "preset-api-key"}, + "api_key": "should-not-be-passed", + }, + " 203 Rue de Bercy, 75012 Paris", + {"key": "preset-api-key"}, + id="api-key", + ), + pytest.param( + "google", + { + "batch_enabled": False, + "use_preset": True, + "google_geocoder_preset": { + "api_key": "preset-google-key", + "google_client": "preset-google-client", + "google_client_secret": "preset-google-secret", + }, + "api_key": "should-not-be-passed", + "google_client": "should-not-be-passed", + "google_client_secret": "should-not-be-passed", + }, + " 203 Rue de Bercy, 75012 Paris", + { + "key": "preset-google-key", + "client": "preset-google-client", + "client_secret": "preset-google-secret", + }, + id="google", + ), + pytest.param( + "here", + { + "batch_enabled": False, + "use_preset": True, + "here_geocoder_preset": { + "here_app_id": "preset-here-app-id", + "here_app_code": "preset-here-app-code", + }, + "here_app_id": "should-not-be-passed", + "here_app_code": "should-not-be-passed", + }, + " 203 Rue de Bercy, 75012 Paris", + { + "app_id": "preset-here-app-id", + "app_code": "preset-here-app-code", + }, + id="here", + ), + ], +) +def test_forward_geocode_function_uses_preset( + monkeypatch, provider: str, config: dict, argument: str, expected_kwargs: dict +) -> None: + """Checks that forward geocoding uses credentials from the selected preset.""" + config = dict(config) + config["provider"] = provider + calls = record_provider_calls(monkeypatch, forward_geocoding, provider) + + geocode_function = forward_geocoding.get_forward_geocode_function(config) + geocode_function(argument) + + assert calls == [((argument,), expected_kwargs)] + + +@pytest.mark.parametrize( + "provider, config, arguments, expected_arguments, expected_kwargs", + [ + pytest.param( + "opencage", + { + "batch_enabled": False, + "use_preset": True, + "api_key_preset": {"api_key": "preset-api-key"}, + "api_key": "should-not-be-passed", + }, + (48.8444456, 2.3719039), + ([48.8444456, 2.3719039],), + {"method": "reverse", "key": "preset-api-key"}, + id="api-key", + ), + pytest.param( + "google", + { + "batch_enabled": False, + "use_preset": True, + "google_geocoder_preset": { + "api_key": "preset-google-key", + "google_client": "preset-google-client", + "google_client_secret": "preset-google-secret", + }, + "api_key": "should-not-be-passed", + "google_client": "should-not-be-passed", + "google_client_secret": "should-not-be-passed", + }, + (48.8444456, 2.3719039), + ([48.8444456, 2.3719039],), + { + "method": "reverse", + "key": "preset-google-key", + "client": "preset-google-client", + "client_secret": "preset-google-secret", + }, + id="google", + ), + pytest.param( + "here", + { + "batch_enabled": False, + "use_preset": True, + "here_geocoder_preset": { + "here_app_id": "preset-here-app-id", + "here_app_code": "preset-here-app-code", + }, + "here_app_id": "should-not-be-passed", + "here_app_code": "should-not-be-passed", + }, + (48.8444456, 2.3719039), + ([48.8444456, 2.3719039],), + { + "method": "reverse", + "app_id": "preset-here-app-id", + "app_code": "preset-here-app-code", + }, + id="here", + ), + ], +) +def test_reverse_geocode_function_uses_preset( + monkeypatch, + provider: str, + config: dict, + arguments: tuple, + expected_arguments: tuple, + expected_kwargs: dict, +) -> None: + """Checks that reverse geocoding uses credentials from the selected preset.""" + config = dict(config) + config["provider"] = provider + calls = record_provider_calls(monkeypatch, reverse_geocoding, provider) + + geocode_function = reverse_geocoding.get_reverse_geocode_function(config) + geocode_function(*arguments) + + assert calls == [(expected_arguments, expected_kwargs)] + + +@pytest.mark.parametrize( + "module, factory, provider, config, arguments, expected_arguments, expected_kwargs", + [ + pytest.param( + forward_geocoding, + forward_geocoding.get_forward_geocode_function, + "google", + { + "batch_enabled": False, + "api_key": "inline-google-key", + "google_client": "inline-google-client", + "google_client_secret": "inline-google-secret", + "google_geocoder_preset": { + "api_key": "should-not-be-passed", + "google_client": "should-not-be-passed", + "google_client_secret": "should-not-be-passed", + }, + }, + (" 203 Rue de Bercy, 75012 Paris",), + (" 203 Rue de Bercy, 75012 Paris",), + { + "key": "inline-google-key", + "client": "inline-google-client", + "client_secret": "inline-google-secret", + }, + id="forward-without-use-preset", + ), + pytest.param( + reverse_geocoding, + reverse_geocoding.get_reverse_geocode_function, + "here", + { + "batch_enabled": False, + "use_preset": False, + "here_app_id": "inline-here-app-id", + "here_app_code": "inline-here-app-code", + "here_geocoder_preset": { + "here_app_id": "should-not-be-passed", + "here_app_code": "should-not-be-passed", + }, + }, + (48.8444456, 2.3719039), + ([48.8444456, 2.3719039],), + { + "method": "reverse", + "app_id": "inline-here-app-id", + "app_code": "inline-here-app-code", + }, + id="reverse-with-use-preset-disabled", + ), + ], +) +def test_geocode_function_uses_inline_credentials_when_preset_is_disabled( + monkeypatch, + module, + factory, + provider: str, + config: dict, + arguments: tuple, + expected_arguments: tuple, + expected_kwargs: dict, +) -> None: + """Checks that disabled presets preserve legacy inline authentication.""" + config = dict(config) + config["provider"] = provider + calls = record_provider_calls(monkeypatch, module, provider) + + geocode_function = factory(config) + geocode_function(*arguments) + + assert calls == [(expected_arguments, expected_kwargs)] + + +@pytest.mark.parametrize( + "config_factory, recipe_config", + [ + pytest.param( + get_config_forward_geocoding, + { + "address_column": "address", + "cache_enabled": False, + "column_prefix": "geo_", + "provider": "opencage", + }, + id="forward", + ), + pytest.param( + get_config_reverse_geocoding, + { + "address": True, + "cache_enabled": False, + "column_prefix": "geo_", + "lat_column": "latitude", + "lng_column": "longitude", + "provider": "opencage", + }, + id="reverse", + ), + ], +) +def test_config_preserves_preset_configuration(config_factory, recipe_config: dict) -> None: + """Checks that processing preserves preset fields from the recipe configuration.""" + preset_fields_to_preserve = { + "use_preset": True, + "api_key_preset": {"api_key": "preset-api-key"}, + "google_geocoder_preset": { + "api_key": "preset-google-key", + "google_client": "preset-google-client", + "google_client_secret": "preset-google-secret", + }, + "here_geocoder_preset": { + "here_app_id": "preset-here-app-id", + "here_app_code": "preset-here-app-code", + }, + } + recipe_config = dict(recipe_config) + recipe_config.update(preset_fields_to_preserve) + plugin_config = { + "cache_location": "custom", + "cache_location_custom": "/tmp/geocoder-test-cache", + } + + # Processes the recipe configuration through the function under test. + config = config_factory(plugin_config, recipe_config) + + # Verifies that every preset field is preserved in the processed configuration. + for name, value in preset_fields_to_preserve.items(): + assert config[name] == value