From da4f13e7345653eba8aab5b8aceeaeff7367989e Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 27 Mar 2023 15:12:06 +0200 Subject: Do not convert to string ``url.URL`` objects The SQLAlchemy method ``url.make_url`` accepts strings and ``url.URL`` object types. By default, oslo.db was converting any object to string before calling ``url.make_url``, that parses this string. Since SQLAlchemy 2.0, the ``url.URL.__str__`` method is removed and the ``url.URL.__repr__`` method returns a string with the password hidden. The new utility method checks what type of object is passed and only if the object is not a string nor a ``url.URL`` object, is converted to a string. Closes-Bug: #2012928 Change-Id: I84f13f378f83e2a55078370ae2b4787f00982c23 --- oslo_db/sqlalchemy/engines.py | 2 +- oslo_db/sqlalchemy/provision.py | 9 ++++---- oslo_db/sqlalchemy/test_fixtures.py | 2 +- oslo_db/sqlalchemy/utils.py | 8 +++++++ oslo_db/tests/sqlalchemy/test_exc_filters.py | 6 +++--- oslo_db/tests/sqlalchemy/test_sqlalchemy.py | 32 ++++++++++++++-------------- oslo_db/tests/sqlalchemy/test_utils.py | 16 ++++++-------- 7 files changed, 39 insertions(+), 36 deletions(-) diff --git a/oslo_db/sqlalchemy/engines.py b/oslo_db/sqlalchemy/engines.py index 31dabf6..4634ce5 100644 --- a/oslo_db/sqlalchemy/engines.py +++ b/oslo_db/sqlalchemy/engines.py @@ -181,7 +181,7 @@ def create_engine(sql_connection, sqlite_fk=False, mysql_sql_mode=None, json_deserializer=None, connection_parameters=None): """Return a new SQLAlchemy engine.""" - url = sqlalchemy.engine.url.make_url(sql_connection) + url = utils.make_url(sql_connection) if connection_parameters: url = _extend_url_parameters(url, connection_parameters) diff --git a/oslo_db/sqlalchemy/provision.py b/oslo_db/sqlalchemy/provision.py index 21eb90a..a4830b6 100644 --- a/oslo_db/sqlalchemy/provision.py +++ b/oslo_db/sqlalchemy/provision.py @@ -24,7 +24,6 @@ import re import string import sqlalchemy -from sqlalchemy.engine import url as sa_url from sqlalchemy import schema from sqlalchemy import sql import testresources @@ -259,7 +258,7 @@ class Backend(object): @classmethod def _ensure_backend_available(cls, url): - url = sa_url.make_url(str(url)) + url = utils.make_url(url) try: eng = sqlalchemy.create_engine(url) except ImportError as i_e: @@ -362,7 +361,7 @@ class Backend(object): ] for url_str in configured_urls: - url = sa_url.make_url(url_str) + url = utils.make_url(url_str) m = re.match(r'([^+]+?)(?:\+(.+))?$', url.drivername) database_type = m.group(1) Backend.backends_by_database_type[database_type] = \ @@ -495,7 +494,7 @@ class BackendImpl(object, metaclass=abc.ABCMeta): """ - url = sa_url.make_url(str(base_url)) + url = utils.make_url(base_url) # TODO(zzzeek): remove hasattr() conditional in favor of "url.set()" # when SQLAlchemy 1.4 is the minimum version in requirements @@ -571,7 +570,7 @@ class SQLiteBackendImpl(BackendImpl): def provisioned_database_url(self, base_url, ident): if base_url.database: - return sa_url.make_url("sqlite:////tmp/%s.db" % ident) + return utils.make_url("sqlite:////tmp/%s.db" % ident) else: return base_url diff --git a/oslo_db/sqlalchemy/test_fixtures.py b/oslo_db/sqlalchemy/test_fixtures.py index 8b69d3f..f7157c0 100644 --- a/oslo_db/sqlalchemy/test_fixtures.py +++ b/oslo_db/sqlalchemy/test_fixtures.py @@ -360,7 +360,7 @@ class AdHocDbFixture(SimpleDbFixture): """ def __init__(self, url=None): if url: - self.url = provision.sa_url.make_url(str(url)) + self.url = utils.make_url(url) driver = self.url.get_backend_name() else: driver = None diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index ba0a607..b539b6d 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -1051,3 +1051,11 @@ def suspend_fk_constraints_for_col_alter( deferrable=fk['options'].get('deferrable'), initially=fk['options'].get('initially'), ) + + +def make_url(target): + """Return a ``url.URL`` object""" + if isinstance(target, (str, sa_url.URL)): + return sa_url.make_url(target) + else: + return sa_url.make_url(str(target)) diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index 7e03fce..7f64594 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -19,7 +19,6 @@ import itertools from unittest import mock import sqlalchemy as sqla -from sqlalchemy.engine import url as sqla_url from sqlalchemy import event import sqlalchemy.exc from sqlalchemy.orm import declarative_base @@ -29,6 +28,7 @@ from sqlalchemy import sql from oslo_db import exception from oslo_db.sqlalchemy import engines from oslo_db.sqlalchemy import exc_filters +from oslo_db.sqlalchemy import utils from oslo_db.tests import base as test_base from oslo_db.tests.sqlalchemy import base as db_test_base from oslo_db.tests import utils as test_utils @@ -403,7 +403,7 @@ class TestNonExistentDatabase( def setUp(self): super(TestNonExistentDatabase, self).setUp() - url = sqla_url.make_url(str(self.engine.url)) + url = utils.make_url(self.engine.url) # TODO(zzzeek): remove hasattr() conditional in favor of "url.set()" # when SQLAlchemy 1.4 is the minimum version in requirements @@ -419,7 +419,7 @@ class TestNonExistentDatabase( matched = self.assertRaises( exception.DBNonExistentDatabase, engines.create_engine, - sqla_url.make_url( + utils.make_url( 'sqlite:////non_existent_dir/non_existent_database') ) self.assertIsNone(matched.database) diff --git a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py index ec75ffc..a51d2cb 100644 --- a/oslo_db/tests/sqlalchemy/test_sqlalchemy.py +++ b/oslo_db/tests/sqlalchemy/test_sqlalchemy.py @@ -29,7 +29,6 @@ from sqlalchemy.engine import base as base_engine from sqlalchemy import exc from sqlalchemy import sql from sqlalchemy import Column, MetaData, Table -from sqlalchemy.engine import url from sqlalchemy import Integer, String from sqlalchemy.orm import declarative_base @@ -39,6 +38,7 @@ from oslo_db.sqlalchemy import enginefacade from oslo_db.sqlalchemy import engines from oslo_db.sqlalchemy import models from oslo_db.sqlalchemy import session +from oslo_db.sqlalchemy import utils from oslo_db.tests import base as test_base from oslo_db.tests.sqlalchemy import base as db_test_base @@ -696,7 +696,7 @@ class CreateEngineTest(test_base.BaseTestCase): def test_queuepool_args(self): engines._init_connection_args( - url.make_url("mysql+pymysql://u:p@host/test"), self.args, + utils.make_url("mysql+pymysql://u:p@host/test"), self.args, max_pool_size=10, max_overflow=10) self.assertEqual(10, self.args['pool_size']) self.assertEqual(10, self.args['max_overflow']) @@ -704,7 +704,7 @@ class CreateEngineTest(test_base.BaseTestCase): def test_sqlite_memory_pool_args(self): for _url in ("sqlite://", "sqlite:///:memory:"): engines._init_connection_args( - url.make_url(_url), self.args, + utils.make_url(_url), self.args, max_pool_size=10, max_overflow=10) # queuepool arguments are not peresnet @@ -721,7 +721,7 @@ class CreateEngineTest(test_base.BaseTestCase): def test_sqlite_file_pool_args(self): engines._init_connection_args( - url.make_url("sqlite:///somefile.db"), self.args, + utils.make_url("sqlite:///somefile.db"), self.args, max_pool_size=10, max_overflow=10) # queuepool arguments are not peresnet @@ -741,39 +741,39 @@ class CreateEngineTest(test_base.BaseTestCase): def test_mysql_connect_args_default(self): engines._init_connection_args( - url.make_url("mysql://u:p@host/test"), self.args) + utils.make_url("mysql://u:p@host/test"), self.args) self._test_mysql_connect_args_default(self.args['connect_args']) def test_mysql_oursql_connect_args_default(self): engines._init_connection_args( - url.make_url("mysql+oursql://u:p@host/test"), self.args) + utils.make_url("mysql+oursql://u:p@host/test"), self.args) self._test_mysql_connect_args_default(self.args['connect_args']) def test_mysql_pymysql_connect_args_default(self): engines._init_connection_args( - url.make_url("mysql+pymysql://u:p@host/test"), self.args) + utils.make_url("mysql+pymysql://u:p@host/test"), self.args) self.assertEqual({'charset': 'utf8'}, self.args['connect_args']) def test_mysql_mysqldb_connect_args_default(self): engines._init_connection_args( - url.make_url("mysql+mysqldb://u:p@host/test"), self.args) + utils.make_url("mysql+mysqldb://u:p@host/test"), self.args) self._test_mysql_connect_args_default(self.args['connect_args']) def test_postgresql_connect_args_default(self): engines._init_connection_args( - url.make_url("postgresql://u:p@host/test"), self.args) + utils.make_url("postgresql://u:p@host/test"), self.args) self.assertEqual('utf8', self.args['client_encoding']) self.assertFalse(self.args['connect_args']) def test_mysqlconnector_raise_on_warnings_default(self): engines._init_connection_args( - url.make_url("mysql+mysqlconnector://u:p@host/test"), + utils.make_url("mysql+mysqlconnector://u:p@host/test"), self.args) self.assertEqual(False, self.args['connect_args']['raise_on_warnings']) def test_mysqlconnector_raise_on_warnings_override(self): engines._init_connection_args( - url.make_url( + utils.make_url( "mysql+mysqlconnector://u:p@host/test" "?raise_on_warnings=true"), self.args @@ -806,14 +806,14 @@ class CreateEngineTest(test_base.BaseTestCase): warn_interpolate): engines._vet_url( - url.make_url("mysql://scott:tiger@some_host/some_db")) - engines._vet_url(url.make_url( + utils.make_url("mysql://scott:tiger@some_host/some_db")) + engines._vet_url(utils.make_url( "mysql+mysqldb://scott:tiger@some_host/some_db")) - engines._vet_url(url.make_url( + engines._vet_url(utils.make_url( "mysql+pymysql://scott:tiger@some_host/some_db")) - engines._vet_url(url.make_url( + engines._vet_url(utils.make_url( "postgresql+psycopg2://scott:tiger@some_host/some_db")) - engines._vet_url(url.make_url( + engines._vet_url(utils.make_url( "postgresql://scott:tiger@some_host/some_db")) self.assertEqual( diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 8fe11c5..c78ec06 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -23,7 +23,6 @@ from sqlalchemy import CheckConstraint from sqlalchemy import MetaData, Table, Column from sqlalchemy import ForeignKey, ForeignKeyConstraint from sqlalchemy.dialects.postgresql import psycopg2 -from sqlalchemy.engine import url as sa_url from sqlalchemy.exc import OperationalError from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import declarative_base @@ -1001,7 +1000,7 @@ class TestConnectionUtils(test_base.BaseTestCase): self.connect_string) mock_create.assert_called_once_with( - sa_url.make_url(self.connect_string)) + utils.make_url(self.connect_string)) self.assertEqual( "Backend 'postgresql' is unavailable: " @@ -1356,10 +1355,9 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase): ) ) - mysqldb_url = sqlalchemy.engine.url.make_url("mysql+mysqldb://") - mysqlconnector_url = sqlalchemy.engine.url.make_url( - "mysql+mysqlconnector://") - sqlite_url = sqlalchemy.engine.url.make_url("sqlite://") + mysqldb_url = utils.make_url("mysql+mysqldb://") + mysqlconnector_url = utils.make_url("mysql+mysqlconnector://") + sqlite_url = utils.make_url("sqlite://") dispatcher(mysqldb_url, 1) dispatcher(mysqlconnector_url, 2) @@ -1395,8 +1393,7 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase): ) def test_url_pymysql(self): - url = sqlalchemy.engine.url.make_url( - "mysql+pymysql://scott:tiger@localhost/test") + url = utils.make_url("mysql+pymysql://scott:tiger@localhost/test") dispatcher, callable_fn = self._single_fixture() dispatcher(url, 15) @@ -1406,8 +1403,7 @@ class TestDialectFunctionDispatcher(test_base.BaseTestCase): ) def test_url_mysql_generic(self): - url = sqlalchemy.engine.url.make_url( - "mysql://scott:tiger@localhost/test") + url = utils.make_url("mysql://scott:tiger@localhost/test") dispatcher, callable_fn = self._single_fixture() dispatcher(url, 15) -- cgit v1.2.1