Jelajahi Sumber

Fix for database "restore" action not actually restore anything (#738).

Dan Helfman 1 tahun lalu
induk
melakukan
cd51e9c1ea

+ 12 - 1
.drone.yml

@@ -13,7 +13,6 @@ services:
     environment:
       POSTGRES_PASSWORD: test2
       POSTGRES_DB: test
-      POSTGRES_USER: postgres2
     commands:
       - docker-entrypoint.sh -p 5433
   - name: mariadb
@@ -28,6 +27,18 @@ services:
       MARIADB_DATABASE: test
     commands:
       - docker-entrypoint.sh --port=3307
+  - name: not-actually-mysql
+    image: docker.io/mariadb:10.11.4
+    environment:
+      MARIADB_ROOT_PASSWORD: test
+      MARIADB_DATABASE: test
+  - name: not-actually-mysql2
+    image: docker.io/mariadb:10.11.4
+    environment:
+      MARIADB_ROOT_PASSWORD: test2
+      MARIADB_DATABASE: test
+    commands:
+      - docker-entrypoint.sh --port=3307
   - name: mongodb
     image: docker.io/mongo:5.0.5
     environment:

+ 3 - 0
NEWS

@@ -4,6 +4,9 @@
  * #727: Add a MariaDB database hook that uses native MariaDB commands instead of the deprecated
    MySQL ones. Be aware though that any existing backups made with the "mysql_databases:" hook are
    only restorable with a "mysql_databases:" configuration.
+ * #738: Fix for potential data loss (data not getting restored) in which the database "restore"
+   action didn't actually restore anything and indicated success anyway.
+ * Remove the deprecated use of the MongoDB hook's "--db" flag for database restoration.
  * Add source code reference documentation for getting oriented with the borgmatic code as a
    developer: https://torsion.org/borgmatic/docs/reference/source-code/
 

+ 18 - 14
borgmatic/actions/restore.py

@@ -27,7 +27,8 @@ def get_configured_database(
     hooks for the named database. If a configuration database name is given, use that instead of the
     database name to lookup the database in the given hooks configuration.
 
-    Return the found database as a tuple of (found hook name, database configuration dict).
+    Return the found database as a tuple of (found hook name, database configuration dict) or (None,
+    None) if not found.
     '''
     if not configuration_database_name:
         configuration_database_name = database_name
@@ -39,7 +40,10 @@ def get_configured_database(
             if hook_name in borgmatic.hooks.dump.DATABASE_HOOK_NAMES
         }
     else:
-        hooks_to_search = {hook_name: config[hook_name]}
+        try:
+            hooks_to_search = {hook_name: config[hook_name]}
+        except KeyError:
+            return (None, None)
 
     return next(
         (
@@ -73,9 +77,9 @@ def restore_single_database(
     connection_params,
 ):  # pragma: no cover
     '''
-    Given (among other things) an archive name, a database hook name, the hostname,
-    port, username and password as connection params, and a configured database
-    configuration dict, restore that database from the archive.
+    Given (among other things) an archive name, a database hook name, the hostname, port,
+    username/password as connection params, and a configured database configuration dict, restore
+    that database from the archive.
     '''
     logger.info(
         f'{repository.get("label", repository["path"])}: Restoring database {database["name"]}'
@@ -108,14 +112,14 @@ def restore_single_database(
 
     # Run a single database restore, consuming the extract stdout (if any).
     borgmatic.hooks.dispatch.call_hooks(
-        'restore_database_dump',
-        config,
-        repository['path'],
-        database['name'],
-        borgmatic.hooks.dump.DATABASE_HOOK_NAMES,
-        global_arguments.dry_run,
-        extract_process,
-        connection_params,
+        function_name='restore_database_dump',
+        config=config,
+        log_prefix=repository['path'],
+        hook_names=[hook_name],
+        database=database,
+        dry_run=global_arguments.dry_run,
+        extract_process=extract_process,
+        connection_params=connection_params,
     )
 
 
@@ -333,7 +337,7 @@ def run_restore(
                 connection_params,
             )
 
-    # For any database that weren't found via exact matches in the configuration, try to fallback
+    # For any databases that weren't found via exact matches in the configuration, try to fallback
     # to "all" entries.
     for hook_name, database_names in remaining_restore_names.items():
         for database_name in database_names:

+ 6 - 18
borgmatic/hooks/mariadb.py

@@ -184,28 +184,16 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None):  # pra
 
 
 def restore_database_dump(
-    databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params
+    hook_config, config, log_prefix, database, dry_run, extract_process, connection_params
 ):
     '''
-    Restore the given MariaDB database from an extract stream. The databases are supplied as a
-    sequence containing one dict describing each database (as per the configuration schema), but
-    only the database corresponding to the given database name is restored. Use the given log prefix
-    in any log entries. If this is a dry run, then don't actually restore anything. Trigger the
-    given active extract process (an instance of subprocess.Popen) to produce output to consume.
+    Restore a database from the given extract stream. The database is supplied as a configuration
+    dict, but the given hook configuration is ignored. The given configuration dict is used to
+    construct the destination path, and the given log prefix is used for any log entries. If this is
+    a dry run, then don't actually restore anything. Trigger the given active extract process (an
+    instance of subprocess.Popen) to produce output to consume.
     '''
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
-
-    try:
-        database = next(
-            database_config
-            for database_config in databases_config
-            if database_config.get('name') == database_name
-        )
-    except StopIteration:
-        raise ValueError(
-            f'A database named "{database_name}" could not be found in the configuration'
-        )
-
     hostname = connection_params['hostname'] or database.get(
         'restore_hostname', database.get('hostname')
     )

+ 8 - 20
borgmatic/hooks/mongodb.py

@@ -97,32 +97,19 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None):  # pra
 
 
 def restore_database_dump(
-    databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params
+    hook_config, config, log_prefix, database, dry_run, extract_process, connection_params
 ):
     '''
-    Restore the given MongoDB database from an extract stream. The databases are supplied as a
-    sequence containing one dict describing each database (as per the configuration schema), but
-    only the database corresponding to the given database name is restored. Use the configuration
-    dict to construct the destination path and the given log prefix in any log entries. If this is a
-    dry run, then don't actually restore anything. Trigger the given active extract process (an
+    Restore a database from the given extract stream. The database is supplied as a configuration
+    dict, but the given hook configuration is ignored. The given configuration dict is used to
+    construct the destination path, and the given log prefix is used for any log entries. If this is
+    a dry run, then don't actually restore anything. Trigger the given active extract process (an
     instance of subprocess.Popen) to produce output to consume.
 
     If the extract process is None, then restore the dump from the filesystem rather than from an
     extract stream.
     '''
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
-
-    try:
-        database = next(
-            database_config
-            for database_config in databases_config
-            if database_config.get('name') == database_name
-        )
-    except StopIteration:
-        raise ValueError(
-            f'A database named "{database_name}" could not be found in the configuration'
-        )
-
     dump_filename = dump.make_database_dump_filename(
         make_dump_path(config), database['name'], database.get('hostname')
     )
@@ -165,7 +152,7 @@ def build_restore_command(extract_process, database, dump_filename, connection_p
     else:
         command.extend(('--dir', dump_filename))
     if database['name'] != 'all':
-        command.extend(('--drop', '--db', database['name']))
+        command.extend(('--drop',))
     if hostname:
         command.extend(('--host', hostname))
     if port:
@@ -178,7 +165,8 @@ def build_restore_command(extract_process, database, dump_filename, connection_p
         command.extend(('--authenticationDatabase', database['authentication_database']))
     if 'restore_options' in database:
         command.extend(database['restore_options'].split(' '))
-    if database['schemas']:
+    if database.get('schemas'):
         for schema in database['schemas']:
             command.extend(('--nsInclude', schema))
+
     return command

+ 6 - 18
borgmatic/hooks/mysql.py

@@ -181,28 +181,16 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None):  # pra
 
 
 def restore_database_dump(
-    databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params
+    hook_config, config, log_prefix, database, dry_run, extract_process, connection_params
 ):
     '''
