diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml new file mode 100644 index 0000000..ba33f78 --- /dev/null +++ b/.github/workflows/checks.yml @@ -0,0 +1,44 @@ +name: run checks +on: + push: + branches-ignore: + - master +jobs: + check_min: + name: Lint, format and test on lowest Python + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: '3.12' + - uses: yezz123/setup-uv@v4 + with: + uv-venv: ".venv" + - run: uv pip install .[dev,all] + # ruff + ty + - run: edwh lint --output ci + # what `edwh fmt` would change: import order and formatting + - run: ruff check --select I . && ruff format --check . + # pytest with coverage over src/; [tool.coverage.report] fail_under enforces 100% + - run: edwh test.run + + check_max: + name: Lint, format and test on highest Python + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: '3.15' + allow-prereleases: true + - uses: yezz123/setup-uv@v4 + with: + uv-venv: ".venv" + - run: uv pip install .[dev,all] + # ruff + ty + - run: edwh lint --output ci + # what `edwh fmt` would change: import order and formatting + - run: ruff check --select I . && ruff format --check . + # pytest with coverage over src/; [tool.coverage.report] fail_under enforces 100% + - run: edwh test.run diff --git a/.github/workflows/su6.yml b/.github/workflows/su6.yml deleted file mode 100644 index 9bb5206..0000000 --- a/.github/workflows/su6.yml +++ /dev/null @@ -1,34 +0,0 @@ -name: run checks -on: - push: - branches-ignore: - - master -jobs: - check_min: - name: Check with `su6 all` on lowest Python - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-python@v4 - with: - python-version: '3.12' - - uses: yezz123/setup-uv@v4 - with: - uv-venv: ".venv" - - run: uv pip install su6[all] .[dev,all] - - run: su6 all --coverage 100 - - check_max: - name: Check with `su6 all` on highest Python - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-python@v4 - with: - python-version: '3.14' - allow-prereleases: true - - uses: yezz123/setup-uv@v4 - with: - uv-venv: ".venv" - - run: uv pip install su6[all] .[dev,all] - - run: su6 all --coverage 100 diff --git a/README.md b/README.md index c68cecb..2311f46 100644 --- a/README.md +++ b/README.md @@ -2,10 +2,9 @@ [![PyPI - Version](https://img.shields.io/pypi/v/TypeDAL.svg)](https://pypi.org/project/TypeDAL) [![PyPI - Python Version](https://img.shields.io/pypi/pyversions/TypeDAL.svg)](https://pypi.org/project/TypeDAL) -[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) +[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) -[![su6 checks](https://github.com/trialandsuccess/TypeDAL/actions/workflows/su6.yml/badge.svg?branch=development)](https://github.com/trialandsuccess/TypeDAL/actions) -![coverage.svg](coverage.svg) +[![checks](https://github.com/trialandsuccess/TypeDAL/actions/workflows/checks.yml/badge.svg?branch=development)](https://github.com/trialandsuccess/TypeDAL/actions) Typing support for [PyDAL](http://web2py.com/books/default/chapter/29/6). This package aims to improve the typing support for PyDAL. By using classes instead of the define_table method, diff --git a/coverage.svg b/coverage.svg deleted file mode 100644 index 2d3d236..0000000 --- a/coverage.svg +++ /dev/null @@ -1 +0,0 @@ -coverage: 100.00%coverage100.00% \ No newline at end of file diff --git a/docs/10_advanced_apis.md b/docs/10_advanced_apis.md index f152d5a..5bc305c 100644 --- a/docs/10_advanced_apis.md +++ b/docs/10_advanced_apis.md @@ -40,6 +40,106 @@ migrate that table to `TypedTable`. ## Upsert and validation helpers +### Unique-key upsert in v6 + +`upsert(key, **values)` inserts or updates by a unique key and returns the resulting +typed instance. It is a table-class operation; query builders and row instances do +not expose it. `upsert_async` has the same contract. + +```python +user = User.upsert({"email": "a@example.com"}, name="Alice") +user = await User.upsert_async({"email": "a@example.com"}, name="Bob") +``` + +The key must be nonempty, contain known fields, exclude `id`, and have no `None` +values; its values must be plain scalars (`str`, `int`, `float`, `bool`, `bytes`, `Decimal`, `UUID`, +dates and times). Key fields cannot also occur in the values, and `id` can't be a value either +(it would re-key the row). All of these raise `UpsertKeyError` (a `ValueError`) before any SQL runs. +The upsert exceptions live in `typedal.exceptions` (all subclass `typedal.TypeDALError`), +`UpsertHooksWarning` in `typedal.warnings`, and the `UpsertKey` types in `typedal.types`. +To change a key field, use +`update_or_insert({"email": "old@example.com"}, email="new@example.com")`. +A call with only a key returns an existing row unchanged, without after-hooks, +or inserts a new row using defaults. + +#### Native and fallback paths + +Only PostgreSQL has a native path: an atomic `INSERT ... ON CONFLICT ... RETURNING`. A key without a +matching unique constraint or index raises `UpsertKeyError`. PostgreSQL checks the proposed insert's +constraints before resolving the conflict; provide required insertion values even when you expect to update +an existing row. The key-only existing-row path avoids attempting that insertion, and skips the +conflict-target validation because it executes no write. + +SQLite and MySQL always use the fallback: a lookup by key followed by an insert or update. PostgreSQL uses it too +for tables with a common filter or a multi-tenant (`request_tenant`) field, because `ON CONFLICT` would bypass those +filters, and when the values replace an upload field with `autodelete=True`. The fallback respects the filters, +does not inspect indexes, and raises `UpsertAmbiguityError` if the key matches multiple visible rows. It is not +atomic under concurrent writes, so database unique constraints remain recommended. On SQLite 3.35+ the update +branch reads the row back with `UPDATE ... RETURNING`; MySQL and older SQLite need one extra `SELECT`. +Conflicts on a different unique column raise the database's normal integrity error on every backend. + +#### Hooks + +Before-hooks never run on either path because the branch is unknown beforehand. +After-hooks run for the branch that occurred: `after_insert(row, id)` or +`after_update(affected_set, row)`. The hook row is PyDAL's operation row, and the +update set is restricted to the affected ID, ignoring common filters so the hook +can still access a row moved outside its filter. + +Register a before-hook's upsert policy explicitly (also accepted by `before_insert_once`/`before_update_once`): + +```python +User.before_insert(validate_user, upsert="error") # Require update_or_insert instead. +User.before_update(normalize_user, upsert="ignore") # Skip silently during upsert. +``` + +An unmarked before-hook you registered emits `UpsertHooksWarning` (pointing at the `upsert()` call) and is skipped. +An `"error"` registration raises `UpsertHookError` before executing SQL. Policies belong to each model registration; +they do not change ordinary inserts, updates, or `update_or_insert`. +PyDAL's own upload hooks, present on every table, don't warn: upsert stores uploaded files itself and, on the update +branch, removes the replaced file for `autodelete` fields just like a normal update. + +The built-in `SlugMixin` and `TimestampsMixin` register their hooks with `upsert="error"`: skipping them would insert +rows without a slug or leave `updated_at` stale, so `upsert()` on those tables raises `UpsertHookError`. +Use `update_or_insert` there. + +Inserts apply field defaults and computations. Upsert updates write only explicitly +supplied values: omitted `Field(update=...)` and `compute` fields stay unchanged. +Supply these values explicitly if they need to change. TypeDAL cache invalidation +runs through the normal after-hooks. + +#### Decimal values + +PyDAL writes decimal values into SQL unquoted. Since 6.0, TypeDAL converts every value for a `decimal` field to +`Decimal` first (for inserts, updates, queries and upserts alike) and raises `ValueError` for anything that isn't a +finite number, so a string from request data can't change the statement. + +### Affected-ID update hooks since 6.0 + +Updates through TypeDAL's query builders, `db(query).update(...)`, and record updates +pass an `AffectedSet` (`typedal.types.AffectedSet`, matching the updated rows by primary key) to after-update hooks instead of the original +query, so hooks still find rows whose filtered columns the update changed. Its `affected_ids` lists the primary +keys already captured by the write; for keyed tables (`primarykey=[...]`) these are the key values, or tuples for +composite keys. Calling `.where(...)` or `rows(...)` on it returns a narrowed (plain) `UpdateSet`. + +After-update hooks only run when at least one row was updated. That's unchanged from PyDAL, which only calls them +for a nonzero rowcount. + +PostgreSQL and SQLite 3.35+ obtain the IDs with `UPDATE ... RETURNING`. MySQL and older SQLite select the IDs first +(MySQL with `FOR UPDATE`) and then update exactly those rows, so a row that starts matching in between is neither +updated nor reported; the returned count is the real rowcount. That costs one extra `SELECT` per update on those +backends whenever IDs are needed: with an after-update hook (including TypeDAL's own cache invalidation) or for a +QueryBuilder `update()`, which returns the IDs. +Raw updates with no after-hooks use ordinary rowcount without collecting IDs. +Large affected-ID lists currently stay in memory; there is no threshold or temporary-table strategy. + +PyDAL's reverse-reference LazySets (`row.articles.update(...)`) and `db.smart_query(...)` construct plain Sets, +which keep PyDAL's original-query after-hook semantics: their hooks receive that plain `Set` without +`affected_ids`. Because the original query may no longer match the updated rows, TypeDAL's cache invalidation +drops every cached result that depends on the table in that case. + +### Validation and general update-or-insert + `TypedTable` exposes convenience methods for common upsert/validation flows: ```python diff --git a/docs/2_defining_tables.md b/docs/2_defining_tables.md index efa22f8..5588fa2 100644 --- a/docs/2_defining_tables.md +++ b/docs/2_defining_tables.md @@ -118,7 +118,7 @@ see [their docs](http://www.web2py.com/books/default/chapter/29/06/the-database- ```python from typedal import TypedTable -from typedal.types import OpRow, Reference, Set +from typedal.types import AffectedSet, OpRow, Reference, Set class MyTable(TypedTable): ... @@ -144,8 +144,12 @@ def my_before_update(query: Set, changes: OpRow): # return True to cancel -def my_after_update(query: Set, changes: OpRow): - """`changes` that were applied to the row selection Set""" +def my_after_update(rows: AffectedSet, changes: OpRow): + """ + `changes` that were applied. `rows` selects exactly the updated rows (by id), even if the update + changed the columns the original query filtered on. `rows.affected_ids` lists their ids. + Only called when at least one row was updated. + """ MyTable.before_update(my_before_update) @@ -168,5 +172,10 @@ row.delete_record() # to trigger MyTable.where(...).delete() # to trigger ``` +Since 6.0, `after_update` receives an `AffectedSet` instead of a Set with the original update query +(see [10. Advanced APIs](./10_advanced_apis.md#affected-id-update-hooks-since-60)). +Before-hooks also accept an `upsert=` policy, which decides what `upsert()` does with them, +since upsert can't run before-hooks (see [Unique-key upsert](./10_advanced_apis.md#unique-key-upsert-in-v6)). + Now that we have some tables, it's time to actually query them! Let's go to [3. Building Queries](./3_building_queries.md) to learn how. diff --git a/docs/3_building_queries.md b/docs/3_building_queries.md index 5f5fabd..ed043b9 100644 --- a/docs/3_building_queries.md +++ b/docs/3_building_queries.md @@ -169,6 +169,61 @@ Person.join("articles", method="inner") # will only yield persons that have rel For more details about relationships and joins, see [4. Relationships](./4_relationships.md). +### cross_join + +Since 6.0, a query that mentions a table without relating it to the rest of the query raises +`ImplicitCrossJoinError` instead of silently producing a `CROSS JOIN` (every row combined with every other row). +This catches a common mistake: filtering on a table that isn't joined. + +```python +from typedal.exceptions import ImplicitCrossJoinError + +Person.where(Article.title == "Hello") # raises ImplicitCrossJoinError when the query is built +Person.select(Person.ALL, Article.ALL) # same: nothing relates article to person + +# fine, the comparison links the two tables: +Person.where(Person.id == Article.author) +# also fine: any comparison mentioning both tables counts as a link +Person.where((Person.age + Article.word_count) > 1000) +``` + +If you really want a cross join, ask for it explicitly with `cross_join()`. +Tables reached through a cross-joined table are accepted too: + +```python +Person.cross_join(Color) # every person combined with every color +Person.cross_join(Article).where(Article.id == Comment.article) # comment is linked via article +``` + +A relationship join puts the related table in the SQL under an alias. Filtering or ordering on that table +(`where()`, `orderby()`, `groupby()`, `having()`) is pointed at the alias when the query runs, so it doesn't matter +whether `where()` comes before or after `join()`, or whether the builder is returned and extended somewhere else: + +```python +Person.join("articles", method="inner").where(Article.published == True) +Person.where(Article.published == True).join("articles", method="inner") # the same query +``` + +Like `condition_and`, this filters the joined rows too: each person comes back with only their published articles. +On a left join it also drops the persons without a matching article, and `where(Article.id == None)` finds the persons +without any. Pagination and `count()` take the filter into account. + +When the same table is joined more than once, `Article` alone is ambiguous and raises `AliasedTableMismatchError` +(a subclass of `ImplicitCrossJoinError`). Pick the join with extra lambda arguments, named after the relationship +(a nested one as `parent__child`, or just `child` when that name is unique). These are resolved when the query runs, +so they can also come before the `join()`: + +```python +builder = Article.join("writer").join("reviewer") # both relationships to Author +builder.where(Author.name == "ann") # raises: author is joined as 'writer' and as 'reviewer' +builder.where(lambda article, reviewer: reviewer.name == "ann") +``` + +`delete()` and `update()` ignore joins, so they refuse a builder with such a lambda. + +If you want a second, independent copy of a joined table, `cross_join(Article)` makes it explicit: the table name +then means that copy, not the joined one. + ### groupby & having Group query results by one or more fields, typically used with aggregate functions like `count()`, `sum()`, `avg()`, diff --git a/docs/9_memoization.md b/docs/9_memoization.md index fd51c2b..2ff952f 100644 --- a/docs/9_memoization.md +++ b/docs/9_memoization.md @@ -83,19 +83,24 @@ db.memoize(func, data, ttl=datetime(2026, 1, 7)) ## Cache Maintenance -The `typedal.caching` module provides utilities for cache management: +The `typedal.caching` module provides utilities for cache management. +Every database with caching enabled has its own cache tables, so each function takes the database to act on: ```python -from typedal.caching import clear_cache, remove_cache_for_table, clear_expired +from typedal.caching import cache_models, clear_cache, remove_cache_for_table, clear_expired # Remove all cache entries -clear_cache() +clear_cache(db) # Invalidate all cache entries related to a specific table -remove_cache_for_table(User) +remove_cache_for_table(db, User) # Clean up expired entries only -clear_expired() +clear_expired(db) + +# The cache models bound to this database, e.g. to count entries: +Cache, CacheDependency = cache_models(db) +Cache.count() ``` You can also use the CLI: diff --git a/pyproject.toml b/pyproject.toml index ddf2b4b..9a60eb3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "uv_build" name = "TypeDAL" description = 'Typing support for PyDAL' readme = "README.md" -version = "5.1.5" +version = "6.0.0b1" requires-python = ">=3.12" license-expression = "MIT" keywords = [] @@ -14,16 +14,19 @@ authors = [ { name = "Robin van der Noord", email = "contact@trialandsuccess.nl" }, ] classifiers = [ - "Development Status :: 4 - Beta", + "Development Status :: 5 - Production/Stable", "Programming Language :: Python", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", "Programming Language :: Python :: 3.14", + "Programming Language :: Python :: 3.15", "Programming Language :: Python :: Implementation :: CPython", "Programming Language :: Python :: Implementation :: PyPy", ] dependencies = [ - "pydal >= 20251012.3", # core + # core; 20260520.0 is the first release with Set._apply_update, which UpdateSet relies on. + # Note: pydal's newer 3.YYYYMMDD versions sort *below* this under PEP 440 and are excluded on purpose for now. + "pydal >= 20260520.0", "pydal-stubs >= 0.2.0", # improved typing for pydal "dill < 1", # caching "configuraptor >= 2.1.0, < 3", # config @@ -85,14 +88,18 @@ all = [ dev = [ # build: "ewok", + # lint (`edwh lint`: ruff + ty), format (`edwh fmt`) and test (`edwh test.run`): + "edwh>=1.18", # test: - "su6[all]>=1.9.0", + "pytest", + "pytest-cov", + "mypy", # used by pytest-typing # "pytest-mypy-testing", "pytest-typing", "pytest-asyncio", "pyright < 1.1.400", "contextlib-chdir", - "testcontainers", + "testcontainers[mysql]>=4.15", # testcontainers.community.mysql "pydantic < 3", "psycopg2-binary", # depends on -> @@ -166,41 +173,19 @@ fix = "patch" perf = "patch" [tool.edwh.test] -# measure coverage over src/ only, matching su6 +# measure coverage over src/ only directory = "src" -### required in every su6 pyproject: ### -[tool.su6] -directory = "src" -stop-after-first-failure = true -include = [] -exclude = [] -coverage = 100 -badge = true - -[tool.su6.default-flags] -mypy = "--disable-error-code misc" - -[tool.black] -target-version = ["py312"] -line-length = 120 -# 'extend-exclude' excludes files or directories in addition to the defaults -extend-exclude = ''' -# A regex preceded with ^/ will apply only to files and directories -# in the root of the project. -( - ^.*\.bak\/.+ # ignore every .bak directory - ^.*venv.+\/.+ # ignore every venv directory - venv.+|.+\.bak # idk why it suddenly works, let's not touch it -) -''' - [tool.coverage.run] -# measure src/ only, whatever invokes coverage: `su6 pytest` passes `--cov src` itself, but a +# measure src/ only, whatever invokes coverage: `edwh test.run` passes `--cov=src` itself, but a # bare `pytest --cov` would otherwise measure the whole tree (tests, tasks.py, example_*.py). source = ["src"] +# line coverage alone hides untested branches (an `if` that is never false still counts as 100%): +branch = true [tool.coverage.report] +# the CI gate: pytest-cov fails the run below this +fail_under = 100 exclude_also = [ "if TYPE_CHECKING:", "if t.TYPE_CHECKING:", @@ -256,7 +241,9 @@ select = [ # "COM", # comma's - NO: annoying # "PTH", # use pathlib - NO: annoying "RUF", # ruff rules - # "D", # docs + "D", # docstrings (google convention, see below) + "D213", # multi-line docstring summary on the second line (not part of the google convention) + "S", # security (bandit) ] unfixable = [ # Don't touch unused imports @@ -273,36 +260,14 @@ extend-ignore = [ ignore = [ "RUF012", # mutable class attributes "RUF013", # implicit optional + "S101", # assert: used for type narrowing and invariants + "S108", # hard coded /tmp/... files are fine ] [tool.ruff.lint.pydocstyle] convention = "google" -[tool.bandit] -# bandit -c pyproject.toml -r . -exclude_dirs = [".bak", "venv"] -skips = [ - "B108" # hard coded /tmp/... files are fine for me tbh -] - -[tool.isort] -profile = "black" -extend_skip_glob = ["*.bak/*"] - -[tool.pydocstyle] -convention = "google" -match-dir = '(?!venv)[^\.].*' -add_select = [ - "D213", # = Multi-line docstring summary should start at the second line - "D416", # = Google-style section name checks. - "D417", # = Missing argument descriptions in the docstring -] -add_ignore = [ - "D200", # = One-line docstring should fit on one line with quotes - "D212", # = Multi-line docstring summary should start at the first line -] - ### and if it's a project and NOT a package, add this to make it not look for anything buildable: ### # make this a meta package: not a library but simply allow me to run `pip install .[dev]` #[build-system] diff --git a/src/typedal/__init__.py b/src/typedal/__init__.py index dbc6ea6..0976c35 100644 --- a/src/typedal/__init__.py +++ b/src/typedal/__init__.py @@ -4,6 +4,7 @@ from .asynchronous import AsyncSession, BlockingAccessHandler, BlockingDatabaseAccessWarning from .core import TypeDAL +from .exceptions import TypeDALError from .fields import TypedField from .helpers import sql_expression from .query_builder import QueryBuilder @@ -27,6 +28,7 @@ "Ref", "Relationship", "TypeDAL", + "TypeDALError", "TypedField", "TypedRows", "TypedTable", diff --git a/src/typedal/asynchronous.py b/src/typedal/asynchronous.py index a83b9c6..1fb2145 100644 --- a/src/typedal/asynchronous.py +++ b/src/typedal/asynchronous.py @@ -90,9 +90,9 @@ def _close_connection(self) -> None: if adapter is None: # pragma: no cover return - # `really=False` lets pydal recycle the connection into its own pool when it has one; - # it closes for real when it doesn't (sqlite) or when that pool is full. - adapter.close(action="rollback", really=False) + # pydal recycles the connection into its own pool when that has room and closes it otherwise. + # Don't pass `really=False`: without a pool that drops the connection unclosed. + adapter.close(action="rollback") class ConnectionWorkerPool: @@ -187,7 +187,7 @@ def shutdown(self) -> None: while waiters: waiter = waiters.popleft() - if waiter.set_running_or_notify_cancel(): + if waiter.set_running_or_notify_cancel(): # pragma: no branch - a cancelled waiter needs nothing waiter.set_exception(RuntimeError("The async worker pool is shutting down.")) errors: list[Exception] = [] @@ -273,14 +273,15 @@ async def __aexit__( # a no-op when the settle got through - by then there is no worker left to hold. self._abandon_worker() - if self._token is not None: + if self._token is not None: # pragma: no branch - defensive, exit always follows enter ACTIVE_SESSIONS.reset(self._token) self._token = None async def run_sync[**P, T](self, fn: t.Callable[P, T], *args: P.args, **kwargs: P.kwargs) -> T: """ - Run ordinary *sync* TypeDAL code on this session's worker, sharing its connection and - transaction. + Run ordinary *sync* TypeDAL code on this session's worker. + + It shares the session's connection and transaction. """ worker = await self._acquire() return await worker.run(functools.partial(fn, *args, **kwargs)) diff --git a/src/typedal/caching.py b/src/typedal/caching.py index 6cbf9a3..8dbbc5d 100644 --- a/src/typedal/caching.py +++ b/src/typedal/caching.py @@ -8,10 +8,9 @@ import json import typing as t -import dill # nosec +import dill from .fields import TypedField -from .helpers import throw from .rows import TypedRows from .tables import TypedTable from .types import CacheStatus, Field, Query, Rows, Set @@ -22,10 +21,14 @@ class FunctionWithMetadata[T](t.Protocol): + """A callable with the name attributes memoize uses for its cache key.""" + __name__: str __qualname__: str - def __call__(self, *args: t.Any, **kwargs: t.Any) -> T: ... + def __call__(self, *args: t.Any, **kwargs: t.Any) -> T: + """Call the memoized function.""" + ... def get_now(tz: dt.timezone = dt.timezone.utc) -> dt.datetime: @@ -56,6 +59,45 @@ class _TypedalCacheDependency(TypedTable): idx: TypedField[int] +CACHE_TABLE = "typedal_cache" +CACHE_DEPENDENCY_TABLE = "typedal_cache_dependency" + +type CacheModels = tuple[type[_TypedalCache], type[_TypedalCacheDependency]] + + +def define_cache_models(db: "TypeDAL") -> CacheModels: + """ + Define the cache tables on `db`, using subclasses that belong to this database only. + + `db.define` binds the class it is given, so defining the module-level `_TypedalCache` itself on + every database would leave it bound to whichever database was created last (and `close()` on + any of them would unbind it for all). A fresh subclass per database keeps the bindings apart, + while the table names stay `typedal_cache` and `typedal_cache_dependency`. + """ + cache: type[_TypedalCache] = type("_TypedalCache", (_TypedalCache,), {"__module__": __name__}) + dependency: type[_TypedalCacheDependency] = type( + "_TypedalCacheDependency", + (_TypedalCacheDependency,), + {"__module__": __name__, "entry": TypedField(cache)}, + ) + return db.try_define(cache), db.try_define(dependency) + + +def cache_models(db: "TypeDAL") -> CacheModels: + """ + Get the cache models bound to `db`. + + Raises: + RuntimeError: when `db` was created with caching disabled. + """ + cache = db._class_map.get(CACHE_TABLE) + dependency = db._class_map.get(CACHE_DEPENDENCY_TABLE) + if cache is None or dependency is None: + raise RuntimeError("TypeDAL caching is not enabled for this database (enable_typedal_caching=False)!") + + return t.cast(type[_TypedalCache], cache), t.cast(type[_TypedalCacheDependency], dependency) + + def prepare(field: t.Any) -> str: """ Prepare data to be used in a cache key. @@ -100,8 +142,8 @@ def create_and_hash_cache_key(*fields: t.Any) -> tuple[str, str]: return key, hash_cache_key(key) -DependencyTuple = tuple[str, int] # table + id -DependencyTupleSet = set[DependencyTuple] +type DependencyTuple = tuple[str, int] # table + id +type DependencyTupleSet = set[DependencyTuple] def _get_table_name(field: Field) -> str: @@ -153,61 +195,78 @@ def _determine_dependencies(instance: TypedRows[t.Any], rows: Rows, depends_on: return _get_dependency_ids(rows, dependency_keys) -def remove_cache(idx: int | t.Iterable[int], table: str) -> None: +def remove_cache(db: "TypeDAL", idx: int | t.Iterable[int], table: str) -> None: """ Remove any cache entries that are dependant on one or multiple indices of a table. """ if not isinstance(idx, t.Iterable): idx = [idx] - related = ( - _TypedalCacheDependency.where(table=table).where(lambda row: row.idx.belongs(idx)).select("entry").to_sql() - ) + cache, dependency = cache_models(db) + related = dependency.where(table=table).where(lambda row: row.idx.belongs(idx)).select("entry").to_sql() - _TypedalCache.where(_TypedalCache.id.belongs(related)).delete() + cache.where(cache.id.belongs(related)).delete() -def remove_cache_for_table(table: str) -> None: +def remove_cache_for_table(db: "TypeDAL", table: str | type[TypedTable]) -> None: """ Remove all cache entries that depend on a table. Used for inserts where we don't know which cached queries the new row would match. """ - related = _TypedalCacheDependency.where(table=table).select("entry").to_sql() - _TypedalCache.where(_TypedalCache.id.belongs(related)).delete() + if not isinstance(table, str): + table = str(table._table) + + cache, dependency = cache_models(db) + related = dependency.where(table=table).select("entry").to_sql() + cache.where(cache.id.belongs(related)).delete() -def clear_cache() -> None: +def clear_cache(db: "TypeDAL") -> None: """ Remove everything from the cache. Immediately commits """ - db: TypeDAL = _TypedalCache._db or throw( - RuntimeError("@define or db.define is not called on typedal caching classes yet!") - ) - - _TypedalCache.truncate("RESTART IDENTITY CASCADE") + cache, _ = cache_models(db) + cache.truncate("RESTART IDENTITY CASCADE") db.commit() -def clear_expired() -> int: +def clear_expired(db: "TypeDAL") -> int: """ Remove all expired items from the cache. By default, expired items are only removed when trying to access them. """ + cache, _ = cache_models(db) now = get_now() - return len(_TypedalCache.where(_TypedalCache.expires_at != None).where(_TypedalCache.expires_at < now).delete()) + return len(cache.where(cache.expires_at != None).where(cache.expires_at < now).delete()) -def _remove_cache(s: Set, tablename: str) -> None: +def _remove_cache(db: "TypeDAL", s: Set, tablename: str) -> None: """ - Used as the table._before_update and table._after_update for every TypeDAL table (on by default). + Used as the table._before_delete for every TypeDAL table (on by default). """ indeces = s.select("id").column("id") - remove_cache(indeces, tablename) + remove_cache(db, indeces, tablename) + + +def _remove_cache_after_update(db: "TypeDAL", rows: Set, tablename: str) -> None: + """ + Used as the table._after_update for every TypeDAL table (on by default). + + TypeDAL's own update paths pass an AffectedSet carrying the updated IDs, so only those rows are invalidated. + A plain PyDAL Set (from `db.smart_query` or a lazy reference set) still holds its original query, which may no + longer match the updated rows, so the whole table is invalidated instead. + """ + affected_ids = getattr(rows, "affected_ids", None) + if affected_ids is None: + remove_cache_for_table(db, tablename) + else: + # cache dependencies only exist for TypedTables, whose primary key is always the integer id + remove_cache(db, t.cast(list[int], affected_ids), tablename) def get_expire( @@ -233,6 +292,7 @@ def get_expire( def _insert_cache_entry( + db: "TypeDAL", key: str, data: t.Any, expires_at: dt.datetime | None, @@ -247,13 +307,14 @@ def _insert_cache_entry( into a half-applied one. The entry lands with the caller's own next commit instead - and is rolled back with everything else when there isn't one, which is the correct outcome for a cache. """ - entry = _TypedalCache.insert( + cache, dependency = cache_models(db) + entry = cache.insert( key=key, data=dill.dumps(data), expires_at=expires_at, ) - _TypedalCacheDependency.bulk_insert([{"entry": entry, "table": table, "idx": idx} for table, idx in deps]) + dependency.bulk_insert([{"entry": entry, "table": table, "idx": idx} for table, idx in deps]) def save_to_cache[T_TypedTable: TypedTable]( @@ -268,10 +329,12 @@ def save_to_cache[T_TypedTable: TypedTable]( You can call .cache(...) with dependent fields (e.g. User.id) or this function will determine them automatically. """ if (c := instance.metadata.get("cache", {})) and c.get("enabled") and (key := c.get("key")): - expires_at = get_expire(expires_at=expires_at, ttl=ttl) or c.get("expires_at") - deps = _determine_dependencies(instance, rows, c["depends_on"]) - - _insert_cache_entry(key, instance, expires_at, deps) + # An empty result has no row dependencies, so no insert could ever invalidate it; loading also treats it + # as a miss. Storing it anyway only piles up dead entries that shadow this key for later, non-empty results. + if rows: + expires_at = get_expire(expires_at=expires_at, ttl=ttl) or c.get("expires_at") + deps = _determine_dependencies(instance, rows, c["depends_on"]) + _insert_cache_entry(instance.db, key, instance, expires_at, deps) instance.metadata["cache"]["status"] = "fresh" return instance @@ -295,7 +358,7 @@ def __bool__(self) -> bool: # pragma: no cover _CACHE_SENTINEL: t.Final[CacheMiss] = CacheMiss() -def _fetch_cached_payload(key: str) -> tuple[t.Any, t.Any] | None: +def _fetch_cached_payload(key: str, db: "TypeDAL") -> tuple[t.Any, t.Any] | None: """ Retrieves and validates a cache entry from the database. @@ -304,11 +367,13 @@ def _fetch_cached_payload(key: str) -> tuple[t.Any, t.Any] | None: Args: key: The unique string identifier for the cache entry. + db: The database whose cache tables to read. Returns: A tuple of (deserialized_data, db_row) if valid; None if missing or expired. """ - row = _TypedalCache.where(key=key).first() + cache, _ = cache_models(db) + row = cache.where(key=key).first() if not row: return None @@ -322,7 +387,7 @@ def _fetch_cached_payload(key: str) -> tuple[t.Any, t.Any] | None: return None # Only one place for deserialization to happen - return dill.loads(row.data), row # nosec + return dill.loads(row.data), row # noqa: S301 - written by TypeDAL itself, never by users def _load_from_cache(key: str, db: "TypeDAL") -> t.Any | None: @@ -339,7 +404,7 @@ def _load_from_cache(key: str, db: "TypeDAL") -> t.Any | None: Returns: The re-hydrated model instance, or None if the load fails. """ - result = _fetch_cached_payload(key) + result = _fetch_cached_payload(key, db) if not result: return None @@ -356,7 +421,7 @@ def _load_from_cache(key: str, db: "TypeDAL") -> t.Any | None: return inst -def _load_memoize_from_cache(key: str) -> t.Any: +def _load_memoize_from_cache(key: str, db: "TypeDAL") -> t.Any: """ Low-level retrieval for memoized results. @@ -364,12 +429,13 @@ def _load_memoize_from_cache(key: str) -> t.Any: Args: key: Cache key. + db: The database whose cache tables to read. Returns: The deserialized data or the _CACHE_SENTINEL object if not found. """ with contextlib.suppress(Exception): - if result := _fetch_cached_payload(key): + if result := _fetch_cached_payload(key, db): return result[0] return _CACHE_SENTINEL @@ -412,14 +478,13 @@ def humanize_bytes(size: int | float) -> str: return f"{size:.2f} {suffixes[suffix_index]}" -def _expired_and_valid_query() -> tuple[str, str]: +def _expired_and_valid_query(db: "TypeDAL") -> tuple[str, str]: + cache, _ = cache_models(db) expired_items = ( - _TypedalCache.where(lambda row: (row.expires_at < get_now()) & (row.expires_at != None)) - .select(_TypedalCache.id) - .to_sql() + cache.where(lambda row: (row.expires_at < get_now()) & (row.expires_at != None)).select(cache.id).to_sql() ) - valid_items = _TypedalCache.where(~_TypedalCache.id.belongs(expired_items)).select(_TypedalCache.id).to_sql() + valid_items = cache.where(~cache.id.belongs(expired_items)).select(cache.id).to_sql() return expired_items, valid_items @@ -441,11 +506,12 @@ class Stats[T](t.TypedDict): def _row_stats(db: "TypeDAL", table: str, query: Query) -> RowStats: - count_field = _TypedalCacheDependency.entry.count() - stats: TypedRows[_TypedalCacheDependency] = db(query & (_TypedalCacheDependency.table == table)).select( - _TypedalCacheDependency.entry, + _, dependency = cache_models(db) + count_field = dependency.entry.count() + stats: TypedRows[_TypedalCacheDependency] = db(query & (dependency.table == table)).select( + dependency.entry, count_field, - groupby=_TypedalCacheDependency.entry, + groupby=dependency.entry, ) return { "Dependent Cache Entries": len(stats), @@ -456,14 +522,15 @@ def row_stats(db: "TypeDAL", table: str, row_id: str) -> Stats[RowStats]: """ Collect caching stats for a specific table row (by ID). """ - expired_items, valid_items = _expired_and_valid_query() + _, dependency = cache_models(db) + expired_items, valid_items = _expired_and_valid_query(db) - query = _TypedalCacheDependency.idx == row_id + query = dependency.idx == row_id return { "total": _row_stats(db, table, query), - "valid": _row_stats(db, table, _TypedalCacheDependency.entry.belongs(valid_items) & query), - "expired": _row_stats(db, table, _TypedalCacheDependency.entry.belongs(expired_items) & query), + "valid": _row_stats(db, table, dependency.entry.belongs(valid_items) & query), + "expired": _row_stats(db, table, dependency.entry.belongs(expired_items) & query), } @@ -477,11 +544,12 @@ def row_stats(db: "TypeDAL", table: str, row_id: str) -> Stats[RowStats]: def _table_stats(db: "TypeDAL", table: str, query: Query) -> TableStats: - count_field = _TypedalCacheDependency.entry.count() - stats: TypedRows[_TypedalCacheDependency] = db(query & (_TypedalCacheDependency.table == table)).select( - _TypedalCacheDependency.entry, + _, dependency = cache_models(db) + count_field = dependency.entry.count() + stats: TypedRows[_TypedalCacheDependency] = db(query & (dependency.table == table)).select( + dependency.entry, count_field, - groupby=_TypedalCacheDependency.entry, + groupby=dependency.entry, ) return { "Dependent Cache Entries": len(stats), @@ -493,12 +561,13 @@ def table_stats(db: "TypeDAL", table: str) -> Stats[TableStats]: """ Collect caching stats for a table. """ - expired_items, valid_items = _expired_and_valid_query() + _, dependency = cache_models(db) + expired_items, valid_items = _expired_and_valid_query(db) return { - "total": _table_stats(db, table, _TypedalCacheDependency.id > 0), - "valid": _table_stats(db, table, _TypedalCacheDependency.entry.belongs(valid_items)), - "expired": _table_stats(db, table, _TypedalCacheDependency.entry.belongs(expired_items)), + "total": _table_stats(db, table, dependency.id > 0), + "valid": _table_stats(db, table, dependency.entry.belongs(valid_items)), + "expired": _table_stats(db, table, dependency.entry.belongs(expired_items)), } @@ -513,14 +582,15 @@ def table_stats(db: "TypeDAL", table: str) -> Stats[TableStats]: def _calculate_stats(db: "TypeDAL", query: Query) -> GenericStats: - sum_len_field = _TypedalCache.data.len().sum() + cache, dependency = cache_models(db) + sum_len_field = cache.data.len().sum() size_row = db(query).select(sum_len_field).first() size = size_row[sum_len_field] if size_row else 0 return { - "entries": _TypedalCache.where(query).count(), - "dependencies": db(_TypedalCacheDependency.entry.belongs(query)).count(), + "entries": cache.where(query).count(), + "dependencies": db(dependency.entry.belongs(query)).count(), "size": humanize_bytes(size), } @@ -529,12 +599,13 @@ def calculate_stats(db: "TypeDAL") -> Stats[GenericStats]: """ Collect generic caching stats. """ - expired_items, valid_items = _expired_and_valid_query() + cache, _ = cache_models(db) + expired_items, valid_items = _expired_and_valid_query(db) return { - "total": _calculate_stats(db, _TypedalCache.id > 0), - "valid": _calculate_stats(db, _TypedalCache.id.belongs(valid_items)), - "expired": _calculate_stats(db, _TypedalCache.id.belongs(expired_items)), + "total": _calculate_stats(db, cache.id > 0), + "valid": _calculate_stats(db, cache.id.belongs(valid_items)), + "expired": _calculate_stats(db, cache.id.belongs(expired_items)), } @@ -583,7 +654,7 @@ def memoize[T: t.Any]( _, hashed_key = create_and_hash_cache_key(key, *[getattr(arg, "id", None) for arg in args], kwargs) # Try to load from cache - cached = _load_memoize_from_cache(hashed_key) + cached = _load_memoize_from_cache(hashed_key, db) if cached is not _CACHE_SENTINEL: return cached, "cached" # Cache miss - compute result @@ -621,6 +692,6 @@ def track_collect(qb: "QueryBuilder[t.Any]", _: TypedRows[t.Any], raw: Rows) -> else: expires_at = get_expire(ttl=ttl) - _insert_cache_entry(hashed_key, result, expires_at, deps) + _insert_cache_entry(db, hashed_key, result, expires_at, deps) return result, "fresh" diff --git a/src/typedal/cli.py b/src/typedal/cli.py index 199e36a..838d34c 100644 --- a/src/typedal/cli.py +++ b/src/typedal/cli.py @@ -227,7 +227,8 @@ def setup( data.pop("connection", None) # ignore any None: - old_contents["tool"]["typedal"] = {k: v for k, v in data.items() if v is not None} # type: ignore + # older tomlkit types this lookup as `Item` (not subscriptable); [tool] is always a table: + t.cast(AnyDict, old_contents["tool"])["typedal"] = {k: v for k, v in data.items() if v is not None} with toml_path.open("w") as f: tomlkit.dump(old_contents, f) @@ -562,13 +563,14 @@ def tabulate_data(data: AnyNestedDict) -> None: print(tabulate(flattened_data, headers="keys")) -type FormatOptions = t.Literal["plaintext", "json", "yaml", "toml"] +FormatOptions = t.Literal["plaintext", "json", "yaml", "toml"] def get_output_format(fmt: FormatOptions) -> t.Callable[[AnyNestedDict], None]: """ - This function takes a format option as input and \ - returns a function that can be used to output data in the specified format. + Get the output function for a format option. + + The returned function prints data in the specified format. """ match fmt: case "plaintext": @@ -662,10 +664,10 @@ def cache_clear( db = TypeDAL(config=config, migrate=False, fake_migrate=False) if purge: - caching.clear_cache() + caching.clear_cache(db) print("Emptied cache") else: - n = caching.clear_expired() + n = caching.clear_expired(db) print(f"Removed {n} expired from cache") db.commit() diff --git a/src/typedal/core.py b/src/typedal/core.py index 99379b7..325f381 100644 --- a/src/typedal/core.py +++ b/src/typedal/core.py @@ -24,6 +24,7 @@ run_async, ) from .config import LazyPolicy, TypeDALConfig, default_async_workers, load_config +from .extensions import install_extensions from .helpers import ( SYSTEM_SUPPORTS_TEMPLATES, default_representer, @@ -34,7 +35,7 @@ from .serializers.typescript import TypedDictRegistry # noinspection PyUnusedImports -from .types import CacheStatus, Expression, Field, Template +from .types import CacheStatus, Expression, Field, Template, UpdateSet try: # python 3.14+ @@ -57,7 +58,7 @@ def _expression_subclasses() -> t.Iterator[type[Expression]]: stack: list[type[Expression]] = [Expression] while stack: for subclass in stack.pop().__subclasses__(): - if subclass not in seen: + if subclass not in seen: # pragma: no branch - only multiple inheritance revisits a class seen.add(subclass) stack.append(subclass) @@ -334,9 +335,10 @@ def __init__( table_hash, ) + install_extensions(self._adapter) + if config.caching: - self.try_define(_TypedalCache) - self.try_define(_TypedalCacheDependency) + define_cache_models(self) def session(self) -> AsyncSession: """ @@ -399,10 +401,12 @@ def close(self) -> None: super().close() finally: for model in set(self._builder.class_map.values()): - model.unbind() + # a model defined on several databases is bound to the last one; leave that binding be: + if model._db is self: + model.unbind() self._builder.class_map.clear() - if adapter is not None: + if adapter is not None: # pragma: no branch - defensive cleanup adapter.db = None _purge_dialect_expressions(adapter) @@ -420,10 +424,10 @@ def close(self) -> None: "_after_delete", ): hooks = getattr(table, hook_name, None) - if isinstance(hooks, list): + if isinstance(hooks, list): # pragma: no branch - defensive cleanup hooks.clear() - if hasattr(table, "_db"): + if hasattr(table, "_db"): # pragma: no branch - defensive cleanup table._db = None for field_name in getattr(table, "fields", ()): @@ -431,15 +435,15 @@ def close(self) -> None: if field is None: # pragma: no cover continue - if hasattr(field, "_db"): + if hasattr(field, "_db"): # pragma: no branch - defensive cleanup field._db = None - if hasattr(field, "db"): + if hasattr(field, "db"): # pragma: no branch - defensive cleanup field.db = None - if hasattr(field, "table"): + if hasattr(field, "table"): # pragma: no branch - defensive cleanup field.table = None - if hasattr(field, "_table"): + if hasattr(field, "_table"): # pragma: no branch - defensive cleanup field._table = None - if hasattr(field, "requires"): + if hasattr(field, "requires"): # pragma: no branch - defensive cleanup field.requires = [] def try_define[T: t.Any](self, model: t.Type[T], verbose: bool = False) -> t.Type[T]: @@ -451,7 +455,7 @@ def try_define[T: t.Any](self, model: t.Type[T], verbose: bool = False) -> t.Typ except Exception as e: # clean up: self.rollback() - if (tablename := self.to_snake(model.__name__)) and tablename in dir(self): + if (tablename := self.to_snake(model.__name__)) and tablename in dir(self): # pragma: no branch delattr(self, tablename) if verbose: @@ -498,7 +502,9 @@ def define[T: t.Any]( **kwargs: t.Unpack[DefineKwargs], ) -> t.Type[T] | t.Callable[[t.Type[T]], t.Type[T]]: """ - Can be used as a decorator on a class that inherits `TypedTable`, \ + Define a `TypedTable` class on this database. + + Can be used as a decorator on a class that inherits `TypedTable`, or as a regular method if you need to define your classes before you have access to a 'db' instance. You can also pass extra arguments to db.define_table. @@ -554,6 +560,11 @@ def __call__(self, *_args: T_Query, **kwargs: t.Any) -> "TypedSet": _set = super().__call__(*args, **kwargs) return t.cast(TypedSet, _set) + def where(self, query: T_Query | AnyDict | None = None, ignore_common_filters: bool | None = None) -> "UpdateSet": + """Build a PyDAL Set with TypeDAL's affected-ID update callbacks.""" + rows = super().where(query, ignore_common_filters=ignore_common_filters) + return UpdateSet(self, rows.query) + def __getitem__(self, key: str) -> "Table": """ Allows dynamically accessing a table by its name as a string. @@ -741,7 +752,6 @@ def as_typescript(self, *tables: str | type[TypedTable]) -> str: from .tables import TypedTable # noqa: E402 from .caching import ( # isort: skip # noqa: E402 + define_cache_models, memoize, - _TypedalCache, - _TypedalCacheDependency, ) diff --git a/src/typedal/define.py b/src/typedal/define.py index fe1845c..02d387c 100644 --- a/src/typedal/define.py +++ b/src/typedal/define.py @@ -129,11 +129,12 @@ def define[T: t.Any](self, cls: t.Type[T], **kwargs: t.Unpack[DefineKwargs]) -> ) if not tablename.startswith("typedal_") and cache_dependency: - from .caching import _remove_cache, remove_cache_for_table + from .caching import _remove_cache, _remove_cache_after_update, remove_cache_for_table - table._after_insert.append(lambda _row, _id: remove_cache_for_table(tablename)) - table._before_update.append(lambda s, _: _remove_cache(s, tablename)) - table._before_delete.append(lambda s: _remove_cache(s, tablename)) + db = self.db + table._after_insert.append(lambda _row, _id: remove_cache_for_table(db, tablename)) + table._after_update.append(lambda rows, _row: _remove_cache_after_update(db, rows, tablename)) + table._before_delete.append(lambda s: _remove_cache(db, s, tablename)) return cls diff --git a/src/typedal/exceptions.py b/src/typedal/exceptions.py new file mode 100644 index 0000000..fbfc61e --- /dev/null +++ b/src/typedal/exceptions.py @@ -0,0 +1,29 @@ +"""Exceptions raised by TypeDAL operations.""" + + +class TypeDALError(Exception): + """Base class for all TypeDAL exceptions.""" + + +class TypeDALQueryError(TypeDALError, ValueError): + """A query cannot be compiled safely.""" + + +class ImplicitCrossJoinError(TypeDALQueryError): + """A query includes an unrelated table without an explicit cross join.""" + + +class AliasedTableMismatchError(ImplicitCrossJoinError): + """A filter refers to an unaliased copy of an already joined table.""" + + +class UpsertKeyError(TypeDALError, ValueError): + """The upsert key is invalid or has no usable unique constraint.""" + + +class UpsertAmbiguityError(UpsertKeyError): + """The Python upsert path found multiple rows matching the key.""" + + +class UpsertHookError(TypeDALError, RuntimeError): + """A before-hook registration explicitly forbids upsert.""" diff --git a/src/typedal/extensions.py b/src/typedal/extensions.py new file mode 100644 index 0000000..e2538ee --- /dev/null +++ b/src/typedal/extensions.py @@ -0,0 +1,54 @@ +""" +Instance-local extensions TypeDAL installs on each database's PyDAL adapter. + +They patch only that adapter and its dialect, never PyDAL's global registrations. +""" + +from decimal import Decimal, InvalidOperation + +from pydal.adapters.base import BaseAdapter + +from .updates import install_update +from .upsert import install_upsert + + +def _coerce_decimal(value: object) -> object: + # PyDAL's own pre-representer turns "" into NULL for non-string types (an empty optional form field): + if value is None or value == "": + return value + try: + number = value if isinstance(value, Decimal) else Decimal(str(value).strip()) + except InvalidOperation: + raise ValueError(f"Invalid decimal value: {str(value)[:32]!r}") from None + if not number.is_finite(): + raise ValueError(f"Invalid decimal value: {str(value)[:32]!r}") + return number + + +def install_decimal_guard(adapter: BaseAdapter) -> None: + """ + Coerce values for decimal fields before PyDAL renders them into SQL. + + PyDAL's `decimal` representer inlines `str(value)` unquoted, so a string value for a decimal field (from request + data, for example) ends up in the statement verbatim. Integer and double fields don't have this problem because + their representers go through int() and float(). Patched on this adapter's representer only. + """ + representer = getattr(adapter, "representer", None) + if representer is None or getattr(representer, "_typedal_decimal_guard", False): + return + represent = representer.represent + + def guarded(value: object, field_type: object) -> object: + if isinstance(field_type, str) and field_type.startswith("decimal"): + value = _coerce_decimal(value) + return represent(value, field_type) + + representer.represent = guarded + representer._typedal_decimal_guard = True + + +def install_extensions(adapter: BaseAdapter) -> None: + """Install the decimal guard and the upsert and affected-ID update extensions on `adapter`.""" + install_decimal_guard(adapter) + install_upsert(adapter) + install_update(adapter) diff --git a/src/typedal/fields.py b/src/typedal/fields.py index b0346a4..8d1147b 100644 --- a/src/typedal/fields.py +++ b/src/typedal/fields.py @@ -127,7 +127,9 @@ def __get__( owner: t.Type[t.Any], ) -> t.Union[T_Value, "TypedField[T_Value]"]: """ - Since this class is a Descriptor field, \ + Return the field on the class, or the value on an instance. + + Since this class is a Descriptor field, it returns something else depending on if it's called on a class or instance. (this is mostly for mypy/typing) @@ -284,7 +286,6 @@ def rname(field: TypedField[t.Any] | Field) -> str: """ Return the full rname (table and field). """ - table = field._table inner_field = field._field if isinstance(field, TypedField) else field diff --git a/src/typedal/helpers.py b/src/typedal/helpers.py index 0488786..c9ccfa0 100644 --- a/src/typedal/helpers.py +++ b/src/typedal/helpers.py @@ -63,7 +63,9 @@ def _cls_annotations(c: type) -> dict[str, type]: # pragma: no cover def _all_annotations(cls: type) -> ChainMap[str, type]: """ - Returns a dictionary-like ChainMap that includes annotations for all \ + Collect the annotations of cls and its superclasses. + + Returns a dictionary-like ChainMap that includes annotations for all attributes defined in cls or inherited from superclasses. """ # chainmap reverses the iterable, so reverse again beforehand to keep order normally: @@ -567,8 +569,9 @@ def sql_expression( def normalize_table_keys(row: Row, pattern: re.Pattern[str] = re.compile(r"^([a-zA-Z_]+)_(\d{5,})$")) -> Row: """ - Normalize table keys in a PyDAL Row object by stripping numeric hash suffixes from table names, \ - only if the suffix is 5 or more digits. + Normalize table keys in a PyDAL Row object by stripping numeric hash suffixes from table names. + + Only suffixes of 5 or more digits are stripped. For example: Row({'articles_12345': {...}}) -> Row({'articles': {...}}) diff --git a/src/typedal/mixins.py b/src/typedal/mixins.py index 25760ea..c0ad824 100644 --- a/src/typedal/mixins.py +++ b/src/typedal/mixins.py @@ -71,7 +71,8 @@ def set_updated_at(_: Set, row: OpRow) -> None: """ row["updated_at"] = dt.datetime.now() - cls._before_update.append(set_updated_at) + # upsert skips before-hooks, which would leave updated_at stale, so it refuses instead: + cls.before_update(set_updated_at, upsert="error") def slug_random_suffix(length: int = 8) -> str: @@ -122,7 +123,7 @@ def validate[T](self, original: T, record_id: t.Optional[int] = None) -> T: # t row_id = record_id or self.record_id if isinstance(row_id, dict): # pragma: no cover row_id = table(**row_id) - if row_id is not None: + if row_id: # IS_NOT_IN_DB defaults record_id to 0, meaning "no record to exclude" query &= table._id != row_id subset = self.dbset(query) @@ -215,7 +216,8 @@ def __on_define__(cls, db: TypeDAL) -> None: settings = cls.__settings__ # slugs should not be editable (for SEO reasons), so there is only a before insert hook: - cls._before_insert.append(cls.__generate_slug_before_insert) + # upsert skips before-hooks, so the slug would be missing on insert; refuse instead: + cls.before_insert(cls.__generate_slug_before_insert, upsert="error") if settings["slug_suffix"] == 0: # add a validator to the field that will be slugified: diff --git a/src/typedal/query_builder.py b/src/typedal/query_builder.py index f08fa45..1ff0825 100644 --- a/src/typedal/query_builder.py +++ b/src/typedal/query_builder.py @@ -4,7 +4,10 @@ from __future__ import annotations +import copy import datetime as dt +import functools +import inspect import math import time import typing as t @@ -16,6 +19,7 @@ from .asynchronous import run_async from .constants import DEFAULT_JOIN_OPTION, JOIN_OPTIONS from .core import TypeDAL +from .exceptions import AliasedTableMismatchError, ImplicitCrossJoinError from .fields import TypedField, is_typed_field from .helpers import ( DummyQuery, @@ -37,17 +41,151 @@ OrderBy, Permissions, Query, + QueryLike, Row, Rows, Select, SelectKwargs, T_MetaInstance, Table, + UpdateSet, merge_permissions, require_permission, ) from .warnings import NoopQueryWarning, UnusedWindowWarning +_BOOL_OPS = frozenset(("_and", "_or", "_not")) + +# (id of the parent record, relationship path, related row id) -> the related instance attached to that parent +type SeenRelations = dict[tuple[int, str, t.Any], t.Any] +# one argument of where(): a query, a lambda building one, or keyword filters +type WherePart = Query | t.Callable[..., Query] | dict[str, t.Any] + + +class JoinedTable(t.NamedTuple): + """A relationship table as it ends up in the SQL of a builder with joins.""" + + path: str # e.g. "users" or "users.bestie" + table: Table # aliased when the relationship is joined under an alias + tablename: str # the original, unaliased table name + aliased: bool + + +class ResolvedQuery(t.NamedTuple): + """A builder's query as it runs: deferred where() lambdas applied, joined tables pointing at their alias.""" + + query: Query + joined: list[JoinedTable] + rewrites: dict[str, Table] # table name -> the one alias it is joined under, see QueryBuilder._alias_rewrites + + def rewrite(self, node: t.Any) -> t.Any: + """Point `node` (an orderby, a counted field, ...) at the aliases too.""" + return _rewrite_tables(node, self.rewrites) if self.rewrites else node + + +def _as_join_list(value: Expression | Table | t.Sequence[Expression | Table] | None) -> list[Expression | Table]: + """Normalize join/left: PyDAL accepts a bare expression as well as a list of them.""" + if value is None: + return [] + elif isinstance(value, (Expression, Table)): + return [value] + else: + return list(value) + + +def _find_table(parents: dict[str, str], name: str) -> str: + """Union-find lookup of the component a table belongs to.""" + while (parent := parents.setdefault(name, name)) != name: + name = parent + return name + + +def _link_tables(parents: dict[str, str], names: t.Iterable[str]) -> None: + first, *rest = names + anchor = _find_table(parents, first) + for name in rest: + parents[_find_table(parents, name)] = anchor + + +def _walk_tables(node: object, tables: set[str], parents: dict[str, str]) -> set[str]: + """ + Collect the tables referenced by node (like adapter.tables) and link the tables of every comparison. + + Module-level rather than a closure: a self-referencing nested function creates a reference cycle per call, + and the extra cyclic GC runs made SQL building noticeably slower. + """ + if isinstance(node, (list, tuple, set)): + found: set[str] = set() + for item in node: + found |= _walk_tables(item, tables, parents) + return found + elif isinstance(node, Field): + name = t.cast(str, node.tablename) + tables.add(name) + return {name} + elif isinstance(node, SQLALL): + name = node._table._tablename + tables.add(name) + return {name} + elif not isinstance(node, (Expression, Query)): + return set() + found = _walk_tables(node.first, tables, parents) | _walk_tables(node.second, tables, parents) + if len(found) > 1 and isinstance(node, Query) and getattr(node.op, "__name__", None) not in _BOOL_OPS: + _link_tables(parents, found) + return found + + +def _join_alias(key: str, relation: Relationship[t.Any]) -> str: + """The alias a relationship's table is joined under; normalize_table_keys strips the suffix from result rows.""" + return f"{key}_{hash(relation)}" + + +def _rewrite_tables(node: t.Any, tables: dict[str, Table]) -> t.Any: + """ + Rebuild node (a query, expression or field) with every field of a table in `tables` pointing at its alias. + + Unchanged branches are returned as-is, so a query without such fields comes back as the same object. + """ + if isinstance(node, Field): + alias = tables.get(t.cast(str, node.tablename)) + return node if alias is None else alias[node.name] + elif isinstance(node, (list, tuple)): + items = [_rewrite_tables(item, tables) for item in node] + changed = any(new is not old for new, old in zip(items, node, strict=True)) + return type(node)(items) if changed else node + elif not isinstance(node, (Expression, Query)): + return node + + first = _rewrite_tables(node.first, tables) + second = _rewrite_tables(node.second, tables) + if first is node.first and second is node.second: + return node + + clone = copy.copy(node) + clone.first = first + clone.second = second + if isinstance(clone, Expression): + # as Expression.__init__ derives it: + clone._table = getattr(first, "_table", None) + return clone + + +def _requested_tables(part: t.Any) -> list[str]: + """ + The joined tables a where() lambda asks for: its arguments after the model, e.g. `lambda article, tags: ...`. + + Arguments with a default don't ask for anything. + """ + if not callable(part) or isinstance(part, (Field, Query, Expression, dict)) or is_typed_field(part): + return [] + try: + parameters = inspect.signature(part).parameters.values() + except (TypeError, ValueError): # pragma: no cover - builtins without a signature + return [] + positional = (inspect.Parameter.POSITIONAL_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD) + names = [p.name for p in parameters if p.kind in positional and p.default is inspect.Parameter.empty] + return names[1:] + def warn_noop(method: str) -> None: """ @@ -73,6 +211,9 @@ class QueryBuilder[T_MetaInstance: _TypedTable](Select): relationships: dict[str, Relationship[t.Any]] metadata: Metadata _permissions: Permissions + cross_joins: list[t.Type[TypedTable]] + # where() calls with a lambda asking for joined tables; resolved at collect time, ANDed with `query`: + deferred_queries: list[tuple[WherePart, ...]] def __setattr__(self, key: str, value: t.Any) -> None: """Keep QueryBuilder state independent from Select's table-like storage.""" @@ -102,6 +243,8 @@ def __init__( relationships: dict[str, Relationship[t.Any]] | None = None, metadata: Metadata | None = None, permissions: Permissions | None = None, + cross_joins: list[t.Type[TypedTable]] | None = None, + deferred_queries: list[tuple[WherePart, ...]] | None = None, ): """ Normally, you wouldn't manually initialize a QueryBuilder but start using a method on a TypedTable. @@ -118,6 +261,8 @@ def __init__( self.relationships = relationships or {} self.metadata = metadata or {} self._permissions = merge_permissions(getattr(model, "_permissions", None), permissions) + self.cross_joins = cross_joins or [] + self.deferred_queries = deferred_queries or [] def _ensure_table_defined(self) -> Table: model = self.model @@ -160,6 +305,8 @@ def __bool__(self) -> bool: self.select_kwargs, self.relationships, self.metadata, + self.cross_joins, + self.deferred_queries, ], ) @@ -172,6 +319,8 @@ def _extend( relationships: dict[str, Relationship[t.Any]] | None = None, metadata: Metadata | None = None, permissions: Permissions | None = None, + cross_joins: list[t.Type[TypedTable]] | None = None, + deferred_queries: list[tuple[WherePart, ...]] | None = None, ) -> "QueryBuilder[T_MetaInstance]": return QueryBuilder( self.model, @@ -181,6 +330,8 @@ def _extend( (self.relationships | relationships) if relationships else self.relationships, (self.metadata | (metadata or {})) if metadata else self.metadata, # ty: ignore[invalid-argument-type] permissions=merge_permissions(self._permissions, permissions), + cross_joins=self.cross_joins + (cross_joins or []), + deferred_queries=self.deferred_queries + (deferred_queries or []), ) def permissions(self, **permissions: t.Unpack[Permissions]) -> "QueryBuilder[T_MetaInstance]": @@ -236,7 +387,6 @@ def select(self, *fields: t.Any, **options: t.Unpack[SelectKwargs]) -> "QueryBui Calling this without any fields or options on a builder that already has settings does nothing and emits a NoopQueryWarning. """ - if not fields and not options and self: warn_noop("select") return self @@ -290,7 +440,7 @@ def having(self, condition: t.Any) -> "QueryBuilder[T_MetaInstance]": def where( self, - *queries_or_lambdas: Query | t.Callable[[t.Type[T_MetaInstance]], Query] | dict[str, t.Any], + *queries_or_lambdas: WherePart, **filters: t.Any, ) -> "QueryBuilder[T_MetaInstance]": """ @@ -306,6 +456,12 @@ def where( When passing multiple queries to a single .where, they will be ORed: .where(lambda table: table.id == 5, lambda table: table.id == 6) == (table.id == 5) | (table.id=6) + Joined relationships live under an alias in the SQL. A field of a joined table (`.where(Tag.name == "x")`) + is resolved to that alias when the builder runs, so `.where(...).join(...)` and `.join(...).where(...)` both + work. When the same table is joined more than once, pick the join with extra lambda arguments, named after + the relationship (nested ones as `parent__child`), which are also resolved when the builder runs: + .join("tags").where(lambda article, tags: tags.name == "x") + Calling this without any arguments on a builder that already has settings does nothing and emits a NoopQueryWarning. Starting an empty builder (e.g. `Model.where()`) is allowed and stays silent. @@ -314,23 +470,34 @@ def where( warn_noop("where") return self - new_query = self.query - table = self._ensure_table_defined() - queries_or_lambdas = ( *queries_or_lambdas, filters, ) + if any(_requested_tables(query_part) for query_part in queries_or_lambdas): + # the joined tables (and their aliases) are only known once the builder runs: + return self._extend(deferred_queries=[queries_or_lambdas]) + + new_query = self.query + if subquery := self._where_group(queries_or_lambdas, {}): + new_query &= subquery + + return self._extend(overwrite_query=new_query) + + def _where_group(self, queries_or_lambdas: tuple[WherePart, ...], joined: dict[str, Table]) -> Query: + """ + OR the parts of one where() call together. + + `joined` maps relationship names to their (aliased) tables, for lambdas that ask for them by name. + """ + table = self._ensure_table_defined() subquery = t.cast(Query, DummyQuery()) for query_part in queries_or_lambdas: if isinstance(query_part, Field) or is_typed_field(query_part): subquery |= query_part != None elif isinstance(query_part, (Query, Expression)): subquery |= t.cast(Query, query_part) - elif callable(query_part): - if result := query_part(self.model): - subquery |= t.cast(Query, result) elif isinstance(query_part, dict): subsubquery = DummyQuery() for field, value in query_part.items(): @@ -338,13 +505,21 @@ def where( if subsubquery: # DummyQuery is falsy, so reaching here means it became a Query. subquery |= t.cast(Query, subsubquery) + elif callable(query_part): + if result := query_part(self.model, *self._bind_joined_tables(query_part, joined)): + subquery |= t.cast(Query, result) else: raise ValueError(f"Unexpected query type ({type(query_part)}).") - if subquery: - new_query &= subquery + return subquery - return self._extend(overwrite_query=new_query) + def _bind_joined_tables(self, fn: t.Callable[..., t.Any], joined: dict[str, Table]) -> list[Table]: + """Look up the joined tables that a where() lambda asks for, by the names of its extra arguments.""" + names = _requested_tables(fn) + if missing := [name for name in names if name not in joined]: + available = ", ".join(sorted(joined)) or "none" + raise ValueError(f"where() asks for unjoined relationship(s) {', '.join(missing)} (joined: {available})") + return [joined[name] for name in names] def _parse_relationships( self, @@ -358,7 +533,8 @@ def _parse_relationships( Args: fields: Iterable of relationship field names (e.g., ['relationship', 'relationship.with_nested', 'relationship.no2']) - condition_and: Optional condition to pass to relationship clones + method: Join method to use instead of each relationship's own + update: Extra settings passed to the relationship clones (e.g. condition_and) Returns: Dict mapping base relationship names to Relationship objects with nested relationships @@ -440,6 +616,8 @@ def join( condition = as_lambda(condition) field = fields[0] + if isinstance(field, str): + field = self.model.get_relationships().get(field, field) if isinstance(field, Relationship) and field.name: relationships = { field.name: field.clone(condition=condition, on=None, join=method, condition_and=condition_and) @@ -485,7 +663,7 @@ def join( # Clone direct Relationship instances (preserving their settings) for relationship in relationship_instances.values(): - if relationship.name: + if relationship.name: # pragma: no branch - bound relationships always have a name relationships[relationship.name] = relationship.clone( join=method, condition_and=condition_and, @@ -506,6 +684,14 @@ def join( return self._extend(relationships=relationships) + def cross_join(self, table: t.Type[TypedTable]) -> "QueryBuilder[T_MetaInstance]": + """Explicitly include an unrelated table in the query.""" + if table._db is not self._get_db(): + raise ValueError("cross join table must belong to the same database") + if table in self.cross_joins: + return self + return self._extend(cross_joins=[table]) + def cache( self, *deps: t.Any, @@ -513,8 +699,9 @@ def cache( ttl: t.Optional[int | dt.timedelta] = None, ) -> "QueryBuilder[T_MetaInstance]": """ - Enable caching for this query to load repeated calls from a dill row \ - instead of executing the sql and collecing matching rows again. + Enable caching for this query. + + Repeated calls are loaded from a dill row instead of executing the sql and collecting matching rows again. """ existing = self.metadata.get("cache", {}) @@ -543,14 +730,25 @@ def _select_arg_convert(self, arg: t.Any) -> t.Any: return arg + def _mutation_query(self) -> Query: + """ + The query for delete and update, which ignore joins. + + A where() lambda that asks for joined tables only filters through those joins, so leaving it out would + delete or update more rows than asked for. + """ + if self.deferred_queries: + raise ValueError("delete() and update() can't use where() lambdas that ask for joined relationships") + return self.query + def delete(self) -> list[int]: """ Based on the current query, delete rows and return a list of deleted IDs. """ require_permission(self._permissions, "delete") db = self._get_db() - removed_ids = [_.id for _ in db(self.query).select("id")] - if db(self.query).delete(): + removed_ids = [_.id for _ in db(self._mutation_query()).select("id")] + if db(self._mutation_query()).delete(): # success! return removed_ids @@ -558,7 +756,7 @@ def delete(self) -> list[int]: def _delete(self) -> str: db = self._get_db() - return str(db(self.query)._delete()) + return str(db(self._mutation_query())._delete()) # QueryBuilder subclasses Select for its query-building surface but yields # model instances rather than fields, so these two are not substitutable. @@ -569,21 +767,153 @@ def update(self, **fields: t.Any) -> list[int]: # ty: ignore[invalid-method-ove # todo: limit? require_permission(self._permissions, "update") db = self._get_db() - updated_ids = db(self.query).select("id").column("id") - if db(self.query).update(**fields): - # success! - return updated_ids - - return [] + rows = t.cast(UpdateSet, db(self._mutation_query())) + updated_ids = rows.update_ids(**fields) + # a TypedTable's primary key is always its integer id + return t.cast(list[int], updated_ids) def _update(self, **fields: t.Any) -> str: db = self._get_db() - return str(db(self.query)._update(**fields)) + return str(db(self._mutation_query())._update(**fields)) + + def _joined_tables(self) -> list[JoinedTable]: + """Every joined relationship (nested ones included) with the table it uses in the SQL.""" + joined: list[JoinedTable] = [] + for key, relation in self.relationships.items(): + self._collect_joined_tables(relation, key, self.model, key, joined) + return joined + + def _join_table(self, relation: Relationship[t.Any], key: str, parent: t.Any) -> tuple[t.Type[TypedTable], bool]: + """ + The table a relationship uses in the SQL, and whether that is an alias. + + Every join is aliased, except an `on=` relationship to a table other than its parent or the root table: + its `on` callback gets the real table. All join building goes through here, so the aliases always agree. + """ + other = relation.get_table(self._get_db()) + if other is parent or other is self.model or not relation.on: + return other.with_alias(_join_alias(key, relation)), True + return other, False + + def _collect_joined_tables( + self, + relation: Relationship[t.Any], + key: str, + parent: t.Any, + path: str, + joined: list[JoinedTable], + ) -> None: + table, aliased = self._join_table(relation, key, parent) + tablename = relation.get_table(self._get_db())._tablename + joined.append(JoinedTable(path, t.cast(Table, table), tablename, aliased)) + + for nested_name, nested in relation.nested.items(): + self._collect_joined_tables(nested, nested_name, table, f"{path}.{nested_name}", joined) + + def _alias_rewrites(self, joined: list[JoinedTable]) -> dict[str, Table]: + """ + Map table names to the one alias they are joined under. + + Left out (so the table name keeps meaning the table itself): the root table, `cross_join()` tables, + tables joined without an alias (`on=`) and tables joined more than once, which _validate_joins rejects. + """ + excluded = {self._ensure_table_defined()._tablename} + excluded |= {table._ensure_table_defined()._tablename for table in self.cross_joins} + + per_table: dict[str, list[JoinedTable]] = defaultdict(list) + for entry in joined: + per_table[entry.tablename].append(entry) + + return { + name: entries[0].table + for name, entries in per_table.items() + if len(entries) == 1 and entries[0].aliased and name not in excluded + } + + @staticmethod + def _joined_by_name(joined: list[JoinedTable]) -> dict[str, Table]: + """The names a where() lambda can ask for: `tags`, `tags__author` and, when unambiguous, `author`.""" + names = {entry.path.replace(".", "__"): entry.table for entry in joined} + + per_leaf: dict[str, list[Table]] = defaultdict(list) + for entry in joined: + per_leaf[entry.path.rsplit(".", 1)[-1]].append(entry.table) + for name, tables in per_leaf.items(): + if len(tables) == 1: + names.setdefault(name, tables[0]) + + return names + + def _resolve(self) -> ResolvedQuery: + """The query as it runs, see ResolvedQuery. Collect, count and the cache key all start from this.""" + joined = self._joined_tables() + rewrites = self._alias_rewrites(joined) + query = self.query + if self.deferred_queries: + by_name = self._joined_by_name(joined) + for queries_or_lambdas in self.deferred_queries: + if subquery := self._where_group(queries_or_lambdas, by_name): + query &= subquery + + if rewrites: + # a plain table relating to another table (`Article.reviewer == Author.id`) was an independent copy in v5; + # rewriting it to the join's alias would silently change the result, so refuse instead: + parents: dict[str, str] = {} + tables = _walk_tables(query, set(), parents) + for name in rewrites.keys() & tables: + if any(other != name and _find_table(parents, other) == _find_table(parents, name) for other in tables): + raise AliasedTableMismatchError( + f"Table {name!r} is joined, so a plain {name!r} in where() means that join and can't be " + f"compared with other tables; use a lambda to pick the join or cross_join() for a separate copy" + ) + query = t.cast(Query, _rewrite_tables(query, rewrites)) + return ResolvedQuery(query, joined, rewrites) + + @staticmethod + def _required_join_names(joined: list[JoinedTable], *nodes: t.Any) -> set[str]: + """ + The (aliased) names of the joined tables that `nodes` (a predicate, a counted field) use. + + Includes the joins they hang from, since a nested join's ON clause references its parent's join. + """ + referenced: set[str] = set() + for node in nodes: + referenced |= _walk_tables(node, set(), {}) + paths = {entry.path for entry in joined if entry.table._tablename in referenced} + paths |= {path.rsplit(".", depth)[0] for path in paths for depth in range(1, path.count(".") + 1)} + return {entry.table._tablename for entry in joined if entry.path in paths} + + @staticmethod + def _required_left_joins(names: set[str], left_joins: list[Expression]) -> list[Expression]: + """ + The left joins of the tables in `names` and their transitive ON dependencies. + + Queries that replace the select (the id subquery of limitby, count) need these: without them, a filter on a + left-joined table references a table that isn't in their FROM clause. + """ + dependencies: dict[str, set[str]] = defaultdict(set) + for join in left_joins: + if isinstance(join.first, Table): + dependencies[join.first._tablename] |= _walk_tables(join.second, set(), {}) + + required = names.copy() + pending = names.copy() + while pending: + name = pending.pop() + referenced = dependencies.get(name, set()) - required + required |= referenced + pending |= referenced + + return [join for join in left_joins if isinstance(join.first, Table) and join.first._tablename in required] def _before_query(self, mut_metadata: Metadata, add_id: bool = True) -> tuple[Query, list[t.Any], SelectKwargs]: select_args = [self._select_arg_convert(_) for _ in self.select_args] or [self.model.ALL] select_kwargs = self.select_kwargs.copy() - query = self.query + resolved = self._resolve() + query, joined = resolved.query, resolved.joined + for option in ("orderby", "groupby", "having"): + if option in select_kwargs: + select_kwargs[option] = resolved.rewrite(select_kwargs[option]) model = self.model mut_metadata["query"] = query # require at least id of main table: @@ -595,10 +925,77 @@ def _before_query(self, mut_metadata: Metadata, add_id: bool = True) -> tuple[Qu select_args.append(model.id) if self.relationships: - query, select_args = self._handle_relationships_pre_select(query, select_args, select_kwargs, mut_metadata) + query, select_args = self._handle_relationships_pre_select( + query, select_args, select_kwargs, mut_metadata, joined + ) + else: + self._validate_joins(query, [], select_args, select_kwargs, joined) + + for table in self.cross_joins: + query &= table.id > 0 return query, select_args, select_kwargs + def _validate_joins( + self, + query: Query, + conditions: list[QueryLike], + fields: list[t.Any], + options: SelectKwargs, + joined: list[JoinedTable], + ) -> None: + """ + Reject tables that would end up in the FROM clause without anything relating them to the root table. + + Walks the (resolved) predicate, the extra `conditions`, the selected `fields` and the ON clauses once. + Every table referenced inside a single comparison (`A.x == B.y`, but also `(A.x + B.y) > 3`) counts as + linked. The root table, explicit JOIN targets and `cross_join()` tables seed the connected component. + """ + root = self._ensure_table_defined()._tablename + explicit = {table._ensure_table_defined()._tablename for table in self.cross_joins} + joins = [*_as_join_list(options.get("join")), *_as_join_list(options.get("left"))] + + tables: set[str] = set() + parents: dict[str, str] = {} + walk = functools.partial(_walk_tables, tables=tables, parents=parents) + + predicate_tables = walk(query) + for condition in conditions: + walk(condition) + for field in fields: + walk(field) + + seeds = {root, *explicit} + for join in joins: + if isinstance(join, Table): + seeds.add(join._tablename) + continue + if isinstance(join.first, Table): # pragma: no branch - `table.on(...)` always starts with a table + seeds.add(join.first._tablename) + walk(join.second) + + # _alias_rewrites resolved every table joined under exactly one alias, what's left is ambiguous: + per_table: dict[str, list[JoinedTable]] = defaultdict(list) + for entry in joined: + per_table[entry.tablename].append(entry) + for original, entries in per_table.items(): + if original not in predicate_tables or original == root or original in explicit: + continue + if all(entry.aliased for entry in entries): + paths = ", ".join(repr(entry.path) for entry in entries) + pick = entries[0].path.replace(".", "__") + raise AliasedTableMismatchError( + f"Table {original!r} is joined under multiple aliases ({paths}); " + f"pick one with .where(lambda row, {pick}: ...)" + ) + + tables |= seeds + _link_tables(parents, seeds) + anchor = _find_table(parents, root) + if disconnected := {name for name in tables if _find_table(parents, name) != anchor}: + names = ", ".join(sorted(disconnected)) + raise ImplicitCrossJoinError(f"Implicit cross join involving {names}; use cross_join()") + def to_sql(self, add_id: bool = False) -> str: """ Generate the SQL for the built query. @@ -633,7 +1030,7 @@ def _collect_cached( self.model, f"{into.__module__}.{into.__qualname__}", metadata, - self.query, + self._cache_key_query(), self.select_args, self.select_kwargs, self.relationships.keys(), @@ -645,6 +1042,18 @@ def _collect_cached( return load_from_cache(key, self._get_db()) + def _cache_key_query(self) -> Query | str: + """The resolved query, with alias names (which contain a per-process hash) replaced by relationship paths.""" + resolved = self._resolve() + if resolved.query is self.query: + return resolved.query + + key = str(resolved.query) + for entry in resolved.joined: + if entry.aliased: + key = key.replace(entry.table._tablename, f"<{entry.path}>") + return key + def execute(self, add_id: bool = False) -> Rows: """ Raw version of .collect which only executes the SQL, without performing t.Any magic afterwards. @@ -808,22 +1217,34 @@ def _handle_relationships_pre_select( select_args: list[t.Any], select_kwargs: SelectKwargs, metadata: Metadata, + joined: list[JoinedTable], ) -> tuple[Query, list[t.Any]]: """Handle relationship joins and field selection for database query.""" # Collect all relationship keys including nested ones metadata["relationships"] = self._collect_all_relationship_keys() - # Build joins and apply limitby optimization if needed + # Build joins and handle field selection inner_joins = self._build_inner_joins() - query = self._apply_limitby_optimization(query, select_kwargs, inner_joins, metadata) + left_joins: list[Expression] = [] + select_args = self._build_left_joins_and_fields(select_args, left_joins) + + # validate before the limitby optimization replaces the predicate with an ID subquery: + self._validate_joins( + query, + [], + select_args, + {"join": inner_joins or select_kwargs.get("join"), "left": left_joins}, + joined, + ) + required_names = self._required_join_names( + joined, query, select_kwargs.get("orderby"), select_kwargs.get("groupby"), select_kwargs.get("having") + ) + required_left = self._required_left_joins(required_names, left_joins) + query = self._apply_limitby_optimization(query, select_kwargs, inner_joins, metadata, required_left) if inner_joins: select_kwargs["join"] = inner_joins - # Build left joins and handle field selection - left_joins: list[Expression] = [] - select_args = self._build_left_joins_and_fields(select_args, left_joins) - select_kwargs["left"] = left_joins return query, select_args @@ -864,13 +1285,11 @@ def _build_inner_joins_recursive( parent_key: str = "", ) -> list[t.Any]: """Recursively build inner joins for a relationship and its nested relationships.""" - db = self._get_db() joins = [] # Handle current level if relation.condition and relation.join == "inner": - other = relation.get_table(db) - other = other.with_alias(f"{key}_{hash(relation)}") + other, _ = self._join_table(relation, key, parent_table) condition = relation.condition(parent_table, other) # ty: ignore[invalid-argument-type] if callable(relation.condition_and): @@ -911,7 +1330,7 @@ def _selectable_orderby_fields(self, orderby: OrderBy | t.Iterable[OrderBy] | No first = getattr(orderby, "first", None) second = getattr(orderby, "second", None) - if first is not None: + if first is not None: # pragma: no branch - an orderby expression with a second part has a first fields.extend(self._selectable_orderby_fields(first)) if second is not None: fields.extend(self._selectable_orderby_fields(second)) @@ -923,7 +1342,8 @@ def _select_distinct_ids_with_orderby_fields(self, query: Query, select_kwargs: model = self.model id_field = t.cast(TypedField[int], model.id) - select_args: list[OrderBy] = [id_field] + # named, because a joined table's id can be among the orderby fields too: + select_args: list[OrderBy] = [t.cast(OrderBy, id_field.with_alias("typedal_paginate_id"))] seen = {str(model.id)} for field in self._selectable_orderby_fields(select_kwargs.get("orderby")): @@ -932,11 +1352,28 @@ def _select_distinct_ids_with_orderby_fields(self, query: Query, select_kwargs: select_args.append(field) seen.add(key) - ids = db(query)._select(*select_args, **select_kwargs).rstrip(";") - id_column = getattr(model.id, "_raw_rname", id_field.name) - return f'SELECT "{id_column}" FROM ({ids}) AS typedal_paginate_ids' # nosec: - # id_column originates from code - # ids is a safe subquery, originating from code + kwargs = select_kwargs.copy() + limitby = kwargs.pop("limitby", None) + orderby = kwargs.pop("orderby") + ordering = orderby if isinstance(orderby, (list, tuple)) else [orderby] + sql_orderby = ", ".join(db._adapter.expand(field) for field in ordering) + # Rank joined rows before reducing them to one entry per root. The first + # occurrence in the requested ordering determines each root's page. + select_args.append(f"ROW_NUMBER() OVER (ORDER BY {sql_orderby}) AS typedal_paginate_position") + kwargs["distinct"] = False + ids = db(query)._select(*select_args, **kwargs).rstrip(";") + dialect = db._adapter.dialect + id_column = dialect.quote("typedal_paginate_id") + position_column = dialect.quote("typedal_paginate_position") + limited_ids = dialect.select( + id_column, + f"({ids}) AS typedal_paginate_ids", + groupby=id_column, + orderby=f"MIN({position_column})", + limitby=limitby, + ).rstrip(";") + # MySQL requires an unlimited outer SELECT for a limited IN subquery. + return dialect.select(id_column, f"({limited_ids}) AS typedal_paginate_limited_ids") def _apply_limitby_optimization( self, @@ -944,8 +1381,13 @@ def _apply_limitby_optimization( select_kwargs: SelectKwargs, joins: list[t.Any], metadata: Metadata, + left_joins: list[Expression] | None = None, ) -> Query: - """Apply limitby optimization when relationships are present.""" + """ + Apply limitby optimization when relationships are present. + + `left_joins` are the left joins used by filtering and ordering, see _required_left_joins. + """ if not (limitby := select_kwargs.pop("limitby", ())): return query @@ -957,16 +1399,22 @@ def _apply_limitby_optimization( if joins: kwargs["join"] = joins + if left_joins: + kwargs["left"] = left_joins + if joins or left_joins: kwargs["distinct"] = True + if left_joins and not kwargs.get("orderby") and kwargs.get("orderby_on_limitby", True): + # pydal's implicit limitby order includes the left-joined ids, which DISTINCT doesn't select: + kwargs["orderby"] = t.cast(OrderBy, model.id) - if joins and kwargs.get("orderby"): + if (joins or left_joins) and kwargs.get("orderby"): ids = self._select_distinct_ids_with_orderby_fields(query, kwargs) else: ids = db(query)._select(model.id, **kwargs) id_field = t.cast(TypedField[int], model.id) - query = id_field.belongs(ids) + query &= id_field.belongs(ids) metadata["ids"] = ids return query @@ -990,52 +1438,44 @@ def _process_relationship_for_left_join( parent_key: str = "", ) -> list[t.Any]: """Process a single relationship for left join and field selection.""" - db = self._get_db() - other = relation.get_table(db) + other = relation.get_table(self._get_db()) + table, _ = self._join_table(relation, key, parent_table) method: JOIN_OPTIONS = relation.join or DEFAULT_JOIN_OPTION - select_fields = ", ".join([str(_) for _ in select_args]) - pre_alias = str(other) - # Self-referencing relationship: 'other' is the same table as 'parent_table', so the - # name-based helpers below can't tell their fields apart. Alias upfront and skip them. - is_self_reference = other is parent_table + # Self-referencing relationship: 'other' is the same table as 'parent_table' (or, for a nested relationship + # like 'writer.articles', as the root table), so the name-based helpers below can't tell their fields apart. + # Select the alias directly and skip them. + is_self_reference = other is parent_table or other is self.model if is_self_reference: - other = other.with_alias(f"{key}_{hash(relation)}") - select_args = [*select_args, other.ALL] + select_args = [*select_args, table.ALL] else: # Ensure required fields are selected + select_fields = ", ".join([str(_) for _ in select_args]) select_args = self._ensure_relationship_fields(select_args, other, select_fields) # Build join condition if relation.on: # Custom .on condition - always left join - # (for is_self_reference, 'other' was already aliased above so the on() callback - # receives distinguishable tables for both sides of the join) - on = relation.on(parent_table, other) # ty: ignore[invalid-argument-type] + # (for is_self_reference, 'table' is an alias, so the on() callback gets distinguishable tables) + on = relation.on(parent_table, table) # ty: ignore[invalid-argument-type] if not isinstance(on, list): on = [on] on = [_ for _ in on if isinstance(_, Expression)] left_joins.extend(on) elif method == "left": - # Generate left join condition - if not is_self_reference: - other = other.with_alias(f"{key}_{hash(relation)}") - condition = t.cast(Query, relation.condition(parent_table, other)) # ty: ignore[call-non-callable, invalid-argument-type] + condition = t.cast(Query, relation.condition(parent_table, table)) # ty: ignore[call-non-callable, invalid-argument-type] if callable(relation.condition_and): - condition &= relation.condition_and(parent_table, other) # ty: ignore[invalid-argument-type] + condition &= relation.condition_and(parent_table, table) # ty: ignore[invalid-argument-type] - left_joins.append(other.on(condition)) - else: - # Inner join (handled in _build_inner_joins) - if not is_self_reference: - other = other.with_alias(f"{key}_{hash(relation)}") + left_joins.append(table.on(condition)) + # (inner joins are built by _build_inner_joins) - # Handle aliasing in select_args + # Point the selected fields at the alias if not is_self_reference: - select_args = self._update_select_args_with_alias(select_args, pre_alias, other) + select_args = self._update_select_args_with_alias(select_args, str(other), table) # Process nested relationships for nested_name, nested in relation.nested.items(): @@ -1046,7 +1486,7 @@ def _process_relationship_for_left_join( nested_name, select_args, left_joins, - other, + table, nested_key, ) @@ -1109,8 +1549,8 @@ def _collect_with_relationships( # id: [Row] raw_per_id: dict[t.Any, list[t.Any]] = defaultdict(list) - # Track what we've seen: main_id -> "column-relation_id" - seen_relations: dict[str, set[str]] = defaultdict(set) + # Track what we've seen: (parent record, column, relation id) -> instance + seen_relations: SeenRelations = {} for row in rows: main = row[main_table] @@ -1135,7 +1575,6 @@ def _collect_with_relationships( column=column, relation=relation, parent_record=records[main_id], - parent_id=main_id, seen_relations=seen_relations, db=db, ) @@ -1148,8 +1587,7 @@ def _process_relationship_data( column: str, relation: Relationship[t.Any], parent_record: t.Any, - parent_id: t.Any, - seen_relations: dict[str, set[str]], + seen_relations: SeenRelations, db: t.Any, path: str = "", ) -> t.Any | None: @@ -1163,19 +1601,18 @@ def _process_relationship_data( column: The relationship column name relation: The Relationship object parent_record: The parent model instance to attach data to - parent_id: ID of the parent for tracking seen_relations: Dict tracking which relationships we've already processed db: Database instance path: Current relationship path (e.g., "users.bestie") Returns: - The created relationship instance, or None if skipped + The created relationship instance, or None if skipped (no data, or already attached) """ # Build the full path for tracking (e.g., "users", "users.bestie", "users.bestie.articles") current_path = f"{path}.{column}" if path else column - # Get the relationship column name (with hash for alias) - relationship_column = f"{column}_{hash(relation)}" + # the alias this relationship was joined under (see _join_table) + relationship_column = _join_alias(column, relation) # Get relation data from row relation_data = row[relationship_column] if relationship_column in row else row.get(relation.get_table_name()) @@ -1184,16 +1621,19 @@ def _process_relationship_data( if not relation_data or relation_data.id is None: return None - # Check if we've already seen this relationship instance - seen_key = f"{current_path}-{relation_data.id}" - if seen_key in seen_relations[parent_id]: - return None # Already processed - - seen_relations[parent_id].add(seen_key) + # Check if we've already attached this relationship instance to this parent. Keyed by the parent object: + # the same row (e.g. an author) reached through different parents is a separate instance for each of them. + seen_key = (id(parent_record), current_path, relation_data.id) + if (seen := seen_relations.get(seen_key)) is not None: + # a later row can still hold new data for its nested relationships (e.g. the author's second article) + if relation.nested: + self._process_nested_relationships(row, relation, seen, seen_relations, db, current_path) + return None # Create the relationship instance relation_table = relation.get_table(db) instance = relation_table(relation_data) if looks_like(relation_table, TypedTable) else relation_data + seen_relations[seen_key] = instance # Process nested relationships on this instance if relation.nested: @@ -1201,7 +1641,6 @@ def _process_relationship_data( row=row, relation=relation, instance=instance, - # parent_id=parent_id, seen_relations=seen_relations, db=db, path=current_path, @@ -1223,7 +1662,7 @@ def _process_nested_relationships( row: t.Any, relation: Relationship[t.Any], instance: t.Any, - seen_relations: dict[str, set[str]], + seen_relations: SeenRelations, db: t.Any, path: str, ) -> None: @@ -1251,7 +1690,6 @@ def _process_nested_relationships( column=nested_col, relation=nested_relation, parent_record=instance, - parent_id=instance.id, seen_relations=seen_relations, db=db, path=path, @@ -1280,9 +1718,11 @@ def __iter__(self) -> t.Generator[T_MetaInstance, None, None]: # ty: ignore[inv yield from chunk def __await__(self): + """`await builder` collects the rows asynchronously.""" return self.collect_async().__await__() async def __aiter__(self): + """`async for row in builder` yields rows, in windows when `.window()` was used.""" builder = self._extend(metadata={"iterating": True}) window_size = self.metadata.get("window_size", None) @@ -1297,6 +1737,7 @@ async def __aiter__(self): yield row def window(self, window_size: int) -> QueryBuilder[T_MetaInstance]: + """Iterate in chunks of `window_size` rows instead of loading everything at once.""" return self._extend(metadata={"window_size": window_size}) def __count( @@ -1305,27 +1746,84 @@ def __count( distinct: bool | Field | TypedField[t.Any] = False, *, include_left_for_distinct: bool = True, - ) -> Query: - # internal, shared logic between .count and ._count - model = self.model - query = self.query - for key, relation in self.relationships.items(): - if not relation.condition: - continue + ) -> tuple[Query | str, bool | Field | TypedField[t.Any]]: + """ + Internal, shared logic between .count and ._count. - include_left_join = distinct and include_left_for_distinct - if relation.join != "inner" and not include_left_join: - continue + Returns the query to count, or the full SQL when the query filters on a left-joined table + (pydal's count can't LEFT JOIN), plus the `distinct` to count with. + """ + resolved = self._resolve() + query = resolved.query + if isinstance(distinct, (Field, Expression)): + distinct = resolved.rewrite(distinct) - other = relation.get_table(db) - if not distinct: - # todo: can this lead to other issues? - other = other.with_alias(f"{key}_{hash(relation)}") + for table in self.cross_joins: + query &= table.id > 0 - if relation.condition is not None: - query &= relation.condition(model, other) # ty: ignore[invalid-argument-type] + if left_joins := self._count_left_joins(resolved.joined, query, distinct): + return self._count_sql_with_left_joins(db, query, distinct, left_joins, resolved.joined), distinct - return query + conditions: list[QueryLike] = [join.second for join in self._build_inner_joins()] + if distinct and include_left_for_distinct: + conditions += self._left_join_conditions() + + self._validate_joins(query, conditions, [self.model.id], {}, resolved.joined) + for condition in conditions: + query &= condition + return query, distinct + + def _count_left_joins(self, joined: list[JoinedTable], query: Query, distinct: t.Any) -> list[Expression]: + """The left joins a count has to keep, because the query or the counted field uses their table.""" + names = self._required_join_names(joined, query, distinct) + if not names: + return [] + left_joins: list[Expression] = [] + self._build_left_joins_and_fields([], left_joins) + return self._required_left_joins(names, left_joins) + + def _count_sql_with_left_joins( + self, + db: TypeDAL, + query: Query, + distinct: bool | Field | TypedField[t.Any], + left_joins: list[Expression], + joined: list[JoinedTable], + ) -> str: + """COUNT over a subquery, since pydal's count() can't LEFT JOIN.""" + inner_joins = self._build_inner_joins() + self._validate_joins(query, [], [self.model.id], {"join": inner_joins, "left": left_joins}, joined) + options: SelectKwargs = {"left": left_joins, "distinct": bool(distinct)} + if inner_joins: + options["join"] = inner_joins + counted = t.cast(Field, distinct if isinstance(distinct, (Field, Expression)) else self.model.id) + # COUNT(column) skips the NULL a left join yields, like COUNT(DISTINCT column) does: + subquery = db(query)._select(counted.with_alias("typedal_counted"), **options).rstrip().rstrip(";") + column = db._adapter.dialect.quote("typedal_counted") + return f"SELECT COUNT({column}) FROM ({subquery}) AS typedal_count;" # noqa: S608 + # subquery is generated by pydal + + def _left_join_conditions(self) -> list[QueryLike]: + """ + The join conditions of the left-joined relationships, as WHERE conditions for a distinct count. + + Uses the same aliases as the select, so a filter on such a relationship's table applies to its join. + """ + model = self.model + conditions: list[QueryLike] = [] + for key, relation in self.relationships.items(): + if not relation.condition or relation.join == "inner": + continue + other, _ = self._join_table(relation, key, model) + conditions.append(relation.condition(model, other)) # ty: ignore[invalid-argument-type] + if callable(relation.condition_and): + conditions.append(relation.condition_and(model, other)) # ty: ignore[invalid-argument-type] + return conditions + + def __execute_count(self, db: TypeDAL, query: Query | str, distinct: bool | Field | TypedField[t.Any]) -> int: + if isinstance(query, str): + return int(db.executesql(query)[0][0]) + return db(query).count(distinct) def count(self, distinct: bool | Field | TypedField[t.Any] = False) -> int: """ @@ -1335,18 +1833,18 @@ def count(self, distinct: bool | Field | TypedField[t.Any] = False) -> int: """ require_permission(self._permissions, "read") db = self._get_db() - query = self.__count(db, distinct=distinct) + query, distinct = self.__count(db, distinct=distinct) - return db(query).count(distinct) + return self.__execute_count(db, query, distinct) def _count(self, distinct: bool | Field | TypedField[t.Any] = False) -> str: """ Return the SQL for .count(). """ db = self._get_db() - query = self.__count(db, distinct=distinct) + query, distinct = self.__count(db, distinct=distinct) - return t.cast(str, db(query)._count(distinct)) + return query if isinstance(query, str) else db(query)._count(distinct) def exists(self) -> bool: """ @@ -1366,8 +1864,8 @@ def __pagination_count(self) -> int: db = self._get_db() distinct = t.cast(TypedField[int], self.model.id) - query = self.__count(db, distinct=distinct, include_left_for_distinct=False) - return db(query).count(distinct) + query, distinct = self.__count(db, distinct=distinct, include_left_for_distinct=False) + return self.__execute_count(db, query, distinct) def __paginate( self, diff --git a/src/typedal/relationships.py b/src/typedal/relationships.py index 8e007ec..8697f0c 100644 --- a/src/typedal/relationships.py +++ b/src/typedal/relationships.py @@ -58,7 +58,7 @@ def __init__( raise self._error_duplicate_condition(condition, on) resolved_type = resolve_relationship_type(_type, keep_unresolved=True) - if resolved_type is not None: + if resolved_type is not None: # pragma: no branch - keep_unresolved returns the input when unresolved _type = resolved_type self._type = _type @@ -289,7 +289,7 @@ def __get__( resolved_table = self.get_table(db) builder = owner.where(id=instance.id).join(self.name) - if issubclass(resolved_table, TypedTable) or isinstance(resolved_table, Table): + if issubclass(resolved_table, TypedTable) or isinstance(resolved_table, Table): # pragma: no branch # is a table so we can select ALL and ignore non-required fields of parent row: builder = builder.select(owner.id, resolved_table.ALL) @@ -327,7 +327,7 @@ class Ref[To_Type: TypedTable]: @t.overload -def relationship( +def relationship( # noqa: D417 - overloads only document what differs _type: type[list[To_Type]], condition: Condition = None, join: JOIN_OPTIONS = None, @@ -348,7 +348,7 @@ def relationship( @t.overload -def relationship( +def relationship( # noqa: D417 - overloads only document what differs _type: t.Type[To_Type] | str | t.Type[Ref[To_Type]], condition: Condition = None, *, @@ -371,7 +371,7 @@ def relationship( @t.overload -def relationship( +def relationship( # noqa: D417 - overloads only document what differs _type: t.Type[To_Type] | str | t.Type[Ref[To_Type]], condition: Condition = None, join: JOIN_OPTIONS = None, diff --git a/src/typedal/rows.py b/src/typedal/rows.py index e3416f7..d9eb789 100644 --- a/src/typedal/rows.py +++ b/src/typedal/rows.py @@ -94,7 +94,7 @@ def __len__(self) -> int: """ return len(self.records) - def __iter__(self) -> t.Iterator[T_MetaInstance]: + def __iter__(self) -> t.Iterator[T_MetaInstance]: # ty: ignore[invalid-method-override] """ Loop through the rows. """ @@ -159,8 +159,9 @@ def exclude( # ty: ignore[invalid-method-override] self, f: t.Callable[[T_MetaInstance], Query] ) -> "TypedRows[T_MetaInstance]": """ - Removes elements from the calling Rows object, filtered by the function `f`, \ - and returns a new Rows object containing the removed elements. + Remove elements matching `f` from these rows. + + Returns a new Rows object containing the removed elements. """ if not self.records: return self.__class__(self, self.model, {}) diff --git a/src/typedal/set_types.py b/src/typedal/set_types.py new file mode 100644 index 0000000..84bce81 --- /dev/null +++ b/src/typedal/set_types.py @@ -0,0 +1,133 @@ +""" +TypeDAL's PyDAL Set subclasses with affected-ID update callbacks. + +Separate from updates.py (the adapter and dialect extensions) so that types.py can re-export these classes: +this module only needs pydal at runtime. +""" + +from __future__ import annotations + +import functools +import operator +import typing as t +from dataclasses import dataclass + +from pydal.objects import Set + +if t.TYPE_CHECKING: + from pydal import DAL + + from .types import Field, OpRow, PrimaryKey, Query, Table, UpsertKeyValue + + +@dataclass +class UpdateResult: + """Rowcount and IDs, collected only when requested or needed by after-hooks.""" + + count: int + ids: list[PrimaryKey] + + +class UpdateAdapter(t.Protocol): + """Typed interface for the adapter's instance-local `update_with_ids` extension (see updates.install_update).""" + + def update_with_ids(self, table: Table, query: Query | None, fields: list[tuple[Field, t.Any]]) -> UpdateResult: + """Update the matching rows and return the rowcount and their primary keys.""" + ... + + +def key_fields(table: Table) -> list[Field]: + """The primary key columns: `id` for normal tables, `primarykey=[...]` for keyed tables.""" + primarykey = getattr(table, "_primarykey", None) + if primarykey: + return [table[name] for name in primarykey] + return [table._id] + + +def key_value(fields: list[Field], values: t.Sequence[UpsertKeyValue]) -> PrimaryKey: + """A scalar for single-column keys (so integer ids stay plain ints), a tuple for composite keys.""" + return values[0] if len(fields) == 1 else tuple(values) + + +def match_fields(pairs: t.Iterable[tuple[Field, t.Any]]) -> Query: + """AND together `field == value` for every pair; there must be at least one.""" + return t.cast("Query", functools.reduce(operator.and_, (field == value for field, value in pairs))) + + +def ids_query(table: Table, ids: list[PrimaryKey]) -> Query: + """Match exactly the rows identified by `ids` (as produced by `key_value`).""" + fields = key_fields(table) + if len(fields) == 1 or not ids: + return fields[0].belongs(ids) + # composite keys (more than one key field) are always tuples, see key_value + matches = [match_fields(zip(fields, t.cast("tuple[UpsertKeyValue, ...]", key), strict=True)) for key in ids] + return t.cast("Query", functools.reduce(operator.or_, matches)) + + +def affected_set(table: Table, ids: list[PrimaryKey]) -> AffectedSet: + """After-hooks must still see rows that no longer match a common filter.""" + return AffectedSet(table._db, ids_query(table, ids), ids) + + +class UpdateSet(Set): + """A real PyDAL Set with affected-ID after-update callback semantics.""" + + def where(self, query: Query | None, ignore_common_filters: bool = False) -> UpdateSet: + """Narrow this set; the result is a plain UpdateSet.""" + if query is None: + return self + rows = super().where(query, ignore_common_filters=ignore_common_filters) + # always a plain UpdateSet: a narrowed AffectedSet no longer matches its affected_ids + return UpdateSet(self.db, rows.query) + + def _update_row(self, fields: dict[str, t.Any]) -> tuple[Table, OpRow]: + """The table this set updates and the validated changes; refuses an update without any.""" + table = self.db._adapter.get_table(self.query) + row = table._fields_and_values_for_update(fields) + if not row.op_values(): + raise ValueError("No fields to update") + return table, row + + def _write(self, table: Table, row: OpRow, run_callbacks: bool, need_ids: bool = False) -> UpdateResult: + """ + Run the update, with PyDAL's before- and after-hooks when `run_callbacks`. + + The affected IDs are only collected when the caller (`need_ids`) or an after-hook needs them. + """ + if run_callbacks and any(hook(self, row) for hook in table._before_update): + return UpdateResult(0, []) + if not need_ids and not (run_callbacks and table._after_update): + count = self.db._adapter.update(table, self.query, row.op_values()) + return UpdateResult(count, []) + adapter = t.cast("UpdateAdapter", self.db._adapter) + result = adapter.update_with_ids(table, self.query, row.op_values()) + if result.ids and run_callbacks: + rows = affected_set(table, result.ids) + for hook in table._after_update: + hook(rows, row) + return result + + def update_ids(self, **fields: t.Any) -> list[PrimaryKey]: + """Update the matching rows, running hooks, and return the primary keys of the updated rows.""" + return self._write(*self._update_row(fields), run_callbacks=True, need_ids=True).ids + + def update(self, **fields: t.Any) -> int: + """Update the matching rows, running hooks, and return the rowcount.""" + return self._write(*self._update_row(fields), run_callbacks=True).count + + def _apply_update(self, table: Table, row: OpRow, run_callbacks: bool) -> int: + """Route PyDAL's validated updates through the same affected-ID callbacks.""" + return self._write(table, row, run_callbacks=run_callbacks).count + + def update_naive(self, **fields: t.Any) -> int: + """Update the matching rows without running any hooks and return the rowcount.""" + return self._write(*self._update_row(fields), run_callbacks=False).count + + +class AffectedSet(UpdateSet): + """An ID-restricted hook Set carrying the IDs already obtained by the write.""" + + def __init__(self, db: DAL, query: Query, affected_ids: list[PrimaryKey]): + """Match the affected rows by key, ignoring common filters (the update may have moved them out).""" + super().__init__(db, query, ignore_common_filters=True) + self.affected_ids = affected_ids diff --git a/src/typedal/tables.py b/src/typedal/tables.py index 9d8f517..7f58e83 100644 --- a/src/typedal/tables.py +++ b/src/typedal/tables.py @@ -23,6 +23,8 @@ from .helpers import all_dict, classproperty, filter_out, throw from .serializers import as_json from .types import ( + AffectedSet, + AnyCallable, AnyDict, Condition, Expression, @@ -40,9 +42,12 @@ T_MetaInstance, T_Query, Table, + UpsertHookPolicy, + UpsertKey, merge_permissions, require_permission, ) +from .upsert import HookRegistration, check_before_hooks, execute_upsert, register_before_hook, validate_upsert if t.TYPE_CHECKING: from .relationships import Relationship @@ -105,6 +110,7 @@ class MyTable(TypedTable): _table: Table | None = None _relationships: dict[str, Relationship[t.Any]] | None = None _permissions: Permissions | None = None + _upsert_hook_registrations: list[HookRegistration] _singular: str _plural: str @@ -126,6 +132,7 @@ def __set_internals__( self._table = table self._relationships = relationships self._permissions = merge_permissions(permissions) + self._upsert_hook_registrations = [] def unbind(self) -> None: """Remove the database bindings created by `db.define`.""" @@ -289,6 +296,38 @@ def update_or_insert( record.update_record(**values) return self(record) + def upsert( + self: t.Type[T_MetaInstance], + key: UpsertKey, + /, + **values: t.Any, + ) -> T_MetaInstance: + """ + Insert or update by a database unique key and return the resulting instance. + + Before-hooks never run; after-hooks run only for the branch performed. + Unmarked before-hooks warn, 'error' policies block, and 'ignore' is silent. + Updates write only explicit values, without omitted Field(update=...) or + compute fields. Inserts apply defaults and computations. A key-only call + returns an existing row without hooks or inserts a new row. + + PostgreSQL uses atomic ON CONFLICT and requires a matching unique index. + SQLite, MySQL, and common-filter tables select then write without atomicity; + their keys must match at most one visible row. Another unique-column + conflict raises the normal database integrity error on every backend. + """ + table = self._ensure_table_defined() + check_before_hooks(table._before_insert, table._before_update, self._upsert_hook_registrations, self.__name__) + return self._upsert(key, **values) + + def _upsert(self: t.Type[T_MetaInstance], key: UpsertKey, /, **values: t.Any) -> T_MetaInstance: + """Validate and execute after the caller has checked its hook policies.""" + table = self._ensure_table_defined() + require_permission(self._permissions, "insert") + require_permission(self._permissions, "update") + key = validate_upsert(table, key, values, self.__name__) + return self(execute_upsert(table, key, values).row) + def validate_and_insert( self: t.Type[T_MetaInstance], **fields: t.Any, @@ -482,6 +521,10 @@ def join( """ return QueryBuilder(self).join(*fields, on=on, condition=condition, method=method, condition_and=condition_and) + def cross_join(self: t.Type[T_MetaInstance], table: t.Type[TypedTable]) -> "QueryBuilder[T_MetaInstance]": + """See QueryBuilder.cross_join!""" + return QueryBuilder(self).cross_join(table) + def window(self: t.Type[T_MetaInstance], window_size: int) -> "QueryBuilder[T_MetaInstance]": """ See QueryBuilder.window! @@ -545,6 +588,17 @@ async def update_or_insert_async( **values, ) + async def upsert_async( + self: t.Type[T_MetaInstance], + key: UpsertKey, + /, + **values: t.Any, + ) -> T_MetaInstance: + """Async twin of upsert(), including its before-hook policies and after-hook contract.""" + table = self._ensure_table_defined() + check_before_hooks(table._before_insert, table._before_update, self._upsert_hook_registrations, self.__name__) + return await run_async(self._ensure_db(), self._upsert, key, **values) # ty: ignore[invalid-return-type] + async def validate_and_insert_async( self: t.Type[T_MetaInstance], **fields: t.Any, @@ -795,6 +849,7 @@ def _hook_once[**P, R, T_MetaInstance: _TypedTable]( cls: t.Type[T_MetaInstance], hooks: list[t.Callable[P, R]], fn: t.Callable[P, R], + register: t.Callable[[t.Callable[P, R]], None] | None = None, ) -> t.Type[T_MetaInstance]: @functools.wraps(fn) def wraps(*a: P.args, **kw: P.kwargs) -> R: @@ -803,28 +858,37 @@ def wraps(*a: P.args, **kw: P.kwargs) -> R: finally: hooks.remove(wraps) - hooks.append(wraps) + (register or hooks.append)(wraps) return cls + def _register_before( + cls, branch: t.Literal["insert", "update"], fn: AnyCallable, upsert: UpsertHookPolicy | None + ) -> None: + """Add a before-hook to PyDAL's list for `branch` and record its upsert policy.""" + hooks = cls._before_insert if branch == "insert" else cls._before_update + register_before_hook(hooks, cls._upsert_hook_registrations, branch, fn, upsert) + def before_insert( cls: t.Type[T_MetaInstance], fn: t.Callable[[T_MetaInstance], t.Optional[bool]] | t.Callable[[OpRow], t.Optional[bool]], + upsert: UpsertHookPolicy | None = None, ) -> t.Type[T_MetaInstance]: """ - Add a before insert hook. + Add a before insert hook; upsert='error' blocks upsert, 'ignore' skips silently. """ - if fn not in cls._before_insert: - cls._before_insert.append(fn) + cls._register_before("insert", fn, upsert) return cls def before_insert_once( cls: t.Type[T_MetaInstance], fn: t.Callable[[T_MetaInstance], t.Optional[bool]] | t.Callable[[OpRow], t.Optional[bool]], + upsert: UpsertHookPolicy | None = None, ) -> t.Type[T_MetaInstance]: """ - Add a before insert hook that only fires once and then removes itself. + Add a before insert hook that only fires once and then removes itself (see before_insert for `upsert`). """ - return cls._hook_once(cls._before_insert, fn) # type: ignore + register = functools.partial(cls._register_before, "insert", upsert=upsert) + return cls._hook_once(cls._before_insert, fn, register) # type: ignore def after_insert( cls: t.Type[T_MetaInstance], @@ -853,29 +917,32 @@ def after_insert_once( def before_update( cls: t.Type[T_MetaInstance], fn: t.Callable[[Set, T_MetaInstance], t.Optional[bool]] | t.Callable[[Set, OpRow], t.Optional[bool]], + upsert: UpsertHookPolicy | None = None, ) -> t.Type[T_MetaInstance]: """ - Add a before update hook. + Add a before update hook; upsert='error' blocks upsert, 'ignore' skips silently. """ - if fn not in cls._before_update: - cls._before_update.append(fn) + cls._register_before("update", fn, upsert) return cls def before_update_once( cls, fn: t.Callable[[Set, T_MetaInstance], t.Optional[bool]] | t.Callable[[Set, OpRow], t.Optional[bool]], + upsert: UpsertHookPolicy | None = None, ) -> t.Type[T_MetaInstance]: """ - Add a before update hook that only fires once and then removes itself. + Add a before update hook that only fires once and then removes itself (see before_update for `upsert`). """ - return cls._hook_once(cls._before_update, fn) # type: ignore + register = functools.partial(cls._register_before, "update", upsert=upsert) + return cls._hook_once(cls._before_update, fn, register) # type: ignore def after_update( cls: t.Type[T_MetaInstance], - fn: t.Callable[[Set, T_MetaInstance], t.Optional[bool]] | t.Callable[[Set, OpRow], t.Optional[bool]], + fn: t.Callable[[AffectedSet, T_MetaInstance], t.Optional[bool]] + | t.Callable[[AffectedSet, OpRow], t.Optional[bool]], ) -> t.Type[T_MetaInstance]: """ - Add an after update hook. + Add an after update hook receiving an AffectedSet with its known affected_ids. """ if fn not in cls._after_update: cls._after_update.append(fn) @@ -883,7 +950,8 @@ def after_update( def after_update_once( cls: t.Type[T_MetaInstance], - fn: t.Callable[[Set, T_MetaInstance], t.Optional[bool]] | t.Callable[[Set, OpRow], t.Optional[bool]], + fn: t.Callable[[AffectedSet, T_MetaInstance], t.Optional[bool]] + | t.Callable[[AffectedSet, OpRow], t.Optional[bool]], ) -> t.Type[T_MetaInstance]: """ Add an after update hook that only fires once and then removes itself. @@ -959,7 +1027,9 @@ class _TypedTable(metaclass=TableMeta): t.Callable[[t.Self, Reference], t.Optional[bool]] | t.Callable[[OpRow, Reference], t.Optional[bool]] ] _before_update: list[t.Callable[[Set, t.Self], t.Optional[bool]] | t.Callable[[Set, OpRow], t.Optional[bool]]] - _after_update: list[t.Callable[[Set, t.Self], t.Optional[bool]] | t.Callable[[Set, OpRow], t.Optional[bool]]] + _after_update: list[ + t.Callable[[AffectedSet, t.Self], t.Optional[bool]] | t.Callable[[AffectedSet, OpRow], t.Optional[bool]] + ] _before_delete: list[t.Callable[[Set], t.Optional[bool]]] _after_delete: list[t.Callable[[Set], t.Optional[bool]]] _rows: tuple[Row, ...] @@ -1081,7 +1151,7 @@ def _collect_model_fields( property_hints = t.get_type_hints(getter, include_extras=True) return_type = property_hints.get("return") - if return_type is not None: + if return_type is not None: # pragma: no branch - unannotated properties are skipped annotations[field_name] = return_type fields = { @@ -1124,7 +1194,7 @@ def _typedal_collect_relationship_fields( for field_name, relationship_value in relationship_items: relationship_type = cls._typedal_resolve_relationship_python_type(relationship_value) - if relationship_type is not None: + if relationship_type is not None: # pragma: no branch - every relationship has a type relationship_fields[field_name] = relationship_type return relationship_fields @@ -1670,7 +1740,7 @@ def render(self, fields: list[Field] | None = None, compact: bool = False) -> t. # else: relationship, different logic: for relation_name in getattr(row, "_with", []): - if relation := self._relationships.get(relation_name): + if relation := self._relationships.get(relation_name): # pragma: no branch - _with holds joined names relation_table = relation.table if isinstance(relation_table, str): relation_table = self._db[relation_table] diff --git a/src/typedal/types.py b/src/typedal/types.py index 449a83c..42c7ec6 100644 --- a/src/typedal/types.py +++ b/src/typedal/types.py @@ -10,6 +10,8 @@ import datetime as dt import types import typing as t +import uuid +from decimal import Decimal # Third-party from pydal.helpers.classes import OpRow as _OpRow @@ -25,6 +27,8 @@ from pydal.objects import Table as _Table from pydal.validators import Validator as _Validator +from .set_types import AffectedSet, UpdateSet # noqa: F401 - re-exported: `from typedal.types import AffectedSet` + try: from string.templatelib import Template as TemplateAlias except ImportError: @@ -41,15 +45,24 @@ # Aliases # --------------------------------------------------------------------------- +# `type` aliases, except Literals (plain, so t.get_args works) and names used at runtime (isinstance, subclassing) Template: t.TypeAlias = TemplateAlias # explicit export for mypy, NOT a `type` because it's used at runtime type AnyCallable = t.Callable[..., t.Any] type AnyDict = dict[str, t.Any] +type UpsertKeyValue = str | int | float | bool | bytes | Decimal | uuid.UUID | dt.date | dt.time +UPSERT_KEY_TYPES: tuple[type, ...] = t.get_args(UpsertKeyValue.__value__) # for isinstance checks +# a row's primary key: its id, the key value for `primarykey=["k"]`, or a tuple of values for composite keys +type PrimaryKey = UpsertKeyValue | tuple[UpsertKeyValue, ...] +type UpsertKey = t.Mapping[str, UpsertKeyValue] +UpsertHookPolicy = t.Literal["error", "ignore"] PermissionType = t.Literal["read", "insert", "update", "delete"] # type Permissions = dict[PermissionType, bool] class Permissions(t.TypedDict): + """Which operations a model or query may perform.""" + # note: extra source of truth because the dynamic dict doesn't work for all type checkers read: bool insert: bool diff --git a/src/typedal/updates.py b/src/typedal/updates.py new file mode 100644 index 0000000..59a8d36 --- /dev/null +++ b/src/typedal/updates.py @@ -0,0 +1,137 @@ +"""Instance-local PyDAL adapter and dialect extensions for affected-ID updates (see set_types.UpdateSet).""" + +from __future__ import annotations + +import types +import typing as t + +from pydal.adapters.base import BaseAdapter, SQLAdapter +from pydal.dialects.base import SQLDialect +from pydal.dialects.postgre import PostgreDialect +from pydal.dialects.sqlite import SQLiteDialect +from pydal.helpers._internals import Dispatcher + +from .set_types import UpdateResult, ids_query, key_fields, key_value +from .types import Field, Query, Row, Rows, Table + +update_dialects = Dispatcher("update returning dialect") + + +class UpdateDialect(t.Protocol): + """Typed interface for the dialect's instance-local UPDATE ... RETURNING extension.""" + + update_returning_supported: bool + + def update_returning(self, sql: str, field: str) -> str: + """Append a RETURNING clause for `field` to an UPDATE statement.""" + ... + + +@update_dialects.register_for(SQLDialect) +class SQLUpdateDialect: + """Dialect without UPDATE ... RETURNING; IDs are selected before updating.""" + + supported = False + + def __init__(self, dialect: SQLDialect): + """Keep the adapter of the dialect this extension is installed on.""" + self.adapter = dialect.adapter + + def update_returning(self, sql: str, field: str) -> str: + """Not available on this dialect.""" + raise NotImplementedError("This dialect does not support UPDATE RETURNING") + + +@update_dialects.register_for(PostgreDialect) +class PostgreUpdateDialect(SQLUpdateDialect): + """PostgreSQL returns the updated rows' keys in the UPDATE itself.""" + + supported = True + + def update_returning(self, sql: str, field: str) -> str: + """Append `RETURNING field` to the UPDATE statement.""" + return sql.rstrip().removesuffix(";") + f" RETURNING {field};" + + +@update_dialects.register_for(SQLiteDialect) +class SQLiteUpdateDialect(PostgreUpdateDialect): + """SQLite has the same RETURNING syntax, from version 3.35 on.""" + + def __init__(self, dialect: SQLDialect): + """Enable RETURNING only when the linked SQLite library supports it.""" + super().__init__(dialect) + self.supported = getattr(self.adapter.driver, "sqlite_version_info", (0,)) >= (3, 35, 0) + + +def _returning_update( + adapter: SQLAdapter, table: Table, query: Query | None, fields: list[tuple[Field, t.Any]], returning: list[Field] +) -> list[t.Any]: + """Run `UPDATE ... RETURNING` and return the raw rows, for dialects with `update_returning_supported`.""" + dialect = t.cast(UpdateDialect, adapter.dialect) + sql = dialect.update_returning( + adapter._update(table, query, fields), ", ".join(field._rname for field in returning) + ) + adapter.execute(sql) + return t.cast(list[t.Any], adapter.fetchall()) + + +def parse_returned(adapter: SQLAdapter, table: Table, returned: list[t.Any]) -> Rows: + """Parse raw rows holding every column of `table` (from a RETURNING clause) into PyDAL Rows.""" + columns = list(table) + colnames = [f"{table._tablename}.{field.name}" for field in columns] + return t.cast(Rows, adapter.parse(returned, columns, colnames)) + + +def update_returning_row( + adapter: SQLAdapter, table: Table, query: Query | None, fields: list[tuple[Field, t.Any]] +) -> Row | None: + """Update and read the row back in one statement; only for dialects with UPDATE ... RETURNING.""" + returned = _returning_update(adapter, table, query, fields, list(table)) + return parse_returned(adapter, table, returned).first() if returned else None + + +def _restrict_to_ids(table: Table, query: Query | None, ids: list[t.Any]) -> Query: + """Narrow `query` to exactly the rows in `ids`, keeping whether it bypasses common filters.""" + if query is None: + return ids_query(table, ids) + restricted = query & ids_query(table, ids) + restricted.ignore_common_filters = query.ignore_common_filters + return restricted + + +def _update_with_ids( + adapter: SQLAdapter, table: Table, query: Query | None, fields: list[tuple[Field, t.Any]] +) -> UpdateResult: + keys = key_fields(table) + if t.cast(UpdateDialect, adapter.dialect).update_returning_supported: + try: + returned = _returning_update(adapter, table, query, fields, keys) + except Exception as error: + # like PyDAL's own adapter.update: a table can handle a failing UPDATE itself + if not hasattr(table, "_on_update_error"): + raise + return UpdateResult(t.cast(t.Callable[..., int], table._on_update_error)(table, query, fields, error), []) + ids = [key_value(keys, row) for row in returned] + return UpdateResult(len(ids), ids) + + # Without RETURNING: lock the matching rows where the backend can (MySQL), then restrict the + # UPDATE to exactly those keys, so rows changed in between are neither updated nor reported. + # (PyDAL's SQLite for_update opens a new transaction and emits invalid FOR UPDATE syntax, so not there.) + # Primary keys must stay unchanged here: returned IDs and after-hooks use the pre-update keys. + for_update = bool(adapter.can_select_for_update) and adapter.dbengine not in {"sqlite", "spatialite"} + rows = adapter.db(query).select(*keys, for_update=for_update) + ids = [key_value(keys, [row[field] for field in keys]) for row in rows] + if not ids: + return UpdateResult(0, []) + count = adapter.update(table, _restrict_to_ids(table, query, ids), fields) + return UpdateResult(count, ids if count else []) + + +def install_update(adapter: BaseAdapter) -> None: + """Extend this adapter and its dialect with `update_with_ids`, without touching PyDAL's global registrations.""" + if not isinstance(adapter.dialect, SQLDialect): + return + extension = update_dialects.get_for(adapter.dialect) + setattr(adapter.dialect, "update_returning_supported", extension.supported) + setattr(adapter.dialect, "update_returning", types.MethodType(type(extension).update_returning, adapter.dialect)) + setattr(adapter, "update_with_ids", types.MethodType(_update_with_ids, adapter)) diff --git a/src/typedal/upsert.py b/src/typedal/upsert.py new file mode 100644 index 0000000..688b12e --- /dev/null +++ b/src/typedal/upsert.py @@ -0,0 +1,432 @@ +"""Instance-local PyDAL adapter and dialect extensions for upsert.""" + +from __future__ import annotations + +import types +import typing as t +import warnings +from dataclasses import dataclass + +from pydal.adapters.base import BaseAdapter, SQLAdapter +from pydal.dialects.base import SQLDialect +from pydal.dialects.postgre import PostgreDialect +from pydal.helpers._internals import Dispatcher +from pydal.helpers.methods import attempt_upload, delete_uploaded_files + +from .exceptions import UpsertAmbiguityError, UpsertHookError, UpsertKeyError +from .set_types import affected_set, match_fields +from .types import UPSERT_KEY_TYPES, AnyCallable, AnyDict, Field, OpRow, Reference, Row, Table, UpsertHookPolicy +from .updates import parse_returned, update_returning_row +from .warnings import UpsertHooksWarning + +upsert_dialects = Dispatcher("upsert dialect") + + +@dataclass +class HookRegistration: + """A model-local policy for one before-hook registration.""" + + branch: t.Literal["insert", "update"] + hook: t.Callable[..., t.Any] + policy: UpsertHookPolicy | None + + +def register_before_hook( + hooks: list[t.Callable[..., t.Any]], + registrations: list[HookRegistration], + branch: t.Literal["insert", "update"], + hook: t.Callable[..., t.Any], + policy: UpsertHookPolicy | None, +) -> None: + """Register the callable with PyDAL and the policy with its TypeDAL model.""" + if policy is not None and policy not in t.get_args(UpsertHookPolicy): + raise ValueError(f"Invalid upsert hook policy: {policy!r}") + if hook not in hooks: + hooks.append(hook) + # Replace this hook's policy while preserving the model's registration list. + registrations[:] = [ + registration + for registration in registrations + if not (registration.branch == branch and registration.hook == hook) + ] + registrations.append(HookRegistration(branch, hook, policy)) + + +def is_pydal_upload_hook(hook: AnyCallable) -> bool: + """Recognize the upload before-hooks PyDAL registers on every table; upsert handles uploads itself.""" + if hook is delete_uploaded_files: + return True + qualname = getattr(hook, "__qualname__", "") + return getattr(hook, "__module__", None) == attempt_upload.__module__ and qualname.startswith( + ("attempt_upload_on_insert.", "attempt_upload_on_update.") + ) + + +def check_before_hooks( + before_insert_hooks: list[t.Callable[..., t.Any]], + before_update_hooks: list[t.Callable[..., t.Any]], + registrations: list[HookRegistration], + table_name: str, +) -> None: + """Enforce policies without running any before-hook.""" + unmarked: list[str] = [] + for branch, hooks in (("insert", before_insert_hooks), ("update", before_update_hooks)): + for hook in hooks: + if is_pydal_upload_hook(hook): + continue + policy = next( + ( + registration.policy + for registration in registrations + if registration.branch == branch and registration.hook == hook + ), + None, + ) + if policy == "error": + hook_name = getattr(hook, "__name__", type(hook).__name__) + raise UpsertHookError( + f"{table_name}.before_{branch} hook {hook_name!r} forbids upsert; " + "use update_or_insert to run before-hooks" + ) + if policy is None: + unmarked.append(f"before_{branch}: {getattr(hook, '__name__', type(hook).__name__)}") + if unmarked: + warnings.warn( + f"{table_name}.upsert skips unmarked before-hooks (" + ", ".join(unmarked) + "). " + "Register them with upsert='ignore' or upsert='error' to choose an explicit policy.", + UpsertHooksWarning, + stacklevel=3, + ) + + +@dataclass +class UpsertResult: + """The written row and the operation data needed by PyDAL after-hooks.""" + + row: Row + outcome: t.Literal["inserted", "updated", "unchanged"] + operation: OpRow + + +class UpsertDialect(t.Protocol): + """Additional dialect methods installed on an existing PyDAL dialect.""" + + upsert_supported: bool + upsert_returning: bool + + def insert(self, table: str, fields: str, values: str) -> str: + """Render PyDAL's own INSERT statement.""" + ... + + def upsert( + self, + table: str, + fields: str, + values: str, + conflict: str, + update: str, + returning: str | None = None, + ) -> str: + """Render the native upsert statement.""" + ... + + +class UpsertAdapter(t.Protocol): + """Typed interface for the adapter's instance-local upsert extension.""" + + dialect: UpsertDialect + + def _upsert( + self, + table: Table, + key_fields: list[Field], + insert_fields: list[tuple[Field, t.Any]], + update_fields: list[Field], + ) -> str: ... + + def upsert( + self, + table: Table, + key_fields: list[Field], + insert_fields: list[tuple[Field, t.Any]], + update_fields: list[Field], + ) -> UpsertResult: + """Execute the native upsert and return the written row.""" + ... + + +@upsert_dialects.register_for(SQLDialect) +class SQLUpsertDialect: + """Dialect without native upsert; execution uses the Python implementation.""" + + native = False + returning = False + + def __init__(self, dialect: SQLDialect): + """Keep the adapter of the dialect this extension is installed on.""" + self.adapter = dialect.adapter + + def upsert( + self, + table: str, + fields: str, + values: str, + conflict: str, + update: str, + returning: str | None = None, + ) -> str: + """Render native upsert SQL where the dialect supports it.""" + raise NotImplementedError("This dialect does not support native upsert") + + +@upsert_dialects.register_for(PostgreDialect) +class PostgreUpsertDialect(SQLUpsertDialect): + """PostgreSQL conflict handling returns the branch as execution metadata.""" + + native = True + returning = True + + def upsert( + self, + table: str, + fields: str, + values: str, + conflict: str, + update: str, + returning: str | None = None, + ) -> str: + """Render INSERT using the existing dialect's quoting and INSERT syntax.""" + dialect = t.cast(UpsertDialect, self) + sql = dialect.insert(table, fields, values).rstrip().removesuffix(";") + action = f"DO UPDATE SET {update}" if update else "DO NOTHING" + sql += f" ON CONFLICT ({conflict}) {action}" + if returning: # pragma: no branch - the adapter always asks Postgres for the row + sql += f" RETURNING {returning}, (xmax = 0) AS inserted" + return sql + ";" + + +def validate_upsert(table: Table, key: t.Any, values: AnyDict, model_name: str) -> AnyDict: + """Reject an invalid key or values before any SQL runs; returns the key as a dict.""" + if not isinstance(key, t.Mapping) or not key: + raise UpsertKeyError("upsert requires a nonempty key mapping") + key = dict(key) + id_names = {"id", table._id.name} + if id_names & key.keys(): + raise UpsertKeyError("upsert key must not contain id") + if id_names & values.keys(): + raise UpsertKeyError("upsert values must not contain id; it would re-key the existing row") + if any(value is None for value in key.values()): + raise UpsertKeyError("upsert key values must not be None") + if invalid := sorted(name for name, value in key.items() if not isinstance(value, UPSERT_KEY_TYPES)): + raise UpsertKeyError(f"upsert key values must be plain scalars; invalid for: {', '.join(invalid)}") + if overlap := key.keys() & values.keys(): + # field names only: values may come from request data and should not end up in logs + lookup = ", ".join(f"{name!r}: ..." for name in key) + kwargs = ", ".join(f"{name}=..." for name in key | values) + raise UpsertKeyError( + f"upsert key fields must not also be supplied as values ({', '.join(sorted(overlap))}). " + f"To change a key field, use {model_name}.update_or_insert({{{lookup}}}, {kwargs})." + ) + if unknown := (key.keys() | values.keys()) - set(table.fields): + raise UpsertKeyError(f"Unknown upsert fields: {', '.join(sorted(unknown))}") + return key + + +def _execute_native(adapter: SQLAdapter, sql: str) -> None: + try: + adapter.execute(sql) + except Exception as error: + if getattr(error, "sqlstate", None) == "42P10" or getattr(error, "pgcode", None) == "42P10": + raise UpsertKeyError("upsert key must match a database unique constraint or unique index") from error + raise + + +def _require_row(record: Row | None) -> Row: + """The row a write just produced; a concurrent delete (or a trigger) can make it disappear before it's read.""" + if record is None: + raise RuntimeError("The upserted row could not be retrieved; it may have been deleted concurrently") + return record + + +def _lookup(table: Table, key: AnyDict) -> Row | None: + rows = table._db(match_fields((table[name], value) for name, value in key.items())).select( + table.ALL, limitby=(0, 2) + ) + if len(rows) > 1: + raise UpsertAmbiguityError("upsert key matches multiple rows; use a unique key") + return t.cast(Row | None, rows.first()) + + +def _insert_operation(table: Table, key: AnyDict, values: AnyDict) -> OpRow: + """Apply insert defaults and computations while reusing already converted keys.""" + filtered = table._filter_fields_for_operation(values) + empty, fields = t.cast(tuple[list[str], dict[str, tuple[Field, t.Any]]], filtered) + fields.update({name: (table[name], value) for name, value in key.items()}) + to_compute = [] + for name in empty: + if name in key: + continue + field = table[name] + if field.compute: + to_compute.append((name, field)) + elif field.default is not None: + fields[name] = (field, field.default) + elif field.required: + raise RuntimeError("Table: missing required field: %s" % name) + return table._compute_fields_for_operation(fields, to_compute) + + +def _autodelete_upload_fields(table: Table, values: AnyDict) -> list[str]: + """Upload fields in `values` whose replaced file PyDAL would delete on a normal update.""" + if delete_uploaded_files not in table._before_update: + return [] + return [ + name + for name in values + if table[name].type == "upload" and table[name].uploadfield is True and table[name].autodelete + ] + + +def _is_filtered(table: Table) -> bool: + """Common filters and multi-tenant defaults restrict which rows a write may touch.""" + tenant = table._db._request_tenant + return bool(table._common_filter) or (tenant in table.fields and table[tenant].default is not None) + + +def _prepare(table: Table, key: AnyDict, values: AnyDict) -> tuple[AnyDict, AnyDict]: + """Convert the key once (so lookup and insert agree) and store uploaded files, like PyDAL's upload hooks.""" + _, key_fields = table._filter_fields_for_operation(key) + key_operation = table._compute_fields_for_operation(key_fields, []) + key = {name: key_operation[name] for name in key} + if t.cast(set[str], table._upload_fieldnames) & values.keys(): + # same conversion PyDAL's attempt_upload_on_insert/update hooks do (store file objects, keep names) + values = dict(values) + attempt_upload(table, values) + return key, values + + +def _write_native(table: Table, key: AnyDict, values: AnyDict) -> UpsertResult: + operation = _insert_operation(table, key, values) + return t.cast(UpsertAdapter, table._db._adapter).upsert( + table, [table[name] for name in key], operation.op_values(), [table[name] for name in values] + ) + + +def _write_insert(table: Table, key: AnyDict, values: AnyDict) -> UpsertResult: + operation = _insert_operation(table, key, values) + row_id = table._db._adapter.insert(table, operation.op_values()) + record = affected_set(table, [int(row_id)]).select(table.ALL).first() + return UpsertResult(_require_row(record), "inserted", operation) + + +def _write_update(table: Table, existing: Row, values: AnyDict, autodelete: list[str]) -> UpsertResult: + adapter = table._db._adapter + _, fields = table._filter_fields_for_operation(values) + operation = table._compute_fields_for_operation(fields, []) + rows = affected_set(table, [int(existing.id)]) + if autodelete: + delete_uploaded_files(rows, {name: values[name] for name in autodelete}) + if getattr(adapter.dialect, "update_returning_supported", False): # Postgres, SQLite 3.35+ + record = update_returning_row(adapter, table, rows.query, operation.op_values()) + else: + adapter.update(table, rows.query, operation.op_values()) + record = rows.select(table.ALL).first() + return UpsertResult(_require_row(record), "updated", operation) + + +def _run_after_hooks(table: Table, result: UpsertResult) -> None: + """PyDAL's after-hooks for the branch that was actually taken; 'unchanged' runs none.""" + row_id = Reference(int(result.row.id)) + row_id._table = table + if result.outcome == "inserted": + for after_insert_hook in table._after_insert: + after_insert_hook(result.operation, row_id) + elif result.outcome == "updated": # pragma: no branch - only a race makes a native upsert 'unchanged' + rows = affected_set(table, [int(row_id)]) + for after_update_hook in table._after_update: + after_update_hook(rows, result.operation) + + +def execute_upsert(table: Table, key: AnyDict, values: AnyDict) -> UpsertResult: + """Write without PyDAL callbacks, then run after-hooks for the actual branch.""" + key, values = _prepare(table, key, values) + autodelete = _autodelete_upload_fields(table, values) + # ON CONFLICT bypasses common filters and tenant defaults, and can't remove a replaced upload first, + # so those cases take the select-then-write path (which applies the filters to its lookup): + native = getattr(table._db._adapter.dialect, "upsert_supported", False) and not _is_filtered(table) + native = native and not autodelete + # a key-only call returns an existing row as-is, so look it up even when the write would be native: + existing = None if native and values else _lookup(table, key) + if existing is not None and not values: + return UpsertResult(existing, "unchanged", OpRow(table)) + + if native: + result = _write_native(table, key, values) + elif existing is None: + result = _write_insert(table, key, values) + else: + result = _write_update(table, existing, values, autodelete) + _run_after_hooks(table, result) + return result + + +def _adapter_upsert_sql( + adapter: SQLAdapter, + table: Table, + key_fields: list[Field], + insert_fields: list[tuple[Field, t.Any]], + update_fields: list[Field], +) -> str: + """Adapter._upsert: expand typed values and delegate SQL syntax to its dialect.""" + columns = ", ".join(field._rname for field, _ in insert_fields) + values = ", ".join(adapter.expand(value, field.type) for field, value in insert_fields) + conflict = ", ".join(field._rname for field in key_fields) + update = ", ".join(f"{field._rname} = EXCLUDED.{field._rname}" for field in update_fields) + dialect = t.cast(UpsertDialect, adapter.dialect) + returning = ", ".join(field._rname for field in table) if dialect.upsert_returning else None + return dialect.upsert(table._rname, columns, values, conflict, update, returning=returning) + + +def _adapter_upsert( + adapter: SQLAdapter, + table: Table, + key_fields: list[Field], + insert_fields: list[tuple[Field, t.Any]], + update_fields: list[Field], +) -> UpsertResult: + """Adapter.upsert: execute on the current connection and decode the resulting row.""" + upsert_adapter = t.cast(UpsertAdapter, adapter) + _execute_native(adapter, upsert_adapter._upsert(table, key_fields, insert_fields, update_fields)) + record = None + outcome: t.Literal["inserted", "updated", "unchanged"] = "unchanged" + if upsert_adapter.dialect.upsert_returning: # pragma: no branch - the only native dialect returns rows + returned = adapter.fetchall() + if returned: + outcome = "inserted" if returned[0][-1] else "updated" + record = parse_returned(adapter, table, [row[:-1] for row in returned]).first() + if record is None: + key_names = {field.name for field in key_fields} + # Reuse converted values instead of evaluating filter_in or callable values twice. + query = match_fields((field, value) for field, value in insert_fields if field.name in key_names) + record = table._db(query, ignore_common_filters=True).select(table.ALL, limitby=(0, 1)).first() + record = _require_row(record) + names = {field.name for field in update_fields} + operation = OpRow(table) + for field, value in insert_fields: + if outcome == "inserted" or field.name in names: + operation.set_value(field.name, value, field) + return UpsertResult(record, outcome, operation) + + +def install_upsert(adapter: BaseAdapter) -> None: + """Extend this adapter and dialect without modifying PyDAL's global registrations.""" + if hasattr(adapter, "upsert") or not isinstance(adapter.dialect, SQLDialect): + return + extension = upsert_dialects.get_for(adapter.dialect) + render_upsert = type(extension).upsert + # Bind the extension function to the initialized dialect, preserving its INSERT behavior. + setattr(adapter.dialect, "upsert", types.MethodType(render_upsert, adapter.dialect)) + setattr(adapter.dialect, "upsert_supported", extension.native) + setattr(adapter.dialect, "upsert_returning", extension.returning) + setattr(adapter, "_upsert", types.MethodType(_adapter_upsert_sql, adapter)) + setattr(adapter, "upsert", types.MethodType(_adapter_upsert, adapter)) diff --git a/src/typedal/warnings.py b/src/typedal/warnings.py index 694d220..68e4b25 100644 --- a/src/typedal/warnings.py +++ b/src/typedal/warnings.py @@ -7,3 +7,7 @@ class UnusedWindowWarning(RuntimeWarning): class NoopQueryWarning(RuntimeWarning): """Warn when a query builder method is called without arguments, so it does nothing.""" + + +class UpsertHooksWarning(RuntimeWarning): + """Warn that upsert skips before-hooks without an explicit upsert policy.""" diff --git a/tests/test_async.py b/tests/test_async.py index b681049..a3d7127 100644 --- a/tests/test_async.py +++ b/tests/test_async.py @@ -1935,3 +1935,23 @@ class AsyncGuardStrict(TypedTable): AsyncGuardStrict.first() assert not _blocking(records) + + +@pytest.mark.asyncio +async def test_worker_shutdown_closes_its_connection_without_a_pool(tmp_path: Path): + """ + Without a pydal pool (pool_size=0, the default), a worker must really close its connection on shutdown. + + `close(really=False)` used to drop the connection unclosed in that case: the thread went away, but the + connection stayed open (and on Postgres, kept a server slot) until garbage collection got to it. + """ + db = TypeDAL(f"sqlite://{tmp_path / 'worker.db'}", folder=str(tmp_path), enable_typedal_caching=False) + try: + worker = ConnectionWorker(db, "close-test") + connection = await worker.run(lambda: (db.executesql("SELECT 1;"), db._adapter.connection)[1]) + worker.shutdown() + + with pytest.raises(Exception, match="closed"): + connection.execute("SELECT 1;") + finally: + db.close() diff --git a/tests/test_caching_multi_db.py b/tests/test_caching_multi_db.py new file mode 100644 index 0000000..f56049e --- /dev/null +++ b/tests/test_caching_multi_db.py @@ -0,0 +1,147 @@ +""" +Two caching-enabled databases in one process must each keep their own cache (issue #21). +""" + +import contextlib + +import pytest + +from src.typedal import TypeDAL, TypedTable +from src.typedal.caching import _TypedalCache, cache_models, clear_cache, remove_cache_for_table + + +# module level, because the cache stores rows with `dill`: +class MultiDbAppItem(TypedTable): + name: str + + +class MultiDbReportItem(TypedTable): + name: str + + +@pytest.fixture +def two_dbs(tmp_path): + app = TypeDAL(f"sqlite://{tmp_path / 'app.db'}", folder=str(tmp_path)) + report = TypeDAL(f"sqlite://{tmp_path / 'report.db'}", folder=str(tmp_path)) + app.define(MultiDbAppItem) + report.define(MultiDbReportItem) + yield app, report + report.close() + app.close() + + +def test_cache_models_are_per_database(two_dbs): + app, report = two_dbs + app_cache, app_dependency = cache_models(app) + report_cache, report_dependency = cache_models(report) + + assert app_cache is not report_cache + assert app_dependency is not report_dependency + assert app_cache._db is app + assert report_cache._db is report + # the module-level base class is never bound by enabling caching: + assert _TypedalCache._db is None + assert str(app_cache._table) == str(report_cache._table) == "typedal_cache" + + +def test_cache_dependency_join_uses_its_database_cache(two_dbs): + app, report = two_dbs + MultiDbAppItem.insert(name="a") + MultiDbReportItem.insert(name="r") + MultiDbAppItem.cache().collect() + MultiDbReportItem.cache().collect() + + for db in (app, report): + cache, dependency = cache_models(db) + entry = cache.collect().first() + rows = dependency.join("entry").collect() + + assert len(rows) == 1 + joined_entry = rows.first().entry + assert isinstance(joined_entry, cache) + assert joined_entry._db is db + assert joined_entry.id == entry.id + assert joined_entry.key == entry.key + assert joined_entry.data == entry.data + + +def test_cache_rows_land_in_the_queried_database(two_dbs): + app, report = two_dbs + app_cache, _ = cache_models(app) + report_cache, _ = cache_models(report) + + MultiDbAppItem.insert(name="a") + assert MultiDbAppItem.cache().collect().metadata["cache"]["status"] == "fresh" + assert MultiDbAppItem.cache().collect().metadata["cache"]["status"] == "cached" + + assert app_cache.count() == 1 + assert report_cache.count() == 0 + + # invalidation through the insert hook touches only the app database: + MultiDbReportItem.insert(name="r") + MultiDbReportItem.cache().collect() + assert report_cache.count() == 1 + + MultiDbAppItem.insert(name="b") + assert app_cache.count() == 0 + assert report_cache.count() == 1 + + clear_cache(report) + assert report_cache.count() == 0 + + +def test_closing_one_database_keeps_caching_working_on_another(two_dbs, tmp_path): + app, _report = two_dbs + + other = TypeDAL(f"sqlite://{tmp_path / 'other.db'}", folder=str(tmp_path)) + other.close() + + # used to fail in the after-insert hook: "@define or db.define is not called on this class yet!" + MultiDbAppItem.insert(name="after close") + MultiDbAppItem.cache().collect() + assert cache_models(app)[0].count() == 1 + + +def test_cache_models_without_caching(tmp_path): + db = TypeDAL("sqlite:memory", enable_typedal_caching=False, folder=str(tmp_path)) + try: + with pytest.raises(RuntimeError): + cache_models(db) + finally: + db.close() + + +def test_remove_cache_for_model_keeps_other_database_cache(two_dbs): + app, report = two_dbs + MultiDbAppItem.insert(name="a") + MultiDbReportItem.insert(name="r") + MultiDbAppItem.cache().collect() + MultiDbReportItem.cache().collect() + assert cache_models(app)[0].count() == 1 + assert cache_models(report)[0].count() == 1 + + remove_cache_for_table(app, MultiDbAppItem) + + assert cache_models(app)[0].count() == 0 + assert MultiDbAppItem.cache().collect().metadata["cache"]["status"] == "fresh" + assert cache_models(report)[0].count() == 1 + assert MultiDbReportItem.cache().collect().metadata["cache"]["status"] == "cached" + + +def test_closing_old_database_preserves_rebound_model(tmp_path): + class SharedItem(TypedTable): + name: str + + with contextlib.closing(TypeDAL("sqlite:memory", folder=str(tmp_path))) as report: + with contextlib.closing(TypeDAL("sqlite:memory", folder=str(tmp_path))) as app: + app.define(SharedItem) + report.define(SharedItem) + assert SharedItem._db is report + + assert SharedItem._db is report + assert SharedItem._table is report.shared_item + SharedItem.insert(name="after close") + assert SharedItem.collect().first().name == "after close" + + assert SharedItem._db is None + assert SharedItem._table is None diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py new file mode 100644 index 0000000..8a17d87 --- /dev/null +++ b/tests/test_edge_cases.py @@ -0,0 +1,218 @@ +""" +Small behaviours that are easy to miss: mostly the 'other' side of a condition, kept here so branch coverage stays honest. +""" + +import contextlib +import typing as t +import warnings +from decimal import Decimal +from pathlib import Path + +import pytest +from pydal import Field +from pydal.validators import IS_NOT_EMPTY + +from src.typedal import TypeDAL, TypedField, TypedTable, relationship +from src.typedal.extensions import _coerce_decimal, install_decimal_guard +from src.typedal.helpers import all_annotations, sql_expression +from src.typedal.mixins import HAS_UNIQUE_SLUG, SlugMixin +from src.typedal.updates import SQLUpdateDialect + + +class EdgeAuthor(TypedTable): + name: str + books = relationship(list["EdgeBook"], lambda author, book: author.id == book.author, lazy="allow") + + +class EdgeBook(TypedTable): + author: EdgeAuthor + title: str + + +class EdgeArticle(TypedTable, SlugMixin, slug_field="title"): + title = TypedField(str, requires=[IS_NOT_EMPTY()]) + + +class EdgeArticleCopy(TypedTable): + title: str + + +@pytest.fixture +def db() -> t.Iterator[TypeDAL]: + database = TypeDAL("sqlite:memory", enable_typedal_caching=False) + database.define(EdgeAuthor) + database.define(EdgeBook) + database.define(EdgeArticle) + try: + yield database + finally: + database.close() + + +def test_folder_is_optional() -> None: + with contextlib.closing(TypeDAL("sqlite:memory", folder="", enable_typedal_caching=False)) as database: + assert database._adapter is not None + + +def test_all_annotations_can_exclude_names() -> None: + annotations = all_annotations(EdgeBook, _except={"title"}) + assert "author" in annotations + assert "title" not in annotations + + +def test_sql_expression_renders_none_as_null(db: TypeDAL) -> None: + assert "NULL" in str(sql_expression(db, "title IS %s", None)) + + +@pytest.mark.parametrize("value", [None, 1, 1.25, Decimal("2.50"), " 2.50 "]) +def test_decimal_coercion_preserves_null_and_finite_values(value: t.Any) -> None: + expected = None if value is None else Decimal(str(value).strip()) + assert _coerce_decimal(value) == expected + + +@pytest.mark.parametrize( + "value", + [ + Decimal("NaN"), + Decimal("sNaN"), + Decimal("Infinity"), + Decimal("-Infinity"), + float("nan"), + float("inf"), + float("-inf"), + "NaN", + "Infinity", + "invalid", + ], +) +def test_decimal_coercion_rejects_nonfinite_and_invalid_values(value: t.Any) -> None: + with pytest.raises(ValueError, match="Invalid decimal"): + _coerce_decimal(value) + + +def test_decimal_coercion_passes_empty_values_to_pydal(db: TypeDAL) -> None: + assert _coerce_decimal(None) is None + assert _coerce_decimal("") == "" + + # PyDAL renders "" as NULL for decimal fields, as it did before the guard existed: + table = db.define_table("decimal_empty", Field("price", "decimal(10,2)")) + row_id = table.insert(price="") + assert table[row_id].price is None + assert db(table.price == "").count() == 0 + + +def test_decimal_guard_is_installed_once_and_skips_adapters_without_representer(db: TypeDAL) -> None: + representer = db._adapter.representer + guarded = representer.represent + install_decimal_guard(db._adapter) + assert representer.represent is guarded + + class NoRepresenter: + pass + + install_decimal_guard(NoRepresenter()) # nothing to patch, no error + + +def test_unsupported_update_returning_dialect_raises(db: TypeDAL) -> None: + with pytest.raises(NotImplementedError, match="UPDATE RETURNING"): + SQLUpdateDialect(db._adapter.dialect).update_returning("UPDATE x SET y = 1;", "id") + + +def test_where_ignores_lambda_returning_nothing(db: TypeDAL) -> None: + EdgeAuthor.insert(name="a") + EdgeAuthor.insert(name="b") + assert EdgeAuthor.where(lambda _: None).count() == 2 + + +def test_bare_table_in_join_counts_as_explicit(db: TypeDAL) -> None: + sql = EdgeAuthor.select(EdgeAuthor.name, EdgeBook.title, join=db.edge_book).to_sql() + assert "edge_book" in sql + + +def test_collect_into_keeps_explicit_selection(db: TypeDAL) -> None: + EdgeArticle.insert(title="Hello") + rows = EdgeArticle.select(EdgeArticle.id, EdgeArticle.title).collect_into(EdgeArticleCopy) + assert [row.title for row in rows] == ["Hello"] + + +class EdgeAsyncRow(TypedTable): + name: str + + +@pytest.mark.asyncio +async def test_async_iteration_without_window_yields_all_rows(tmp_path: Path) -> None: + # a file database: async workers use their own connection, which sqlite:memory can't share + database = TypeDAL(f"sqlite://{tmp_path / 'async.db'}", folder=str(tmp_path), enable_typedal_caching=False) + try: + database.define(EdgeAsyncRow) + EdgeAsyncRow.insert(name="a") + EdgeAsyncRow.insert(name="b") + database.commit() + builder = EdgeAsyncRow.where(EdgeAsyncRow.id > 0).orderby(EdgeAsyncRow.id) + assert [row.name async for row in builder] == ["a", "b"] + finally: + database.close() + + +def test_find_with_limitby_skips_leading_matches(db: TypeDAL) -> None: + for name in "abc": + EdgeAuthor.insert(name=name) + rows = EdgeAuthor.collect() + assert [row.name for row in rows.find(lambda _: True, limitby=(1, 3))] == ["b", "c"] + + +def test_hooks_are_registered_once(db: TypeDAL) -> None: + def hook(*_args: t.Any) -> None: ... + + for register, hooks in [ + (EdgeBook.after_update, EdgeBook._after_update), + (EdgeBook.before_delete, EdgeBook._before_delete), + (EdgeBook.after_delete, EdgeBook._after_delete), + ]: + register(hook) + register(hook) + assert hooks.count(hook) == 1 + hooks.remove(hook) + + +def test_render_with_explicit_fields_skips_other_tables(db: TypeDAL) -> None: + author = EdgeAuthor.insert(name="a") + rendered = author.render(fields=[db.edge_author.name, db.edge_book.title]) + assert rendered.id == author.id + assert rendered is not author + + +def test_lazy_allow_loads_without_warning(db: TypeDAL) -> None: + author = EdgeAuthor.insert(name="a") + EdgeBook.insert(author=author, title="t") + with warnings.catch_warnings(): + warnings.simplefilter("error") + books = EdgeAuthor(author.id).books + assert [book.title for book in books] == ["t"] + + +def test_slug_validator_without_record_id(db: TypeDAL) -> None: + validator = HAS_UNIQUE_SLUG(db, "edge_article.slug") + assert validator.validate("Brand new") == "Brand new" + + +def test_slug_validator_excludes_the_record_being_edited(db: TypeDAL) -> None: + from pydal.validators import ValidationError + + article = EdgeArticle.insert(title="Taken") + validator = HAS_UNIQUE_SLUG(db, "edge_article.slug") + with pytest.raises(ValidationError): + validator.validate("Taken") + assert validator.validate("Taken", record_id=article.id) == "Taken" + + +def test_slug_mixin_keeps_existing_requires_list(db: TypeDAL) -> None: + requires = db.edge_article.title.requires + assert isinstance(requires, list) + assert any(isinstance(validator, HAS_UNIQUE_SLUG) for validator in requires) + + +def test_from_slug_without_join(db: TypeDAL) -> None: + article = EdgeArticle.insert(title="Hello World") + assert EdgeArticle.from_slug("hello-world", join=False).id == article.id + assert EdgeArticle.from_slug_or_fail("hello-world", join=False).id == article.id diff --git a/tests/test_join_aliases.py b/tests/test_join_aliases.py new file mode 100644 index 0000000..a599563 --- /dev/null +++ b/tests/test_join_aliases.py @@ -0,0 +1,388 @@ +""" +Joined relationships live under an alias; where() and orderby() on the joined table resolve to it when the builder runs. +""" + +import pytest + +from src.typedal import QueryBuilder, TypeDAL, TypedTable, relationship +from src.typedal.exceptions import AliasedTableMismatchError + + +def define_models(db: TypeDAL): + @db.define() + class AliasAuthor(TypedTable): + name: str + + articles = relationship( + list["AliasArticle"], + condition=lambda author, article: author.id == article.author, + join="left", + ) + + @db.define() + class AliasArticle(TypedTable): + title: str + author: AliasAuthor + reviewer: AliasAuthor + + writer = relationship(AliasAuthor, condition=lambda article, author: article.author == author.id, join="left") + critic = relationship(AliasAuthor, condition=lambda article, author: article.reviewer == author.id, join="left") + comments = relationship( + list["AliasComment"], + condition=lambda article, comment: article.id == comment.article, + join="left", + ) + # `on` joins the table itself, without an alias: + replies = relationship( + list["AliasComment"], + on=lambda article, comment: [comment.on(comment.article == article.id)], + ) + + @db.define() + class AliasComment(TypedTable): + article: AliasArticle + body: str + + post = relationship(AliasArticle, condition=lambda comment, article: comment.article == article.id) + + ann, bob, cat = AliasAuthor.bulk_insert([{"name": "ann"}, {"name": "bob"}, {"name": "cat"}]) + first = AliasArticle.insert(title="First", author=ann, reviewer=bob) + AliasArticle.insert(title="Second", author=bob, reviewer=ann) + third = AliasArticle.insert(title="Third", author=ann, reviewer=cat) + AliasComment.bulk_insert( + [ + {"article": first, "body": "nice"}, + {"article": first, "body": "meh"}, + {"article": third, "body": "nice"}, + ] + ) + db.commit() + + return AliasAuthor, AliasArticle, AliasComment + + +db = TypeDAL("sqlite:memory") +Author, Article, Comment = define_models(db) + + +def titles(builder) -> list[str]: + return [row.title for row in builder] + + +def test_filter_on_left_join_paginates_and_counts(): + builder = Article.join("comments").where(Comment.body == "nice").orderby(Article.id) + + rows = builder.collect() + assert titles(rows) == ["First", "Third"] + # the filter applies to the joined rows too, like condition_and does: + assert [[comment.body for comment in row.comments] for row in rows] == [["nice"], ["nice"]] + + # with limitby, the predicate moves into an id subquery, which needs the left join as well: + for page_number, expected_title in enumerate(["First", "Third"], start=1): + page = builder.paginate(limit=1, page=page_number) + assert titles(page) == [expected_title] + assert (page.pagination["total_items"], page.pagination["total_pages"]) == (2, 2) + assert [comment.body for comment in page.first().comments] == ["nice"] + + assert builder.count() == 2 + assert builder.count(distinct=Article.id) == 2 + assert "LEFT JOIN" in builder._count() + + with_inner = Article.join("writer", method="inner").join("comments").where(Comment.body == "nice") + assert with_inner.count() == 2 + assert with_inner.paginate(limit=1).pagination["total_items"] == 2 + + +def test_count_distinct_joined_field(): + # "Second" has no comments, so its left-joined row is NULL, which isn't a distinct value: + assert Article.join("comments").count(distinct=Comment.body) == 2 + assert Article.join("comments").where(Article.title != "Third").count(distinct=Comment.body) == 2 + + +def test_required_left_joins_ignores_expressions_without_table_targets(): + """Ignore non-table targets while retaining transitive join dependencies.""" + writer_join = Author.on(Author.id == Article.author) + article_join = Article.on(Article.author == Author.id) + comment_join = Comment.on(Comment.article == Article.id) + non_table_expression = Author.name == "ann" + + assert QueryBuilder._required_left_joins( + {Comment._tablename}, + [non_table_expression, writer_join, article_join, comment_join], + ) == [writer_join, article_join, comment_join] + assert QueryBuilder._required_left_joins(set(), [non_table_expression, comment_join]) == [] + + +def test_count_distinct_roots_applies_left_join_condition_and(): + """Distinct root counts include only comments matching the additional join condition.""" + builder = Article.join("comments", condition_and=lambda _article, comment: comment.body == "meh") + + rows = builder.orderby(Article.id).collect() + assert titles(rows) == ["First", "Second", "Third"] + assert [[comment.body for comment in row.comments] for row in rows] == [["meh"], [], []] + assert builder.count(distinct=True) == 1 + + +@pytest.mark.parametrize("method", ["left", "inner"]) +def test_orderby_on_joined_table_resolves(method): + builder = Article.join("writer", method=method).orderby(~Author.name, Article.id) + + assert titles(builder) == ["Second", "First", "Third"] + assert [titles(builder.paginate(limit=1, page=page)) for page in range(1, 4)] == [["Second"], ["First"], ["Third"]] + + +@pytest.mark.parametrize("method", ["left", "inner"]) +def test_nested_relationship_resolves(method): + builder = Author.join("articles.comments", method=method).where(Comment.body == "meh") + + rows = builder.collect() + assert [row.name for row in rows] == ["ann"] + assert [article.title for article in rows.first().articles] == ["First"] + assert [comment.body for comment in rows.first().articles[0].comments] == ["meh"] + assert builder.count() == 1 + page = builder.paginate(limit=1) + assert [row.name for row in page] == ["ann"] + assert (page.pagination["total_items"], page.pagination["total_pages"]) == (1, 1) + + by_path = Author.join("articles.comments", method=method).where( + lambda author, articles__comments: articles__comments.body == "meh" + ) + by_name = Author.join("articles.comments", method=method).where(lambda author, comments: comments.body == "meh") + assert [row.name for row in by_path] == [row.name for row in by_name] == ["ann"] + + +def test_custom_on_intermediate_join_filter_counts_and_paginates(): + db = TypeDAL("sqlite:memory") + + try: + @db.define() + class Parent(TypedTable): + name: str + + children = relationship( + list["Child"], + on=lambda parent, child: [ + ParentChild.on(ParentChild.parent == parent.id), + child.on(child.id == ParentChild.child), + ], + join="left", + ) + + @db.define() + class Child(TypedTable): + name: str + + @db.define() + class ParentChild(TypedTable): + parent: Parent + child: Child + + Parent.insert(name="unlinked") + linked = Parent.insert(name="linked") + child = Child.insert(name="matching") + ParentChild.insert(parent=linked, child=child) + + builder = Parent.join("children").where(Child.name == "matching").orderby(Parent.id) + rows = builder.collect() + assert [row.id for row in rows] == [linked.id] + assert [child.id for child in rows.first().children] == [child.id] + + assert builder.count() == 1 + assert builder.count(distinct=Parent.id) == 1 + page = builder.paginate(limit=1) + assert [row.id for row in page] == [linked.id] + assert (page.pagination["total_items"], page.pagination["total_pages"]) == (1, 1) + finally: + db.close() + + +def test_paginated_join_deduplicates_root_ids_before_limit(): + builder = Article.join("comments").where(Comment.id > 0).orderby(Comment.id) + + assert titles(builder.collect()) == ["First", "Third"] + assert builder.count(distinct=Article.id) == 2 + + first_page = builder.paginate(limit=1) + second_page = builder.paginate(limit=1, page=2) + + assert (first_page.pagination["total_items"], first_page.pagination["total_pages"]) == (2, 2) + assert titles(first_page) == ["First"] + assert titles(second_page) == ["Third"] + + +def test_filter_for_missing_left_join_rows(): + builder = Article.join("comments").where(Comment.id == None) + + assert titles(builder) == ["Second"] + assert builder.paginate(limit=1).pagination["total_items"] == 1 + assert builder.count() == 1 + + +def test_expression_on_joined_table_resolves(): + builder = Article.join("comments", method="inner").where(Comment.body.upper() == "MEH") + + assert titles(builder) == ["First"] + assert builder.count() == 1 + + +def test_same_table_joined_twice_needs_a_lambda(): + builder = Article.join("writer").join("critic") + + with pytest.raises(AliasedTableMismatchError, match="multiple aliases.*'writer', 'critic'"): + builder.where(Author.name == "ann").to_sql() + + assert titles(builder.where(lambda article, critic: critic.name == "ann")) == ["Second"] + assert titles(builder.where(lambda article, writer: writer.name == "ann").orderby(Article.id)) == [ + "First", + "Third", + ] + # the lambda is resolved when the builder runs, so it can come before the joins: + deferred = Article.where(lambda article, critic: critic.name == "ann").join("writer").join("critic") + assert titles(deferred) == ["Second"] + assert deferred.count() == 1 + assert deferred.paginate(limit=1).pagination["total_items"] == 1 + + +def test_joined_table_compared_with_other_table_needs_a_lambda(): + builder = Article.join("critic") + + # in v5 this plain Author was an independent copy, rewriting it to the `critic` alias would change the result: + with pytest.raises(AliasedTableMismatchError, match="'alias_author' is joined"): + builder.where(Article.author == Author.id).to_sql() + + assert titles(builder.where(lambda article, critic: article.author == critic.id)) == [] + # on its own, the plain table still means the join: + assert titles(builder.where(Author.name == "cat")) == ["Third"] + + +def test_lambda_mixes_with_other_where_parts(): + builder = Article.join("critic").where( + lambda article, critic: critic.name == "cat", + lambda article: article.title == "First", + title="Second", + ) + + assert titles(builder.orderby(Article.id)) == ["First", "Second", "Third"] + + # like a plain lambda, one that returns nothing doesn't filter: + assert len(Article.join("critic").where(lambda article, critic: None).collect()) == 3 + + # an argument with a default doesn't ask for a joined table: + assert titles(Article.where(lambda article, title="Second": article.title == title)) == ["Second"] + + +def test_lambda_asking_for_unjoined_relationship(): + with pytest.raises(ValueError, match="unjoined relationship.*critic.*joined: none"): + Article.where(lambda article, critic: critic.name == "ann").to_sql() + + with pytest.raises(ValueError, match=r"unjoined relationship\(s\) critic \(joined: writer\)"): + Article.join("writer").where(lambda article, critic: critic.name == "ann").collect() + + +def test_nested_one_to_many_collects_every_row(): + # "First" has two comments, which arrive on two rows that repeat ann and "First": + authors = Author.join("articles.comments").orderby(Author.id).collect() + + assert [ + (author.name, [(article.title, sorted(c.body for c in article.comments)) for article in author.articles]) + for author in authors + ] == [ + ("ann", [("First", ["meh", "nice"]), ("Third", ["nice"])]), + ("bob", [("Second", [])]), + ("cat", []), + ] + + +def test_nested_relationship_back_to_the_root_table(): + def overview(builder): + return [(row.title, row.writer.name, sorted(a.title for a in row.writer.articles)) for row in builder] + + expected = [ + ("First", "ann", ["First", "Third"]), + ("Second", "bob", ["Second"]), + ("Third", "ann", ["First", "Third"]), + ] + + assert overview(Article.join("writer.articles").orderby(Article.id)) == expected + assert overview(Article.join("writer.articles", method="inner").orderby(Article.id)) == expected + + # a selected root field stays the root's, it isn't moved to the nested 'articles' alias: + selected = Article.join("writer.articles").select(Article.title).orderby(Article.id).collect() + assert [row.title for row in selected] == ["First", "Second", "Third"] + assert [len(row.writer.articles) for row in selected] == [2, 1, 2] + + # and a filter on the root table keeps meaning the root: + assert overview(Article.join("writer.articles").where(Article.title == "Second")) == [("Second", "bob", ["Second"])] + assert ( + Article.join("writer.articles").where(Article.title == "Second").paginate(limit=1).pagination["total_items"] + == 1 + ) + + +def test_ambiguous_nested_name_needs_full_path(): + builder = Comment.join("post.writer.articles", "post.critic.articles") + + with pytest.raises(ValueError, match="unjoined relationship.*articles"): + builder.where(lambda comment, articles: articles.title == "Second").to_sql() + + # comments on posts whose writer (ann) also wrote "Third": + by_writer = builder.where(lambda comment, post__writer__articles: post__writer__articles.title == "Third") + assert sorted(row.body for row in by_writer) == ["meh", "nice", "nice"] + # comments on posts whose critic (bob, cat) wrote "Second": only bob reviewed one with comments + by_critic = builder.where(lambda comment, post__critic__articles: post__critic__articles.title == "Second") + assert sorted(row.body for row in by_critic) == ["meh", "nice"] + + +def test_on_relationship_keeps_the_table_itself(): + replies = Article.join("replies").where(Comment.body == "meh") + + assert titles(replies) == ["First"] + assert titles(replies.paginate(limit=1)) == ["First"] + assert replies.count() == 1 + + # joined both with and without an alias: the table name means the un-aliased `on` join + both = Article.join("replies", "comments").where(Comment.body == "meh") + assert titles(both) == ["First"] + assert sorted(comment.body for comment in both.first().comments) == ["meh", "nice"] + + +def test_mutations_reject_lambdas_on_joined_tables(): + builder = Article.join("critic").where(lambda article, critic: critic.name == "ann") + + for mutation in (builder.delete, builder._delete): + with pytest.raises(ValueError, match=r"delete\(\) and update\(\)"): + mutation() + for mutation in (builder.update, builder._update): + with pytest.raises(ValueError, match=r"delete\(\) and update\(\)"): + mutation(title="changed") + + assert Article.count() == 3 + + +def test_cache_key_does_not_depend_on_alias_hashes(): + # (compared directly: these models are local classes, which caching can't pickle) + def build(): + # every join() clones the relationship, so each builder gets its own alias hashes; + # 'replies' is joined without an alias, so Comment keeps its table name + return Article.join("replies", "critic").where(lambda article, critic: critic.name == "bob").where( + Comment.body == "meh" + ) + + first, second = build(), build() + assert hash(first.relationships["critic"]) != hash(second.relationships["critic"]) + key = str(first._cache_key_query()) + assert key == second._cache_key_query() + assert "" in key + assert '"alias_comment"."body"' in key + + +def test_left_join_filter_paginates_with_distinct_psql(dal_psql: TypeDAL): + _, article, comment = define_models(dal_psql) + + builder = article.join("comments").where(comment.body == "nice") + assert titles(builder.paginate(limit=1)) == ["First"] + assert builder.paginate(limit=1).pagination["total_items"] == 2 + + ordered = builder.orderby(~comment.id) + assert titles(ordered.paginate(limit=1)) == ["Third"] + assert ordered.count(distinct=article.id) == 2 diff --git a/tests/test_query_builder.py b/tests/test_query_builder.py index 3b22d7a..e1f6d1a 100644 --- a/tests/test_query_builder.py +++ b/tests/test_query_builder.py @@ -2,7 +2,14 @@ import pytest -from src.typedal import QueryBuilder, TypeDAL, TypedField, TypedTable, relationship +from src.typedal import ( + QueryBuilder, + TypeDAL, + TypedField, + TypedTable, + relationship, +) +from src.typedal.exceptions import ImplicitCrossJoinError from src.typedal.fields import rname from src.typedal.types import Query, Field from src.typedal.warnings import NoopQueryWarning, UnusedWindowWarning @@ -32,6 +39,12 @@ class TestRelationship(TypedTable): querytable: TestQueryTable +@db.define() +class TestThirdTable(TypedTable): + relation: TestRelationship + score: TypedField[int] + + class TestQueryTableBound(TestQueryTable): pass @@ -74,6 +87,152 @@ def _setup_data(): db.commit() +def test_where_on_joined_table_resolves_to_its_alias(): + _setup_data() + safe = TestQueryTable.join( + "relations", + method="inner", + condition_and=lambda _parent, relation: relation.value > 10, + ) + assert "CROSS JOIN" not in safe.to_sql() + rows = safe.collect() + # only the second parent has relations with value 33; dropping condition_and would also return the first + assert [row.number for row in rows] == [1] + assert [relation.value for relation in rows.first().relations] == [33, 33, 33, 33] + + # a builder that is extended later can't use condition_and anymore, so a plain where() on the joined table + # targets the alias too, whether it comes before or after the join: + joined_first = TestQueryTable.join("relations", method="inner").where(TestRelationship.value > 10) + where_first = TestQueryTable.where(TestRelationship.value > 10).join("relations", method="inner") + for builder in (joined_first, where_first): + assert "CROSS JOIN" not in builder.to_sql() + assert [row.number for row in builder.collect()] == [1] + assert builder.count() == 4 + assert builder.count(distinct=TestQueryTable.id) == 1 + assert builder.paginate(limit=1).pagination["total_items"] == 1 + +@pytest.mark.parametrize( + "build", + [ + lambda: TestQueryTable.where(TestRelationship.value > 0), + lambda: TestQueryTable.select(TestRelationship.name), + lambda: TestQueryTable.select(TestQueryTable.ALL, TestRelationship.ALL), + ], + ids=["where", "select_field", "select_all"], +) +def test_unrelated_table_is_rejected(build): + with pytest.raises(ImplicitCrossJoinError, match="cross join involving test_relationship"): + build().to_sql() + with pytest.raises(ImplicitCrossJoinError, match="cross join involving test_relationship"): + build().collect() + + +def test_where_linking_two_tables_is_allowed(): + query = TestQueryTable.where(TestQueryTable.id == TestRelationship.querytable) + + assert query.to_sql() + + +def test_explicit_cross_join_is_allowed(): + _setup_data() + query = TestQueryTable.cross_join(TestRelationship) + assert query.cross_join(TestRelationship) is query + assert query.count() == 40 + + joined = TestQueryTable.join("relations", method="inner").cross_join(TestRelationship) + assert "CROSS JOIN" in joined.to_sql() + assert len(joined.collect()) == 2 + + +def test_explicit_cross_join_is_exempt_from_alias_mismatch(): + _setup_data() + query = ( + TestQueryTable.join("relations", method="inner") + .cross_join(TestRelationship) + .where(TestRelationship.value > 10) + ) + + assert "CROSS JOIN" in query.to_sql() + rows = query.collect() + assert [row.number for row in rows] == [0, 1] + # the alias-joined relations are untouched by the predicate on the cross-joined copy: + assert [len(row.relations) for row in rows] == [4, 4] + + +def test_tables_linked_through_cross_joined_table_are_allowed(): + _setup_data() + relation = TestRelationship.first_or_fail() + TestThirdTable.truncate() + TestThirdTable.insert(relation=relation, score=5) + + query = TestQueryTable.cross_join(TestRelationship).where(TestRelationship.id == TestThirdTable.relation) + + assert query.count() == 5 + assert len(query.collect()) == 5 + + +def test_computed_link_between_tables_is_allowed(): + _setup_data() + TestThirdTable.truncate() + TestThirdTable.insert(relation=TestRelationship.first_or_fail(), score=-30) + + query = TestRelationship.where((TestRelationship.value + TestThirdTable.score) > 0) + + # 33 - 30 > 0 for the four relations of the second parent: + assert query.count() == 4 + assert {row.value for row in query.collect()} == {33} + + +def test_relationship_query_with_limitby_is_validated(): + _setup_data() + TestThirdTable.truncate() + TestThirdTable.insert(relation=TestRelationship.first_or_fail(), score=5) + builder = TestQueryTable.join("relations", method="inner").where(TestThirdTable.score > 0) + + with pytest.raises(ImplicitCrossJoinError, match="test_third_table"): + builder.select(limitby=(0, 1)).collect() + with pytest.raises(ImplicitCrossJoinError, match="test_third_table"): + builder.paginate(limit=1) + with pytest.raises(ImplicitCrossJoinError, match="test_third_table"): + builder.first() + + linked = builder.where(TestThirdTable.relation == TestRelationship.id).cross_join(TestRelationship) + assert linked.select(limitby=(0, 1)).collect() + + +def test_bare_left_expression_is_accepted(): + _setup_data() + query = TestQueryTable.select( + TestQueryTable.number, + TestRelationship.name, + left=TestRelationship.on(TestRelationship.querytable == TestQueryTable.id), + ) + + assert "LEFT JOIN" in query.to_sql() + # 2 parents x 4 relations + 3 parents without relations: + assert len(query.execute()) == 11 + + with pytest.raises(ImplicitCrossJoinError, match="test_third_table"): + TestQueryTable.select( + TestThirdTable.score, + left=TestRelationship.on(TestRelationship.querytable == TestQueryTable.id), + ).to_sql() + + +def test_cross_join_rejects_table_from_another_database(): + other_db = TypeDAL("sqlite:memory") + try: + + @other_db.define() + class OtherDatabaseTable(TypedTable): + value: int + + with pytest.raises(ValueError, match="same database"): + TestQueryTable.cross_join(OtherDatabaseTable) + finally: + other_db.close() + + def test_where_builder(): _setup_data() diff --git a/tests/test_relationships.py b/tests/test_relationships.py index 1191819..10402a2 100644 --- a/tests/test_relationships.py +++ b/tests/test_relationships.py @@ -10,8 +10,7 @@ from src.typedal import Relationship, TypeDAL, TypedField, TypedRows, TypedTable, relationship from src.typedal.caching import ( - _TypedalCache, - _TypedalCacheDependency, + cache_models, clear_cache, clear_expired, remove_cache, @@ -22,6 +21,100 @@ db = TypeDAL("sqlite:memory", lazy_policy="warn") +@db.define +class CacheUpsertUser(TypedTable): + email = TypedField(str, unique=True) + name: str + + +@db.define +class CacheUpsertArticle(TypedTable): + author: CacheUpsertUser + title: str + + +@pytest.mark.parametrize("moved", [False, True]) +def test_lazy_reference_update_invalidates_cache(moved: bool) -> None: + author = CacheUpsertUser.insert(email=f"lazy-{moved}@example.com", name="author") + other = CacheUpsertUser.insert(email=f"lazy-other-{moved}@example.com", name="other") + article = CacheUpsertArticle.insert(author=author.id, title="old") + query = CacheUpsertArticle.where(author=author.id).cache() + try: + assert query.collect().first().title == "old" + assert query.collect().metadata["cache"]["status"] == "cached" + # a plain pydal Set: when the update moves the row out of its query (author == author.id), + # invalidation can't look the rows up afterwards and has to drop the table's cache: + changes = {"author": other.id} if moved else {"title": "new"} + author_row = db.cache_upsert_user(author.id) + assert author_row.cache_upsert_article.update(**changes) == 1 + refreshed = query.collect() + assert refreshed.metadata["cache"]["status"] == "fresh" + if moved: + assert not refreshed + else: + assert (refreshed.first().id, refreshed.first().title) == (article.id, "new") + finally: + CacheUpsertArticle.where(id=article.id).delete() + CacheUpsertUser.where(CacheUpsertUser.id.belongs([author.id, other.id])).delete() + + +def test_smart_query_update_invalidates_cache() -> None: + user = CacheUpsertUser.insert(email="smart@example.com", name="old") + query = CacheUpsertUser.where(email=user.email).cache() + try: + assert query.collect().first().name == "old" + assert query.collect().metadata["cache"]["status"] == "cached" + rows = db.smart_query([CacheUpsertUser.email], "email = 'smart@example.com'") + assert rows.update(name="new") == 1 + refreshed = query.collect() + assert refreshed.metadata["cache"]["status"] == "fresh" + assert refreshed.first().id == user.id + assert refreshed.first().name == "new" + finally: + CacheUpsertUser.where(id=user.id).delete() + + +def test_empty_result_is_not_stored_and_does_not_shadow_later_results() -> None: + CacheUpsertUser.where(CacheUpsertUser.id > 0).delete() + query = CacheUpsertUser.where(name="nobody-yet").cache() + entries = db(db.typedal_cache).count() + assert not query.collect() + assert not query.collect() + assert db(db.typedal_cache).count() == entries + + user = CacheUpsertUser.insert(email="later@example.com", name="nobody-yet") + try: + assert query.collect().metadata["cache"]["status"] == "fresh" + cached = query.collect() + assert cached.metadata["cache"]["status"] == "cached" + assert cached.first().id == user.id + finally: + CacheUpsertUser.where(id=user.id).delete() + + +@pytest.mark.parametrize("operation", ["upsert", "update", "raw", "validated", "smart_query"]) +def test_cache_invalidation_after_changed_predicate(operation: str) -> None: + CacheUpsertUser.where(CacheUpsertUser.id > 0).delete() + inserted = CacheUpsertUser.insert(email="a@example.com", name="old") + query = CacheUpsertUser.where(name="old").cache() + assert query.collect().first().name == "old" + # make sure the second read is served from the cache, otherwise the assertions below prove nothing: + assert query.collect().metadata["cache"]["status"] == "cached" + if operation == "upsert": + CacheUpsertUser.upsert({"email": "a@example.com"}, name="new") + elif operation == "raw": + db(CacheUpsertUser.name == "old").update(name="new") + elif operation == "validated": + result = db(CacheUpsertUser.name == "old").validate_and_update(name="new") + assert result["updated"] == 1 + elif operation == "smart_query": + assert db.smart_query([CacheUpsertUser.name], "name = 'old'").update(name="new") == 1 + else: + assert CacheUpsertUser.where(name="old").update(name="new") == [inserted.id] + assert not query.collect() + assert CacheUpsertUser(inserted.id).name == "new" + + class TaggableMixin: tags = relationship( list["Tag"], @@ -314,8 +407,7 @@ def test_typedal_way(): author1 = User.where(id=4).join("articles").first() assert ( - len(author1.as_dict()["articles"]) == len(author1.__dict__["articles"]) == len( - dict(author1)["articles"]) == 2 + len(author1.as_dict()["articles"]) == len(author1.__dict__["articles"]) == len(dict(author1)["articles"]) == 2 ) @@ -453,8 +545,60 @@ def test_join_with_different_condition(): assert role_with_users.users[0].name != "Reader 1" +def test_condition_and_counts_match_combined_condition(): + with contextlib.closing(TypeDAL("sqlite:memory", enable_typedal_caching=False)) as local_db: + + @local_db.define() + class Parent(TypedTable): + name: str + children = relationship(list["Child"], lambda parent, child: child.parent == parent.id) + + @local_db.define() + class Child(TypedTable): + parent: Parent + active: bool + + first = Parent.insert(name="First") + second = Parent.insert(name="Second") + filtered = Parent.insert(name="Filtered") + Child.insert(parent=first, active=True) + Child.insert(parent=first, active=True) + Child.insert(parent=first, active=False) + Child.insert(parent=second, active=True) + Child.insert(parent=filtered, active=False) + + combined = Parent.join( + "children", + method="inner", + condition=lambda parent, child: (child.parent == parent.id) & (child.active == True), + ).orderby(Parent.id) + additional = Parent.join( + "children", + method="inner", + condition_and=lambda _parent, child: child.active == True, + ).orderby(Parent.id) + + combined_rows = combined.collect() + additional_rows = additional.collect() + assert combined_rows.as_list() == additional_rows.as_list() + assert [row.name for row in additional_rows] == ["First", "Second"] + assert [len(row.children) for row in additional_rows] == [2, 1] + assert all(child.active for row in additional_rows for child in row.children) + + combined_page = combined.paginate(limit=1) + additional_page = additional.paginate(limit=1) + assert combined_page.as_list() == additional_page.as_list() + assert (additional.count(), additional_page.pagination["total_items"]) == ( + combined.count(), + combined_page.pagination["total_items"], + ) == (3, 2) + assert additional.count(distinct=Parent.id) == combined.count(distinct=Parent.id) == 2 + assert additional_page.pagination["total_pages"] == combined_page.pagination["total_pages"] == 2 + + def test_caching(): _setup_data() + _TypedalCache, _TypedalCacheDependency = cache_models(db) uncached = User.join().collect_or_fail() cached = User.cache().join().collect_or_fail() # not actually cached yet! @@ -485,12 +629,12 @@ def test_caching(): cached_user_only2 = User.join().cache(User.id).collect_or_fail() assert ( - len(uncached2) - == len(uncached) - == len(cached2) - == len(cached) - == len(cached_user_only2) - == len(cached_user_only) + len(uncached2) + == len(uncached) + == len(cached2) + == len(cached) + == len(cached_user_only2) + == len(cached_user_only) ) assert uncached.as_json() == uncached2.as_json() == cached.as_json() == cached2.as_json() @@ -498,9 +642,9 @@ def test_caching(): assert cached.first().gid == cached2.first().gid assert ( - [_.name for _ in uncached2.first().roles] - == [_.name for _ in cached.first().roles] - == [_.name for _ in cached2.first().roles] + [_.name for _ in uncached2.first().roles] + == [_.name for _ in cached.first().roles] + == [_.name for _ in cached2.first().roles] ) assert not uncached2.metadata.get("cache", {}).get("enabled") @@ -541,8 +685,8 @@ def test_caching(): assert User.cache().join().paginate(limit=1, page=2).metadata["cache"].get("status") == "cached" assert User.cache().join().paginate(limit=1, page=2).metadata["cache"].get("cached_at") - remove_cache(1, "user") - remove_cache([2], "user") + remove_cache(db, 1, "user") + remove_cache(db, [2], "user") assert User.cache("id").join().paginate(limit=1, page=1).metadata["cache"].get("status") == "fresh" assert User.cache().join().paginate(limit=1, page=2).metadata["cache"].get("status") == "fresh" @@ -557,7 +701,7 @@ def test_caching(): assert _TypedalCache.count() > 0 assert _TypedalCacheDependency.count() > 0 - clear_cache() + clear_cache(db) assert _TypedalCache.count() == 0 assert _TypedalCacheDependency.count() == 0 @@ -578,8 +722,8 @@ def test_caching(): time.sleep(3) # for TTL - assert clear_expired() - assert not clear_expired() + assert clear_expired(db) + assert not clear_expired(db) assert not _TypedalCache.count() assert not _TypedalCacheDependency.count() @@ -828,6 +972,7 @@ def get_all_users() -> list[str]: def test_illegal(): with pytest.raises(ValueError), pytest.warns(UserWarning): + class HasRelationship: something = relationship("...", condition=lambda: 1, on=lambda: 2) @@ -846,15 +991,19 @@ def test_join_relationship_custom_on(): rows2 = Tag.join( Tag.articles, + # each item must be a `table.on(...)` join; a bare table or query used to be dropped silently, + # which turned this into an implicit cross join with article: on=lambda tag, article: [ - tagged := Tagged.unique_alias(), - (tagged.tag == tag.id) & (article.gid == tagged.entity) & (article.author == 3), + (tagged := Tagged.unique_alias()).on(tagged.tag == tag.id), + article.on((article.gid == tagged.entity) & (article.author == 3)), ], method="inner", ) assert all([row.articles for row in rows1]) - assert all([row.articles for row in rows2]) + # custom `on` is always a left join, so tags without a matching article are kept (with no articles): + matched = {row.id: sorted(article.id for article in row.articles) for row in rows2 if row.articles} + assert matched == {row.id: sorted(article.id for article in row.articles) for row in rows1} def test_join_with_select(): @@ -1092,16 +1241,16 @@ class SelfReferencing(TypedTable): key: str value: str - related_directly = relationship(list["SelfReferencing"], - lambda self, other: self.value == other.value - ) + related_directly = relationship(list["SelfReferencing"], lambda self, other: self.value == other.value) - related_indirectly = relationship(list["SelfReferencing"], - on=lambda self, other: [ - via := IntermediateTable.unique_alias(), - via.on(via.left == self.value), - other.on(via.right == other.value) - ]) + related_indirectly = relationship( + list["SelfReferencing"], + on=lambda self, other: [ + via := IntermediateTable.unique_alias(), + via.on(via.left == self.value), + other.on(via.right == other.value), + ], + ) IntermediateTable.insert(left="two", right="two") IntermediateTable.insert(left="three", right="one") @@ -1122,6 +1271,7 @@ class SelfReferencing(TypedTable): assert len(rows.related_directly) == 2 assert set(row.value for row in rows.related_directly) == {"two"} + def test_relationship_labels(): @db.define @@ -1144,3 +1294,12 @@ class TableWithRelationship(TypedTable): assert TableWithRelationship.other_gid.label == "Relationship Reference" assert TableWithRelationship.target.label == "Other Gid" + +def _double(value: int) -> int: + return value * 2 + + +def test_memoize_ignores_plain_arguments_for_dependencies() -> None: + result, status = db.memoize(_double, 21, key="double-plain-arg") + assert (result, status) == (42, "fresh") + assert db.memoize(_double, 21, key="double-plain-arg") == (42, "cached") diff --git a/tests/test_upsert.py b/tests/test_upsert.py new file mode 100644 index 0000000..75bc51a --- /dev/null +++ b/tests/test_upsert.py @@ -0,0 +1,972 @@ +""" +Integration tests for table-level upsert against real database connections. + +Tests that exercise backend-specific SQL (native ON CONFLICT on Postgres, the select-then-write path on SQLite and +MySQL, UPDATE ... RETURNING or not) use `upsert_db`, which runs them on every backend. Everything decided in Python +before any SQL runs (validation, hook policies, warnings) uses `sqlite_db`. +""" + +import datetime as dt +import typing as t +import warnings +from collections.abc import Iterator +from pathlib import Path + +import pytest +from pydal import DAL, Field +from pydal.helpers.classes import ExecutionHandler +from pydal.helpers.methods import delete_uploaded_files +from testcontainers.community.mysql import MySqlContainer + +from src.typedal import TypeDAL, TypedField, TypedTable +from src.typedal.exceptions import UpsertAmbiguityError, UpsertHookError, UpsertKeyError +from src.typedal.fields import UploadField +from src.typedal.mixins import SlugMixin, TimestampsMixin +from src.typedal.types import AffectedSet, OpRow, Reference, Set +from src.typedal.updates import install_update +from src.typedal.upsert import install_upsert +from src.typedal.warnings import UpsertHooksWarning + + +class RecordingHandler(ExecutionHandler): + """Observe actual statements through PyDAL's execution-handler API.""" + + def before_execute(self, command: str) -> None: + db = t.cast(RecordingDAL, self.adapter.db) + if hasattr(db, "statements"): + db.statements.append(command) + + +class RecordingDAL(TypeDAL): + """A real DAL connection with statement recording enabled.""" + + statements: list[str] + execution_handlers = [*TypeDAL.execution_handlers, RecordingHandler] + + +class UpsertUser(TypedTable): + email = TypedField(str, unique=True, rname="email_address") + name = TypedField(str, default="default") + created_at = TypedField(dt.datetime, default=lambda: dt.datetime(2026, 1, 1)) + + +@pytest.fixture(scope="module") +def mysql_uri() -> Iterator[str]: + with MySqlContainer("mysql:8.4", dialect="pymysql") as container: + yield "mysql:pymysql://" + container.get_connection_url().split("://", 1)[1] + + +def _recording_db(request: pytest.FixtureRequest, backend: str, tmp_path: Path) -> Iterator[RecordingDAL]: + """A database with UpsertUser defined, dropped again afterwards.""" + uri = "sqlite:memory" + if backend == "postgres": + uri = request.getfixturevalue("dal_psql_uri") + elif backend == "mysql": + uri = request.getfixturevalue("mysql_uri") + db = RecordingDAL(uri, enable_typedal_caching=False, folder=str(tmp_path)) + db.statements = [] + + db.define(UpsertUser) + db.commit() + db.statements.clear() + + try: + yield db + finally: + db.rollback() + for name in reversed(db.tables): + db[name].drop() + db.commit() + db.close() + UpsertUser.unbind() + + +@pytest.fixture(params=["sqlite", "postgres", "mysql"]) +def upsert_db(request: pytest.FixtureRequest, tmp_path: Path) -> Iterator[RecordingDAL]: + yield from _recording_db(request, request.param, tmp_path) + + +@pytest.fixture +def sqlite_db(request: pytest.FixtureRequest, tmp_path: Path) -> Iterator[RecordingDAL]: + yield from _recording_db(request, "sqlite", tmp_path) + + +@pytest.fixture +def hook_events() -> list[str]: + """Register all four insert and update hooks on UpsertUser (before-hooks with upsert='ignore').""" + events: list[str] = [] + + def before_insert(row: OpRow) -> None: + events.append("before_insert") + assert UpsertUser.where(email=row.email).count() == 0 + + def after_insert(row: OpRow, row_id: Reference) -> None: + events.append("after_insert") + assert UpsertUser(int(row_id)).name == row.name + + def before_update(rows: Set, row: OpRow) -> None: + events.append("before_update") + assert rows.count() == 1 + assert rows.select().first().name != row.name + + def after_update(rows: Set, row: OpRow) -> None: + events.append("after_update") + assert rows.select().first().name == row.name + + UpsertUser.before_insert(before_insert, upsert="ignore") + UpsertUser.after_insert(after_insert) + UpsertUser.before_update(before_update, upsert="ignore") + UpsertUser.after_update(after_update) + return events + + +# --- every backend: the SQL paths --- + + +def test_insert_update_and_key_only(upsert_db: RecordingDAL) -> None: + # PyDAL's own upload hooks are registered on every table; they must not count as unmarked before-hooks: + with warnings.catch_warnings(): + warnings.simplefilter("error", UpsertHooksWarning) + defaults = UpsertUser.upsert({"email": "defaults@example.com"}) + upsert_db.statements.clear() + inserted = UpsertUser.upsert({"email": "a@example.com"}, name="Alice 'quoted'") + updated = UpsertUser.upsert({"email": "a@example.com"}, name="Bob") + + assert (defaults.name, defaults.created_at) == ("default", dt.datetime(2026, 1, 1)) + assert isinstance(inserted, UpsertUser) + assert isinstance(updated, UpsertUser) + assert inserted.name == "Alice 'quoted'" + assert updated.id == inserted.id + assert updated.name == "Bob" + assert updated.created_at == dt.datetime(2026, 1, 1) + if upsert_db._adapter.dbengine == "postgres": + assert len(upsert_db.statements) == 2 + assert all(sql.startswith("INSERT INTO") and "ON CONFLICT" in sql for sql in upsert_db.statements) + + # a key-only call returns the existing row as-is: + upsert_db.statements.clear() + existing = UpsertUser.upsert({"email": "a@example.com"}) + assert (existing.id, existing.name) == (inserted.id, "Bob") + assert not any(sql.startswith("UPDATE") or "DO UPDATE" in sql for sql in upsert_db.statements) + + +def test_composite_unique_key(upsert_db: RecordingDAL) -> None: + @upsert_db.define + class UpsertMembership(TypedTable): + user_code: TypedField[str] + group_code: TypedField[str] + role: TypedField[str] + + UpsertMembership.create_index("upsert_membership_unique", "user_code", "group_code", unique=True) + first = UpsertMembership.upsert({"user_code": "u", "group_code": "g"}, role="member") + updated = UpsertMembership.upsert({"user_code": "u", "group_code": "g"}, role="admin") + other = UpsertMembership.upsert({"user_code": "u", "group_code": "other"}, role="member") + assert first.id == updated.id + assert updated.role == "admin" + assert other.id != first.id + + +def test_upsert_participates_in_transaction(upsert_db: RecordingDAL) -> None: + UpsertUser.upsert({"email": "a@example.com"}, name="Alice") + upsert_db.rollback() + assert UpsertUser.count() == 0 + + +@pytest.mark.asyncio +async def test_async_upsert_and_session_rollback(upsert_db: RecordingDAL) -> None: + with pytest.raises(RuntimeError, match="rollback requested"): + async with upsert_db.session(): + first = await UpsertUser.upsert_async({"email": "a@example.com"}, name="Alice") + updated = await UpsertUser.upsert_async({"email": "a@example.com"}, name="Bob") + existing = await UpsertUser.upsert_async({"email": "a@example.com"}) + assert isinstance(updated, UpsertUser) + assert first.id == updated.id == existing.id + assert existing.name == "Bob" + raise RuntimeError("rollback requested") + assert await UpsertUser.count_async() == 0 + + +def test_hooks_run_only_for_the_branch_taken(upsert_db: RecordingDAL, hook_events: list[str]) -> None: + # registered with upsert='ignore', so the before-hooks are skipped silently: + with warnings.catch_warnings(): + warnings.simplefilter("error", UpsertHooksWarning) + inserted = UpsertUser.upsert({"email": "a@example.com"}) + assert inserted.name == "default" + assert hook_events == ["after_insert"] + + hook_events.clear() + assert UpsertUser.upsert({"email": "a@example.com"}).id == inserted.id + assert hook_events == [] + + assert UpsertUser.upsert({"email": "a@example.com"}, name="Bob").id == inserted.id + assert hook_events == ["after_update"] + + hook_events.clear() + UpsertUser.upsert({"email": "b@example.com"}, name="Carol") + assert hook_events == ["after_insert"] + + # the before-hooks still run for normal writes: + hook_events.clear() + UpsertUser.update_or_insert({"email": "a@example.com"}, name="Dave") + assert hook_events == ["before_update", "after_update"] + + +@pytest.mark.asyncio +async def test_async_upsert_after_hooks(upsert_db: RecordingDAL, hook_events: list[str]) -> None: + inserted = await UpsertUser.upsert_async({"email": "a@example.com"}, name="Alice") + updated = await UpsertUser.upsert_async({"email": "a@example.com"}, name="Bob") + assert updated.id == inserted.id + assert hook_events == ["after_insert", "after_update"] + + +def test_missing_unique_key_or_ambiguous_python_lookup(upsert_db: RecordingDAL) -> None: + @upsert_db.define + class NonUniqueUpsert(TypedTable): + code: str + value: str + + upsert_db.commit() + if upsert_db._adapter.dbengine == "postgres": + with pytest.raises(UpsertKeyError, match="unique"): + NonUniqueUpsert.upsert({"code": "same"}, value="new") + else: + first = NonUniqueUpsert.upsert({"code": "same"}, value="first") + updated = NonUniqueUpsert.upsert({"code": "same"}, value="second") + assert updated.id == first.id + NonUniqueUpsert.insert(code="same", value="other") + with pytest.raises(UpsertAmbiguityError, match="multiple rows"): + NonUniqueUpsert.upsert({"code": "same"}, value="ambiguous") + + +@pytest.mark.parametrize("path", ["builder", "raw", "raw_where", "record", "table", "validated"]) +def test_update_paths_pass_affected_rows_to_after_hooks(upsert_db: RecordingDAL, path: str) -> None: + # every update changes the column it filters on, so the original query no longer finds the row afterwards: + original = UpsertUser.insert(email="a@example.com", name="old") + UpsertUser.insert(email="b@example.com", name="other") + affected: list[int] = [] + + def after_update(rows: Set, row: OpRow) -> None: + affected.extend(rows.select().column(UpsertUser.id)) + assert row.name == "new" + assert rows.select().first().name == "new" + + UpsertUser.after_update(after_update) + if path == "builder": + assert UpsertUser.where(name="old").update(name="new") == [original.id] + elif path == "raw": + assert upsert_db(UpsertUser.name == "old").update(name="new") == 1 + elif path == "raw_where": + assert upsert_db(UpsertUser.id > 0).where(UpsertUser.name == "old").update(name="new") == 1 + elif path == "record": + original.update_record(name="new") + elif path == "validated": + response = upsert_db(UpsertUser.name == "old").validate_and_update(name="new") + assert response["updated"] == 1 + assert not response["errors"] + else: + UpsertUser.update_or_insert({"name": "old"}, name="new") + assert affected == [original.id] + + +def test_after_update_hook_gets_the_known_ids(upsert_db: RecordingDAL) -> None: + original = UpsertUser.insert(email="a@example.com", name="old") + ids: list[int] = [] + + def after_update(rows: AffectedSet, row: OpRow) -> None: + ids.extend(rows.affected_ids) + assert row.name == "new" + + UpsertUser.after_update(after_update) + + # nothing updated, no hook: + no_match = upsert_db(UpsertUser.email == "missing@example.com") + assert no_match.where(None) is no_match + assert no_match.update(name="new") == 0 + assert ids == [] + + # the ids come from the write itself (MySQL, without RETURNING, selects them first): + upsert_db.statements.clear() + assert upsert_db(UpsertUser.id == original.id).update(name="new") == 1 + assert ids == [original.id] + assert len(upsert_db.statements) == (2 if upsert_db._adapter.dbengine == "mysql" else 1) + + # an upsert that changes nothing is still an update: + ids.clear() + assert UpsertUser.upsert({"email": "a@example.com"}, name="new").id == original.id + assert ids == [original.id] + + +def test_upsert_updates_only_explicit_values(upsert_db: RecordingDAL) -> None: + @upsert_db.define + class ComputedUpsert(TypedTable): + code = TypedField(str, unique=True) + name: str + stamp = TypedField(str, default="insert", update="update") + computed = TypedField(str, compute=lambda row: row.name.upper()) + + inserted = ComputedUpsert.upsert({"code": "same"}, name="alice") + updated = ComputedUpsert.upsert({"code": "same"}, name="bob") + assert inserted.computed == updated.computed == "ALICE" + assert updated.stamp == "insert" + explicit = ComputedUpsert.upsert({"code": "same"}, name="bob", stamp="explicit", computed="manual") + assert explicit.stamp == "explicit" + assert explicit.computed == "manual" + + +@pytest.mark.parametrize("operation", ["upsert", "update"]) +def test_common_filter_after_hooks_see_row_outside_filter(upsert_db: RecordingDAL, operation: str) -> None: + @upsert_db.define(common_filter=lambda _query: upsert_db.filtered_upsert.active == True) # noqa: E712 + class FilteredUpsert(TypedTable): + code = TypedField(str, unique=True) + active = TypedField(bool, default=True) + + inserted = FilteredUpsert.upsert({"code": "same"}) + affected: list[int] = [] + + def after_update(rows: Set, row: OpRow) -> None: + affected.extend(rows.select().column(FilteredUpsert.id)) + assert row.active is False + assert rows.select().first().active is False + + FilteredUpsert.after_update(after_update) + if operation == "upsert": + updated = FilteredUpsert.upsert({"code": "same"}, active=False) + assert updated.id == inserted.id + else: + assert FilteredUpsert.where(active=True).update(active=False) == [inserted.id] + assert affected == [inserted.id] + assert FilteredUpsert.count() == 0 + + +def test_conflict_on_another_unique_column_is_integrity_error(upsert_db: RecordingDAL) -> None: + @upsert_db.define + class OtherUniqueUpsert(TypedTable): + email = TypedField(str, unique=True) + username = TypedField(str, unique=True) + + OtherUniqueUpsert.insert(email="old@example.com", username="taken") + upsert_db.commit() + with pytest.raises(upsert_db._adapter.driver.IntegrityError): + OtherUniqueUpsert.upsert({"email": "new@example.com"}, username="taken") + + +def test_branch_metadata_does_not_shadow_inserted_field(upsert_db: RecordingDAL) -> None: + # Postgres reports the branch as an extra `inserted` column + @upsert_db.define + class BranchUpsert(TypedTable): + code = TypedField(str, unique=True) + inserted: bool + + events: list[str] = [] + BranchUpsert.after_insert(lambda _row, _row_id: events.append("insert")) + BranchUpsert.after_update(lambda _rows, _row: events.append("update")) + first = BranchUpsert.upsert({"code": "same"}, inserted=False) + second = BranchUpsert.upsert({"code": "same"}, inserted=True) + assert first.inserted is False + assert second.inserted is True + assert second.id == first.id + assert events == ["insert", "update"] + + +@pytest.mark.parametrize("primarykey", [["k"], ["k", "n"]]) +def test_keyed_tables_support_after_update_hooks(upsert_db: RecordingDAL, primarykey: list[str]) -> None: + table = upsert_db.define_table( + f"upsert_keyed_{len(primarykey)}", + Field("k", "string", length=32), + Field("n", "integer"), + Field("v", "integer"), + primarykey=primarykey, + ) + table.insert(k="a", n=1, v=0) + table.insert(k="a", n=2, v=0) if len(primarykey) > 1 else table.insert(k="b", n=2, v=0) + seen: list[t.Any] = [] + + def after_update(rows: AffectedSet, _row: OpRow) -> None: + seen.extend(rows.affected_ids) + assert rows.count() == len(rows.affected_ids) + + table._after_update.append(after_update) + assert upsert_db(table.n == 1).update(v=5) == 1 + expected = "a" if len(primarykey) == 1 else ("a", 1) + assert seen == [expected] + assert upsert_db(table.v == 5).count() == 1 + + +@pytest.mark.parametrize("autodelete_hook", [True, False]) +def test_upload_fields_are_stored(upsert_db: RecordingDAL, tmp_path: Path, autodelete_hook: bool) -> None: + @upsert_db.define() + class UpsertDocument(TypedTable): + code = TypedField(str, unique=True) + attachment = UploadField(uploadfolder=str(tmp_path), autodelete=True) + + if not autodelete_hook: + UpsertDocument._before_update.remove(delete_uploaded_files) + + with warnings.catch_warnings(): + warnings.simplefilter("error", UpsertHooksWarning) + first = UpsertDocument.upsert({"code": "a"}, attachment={"data": b"one", "filename": "one.txt"}) + first_file = tmp_path / first.attachment + assert first_file.read_bytes() == b"one" + + second = UpsertDocument.upsert({"code": "a"}, attachment={"data": b"two", "filename": "two.txt"}) + + assert second.id == first.id + assert second.attachment != first.attachment + assert (tmp_path / second.attachment).read_bytes() == b"two" + # like a normal update: PyDAL's autodelete hook removes the replaced file, without it the file stays + assert first_file.exists() is not autodelete_hook + + +def test_decimal_values_are_coerced_not_inlined(upsert_db: RecordingDAL) -> None: + from decimal import Decimal + + @upsert_db.define() + class UpsertPrice(TypedTable): + code = TypedField(str, unique=True) + amount = TypedField(Decimal, type="decimal(10,2)") + + UpsertPrice.upsert({"code": "keep"}, amount=Decimal("1.00")) + for payload in ["1); DELETE FROM upsert_price; --", "1, code = 'pwned'", "NaN"]: + with pytest.raises(ValueError, match="Invalid decimal"): + UpsertPrice.upsert({"code": "keep"}, amount=payload) + with pytest.raises(ValueError, match="Invalid decimal"): + UpsertPrice.insert(code="other", amount=payload) + with pytest.raises(ValueError, match="Invalid decimal"): + UpsertPrice.where(UpsertPrice.amount == payload).collect() + upsert_db.rollback() + + UpsertPrice.upsert({"code": "keep"}, amount=Decimal("1.00")) + updated = UpsertPrice.upsert({"code": "keep"}, amount=" 2.50 ") + assert updated.amount == Decimal("2.50") + assert [row.code for row in UpsertPrice.collect()] == ["keep"] + + +def test_tenant_filtered_upsert_cannot_overwrite_other_tenant(upsert_db: RecordingDAL) -> None: + @upsert_db.define() + class UpsertTenantRow(TypedTable): + code = TypedField(str, unique=True) + secret: str + request_tenant = TypedField(str, default="tenant-a") + + tenant = upsert_db.upsert_tenant_row.request_tenant + first = UpsertTenantRow.upsert({"code": "shared"}, secret="a-secret") + upsert_db.commit() + + tenant.default = "tenant-b" + try: + upsert_db.statements.clear() + with pytest.raises(Exception) as error: + UpsertTenantRow.upsert({"code": "shared"}, secret="b-overwrite") + # the lookup is tenant-filtered, so the write is an INSERT that hits the unique constraint: + assert "ON CONFLICT" not in " ".join(upsert_db.statements) + assert not isinstance(error.value, AssertionError) + upsert_db.rollback() + finally: + tenant.default = "tenant-a" + + row = upsert_db(upsert_db.upsert_tenant_row.id == first.id).select().first() + assert (row.secret, row.request_tenant) == ("a-secret", "tenant-a") + + +def test_upsert_normalizes_keys_once(upsert_db: RecordingDAL) -> None: + """Insert, update and key-only lookup share keys converted exactly once per call.""" + conversions: list[str] = [] + + def normalize(value: str) -> str: + conversions.append(value) + return value.lower() + + @upsert_db.define + class NormalizedUpsert(TypedTable): + email = TypedField(str, unique=True, filter_in=normalize) + name = TypedField(str, default="default") + + first = NormalizedUpsert.upsert({"email": "A@EXAMPLE.COM"}, name="old") + updated = NormalizedUpsert.upsert({"email": "A@EXAMPLE.COM"}, name="new") + unchanged = NormalizedUpsert.upsert({"email": "A@EXAMPLE.COM"}) + assert first.email == "a@example.com" + assert updated.id == unchanged.id == first.id + assert updated.name == unchanged.name == "new" + assert NormalizedUpsert.count() == 1 + assert conversions == ["A@EXAMPLE.COM"] * 3 + + +# --- SQLite only: decided in Python, before any SQL --- + + +@pytest.mark.parametrize( + "key,values,match", + [ + ({}, {}, "nonempty"), + ({"missing": 1}, {}, "Unknown"), + ({"email": "a@example.com"}, {"missing": 1}, "Unknown"), + ({"email": None}, {}, "must not be None"), + ({"id": 1}, {}, "key must not contain id"), + ({"email": "a@example.com"}, {"id": 5}, "values must not contain id"), + ({"email": "a@example.com"}, {"name": "x", "id": 5}, "values must not contain id"), + ({"email": ["a@example.com"]}, {}, "plain scalars"), + ({"email": {"a": 1}}, {}, "plain scalars"), + ({"email": object()}, {}, "plain scalars"), + ], +) +def test_invalid_arguments_are_rejected_before_sql( + sqlite_db: RecordingDAL, key: dict[str, t.Any], values: dict[str, t.Any], match: str +) -> None: + original = UpsertUser.insert(email="a@example.com", name="Alice") + sqlite_db.statements.clear() + with pytest.raises(UpsertKeyError, match=match): + UpsertUser.upsert(key, **values) + assert sqlite_db.statements == [] + assert UpsertUser(original.id).name == "Alice" + + +def test_overlap_error_contains_concrete_alternative(sqlite_db: RecordingDAL) -> None: + with pytest.raises(UpsertKeyError) as error: + UpsertUser.upsert({"email": "old@example.com"}, email="secret@example.com", name="hunter2") + assert "UpsertUser.update_or_insert({'email': ...}, email=..., name=...)" in str(error.value) + # values may come from request data and must not leak into logs: + assert "secret@example.com" not in str(error.value) + assert "hunter2" not in str(error.value) + original = UpsertUser.insert(email="old@example.com", name="original") + changed = UpsertUser.update_or_insert({"email": "old@example.com"}, email="new@example.com", name="fixed") + assert changed.id == original.id + assert changed.email == "new@example.com" + + +def test_key_only_with_required_fields(sqlite_db: RecordingDAL) -> None: + @sqlite_db.define + class RequiredUpsert(TypedTable): + code = TypedField(str, unique=True) + name = TypedField(str, required=True) + + # inserting needs the required value... + with pytest.raises(RuntimeError, match="missing required field: name"): + RequiredUpsert.upsert({"code": "new"}) + assert RequiredUpsert.count() == 0 + + # ...returning an existing row doesn't: + inserted = RequiredUpsert.insert(code="a", name="Alice") + existing = RequiredUpsert.upsert({"code": "a"}) + assert (existing.id, existing.name) == (inserted.id, "Alice") + + +def test_upsert_is_table_class_only(sqlite_db: RecordingDAL) -> None: + row = UpsertUser.upsert({"email": "a@example.com"}) + with pytest.raises(AttributeError): + UpsertUser.where(id=0).upsert({"email": "a@example.com"}) + with pytest.raises(AttributeError): + row.upsert({"email": "a@example.com"}) + + +@pytest.mark.parametrize("denied", ["insert", "update"]) +def test_upsert_requires_both_write_permissions(sqlite_db: RecordingDAL, denied: str) -> None: + @sqlite_db.define(permissions={denied: False}) + class RestrictedUpsertUser(TypedTable): + email = TypedField(str, unique=True) + + sqlite_db.statements.clear() + with pytest.raises(PermissionError, match=f"{denied} is not allowed"): + RestrictedUpsertUser.upsert({"email": "a@example.com"}) + assert sqlite_db.statements == [] + + +def test_unmarked_before_hook_warns_without_running(sqlite_db: RecordingDAL) -> None: + events: list[str] = [] + + def unmarked_hook(_row: OpRow) -> None: + events.append("before_insert") + + UpsertUser.before_insert(unmarked_hook) + with pytest.warns(UpsertHooksWarning, match="unmarked_hook"): + UpsertUser.upsert({"email": "a@example.com"}, name="Alice") + assert events == [] + + +@pytest.mark.asyncio +async def test_async_warning_points_to_calling_file(sqlite_db: RecordingDAL) -> None: + def unmarked_hook(_row: OpRow) -> None: + pytest.fail("Before-hook must not run during upsert") + + UpsertUser.before_insert(unmarked_hook) + with pytest.warns(UpsertHooksWarning) as emitted: + await UpsertUser.upsert_async({"email": "a@example.com"}) + # pytest.warns records every warning; drivers may add their own when the worker connects. + hook_warnings = [w for w in emitted if issubclass(w.category, UpsertHooksWarning)] + assert len(hook_warnings) == 1, [(w.category.__name__, str(w.message), w.filename) for w in emitted] + assert hook_warnings[0].filename == __file__ + + +@pytest.mark.parametrize("policy", ["error", "ignore"]) +@pytest.mark.parametrize("register", ["before_insert", "before_update", "before_insert_once", "before_update_once"]) +def test_before_hook_policies(sqlite_db: RecordingDAL, register: str, policy: t.Literal["error", "ignore"]) -> None: + def policy_hook(*_args: t.Any) -> None: + pytest.fail("Before-hook must not run during upsert") + + getattr(UpsertUser, register)(policy_hook, upsert=policy) + if policy == "error": + branch = "insert" if "insert" in register else "update" + with pytest.raises(UpsertHookError, match=f"UpsertUser.before_{branch} hook 'policy_hook'.*update_or_insert"): + UpsertUser.upsert({"email": "a@example.com"}) + assert sqlite_db.statements == [] + else: + with warnings.catch_warnings(): + warnings.simplefilter("error", UpsertHooksWarning) + UpsertUser.upsert({"email": "a@example.com"}) + + +def test_bound_method_policy_uses_callable_equality(sqlite_db: RecordingDAL) -> None: + class HookService: + def before_insert(self, _row: OpRow) -> None: + pytest.fail("Before-hook must not run during upsert") + + service = HookService() + UpsertUser.before_insert(service.before_insert, upsert="error") + UpsertUser.before_insert(service.before_insert, upsert="ignore") + with warnings.catch_warnings(record=True) as emitted: + warnings.simplefilter("always", UpsertHooksWarning) + UpsertUser.upsert({"email": "a@example.com"}) + assert emitted == [] + + +def test_hook_policy_belongs_to_model_registration(sqlite_db: RecordingDAL) -> None: + @sqlite_db.define + class OtherUpsertUser(TypedTable): + email = TypedField(str, unique=True) + + def shared_hook(_row: OpRow) -> None: + pytest.fail("Before-hook must not run during upsert") + + UpsertUser.before_insert(shared_hook, upsert="ignore") + OtherUpsertUser.before_insert(shared_hook, upsert="error") + UpsertUser.upsert({"email": "a@example.com"}) + with pytest.raises(UpsertHookError): + OtherUpsertUser.upsert({"email": "a@example.com"}) + OtherUpsertUser.before_insert(shared_hook, upsert="ignore") + OtherUpsertUser.upsert({"email": "a@example.com"}) + + +def test_invalid_hook_policy_leaves_registration_unchanged(sqlite_db: RecordingDAL) -> None: + def before_insert(_row: OpRow) -> None: + pytest.fail("Invalid hook registration must not be installed") + + with pytest.raises(ValueError, match="Invalid upsert hook policy"): + UpsertUser.before_insert(before_insert, upsert=t.cast(t.Any, "invalid")) + UpsertUser.insert(email="a@example.com") + UpsertUser.upsert({"email": "a@example.com"}, name="updated") + + +def test_timestamps_and_slug_mixins_refuse_upsert(sqlite_db: RecordingDAL) -> None: + @sqlite_db.define() + class UpsertArticle(TypedTable, SlugMixin, TimestampsMixin, slug_field="title"): + code = TypedField(str, unique=True) + title: str + + sqlite_db.statements.clear() + with pytest.raises(UpsertHookError, match="before_insert hook .*generate_slug"): + UpsertArticle.upsert({"code": "a"}, title="Hello") + assert sqlite_db.statements == [] + + # update_or_insert runs the hooks, so it keeps working: + article = UpsertArticle.update_or_insert({"code": "a"}, code="a", title="Hello") + assert article.slug == "hello" + + +def test_timestamps_mixin_alone_refuses_upsert(sqlite_db: RecordingDAL) -> None: + @sqlite_db.define() + class UpsertStamped(TypedTable, TimestampsMixin): + code = TypedField(str, unique=True) + + with pytest.raises(UpsertHookError, match="before_update hook 'set_updated_at'"): + UpsertStamped.upsert({"code": "a"}) + + +@pytest.mark.parametrize("write", ["update", "update_naive", "builder"]) +def test_empty_update_raises_before_sql(sqlite_db: RecordingDAL, write: str) -> None: + rows = sqlite_db(UpsertUser.id > 0) + with pytest.raises(ValueError, match="No fields to update"): + if write == "builder": + UpsertUser.where(UpsertUser.id > 0).update() + else: + getattr(rows, write)() + assert sqlite_db.statements == [] + + +def test_update_veto_and_naive_bypass(sqlite_db: RecordingDAL) -> None: + inserted = UpsertUser.insert(email="a@example.com", name="old") + events: list[str] = [] + + def veto(_rows: Set, _row: OpRow) -> bool: + events.append("before") + return True + + UpsertUser.before_update(veto, upsert="ignore") + UpsertUser.after_update(lambda _rows, _row: events.append("after")) + assert UpsertUser.where(id=inserted.id).update(name="blocked") == [] + assert UpsertUser(inserted.id).name == "old" + assert events == ["before"] + events.clear() + assert sqlite_db(UpsertUser.id == inserted.id).update_naive(name="naive") == 1 + assert UpsertUser(inserted.id).name == "naive" + assert events == [] + + +def test_plain_update_without_after_hooks_uses_only_rowcount(sqlite_db: RecordingDAL) -> None: + original = UpsertUser.insert(email="a@example.com", name="old") + sqlite_db.statements.clear() + assert sqlite_db(UpsertUser.id == original.id).update(name="new") == 1 + assert len(sqlite_db.statements) == 1 + assert sqlite_db.statements[0].startswith("UPDATE") + assert "RETURNING" not in sqlite_db.statements[0] + assert UpsertUser(original.id).name == "new" + + +def test_affected_set_can_be_narrowed_inside_hook(sqlite_db: RecordingDAL) -> None: + first = UpsertUser.insert(email="a@example.com", name="old") + second = UpsertUser.insert(email="b@example.com", name="old") + seen: dict[str, t.Any] = {} + + def after_update(rows: AffectedSet, _row: OpRow) -> None: + narrowed = rows.where(UpsertUser.email == "a@example.com") + called = rows(UpsertUser.email == "b@example.com") + seen["types"] = (type(narrowed).__name__, type(called).__name__) + seen["narrowed"] = narrowed.select(UpsertUser.id).column(UpsertUser.id) + seen["called"] = called.select(UpsertUser.id).column(UpsertUser.id) + seen["none"] = rows.where(None) is rows + + UpsertUser.after_update(after_update) + assert sqlite_db(UpsertUser.name == "old").update(name="new") == 2 + assert seen == { + "types": ("UpdateSet", "UpdateSet"), + "narrowed": [first.id], + "called": [second.id], + "none": True, + } + + +# --- paths that normal use rarely or never reaches on the backends at hand --- + + +@pytest.mark.parametrize("upsert_db", ["postgres"], indirect=True) +def test_native_adapter_key_only_conflict_returns_unchanged_row(upsert_db: RecordingDAL) -> None: + # upsert() looks a key-only call up first; called directly, ON CONFLICT DO NOTHING returns no row + @upsert_db.define + class NativeMembership(TypedTable): + user_code: str + group_code: str + name = TypedField(str, default="default") + + NativeMembership.create_index("native_membership_unique", "user_code", "group_code", unique=True) + existing = NativeMembership.insert(user_code="u", group_code="g", name="existing") + table = NativeMembership._ensure_table_defined() + operation = table._fields_and_values_for_insert({"user_code": "u", "group_code": "g"}) + result = upsert_db._adapter.upsert(table, [table.user_code, table.group_code], operation.op_values(), []) + assert result.outcome == "unchanged" + assert result.row.id == existing.id + assert result.row.name == "existing" + + +def test_adapter_extensions(sqlite_db: RecordingDAL, tmp_path: Path) -> None: + # installed once per adapter: + original = sqlite_db._adapter.upsert + install_upsert(sqlite_db._adapter) + assert sqlite_db._adapter.upsert is original + + # dialects without native upsert refuse direct use: + table = UpsertUser._ensure_table_defined() + operation = table._fields_and_values_for_insert({"email": "a@example.com"}) + with pytest.raises(NotImplementedError, match="native upsert"): + sqlite_db._adapter._upsert(table, [table.email], operation.op_values(), []) + assert sqlite_db.statements == [] + + # PyDAL's null adapter has no SQL dialect to extend: + null_db = DAL(None, folder=str(tmp_path)) + try: + install_upsert(null_db._adapter) + install_update(null_db._adapter) + assert not hasattr(null_db._adapter, "upsert") + assert not hasattr(null_db._adapter, "update_with_ids") + finally: + null_db.close() + + +@pytest.mark.parametrize("branch", ["insert", "update"]) +def test_python_path_reports_trigger_deleted_row(sqlite_db: RecordingDAL, branch: str) -> None: + original = UpsertUser.insert(email="a@example.com", name="old") + events: list[str] = [] + UpsertUser.after_insert(lambda _row, _row_id: events.append("insert")) + UpsertUser.after_update(lambda _rows, _row: events.append("update")) + if branch == "insert": + sqlite_db.executesql( + 'CREATE TRIGGER remove_inserted AFTER INSERT ON "upsert_user" BEGIN ' + 'DELETE FROM "upsert_user" WHERE id = NEW.id; END;' + ) + with pytest.raises(RuntimeError, match="deleted concurrently"): + UpsertUser.upsert({"email": "other@example.com"}, name="new") + else: + sqlite_db.executesql( + 'CREATE TRIGGER remove_updated BEFORE UPDATE ON "upsert_user" BEGIN ' + 'DELETE FROM "upsert_user" WHERE id = OLD.id; SELECT RAISE(IGNORE); END;' + ) + with pytest.raises(RuntimeError, match="deleted concurrently"): + UpsertUser.upsert({"email": original.email}, name="new") + assert events == [] + + +@pytest.mark.parametrize("upsert_db", ["postgres"], indirect=True) +def test_native_path_reports_suppressed_insert(upsert_db: RecordingDAL) -> None: + upsert_db.executesql( + "CREATE FUNCTION suppress_upsert_insert() RETURNS trigger AS $$ BEGIN RETURN NULL; END; $$ LANGUAGE plpgsql;" + ) + try: + upsert_db.executesql( + 'CREATE TRIGGER suppress_insert BEFORE INSERT ON "upsert_user" ' + "FOR EACH ROW EXECUTE FUNCTION suppress_upsert_insert();" + ) + with pytest.raises(RuntimeError, match="deleted concurrently"): + UpsertUser.upsert({"email": "a@example.com"}, name="new") + finally: + upsert_db.executesql('DROP TRIGGER suppress_insert ON "upsert_user";') + upsert_db.executesql("DROP FUNCTION suppress_upsert_insert();") + + +def test_fallback_update_without_returning(sqlite_db: RecordingDAL) -> None: + # what MySQL and SQLite < 3.35 do, checked on SQLite for the cases MySQL doesn't reach + sqlite_db._adapter.dialect.update_returning_supported = False + ids: list[int] = [] + UpsertUser.after_update(lambda rows, _row: ids.extend(rows.affected_ids)) + + assert sqlite_db(UpsertUser.email == "missing@example.com").update(name="x") == 0 + original = UpsertUser.upsert({"email": "a@example.com"}, name="old") + updated = UpsertUser.upsert({"email": "a@example.com"}, name="new") + assert (updated.id, updated.name) == (original.id, "new") + assert ids == [original.id] + + # an unrestricted adapter update reports every updated row's key: + second = UpsertUser.insert(email="b@example.com") + table = UpsertUser._ensure_table_defined() + operation = table._fields_and_values_for_update({"name": "updated"}) + result = sqlite_db._adapter.update_with_ids(table, None, operation.op_values()) + assert result.count == 2 + assert set(result.ids) == {original.id, second.id} + + # restricting the update to the selected ids keeps a common filter bypass: + @sqlite_db.define(common_filter=lambda _query: sqlite_db.bypass_update.active == True) # noqa: E712 + class BypassUpdate(TypedTable): + active: bool + name: str + + row_id = BypassUpdate._ensure_table_defined().insert(active=False, name="hidden") + seen: list[str] = [] + BypassUpdate.after_update(lambda rows, _row: seen.extend(rows.select().column("name"))) + rows = sqlite_db(BypassUpdate.id == row_id, ignore_common_filters=True) + assert rows.update_ids(name="updated") == [row_id] + assert seen == ["updated"] + assert rows.select().first().name == "updated" + + +def test_update_without_returning_is_restricted_to_selected_rows(tmp_path: Path) -> None: + # simulate MySQL / SQLite < 3.35 on a file database, so a second connection can write in between + uri = f"sqlite://{tmp_path / 'race.sqlite'}" + db = RecordingDAL(uri, enable_typedal_caching=False, folder=str(tmp_path)) + other = DAL(uri, folder=str(tmp_path)) + db.statements = [] + + class RaceRow(TypedTable): + name: str + + try: + db.define(RaceRow) + db.commit() + other.define_table("race_row", Field("name"), migrate=False) + db._adapter.dialect.update_returning_supported = False + first = RaceRow.insert(name="old") + db.commit() + ids: list[int] = [] + RaceRow.after_update(lambda rows, _row: ids.extend(rows.affected_ids)) + + original_execute = db._adapter.execute + + def execute(sql: str, *args: t.Any, **kwargs: t.Any) -> t.Any: + if sql.startswith("UPDATE"): + # a concurrent writer adds a matching row between the SELECT and the UPDATE + other.race_row.insert(name="old") + other.commit() + return original_execute(sql, *args, **kwargs) + + db._adapter.execute = execute + assert db(RaceRow.name == "old").update(name="new") == 1 + db._adapter.execute = original_execute + assert ids == [first.id] + assert RaceRow.where(name="old").count() == 1 # the late row was not touched + finally: + db.rollback() + db.close() + other.close() + + +@pytest.mark.parametrize("handle_error", [False, True]) +@pytest.mark.parametrize("need_ids", [False, True]) +def test_update_returning_preserves_error_callback(sqlite_db: RecordingDAL, handle_error: bool, need_ids: bool) -> None: + """Constraint failures call the configured handler or propagate without after-hooks.""" + original = UpsertUser.insert(email="a@example.com") + UpsertUser.insert(email="taken@example.com") + table = UpsertUser._ensure_table_defined() + events: list[str] = [] + UpsertUser.after_update(lambda _rows, _row: events.append("after")) + rows = sqlite_db(UpsertUser.id == original.id) + + def on_error(error_table: t.Any, query: t.Any, fields: t.Any, error: Exception) -> int: + assert error_table is table + assert query is rows.query + assert fields == [(table.email, "taken@example.com")] + assert isinstance(error, sqlite_db._adapter.driver.IntegrityError) + events.append("error") + return 0 + + if handle_error: + table._on_update_error = on_error + write = rows.update_ids if need_ids else rows.update + if handle_error: + assert write(email="taken@example.com") == ([] if need_ids else 0) + assert events == ["error"] + else: + with pytest.raises(sqlite_db._adapter.driver.IntegrityError): + write(email="taken@example.com") + assert events == [] + assert UpsertUser(original.id).email == "a@example.com" + + +class CachedUpsert(TypedTable): + """Module level: cached rows are pickled, which needs an importable class.""" + + code = TypedField(str, unique=True) + status: str + + +def test_upsert_invalidates_cache(tmp_path: Path) -> None: + # the shared fixtures disable caching; this one needs it + db = TypeDAL("sqlite:memory", enable_typedal_caching=True, folder=str(tmp_path)) + try: + db.define(CachedUpsert) + first = CachedUpsert.upsert({"code": "a"}, status="draft") + drafts = CachedUpsert.where(status="draft").cache() + everything = CachedUpsert.where(CachedUpsert.id > 0).cache() + assert [row.id for row in drafts.collect()] == [first.id] + assert len(everything.collect()) == 1 + assert drafts.collect().metadata["cache"]["status"] == "cached" + assert everything.collect().metadata["cache"]["status"] == "cached" + + # update branch that moves the row out of the cached predicate: + updated = CachedUpsert.upsert({"code": "a"}, status="published") + assert updated.id == first.id + refreshed = drafts.collect() + assert refreshed.metadata["cache"]["status"] == "fresh" + assert not refreshed + + # insert branch: + CachedUpsert.upsert({"code": "b"}, status="draft") + assert everything.collect().metadata["cache"]["status"] == "fresh" + assert len(everything.collect()) == 2 + finally: + db.close() + CachedUpsert.unbind()