diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml new file mode 100644 index 0000000..bb7a669 --- /dev/null +++ b/.github/workflows/python-package.yml @@ -0,0 +1,40 @@ +name: Python package + +on: + push: + branches: [ master, chore/updates ] + pull_request: + branches: [ master ] + +permissions: + contents: read + +jobs: + build: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.10", "3.11", "3.12", "3.13"] + + steps: + - uses: actions/checkout@v6.0.3 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v6.2.0 + with: + python-version: ${{ matrix.python-version }} + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install tox + + - name: Run tests with tox + run: | + tox -e py + + - name: Run style checks with tox (Python 3.12 only) + if: ${{ matrix.python-version == '3.12' }} + run: | + tox -e codestyle diff --git a/README.md b/README.md index ee419cd..98642b1 100644 --- a/README.md +++ b/README.md @@ -21,3 +21,25 @@ By using [SQLAlchemy](sqlalchemy.org), it has the support for Firebird, Microsof If you want to migrate from the local storage to SQL, you should be able to backup your data (with STORAGE commented) then restore it back with STORAGE uncommented. + +### Running Tests + +To run the automated test suite: + +1. Create and activate a Python virtual environment: + ```bash + python3 -m venv .venv + source .venv/bin/activate + ``` +2. Install the dependencies and testing requirements: + ```bash + pip install -r requirements.txt errbot pytest tox + ``` +3. Run the tests using `pytest` directly: + ```bash + pytest + ``` + Or run both tests and style checks using `tox`: + ```bash + tox + ``` diff --git a/requirements.txt b/requirements.txt index 138fc5d..13c77e7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ -jsonpickle -sqlalchemy +jsonpickle>=3.0.0 +sqlalchemy>=2.0.0 diff --git a/sql.py b/sql.py index f20e2b2..60c2b6a 100644 --- a/sql.py +++ b/sql.py @@ -2,25 +2,25 @@ import logging from contextlib import contextmanager -from jsonpickle import encode, decode from typing import Any -from sqlalchemy import ( - Table, MetaData, Column, Integer, String, - ForeignKey, create_engine, select) -from sqlalchemy.orm import mapper, sessionmaker -from sqlalchemy.orm.exc import NoResultFound + from errbot.storage.base import StorageBase, StoragePluginBase +from jsonpickle import decode, encode +from sqlalchemy import Column, MetaData, String, Table, create_engine +from sqlalchemy.orm import registry, sessionmaker +from sqlalchemy.orm.exc import NoResultFound -log = logging.getLogger('errbot.storage.sql') +log = logging.getLogger("errbot.storage.sql") -DATA_URL_ENTRY = 'data_url' +DATA_URL_ENTRY = "data_url" class KV(object): """This is a basic key/value. Pickling in JSON.""" + def __init__(self, key: str, value: Any): self._key = key - self._value = encode(value) + self._value = encode(value, keys=True) @property def key(self) -> str: @@ -28,7 +28,7 @@ def key(self) -> str: @property def value(self) -> Any: - return decode(self._value) + return decode(self._value, keys=True) class SQLStorage(StorageBase): @@ -48,17 +48,18 @@ def _session_op(self): def get(self, key: str) -> Any: try: with self._session_op() as session: - result = session.query(self.clazz).filter(self.clazz._key == key).one().value + result = ( + session.query(self.clazz).filter(self.clazz._key == key).one().value + ) except NoResultFound: raise KeyError("%s doesn't exists." % key) return result def remove(self, key: str): - try: - with self._session_op() as session: - session.query(self.clazz).filter(self.clazz._key == key).delete() - except NoResultFound: - raise KeyError("%s doesn't exists." % key) + with self._session_op() as session: + count = session.query(self.clazz).filter(self.clazz._key == key).delete() + if count == 0: + raise KeyError("%s doesn't exists." % key) def set(self, key: str, value: Any) -> None: with self._session_op() as session: @@ -82,28 +83,32 @@ def __init__(self, bot_config): config = self._storage_config if DATA_URL_ENTRY not in config: raise Exception( - 'You need to specify a connection URL for the database in your' - 'config.py. For example:\n' - 'STORAGE_CONFIG={\n' + "You need to specify a connection URL for the database in your" + "config.py. For example:\n" + "STORAGE_CONFIG={\n" '"data_url": "postgresql://' 'scott:tiger@localhost/mydatabase/",\n' - '}') + "}" + ) # Hack around the multithreading issue in memory only sqlite. # This mode is useful for testing. - if config[DATA_URL_ENTRY].startswith('sqlite://'): + if config[DATA_URL_ENTRY].startswith("sqlite://"): from sqlalchemy.pool import StaticPool + self._engine = create_engine( config[DATA_URL_ENTRY], - connect_args={'check_same_thread': False}, + connect_args={"check_same_thread": False}, poolclass=StaticPool, - echo=bot_config.BOT_LOG_LEVEL == logging.DEBUG) + echo=bot_config.BOT_LOG_LEVEL == logging.DEBUG, + ) else: self._engine = create_engine( config[DATA_URL_ENTRY], - pool_recycle=config.get('connection_recycle', 1800), - pool_pre_ping=config.get('connection_ping', True), - echo=bot_config.BOT_LOG_LEVEL == logging.DEBUG) + pool_recycle=config.get("connection_recycle", 1800), + pool_pre_ping=config.get("connection_ping", True), + echo=bot_config.BOT_LOG_LEVEL == logging.DEBUG, + ) self._metadata = MetaData() self._sessionmaker = sessionmaker() self._sessionmaker.configure(bind=self._engine) @@ -111,17 +116,21 @@ def __init__(self, bot_config): def open(self, namespace: str) -> StorageBase: # Create a table with the given namespace - table = Table(namespace, self._metadata, - Column('key', String(767), primary_key=True), - Column('value', String(32768)), - extend_existing=True) + table = Table( + namespace, + self._metadata, + Column("key", String(767), primary_key=True), + Column("value", String(32768)), + extend_existing=True, + ) class NewKV(KV): pass - mapper(NewKV, table, properties={ - '_key': table.c.key, - '_value': table.c.value}) + reg = registry() + reg.map_imperatively( + NewKV, table, properties={"_key": table.c.key, "_value": table.c.value} + ) # ensure that the table for this namespace exists self._metadata.create_all(self._engine) diff --git a/test_sql.py b/test_sql.py new file mode 100644 index 0000000..fc2a16f --- /dev/null +++ b/test_sql.py @@ -0,0 +1,120 @@ +import logging + +import pytest +from sqlalchemy.exc import ArgumentError + +import sql + + +class MockBotConfig: + def __init__(self, data_url=None): + self.STORAGE_CONFIG = {} + if data_url is not None: + self.STORAGE_CONFIG["data_url"] = data_url + self.BOT_LOG_LEVEL = logging.WARNING + + +def test_plugin_init_missing_config(): + config = MockBotConfig() + with pytest.raises(Exception, match="You need to specify a connection URL"): + sql.SQLPlugin(config) + + +def test_plugin_init_invalid_url(): + config = MockBotConfig(data_url="invalid_url://") + with pytest.raises((ArgumentError, Exception)): + plugin = sql.SQLPlugin(config) + plugin.open("test") + + +def test_storage_operations(): + config = MockBotConfig(data_url="sqlite:///:memory:") + plugin = sql.SQLPlugin(config) + storage = plugin.open("test_namespace") + + # Initial state + assert storage.len() == 0 + assert list(storage.keys()) == [] + + # Get non-existent + with pytest.raises(KeyError): + storage.get("key1") + + # Set and get basic values + storage.set("key1", "value1") + assert storage.len() == 1 + assert storage.get("key1") == "value1" + assert list(storage.keys()) == ["key1"] + + # Set and get complex objects (list, dict, int) + storage.set("key2", {"a": 1, "b": [2, 3]}) + assert storage.len() == 2 + assert storage.get("key2") == {"a": 1, "b": [2, 3]} + assert set(storage.keys()) == {"key1", "key2"} + + # Overwrite value + storage.set("key1", "new_value1") + assert storage.get("key1") == "new_value1" + + # Remove non-existent key + with pytest.raises(KeyError): + storage.remove("nonexistent") + + # Remove existing key + storage.remove("key1") + assert storage.len() == 1 + with pytest.raises(KeyError): + storage.get("key1") + + assert list(storage.keys()) == ["key2"] + + # Close storage + storage.close() + + +def test_multiple_namespaces(): + config = MockBotConfig(data_url="sqlite:///:memory:") + plugin = sql.SQLPlugin(config) + + storage_a = plugin.open("namespace_a") + storage_b = plugin.open("namespace_b") + + storage_a.set("shared_key", "value_a") + storage_b.set("shared_key", "value_b") + + assert storage_a.get("shared_key") == "value_a" + assert storage_b.get("shared_key") == "value_b" + + assert storage_a.len() == 1 + assert storage_b.len() == 1 + + storage_a.close() + storage_b.close() + + +def test_simple_store_retrieve(): + from errbot.storage import StoreMixin + + config = MockBotConfig(data_url="sqlite:///:memory:") + plugin = sql.SQLPlugin(config) + sm = StoreMixin() + sm.open_storage(plugin, "ns") + sm["toto"] = "titui" + assert sm["toto"] == "titui" + sm.close_storage() + + +def test_mutable(): + from errbot.storage import StoreMixin + + config = MockBotConfig(data_url="sqlite:///:memory:") + plugin = sql.SQLPlugin(config) + sm = StoreMixin() + sm.open_storage(plugin, "ns") + sm["toto"] = [1, 3] + + with sm.mutable("toto") as titi: + titi[1] = 5 + + assert sm["toto"] == [1, 5] + sm.close_storage() diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000..fd30f2f --- /dev/null +++ b/tox.ini @@ -0,0 +1,20 @@ +[tox] +envlist = py, codestyle +skipsdist = True + +[testenv] +deps = + pytest + jsonpickle>=3.0.0 + sqlalchemy>=2.0.0 + errbot +commands = pytest -v + +[testenv:codestyle] +deps = + ruff + isort +commands = + ruff check sql.py test_sql.py + ruff format --check sql.py test_sql.py + isort --check-only sql.py test_sql.py