-    Restore the given MySQL/MariaDB database from an extract stream. The databases are supplied as a
-    sequence containing one dict describing each database (as per the configuration schema), but
-    only the database corresponding to the given database name is restored. Use the given log
-    prefix in any log entries. If this is a dry run, then don't actually restore anything. Trigger
-    the given active extract process (an instance of subprocess.Popen) to produce output to consume.
+    Restore a database from the given extract stream. The database is supplied as a configuration
+    dict, but the given hook configuration is ignored. The given configuration dict is used to
+    construct the destination path, and the given log prefix is used for any log entries. If this is
+    a dry run, then don't actually restore anything. Trigger the given active extract process (an
+    instance of subprocess.Popen) to produce output to consume.
     '''
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
-
-    try:
-        database = next(
-            database_config
-            for database_config in databases_config
-            if database_config.get('name') == database_name
-        )
-    except StopIteration:
-        raise ValueError(
-            f'A database named "{database_name}" could not be found in the configuration'
-        )
-
     hostname = connection_params['hostname'] or database.get(
         'restore_hostname', database.get('hostname')
     )

+ 6 - 19
borgmatic/hooks/postgresql.py

@@ -202,15 +202,14 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None):  # pra
 
 
 def restore_database_dump(
-    databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params
+    hook_config, config, log_prefix, database, dry_run, extract_process, connection_params
 ):
     '''
-    Restore the given PostgreSQL database from an extract stream. The databases are supplied as a
-    sequence containing one dict describing each database (as per the configuration schema), but
-    only the database corresponding to the given database name is restored. Use the given
-    configuration dict to construct the destination path and the given log prefix in any log
-    entries. If this is a dry run, then don't actually restore anything. Trigger the given active
-    extract process (an instance of subprocess.Popen) to produce output to consume.
+    Restore a database from the given extract stream. The database is supplied as a configuration
+    dict, but the given hook configuration is ignored. The given configuration dict is used to
+    construct the destination path, and the given log prefix is used for any log entries. If this is
+    a dry run, then don't actually restore anything. Trigger the given active extract process (an
+    instance of subprocess.Popen) to produce output to consume.
 
     If the extract process is None, then restore the dump from the filesystem rather than from an
     extract stream.
