From d436f54643f19f4b1fbe4d7428dd4ad8fd0a75ba Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 11:33:49 +0100 Subject: [PATCH] switch pool for sqlite with new factory, add tests if fks are reflected in db --- docs/releases.md | 24 +++++++++++++ ormar/fields/model_fields.py | 2 +- ormar/models/helpers/models.py | 53 ++++++++++++++++++++++++++-- ormar/models/metaclass.py | 2 ++ ormar/models/model.py | 4 +++ ormar/queryset/queryset.py | 1 - tests/test_cascades.py | 44 ++++++------------------ tests/test_database_fk_creation.py | 55 ++++++++++++++++++++++++++++++ tests/test_model_definition.py | 6 ++++ 9 files changed, 154 insertions(+), 37 deletions(-) create mode 100644 tests/test_database_fk_creation.py diff --git a/docs/releases.md b/docs/releases.md index 14ed389..fb7f0e2 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1,3 +1,27 @@ +# 0.9.0 + +## Important +* **Braking Fix:** Version 0.8.0 introduced a bug that prevents generation of foreign_keys constraint in the database, +both in alembic and during creation through sqlalchemy.engine, this is fixed now. +* **THEREFORE IF YOU USE VERSION >=0.8.0 YOU ARE STRONGLY ADVISED TO UPDATE** cause despite +that most of the `ormar` functions are working your database **CREATED with ormar (or ormar + alembic)** + does not have relations and suffer from perspective of performance and data integrity. +* If you were using `ormar` to connect to existing database your performance and integrity + should be fine nevertheless you should update to reflect all future schema updates in your models. + + +## Breaking +* **Breaking:** All foreign_keys and unique constraints now have a name so `alembic` + can identify them in db and not depend on db +* **Breaking:** During model construction if `Meta` class of the `Model` does not + include `metadata` or `database` now `ModelDefinitionError` will be raised instead of generic `AttributeError`. +* **Breaking:** `encode/databases` used for running the queries does not have a connection pool +for sqlite backend, meaning that each querry is run with a new connection and there is no way to + enable enforcing ForeignKeys constraints as those are by default turned off on every connection. + This is changed in `ormar` since >=0.9.0 and by default each sqlite3 query has `"PRAGMA foreign_keys=1;"` + run so now each sqlite3 connection by default enforces ForeignKey constraints including cascades. + + # 0.8.1 ## Features diff --git a/ormar/fields/model_fields.py b/ormar/fields/model_fields.py index 3460e58..2222b78 100644 --- a/ormar/fields/model_fields.py +++ b/ormar/fields/model_fields.py @@ -214,7 +214,7 @@ class Integer(ModelFieldFactory, int): :return: initialized column with proper options :rtype: sqlalchemy Column """ - return sqlalchemy.Integer + return sqlalchemy.Integer() class Text(ModelFieldFactory, str): diff --git a/ormar/models/helpers/models.py b/ormar/models/helpers/models.py index 727bdd7..1cf728a 100644 --- a/ormar/models/helpers/models.py +++ b/ormar/models/helpers/models.py @@ -1,10 +1,11 @@ import itertools +import sqlite3 from typing import Any, Dict, List, Optional, TYPE_CHECKING, Tuple, Type +from pydantic.typing import ForwardRef import ormar # noqa: I100 from ormar.fields.foreign_key import ForeignKeyField from ormar.models.helpers.pydantic import populate_pydantic_default_values -from pydantic.typing import ForwardRef if TYPE_CHECKING: # pragma no cover from ormar import Model @@ -41,7 +42,7 @@ def populate_default_options_values( :param new_model: newly constructed Model :type new_model: Model class - :param model_fields: + :param model_fields: dict of model fields :type model_fields: Union[Dict[str, type], Dict] """ if not hasattr(new_model.Meta, "constraints"): @@ -59,6 +60,54 @@ def populate_default_options_values( new_model.Meta.requires_ref_update = False +class Connection(sqlite3.Connection): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.execute("PRAGMA foreign_keys=1;") + + +def substitue_backend_pool_for_sqlite(new_model: Type["Model"]) -> None: + """ + Recreates Connection pool for sqlite3 with new factory that + executes "PRAGMA foreign_keys=1; on initialization to enable foreign keys. + + :param new_model: newly declared ormar Model + :type new_model: Model class + """ + backend = new_model.Meta.database._backend + if ( + backend._dialect.name == "sqlite" and "factory" not in backend._options + ): # pragma: no cover + backend._options["factory"] = Connection + old_pool = backend._pool + backend._pool = old_pool.__class__(backend._database_url, **backend._options) + + +def check_required_meta_parameters(new_model: Type["Model"]) -> None: + """ + Verifies if ormar.Model has database and metadata set. + + Recreates Connection pool for sqlite3 + + :param new_model: newly declared ormar Model + :type new_model: Model class + """ + if not hasattr(new_model.Meta, "database"): + if not getattr(new_model.Meta, "abstract", False): + raise ormar.ModelDefinitionError( + f"{new_model.__name__} does not have database defined." + ) + + else: + substitue_backend_pool_for_sqlite(new_model=new_model) + + if not hasattr(new_model.Meta, "metadata"): + if not getattr(new_model.Meta, "abstract", False): + raise ormar.ModelDefinitionError( + f"{new_model.__name__} does not have metadata defined." + ) + + def extract_annotations_and_default_vals(attrs: Dict) -> Tuple[Dict, Dict]: """ Extracts annotations from class namespace dict and triggers diff --git a/ormar/models/metaclass.py b/ormar/models/metaclass.py index ddb752e..2d0bb55 100644 --- a/ormar/models/metaclass.py +++ b/ormar/models/metaclass.py @@ -33,6 +33,7 @@ from ormar.models.helpers import ( populate_meta_tablename_columns_and_pk, register_relation_in_alias_manager, ) +from ormar.models.helpers.models import check_required_meta_parameters from ormar.models.helpers.sqlalchemy import sqlalchemy_columns_from_model_fields from ormar.models.quick_access_views import quick_access_set from ormar.queryset import QuerySet @@ -582,6 +583,7 @@ class ModelMetaclass(pydantic.main.ModelMetaclass): if hasattr(new_model, "Meta"): populate_default_options_values(new_model, model_fields) + check_required_meta_parameters(new_model) add_property_fields(new_model, attrs) register_signals(new_model=new_model) populate_choices_validators(new_model) diff --git a/ormar/models/model.py b/ormar/models/model.py index 4f1b0e3..3c66a0a 100644 --- a/ormar/models/model.py +++ b/ormar/models/model.py @@ -62,6 +62,10 @@ class Model(NewBaseModel): where rows are populated in a different way as they do not have nested models in result. + :param current_relation_str: name of the relation field + :type current_relation_str: str + :param source_model: model on which relation was defined + :type source_model: Type[Model] :param row: raw result row from the database :type row: sqlalchemy.engine.result.ResultProxy :param select_related: list of names of related models fetched from database diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 927e76b..8a03866 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -590,7 +590,6 @@ class QuerySet: expr = FilterQuery(filter_clauses=self.filter_clauses).apply( self.table.delete() ) - print("\n", expr.compile(compile_kwargs={"literal_binds": True})) return await self.database.execute(expr) def paginate(self, page: int, page_size: int = 20) -> "QuerySet": diff --git a/tests/test_cascades.py b/tests/test_cascades.py index 5ac1bcd..d1c0e6c 100644 --- a/tests/test_cascades.py +++ b/tests/test_cascades.py @@ -1,27 +1,14 @@ -import time from typing import Optional import databases import pytest import sqlalchemy -from sqlalchemy import ( - Column, - ForeignKey, - ForeignKeyConstraint, - Integer, - String, - Table, - create_engine, - inspect, -) -from sqlalchemy.dialects import postgresql import ormar from tests.settings import DATABASE_URL database = databases.Database(DATABASE_URL) metadata = sqlalchemy.MetaData() -engine = sqlalchemy.create_engine(DATABASE_URL, echo=True) class Artist(ormar.Model): @@ -45,43 +32,34 @@ class Album(ormar.Model): artist: Optional[Artist] = ormar.ForeignKey(Artist, ondelete="CASCADE") -# -# class Track(ormar.Model): -# class Meta: -# tablename = "tracks" -# metadata = metadata -# database = database -# -# id: int = ormar.Integer(primary_key=True) -# album: Optional[Album] = ormar.ForeignKey(Album, ondelete='CASCADE') -# title: str = ormar.String(max_length=100) +class Track(ormar.Model): + class Meta: + tablename = "tracks" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + album: Optional[Album] = ormar.ForeignKey(Album, ondelete="CASCADE") + title: str = ormar.String(max_length=100) @pytest.fixture(autouse=True, scope="module") def create_test_database(): - # if "sqlite" in DATABASE_URL: - # with engine.connect() as connection: - # connection.execute("PRAGMA foreign_keys = ON;") + engine = sqlalchemy.create_engine(DATABASE_URL) metadata.drop_all(engine) metadata.create_all(engine) - # tables = list(metadata.tables.values()) - # tab = Album.Meta.table - # breakpoint() yield - # metadata.drop_all(engine) + metadata.drop_all(engine) @pytest.mark.asyncio async def test_simple_cascade(): async with database: - # async with database.transaction(force_rollback=True): artist = await Artist(name="Dr Alban").save() await Album(name="Jamaica", artist=artist).save() await Artist.objects.delete(id=artist.id) - artists = await Artist.objects.all() assert len(artists) == 0 - async with database: albums = await Album.objects.all() assert len(albums) == 0 diff --git a/tests/test_database_fk_creation.py b/tests/test_database_fk_creation.py new file mode 100644 index 0000000..4d7cfc2 --- /dev/null +++ b/tests/test_database_fk_creation.py @@ -0,0 +1,55 @@ +from typing import Optional + +import databases +import pytest +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL) +metadata = sqlalchemy.MetaData() +engine = sqlalchemy.create_engine(DATABASE_URL) + + +class Artist(ormar.Model): + class Meta: + tablename = "artists" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Album(ormar.Model): + class Meta: + tablename = "albums" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + artist: Optional[Artist] = ormar.ForeignKey(Artist, ondelete="CASCADE") + + +@pytest.fixture(autouse=True, scope="module") +def create_test_database(): + metadata.drop_all(engine) + metadata.create_all(engine) + yield + metadata.drop_all(engine) + + +def test_simple_cascade(): + inspector = sqlalchemy.inspect(engine) + columns = inspector.get_columns("albums") + assert len(columns) == 3 + col_names = [col.get("name") for col in columns] + assert sorted(["id", "name", "artist"]) == sorted(col_names) + fks = inspector.get_foreign_keys("albums") + assert len(fks) == 1 + assert fks[0]["name"] == "fk_albums_artists_id_artist" + assert fks[0]["constrained_columns"][0] == "artist" + assert fks[0]["referred_columns"][0] == "id" + assert fks[0]["options"].get("ondelete") == "CASCADE" diff --git a/tests/test_model_definition.py b/tests/test_model_definition.py index 5f6d360..aaf6b66 100644 --- a/tests/test_model_definition.py +++ b/tests/test_model_definition.py @@ -3,6 +3,7 @@ import asyncio import datetime import decimal +import databases import pydantic import pytest import sqlalchemy @@ -14,12 +15,16 @@ from ormar.models import Model from tests.settings import DATABASE_URL metadata = sqlalchemy.MetaData() +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL, force_rollback=True) class ExampleModel(Model): class Meta: tablename = "example" metadata = metadata + database = database test: int = ormar.Integer(primary_key=True) test_string: str = ormar.String(max_length=250) @@ -52,6 +57,7 @@ class ExampleModel2(Model): class Meta: tablename = "examples" metadata = metadata + database = database test: int = ormar.Integer(primary_key=True) test_string: str = ormar.String(max_length=250)