chore: various updates

.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

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
+   ```

requirements.txt

@@ -1,2 +1,2 @@
-jsonpickle
-sqlalchemy
+jsonpickle>=3.0.0
+sqlalchemy>=2.0.0

sql.py

@@ -2,33 +2,33 @@
 
 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:
         return self._key
 
     @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,46 +83,54 @@ 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)
 
     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)

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()

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