Simplify logic to get configured data sources during restoration (#418).

borgmatic/actions/restore.py

@@ -68,27 +68,17 @@ def render_dump_metadata(dump):
 def get_configured_data_source(config, restore_dump):
     '''
     Search in the given configuration dict for dumps corresponding to the given dump to restore. If
-    there are multiple matches, error. If UNSPECIFIED is given as any field in the restore dump,
-    then that can match any value.
+    there are multiple matches, error.
 
-    Return the found data source as a tuple of (found hook name, data source configuration dict) or
-    (None, None) if not found.
+    Return the found data source as a data source configuration dict or None if not found.
     '''
-    if restore_dump.hook_name == UNSPECIFIED:
-        hooks_to_search = {
-            hook_name: value
-            for (hook_name, value) in config.items()
-            if hook_name.split('_databases')[0]
-            in borgmatic.hooks.dispatch.get_submodule_names(borgmatic.hooks.data_source)
-        }
-    else:
-        try:
-            hooks_to_search = {restore_dump.hook_name: config[restore_dump.hook_name]}
-        except KeyError:
-            return (None, None)
+    try:
+        hooks_to_search = {restore_dump.hook_name: config[restore_dump.hook_name]}
+    except KeyError:
+        return None
 
     matching_dumps = tuple(
-        (hook_name, hook_data_source)
+        hook_data_source
         for (hook_name, hook) in hooks_to_search.items()
         for hook_data_source in hook
         if dumps_match(
@@ -103,11 +93,11 @@ def get_configured_data_source(config, restore_dump):
     )
 
     if not matching_dumps:
-        return (None, None)
+        return None
 
     if len(matching_dumps) > 1:
         raise ValueError(
-            f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources. Try adding additional flags to disambiguate.'
+            f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources.'
         )
 
     return matching_dumps[0]
@@ -368,7 +358,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive):
         if requested_dump.data_source_name == 'all':
             continue
 
-        matching_dumps = (
+        matching_dumps = tuple(
             archive_dump
             for archive_dump in dumps_from_archive
             if dumps_match(requested_dump, archive_dump)
@@ -377,10 +367,10 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive):
         if len(matching_dumps) == 0:
             missing_dumps.add(requested_dump)
         elif len(matching_dumps) == 1:
-            dumps_to_restore.add(archive_dump)
+            dumps_to_restore.add(matching_dumps[0])
         else:
             raise ValueError(
-                f'Cannot restore data source {render_dump_metadata(requested_dump)} because there are multiple matching dumps in the archive. Try adding additional flags to disambiguate.'
+                f'Cannot restore data source {render_dump_metadata(requested_dump)} because there are multiple matching dumps in the archive. Try adding flags to disambiguate.'
             )
 
     if missing_dumps:
@@ -483,7 +473,7 @@ def run_restore(
 
         # Restore each dump.
         for restore_dump in dumps_to_restore:
-            found_hook_name, found_data_source = get_configured_data_source(
+            found_data_source = get_configured_data_source(
                 config,
                 restore_dump,
             )
@@ -491,7 +481,7 @@ def run_restore(
             # For any data sources that weren't found via exact matches in the configuration, try to
             # fallback to "all" entries.
             if not found_data_source:
-                found_hook_name, found_data_source = get_configured_data_source(
+                found_data_source = get_configured_data_source(
                     config,
                     Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port),
                 )
@@ -512,7 +502,7 @@ def run_restore(
                 local_path,
                 remote_path,
                 archive_name,
-                found_hook_name or restore_dump.hook_name,
+                restore_dump.hook_name,
                 dict(found_data_source, **{'schemas': restore_arguments.schemas}),
                 connection_params,
                 borgmatic_runtime_directory,

tests/unit/actions/test_restore.py

@@ -19,7 +19,7 @@ def test_get_configured_data_source_matches_data_source_by_name():
             'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}],
         },
         restore_dump=module.Dump('postgresql_databases', 'bar'),
-    ) == ('postgresql_databases', {'name': 'bar'})
+    ) == {'name': 'bar'}
 
 
 def test_get_configured_data_source_matches_nothing_when_nothing_configured():
@@ -28,7 +28,7 @@ def test_get_configured_data_source_matches_nothing_when_nothing_configured():
     assert module.get_configured_data_source(
         config={},
         restore_dump=module.Dump('postgresql_databases', 'quux'),
-    ) == (None, None)
+    ) is None
 
 
 def test_get_configured_data_source_matches_nothing_when_data_source_name_not_configured():
@@ -39,23 +39,7 @@ def test_get_configured_data_source_matches_nothing_when_data_source_name_not_co
             'postgresql_databases': [{'name': 'foo'}],
         },
         restore_dump=module.Dump('postgresql_databases', 'quux'),
-    ) == (None, None)
-
-
-def test_get_configured_data_source_with_unspecified_hook_matches_data_source_by_name():
-    flexmock(module).should_receive('dumps_match').and_return(False)
-    flexmock(module).should_receive('dumps_match').with_args(
-        module.Dump('postgresql_databases', 'bar'),
-        module.Dump(module.UNSPECIFIED, 'bar'),
-    ).and_return(True)
-
-    assert module.get_configured_data_source(
-        config={
-            'other_databases': [{'name': 'other'}],
-            'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}],
-        },
-        restore_dump=module.Dump(module.UNSPECIFIED, 'bar'),
-    ) == ('postgresql_databases', {'name': 'bar'})
+    ) is None
 
 
 def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matching_databases_subdirectory():
@@ -565,8 +549,8 @@ def test_run_restore_restores_each_data_source():
     flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock())
     flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore)
     flexmock(module).should_receive('get_configured_data_source').and_return(
-        ('postgresql_databases', {'name': 'foo'})
-    ).and_return(('postgresql_databases', {'name': 'bar'}))
+        {'name': 'foo'}
+    ).and_return({'name': 'bar'})
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,
         config=object,
@@ -665,15 +649,15 @@ def test_run_restore_restores_data_source_configured_with_all_name():
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'),
-    ).and_return(('postgresql_databases', {'name': 'foo'}))
+    ).and_return({'name': 'foo'})
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='bar'),
-    ).and_return((None, None))
+    ).and_return(None)
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='all'),
-    ).and_return(('postgresql_databases', {'name': 'bar'}))
+    ).and_return({'name': 'bar'})
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,
         config=object,
@@ -746,15 +730,15 @@ def test_run_restore_skips_missing_data_source():
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'),
-    ).and_return(('postgresql_databases', {'name': 'foo'}))
+    ).and_return({'name': 'foo'})
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='bar'),
-    ).and_return((None, None))
+    ).and_return(None)
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='all'),
-    ).and_return((None, None))
+    ).and_return(None)
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,
         config=object,
@@ -827,11 +811,11 @@ def test_run_restore_restores_data_sources_from_different_hooks():
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'),
-    ).and_return(('postgresql_databases', {'name': 'foo'}))
+    ).and_return({'name': 'foo'})
     flexmock(module).should_receive('get_configured_data_source').with_args(
         config=object,
         restore_dump=module.Dump(hook_name='mysql_databases', data_source_name='foo'),
-    ).and_return(('mysql_databases', {'name': 'bar'}))
+    ).and_return({'name': 'bar'})
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,
         config=object,