From d4f5515f202a305026e39fed6fda5a03fea1d344 Mon Sep 17 00:00:00 2001 From: Lance Edgar Date: Tue, 27 Aug 2024 22:56:32 -0500 Subject: [PATCH] fix: cleanup old code for "record changes" session feature had some tests failing, and one thing led to another.. apparenty we still had support for some pretty old sqlalchemy so no need to keep that around. --- pyproject.toml | 2 +- rattail/db/changes.py | 32 ++++++--------- rattail/testing.py | 14 +++---- tests/db/test_changes.py | 89 ++++++++++++++-------------------------- tests/test_app.py | 20 ++++++--- tox.ini | 6 +-- 6 files changed, 68 insertions(+), 95 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e88fb646..f057ed15 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,7 +69,7 @@ db = [ memcached = ["pylibmc"] supervisor = ["supervisor"] docs = ["Sphinx", "sphinx-paramlinks", "sphinxcontrib-programoutput", "furo"] -tests = ["coverage", "mock", "pytest", "pytest-cov"] +tests = ["pytest-cov", "tox"] [project.scripts] diff --git a/rattail/db/changes.py b/rattail/db/changes.py index 3940c573..6f64e0b4 100644 --- a/rattail/db/changes.py +++ b/rattail/db/changes.py @@ -30,17 +30,13 @@ from packaging.version import parse as parse_version import sqlalchemy as sa from sqlalchemy.orm import object_mapper, RelationshipProperty from sqlalchemy.orm.session import Session - -try: - from sqlalchemy.event import listen -except ImportError: - listen = None # old SQLAlchemy; will have to work around that, below +from sqlalchemy.event import listen from rattail.db.model import Setting, Change, DataSyncChange try: from rattail.db.continuum import versioning_manager -except ImportError: # assume no continuum +except ImportError: # pragma: no cover versioning_manager = None @@ -49,10 +45,14 @@ log = logging.getLogger(__name__) def record_changes(session, recorder=None, config=None): """ - Record all relevant data changes which occur within a session. + Turn on the "record changes" feature. - :param session: A :class:`sqlalchemy:sqlalchemy.orm.session.Session` class, - or instance thereof. + With this enabled, all relevant data changes which occur in the + sesion will be recorded. + + :param session: A + :class:`sqlalchemy:sqlalchemy.orm.session.Session` class, or + instance thereof. """ if isinstance(recorder, ChangeRecorder): pass @@ -61,23 +61,15 @@ def record_changes(session, recorder=None, config=None): elif recorder is None: if config: app = config.get_app() - spec = config.get('rattail.db', 'changes.recorder', usedb=False) + spec = config.get('rattail.db.changes.recorder', usedb=False) if spec: recorder = app.load_object(spec)(config) if not recorder: recorder = ChangeRecorder(config) else: - raise ValueError("Invalid 'recorder' parameter: {}".format(repr(recorder))) - - if listen: - listen(session, 'before_flush', recorder) - else: - extension = ChangeRecorderExtension(recorder) - if isinstance(session, Session): - session.extensions.append(extension) - else: - session.configure(extension=extension) + raise ValueError(f"recorder not valid: {recorder}") + listen(session, 'before_flush', recorder) session.rattail_record_changes = True session.rattail_change_recorder = recorder diff --git a/rattail/testing.py b/rattail/testing.py index 568f71a7..8e7b7097 100644 --- a/rattail/testing.py +++ b/rattail/testing.py @@ -1,10 +1,10 @@ # -*- coding: utf-8; -*- -from wuttjamaican.testing import FileConfigTestCase +from wuttjamaican.testing import FileTestCase from rattail.config import RattailConfig -class DataTestCase(FileConfigTestCase): +class DataTestCase(FileTestCase): """ Base class for test suites requiring a full (typical) database. """ @@ -14,7 +14,9 @@ class DataTestCase(FileConfigTestCase): def setup_db(self): self.setup_files() - self.config = self.make_config() + self.config = self.make_config(defaults={ + 'rattail.db.default.url': 'sqlite://', + }) self.app = self.config.get_app() # init db @@ -28,7 +30,5 @@ class DataTestCase(FileConfigTestCase): def teardown_db(self): self.teardown_files() - def make_config(self): - return RattailConfig(defaults={ - 'rattail.db.default.url': 'sqlite://', - }) + def make_config(self, **kwargs): + return RattailConfig(**kwargs) diff --git a/tests/db/test_changes.py b/tests/db/test_changes.py index e365e97d..3c589b4a 100644 --- a/tests/db/test_changes.py +++ b/tests/db/test_changes.py @@ -1,86 +1,58 @@ # -*- coding: utf-8; -*- from unittest import TestCase -from unittest.mock import patch, DEFAULT, Mock, MagicMock, call +from unittest.mock import patch, DEFAULT, Mock, MagicMock from rattail.config import RattailConfig +from rattail.testing import DataTestCase + try: from sqlalchemy import orm from rattail import db + from rattail.db import changes as mod from rattail.db import changes, model - from .. import DataTestCase except ImportError: pass else: - class TestRecordChangesFunc(TestCase): - def setUp(self): - self.config = RattailConfig() - self.app = self.config.get_app() + class MockRecorder: + def __init__(self, config): + pass - def test_session_class(self): - Session = orm.sessionmaker() - if hasattr(Session, 'kw'): - self.assertRaises(KeyError, Session.kw.__getitem__, 'rattail_record_changes') - self.assertRaises(AttributeError, getattr, Session, 'rattail_record_changes') - changes.record_changes(Session) - self.assertTrue(Session.rattail_record_changes) - def test_session_instance(self): - session = db.Session() - self.assertFalse(session.rattail_record_changes) - changes.record_changes(session) - self.assertTrue(session.rattail_record_changes) - session.close() + class TestRecordChanges(DataTestCase): - def test_recorder(self): + def test_recorder_instance(self): + recorder = mod.ChangeRecorder(self.config) + self.assertFalse(self.session.rattail_record_changes) + mod.record_changes(self.session, recorder=recorder) + self.assertTrue(self.session.rattail_record_changes) - # no recorder - session = db.Session() - self.assertRaises(AttributeError, getattr, session, 'rattail_change_recorder') - session.close() + def test_recorder_factory(self): + self.assertFalse(self.session.rattail_record_changes) + mod.record_changes(self.session, recorder=mod.ChangeRecorder, config=self.config) + self.assertTrue(self.session.rattail_record_changes) - # default recorder - session = db.Session() - changes.record_changes(session) - self.assertIs(type(session.rattail_change_recorder), changes.ChangeRecorder) - session.close() + def test_configured_recorder(self): + self.config.setdefault('rattail.db.changes.recorder', 'tests.db.test_changes:MockRecorder') + self.assertFalse(self.session.rattail_record_changes) + mod.record_changes(self.session, config=self.config) + self.assertTrue(self.session.rattail_record_changes) - # specify recorder instance - recorder = changes.ChangeRecorder(self.config) - session = db.Session() - changes.record_changes(session, recorder=recorder) - self.assertIs(session.rattail_change_recorder, recorder) - session.close() + def test_default_recorder(self): + self.assertFalse(self.session.rattail_record_changes) + mod.record_changes(self.session, config=self.config) + self.assertTrue(self.session.rattail_record_changes) - # specify recorder factory - session = db.Session() - changes.record_changes(session, recorder=changes.ChangeRecorder, config=self.config) - self.assertIs(type(session.rattail_change_recorder), changes.ChangeRecorder) - session.close() - - # specify recorder spec via config - config = RattailConfig() - config.setdefault('rattail.db', 'changes.recorder', 'rattail.db.changes:ChangeRecorder') - session = db.Session() - changes.record_changes(session, config=config) - self.assertIs(type(session.rattail_change_recorder), changes.ChangeRecorder) - session.close() - - # invalid recorder - session = db.Session() - self.assertRaises(ValueError, changes.record_changes, session, recorder='bogus') - session.close() + def test_invalid_recorder(self): + self.assertFalse(self.session.rattail_record_changes) + self.assertRaises(ValueError, mod.record_changes, self.session, recorder=42) class TestChangeRecorder(DataTestCase): - def extra_setup(self): - self.config = RattailConfig() - self.app = self.config.get_app() - def test_ignore_object(self): recorder = changes.ChangeRecorder(self.config) self.assertTrue(recorder.ignore_object(model.Setting())) @@ -311,8 +283,7 @@ else: class TestFunctionalChanges(DataTestCase): def setUp(self): - super().setUp() - self.config = RattailConfig() + self.setup_db() changes.record_changes(self.session, config=self.config) def test_add(self): diff --git a/tests/test_app.py b/tests/test_app.py index 102b0b99..85369594 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -16,11 +16,20 @@ from rattail.core import Object from rattail.db import Session from rattail.autocomplete import Autocompleter from rattail.batch import BatchHandler -from rattail.bouncer import BounceHandler from rattail.importing import ImportHandler from rattail.gpc import GPC +try: + from rattail.bouncer import BounceHandler +except ImportError: + pass +else: + + class FooBarBounceHandler(BounceHandler): + pass + + class TestAppHandler(TestCase): def setUp(self): @@ -251,6 +260,11 @@ class TestAppHandler(TestCase): def test_get_bounce_handler(self): + try: + from rattail.bouncer import BounceHandler + except ImportError: + pytest.skip("test not relevant without flufl.bounce") + # unknown type raises error by default self.assertRaises(ValueError, self.app.get_bounce_handler, 'foobar') @@ -827,10 +841,6 @@ class FooBarBatchHandler(BatchHandler): pass -class FooBarBounceHandler(BounceHandler): - pass - - class FromFooToBar(ImportHandler): host_key = 'rattail' local_key = 'rattail' diff --git a/tox.ini b/tox.ini index 65eede6d..d0490f76 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,13 @@ [tox] -envlist = py38, py39, py310, py311, nodb +envlist = py38, py39, py310, py311, nox [testenv] extras = bouncer,db,tests commands = pytest {posargs} -[testenv:nodb] -extras = bouncer,tests +[testenv:nox] +extras = tests [testenv:coverage] basepython = python3