From 04bbe13c09799cea8193439a4bd44c8bb7b14bee Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 1 Feb 2021 11:30:59 +0100 Subject: [PATCH 1/9] test cascade --- .github/workflows/test-package.yml | 8 ++-- tests/test_cascades.py | 73 ++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 tests/test_cascades.py diff --git a/.github/workflows/test-package.yml b/.github/workflows/test-package.yml index 5b0ce2f..4d92a32 100644 --- a/.github/workflows/test-package.yml +++ b/.github/workflows/test-package.yml @@ -51,10 +51,6 @@ jobs: run: | python -m pip install --upgrade pip if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Run sqlite - env: - DATABASE_URL: "sqlite:///testsuite" - run: bash scripts/test.sh - name: Run postgres env: DATABASE_URL: "postgresql://username:password@localhost:5432/testsuite" @@ -63,6 +59,10 @@ jobs: env: DATABASE_URL: "mysql://username:password@127.0.0.1:3306/testsuite" run: bash scripts/test.sh + - name: Run sqlite + env: + DATABASE_URL: "sqlite:///testsuite" + run: bash scripts/test.sh - run: mypy --config-file mypy.ini ormar tests - name: Upload coverage uses: codecov/codecov-action@v1 diff --git a/tests/test_cascades.py b/tests/test_cascades.py new file mode 100644 index 0000000..c99c3a5 --- /dev/null +++ b/tests/test_cascades.py @@ -0,0 +1,73 @@ +from typing import Optional + +import databases +import pytest +import sqlalchemy +from sqlalchemy import create_engine +from sqlalchemy.dialects import postgresql + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL, force_rollback=True) +metadata = sqlalchemy.MetaData() + + +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') + + +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(): + engine = sqlalchemy.create_engine(DATABASE_URL) + if "sqlite" in DATABASE_URL: + with engine.connect() as connection: + connection.execute("PRAGMA foreign_keys = ON;") + metadata.drop_all(engine) + metadata.create_all(engine) + yield + # metadata.drop_all(engine) + + +# def test_table_structures(): +# col = Album.Meta.table.columns.get('artist') +# breakpoint() + + +@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) + albums = await Album.objects.all() + assert len(albums) == 0 From 5a4c42b318588ca3dea41bfd941171717b173ed8 Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 1 Feb 2021 13:09:01 +0100 Subject: [PATCH 2/9] test wtf --- ormar/models/helpers/sqlalchemy.py | 3 ++- tests/test_cascades.py | 15 ++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ormar/models/helpers/sqlalchemy.py b/ormar/models/helpers/sqlalchemy.py index 1b8c99e..d264cf3 100644 --- a/ormar/models/helpers/sqlalchemy.py +++ b/ormar/models/helpers/sqlalchemy.py @@ -234,12 +234,13 @@ def populate_meta_sqlalchemy_table_if_required(meta: "ModelMeta") -> None: if not hasattr(meta, "table") and check_for_null_type_columns_from_forward_refs( meta ): - meta.table = sqlalchemy.Table( + table = sqlalchemy.Table( meta.tablename, meta.metadata, *[copy.deepcopy(col) for col in meta.columns], *meta.constraints, ) + meta.table = table def update_column_definition( diff --git a/tests/test_cascades.py b/tests/test_cascades.py index c99c3a5..b94cc8f 100644 --- a/tests/test_cascades.py +++ b/tests/test_cascades.py @@ -3,7 +3,7 @@ from typing import Optional import databases import pytest import sqlalchemy -from sqlalchemy import create_engine +from sqlalchemy import ForeignKeyConstraint, create_engine, inspect from sqlalchemy.dialects import postgresql import ormar @@ -11,6 +11,7 @@ from tests.settings import DATABASE_URL database = databases.Database(DATABASE_URL, force_rollback=True) metadata = sqlalchemy.MetaData() +engine = sqlalchemy.create_engine(DATABASE_URL, echo=True) class Artist(ormar.Model): @@ -28,6 +29,7 @@ class Album(ormar.Model): tablename = "albums" metadata = metadata database = database + constraint = [ForeignKeyConstraint(['albums'],['albums.id'])] id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100) @@ -47,20 +49,19 @@ class Track(ormar.Model): @pytest.fixture(autouse=True, scope="module") def create_test_database(): - engine = sqlalchemy.create_engine(DATABASE_URL) if "sqlite" in DATABASE_URL: with engine.connect() as connection: connection.execute("PRAGMA foreign_keys = ON;") metadata.drop_all(engine) metadata.create_all(engine) yield - # metadata.drop_all(engine) + metadata.drop_all(engine) -# def test_table_structures(): -# col = Album.Meta.table.columns.get('artist') -# breakpoint() - +def test_table_structures(): + col = Album.Meta.table.columns.get('artist') + inspector = inspect(engine) + col2 = inspector.get_columns('albums') @pytest.mark.asyncio async def test_simple_cascade(): From 31e15bd7ba6ecf1ed9c830ed524ae5cc56b4f774 Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 1 Feb 2021 14:53:34 +0100 Subject: [PATCH 3/9] test other fk creation --- ormar/models/helpers/sqlalchemy.py | 7 +++++++ tests/test_cascades.py | 24 +++++++++++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/ormar/models/helpers/sqlalchemy.py b/ormar/models/helpers/sqlalchemy.py index d264cf3..6373813 100644 --- a/ormar/models/helpers/sqlalchemy.py +++ b/ormar/models/helpers/sqlalchemy.py @@ -3,6 +3,7 @@ import logging from typing import Dict, List, Optional, TYPE_CHECKING, Tuple, Type, Union import sqlalchemy +from sqlalchemy import ForeignKeyConstraint from ormar import ForeignKey, Integer, ModelDefinitionError # noqa: I202 from ormar.fields import BaseField, ManyToManyField @@ -234,12 +235,18 @@ def populate_meta_sqlalchemy_table_if_required(meta: "ModelMeta") -> None: if not hasattr(meta, "table") and check_for_null_type_columns_from_forward_refs( meta ): + if meta.tablename == 'albums': + meta.constraints.append(ForeignKeyConstraint(['artist'],['artists.id'], + ondelete='CASCADE', + onupdate='CASCADE')) table = sqlalchemy.Table( meta.tablename, meta.metadata, *[copy.deepcopy(col) for col in meta.columns], *meta.constraints, ) + if meta.tablename == 'albums': + pass meta.table = table diff --git a/tests/test_cascades.py b/tests/test_cascades.py index b94cc8f..661b7ec 100644 --- a/tests/test_cascades.py +++ b/tests/test_cascades.py @@ -29,7 +29,7 @@ class Album(ormar.Model): tablename = "albums" metadata = metadata database = database - constraint = [ForeignKeyConstraint(['albums'],['albums.id'])] + constraint = [] id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100) @@ -55,20 +55,18 @@ def create_test_database(): metadata.drop_all(engine) metadata.create_all(engine) yield - metadata.drop_all(engine) + # metadata.drop_all(engine) -def test_table_structures(): - col = Album.Meta.table.columns.get('artist') - inspector = inspect(engine) - col2 = inspector.get_columns('albums') - @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) - albums = await Album.objects.all() - assert len(albums) == 0 + # 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 + # breakpoint() + albums = await Album.objects.all() + assert len(albums) == 0 From 54a4218f1d28371858fb6e1ff4c06205cd753fb7 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 09:33:45 +0100 Subject: [PATCH 4/9] check other backends trial1 --- ormar/fields/base.py | 14 +++++--- ormar/fields/foreign_key.py | 11 ++++--- ormar/fields/model_fields.py | 2 +- ormar/models/helpers/sqlalchemy.py | 23 ++++++-------- ormar/models/metaclass.py | 12 +++++-- ormar/queryset/queryset.py | 1 + tests/test_cascades.py | 51 +++++++++++++++++++----------- 7 files changed, 71 insertions(+), 43 deletions(-) diff --git a/ormar/fields/base.py b/ormar/fields/base.py index 4f05aa1..d6c115d 100644 --- a/ormar/fields/base.py +++ b/ormar/fields/base.py @@ -211,12 +211,17 @@ class BaseField(FieldInfo): :return: List of sqlalchemy foreign keys - by default one. :rtype: List[sqlalchemy.schema.ForeignKey] """ - return [ - sqlalchemy.schema.ForeignKey( - con.name, ondelete=con.ondelete, onupdate=con.onupdate + constraints = [ + sqlalchemy.ForeignKey( + con.reference, + ondelete=con.ondelete, + onupdate=con.onupdate, + name=f"fk_{cls.owner.Meta.tablename}_{cls.to.Meta.tablename}" + f"_{cls.to.get_column_alias(cls.to.Meta.pkname)}_{cls.name}", ) for con in cls.constraints ] + return constraints @classmethod def get_column(cls, name: str) -> sqlalchemy.Column: @@ -230,7 +235,7 @@ class BaseField(FieldInfo): :return: actual definition of the database column as sqlalchemy requires. :rtype: sqlalchemy.Column """ - return sqlalchemy.Column( + column = sqlalchemy.Column( cls.alias or name, cls.column_type, *cls.construct_constraints(), @@ -241,6 +246,7 @@ class BaseField(FieldInfo): default=cls.default, server_default=cls.server_default, ) + return column @classmethod def expand_relationship( diff --git a/ormar/fields/foreign_key.py b/ormar/fields/foreign_key.py index f76c8e9..baaf2ae 100644 --- a/ormar/fields/foreign_key.py +++ b/ormar/fields/foreign_key.py @@ -3,6 +3,7 @@ import uuid from dataclasses import dataclass from typing import Any, List, Optional, TYPE_CHECKING, Tuple, Type, Union +import sqlalchemy from pydantic import BaseModel, create_model from pydantic.typing import ForwardRef, evaluate_forwardref from sqlalchemy import UniqueConstraint @@ -94,6 +95,7 @@ def populate_fk_params_based_on_to_model( :rtype: Tuple[Any, List, Any] """ fk_string = to.Meta.tablename + "." + to.get_column_alias(to.Meta.pkname) + to.Meta.tablename + "." + to.get_column_alias(to.Meta.pkname) to_field = to.Meta.model_fields[to.Meta.pkname] pk_only_model = create_dummy_model(to, to_field) __type__ = ( @@ -103,7 +105,7 @@ def populate_fk_params_based_on_to_model( ) constraints = [ ForeignKeyConstraint( - name=fk_string, ondelete=ondelete, onupdate=onupdate # type: ignore + reference=fk_string, ondelete=ondelete, onupdate=onupdate, name=None ) ] column_type = to_field.column_type @@ -124,9 +126,10 @@ class ForeignKeyConstraint: to produce sqlalchemy.ForeignKeys """ - name: str - ondelete: str - onupdate: str + reference: Union[str, sqlalchemy.Column] + name: Optional[str] + ondelete: Optional[str] + onupdate: Optional[str] def ForeignKey( # noqa CFQ002 diff --git a/ormar/fields/model_fields.py b/ormar/fields/model_fields.py index 2222b78..3460e58 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/sqlalchemy.py b/ormar/models/helpers/sqlalchemy.py index 6373813..ed4a605 100644 --- a/ormar/models/helpers/sqlalchemy.py +++ b/ormar/models/helpers/sqlalchemy.py @@ -1,9 +1,7 @@ -import copy import logging from typing import Dict, List, Optional, TYPE_CHECKING, Tuple, Type, Union import sqlalchemy -from sqlalchemy import ForeignKeyConstraint from ormar import ForeignKey, Integer, ModelDefinitionError # noqa: I202 from ormar.fields import BaseField, ManyToManyField @@ -81,10 +79,12 @@ def create_and_append_m2m_fk( model.Meta.tablename + "." + pk_alias, ondelete="CASCADE", onupdate="CASCADE", + name=f"fk_{model_field.through.Meta.tablename}_{model.Meta.tablename}" + f"_{field_name}_{pk_alias}", ), ) model_field.through.Meta.columns.append(column) - model_field.through.Meta.table.append_column(copy.deepcopy(column)) + model_field.through.Meta.table.append_column(column) def check_pk_column_validity( @@ -235,18 +235,15 @@ def populate_meta_sqlalchemy_table_if_required(meta: "ModelMeta") -> None: if not hasattr(meta, "table") and check_for_null_type_columns_from_forward_refs( meta ): - if meta.tablename == 'albums': - meta.constraints.append(ForeignKeyConstraint(['artist'],['artists.id'], - ondelete='CASCADE', - onupdate='CASCADE')) + for constraint in meta.constraints: + if isinstance(constraint, sqlalchemy.UniqueConstraint): + constraint.name = ( + f"uc_{meta.tablename}_" + f'{"_".join([str(col) for col in constraint._pending_colargs])}' + ) table = sqlalchemy.Table( - meta.tablename, - meta.metadata, - *[copy.deepcopy(col) for col in meta.columns], - *meta.constraints, + meta.tablename, meta.metadata, *meta.columns, *meta.constraints, ) - if meta.tablename == 'albums': - pass meta.table = table diff --git a/ormar/models/metaclass.py b/ormar/models/metaclass.py index 030b05b..cee13a9 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.sqlalchemy import sqlalchemy_columns_from_model_fields from ormar.models.quick_access_views import quick_access_set from ormar.queryset import QuerySet from ormar.relations.alias_manager import AliasManager @@ -348,20 +349,25 @@ def copy_and_replace_m2m_through_model( new_meta: ormar.ModelMeta = type( # type: ignore "Meta", (), dict(through_class.Meta.__dict__), ) + copy_name = through_class.__name__ + attrs.get("__name__", "") + copy_through = type(copy_name, (ormar.Model,), {"Meta": new_meta}) new_meta.tablename += "_" + meta.tablename # create new table with copied columns but remove foreign keys # they will be populated later in expanding reverse relation if hasattr(new_meta, "table"): del new_meta.table - new_meta.columns = [col for col in new_meta.columns if not col.foreign_keys] new_meta.model_fields = { name: field for name, field in new_meta.model_fields.items() if not issubclass(field, ForeignKeyField) } + if hasattr(new_meta, "column"): + del new_meta.columns + _, columns = sqlalchemy_columns_from_model_fields( + new_meta.model_fields, copy_through + ) # type: ignore + new_meta.columns = columns populate_meta_sqlalchemy_table_if_required(new_meta) - copy_name = through_class.__name__ + attrs.get("__name__", "") - copy_through = type(copy_name, (ormar.Model,), {"Meta": new_meta}) copy_field.through = copy_through parent_fields[field_name] = copy_field diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 8a03866..927e76b 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -590,6 +590,7 @@ 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 661b7ec..5ac1bcd 100644 --- a/tests/test_cascades.py +++ b/tests/test_cascades.py @@ -1,15 +1,25 @@ +import time from typing import Optional import databases import pytest import sqlalchemy -from sqlalchemy import ForeignKeyConstraint, create_engine, inspect +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, force_rollback=True) +database = databases.Database(DATABASE_URL) metadata = sqlalchemy.MetaData() engine = sqlalchemy.create_engine(DATABASE_URL, echo=True) @@ -29,31 +39,34 @@ class Album(ormar.Model): tablename = "albums" metadata = metadata database = database - constraint = [] id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100) - artist: Optional[Artist] = ormar.ForeignKey(Artist, ondelete='CASCADE') + 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;") + # if "sqlite" in DATABASE_URL: + # with engine.connect() as connection: + # connection.execute("PRAGMA foreign_keys = ON;") metadata.drop_all(engine) metadata.create_all(engine) + # tables = list(metadata.tables.values()) + # tab = Album.Meta.table + # breakpoint() yield # metadata.drop_all(engine) @@ -62,11 +75,13 @@ def create_test_database(): async def test_simple_cascade(): async with database: # async with database.transaction(force_rollback=True): - artist = await Artist(name='Dr Alban').save() + 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 - # breakpoint() + + async with database: albums = await Album.objects.all() assert len(albums) == 0 From aea6200bfd891fd13189ccf9410853c9db246775 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 09:37:19 +0100 Subject: [PATCH 5/9] fix coverage --- ormar/models/metaclass.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ormar/models/metaclass.py b/ormar/models/metaclass.py index cee13a9..ddb752e 100644 --- a/ormar/models/metaclass.py +++ b/ormar/models/metaclass.py @@ -361,8 +361,6 @@ def copy_and_replace_m2m_through_model( for name, field in new_meta.model_fields.items() if not issubclass(field, ForeignKeyField) } - if hasattr(new_meta, "column"): - del new_meta.columns _, columns = sqlalchemy_columns_from_model_fields( new_meta.model_fields, copy_through ) # type: ignore From d436f54643f19f4b1fbe4d7428dd4ad8fd0a75ba Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 11:33:49 +0100 Subject: [PATCH 6/9] 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) From 1cca385b5f2f932596bb8831261bdf9c96780e65 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 11:57:37 +0100 Subject: [PATCH 7/9] fix coverage, bump version, update relaese, update docs --- docs/api/models/helpers/models.md | 31 ++++++++++- docs/api/models/model.md | 2 + docs/releases.md | 4 ++ ormar/__init__.py | 2 +- tests/test_cascades.py | 93 ++++++++++++++++++++++++++++++- tests/test_model_definition.py | 30 +++++++++- 6 files changed, 158 insertions(+), 4 deletions(-) diff --git a/docs/api/models/helpers/models.md b/docs/api/models/helpers/models.md index ee3b100..b3100c6 100644 --- a/docs/api/models/helpers/models.md +++ b/docs/api/models/helpers/models.md @@ -38,7 +38,36 @@ Current options are: **Arguments**: - `new_model (Model class)`: newly constructed Model -- `model_fields (Union[Dict[str, type], Dict])`: +- `model_fields (Union[Dict[str, type], Dict])`: dict of model fields + + +#### substitue\_backend\_pool\_for\_sqlite + +```python +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. + +**Arguments**: + +- `new_model (Model class)`: newly declared ormar Model + + +#### check\_required\_meta\_parameters + +```python +check_required_meta_parameters(new_model: Type["Model"]) -> None +``` + +Verifies if ormar.Model has database and metadata set. + +Recreates Connection pool for sqlite3 + +**Arguments**: + +- `new_model (Model class)`: newly declared ormar Model #### extract\_annotations\_and\_default\_vals diff --git a/docs/api/models/model.md b/docs/api/models/model.md index 9bf3a90..e78825f 100644 --- a/docs/api/models/model.md +++ b/docs/api/models/model.md @@ -30,6 +30,8 @@ nested models in result. **Arguments**: +- `current_relation_str (str)`: name of the relation field +- `source_model (Type[Model])`: model on which relation was defined - `row (sqlalchemy.engine.result.ResultProxy)`: raw result row from the database - `select_related (List)`: list of names of related models fetched from database - `related_models (Union[List, Dict])`: list or dict of related models diff --git a/docs/releases.md b/docs/releases.md index fb7f0e2..457a048 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -21,6 +21,10 @@ for sqlite backend, meaning that each querry is run with a new connection and th 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. +## Other + +* Update api docs. +* Add tests for fk creation in db and for cascades in db # 0.8.1 diff --git a/ormar/__init__.py b/ormar/__init__.py index 40da69b..0c84957 100644 --- a/ormar/__init__.py +++ b/ormar/__init__.py @@ -65,7 +65,7 @@ class UndefinedType: # pragma no cover Undefined = UndefinedType() -__version__ = "0.8.1" +__version__ = "0.9.0" __all__ = [ "Integer", "BigInteger", diff --git a/tests/test_cascades.py b/tests/test_cascades.py index d1c0e6c..c1e772b 100644 --- a/tests/test_cascades.py +++ b/tests/test_cascades.py @@ -11,6 +11,25 @@ database = databases.Database(DATABASE_URL) metadata = sqlalchemy.MetaData() +class Band(ormar.Model): + class Meta: + tablename = "bands" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class ArtistsBands(ormar.Model): + class Meta: + tablename = "artists_x_bands" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + + class Artist(ormar.Model): class Meta: tablename = "artists" @@ -19,6 +38,7 @@ class Artist(ormar.Model): id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100) + bands = ormar.ManyToMany(Band, through=ArtistsBands) class Album(ormar.Model): @@ -52,8 +72,16 @@ def create_test_database(): metadata.drop_all(engine) +@pytest.fixture(scope="function") +async def cleanup(): + yield + async with database: + await Band.objects.delete(each=True) + await Artist.objects.delete(each=True) + + @pytest.mark.asyncio -async def test_simple_cascade(): +async def test_simple_cascade(cleanup): async with database: artist = await Artist(name="Dr Alban").save() await Album(name="Jamaica", artist=artist).save() @@ -63,3 +91,66 @@ async def test_simple_cascade(): albums = await Album.objects.all() assert len(albums) == 0 + + +@pytest.mark.asyncio +async def test_nested_cascade(cleanup): + async with database: + artist = await Artist(name="Dr Alban").save() + album = await Album(name="Jamaica", artist=artist).save() + await Track(title="Yuhu", album=album).save() + + await Artist.objects.delete(id=artist.id) + + artists = await Artist.objects.all() + assert len(artists) == 0 + + albums = await Album.objects.all() + assert len(albums) == 0 + + tracks = await Track.objects.all() + assert len(tracks) == 0 + + +@pytest.mark.asyncio +async def test_many_to_many_cascade(cleanup): + async with database: + artist = await Artist(name="Dr Alban").save() + band = await Band(name="Scorpions").save() + await artist.bands.add(band) + + check = await Artist.objects.select_related("bands").get() + assert check.bands[0].name == "Scorpions" + + await Artist.objects.delete(id=artist.id) + + artists = await Artist.objects.all() + assert len(artists) == 0 + + bands = await Band.objects.all() + assert len(bands) == 1 + + connections = await ArtistsBands.objects.all() + assert len(connections) == 0 + + +@pytest.mark.asyncio +async def test_reverse_many_to_many_cascade(cleanup): + async with database: + artist = await Artist(name="Dr Alban").save() + band = await Band(name="Scorpions").save() + await artist.bands.add(band) + + check = await Artist.objects.select_related("bands").get() + assert check.bands[0].name == "Scorpions" + + await Band.objects.delete(id=band.id) + + artists = await Artist.objects.all() + assert len(artists) == 1 + + connections = await ArtistsBands.objects.all() + assert len(connections) == 0 + + bands = await Band.objects.all() + assert len(bands) == 0 diff --git a/tests/test_model_definition.py b/tests/test_model_definition.py index aaf6b66..9db3761 100644 --- a/tests/test_model_definition.py +++ b/tests/test_model_definition.py @@ -118,6 +118,29 @@ def test_model_attribute_json_access(example): assert example.test_json == dict(aa=12) +def test_missing_metadata(): + with pytest.raises(ModelDefinitionError): + + class JsonSample2(ormar.Model): + class Meta: + tablename = "jsons2" + database = database + + id: int = ormar.Integer(primary_key=True) + test_json = ormar.JSON(nullable=True) + + +def test_missing_database(): + with pytest.raises(ModelDefinitionError): + + class JsonSample3(ormar.Model): + class Meta: + tablename = "jsons3" + + id: int = ormar.Integer(primary_key=True) + test_json = ormar.JSON(nullable=True) + + def test_non_existing_attr(example): with pytest.raises(ValueError): example.new_attr = 12 @@ -143,12 +166,13 @@ def test_sqlalchemy_table_is_created(example): @typing.no_type_check -def test_no_pk_in_model_definition(): # type: ignore +def test_no_pk_in_model_definition(): with pytest.raises(ModelDefinitionError): # type: ignore class ExampleModel2(Model): # type: ignore class Meta: tablename = "example2" + database = database metadata = metadata test_string: str = ormar.String(max_length=250) # type: ignore @@ -162,6 +186,7 @@ def test_two_pks_in_model_definition(): class ExampleModel2(Model): class Meta: tablename = "example3" + database = database metadata = metadata id: int = ormar.Integer(primary_key=True) @@ -175,6 +200,7 @@ def test_setting_pk_column_as_pydantic_only_in_model_definition(): class ExampleModel2(Model): class Meta: tablename = "example4" + database = database metadata = metadata test: int = ormar.Integer(primary_key=True, pydantic_only=True) @@ -187,6 +213,7 @@ def test_decimal_error_in_model_definition(): class ExampleModel2(Model): class Meta: tablename = "example5" + database = database metadata = metadata test: decimal.Decimal = ormar.Decimal(primary_key=True) @@ -199,6 +226,7 @@ def test_string_error_in_model_definition(): class ExampleModel2(Model): class Meta: tablename = "example6" + database = database metadata = metadata test: str = ormar.String(primary_key=True) From 3c43544e0f1786fe9b38da0519d2744be1573f38 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 18:02:25 +0700 Subject: [PATCH 8/9] Update models.py --- ormar/models/helpers/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ormar/models/helpers/models.py b/ormar/models/helpers/models.py index 1cf728a..8843bcc 100644 --- a/ormar/models/helpers/models.py +++ b/ormar/models/helpers/models.py @@ -61,7 +61,7 @@ def populate_default_options_values( class Connection(sqlite3.Connection): - def __init__(self, *args: Any, **kwargs: Any) -> None: + def __init__(self, *args: Any, **kwargs: Any) -> None: # pragma: no cover super().__init__(*args, **kwargs) self.execute("PRAGMA foreign_keys=1;") From 18602e87c3051ac9a6e414d4e3aa51204b1bbed3 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 2 Feb 2021 12:48:51 +0100 Subject: [PATCH 9/9] fix minor code smells --- ormar/fields/foreign_key.py | 1 - tests/test_model_definition.py | 1 - 2 files changed, 2 deletions(-) diff --git a/ormar/fields/foreign_key.py b/ormar/fields/foreign_key.py index baaf2ae..3ed222b 100644 --- a/ormar/fields/foreign_key.py +++ b/ormar/fields/foreign_key.py @@ -95,7 +95,6 @@ def populate_fk_params_based_on_to_model( :rtype: Tuple[Any, List, Any] """ fk_string = to.Meta.tablename + "." + to.get_column_alias(to.Meta.pkname) - to.Meta.tablename + "." + to.get_column_alias(to.Meta.pkname) to_field = to.Meta.model_fields[to.Meta.pkname] pk_only_model = create_dummy_model(to, to_field) __type__ = ( diff --git a/tests/test_model_definition.py b/tests/test_model_definition.py index 9db3761..bac267c 100644 --- a/tests/test_model_definition.py +++ b/tests/test_model_definition.py @@ -15,7 +15,6 @@ 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)