diff --git a/swh/core/db/db_utils.py b/swh/core/db/db_utils.py --- a/swh/core/db/db_utils.py +++ b/swh/core/db/db_utils.py @@ -5,10 +5,10 @@ from datetime import datetime, timezone import functools -import glob from importlib import import_module import logging from os import path +import pathlib import re import subprocess from typing import Collection, Dict, List, Optional, Tuple, Union @@ -181,7 +181,7 @@ sqlfiles = [ fname for fname in get_sql_for_package(modname, upgrade=True) - if db_version < int(path.splitext(path.basename(fname))[0]) <= to_version + if db_version < int(fname.stem) <= to_version ] for sqlfile in sqlfiles: @@ -293,7 +293,9 @@ return None sqlfiles = [ - fname for fname in get_sql_for_package("swh.core.db") if "dbmodule" in fname + fname + for fname in get_sql_for_package("swh.core.db") + if "dbmodule" in fname.stem ] execute_sqlfiles(sqlfiles, db_or_conninfo) @@ -496,7 +498,7 @@ return m -def get_sql_for_package(modname: str, upgrade=False) -> List[str]: +def get_sql_for_package(modname: str, upgrade=False) -> List[pathlib.Path]: """Return the (sorted) list of sql script files for the given swh module If upgrade is True, return the list of available migration scripts, @@ -505,14 +507,15 @@ m = import_swhmodule(modname) if m is None: raise ValueError(f"Module {modname} cannot be loaded") - sqldir = path.join(path.dirname(m.__file__), "sql") + + sqldir = pathlib.Path(m.__file__).parent / "sql" if upgrade: - sqldir += "/upgrades" - if not path.isdir(sqldir): + sqldir /= "upgrades" + if not sqldir.is_dir(): raise ValueError( - "Module {} does not provide a db schema " "(no sql/ dir)".format(modname) + "Module {} does not provide a db schema (no sql/ dir)".format(modname) ) - return sorted(glob.glob(path.join(sqldir, "*.sql")), key=sortkey) + return sorted(sqldir.glob("*.sql"), key=lambda x: sortkey(x.name)) def populate_database_for_package( @@ -541,8 +544,8 @@ return sortkey(path.basename(key)) sqlfiles = get_sql_for_package(modname) + get_sql_for_package("swh.core.db") - sqlfiles = sorted(sqlfiles, key=globalsortkey) - sqlfiles = [fname for fname in sqlfiles if "-superuser-" not in fname] + sqlfiles = sorted(sqlfiles, key=lambda x: sortkey(x.stem)) + sqlfiles = [fpath for fpath in sqlfiles if "-superuser-" not in fpath.stem] execute_sqlfiles(sqlfiles, conninfo, flavor) # populate the dbmodule table @@ -581,7 +584,7 @@ """ sqlfiles = get_sql_for_package(modname) - sqlfiles = [fname for fname in sqlfiles if "-superuser-" in fname] + sqlfiles = [fname for fname in sqlfiles if "-superuser-" in fname.stem] execute_sqlfiles(sqlfiles, conninfo) @@ -621,7 +624,7 @@ def execute_sqlfiles( - sqlfiles: Collection[str], conninfo: str, flavor: Optional[str] = None + sqlfiles: Collection[pathlib.Path], conninfo: str, flavor: Optional[str] = None ): """Execute a list of SQL files on the database pointed at with ``conninfo``. @@ -643,9 +646,13 @@ flavor_set = False for sqlfile in sqlfiles: logger.debug(f"execute SQL file {sqlfile} dbname={conninfo}") - subprocess.check_call(psql_command + ["-f", sqlfile]) + subprocess.check_call(psql_command + ["-f", str(sqlfile)]) - if flavor is not None and not flavor_set and sqlfile.endswith("-flavor.sql"): + if ( + flavor is not None + and not flavor_set + and sqlfile.name.endswith("-flavor.sql") + ): logger.debug("Setting database flavor %s", flavor) query = f"insert into dbflavor (flavor) values ('{flavor}')" subprocess.check_call(psql_command + ["-c", query]) diff --git a/swh/core/db/tests/conftest.py b/swh/core/db/tests/conftest.py --- a/swh/core/db/tests/conftest.py +++ b/swh/core/db/tests/conftest.py @@ -3,15 +3,13 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information -import glob import os from click.testing import CliRunner from hypothesis import HealthCheck import pytest -from swh.core.db.db_utils import get_sql_for_package -from swh.core.utils import numfile_sortkey as sortkey +from swh.core.db.db_utils import import_swhmodule os.environ["LC_ALL"] = "C.UTF-8" @@ -29,31 +27,41 @@ @pytest.fixture() -def mock_package_sql(mocker, datadir): - """This bypasses the module manipulation to only returns the data test files. +def mock_import_swhmodule(mocker, datadir): + """This bypasses the module manipulation to make import_swhmodule return a mock + object suitable for data test files listing via get_sql_for_package. - For a given module `test.mod`, look for sql files in the directory `data/mod/*.sql`. + For a given module `test.`, return a MagicMock object with a __name__ + set to `` and __file__ pointing to `data//__init__.py`. + + The Mock object also defines a `get_datastore()` attribute on which the + `get_current_version()` exists and will return 42. Typical usage:: - def test_xxx(cli_runner, mock_package_sql): + def test_xxx(cli_runner, mock_import_swhmodule): conninfo = craft_conninfo(test_db, "new-db") module_name = "test.cli" - # the command below will use sql scripts from swh/core/db/tests/data/cli/*.sql + # the command below will use sql scripts from + # swh/core/db/tests/data/cli/sql/*.sql cli_runner.invoke(swhdb, ["init", module_name, "--dbname", conninfo]) + """ + mock = mocker.MagicMock - def get_sql_for_package_mock(modname, upgrade=False): + def import_swhmodule_mock(modname): if modname.startswith("test."): - sqldir = modname.split(".", 1)[1] - if upgrade: - sqldir += "/upgrades" - return sorted( - glob.glob(os.path.join(datadir, sqldir, "*.sql")), key=sortkey + dirname = modname.split(".", 1)[1] + + def get_datastore(*args, **kw): + return mock(get_current_version=lambda: 42) + + return mock( + __name__=modname, + __file__=os.path.join(datadir, dirname, "__init__.py"), + get_datastore=get_datastore, ) - return get_sql_for_package(modname) - mock_sql_files = mocker.patch( - "swh.core.db.db_utils.get_sql_for_package", get_sql_for_package_mock - ) - return mock_sql_files + return import_swhmodule(modname) + + return mocker.patch("swh.core.db.db_utils.import_swhmodule", import_swhmodule_mock) diff --git a/swh/core/db/tests/data/cli/0-superuser-init.sql b/swh/core/db/tests/data/cli/sql/0-superuser-init.sql rename from swh/core/db/tests/data/cli/0-superuser-init.sql rename to swh/core/db/tests/data/cli/sql/0-superuser-init.sql diff --git a/swh/core/db/tests/data/cli/30-schema.sql b/swh/core/db/tests/data/cli/sql/30-schema.sql rename from swh/core/db/tests/data/cli/30-schema.sql rename to swh/core/db/tests/data/cli/sql/30-schema.sql diff --git a/swh/core/db/tests/data/cli/40-funcs.sql b/swh/core/db/tests/data/cli/sql/40-funcs.sql rename from swh/core/db/tests/data/cli/40-funcs.sql rename to swh/core/db/tests/data/cli/sql/40-funcs.sql diff --git a/swh/core/db/tests/data/cli/50-data.sql b/swh/core/db/tests/data/cli/sql/50-data.sql rename from swh/core/db/tests/data/cli/50-data.sql rename to swh/core/db/tests/data/cli/sql/50-data.sql diff --git a/swh/core/db/tests/data/cli_new/0-superuser-init.sql b/swh/core/db/tests/data/cli_new/sql/0-superuser-init.sql rename from swh/core/db/tests/data/cli_new/0-superuser-init.sql rename to swh/core/db/tests/data/cli_new/sql/0-superuser-init.sql diff --git a/swh/core/db/tests/data/cli_new/30-schema.sql b/swh/core/db/tests/data/cli_new/sql/30-schema.sql rename from swh/core/db/tests/data/cli_new/30-schema.sql rename to swh/core/db/tests/data/cli_new/sql/30-schema.sql diff --git a/swh/core/db/tests/data/cli_new/40-funcs.sql b/swh/core/db/tests/data/cli_new/sql/40-funcs.sql rename from swh/core/db/tests/data/cli_new/40-funcs.sql rename to swh/core/db/tests/data/cli_new/sql/40-funcs.sql diff --git a/swh/core/db/tests/data/cli_new/50-data.sql b/swh/core/db/tests/data/cli_new/sql/50-data.sql rename from swh/core/db/tests/data/cli_new/50-data.sql rename to swh/core/db/tests/data/cli_new/sql/50-data.sql diff --git a/swh/core/db/tests/data/cli_new/upgrades/001.sql b/swh/core/db/tests/data/cli_new/sql/upgrades/001.sql rename from swh/core/db/tests/data/cli_new/upgrades/001.sql rename to swh/core/db/tests/data/cli_new/sql/upgrades/001.sql diff --git a/swh/core/db/tests/data/cli_new/upgrades/002.sql b/swh/core/db/tests/data/cli_new/sql/upgrades/002.sql rename from swh/core/db/tests/data/cli_new/upgrades/002.sql rename to swh/core/db/tests/data/cli_new/sql/upgrades/002.sql diff --git a/swh/core/db/tests/data/cli_new/upgrades/003.sql b/swh/core/db/tests/data/cli_new/sql/upgrades/003.sql rename from swh/core/db/tests/data/cli_new/upgrades/003.sql rename to swh/core/db/tests/data/cli_new/sql/upgrades/003.sql diff --git a/swh/core/db/tests/data/cli_new/upgrades/004.sql b/swh/core/db/tests/data/cli_new/sql/upgrades/004.sql rename from swh/core/db/tests/data/cli_new/upgrades/004.sql rename to swh/core/db/tests/data/cli_new/sql/upgrades/004.sql diff --git a/swh/core/db/tests/data/cli_new/upgrades/005.sql b/swh/core/db/tests/data/cli_new/sql/upgrades/005.sql rename from swh/core/db/tests/data/cli_new/upgrades/005.sql rename to swh/core/db/tests/data/cli_new/sql/upgrades/005.sql diff --git a/swh/core/db/tests/data/cli_new/upgrades/006.sql b/swh/core/db/tests/data/cli_new/sql/upgrades/006.sql rename from swh/core/db/tests/data/cli_new/upgrades/006.sql rename to swh/core/db/tests/data/cli_new/sql/upgrades/006.sql diff --git a/swh/core/db/tests/test_cli.py b/swh/core/db/tests/test_cli.py --- a/swh/core/db/tests/test_cli.py +++ b/swh/core/db/tests/test_cli.py @@ -5,14 +5,14 @@ import copy import traceback -from unittest.mock import MagicMock +import os import pytest import yaml from swh.core.cli.db import db as swhdb from swh.core.db import BaseDb -from swh.core.db.db_utils import import_swhmodule, swh_db_module +from swh.core.db.db_utils import import_swhmodule, swh_db_module, swh_db_version from swh.core.tests.test_cli import assert_section_contains @@ -79,7 +79,7 @@ return "postgresql://{user}@{host}:{port}/{dbname}".format(**params) -def test_cli_swh_db_create_and_init_db(cli_runner, postgresql, mock_package_sql): +def test_cli_swh_db_create_and_init_db(cli_runner, postgresql, mock_import_swhmodule): """Create a db then initializing it should be ok """ @@ -104,7 +104,7 @@ def test_cli_swh_db_initialization_fail_without_creation_first( - cli_runner, postgresql, mock_package_sql + cli_runner, postgresql, mock_import_swhmodule ): """Init command on an inexisting db cannot work @@ -118,7 +118,7 @@ def test_cli_swh_db_initialization_fail_without_extension( - cli_runner, postgresql, mock_package_sql + cli_runner, postgresql, mock_import_swhmodule ): """Init command cannot work without privileged extension. @@ -135,7 +135,7 @@ def test_cli_swh_db_initialization_works_with_flags( - cli_runner, postgresql, mock_package_sql + cli_runner, postgresql, mock_import_swhmodule ): """Init commands with carefully crafted libpq conninfo works @@ -157,7 +157,9 @@ assert len(origins) == 1 -def test_cli_swh_db_initialization_with_env(swh_db_cli, mock_package_sql, postgresql): +def test_cli_swh_db_initialization_with_env( + swh_db_cli, mock_import_swhmodule, postgresql +): """Init commands with standard environment variables works """ @@ -181,7 +183,9 @@ assert len(origins) == 1 -def test_cli_swh_db_initialization_idempotent(swh_db_cli, mock_package_sql, postgresql): +def test_cli_swh_db_initialization_idempotent( + swh_db_cli, mock_import_swhmodule, postgresql +): """Multiple runs of the init commands are idempotent """ @@ -217,25 +221,13 @@ def test_cli_swh_db_create_and_init_db_new_api( - cli_runner, postgresql, mock_package_sql, mocker, tmp_path + cli_runner, postgresql, mock_import_swhmodule, mocker, tmp_path ): """Create a db then initializing it should be ok for a "new style" datastore """ module_name = "test.cli_new" - def import_swhmodule_mock(modname): - if modname.startswith("test."): - - def get_datastore(cls, **kw): - # XXX probably not the best way of doing this... - return MagicMock(get_current_version=lambda: 42) - - return MagicMock(name=modname, get_datastore=get_datastore) - - return import_swhmodule(modname) - - mocker.patch("swh.core.db.db_utils.import_swhmodule", import_swhmodule_mock) conninfo = craft_conninfo(postgresql) # This initializes the schema and data @@ -257,31 +249,33 @@ assert len(origins) == 1 -def test_cli_swh_db_upgrade_new_api( - cli_runner, postgresql, mock_package_sql, mocker, tmp_path -): +def test_cli_swh_db_upgrade_new_api(cli_runner, postgresql, datadir, mocker, tmp_path): """Upgrade scenario for a "new style" datastore """ module_name = "test.cli_new" - from unittest.mock import MagicMock - - from swh.core.db.db_utils import import_swhmodule, swh_db_version - # the `current_version` variable is the version that will be returned by # any call to `get_current_version()` in this test session, thanks to the # local mocked version of import_swhmodule() below. current_version = 1 + # custom version of the mockup to make it easy to change the + # current_version returned by get_current_version() + # TODO: find a better solution for this... def import_swhmodule_mock(modname): if modname.startswith("test."): + dirname = modname.split(".", 1)[1] def get_datastore(cls, **kw): - # XXX probably not the best way of doing this... - return MagicMock(get_current_version=lambda: current_version) - - return MagicMock(name=modname, get_datastore=get_datastore) + return mocker.MagicMock(get_current_version=lambda: current_version) + + return mocker.MagicMock( + __name__=modname, + __file__=os.path.join(datadir, dirname, "__init__.py"), + name=modname, + get_datastore=get_datastore, + ) return import_swhmodule(modname) diff --git a/swh/core/db/tests/test_db_utils.py b/swh/core/db/tests/test_db_utils.py --- a/swh/core/db/tests/test_db_utils.py +++ b/swh/core/db/tests/test_db_utils.py @@ -12,6 +12,7 @@ from swh.core.db import BaseDb from swh.core.db.db_utils import ( get_database_info, + get_sql_for_package, now, swh_db_module, swh_db_upgrade, @@ -24,7 +25,19 @@ @pytest.mark.parametrize("module", ["test.cli", "test.cli_new"]) -def test_db_utils_versions(cli_runner, postgresql, mock_package_sql, module): +def test_get_sql_for_package(mock_import_swhmodule, module): + files = get_sql_for_package(module) + assert files + assert [f.name for f in files] == [ + "0-superuser-init.sql", + "30-schema.sql", + "40-funcs.sql", + "50-data.sql", + ] + + +@pytest.mark.parametrize("module", ["test.cli", "test.cli_new"]) +def test_db_utils_versions(cli_runner, postgresql, mock_import_swhmodule, module): """Check get_database_info, swh_db_versions and swh_db_module work ok This test checks db versions for both a db with "new style" set of sql init @@ -88,7 +101,9 @@ @pytest.mark.parametrize("module", ["test.cli_new"]) -def test_db_utils_upgrade(cli_runner, postgresql, mock_package_sql, module, datadir): +def test_db_utils_upgrade( + cli_runner, postgresql, mock_import_swhmodule, module, datadir +): """Check swh_db_upgrade """ @@ -107,7 +122,7 @@ # get rid of dates to ease checking versions = [(v[0], v[2]) for v in versions] assert versions[-1] == (1, "DB initialization") - sqlbasedir = path.join(datadir, module.split(".", 1)[1], "upgrades") + sqlbasedir = path.join(datadir, module.split(".", 1)[1], "sql", "upgrades") assert versions[1:-1] == [ (i, f"Upgraded to version {i} using {sqlbasedir}/{i:03d}.sql") @@ -129,7 +144,7 @@ @pytest.mark.parametrize("module", ["test.cli_new"]) def test_db_utils_swh_db_upgrade_sanity_checks( - cli_runner, postgresql, mock_package_sql, module, datadir + cli_runner, postgresql, mock_import_swhmodule, module, datadir ): """Check swh_db_upgrade