use mysql backend in config.dev

First off: This commit changes the default development database
backend to mysql. sqlite, however, is still completely supported
with the caveat that a user must now modify config.dev to use
the sqlite backend.

While looking into this, it was discovered that our SQLAlchemy
backend for mysql (mysql-connector) completely broke model
attributes when we switched to utf8mb4_bin (binary) -- it does
not correct the correct conversion to and from binary utf8mb4.

The new, replacement dependency mysqlclient does. mysqlclient
is also recommended in SQLAlchemy documentation as the "best"
one available.

The mysqlclient backend uses a different exception flow then
sqlite, and so tests expecting IntegrityError has to be modified
to expect OperationalError from sqlalchemy.exc.

So, for each model that we define, check keys that can't be
NULL and raise sqlalchemy.exc.IntegrityError if we have to.
This way we keep our exceptions uniform.

Signed-off-by: Kevin Morris <kevr@0cost.org>

test/Makefile

@@ -8,7 +8,7 @@ MAKEFLAGS = -j1
 check: sh pytest
 
 pytest:
-	cd .. && AUR_CONFIG=conf/config coverage run --append /usr/bin/pytest test
+	cd .. && coverage run --append /usr/bin/pytest test
 
 ifdef PROVE
 sh:

test/test_accounts_routes.py

@@ -802,18 +802,40 @@ def test_post_account_edit_ssh_pub_key():
     assert response.status_code == int(HTTPStatus.OK)
 
     # Now let's update what's already there to gain coverage over that path.
-    pk = str()
-    with tempfile.TemporaryDirectory() as tmpdir:
-        with open("/dev/null", "w") as null:
-            proc = Popen(["ssh-keygen", "-f", f"{tmpdir}/test.ssh", "-N", ""],
-                         stdout=null, stderr=null)
-            proc.wait()
-        assert proc.returncode == 0
+    post_data["PK"] = make_ssh_pubkey()
 
-        # Read in the public key, then delete the temp dir we made.
-        pk = open(f"{tmpdir}/test.ssh.pub").read().rstrip()
+    with client as request:
+        response = request.post("/account/test/edit", cookies={
+            "AURSID": sid
+        }, data=post_data, allow_redirects=False)
 
