Fix the "restore" action to work on database dumps without a port when a default port is configured (#969).

NEWS

@@ -12,6 +12,8 @@
  * #962: For the LVM hook, add support for nested logical volumes.
  * #965: Fix a borgmatic runtime directory error when running the "spot" check with a database hook
    enabled.
+ * #969: Fix the "restore" action to work on database dumps without a port when a default port is
+   present in configuration.
  * Fix the "spot" check to no longer consider pipe files within an archive for file comparisons.
  * Fix the "spot" check to have a nicer error when there are no source paths to compare.
  * Fix auto-excluding of special files (when databases are configured) to support relative source

borgmatic/actions/restore.py

@@ -27,15 +27,22 @@ Dump = collections.namedtuple(
 )
 
 
-def dumps_match(first, second):
+def dumps_match(first, second, default_port=None):
     '''
     Compare two Dump instances for equality while supporting a field value of UNSPECIFIED, which
-    indicates that the field should match any value.
+    indicates that the field should match any value. If a default port is given, then consider any
+    dump having that port to match with a dump having a None port.
     '''
     for field_name in first._fields:
         first_value = getattr(first, field_name)
         second_value = getattr(second, field_name)
 
+        if default_port is not None and field_name == 'port':
+            if first_value == default_port and second_value is None:
+                continue
+            if second_value == default_port and first_value is None:
+                continue
+
         if first_value == UNSPECIFIED or second_value == UNSPECIFIED:
             continue
 
@@ -64,10 +71,10 @@ def render_dump_metadata(dump):
     return metadata
 
 
-def get_configured_data_source(config, restore_dump):
+def get_configured_data_source(config, restore_dump, log_prefix):
     '''
     Search in the given configuration dict for dumps corresponding to the given dump to restore. If
-    there are multiple matches, error.
+    there are multiple matches, error. Log using the given log prefix.
 
     Return the found data source as a data source configuration dict or None if not found.
     '''
@@ -78,8 +85,16 @@ def get_configured_data_source(config, restore_dump):
 
     matching_dumps = tuple(
         hook_data_source
-        for (hook_name, hook) in hooks_to_search.items()
-        for hook_data_source in hook
+        for (hook_name, hook_config) in hooks_to_search.items()
+        for hook_data_source in hook_config
+        for default_port in (
+            borgmatic.hooks.dispatch.call_hook(
+                function_name='get_default_port',
+                config=config,
+                log_prefix=log_prefix,
+                hook_name=hook_name,
+            ),
+        )
         if dumps_match(
             Dump(
                 hook_name,
@@ -88,6 +103,7 @@ def get_configured_data_source(config, restore_dump):
                 hook_data_source.get('port'),
             ),
             restore_dump,
+            default_port,
         )
     )
 
@@ -478,6 +494,7 @@ def run_restore(
             found_data_source = get_configured_data_source(
                 config,
                 restore_dump,
+                log_prefix=repository['path'],
             )
 
             # For a dump that wasn't found via an exact match in the configuration, try to fallback
@@ -486,6 +503,7 @@ def run_restore(
                 found_data_source = get_configured_data_source(
                     config,
                     Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port),
+                    log_prefix=repository['path'],
                 )
 
                 if not found_data_source:

borgmatic/hooks/data_source/mariadb.py

@@ -118,6 +118,10 @@ def execute_dump_command(
     )
 
 
+def get_default_port(databases, config, log_prefix):  # pragma: no cover
+    return 3306
+
+
 def use_streaming(databases, config, log_prefix):
     '''
     Given a sequence of MariaDB database configuration dicts, a configuration dict (ignored), and a

borgmatic/hooks/data_source/mongodb.py

@@ -17,6 +17,10 @@ def make_dump_path(base_directory):  # pragma: no cover
     return dump.make_data_source_dump_path(base_directory, 'mongodb_databases')
 
 
+def get_default_port(databases, config, log_prefix):  # pragma: no cover
+    return 27017
+
+
 def use_streaming(databases, config, log_prefix):
     '''
     Given a sequence of MongoDB database configuration dicts, a configuration dict (ignored), and a

borgmatic/hooks/data_source/mysql.py

@@ -117,6 +117,10 @@ def execute_dump_command(
     )
 
 
+def get_default_port(databases, config, log_prefix):  # pragma: no cover
+    return 3306
+
+
 def use_streaming(databases, config, log_prefix):
     '''
     Given a sequence of MySQL database configuration dicts, a configuration dict (ignored), and a

borgmatic/hooks/data_source/postgresql.py

@@ -97,6 +97,10 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run):
     )
 
 
+def get_default_port(databases, config, log_prefix):  # pragma: no cover
+    return 5432
+
+
 def use_streaming(databases, config, log_prefix):
     '''
     Given a sequence of PostgreSQL database configuration dicts, a configuration dict (ignored), and

borgmatic/hooks/data_source/sqlite.py

@@ -17,6 +17,10 @@ def make_dump_path(base_directory):  # pragma: no cover
     return dump.make_data_source_dump_path(base_directory, 'sqlite_databases')
 
 
+def get_default_port(databases, config, log_prefix):  # pragma: no cover
+    return None  # SQLite doesn't use a port.
+
+
 def use_streaming(databases, config, log_prefix):
     '''
     Given a sequence of SQLite database configuration dicts, a configuration dict (ignored), and a

tests/unit/actions/test_restore.py

@@ -5,73 +5,96 @@ import borgmatic.actions.restore as module
 
 
 @pytest.mark.parametrize(
-    'first_dump,second_dump,expected_result',
+    'first_dump,second_dump,default_port,expected_result',
     (
         (
             module.Dump('postgresql_databases', 'foo'),
             module.Dump('postgresql_databases', 'foo'),
+            None,
             True,
         ),
         (
             module.Dump('postgresql_databases', 'foo'),
             module.Dump('postgresql_databases', 'bar'),
+            None,
             False,
         ),
         (
             module.Dump('postgresql_databases', 'foo'),
             module.Dump('mariadb_databases', 'foo'),
+            None,
+            False,
+        ),
+        (
+            module.Dump('postgresql_databases', 'foo'),
+            module.Dump(module.UNSPECIFIED, 'foo'),
+            None,
+            True,
+        ),
+        (
+            module.Dump('postgresql_databases', 'foo'),
+            module.Dump(module.UNSPECIFIED, 'bar'),
+            None,
             False,
         ),
-        (module.Dump('postgresql_databases', 'foo'), module.Dump(module.UNSPECIFIED, 'foo'), True),
-        (module.Dump('postgresql_databases', 'foo'), module.Dump(module.UNSPECIFIED, 'bar'), False),
         (
             module.Dump('postgresql_databases', module.UNSPECIFIED),
             module.Dump('postgresql_databases', 'foo'),
+            None,
             True,
         ),
         (
             module.Dump('postgresql_databases', module.UNSPECIFIED),
             module.Dump('mariadb_databases', 'foo'),
+            None,
             False,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost'),
             module.Dump('postgresql_databases', 'foo', 'myhost'),
+            None,
             True,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost'),
             module.Dump('postgresql_databases', 'foo', 'otherhost'),
+            None,
             False,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost'),
             module.Dump('postgresql_databases', 'foo', module.UNSPECIFIED),
+            None,
             True,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost'),
             module.Dump('postgresql_databases', 'bar', module.UNSPECIFIED),
+            None,
             False,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost', 1234),
             module.Dump('postgresql_databases', 'foo', 'myhost', 1234),
+            None,
             True,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost', 1234),
             module.Dump('postgresql_databases', 'foo', 'myhost', 4321),
+            None,
             False,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost', module.UNSPECIFIED),
             module.Dump('postgresql_databases', 'foo', 'myhost', 1234),
+            None,
             True,
         ),
         (
             module.Dump('postgresql_databases', 'foo', 'myhost', module.UNSPECIFIED),
             module.Dump('postgresql_databases', 'foo', 'otherhost', 1234),
+            None,
             False,
         ),
         (
@@ -79,14 +102,33 @@ import borgmatic.actions.restore as module
                 module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED
             ),
             module.Dump('postgresql_databases', 'foo', 'myhost', 1234),
+            None,
+            True,
+        ),
+        (
+            module.Dump('postgresql_databases', 'foo', 'myhost', 5432),
+            module.Dump('postgresql_databases', 'foo', 'myhost', None),
+            5432,
+            True,
+        ),
+        (
+            module.Dump('postgresql_databases', 'foo', 'myhost', None),
+            module.Dump('postgresql_databases', 'foo', 'myhost', 5432),
+            5432,
             True,
         ),
+        (
+            module.Dump('postgresql_databases', 'foo', 'myhost', 5433),
+            module.Dump('postgresql_databases', 'foo', 'myhost', None),
+            5432,
+            False,
+        ),
     ),
 )
 def test_dumps_match_compares_two_dumps_while_respecting_unspecified_values(
-    first_dump, second_dump, expected_result
+    first_dump, second_dump, default_port, expected_result
 ):
-    assert module.dumps_match(first_dump, second_dump) == expected_result
+    assert module.dumps_match(first_dump, second_dump, default_port) == expected_result
 
 
 @pytest.mark.parametrize(
@@ -137,10 +179,13 @@ def test_render_dump_metadata_renders_dump_values_into_string(dump, expected_res
 
 
 def test_get_configured_data_source_matches_data_source_with_restore_dump():
+    default_port = flexmock()
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(default_port)
     flexmock(module).should_receive('dumps_match').and_return(False)
     flexmock(module).should_receive('dumps_match').with_args(
         module.Dump('postgresql_databases', 'bar'),
         module.Dump('postgresql_databases', 'bar'),
+        default_port=default_port,
     ).and_return(True)
 
     assert module.get_configured_data_source(
@@ -149,22 +194,26 @@ def test_get_configured_data_source_matches_data_source_with_restore_dump():
             'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}],
         },
         restore_dump=module.Dump('postgresql_databases', 'bar'),
+        log_prefix='test',
     ) == {'name': 'bar'}
 
 
 def test_get_configured_data_source_matches_nothing_when_nothing_configured():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(flexmock())
     flexmock(module).should_receive('dumps_match').and_return(False)
 
     assert (
         module.get_configured_data_source(
             config={},
             restore_dump=module.Dump('postgresql_databases', 'quux'),
+            log_prefix='test',
         )
         is None
     )
 
 
 def test_get_configured_data_source_matches_nothing_when_restore_dump_does_not_match_configuration():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(flexmock())
     flexmock(module).should_receive('dumps_match').and_return(False)
 
     assert (
@@ -173,16 +222,20 @@ def test_get_configured_data_source_matches_nothing_when_restore_dump_does_not_m
                 'postgresql_databases': [{'name': 'foo'}],
             },
             restore_dump=module.Dump('postgresql_databases', 'quux'),
+            log_prefix='test',
         )
         is None
     )
 
 
 def test_get_configured_data_source_with_multiple_matching_data_sources_errors():
+    default_port = flexmock()
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(default_port)
     flexmock(module).should_receive('dumps_match').and_return(False)
     flexmock(module).should_receive('dumps_match').with_args(
         module.Dump('postgresql_databases', 'bar'),
         module.Dump('postgresql_databases', 'bar'),
+        default_port=default_port,
     ).and_return(True)
     flexmock(module).should_receive('render_dump_metadata').and_return('test')
 
@@ -197,6 +250,7 @@ def test_get_configured_data_source_with_multiple_matching_data_sources_errors()
                 ],
             },
             restore_dump=module.Dump('postgresql_databases', 'bar'),
+            log_prefix='test',
         )
 
 
@@ -1010,14 +1064,17 @@ 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'),
+        log_prefix=object,
     ).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'),
+        log_prefix=object,
     ).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'),
+        log_prefix=object,
     ).and_return({'name': 'bar'})
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,
@@ -1091,14 +1148,17 @@ 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'),
+        log_prefix=object,
     ).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'),
+        log_prefix=object,
     ).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'),
+        log_prefix=object,
     ).and_return(None)
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,
@@ -1172,10 +1232,12 @@ 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'),
+        log_prefix=object,
     ).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'),
+        log_prefix=object,
     ).and_return({'name': 'bar'})
     flexmock(module).should_receive('restore_single_dump').with_args(
         repository=object,