From cccf0a8ed3925e7a53c4ec91b477ce64f817d002 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 21:52:54 -0400 Subject: [PATCH 01/12] Remove sqlalchemy-utils from pyproject and pinned files --- lib/galaxy/dependencies/dev-requirements.txt | 1 - lib/galaxy/dependencies/pinned-requirements.txt | 1 - pyproject.toml | 1 - 3 files changed, 3 deletions(-) diff --git a/lib/galaxy/dependencies/dev-requirements.txt b/lib/galaxy/dependencies/dev-requirements.txt index 80dac09686e8..9eb15390fe58 100644 --- a/lib/galaxy/dependencies/dev-requirements.txt +++ b/lib/galaxy/dependencies/dev-requirements.txt @@ -214,7 +214,6 @@ sphinxcontrib-jsmath==1.0.1; python_version >= "3.5" sphinxcontrib-qthelp==1.0.3; python_version >= "3.5" sphinxcontrib-serializinghtml==1.1.4; python_version >= "3.5" sqlalchemy-migrate==0.13.0 -sqlalchemy-utils==0.36.7 sqlalchemy==1.3.23; (python_version >= "2.7" and python_full_version < "3.0.0") or (python_full_version >= "3.4.0") sqlitedict==1.7.0 sqlparse==0.4.1; python_version >= "3.5" diff --git a/lib/galaxy/dependencies/pinned-requirements.txt b/lib/galaxy/dependencies/pinned-requirements.txt index 3181af293ad7..cfc179cf595d 100644 --- a/lib/galaxy/dependencies/pinned-requirements.txt +++ b/lib/galaxy/dependencies/pinned-requirements.txt @@ -169,7 +169,6 @@ six==1.15.0; python_version >= "3.6" and python_full_version < "3.0.0" and pytho social-auth-core==3.3.0 sortedcontainers==2.3.0 sqlalchemy-migrate==0.13.0 -sqlalchemy-utils==0.36.7 sqlalchemy==1.3.23; (python_version >= "2.7" and python_full_version < "3.0.0") or (python_full_version >= "3.4.0") sqlitedict==1.7.0 sqlparse==0.4.1; python_version >= "3.5" diff --git a/pyproject.toml b/pyproject.toml index 86b00a7b277e..9c2a45cc9c69 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,7 +68,6 @@ social-auth-core = {version = "==3.3.0", extras = ["openidconnect"]} sortedcontainers = "*" SQLAlchemy = ">=1.3.22, <1.4.0" # https://github.com/kvesteri/sqlalchemy-utils/issues/474 sqlalchemy-migrate = "*" -SQLAlchemy-Utils = "!=0.36.8" # https://github.com/kvesteri/sqlalchemy-utils/issues/462 sqlitedict = "*" sqlparse = "*" starlette = "*" From db34dddbbdbd2261776d54c42648f8a5a8be4eb1 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 22:00:35 -0400 Subject: [PATCH 02/12] Add create/drop/exists db functions; tests --- lib/galaxy/model/database_utils.py | 96 ++++++++++++++++++++ test/unit/model/test_database_utils.py | 85 +++++++++++++++++ test/unit/unittest_utils/database_helpers.py | 24 +++++ 3 files changed, 205 insertions(+) create mode 100644 lib/galaxy/model/database_utils.py create mode 100644 test/unit/model/test_database_utils.py create mode 100644 test/unit/unittest_utils/database_helpers.py diff --git a/lib/galaxy/model/database_utils.py b/lib/galaxy/model/database_utils.py new file mode 100644 index 000000000000..820b09f59a1f --- /dev/null +++ b/lib/galaxy/model/database_utils.py @@ -0,0 +1,96 @@ +import sqlite3 + +from sqlalchemy import create_engine +from sqlalchemy.sql.expression import text +from sqlalchemy.sql.compiler import IdentifierPreparer +from sqlalchemy.engine import make_url + +from contextlib import contextmanager + +from galaxy.exceptions import ConfigurationError + + +def database_exists(db_url, database=None): + """Check if database exists; connect with db_url. + + If database is None, use the database name from db_url. + """ + dbm = DatabaseManager.make_manager(db_url, database) + return dbm.exists() + + +def create_database(db_url, database=None, encoding='utf8', template=None): + """Create database; connect with db_url. + + If database is None, use the database name from db_url. + """ + dbm = DatabaseManager.make_manager(db_url, database) + dbm.create(encoding, template) + + +@contextmanager +def sqlalchemy_engine(url): + engine = create_engine(url) + try: + yield engine + finally: + engine.dispose() + + +class DatabaseManager: + + @staticmethod + def make_manager(db_url, database): + if db_url.startswith('postgresql'): + return PosgresDatabaseManager(db_url, database) + elif db_url.startswith('sqlite'): + return SqliteDatabaseManager(db_url, database) + else: + raise ConfigurationError(f'Invalid database URL: {db_url}') + + def __init__(self, db_url, database): + self.url = make_url(db_url) + self.database = database + if not database: + self._handle_no_database() + + +class PosgresDatabaseManager(DatabaseManager): + + def _handle_no_database(self): + self.database = self.url.database # use database from db_url + self.url = self.url.set(database='postgres') # use default database to connect + + def exists(self): + with sqlalchemy_engine(self.url) as engine: + stmt = text('SELECT 1 FROM pg_database WHERE datname=:database') + stmt = stmt.bindparams(database=self.database) + with engine.connect() as conn: + return bool(conn.scalar(stmt)) + + def create(self, encoding, template): + with sqlalchemy_engine(self.url) as engine: + preparer = IdentifierPreparer(engine.dialect) + template = template or 'template1' + database, template = preparer.quote(self.database), preparer.quote(template) + stmt = f"CREATE DATABASE {database} ENCODING '{encoding}' TEMPLATE {template}" + with engine.connect().execution_options(isolation_level='AUTOCOMMIT') as conn: + conn.execute(stmt) + + +class SqliteDatabaseManager(DatabaseManager): + + def _handle_no_database(self): + self.database = self.url.database # use database from db_url + + def exists(self): + try: + sqlite3.connect(f'file:{self.url.database}?mode=ro', uri=True) + except sqlite3.OperationalError: + return False + else: + return True + + def create(self, encoding, template): + # Don't use encoding and template for sqlite + sqlite3.connect(f'file:{self.url.database}', uri=True) diff --git a/test/unit/model/test_database_utils.py b/test/unit/model/test_database_utils.py new file mode 100644 index 000000000000..7ea82889130f --- /dev/null +++ b/test/unit/model/test_database_utils.py @@ -0,0 +1,85 @@ +import os +import pytest +import tempfile +import uuid + +from galaxy.model.database_utils import database_exists, create_database +from ..unittest_utils.database_helpers import drop_database + + +# GALAXY_TEST_CONNECT_POSTGRES_URI='postgresql://postgres@localhost:5432/postgres' pytest test/unit/util/test_database.py +skip_if_not_postgres_uri = pytest.mark.skipif( + not os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI'), + reason="GALAXY_TEST_CONNECT_POSTGRES_URI not set" +) + + +@pytest.fixture +def database_name(): + return f'galaxytest_{uuid.uuid4().hex}' + + +@pytest.fixture +def postgres_url(): + return os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI') + + +@pytest.fixture +def sqlite_memory_url(): + return 'sqlite:///:memory:' + + +@skip_if_not_postgres_uri +def test_postgres_create_exists_drop_database(database_name, postgres_url): + assert not database_exists(postgres_url, database_name) + create_database(postgres_url, database_name) + assert database_exists(postgres_url, database_name) + drop_database(postgres_url, database_name) + assert not database_exists(postgres_url, database_name) + + +@skip_if_not_postgres_uri +def test_postgres_create_exists_drop_database__pass_one_argument(database_name, postgres_url): + # Substitute the database part of postgres_url for database_name: + # url: 'foo/db1', database: 'db2' => url: 'foo/db2' (will not work for unix domain connections) + url = postgres_url + i = url.rfind('/') + url = f'{url[:i]}/{database_name}' + + assert not database_exists(url) + create_database(url) + assert database_exists(url) + drop_database(postgres_url, database_name) + assert not database_exists(url) + + +def test_sqlite_create_exists_drop_database(database_name): + with tempfile.TemporaryDirectory() as tmp_dir: + url = _make_sqlite_url(tmp_dir, database_name) + + assert not database_exists(url, database_name) + create_database(url, database_name) + assert database_exists(url, database_name) + drop_database(url, database_name) + assert not database_exists(url, database_name) + + +def test_sqlite_create_exists_drop_database__pass_one_argument(database_name): + with tempfile.TemporaryDirectory() as tmp_dir: + url = _make_sqlite_url(tmp_dir, database_name) + + assert not database_exists(url) + create_database(url) + assert database_exists(url) + drop_database(url, database_name) + assert not database_exists(url) + + +def test_sqlite_create_exists_drop_in_memory_database(database_name, sqlite_memory_url): + assert database_exists(sqlite_memory_url) + + +def _make_sqlite_url(tmp_dir, database_name): + path = os.path.join(tmp_dir, database_name) + return f'sqlite:///{path}?isolation_level=IMMEDIATE' + diff --git a/test/unit/unittest_utils/database_helpers.py b/test/unit/unittest_utils/database_helpers.py new file mode 100644 index 000000000000..8b8474d0f1b5 --- /dev/null +++ b/test/unit/unittest_utils/database_helpers.py @@ -0,0 +1,24 @@ +import os + +from sqlalchemy.sql.compiler import IdentifierPreparer +from sqlalchemy.engine import make_url + +from galaxy.model.database_utils import sqlalchemy_engine + + +def drop_database(db_url, database): + """Drop database; connect with db_url. + + Used only for test purposes to cleanup after creating a test database. + """ + if db_url.startswith('postgresql'): + with sqlalchemy_engine(db_url) as engine: + preparer = IdentifierPreparer(engine.dialect) + database = preparer.quote(database) + stmt = f'DROP DATABASE IF EXISTS {database}' + with engine.connect().execution_options(isolation_level='AUTOCOMMIT') as conn: + conn.execute(stmt) + else: + url = make_url(db_url) + os.remove(url.database) + From 8af78a3342de017c5a79d09a7cb361527a6c21e3 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 22:03:35 -0400 Subject: [PATCH 03/12] Make test/unit/model a package --- test/unit/model/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/unit/model/__init__.py diff --git a/test/unit/model/__init__.py b/test/unit/model/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 From fdc18522014139ba8add773fc203ede93ebf6c13 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 22:08:21 -0400 Subject: [PATCH 04/12] Replace refs to sqlalchemy-utils --- lib/galaxy/config/__init__.py | 2 +- lib/galaxy/model/migrate/check.py | 2 +- lib/galaxy/model/tool_shed_install/migrate/check.py | 5 +---- lib/galaxy_test/driver/driver_util.py | 5 +---- lib/tool_shed/webapp/model/migrate/check.py | 6 ++---- test/unit/data/test_galaxy_mapping.py | 2 +- 6 files changed, 7 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index 584cd993d97b..7a5be5e45682 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -29,6 +29,7 @@ from galaxy.containers import parse_containers_config from galaxy.exceptions import ConfigurationError from galaxy.model import mapping +from galaxy.model.database_utils import database_exists from galaxy.model.tool_shed_install.migrate.check import create_or_verify_database as tsi_create_or_verify_database from galaxy.util import ( ExecutionTimer, @@ -1270,7 +1271,6 @@ def _configure_signal_handlers(self, handlers): signal.signal(sig, handler) def _wait_for_database(self, url): - from sqlalchemy_utils import database_exists attempts = self.config.database_wait_attempts pause = self.config.database_wait_sleep for i in range(1, attempts): diff --git a/lib/galaxy/model/migrate/check.py b/lib/galaxy/model/migrate/check.py index fc81b7780b82..f864ad60c726 100644 --- a/lib/galaxy/model/migrate/check.py +++ b/lib/galaxy/model/migrate/check.py @@ -9,9 +9,9 @@ Table ) from sqlalchemy.exc import NoSuchTableError -from sqlalchemy_utils import create_database, database_exists from galaxy.model import mapping +from galaxy.model.database_utils import create_database, database_exists log = logging.getLogger(__name__) diff --git a/lib/galaxy/model/tool_shed_install/migrate/check.py b/lib/galaxy/model/tool_shed_install/migrate/check.py index 92176c541f5c..0e1a66248fc4 100644 --- a/lib/galaxy/model/tool_shed_install/migrate/check.py +++ b/lib/galaxy/model/tool_shed_install/migrate/check.py @@ -12,11 +12,8 @@ Table ) from sqlalchemy.exc import NoSuchTableError -from sqlalchemy_utils import ( - create_database, - database_exists, -) +from galaxy.model.database_utils import create_database, database_exists from galaxy.model.tool_shed_install import mapping diff --git a/lib/galaxy_test/driver/driver_util.py b/lib/galaxy_test/driver/driver_util.py index 8f8681fb8cea..7f6108f56f7e 100644 --- a/lib/galaxy_test/driver/driver_util.py +++ b/lib/galaxy_test/driver/driver_util.py @@ -22,14 +22,11 @@ import nose.plugins.manager import yaml from paste import httpserver -from sqlalchemy_utils import ( - create_database, - database_exists, -) from galaxy.app import UniverseApplication as GalaxyUniverseApplication from galaxy.config import LOGGING_CONFIG_DEFAULT from galaxy.model import mapping +from galaxy.model.database_utils import create_database, database_exists from galaxy.model.tool_shed_install import mapping as toolshed_mapping from galaxy.tool_util.verify.interactor import GalaxyInteractorApi, verify_tool from galaxy.util import asbool, download_to_file, galaxy_directory diff --git a/lib/tool_shed/webapp/model/migrate/check.py b/lib/tool_shed/webapp/model/migrate/check.py index 85bd5998920d..2e37c4840b1b 100644 --- a/lib/tool_shed/webapp/model/migrate/check.py +++ b/lib/tool_shed/webapp/model/migrate/check.py @@ -5,10 +5,8 @@ from migrate.versioning import repository, schema from sqlalchemy import create_engine, MetaData, Table from sqlalchemy.exc import NoSuchTableError -from sqlalchemy_utils import ( - create_database, - database_exists, -) + +from galaxy.model.database_utils import create_database, database_exists log = logging.getLogger(__name__) diff --git a/test/unit/data/test_galaxy_mapping.py b/test/unit/data/test_galaxy_mapping.py index 2db5e6fe9d64..0bb99dae0254 100644 --- a/test/unit/data/test_galaxy_mapping.py +++ b/test/unit/data/test_galaxy_mapping.py @@ -5,10 +5,10 @@ import pytest from sqlalchemy import inspect -from sqlalchemy_utils import create_database import galaxy.datatypes.registry import galaxy.model +from galaxy.model.database_utils import create_database import galaxy.model.mapping as mapping from galaxy.model.security import GalaxyRBACAgent From a708d62d49aa265692309e66b3959535c33a350e Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 22:10:32 -0400 Subject: [PATCH 05/12] Implement view handling --- lib/galaxy/model/view/__init__.py | 13 ++++--- lib/galaxy/model/view/utils.py | 64 ++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/model/view/__init__.py b/lib/galaxy/model/view/__init__.py index 6546fce74b42..c741a3e0d7b8 100644 --- a/lib/galaxy/model/view/__init__.py +++ b/lib/galaxy/model/view/__init__.py @@ -1,14 +1,12 @@ """ Galaxy sql view models """ -from sqlalchemy import Integer, MetaData +from sqlalchemy import Integer from sqlalchemy.orm import mapper from sqlalchemy.sql import column, text -from sqlalchemy_utils import create_view from .utils import View - -metadata = MetaData() +from galaxy.model.view.utils import create_view AGGREGATE_STATE_QUERY = """ SELECT @@ -38,9 +36,11 @@ class HistoryDatasetCollectionJobStateSummary(View): + name = 'collection_job_state_summary_view' + pkey = 'hdca_id' __view__ = text(AGGREGATE_STATE_QUERY).columns( - column('hdca_id', Integer), + column(pkey, Integer), column('new', Integer), column('resubmitted', Integer), column('waiting', Integer), @@ -56,7 +56,8 @@ class HistoryDatasetCollectionJobStateSummary(View): column('all_jobs', Integer) ) - __table__ = create_view('collection_job_state_summary_view', __view__, metadata) + __table__ = create_view(name, __view__, pkey) mapper(HistoryDatasetCollectionJobStateSummary, HistoryDatasetCollectionJobStateSummary.__table__) + diff --git a/lib/galaxy/model/view/utils.py b/lib/galaxy/model/view/utils.py index cbc6f55fabbe..5c3c03a776e4 100644 --- a/lib/galaxy/model/view/utils.py +++ b/lib/galaxy/model/view/utils.py @@ -3,32 +3,40 @@ """ from inspect import getmembers +from sqlalchemy import ( + Column, + MetaData, + Table, + event +) +from sqlalchemy.schema import DDLElement from sqlalchemy.ext import compiler -from sqlalchemy_utils import view class View: is_view = True -class DropView(view.DropView): - def __init__(self, ViewModel, **kwargs): - super().__init__(str(ViewModel.__table__.name), **kwargs) +class CreateView(DDLElement): + def __init__(self, name, selectable): + self.name = name + self.selectable = selectable -@compiler.compiles(DropView, "sqlite") -def compile_drop_materialized_view(element, compiler, **kw): - # modified because sqlalchemy_utils adds a cascade for - # sqlite even though sqlite does not support cascade keyword - return 'DROP {}VIEW IF EXISTS {}'.format( - 'MATERIALIZED ' if element.materialized else '', - element.name - ) +@compiler.compiles(CreateView) +def compile_create_view(element, compiler, **kw): + compiled_selectable = compiler.sql_compiler.process(element.selectable, literal_binds=True) + return f'CREATE VIEW {element.name} AS {compiled_selectable}' -class CreateView(view.CreateView): - def __init__(self, ViewModel, **kwargs): - super().__init__(str(ViewModel.__table__.name), ViewModel.__view__, **kwargs) +class DropView(DDLElement): + def __init__(self, name): + self.name = name + + +@compiler.compiles(DropView) +def compile_drop_view(element, compiler, **kw): + return f'DROP VIEW IF EXISTS {element.name}' def is_view_model(o): @@ -38,10 +46,30 @@ def is_view_model(o): def install_views(engine): import galaxy.model.view views = getmembers(galaxy.model.view, is_view_model) - for _name, ViewModel in views: + for _, view in views: # adding DropView here because our unit-testing calls this function when # it mocks the app and CreateView will attempt to rebuild an existing # view in a database that is already made, the right answer is probably # to change the sql that gest emitted when CreateView is rendered. - engine.execute(DropView(ViewModel)) - engine.execute(CreateView(ViewModel)) + engine.execute(DropView(view.name)) + engine.execute(CreateView(view.name, view.__view__)) + + +def create_view(name, selectable, pkey): + metadata = MetaData() + + columns = [ + Column( + c.name, + c.type, + primary_key=(c.name == pkey) + ) + for c in selectable.subquery().c + ] + table = Table(name, metadata, *columns) + + event.listen(metadata, 'after_create', CreateView(name, selectable)) + event.listen(metadata, 'before_drop', DropView(name)) + + return table + From 66f4e81cef94aa281d75d28bba64963868f079a1 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 22:19:07 -0400 Subject: [PATCH 06/12] Downgrade from SA 1.4 to SA 1.3 --- lib/galaxy/model/database_utils.py | 4 ++-- lib/galaxy/model/view/utils.py | 2 +- test/unit/unittest_utils/database_helpers.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/model/database_utils.py b/lib/galaxy/model/database_utils.py index 820b09f59a1f..06b85e51c474 100644 --- a/lib/galaxy/model/database_utils.py +++ b/lib/galaxy/model/database_utils.py @@ -3,7 +3,7 @@ from sqlalchemy import create_engine from sqlalchemy.sql.expression import text from sqlalchemy.sql.compiler import IdentifierPreparer -from sqlalchemy.engine import make_url +from sqlalchemy.engine.url import make_url from contextlib import contextmanager @@ -59,7 +59,7 @@ class PosgresDatabaseManager(DatabaseManager): def _handle_no_database(self): self.database = self.url.database # use database from db_url - self.url = self.url.set(database='postgres') # use default database to connect + self.url.database = 'postgres' def exists(self): with sqlalchemy_engine(self.url) as engine: diff --git a/lib/galaxy/model/view/utils.py b/lib/galaxy/model/view/utils.py index 5c3c03a776e4..b337f11ec7f5 100644 --- a/lib/galaxy/model/view/utils.py +++ b/lib/galaxy/model/view/utils.py @@ -64,7 +64,7 @@ def create_view(name, selectable, pkey): c.type, primary_key=(c.name == pkey) ) - for c in selectable.subquery().c + for c in selectable.c ] table = Table(name, metadata, *columns) diff --git a/test/unit/unittest_utils/database_helpers.py b/test/unit/unittest_utils/database_helpers.py index 8b8474d0f1b5..69b8cc9cfdef 100644 --- a/test/unit/unittest_utils/database_helpers.py +++ b/test/unit/unittest_utils/database_helpers.py @@ -1,7 +1,7 @@ import os from sqlalchemy.sql.compiler import IdentifierPreparer -from sqlalchemy.engine import make_url +from sqlalchemy.engine.url import make_url from galaxy.model.database_utils import sqlalchemy_engine From d40dabbe126fd4e26341551237c4b3ba7bc65a4b Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Sun, 21 Mar 2021 22:32:54 -0400 Subject: [PATCH 07/12] Fix linting errors --- lib/galaxy/model/database_utils.py | 7 +++---- lib/galaxy/model/view/__init__.py | 3 +-- lib/galaxy/model/view/utils.py | 5 ++--- test/unit/data/test_galaxy_mapping.py | 2 +- test/unit/model/test_database_utils.py | 9 ++++++--- test/unit/unittest_utils/database_helpers.py | 3 +-- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/model/database_utils.py b/lib/galaxy/model/database_utils.py index 06b85e51c474..e19506b11c8f 100644 --- a/lib/galaxy/model/database_utils.py +++ b/lib/galaxy/model/database_utils.py @@ -1,11 +1,10 @@ import sqlite3 +from contextlib import contextmanager from sqlalchemy import create_engine -from sqlalchemy.sql.expression import text -from sqlalchemy.sql.compiler import IdentifierPreparer from sqlalchemy.engine.url import make_url - -from contextlib import contextmanager +from sqlalchemy.sql.compiler import IdentifierPreparer +from sqlalchemy.sql.expression import text from galaxy.exceptions import ConfigurationError diff --git a/lib/galaxy/model/view/__init__.py b/lib/galaxy/model/view/__init__.py index c741a3e0d7b8..91484c34b5af 100644 --- a/lib/galaxy/model/view/__init__.py +++ b/lib/galaxy/model/view/__init__.py @@ -5,8 +5,8 @@ from sqlalchemy.orm import mapper from sqlalchemy.sql import column, text -from .utils import View from galaxy.model.view.utils import create_view +from .utils import View AGGREGATE_STATE_QUERY = """ SELECT @@ -60,4 +60,3 @@ class HistoryDatasetCollectionJobStateSummary(View): mapper(HistoryDatasetCollectionJobStateSummary, HistoryDatasetCollectionJobStateSummary.__table__) - diff --git a/lib/galaxy/model/view/utils.py b/lib/galaxy/model/view/utils.py index b337f11ec7f5..bf91e0a7a782 100644 --- a/lib/galaxy/model/view/utils.py +++ b/lib/galaxy/model/view/utils.py @@ -5,12 +5,12 @@ from sqlalchemy import ( Column, + event, MetaData, Table, - event ) -from sqlalchemy.schema import DDLElement from sqlalchemy.ext import compiler +from sqlalchemy.schema import DDLElement class View: @@ -72,4 +72,3 @@ def create_view(name, selectable, pkey): event.listen(metadata, 'before_drop', DropView(name)) return table - diff --git a/test/unit/data/test_galaxy_mapping.py b/test/unit/data/test_galaxy_mapping.py index 0bb99dae0254..f23808c15ae4 100644 --- a/test/unit/data/test_galaxy_mapping.py +++ b/test/unit/data/test_galaxy_mapping.py @@ -8,8 +8,8 @@ import galaxy.datatypes.registry import galaxy.model -from galaxy.model.database_utils import create_database import galaxy.model.mapping as mapping +from galaxy.model.database_utils import create_database from galaxy.model.security import GalaxyRBACAgent datatypes_registry = galaxy.datatypes.registry.Registry() diff --git a/test/unit/model/test_database_utils.py b/test/unit/model/test_database_utils.py index 7ea82889130f..0261e63b5a60 100644 --- a/test/unit/model/test_database_utils.py +++ b/test/unit/model/test_database_utils.py @@ -1,9 +1,13 @@ import os -import pytest import tempfile import uuid -from galaxy.model.database_utils import database_exists, create_database +import pytest + +from galaxy.model.database_utils import ( + create_database, + database_exists, +) from ..unittest_utils.database_helpers import drop_database @@ -82,4 +86,3 @@ def test_sqlite_create_exists_drop_in_memory_database(database_name, sqlite_memo def _make_sqlite_url(tmp_dir, database_name): path = os.path.join(tmp_dir, database_name) return f'sqlite:///{path}?isolation_level=IMMEDIATE' - diff --git a/test/unit/unittest_utils/database_helpers.py b/test/unit/unittest_utils/database_helpers.py index 69b8cc9cfdef..47cebd66efec 100644 --- a/test/unit/unittest_utils/database_helpers.py +++ b/test/unit/unittest_utils/database_helpers.py @@ -1,7 +1,7 @@ import os -from sqlalchemy.sql.compiler import IdentifierPreparer from sqlalchemy.engine.url import make_url +from sqlalchemy.sql.compiler import IdentifierPreparer from galaxy.model.database_utils import sqlalchemy_engine @@ -21,4 +21,3 @@ def drop_database(db_url, database): else: url = make_url(db_url) os.remove(url.database) - From acb4bf61b274659091140ec0b4237d453cef389f Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Mon, 22 Mar 2021 00:54:35 -0400 Subject: [PATCH 08/12] Update migration to use updated signature --- .../model/migrate/versions/0166_job_state_summary_view.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/migrate/versions/0166_job_state_summary_view.py b/lib/galaxy/model/migrate/versions/0166_job_state_summary_view.py index 74d5b924babc..062224e7abd9 100644 --- a/lib/galaxy/model/migrate/versions/0166_job_state_summary_view.py +++ b/lib/galaxy/model/migrate/versions/0166_job_state_summary_view.py @@ -15,12 +15,13 @@ def upgrade(migrate_engine): print(__doc__) # drop first because sqlite does not support or_replace downgrade(migrate_engine) - create_view = CreateView(HistoryDatasetCollectionJobStateSummary) + view = HistoryDatasetCollectionJobStateSummary + create_view = CreateView(view.name, view.__view__) # print(str(create_view.compile(migrate_engine))) migrate_engine.execute(create_view) def downgrade(migrate_engine): - drop_view = DropView(HistoryDatasetCollectionJobStateSummary) + drop_view = DropView(HistoryDatasetCollectionJobStateSummary.name) # print(str(drop_view.compile(migrate_engine))) migrate_engine.execute(drop_view) From ec1ad828e9b680c04d08da85dac00739e153975d Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Mon, 22 Mar 2021 01:03:15 -0400 Subject: [PATCH 09/12] Allow postgres as prefix for valid conn string --- lib/galaxy/model/database_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/model/database_utils.py b/lib/galaxy/model/database_utils.py index e19506b11c8f..311a44bcc2d2 100644 --- a/lib/galaxy/model/database_utils.py +++ b/lib/galaxy/model/database_utils.py @@ -40,7 +40,7 @@ class DatabaseManager: @staticmethod def make_manager(db_url, database): - if db_url.startswith('postgresql'): + if db_url.startswith('postgres'): return PosgresDatabaseManager(db_url, database) elif db_url.startswith('sqlite'): return SqliteDatabaseManager(db_url, database) From 49ca9358bc5360fc4a80d22eba6df1a1b697eeb3 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Tue, 23 Mar 2021 23:23:45 -0400 Subject: [PATCH 10/12] Add view tests, refactor --- lib/galaxy/model/database_utils.py | 22 +++-- lib/galaxy/model/view/__init__.py | 10 +-- lib/galaxy/model/view/utils.py | 60 +++++++------- .../database_helpers.py => model/common.py} | 18 +++++ test/unit/model/conftest.py | 19 +++++ test/unit/model/test_database_utils.py | 55 ++++--------- test/unit/model/test_views.py | 81 +++++++++++++++++++ 7 files changed, 185 insertions(+), 80 deletions(-) rename test/unit/{unittest_utils/database_helpers.py => model/common.py} (56%) create mode 100644 test/unit/model/conftest.py create mode 100644 test/unit/model/test_views.py diff --git a/lib/galaxy/model/database_utils.py b/lib/galaxy/model/database_utils.py index 311a44bcc2d2..af1926213cf6 100644 --- a/lib/galaxy/model/database_utils.py +++ b/lib/galaxy/model/database_utils.py @@ -83,13 +83,19 @@ def _handle_no_database(self): self.database = self.url.database # use database from db_url def exists(self): - try: - sqlite3.connect(f'file:{self.url.database}?mode=ro', uri=True) - except sqlite3.OperationalError: - return False - else: - return True - def create(self, encoding, template): - # Don't use encoding and template for sqlite + def can_connect_to_dbfile(): + try: + sqlite3.connect(f'file:{db}?mode=ro', uri=True) + except sqlite3.OperationalError: + return False + else: + return True + + db = self.url.database + # No database or ':memory:' creates an in-memory database + return not db or db == ':memory:' or can_connect_to_dbfile() + + def create(self, *args): + # Ignore any args (encoding, template) sqlite3.connect(f'file:{self.url.database}', uri=True) diff --git a/lib/galaxy/model/view/__init__.py b/lib/galaxy/model/view/__init__.py index 91484c34b5af..c32d55aa357f 100644 --- a/lib/galaxy/model/view/__init__.py +++ b/lib/galaxy/model/view/__init__.py @@ -5,8 +5,7 @@ from sqlalchemy.orm import mapper from sqlalchemy.sql import column, text -from galaxy.model.view.utils import create_view -from .utils import View +from galaxy.model.view.utils import View AGGREGATE_STATE_QUERY = """ SELECT @@ -37,10 +36,9 @@ class HistoryDatasetCollectionJobStateSummary(View): name = 'collection_job_state_summary_view' - pkey = 'hdca_id' __view__ = text(AGGREGATE_STATE_QUERY).columns( - column(pkey, Integer), + column('hdca_id', Integer), column('new', Integer), column('resubmitted', Integer), column('waiting', Integer), @@ -55,8 +53,8 @@ class HistoryDatasetCollectionJobStateSummary(View): column('upload', Integer), column('all_jobs', Integer) ) - - __table__ = create_view(name, __view__, pkey) + pkeys = {'hdca_id'} + View._make_table(name, __view__, pkeys) mapper(HistoryDatasetCollectionJobStateSummary, HistoryDatasetCollectionJobStateSummary.__table__) diff --git a/lib/galaxy/model/view/utils.py b/lib/galaxy/model/view/utils.py index bf91e0a7a782..a741fd3a6523 100644 --- a/lib/galaxy/model/view/utils.py +++ b/lib/galaxy/model/view/utils.py @@ -1,11 +1,10 @@ """ -View wrappers, currently using sqlalchemy_views +View wrappers """ from inspect import getmembers from sqlalchemy import ( Column, - event, MetaData, Table, ) @@ -14,7 +13,33 @@ class View: - is_view = True + """Base class for Views.""" + + @classmethod + def _make_table(cls, name, selectable, pkeys): + """ Create a view. + + :param name: The name of the view. + :param selectable: SQLAlchemy selectable. + :param pkeys: set of primary keys for the selectable. + """ + columns = [ + Column( + c.name, + c.type, + primary_key=(c.name in pkeys) + ) + for c in selectable.c + ] + # We do not use the metadata object from model.mapping.py that contains all the Table objects + # because that would create a circular import (create_view is called from View objects + # in model.view; but those View objects are imported into model.mapping.py where the + # metadata object we need is defined). Thus, we do not use the after_create/before_drop + # hooks to automate creating/dropping views. Instead, this is taken care of in install_views(). + + # The metadata object passed to Table() should be empty: this table is internal to a View + # object and is not intended to be created in the database. + cls.__table__ = Table(name, MetaData(), *columns) class CreateView(DDLElement): @@ -23,17 +48,17 @@ def __init__(self, name, selectable): self.selectable = selectable +class DropView(DDLElement): + def __init__(self, name): + self.name = name + + @compiler.compiles(CreateView) def compile_create_view(element, compiler, **kw): compiled_selectable = compiler.sql_compiler.process(element.selectable, literal_binds=True) return f'CREATE VIEW {element.name} AS {compiled_selectable}' -class DropView(DDLElement): - def __init__(self, name): - self.name = name - - @compiler.compiles(DropView) def compile_drop_view(element, compiler, **kw): return f'DROP VIEW IF EXISTS {element.name}' @@ -53,22 +78,3 @@ def install_views(engine): # to change the sql that gest emitted when CreateView is rendered. engine.execute(DropView(view.name)) engine.execute(CreateView(view.name, view.__view__)) - - -def create_view(name, selectable, pkey): - metadata = MetaData() - - columns = [ - Column( - c.name, - c.type, - primary_key=(c.name == pkey) - ) - for c in selectable.c - ] - table = Table(name, metadata, *columns) - - event.listen(metadata, 'after_create', CreateView(name, selectable)) - event.listen(metadata, 'before_drop', DropView(name)) - - return table diff --git a/test/unit/unittest_utils/database_helpers.py b/test/unit/model/common.py similarity index 56% rename from test/unit/unittest_utils/database_helpers.py rename to test/unit/model/common.py index 47cebd66efec..9c396dbb70d9 100644 --- a/test/unit/unittest_utils/database_helpers.py +++ b/test/unit/model/common.py @@ -1,10 +1,28 @@ import os +import pytest from sqlalchemy.engine.url import make_url from sqlalchemy.sql.compiler import IdentifierPreparer from galaxy.model.database_utils import sqlalchemy_engine +# GALAXY_TEST_CONNECT_POSTGRES_URI='postgresql://postgres@localhost:5432/postgres' pytest test/unit/util/test_database.py +skip_if_not_postgres_uri = pytest.mark.skipif( + not os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI'), + reason="GALAXY_TEST_CONNECT_POSTGRES_URI not set" +) + + +def replace_database_in_url(url, database_name): + """ + Substitute the database part of url for database_name. + + Example: replace_database_in_url('foo/db1', 'db2') returns 'foo/db2' + This will not work for unix domain connections. + """ + i = url.rfind('/') + return f'{url[:i]}/{database_name}' + def drop_database(db_url, database): """Drop database; connect with db_url. diff --git a/test/unit/model/conftest.py b/test/unit/model/conftest.py new file mode 100644 index 000000000000..960550ce2c4a --- /dev/null +++ b/test/unit/model/conftest.py @@ -0,0 +1,19 @@ +import os +import uuid + +import pytest + + +@pytest.fixture +def database_name(): + return f'galaxytest_{uuid.uuid4().hex}' + + +@pytest.fixture +def postgres_url(): + return os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI') + + +@pytest.fixture +def sqlite_memory_url(): + return 'sqlite:///:memory:' diff --git a/test/unit/model/test_database_utils.py b/test/unit/model/test_database_utils.py index 0261e63b5a60..4d995ec2c19b 100644 --- a/test/unit/model/test_database_utils.py +++ b/test/unit/model/test_database_utils.py @@ -1,40 +1,19 @@ import os import tempfile -import uuid - -import pytest from galaxy.model.database_utils import ( create_database, database_exists, ) -from ..unittest_utils.database_helpers import drop_database - - -# GALAXY_TEST_CONNECT_POSTGRES_URI='postgresql://postgres@localhost:5432/postgres' pytest test/unit/util/test_database.py -skip_if_not_postgres_uri = pytest.mark.skipif( - not os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI'), - reason="GALAXY_TEST_CONNECT_POSTGRES_URI not set" +from .common import ( + drop_database, + replace_database_in_url, + skip_if_not_postgres_uri, ) -@pytest.fixture -def database_name(): - return f'galaxytest_{uuid.uuid4().hex}' - - -@pytest.fixture -def postgres_url(): - return os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI') - - -@pytest.fixture -def sqlite_memory_url(): - return 'sqlite:///:memory:' - - @skip_if_not_postgres_uri -def test_postgres_create_exists_drop_database(database_name, postgres_url): +def test_create_exists_postgres_database(database_name, postgres_url): assert not database_exists(postgres_url, database_name) create_database(postgres_url, database_name) assert database_exists(postgres_url, database_name) @@ -43,12 +22,9 @@ def test_postgres_create_exists_drop_database(database_name, postgres_url): @skip_if_not_postgres_uri -def test_postgres_create_exists_drop_database__pass_one_argument(database_name, postgres_url): - # Substitute the database part of postgres_url for database_name: - # url: 'foo/db1', database: 'db2' => url: 'foo/db2' (will not work for unix domain connections) - url = postgres_url - i = url.rfind('/') - url = f'{url[:i]}/{database_name}' +def test_create_exists_postgres_database__pass_as_url(database_name, postgres_url): + # the database in the url is the one to create/check + url = replace_database_in_url(postgres_url, database_name) assert not database_exists(url) create_database(url) @@ -57,9 +33,9 @@ def test_postgres_create_exists_drop_database__pass_one_argument(database_name, assert not database_exists(url) -def test_sqlite_create_exists_drop_database(database_name): +def test_create_exists_sqlite_database(database_name): with tempfile.TemporaryDirectory() as tmp_dir: - url = _make_sqlite_url(tmp_dir, database_name) + url = make_sqlite_url(tmp_dir, database_name) assert not database_exists(url, database_name) create_database(url, database_name) @@ -68,9 +44,10 @@ def test_sqlite_create_exists_drop_database(database_name): assert not database_exists(url, database_name) -def test_sqlite_create_exists_drop_database__pass_one_argument(database_name): +def test_create_exists_sqlite_database__pass_as_url(database_name): + # the database in the url is the one to create/check with tempfile.TemporaryDirectory() as tmp_dir: - url = _make_sqlite_url(tmp_dir, database_name) + url = make_sqlite_url(tmp_dir, database_name) assert not database_exists(url) create_database(url) @@ -79,10 +56,10 @@ def test_sqlite_create_exists_drop_database__pass_one_argument(database_name): assert not database_exists(url) -def test_sqlite_create_exists_drop_in_memory_database(database_name, sqlite_memory_url): +def test_exists_sqlite_in_memory_database(database_name, sqlite_memory_url): assert database_exists(sqlite_memory_url) -def _make_sqlite_url(tmp_dir, database_name): +def make_sqlite_url(tmp_dir, database_name): path = os.path.join(tmp_dir, database_name) - return f'sqlite:///{path}?isolation_level=IMMEDIATE' + return f'sqlite:///{path}' diff --git a/test/unit/model/test_views.py b/test/unit/model/test_views.py new file mode 100644 index 000000000000..d248f669bf9d --- /dev/null +++ b/test/unit/model/test_views.py @@ -0,0 +1,81 @@ +import pytest +from sqlalchemy import ( + Column, + Integer, + MetaData, + String, + Table, +) +from sqlalchemy.sql import ( + column, + text, +) + +from galaxy.model.database_utils import ( + create_database, + sqlalchemy_engine, +) +from galaxy.model.view.utils import ( + CreateView, + View, +) +from .common import ( + drop_database, + replace_database_in_url, + skip_if_not_postgres_uri, +) + + +@pytest.fixture +def view(): + # A View class we would add to galaxy.model.view + class TestView(View): + name = 'testview' + __view__ = text('select id, first_name from testusers').columns( + column('id', Integer), + column('first_name', String) + ) + pkeys = {'id'} + View._make_table(name, __view__, pkeys) + + return TestView + + +@skip_if_not_postgres_uri +def test_postgres_create_view(database_name, postgres_url, view): + metadata = MetaData() + make_table(metadata) # table from which the view will select + create_database(postgres_url, database_name) + + url = replace_database_in_url(postgres_url, database_name) + with sqlalchemy_engine(url) as engine: + with engine.connect() as conn: + metadata.create_all(conn) # create table in database + conn.execute(CreateView(view.name, view.__view__)) # create view in database + query = f"select * from information_schema.tables where table_name = '{view.name}' and table_type = 'VIEW'" + result = conn.execute(query) + assert result.rowcount == 1 # assert that view exists in database + + drop_database(postgres_url, database_name) + + +def test_sqlite_create_view(sqlite_memory_url, view): + metadata = MetaData() + make_table(metadata) # table from which the view will select + + with sqlalchemy_engine(sqlite_memory_url) as engine: + with engine.connect() as conn: + metadata.create_all(conn) # create table in database + conn.execute(CreateView(view.name, view.__view__)) # create view in database + query = f"SELECT name FROM sqlite_master WHERE type='view' AND name='{view.name}'" + result = conn.execute(query).fetchall() + assert len(result) == 1 # assert that view exists in database + + +def make_table(metadata): + users = Table('testusers', metadata, + Column('id', Integer, primary_key=True), + Column('first_name', String), + Column('last_name', String) + ) + return users From 4d8b185f63d3e5a0da37703a4d74c8cfb27a20f4 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Tue, 23 Mar 2021 23:36:26 -0400 Subject: [PATCH 11/12] Do not use add class attr via classmethod: fix mypy --- lib/galaxy/model/view/__init__.py | 2 +- lib/galaxy/model/view/utils.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/model/view/__init__.py b/lib/galaxy/model/view/__init__.py index c32d55aa357f..ae1958da9fbd 100644 --- a/lib/galaxy/model/view/__init__.py +++ b/lib/galaxy/model/view/__init__.py @@ -54,7 +54,7 @@ class HistoryDatasetCollectionJobStateSummary(View): column('all_jobs', Integer) ) pkeys = {'hdca_id'} - View._make_table(name, __view__, pkeys) + __table__ = View._make_table(name, __view__, pkeys) mapper(HistoryDatasetCollectionJobStateSummary, HistoryDatasetCollectionJobStateSummary.__table__) diff --git a/lib/galaxy/model/view/utils.py b/lib/galaxy/model/view/utils.py index a741fd3a6523..075be7bba083 100644 --- a/lib/galaxy/model/view/utils.py +++ b/lib/galaxy/model/view/utils.py @@ -15,8 +15,8 @@ class View: """Base class for Views.""" - @classmethod - def _make_table(cls, name, selectable, pkeys): + @staticmethod + def _make_table(name, selectable, pkeys): """ Create a view. :param name: The name of the view. @@ -39,7 +39,7 @@ def _make_table(cls, name, selectable, pkeys): # The metadata object passed to Table() should be empty: this table is internal to a View # object and is not intended to be created in the database. - cls.__table__ = Table(name, MetaData(), *columns) + return Table(name, MetaData(), *columns) class CreateView(DDLElement): From 57782d5ff6a6860cddd1aa86a57098eea854c7b3 Mon Sep 17 00:00:00 2001 From: Sergey Golitsynskiy Date: Wed, 24 Mar 2021 02:02:49 -0400 Subject: [PATCH 12/12] Add MySQL functionality, tests; refactor --- lib/galaxy/model/database_utils.py | 24 ++++++++++++ test/unit/model/common.py | 10 ++++- test/unit/model/conftest.py | 5 +++ test/unit/model/test_database_utils.py | 10 +++++ test/unit/model/test_views.py | 54 +++++++++++++++----------- 5 files changed, 78 insertions(+), 25 deletions(-) diff --git a/lib/galaxy/model/database_utils.py b/lib/galaxy/model/database_utils.py index af1926213cf6..5a949f0a0a3f 100644 --- a/lib/galaxy/model/database_utils.py +++ b/lib/galaxy/model/database_utils.py @@ -44,6 +44,8 @@ def make_manager(db_url, database): return PosgresDatabaseManager(db_url, database) elif db_url.startswith('sqlite'): return SqliteDatabaseManager(db_url, database) + elif db_url.startswith('mysql'): + return MySQLDatabaseManager(db_url, database) else: raise ConfigurationError(f'Invalid database URL: {db_url}') @@ -99,3 +101,25 @@ def can_connect_to_dbfile(): def create(self, *args): # Ignore any args (encoding, template) sqlite3.connect(f'file:{self.url.database}', uri=True) + + +class MySQLDatabaseManager(DatabaseManager): + + def _handle_no_database(self): + self.database = self.url.database # use database from db_url + + def exists(self): + with sqlalchemy_engine(self.url) as engine: + stmt = text("SELECT schema_name FROM information_schema.schemata WHERE schema_name=:database") + stmt = stmt.bindparams(database=self.database) + with engine.connect() as conn: + return bool(conn.scalar(stmt)) + + def create(self, encoding, *arg): + # Ignore any args (template) + with sqlalchemy_engine(self.url) as engine: + preparer = IdentifierPreparer(engine.dialect) + database = preparer.quote(self.database) + stmt = f"CREATE DATABASE {database} CHARACTER SET = '{encoding}'" + with engine.connect().execution_options(isolation_level='AUTOCOMMIT') as conn: + conn.execute(stmt) diff --git a/test/unit/model/common.py b/test/unit/model/common.py index 9c396dbb70d9..3f9386b5fe8d 100644 --- a/test/unit/model/common.py +++ b/test/unit/model/common.py @@ -6,12 +6,18 @@ from galaxy.model.database_utils import sqlalchemy_engine -# GALAXY_TEST_CONNECT_POSTGRES_URI='postgresql://postgres@localhost:5432/postgres' pytest test/unit/util/test_database.py +# GALAXY_TEST_CONNECT_POSTGRES_URI='postgresql://postgres@localhost:5432/postgres' pytest test/unit/model skip_if_not_postgres_uri = pytest.mark.skipif( not os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI'), reason="GALAXY_TEST_CONNECT_POSTGRES_URI not set" ) +# GALAXY_TEST_CONNECT_MYSQL_URI='mysql+mysqldb://root@localhost/mysql' pytest test/unit/model +skip_if_not_mysql_uri = pytest.mark.skipif( + not os.environ.get('GALAXY_TEST_CONNECT_MYSQL_URI'), + reason="GALAXY_TEST_CONNECT_MYSQL_URI not set" +) + def replace_database_in_url(url, database_name): """ @@ -29,7 +35,7 @@ def drop_database(db_url, database): Used only for test purposes to cleanup after creating a test database. """ - if db_url.startswith('postgresql'): + if db_url.startswith('postgresql') or db_url.startswith('mysql'): with sqlalchemy_engine(db_url) as engine: preparer = IdentifierPreparer(engine.dialect) database = preparer.quote(database) diff --git a/test/unit/model/conftest.py b/test/unit/model/conftest.py index 960550ce2c4a..02f5252c7b17 100644 --- a/test/unit/model/conftest.py +++ b/test/unit/model/conftest.py @@ -14,6 +14,11 @@ def postgres_url(): return os.environ.get('GALAXY_TEST_CONNECT_POSTGRES_URI') +@pytest.fixture +def mysql_url(): + return os.environ.get('GALAXY_TEST_CONNECT_MYSQL_URI') + + @pytest.fixture def sqlite_memory_url(): return 'sqlite:///:memory:' diff --git a/test/unit/model/test_database_utils.py b/test/unit/model/test_database_utils.py index 4d995ec2c19b..72cbf17c860c 100644 --- a/test/unit/model/test_database_utils.py +++ b/test/unit/model/test_database_utils.py @@ -8,6 +8,7 @@ from .common import ( drop_database, replace_database_in_url, + skip_if_not_mysql_uri, skip_if_not_postgres_uri, ) @@ -60,6 +61,15 @@ def test_exists_sqlite_in_memory_database(database_name, sqlite_memory_url): assert database_exists(sqlite_memory_url) +@skip_if_not_mysql_uri +def test_create_exists_mysql_database(database_name, mysql_url): + assert not database_exists(mysql_url, database_name) + create_database(mysql_url, database_name) + assert database_exists(mysql_url, database_name) + drop_database(mysql_url, database_name) + assert not database_exists(mysql_url, database_name) + + def make_sqlite_url(tmp_dir, database_name): path = os.path.join(tmp_dir, database_name) return f'sqlite:///{path}' diff --git a/test/unit/model/test_views.py b/test/unit/model/test_views.py index d248f669bf9d..990b0c6c6dbe 100644 --- a/test/unit/model/test_views.py +++ b/test/unit/model/test_views.py @@ -3,7 +3,6 @@ Column, Integer, MetaData, - String, Table, ) from sqlalchemy.sql import ( @@ -22,6 +21,7 @@ from .common import ( drop_database, replace_database_in_url, + skip_if_not_mysql_uri, skip_if_not_postgres_uri, ) @@ -31,9 +31,9 @@ def view(): # A View class we would add to galaxy.model.view class TestView(View): name = 'testview' - __view__ = text('select id, first_name from testusers').columns( + __view__ = text('SELECT id, foo FROM testfoo').columns( column('id', Integer), - column('first_name', String) + column('foo', Integer) ) pkeys = {'id'} View._make_table(name, __view__, pkeys) @@ -45,37 +45,45 @@ class TestView(View): def test_postgres_create_view(database_name, postgres_url, view): metadata = MetaData() make_table(metadata) # table from which the view will select - create_database(postgres_url, database_name) - url = replace_database_in_url(postgres_url, database_name) - with sqlalchemy_engine(url) as engine: - with engine.connect() as conn: - metadata.create_all(conn) # create table in database - conn.execute(CreateView(view.name, view.__view__)) # create view in database - query = f"select * from information_schema.tables where table_name = '{view.name}' and table_type = 'VIEW'" - result = conn.execute(query) - assert result.rowcount == 1 # assert that view exists in database - + query = f"SELECT 1 FROM information_schema.views WHERE table_name = '{view.name}'" + create_database(postgres_url, database_name) + run_view_test(url, metadata, view, query) drop_database(postgres_url, database_name) def test_sqlite_create_view(sqlite_memory_url, view): metadata = MetaData() make_table(metadata) # table from which the view will select + url = sqlite_memory_url + query = f"SELECT 1 FROM sqlite_master WHERE type='view' AND name='{view.name}'" + run_view_test(url, metadata, view, query) - with sqlalchemy_engine(sqlite_memory_url) as engine: - with engine.connect() as conn: - metadata.create_all(conn) # create table in database - conn.execute(CreateView(view.name, view.__view__)) # create view in database - query = f"SELECT name FROM sqlite_master WHERE type='view' AND name='{view.name}'" - result = conn.execute(query).fetchall() - assert len(result) == 1 # assert that view exists in database + +@skip_if_not_mysql_uri +def test_mysql_create_view(database_name, mysql_url, view): + metadata = MetaData() + make_table(metadata) # table from which the view will select + url = replace_database_in_url(mysql_url, database_name) + query = f"SELECT 1 FROM information_schema.views WHERE table_name = '{view.name}'" + create_database(mysql_url, database_name) + run_view_test(url, metadata, view, query) + drop_database(mysql_url, database_name) def make_table(metadata): - users = Table('testusers', metadata, + users = Table('testfoo', metadata, Column('id', Integer, primary_key=True), - Column('first_name', String), - Column('last_name', String) + Column('foo', Integer), + Column('bar', Integer) ) return users + + +def run_view_test(url, metadata, view, query): + with sqlalchemy_engine(url) as engine: + with engine.connect() as conn: + metadata.create_all(conn) # create table in database + conn.execute(CreateView(view.name, view.__view__)) # create view in database + result = conn.execute(query).fetchall() + assert len(result) == 1 # assert that view exists in database