-    post_data["PK"] = pk
+    assert response.status_code == int(HTTPStatus.OK)
+
+
+def test_post_account_edit_missing_ssh_pubkey():
+    request = Request()
+    sid = user.login(request, "testPassword")
+
+    post_data = {
+        "U": user.Username,
+        "E": user.Email,
+        "PK": make_ssh_pubkey(),
+        "passwd": "testPassword"
+    }
+
+    with client as request:
+        response = request.post("/account/test/edit", cookies={
+            "AURSID": sid
+        }, data=post_data, allow_redirects=False)
+
+    assert response.status_code == int(HTTPStatus.OK)
+
+    post_data = {
+        "U": user.Username,
+        "E": user.Email,
+        "PK": str(),  # Pass an empty string now to walk the delete path.
+        "passwd": "testPassword"
+    }
 
     with client as request:
         response = request.post("/account/test/edit", cookies={

test/test_api_rate_limit.py

@@ -34,5 +34,5 @@ def test_api_rate_key_null_requests_raises_exception():
 def test_api_rate_key_null_window_start_raises_exception():
     from aurweb.db import session
     with pytest.raises(IntegrityError):
-        create(ApiRateLimit, IP="127.0.0.1", WindowStart=1)
+        create(ApiRateLimit, IP="127.0.0.1", Requests=1)
     session.rollback()

test/test_auth.py

@@ -4,6 +4,8 @@ import pytest
 
 from starlette.authentication import AuthenticationError
 
+import aurweb.config
+
 from aurweb.auth import BasicAuthBackend, has_credential
 from aurweb.db import create, query
 from aurweb.models.account_type import AccountType
@@ -53,13 +55,12 @@ async def test_auth_backend_invalid_sid():
 async def test_auth_backend_invalid_user_id():
     # Create a new session with a fake user id.
     now_ts = datetime.utcnow().timestamp()
-    create(Session, UsersID=666, SessionID="realSession",
-           LastUpdateTS=now_ts + 5)
+    db_backend = aurweb.config.get("database", "backend")
+    with pytest.raises(IntegrityError):
+        create(Session, UsersID=666, SessionID="realSession",
+               LastUpdateTS=now_ts + 5)
 
-    # Here, we specify a real SID; but it's user is not there.
-    request.cookies["AURSID"] = "realSession"
-    with pytest.raises(AuthenticationError, match="Invalid User ID: 666"):
-        await backend.authenticate(request)
+    session.rollback()
 
 
 @pytest.mark.asyncio

test/test_ban.py

@@ -33,8 +33,7 @@ def test_ban():
 def test_invalid_ban():
     from aurweb.db import session
 
-    with pytest.raises(sa_exc.IntegrityError,
-                       match="NOT NULL constraint failed: Bans.IPAddress"):
+    with pytest.raises(sa_exc.IntegrityError):
         bad_ban = Ban(BanTS=datetime.utcnow())
         session.add(bad_ban)
 

test/test_db.py

@@ -5,16 +5,22 @@ import tempfile
 
 from unittest import mock
 
-import mysql.connector
 import pytest
 
 import aurweb.config
+import aurweb.initdb
 
 from aurweb import db
 from aurweb.models.account_type import AccountType
 from aurweb.testing import setup_test_db
 
 
+class Args:
+    """ Stub arguments used for running aurweb.initdb. """
+    use_alembic = True
+    verbose = True
+
+
 class DBCursor:
     """ A fake database cursor object used in tests. """
     items = []
@@ -38,27 +44,73 @@ class DBConnection:
         pass
 
 
+def make_temp_config(config_file, *replacements):
+    """ Generate a temporary config file with a set of replacements.
+
+    :param *replacements: A variable number of tuple regex replacement pairs
+    :return: A tuple containing (temp directory, temp config file)
+    """
+    tmpdir = tempfile.TemporaryDirectory()
+    tmp = os.path.join(tmpdir.name, "config.tmp")
+    with open(config_file) as f:
+        config = f.read()
+        for repl in list(replacements):
+            config = re.sub(repl[0], repl[1], config)
+        with open(tmp, "w") as o:
+            o.write(config)
+        aurwebdir = aurweb.config.get("options", "aurwebdir")
+        defaults = os.path.join(aurwebdir, "conf/config.defaults")
+        with open(defaults) as i:
+            with open(f"{tmp}.defaults", "w") as o:
+                o.write(i.read())
+    return tmpdir, tmp
+
+
+def make_temp_sqlite_config(config_file):
+    return make_temp_config(config_file,
+                            (r"backend = .*", "backend = sqlite"),
+                            (r"name = .*", "name = /tmp/aurweb.sqlite3"))
+
+
+def make_temp_mysql_config(config_file):
+    return make_temp_config(config_file,
+                            (r"backend = .*", "backend = mysql"),
+                            (r"name = .*", "name = aurweb"))
+
+
 @pytest.fixture(autouse=True)
 def setup_db():
-    setup_test_db("Bans")
+    if os.path.exists("/tmp/aurweb.sqlite3"):
+        os.remove("/tmp/aurweb.sqlite3")
+
+    # In various places in this test, we reinitialize the engine.
+    # Make sure we kill the previous engine before initializing
+    # it via setup_test_db().
+    aurweb.db.kill_engine()
+    setup_test_db()
 
 
 def test_sqlalchemy_sqlite_url():
-    with mock.patch.dict(os.environ, {"AUR_CONFIG": "conf/config.dev"}):
-        aurweb.config.rehash()
-        assert db.get_sqlalchemy_url()
+    tmpctx, tmp = make_temp_sqlite_config("conf/config")
+    with tmpctx:
+        with mock.patch.dict(os.environ, {"AUR_CONFIG": tmp}):
+            aurweb.config.rehash()
+            assert db.get_sqlalchemy_url()
     aurweb.config.rehash()
 
 
 def test_sqlalchemy_mysql_url():
-    with mock.patch.dict(os.environ, {"AUR_CONFIG": "conf/config.defaults"}):
-        aurweb.config.rehash()
-        assert db.get_sqlalchemy_url()
+    tmpctx, tmp = make_temp_mysql_config("conf/config")
+    with tmpctx:
+        with mock.patch.dict(os.environ, {"AUR_CONFIG": tmp}):
+            aurweb.config.rehash()
+            assert db.get_sqlalchemy_url()
     aurweb.config.rehash()
 
 
 def test_sqlalchemy_mysql_port_url():
-    tmpctx, tmp = make_temp_config("conf/config.defaults", ";port = 3306", "port = 3306")
+    tmpctx, tmp = make_temp_config("conf/config",
+                                   (r";port = 3306", "port = 3306"))
 
     with tmpctx:
         with mock.patch.dict(os.environ, {"AUR_CONFIG": tmp}):
@@ -67,18 +119,9 @@ def test_sqlalchemy_mysql_port_url():
         aurweb.config.rehash()
 
 
-def make_temp_config(config_file, src_str, replace_with):
-    tmpdir = tempfile.TemporaryDirectory()
-    tmp = os.path.join(tmpdir.name, "config.tmp")
-    with open(config_file) as f:
-        config = re.sub(src_str, f'{replace_with}', f.read())
-        with open(tmp, "w") as o:
-            o.write(config)
-    return tmpdir, tmp
-
-
 def test_sqlalchemy_unknown_backend():
-    tmpctx, tmp = make_temp_config("conf/config", "backend = sqlite", "backend = blah")
+    tmpctx, tmp = make_temp_config("conf/config",
+                                   (r"backend = mysql", "backend = blah"))
 
     with tmpctx:
         with mock.patch.dict(os.environ, {"AUR_CONFIG": tmp}):
@@ -89,22 +132,31 @@ def test_sqlalchemy_unknown_backend():
 
 
 def test_db_connects_without_fail():
+    """ This only tests the actual config supplied to pytest. """
     db.connect()
     assert db.engine is not None
 
 
-def test_connection_class_without_fail():
-    conn = db.Connection()
+def test_connection_class_sqlite_without_fail():
+    tmpctx, tmp = make_temp_sqlite_config("conf/config")
+    with tmpctx:
+        with mock.patch.dict(os.environ, {"AUR_CONFIG": tmp}):
+            aurweb.config.rehash()
 
-    cur = conn.execute(
-        "SELECT AccountType FROM AccountTypes WHERE ID = ?", (1,))
-    account_type = cur.fetchone()[0]
+            aurweb.db.kill_engine()
+            aurweb.initdb.run(Args())
 
-    assert account_type == "User"
+            conn = db.Connection()
+            cur = conn.execute(
+                "SELECT AccountType FROM AccountTypes WHERE ID = ?", (1,))
+            account_type = cur.fetchone()[0]
+            assert account_type == "User"
+        aurweb.config.rehash()
 
 
 def test_connection_class_unsupported_backend():
-    tmpctx, tmp = make_temp_config("conf/config", "backend = sqlite", "backend = blah")
+    tmpctx, tmp = make_temp_config("conf/config",
+                                   (r"backend = mysql", "backend = blah"))
 
     with tmpctx:
         with mock.patch.dict(os.environ, {"AUR_CONFIG": tmp}):
@@ -114,10 +166,9 @@ def test_connection_class_unsupported_backend():
         aurweb.config.rehash()
 
 
-@mock.patch("mysql.connector.connect", mock.MagicMock(return_value=True))
-@mock.patch.object(mysql.connector, "paramstyle", "qmark")
+@mock.patch("MySQLdb.connect", mock.MagicMock(return_value=True))
 def test_connection_mysql():
-    tmpctx, tmp = make_temp_config("conf/config", "backend = sqlite", "backend = mysql")
+    tmpctx, tmp = make_temp_mysql_config("conf/config")
     with tmpctx:
         with mock.patch.dict(os.environ, {
             "AUR_CONFIG": tmp,
@@ -137,44 +188,78 @@ def test_connection_sqlite():
 @mock.patch("sqlite3.connect", mock.MagicMock(return_value=DBConnection()))
 @mock.patch.object(sqlite3, "paramstyle", "format")
 def test_connection_execute_paramstyle_format():
-    conn = db.Connection()
+    tmpctx, tmp = make_temp_sqlite_config("conf/config")
 
-    # First, test ? to %s format replacement.
-    account_types = conn\
-        .execute("SELECT * FROM AccountTypes WHERE AccountType = ?", ["User"])\
-        .fetchall()
-    assert account_types == \
-        ["SELECT * FROM AccountTypes WHERE AccountType = %s", ["User"]]
+    with tmpctx:
+        with mock.patch.dict(os.environ, {
+            "AUR_CONFIG": tmp,
+            "AUR_CONFIG_DEFAULTS": "conf/config.defaults"
+        }):
+            aurweb.config.rehash()
 
-    # Test other format replacement.
-    account_types = conn\
-        .execute("SELECT * FROM AccountTypes WHERE AccountType = %", ["User"])\
-        .fetchall()
-    assert account_types == \
-        ["SELECT * FROM AccountTypes WHERE AccountType = %%", ["User"]]
+            aurweb.db.kill_engine()
+            aurweb.initdb.run(Args())
+
+            conn = db.Connection()
+
+            # First, test ? to %s format replacement.
+            account_types = conn\
+                .execute("SELECT * FROM AccountTypes WHERE AccountType = ?",
+                         ["User"]).fetchall()
+            assert account_types == \
+                ["SELECT * FROM AccountTypes WHERE AccountType = %s", ["User"]]
+
+            # Test other format replacement.
+            account_types = conn\
+                .execute("SELECT * FROM AccountTypes WHERE AccountType = %",
+                         ["User"]).fetchall()
+            assert account_types == \
+                ["SELECT * FROM AccountTypes WHERE AccountType = %%", ["User"]]
+        aurweb.config.rehash()
 
 
 @mock.patch("sqlite3.connect", mock.MagicMock(return_value=DBConnection()))
 @mock.patch.object(sqlite3, "paramstyle", "qmark")
 def test_connection_execute_paramstyle_qmark():
-    conn = db.Connection()
-    # We don't modify anything when using qmark, so test equality.
-    account_types = conn\
-        .execute("SELECT * FROM AccountTypes WHERE AccountType = ?", ["User"])\
-        .fetchall()
-    assert account_types == \
-        ["SELECT * FROM AccountTypes WHERE AccountType = ?", ["User"]]
+    tmpctx, tmp = make_temp_sqlite_config("conf/config")
+
+    with tmpctx:
+        with mock.patch.dict(os.environ, {
+            "AUR_CONFIG": tmp,
+            "AUR_CONFIG_DEFAULTS": "conf/config.defaults"
+        }):
+            aurweb.config.rehash()
+
+            aurweb.db.kill_engine()
+            aurweb.initdb.run(Args())
+
+            conn = db.Connection()
+            # We don't modify anything when using qmark, so test equality.
+            account_types = conn\
+                .execute("SELECT * FROM AccountTypes WHERE AccountType = ?",
+                         ["User"]).fetchall()
+            assert account_types == \
+                ["SELECT * FROM AccountTypes WHERE AccountType = ?", ["User"]]
+        aurweb.config.rehash()
 
 
 @mock.patch("sqlite3.connect", mock.MagicMock(return_value=DBConnection()))
 @mock.patch.object(sqlite3, "paramstyle", "unsupported")
 def test_connection_execute_paramstyle_unsupported():
-    conn = db.Connection()
-    with pytest.raises(ValueError, match="unsupported paramstyle"):
-        conn.execute(
-            "SELECT * FROM AccountTypes WHERE AccountType = ?",
-            ["User"]
-        ).fetchall()
+    tmpctx, tmp = make_temp_sqlite_config("conf/config")
+    with tmpctx:
+        with mock.patch.dict(os.environ, {
+            "AUR_CONFIG": tmp,
+            "AUR_CONFIG_DEFAULTS": "conf/config.defaults"
+        }):
+            aurweb.config.rehash()
+            conn = db.Connection()
+            with pytest.raises(ValueError, match="unsupported paramstyle"):
+                conn.execute(
+                    "SELECT * FROM AccountTypes WHERE AccountType = ?",
+                    ["User"]
+                ).fetchall()
+        aurweb.config.rehash()
 
 
 def test_create_delete():
@@ -186,13 +271,12 @@ def test_create_delete():
     assert record is None
 
 
-@mock.patch("mysql.connector.paramstyle", "qmark")
 def test_connection_executor_mysql_paramstyle():
     executor = db.ConnectionExecutor(None, backend="mysql")
-    assert executor.paramstyle() == "qmark"
+    assert executor.paramstyle() == "format"
 
 
 @mock.patch("sqlite3.paramstyle", "pyformat")
 def test_connection_executor_sqlite_paramstyle():
     executor = db.ConnectionExecutor(None, backend="sqlite")
-    assert executor.paramstyle() == "pyformat"
+    assert executor.paramstyle() == sqlite3.paramstyle

test/test_initdb.py

@@ -1,27 +1,19 @@
-import pytest
-
 import aurweb.config
 import aurweb.db
 import aurweb.initdb
 
 from aurweb.models.account_type import AccountType
-from aurweb.schema import metadata
-from aurweb.testing import setup_test_db
 
 
-@pytest.fixture(autouse=True)
-def setup():
-    setup_test_db()
-
-    tables = metadata.tables.keys()
-    for table in tables:
-        aurweb.db.session.execute(f"DROP TABLE IF EXISTS {table}")
+class Args:
+    use_alembic = True
+    verbose = True
 
 
 def test_run():
-    class Args:
-        use_alembic = True
-        verbose = False
+    from aurweb.schema import metadata
+    aurweb.db.kill_engine()
+    metadata.drop_all(aurweb.db.get_engine())
     aurweb.initdb.run(Args())
     record = aurweb.db.query(AccountType,
                              AccountType.AccountType == "User").first()

test/test_package_relation.py

@@ -1,6 +1,6 @@
 import pytest
 
-from sqlalchemy.exc import IntegrityError
+from sqlalchemy.exc import IntegrityError, OperationalError
 
 from aurweb.db import create, query
 from aurweb.models.account_type import AccountType
@@ -36,7 +36,7 @@ def setup():
                      URL="https://test.package")
 
 
-def test_package_dependencies():
+def test_package_relation():
     conflicts = query(RelationType, RelationType.Name == "conflicts").first()
     pkgrel = create(PackageRelation, Package=package,
                     RelationType=conflicts,
@@ -68,10 +68,12 @@ def test_package_dependencies():
     assert pkgrel in package.package_relations
 
 
-def test_package_dependencies_null_package_raises_exception():
+def test_package_relation_null_package_raises_exception():
     from aurweb.db import session
 
     conflicts = query(RelationType, RelationType.Name == "conflicts").first()
+    assert conflicts is not None
+
     with pytest.raises(IntegrityError):
         create(PackageRelation,
                RelationType=conflicts,
@@ -79,7 +81,7 @@ def test_package_dependencies_null_package_raises_exception():
     session.rollback()
 
 
-def test_package_dependencies_null_dependency_type_raises_exception():
+def test_package_relation_null_relation_type_raises_exception():
     from aurweb.db import session
 
     with pytest.raises(IntegrityError):
@@ -89,11 +91,13 @@ def test_package_dependencies_null_dependency_type_raises_exception():
     session.rollback()
 
 
-def test_package_dependencies_null_depname_raises_exception():
+def test_package_relation_null_relname_raises_exception():
     from aurweb.db import session
 
-    depends = query(RelationType, RelationType.Name == "depends").first()
-    with pytest.raises(IntegrityError):
+    depends = query(RelationType, RelationType.Name == "conflicts").first()
+    assert depends is not None
+
+    with pytest.raises((OperationalError, IntegrityError)):
         create(PackageRelation,
                Package=package,
                RelationType=depends)

test/test_session.py

@@ -25,7 +25,7 @@ def setup():
                   ResetKey="testReset", Passwd="testPassword",
                   AccountType=account_type)
     session = create(Session, UsersID=user.ID, SessionID="testSession",
-                     LastUpdateTS=datetime.utcnow())
+                     LastUpdateTS=datetime.utcnow().timestamp())
 
 
 def test_session():