@@ -219,18 +218,6 @@ def restore_database_dump(
     hostname, port, username, and password.
     '''
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
-
-    try:
-        database = next(
-            database_config
-            for database_config in databases_config
-            if database_config.get('name') == database_name
-        )
-    except StopIteration:
-        raise ValueError(
-            f'A database named "{database_name}" could not be found in the configuration'
-        )
-
     hostname = connection_params['hostname'] or database.get(
         'restore_hostname', database.get('hostname')
     )

+ 8 - 20
borgmatic/hooks/sqlite.py

@@ -32,10 +32,10 @@ def dump_databases(databases, config, log_prefix, dry_run):
         database_path = database['path']
 
         if database['name'] == 'all':
-            logger.warning('The "all" database name has no meaning for SQLite3 databases')
+            logger.warning('The "all" database name has no meaning for SQLite databases')
         if not os.path.exists(database_path):
             logger.warning(
-                f'{log_prefix}: No SQLite database at {database_path}; An empty database will be created and dumped'
+                f'{log_prefix}: No SQLite database at {database_path}; an empty database will be created and dumped'
             )
 
         dump_path = make_dump_path(config)
@@ -84,28 +84,16 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None):  # pra
 
 
 def restore_database_dump(
-    databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params
+    hook_config, config, log_prefix, database, dry_run, extract_process, connection_params
 ):
     '''
-    Restore the given SQLite3 database from an extract stream. The databases are supplied as a
-    sequence containing one dict describing each database (as per the configuration schema), but
-    only the database corresponding to the given database name is restored. Use the given log prefix
-    in any log entries. If this is a dry run, then don't actually restore anything. Trigger the
-    given active extract process (an instance of subprocess.Popen) to produce output to consume.
+    Restore a database from the given extract stream. The database is supplied as a configuration
+    dict, but the given hook configuration is ignored. The given configuration dict is used to
+    construct the destination path, and the given log prefix is used for any log entries. If this is
+    a dry run, then don't actually restore anything. Trigger the given active extract process (an
+    instance of subprocess.Popen) to produce output to consume.
     '''
     dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else ''
-
-    try:
-        database = next(
-            database_config
-            for database_config in databases_config
-            if database_config.get('name') == database_name
-        )
-    except StopIteration:
-        raise ValueError(
-            f'A database named "{database_name}" could not be found in the configuration'
-        )
-
     database_path = connection_params['restore_path'] or database.get(
         'restore_path', database.get('path')
     )

+ 1 - 1
scripts/run-full-tests

@@ -21,7 +21,7 @@ apk add --no-cache python3 py3-pip borgbackup postgresql-client mariadb-client m
     py3-ruamel.yaml py3-ruamel.yaml.clib bash sqlite fish
 # If certain dependencies of black are available in this version of Alpine, install them.
 apk add --no-cache py3-typed-ast py3-regex || true
-python3 -m pip install --no-cache --upgrade pip==22.2.2 setuptools==64.0.1
+python3 -m pip install --no-cache --upgrade pip==22.2.2 setuptools==64.0.1 pymongo==4.4.1
 pip3 install --ignore-installed tox==3.25.1
 export COVERAGE_FILE=/tmp/.coverage
 

+ 11 - 1
tests/end-to-end/docker-compose.yaml

@@ -10,7 +10,6 @@ services:
     environment:
       POSTGRES_PASSWORD: test2
       POSTGRES_DB: test
-      POSTGRES_USER: postgres2
     command: docker-entrypoint.sh -p 5433
   mariadb:
     image: docker.io/mariadb:10.11.4
@@ -23,6 +22,17 @@ services:
       MARIADB_ROOT_PASSWORD: test2
       MARIADB_DATABASE: test
     command: docker-entrypoint.sh --port=3307
+  not-actually-mysql:
+    image: docker.io/mariadb:10.11.4
+    environment:
+      MARIADB_ROOT_PASSWORD: test
+      MARIADB_DATABASE: test
+  not-actually-mysql2:
+    image: docker.io/mariadb:10.11.4
+    environment:
+      MARIADB_ROOT_PASSWORD: test2
+      MARIADB_DATABASE: test
+    command: docker-entrypoint.sh --port=3307
   mongodb:
     image: docker.io/mongo:5.0.5
     environment:

+ 201 - 21
tests/end-to-end/test_database.py

@@ -5,7 +5,9 @@ import subprocess
 import sys
 import tempfile
 
+import pymongo
 import pytest
+import ruamel.yaml
 
 
 def write_configuration(
@@ -21,7 +23,7 @@ def write_configuration(
     for testing. This includes injecting the given repository path, borgmatic source directory for
     storing database dumps, dump format (for PostgreSQL), and encryption passphrase.
     '''
-    config = f'''
+    config_yaml = f'''
 source_directories:
     - {source_directory}
 repositories:
@@ -61,16 +63,16 @@ mariadb_databases:
       password: test
 mysql_databases:
     - name: test
-      hostname: mariadb
+      hostname: not-actually-mysql
       username: root
       password: test
     - name: all
-      hostname: mariadb
+      hostname: not-actually-mysql
       username: root
       password: test
     - name: all
       format: sql
-      hostname: mariadb
+      hostname: not-actually-mysql
       username: root
       password: test
 mongodb_databases:
@@ -90,7 +92,9 @@ sqlite_databases:
 '''
 
     with open(config_path, 'w') as config_file:
-        config_file.write(config)
+        config_file.write(config_yaml)
+
+    return ruamel.yaml.YAML(typ='safe').load(config_yaml)
 
 
 def write_custom_restore_configuration(
@@ -106,7 +110,7 @@ def write_custom_restore_configuration(
     for testing with custom restore options. This includes a custom restore_hostname, restore_port,
     restore_username, restore_password and restore_path.
     '''
-    config = f'''
+    config_yaml = f'''
 source_directories:
     - {source_directory}
 repositories:
@@ -123,7 +127,6 @@ postgresql_databases:
       format: {postgresql_dump_format}
       restore_hostname: postgresql2
       restore_port: 5433
-      restore_username: postgres2
       restore_password: test2
 mariadb_databases:
     - name: test
@@ -136,10 +139,10 @@ mariadb_databases:
       restore_password: test2
 mysql_databases:
     - name: test
-      hostname: mariadb
+      hostname: not-actually-mysql
       username: root
       password: test
-      restore_hostname: mariadb2
+      restore_hostname: not-actually-mysql2
       restore_port: 3307
       restore_username: root
       restore_password: test2
@@ -161,7 +164,9 @@ sqlite_databases:
 '''
 
     with open(config_path, 'w') as config_file:
-        config_file.write(config)
+        config_file.write(config_yaml)
+
+    return ruamel.yaml.YAML(typ='safe').load(config_yaml)
 
 
 def write_simple_custom_restore_configuration(
@@ -177,7 +182,7 @@ def write_simple_custom_restore_configuration(
     custom restore_hostname, restore_port, restore_username and restore_password as we only test
     these options for PostgreSQL.
     '''
-    config = f'''
+    config_yaml = f'''
 source_directories:
     - {source_directory}
 repositories:
@@ -195,7 +200,147 @@ postgresql_databases:
 '''
 
     with open(config_path, 'w') as config_file:
-        config_file.write(config)
+        config_file.write(config_yaml)
+
+    return ruamel.yaml.YAML(typ='safe').load(config_yaml)
+
+
+def get_connection_params(database, use_restore_options=False):
+    hostname = (database.get('restore_hostname') if use_restore_options else None) or database.get(
+        'hostname'
+    )
+    port = (database.get('restore_port') if use_restore_options else None) or database.get('port')
+    username = (database.get('restore_username') if use_restore_options else None) or database.get(
+        'username'
+    )
+    password = (database.get('restore_password') if use_restore_options else None) or database.get(
+        'password'
+    )
+
+    return (hostname, port, username, password)
+
+
+def run_postgresql_command(command, config, use_restore_options=False):
+    (hostname, port, username, password) = get_connection_params(
+        config['postgresql_databases'][0], use_restore_options
+    )
+
+    subprocess.check_call(
+        [
+            '/usr/bin/psql',
+            f'--host={hostname}',
+            f'--port={port or 5432}',
+            f"--username={username or 'root'}",
+            f'--command={command}',
+            'test',
+        ],
+        env={'PGPASSWORD': password},
+    )
+
+
+def run_mariadb_command(command, config, use_restore_options=False, binary_name='mariadb'):
+    (hostname, port, username, password) = get_connection_params(
+        config[f'{binary_name}_databases'][0], use_restore_options
+    )
+
+    subprocess.check_call(
+        [
+            f'/usr/bin/{binary_name}',
+            f'--host={hostname}',
+            f'--port={port or 3306}',
+            f'--user={username}',
+            f'--execute={command}',
+            'test',
+        ],
+        env={'MYSQL_PWD': password},
+    )
+
+
+def get_mongodb_database_client(config, use_restore_options=False):
+    (hostname, port, username, password) = get_connection_params(
+        config['mongodb_databases'][0], use_restore_options
+    )
+
+    return pymongo.MongoClient(f'mongodb://{username}:{password}@{hostname}:{port or 27017}').test
+
+
+def run_sqlite_command(command, config, use_restore_options=False):
+    database = config['sqlite_databases'][0]
+    path = (database.get('restore_path') if use_restore_options else None) or database.get('path')
+
+    subprocess.check_call(
+        [
+            '/usr/bin/sqlite3',
+            path,
+            command,
+            '.exit',
+        ],
+    )
+
+
+DEFAULT_HOOK_NAMES = {'postgresql', 'mariadb', 'mysql', 'mongodb', 'sqlite'}
+
+
+def create_test_tables(config, use_restore_options=False):
+    '''
+    Create test tables for borgmatic to dump and backup.
+    '''
+    command = 'create table test{id} (thing int); insert into test{id} values (1);'
+
+    if 'postgresql_databases' in config:
+        run_postgresql_command(command.format(id=1), config, use_restore_options)
+    if 'mariadb_databases' in config:
+        run_mariadb_command(command.format(id=2), config, use_restore_options)
+    if 'mysql_databases' in config:
+        run_mariadb_command(command.format(id=3), config, use_restore_options, binary_name='mysql')
+    if 'mongodb_databases' in config:
+        get_mongodb_database_client(config, use_restore_options)['test4'].insert_one({'thing': 1})
+    if 'sqlite_databases' in config:
+        run_sqlite_command(command.format(id=5), config, use_restore_options)
+
+
+def drop_test_tables(config, use_restore_options=False):
+    '''
+    Drop the test tables in preparation for borgmatic restoring them.
+    '''
+    command = 'drop table if exists test{id};'
+
+    if 'postgresql_databases' in config:
+        run_postgresql_command(command.format(id=1), config, use_restore_options)
+    if 'mariadb_databases' in config:
+        run_mariadb_command(command.format(id=2), config, use_restore_options)
+    if 'mysql_databases' in config:
+        run_mariadb_command(command.format(id=3), config, use_restore_options, binary_name='mysql')
+    if 'mongodb_databases' in config:
+        get_mongodb_database_client(config, use_restore_options)['test4'].drop()
+    if 'sqlite_databases' in config:
+        run_sqlite_command(command.format(id=5), config, use_restore_options)
+
+
+def select_test_tables(config, use_restore_options=False):
+    '''
+    Select the test tables to make sure they exist.
+
+    Raise if the expected tables cannot be selected, for instance if a restore hasn't worked as
+    expected.
+    '''
+    command = 'select count(*) from test{id};'
+
+    if 'postgresql_databases' in config:
+        run_postgresql_command(command.format(id=1), config, use_restore_options)
+    if 'mariadb_databases' in config:
+        run_mariadb_command(command.format(id=2), config, use_restore_options)
+    if 'mysql_databases' in config:
+        run_mariadb_command(command.format(id=3), config, use_restore_options, binary_name='mysql')
+    if 'mongodb_databases' in config:
+        assert (
+            get_mongodb_database_client(config, use_restore_options)['test4'].count_documents(
+                filter={}
+            )
+            > 0
+        )
+    if 'sqlite_databases' in config:
+        run_sqlite_command(command.format(id=5), config, use_restore_options)
 
 
 def test_database_dump_and_restore():
@@ -211,15 +356,17 @@ def test_database_dump_and_restore():
 
     try:
         config_path = os.path.join(temporary_directory, 'test.yaml')
-        write_configuration(
+        config = write_configuration(
             temporary_directory, config_path, repository_path, borgmatic_source_directory
         )
+        create_test_tables(config)
+        select_test_tables(config)
 
         subprocess.check_call(
             ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey']
         )
 
-        # Run borgmatic to generate a backup archive including a database dump.
+        # Run borgmatic to generate a backup archive including database dumps.
         subprocess.check_call(['borgmatic', 'create', '--config', config_path, '-v', '2'])
 
         # Get the created archive name.
@@ -232,16 +379,21 @@ def test_database_dump_and_restore():
         assert len(parsed_output[0]['archives']) == 1
         archive_name = parsed_output[0]['archives'][0]['archive']
 
-        # Restore the database from the archive.
+        # Restore the databases from the archive.
+        drop_test_tables(config)
         subprocess.check_call(
             ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name]
         )
+
+        # Ensure the test tables have actually been restored.
+        select_test_tables(config)
     finally:
         os.chdir(original_working_directory)
         shutil.rmtree(temporary_directory)
+        drop_test_tables(config)
 
 
-def test_database_dump_and_restore_with_restore_cli_arguments():
+def test_database_dump_and_restore_with_restore_cli_flags():
     # Create a Borg repository.
     temporary_directory = tempfile.mkdtemp()
     repository_path = os.path.join(temporary_directory, 'test.borg')
@@ -251,9 +403,11 @@ def test_database_dump_and_restore_with_restore_cli_arguments():
 
     try:
         config_path = os.path.join(temporary_directory, 'test.yaml')
-        write_simple_custom_restore_configuration(
+        config = write_simple_custom_restore_configuration(
             temporary_directory, config_path, repository_path, borgmatic_source_directory
         )
+        create_test_tables(config)
+        select_test_tables(config)
 
         subprocess.check_call(
             ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey']
@@ -273,6 +427,7 @@ def test_database_dump_and_restore_with_restore_cli_arguments():
         archive_name = parsed_output[0]['archives'][0]['archive']
 
         # Restore the database from the archive.
+        drop_test_tables(config)
         subprocess.check_call(
             [
                 'borgmatic',
@@ -287,15 +442,25 @@ def test_database_dump_and_restore_with_restore_cli_arguments():
                 'postgresql2',
                 '--port',
                 '5433',
-                '--username',
-                'postgres2',
                 '--password',
                 'test2',
             ]
         )
+
+        # Ensure the test tables have actually been restored. But first modify the config to contain
+        # the altered restore values from the borgmatic command above. This ensures that the test
+        # tables are selected from the correct database.
+        database = config['postgresql_databases'][0]
+        database['restore_hostname'] = 'postgresql2'
+        database['restore_port'] = '5433'
+        database['restore_password'] = 'test2'
+
+        select_test_tables(config, use_restore_options=True)
     finally:
         os.chdir(original_working_directory)
         shutil.rmtree(temporary_directory)
+        drop_test_tables(config)
+        drop_test_tables(config, use_restore_options=True)
 
 
 def test_database_dump_and_restore_with_restore_configuration_options():
@@ -308,9 +473,11 @@ def test_database_dump_and_restore_with_restore_configuration_options():
 
     try:
         config_path = os.path.join(temporary_directory, 'test.yaml')
-        write_custom_restore_configuration(
+        config = write_custom_restore_configuration(
             temporary_directory, config_path, repository_path, borgmatic_source_directory
         )
+        create_test_tables(config)
+        select_test_tables(config)
 
         subprocess.check_call(
             ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey']
@@ -330,12 +497,18 @@ def test_database_dump_and_restore_with_restore_configuration_options():
         archive_name = parsed_output[0]['archives'][0]['archive']
 
         # Restore the database from the archive.
+        drop_test_tables(config)
         subprocess.check_call(
             ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name]
         )
+
+        # Ensure the test tables have actually been restored.
+        select_test_tables(config, use_restore_options=True)
     finally:
         os.chdir(original_working_directory)
         shutil.rmtree(temporary_directory)
+        drop_test_tables(config)
+        drop_test_tables(config, use_restore_options=True)
 
 
 def test_database_dump_and_restore_with_directory_format():
@@ -348,7 +521,7 @@ def test_database_dump_and_restore_with_directory_format():
 
     try:
         config_path = os.path.join(temporary_directory, 'test.yaml')
-        write_configuration(
+        config = write_configuration(
             temporary_directory,
             config_path,
             repository_path,
@@ -356,6 +529,8 @@ def test_database_dump_and_restore_with_directory_format():
             postgresql_dump_format='directory',
             mongodb_dump_format='directory',
         )
+        create_test_tables(config)
+        select_test_tables(config)
 
         subprocess.check_call(
             ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey']
@@ -365,12 +540,17 @@ def test_database_dump_and_restore_with_directory_format():
         subprocess.check_call(['borgmatic', 'create', '--config', config_path, '-v', '2'])
 
         # Restore the database from the archive.
+        drop_test_tables(config)
         subprocess.check_call(
             ['borgmatic', '--config', config_path, 'restore', '--archive', 'latest']
         )
+
+        # Ensure the test tables have actually been restored.
+        select_test_tables(config)
     finally:
         os.chdir(original_working_directory)
         shutil.rmtree(temporary_directory)
+        drop_test_tables(config)
 
 
 def test_database_dump_with_error_causes_borgmatic_to_exit():

+ 9 - 0
tests/unit/actions/test_restore.py

@@ -16,6 +16,15 @@ def test_get_configured_database_matches_database_by_name():
     ) == ('postgresql_databases', {'name': 'bar'})
 
 
+def test_get_configured_database_matches_nothing_when_nothing_configured():
+    assert module.get_configured_database(
+        config={},
+        archive_database_names={'postgresql_databases': ['foo']},
+        hook_name='postgresql_databases',
+        database_name='quux',
+    ) == (None, None)
+
+
 def test_get_configured_database_matches_nothing_when_database_name_not_configured():
     assert module.get_configured_database(
         config={'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}]},

+ 21 - 44
tests/unit/hooks/test_mariadb.py

@@ -380,7 +380,7 @@ def test_dump_databases_does_not_error_for_missing_all_databases_with_dry_run():
 
 
 def test_restore_database_dump_runs_mariadb_to_restore():
-    databases_config = [{'name': 'foo'}, {'name': 'bar'}]
+    hook_config = [{'name': 'foo'}, {'name': 'bar'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -392,10 +392,10 @@ def test_restore_database_dump_runs_mariadb_to_restore():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -407,31 +407,8 @@ def test_restore_database_dump_runs_mariadb_to_restore():
     )
 
 
-def test_restore_database_dump_errors_when_database_missing_from_configuration():
-    databases_config = [{'name': 'foo'}, {'name': 'bar'}]
-    extract_process = flexmock(stdout=flexmock())
-
-    flexmock(module).should_receive('execute_command_with_processes').never()
-
-    with pytest.raises(ValueError):
-        module.restore_database_dump(
-            databases_config,
-            {},
-            'test.yaml',
-            database_name='other',
-            dry_run=False,
-            extract_process=extract_process,
-            connection_params={
-                'hostname': None,
-                'port': None,
-                'username': None,
-                'password': None,
-            },
-        )
-
-
 def test_restore_database_dump_runs_mariadb_with_options():
-    databases_config = [{'name': 'foo', 'restore_options': '--harder'}]
+    hook_config = [{'name': 'foo', 'restore_options': '--harder'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -443,10 +420,10 @@ def test_restore_database_dump_runs_mariadb_with_options():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -459,7 +436,7 @@ def test_restore_database_dump_runs_mariadb_with_options():
 
 
 def test_restore_database_dump_runs_mariadb_with_hostname_and_port():
-    databases_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
+    hook_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -480,10 +457,10 @@ def test_restore_database_dump_runs_mariadb_with_hostname_and_port():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -496,7 +473,7 @@ def test_restore_database_dump_runs_mariadb_with_hostname_and_port():
 
 
 def test_restore_database_dump_runs_mariadb_with_username_and_password():
-    databases_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}]
+    hook_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -508,10 +485,10 @@ def test_restore_database_dump_runs_mariadb_with_username_and_password():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -524,7 +501,7 @@ def test_restore_database_dump_runs_mariadb_with_username_and_password():
 
 
 def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'root',
@@ -557,10 +534,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -573,7 +550,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
 
 
 def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'root',
@@ -608,10 +585,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -624,15 +601,15 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
 
 
 def test_restore_database_dump_with_dry_run_skips_restore():
-    databases_config = [{'name': 'foo'}]
+    hook_config = [{'name': 'foo'}]
 
     flexmock(module).should_receive('execute_command_with_processes').never()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=True,
         extract_process=flexmock(),
         connection_params={

+ 42 - 70
tests/unit/hooks/test_mongodb.py

@@ -1,6 +1,5 @@
 import logging
 
-import pytest
 from flexmock import flexmock
 
 from borgmatic.hooks import mongodb as module
@@ -131,7 +130,15 @@ def test_dump_databases_runs_mongodump_with_options():
     flexmock(module.dump).should_receive('create_named_pipe_for_dump')
 
     flexmock(module).should_receive('execute_command').with_args(
-        ('mongodump', '--db', 'foo', '--stuff=such', '--archive', '>', 'databases/localhost/foo'),
+        (
+            'mongodump',
+            '--db',
+            'foo',
+            '--stuff=such',
+            '--archive',
+            '>',
+            'databases/localhost/foo',
+        ),
         shell=True,
         run_to_completion=False,
     ).and_return(process).once()
@@ -158,23 +165,23 @@ def test_dump_databases_runs_mongodumpall_for_all_databases():
 
 
 def test_restore_database_dump_runs_mongorestore():
-    databases_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}]
+    hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_dump_path')
     flexmock(module.dump).should_receive('make_database_dump_filename')
     flexmock(module).should_receive('execute_command_with_processes').with_args(
-        ['mongorestore', '--archive', '--drop', '--db', 'foo'],
+        ['mongorestore', '--archive', '--drop'],
         processes=[extract_process],
         output_log_level=logging.DEBUG,
         input_file=extract_process.stdout,
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -186,33 +193,8 @@ def test_restore_database_dump_runs_mongorestore():
     )
 
 
-def test_restore_database_dump_errors_on_empty_databases_config():
-    databases_config = []
-
-    flexmock(module).should_receive('make_dump_path')
-    flexmock(module.dump).should_receive('make_database_dump_filename')
-    flexmock(module).should_receive('execute_command_with_processes').never()
-    flexmock(module).should_receive('execute_command').never()
-
-    with pytest.raises(ValueError):
-        module.restore_database_dump(
-            databases_config,
-            {},
-            'test.yaml',
-            database_name='foo',
-            dry_run=False,
-            extract_process=flexmock(),
-            connection_params={
-                'hostname': None,
-                'port': None,
-                'username': None,
-                'password': None,
-            },
-        )
-
-
 def test_restore_database_dump_runs_mongorestore_with_hostname_and_port():
-    databases_config = [
+    hook_config = [
         {'name': 'foo', 'hostname': 'database.example.org', 'port': 5433, 'schemas': None}
     ]
     extract_process = flexmock(stdout=flexmock())
@@ -224,8 +206,6 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port():
             'mongorestore',
             '--archive',
             '--drop',
-            '--db',
-            'foo',
             '--host',
             'database.example.org',
             '--port',
@@ -237,10 +217,10 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -253,7 +233,7 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port():
 
 
 def test_restore_database_dump_runs_mongorestore_with_username_and_password():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'mongo',
@@ -271,8 +251,6 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password():
             'mongorestore',
             '--archive',
             '--drop',
-            '--db',
-            'foo',
             '--username',
             'mongo',
             '--password',
@@ -286,10 +264,10 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -302,7 +280,7 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password():
 
 
 def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'mongo',
@@ -324,8 +302,6 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
             'mongorestore',
             '--archive',
             '--drop',
-            '--db',
-            'foo',
             '--host',
             'clihost',
             '--port',
@@ -343,10 +319,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -359,7 +335,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
 
 
 def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'mongo',
@@ -381,8 +357,6 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
             'mongorestore',
             '--archive',
             '--drop',
-            '--db',
-            'foo',
             '--host',
             'restorehost',
             '--port',
@@ -400,10 +374,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -416,23 +390,23 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
 
 
 def test_restore_database_dump_runs_mongorestore_with_options():
-    databases_config = [{'name': 'foo', 'restore_options': '--harder', 'schemas': None}]
+    hook_config = [{'name': 'foo', 'restore_options': '--harder', 'schemas': None}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_dump_path')
     flexmock(module.dump).should_receive('make_database_dump_filename')
     flexmock(module).should_receive('execute_command_with_processes').with_args(
-        ['mongorestore', '--archive', '--drop', '--db', 'foo', '--harder'],
+        ['mongorestore', '--archive', '--drop', '--harder'],
         processes=[extract_process],
         output_log_level=logging.DEBUG,
         input_file=extract_process.stdout,
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -445,7 +419,7 @@ def test_restore_database_dump_runs_mongorestore_with_options():
 
 
 def test_restore_databases_dump_runs_mongorestore_with_schemas():
-    databases_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}]
+    hook_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_dump_path')
@@ -455,8 +429,6 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas():
             'mongorestore',
             '--archive',
             '--drop',
-            '--db',
-            'foo',
             '--nsInclude',
             'bar',
             '--nsInclude',
@@ -468,10 +440,10 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -484,7 +456,7 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas():
 
 
 def test_restore_database_dump_runs_psql_for_all_database_dump():
-    databases_config = [{'name': 'all', 'schemas': None}]
+    hook_config = [{'name': 'all', 'schemas': None}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_dump_path')
@@ -497,10 +469,10 @@ def test_restore_database_dump_runs_psql_for_all_database_dump():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='all',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -513,17 +485,17 @@ def test_restore_database_dump_runs_psql_for_all_database_dump():
 
 
 def test_restore_database_dump_with_dry_run_skips_restore():
-    databases_config = [{'name': 'foo', 'schemas': None}]
+    hook_config = [{'name': 'foo', 'schemas': None}]
 
     flexmock(module).should_receive('make_dump_path')
     flexmock(module.dump).should_receive('make_database_dump_filename')
     flexmock(module).should_receive('execute_command_with_processes').never()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=True,
         extract_process=flexmock(),
         connection_params={
@@ -536,22 +508,22 @@ def test_restore_database_dump_with_dry_run_skips_restore():
 
 
 def test_restore_database_dump_without_extract_process_restores_from_disk():
-    databases_config = [{'name': 'foo', 'format': 'directory', 'schemas': None}]
+    hook_config = [{'name': 'foo', 'format': 'directory', 'schemas': None}]
 
     flexmock(module).should_receive('make_dump_path')
     flexmock(module.dump).should_receive('make_database_dump_filename').and_return('/dump/path')
     flexmock(module).should_receive('execute_command_with_processes').with_args(
-        ['mongorestore', '--dir', '/dump/path', '--drop', '--db', 'foo'],
+        ['mongorestore', '--dir', '/dump/path', '--drop'],
         processes=[],
         output_log_level=logging.DEBUG,
         input_file=None,
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=None,
         connection_params={

+ 21 - 44
tests/unit/hooks/test_mysql.py

@@ -380,7 +380,7 @@ def test_dump_databases_does_not_error_for_missing_all_databases_with_dry_run():
 
 
 def test_restore_database_dump_runs_mysql_to_restore():
-    databases_config = [{'name': 'foo'}, {'name': 'bar'}]
+    hook_config = [{'name': 'foo'}, {'name': 'bar'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -392,10 +392,10 @@ def test_restore_database_dump_runs_mysql_to_restore():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -407,31 +407,8 @@ def test_restore_database_dump_runs_mysql_to_restore():
     )
 
 
-def test_restore_database_dump_errors_when_database_missing_from_configuration():
-    databases_config = [{'name': 'foo'}, {'name': 'bar'}]
-    extract_process = flexmock(stdout=flexmock())
-
-    flexmock(module).should_receive('execute_command_with_processes').never()
-
-    with pytest.raises(ValueError):
-        module.restore_database_dump(
-            databases_config,
-            {},
-            'test.yaml',
-            database_name='other',
-            dry_run=False,
-            extract_process=extract_process,
-            connection_params={
-                'hostname': None,
-                'port': None,
-                'username': None,
-                'password': None,
-            },
-        )
-
-
 def test_restore_database_dump_runs_mysql_with_options():
-    databases_config = [{'name': 'foo', 'restore_options': '--harder'}]
+    hook_config = [{'name': 'foo', 'restore_options': '--harder'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -443,10 +420,10 @@ def test_restore_database_dump_runs_mysql_with_options():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -459,7 +436,7 @@ def test_restore_database_dump_runs_mysql_with_options():
 
 
 def test_restore_database_dump_runs_mysql_with_hostname_and_port():
-    databases_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
+    hook_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -480,10 +457,10 @@ def test_restore_database_dump_runs_mysql_with_hostname_and_port():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -496,7 +473,7 @@ def test_restore_database_dump_runs_mysql_with_hostname_and_port():
 
 
 def test_restore_database_dump_runs_mysql_with_username_and_password():
-    databases_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}]
+    hook_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -508,10 +485,10 @@ def test_restore_database_dump_runs_mysql_with_username_and_password():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -524,7 +501,7 @@ def test_restore_database_dump_runs_mysql_with_username_and_password():
 
 
 def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'root',
@@ -557,10 +534,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -573,7 +550,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
 
 
 def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'username': 'root',
@@ -608,10 +585,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -624,15 +601,15 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
 
 
 def test_restore_database_dump_with_dry_run_skips_restore():
-    databases_config = [{'name': 'foo'}]
+    hook_config = [{'name': 'foo'}]
 
     flexmock(module).should_receive('execute_command_with_processes').never()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=True,
         extract_process=flexmock(),
         connection_params={

+ 36 - 60
tests/unit/hooks/test_postgresql.py

@@ -464,7 +464,7 @@ def test_dump_databases_runs_non_default_pg_dump():
 
 
 def test_restore_database_dump_runs_pg_restore():
-    databases_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}]
+    hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
@@ -500,10 +500,10 @@ def test_restore_database_dump_runs_pg_restore():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -515,32 +515,8 @@ def test_restore_database_dump_runs_pg_restore():
     )
 
 
-def test_restore_database_dump_errors_when_database_missing_from_configuration():
-    databases_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}]
-    extract_process = flexmock(stdout=flexmock())
-
-    flexmock(module).should_receive('execute_command_with_processes').never()
-    flexmock(module).should_receive('execute_command').never()
-
-    with pytest.raises(ValueError):
-        module.restore_database_dump(
-            databases_config,
-            {},
-            'test.yaml',
-            database_name='other',
-            dry_run=False,
-            extract_process=extract_process,
-            connection_params={
-                'hostname': None,
-                'port': None,
-                'username': None,
-                'password': None,
-            },
-        )
-
-
 def test_restore_database_dump_runs_pg_restore_with_hostname_and_port():
-    databases_config = [
+    hook_config = [
         {'name': 'foo', 'hostname': 'database.example.org', 'port': 5433, 'schemas': None}
     ]
     extract_process = flexmock(stdout=flexmock())
@@ -586,10 +562,10 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -602,7 +578,7 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port():
 
 
 def test_restore_database_dump_runs_pg_restore_with_username_and_password():
-    databases_config = [
+    hook_config = [
         {'name': 'foo', 'username': 'postgres', 'password': 'trustsome1', 'schemas': None}
     ]
     extract_process = flexmock(stdout=flexmock())
@@ -646,10 +622,10 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -662,7 +638,7 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password():
 
 
 def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'hostname': 'database.example.org',
@@ -725,10 +701,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -741,7 +717,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
 
 
 def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'hostname': 'database.example.org',
@@ -804,10 +780,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -820,7 +796,7 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
 
 
 def test_restore_database_dump_runs_pg_restore_with_options():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'restore_options': '--harder',
@@ -865,10 +841,10 @@ def test_restore_database_dump_runs_pg_restore_with_options():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -881,7 +857,7 @@ def test_restore_database_dump_runs_pg_restore_with_options():
 
 
 def test_restore_database_dump_runs_psql_for_all_database_dump():
-    databases_config = [{'name': 'all', 'schemas': None}]
+    hook_config = [{'name': 'all', 'schemas': None}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
@@ -904,10 +880,10 @@ def test_restore_database_dump_runs_psql_for_all_database_dump():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='all',
+        database={'name': 'all'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -920,7 +896,7 @@ def test_restore_database_dump_runs_psql_for_all_database_dump():
 
 
 def test_restore_database_dump_runs_psql_for_plain_database_dump():
-    databases_config = [{'name': 'foo', 'format': 'plain', 'schemas': None}]
+    hook_config = [{'name': 'foo', 'format': 'plain', 'schemas': None}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
@@ -948,10 +924,10 @@ def test_restore_database_dump_runs_psql_for_plain_database_dump():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -964,7 +940,7 @@ def test_restore_database_dump_runs_psql_for_plain_database_dump():
 
 
 def test_restore_database_dump_runs_non_default_pg_restore_and_psql():
-    databases_config = [
+    hook_config = [
         {
             'name': 'foo',
             'pg_restore_command': 'docker exec mycontainer pg_restore',
@@ -1013,10 +989,10 @@ def test_restore_database_dump_runs_non_default_pg_restore_and_psql():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -1029,7 +1005,7 @@ def test_restore_database_dump_runs_non_default_pg_restore_and_psql():
 
 
 def test_restore_database_dump_with_dry_run_skips_restore():
-    databases_config = [{'name': 'foo', 'schemas': None}]
+    hook_config = [{'name': 'foo', 'schemas': None}]
 
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
     flexmock(module).should_receive('make_dump_path')
@@ -1037,10 +1013,10 @@ def test_restore_database_dump_with_dry_run_skips_restore():
     flexmock(module).should_receive('execute_command_with_processes').never()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=True,
         extract_process=flexmock(),
         connection_params={
@@ -1053,7 +1029,7 @@ def test_restore_database_dump_with_dry_run_skips_restore():
 
 
 def test_restore_database_dump_without_extract_process_restores_from_disk():
-    databases_config = [{'name': 'foo', 'schemas': None}]
+    hook_config = [{'name': 'foo', 'schemas': None}]
 
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
     flexmock(module).should_receive('make_dump_path')
@@ -1089,10 +1065,10 @@ def test_restore_database_dump_without_extract_process_restores_from_disk():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database={'name': 'foo'},
         dry_run=False,
         extract_process=None,
         connection_params={
@@ -1105,7 +1081,7 @@ def test_restore_database_dump_without_extract_process_restores_from_disk():
 
 
 def test_restore_database_dump_with_schemas_restores_schemas():
-    databases_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}]
+    hook_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}]
 
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
     flexmock(module).should_receive('make_dump_path')
@@ -1145,10 +1121,10 @@ def test_restore_database_dump_with_schemas_restores_schemas():
     ).once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='foo',
+        database=hook_config[0],
         dry_run=False,
         extract_process=None,
         connection_params={

+ 12 - 29
tests/unit/hooks/test_sqlite.py

@@ -1,6 +1,5 @@
 import logging
 
-import pytest
 from flexmock import flexmock
 
 from borgmatic.hooks import sqlite as module
@@ -93,7 +92,7 @@ def test_dump_databases_does_not_dump_if_dry_run():
 
 
 def test_restore_database_dump_restores_database():
-    databases_config = [{'path': '/path/to/database', 'name': 'database'}, {'name': 'other'}]
+    hook_config = [{'path': '/path/to/database', 'name': 'database'}, {'name': 'other'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').with_args(
@@ -109,10 +108,10 @@ def test_restore_database_dump_restores_database():
     flexmock(module.os).should_receive('remove').once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='database',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={'restore_path': None},
@@ -120,7 +119,7 @@ def test_restore_database_dump_restores_database():
 
 
 def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore():
-    databases_config = [
+    hook_config = [
         {'path': '/path/to/database', 'name': 'database', 'restore_path': 'config/path/to/database'}
     ]
     extract_process = flexmock(stdout=flexmock())
@@ -138,10 +137,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
     flexmock(module.os).should_receive('remove').once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='database',
+        database={'name': 'database'},
         dry_run=False,
         extract_process=extract_process,
         connection_params={'restore_path': 'cli/path/to/database'},
@@ -149,7 +148,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for
 
 
 def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore():
-    databases_config = [
+    hook_config = [
         {'path': '/path/to/database', 'name': 'database', 'restore_path': 'config/path/to/database'}
     ]
     extract_process = flexmock(stdout=flexmock())
@@ -167,10 +166,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
     flexmock(module.os).should_receive('remove').once()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='database',
+        database=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={'restore_path': None},
@@ -178,34 +177,18 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_
 
 
 def test_restore_database_dump_does_not_restore_database_if_dry_run():
-    databases_config = [{'path': '/path/to/database', 'name': 'database'}]
+    hook_config = [{'path': '/path/to/database', 'name': 'database'}]
     extract_process = flexmock(stdout=flexmock())
 
     flexmock(module).should_receive('execute_command_with_processes').never()
     flexmock(module.os).should_receive('remove').never()
 
     module.restore_database_dump(
-        databases_config,
+        hook_config,
         {},
         'test.yaml',
-        database_name='database',
+        database={'name': 'database'},
         dry_run=True,
         extract_process=extract_process,
         connection_params={'restore_path': None},
     )
-
-
-def test_restore_database_dump_raises_error_if_database_config_is_empty():
-    databases_config = []
-    extract_process = flexmock(stdout=flexmock())
-
-    with pytest.raises(ValueError):
-        module.restore_database_dump(
-            databases_config,
-            {},
-            'test.yaml',
-            database_name='database',
-            dry_run=False,
-            extract_process=extract_process,
-            connection_params={'restore_path': None},
-        )