Merge pull request #488 from collerek/fix_large_binary

Add comments to columns and bugfixes

docs/releases.md

@@ -1,3 +1,16 @@
+# 0.10.23
+
+## ✨ Features
+
+* Add ability to pass `comment` to sqlalchemy when creating a column [#485](https://github.com/collerek/ormar/issues/485)
+
+## 🐛 Fixes
+
+* Fix `LargeBinary` fields that can be nullable [#409](https://github.com/collerek/ormar/issues/409)
+* Make `ormar.Model` pickable [#413](https://github.com/collerek/ormar/issues/413)
+* Make `first()` and `get()` without arguments respect ordering of main model set by user, fallback to primary key (asc, and desc respectively) [#453](https://github.com/collerek/ormar/issues/453)
+* Fix improper quoting of non-aliased join `on` clauses in postgress [#455](https://github.com/collerek/ormar/issues/455)
+
 # 0.10.22
 
 ## 🐛 Fixes

ormar/fields/base.py

@@ -97,6 +97,8 @@ class BaseField(FieldInfo):
         self.ormar_default: Any = kwargs.pop("default", None)
         self.server_default: Any = kwargs.pop("server_default", None)
 
+        self.comment: str = kwargs.pop("comment", None)
+
         self.represent_as_base64_str: bool = kwargs.pop(
             "represent_as_base64_str", False
         )
@@ -271,6 +273,7 @@ class BaseField(FieldInfo):
                 unique=self.unique,
                 default=self.ormar_default,
                 server_default=self.server_default,
+                comment=self.comment,
             )
         else:
             column = self._get_encrypted_column(name=name)

ormar/models/descriptors/descriptors.py

@@ -59,7 +59,11 @@ class BytesDescriptor:
     def __get__(self, instance: "Model", owner: Type["Model"]) -> Any:
         value = instance.__dict__.get(self.name, None)
         field = instance.Meta.model_fields[self.name]
-        if field.represent_as_base64_str and not isinstance(value, str):
+        if (
+            value is not None
+            and field.represent_as_base64_str
+            and not isinstance(value, str)
+        ):
             value = base64.b64encode(value).decode()
         return value
 

ormar/models/mixins/save_mixin.py

@@ -1,3 +1,4 @@
+import base64
 import uuid
 from typing import (
     Any,
@@ -51,6 +52,7 @@ class SavePrepareMixin(RelationMixin, AliasMixin):
         new_kwargs = cls._remove_not_ormar_fields(new_kwargs)
         new_kwargs = cls.substitute_models_with_pks(new_kwargs)
         new_kwargs = cls.populate_default_values(new_kwargs)
+        new_kwargs = cls.reconvert_str_to_bytes(new_kwargs)
         new_kwargs = cls.translate_columns_to_aliases(new_kwargs)
         return new_kwargs
 
@@ -144,6 +146,36 @@ class SavePrepareMixin(RelationMixin, AliasMixin):
                     model_dict.pop(field, None)
         return model_dict
 
+    @classmethod
+    def reconvert_str_to_bytes(cls, model_dict: Dict) -> Dict:
+        """
+        Receives dictionary of model that is about to be saved and changes
+        all bytes fields that are represented as strings back into bytes.
+
+        :param model_dict: dictionary of model that is about to be saved
+        :type model_dict: Dict
+        :return: dictionary of model that is about to be saved
+        :rtype: Dict
+        """
+        bytes_fields = {
+            name
+            for name, field in cls.Meta.model_fields.items()
+            if field.__type__ == bytes
+        }
+        bytes_base64_fields = {
+            name
+            for name, field in cls.Meta.model_fields.items()
+            if field.represent_as_base64_str
+        }
+        for key, value in model_dict.items():
+            if key in bytes_fields and isinstance(value, str):
+                model_dict[key] = (
+                    value.encode("utf-8")
+                    if key not in bytes_base64_fields
+                    else base64.b64decode(value)
+                )
+        return model_dict
+
     @classmethod
     def populate_default_values(cls, new_kwargs: Dict) -> Dict:
         """

ormar/models/newbasemodel.py

@@ -195,6 +195,29 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass
         """
         return super().__getattribute__(item)
 
+    def __getstate__(self) -> Dict[Any, Any]:
+        state = super().__getstate__()
+        self_dict = self.dict()
+        state["__dict__"].update(**self_dict)
+        return state
+
+    def __setstate__(self, state: Dict[Any, Any]) -> None:
+        relations = {
+            k: v
+            for k, v in state["__dict__"].items()
+            if k in self.extract_related_names()
+        }
+        basic_state = {
+            k: v
+            for k, v in state["__dict__"].items()
+            if k not in self.extract_related_names()
+        }
+        state["__dict__"] = basic_state
+        super().__setstate__(state)
+        self._initialize_internal_attributes()
+        for name, value in relations.items():
+            setattr(self, name, value)
+
     def _internal_set(self, name: str, value: Any) -> None:
         """
         Delegates call to pydantic.
@@ -861,7 +884,7 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass
         if column_name not in self._bytes_fields:
             return value
         field = self.Meta.model_fields[column_name]
-        if not isinstance(value, bytes):
+        if not isinstance(value, bytes) and value is not None:
             if field.represent_as_base64_str:
                 value = base64.b64decode(value)
             else:
@@ -882,7 +905,11 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass
         if column_name not in self._bytes_fields:
             return value
         field = self.Meta.model_fields[column_name]
-        if not isinstance(value, str) and field.represent_as_base64_str:
+        if (
+            value is not None
+            and not isinstance(value, str)
+            and field.represent_as_base64_str
+        ):
             return base64.b64encode(value).decode()
         return value
 

ormar/queryset/join.py

@@ -108,7 +108,14 @@ class SqlJoin:
         :rtype: sqlalchemy.text
         """
         left_part = f"{self.next_alias}_{to_clause}"
-        right_part = f"{previous_alias + '_' if previous_alias else ''}{from_clause}"
+        if not previous_alias:
+            dialect = self.main_model.Meta.database._backend._dialect
+            table, column = from_clause.split(".")
+            quotter = dialect.identifier_preparer.quote
+            right_part = f"{quotter(table)}.{quotter(column)}"
+        else:
+            right_part = f"{previous_alias}_{from_clause}"
+
         return text(f"{left_part}={right_part}")
 
     def build_join(self) -> Tuple[List, sqlalchemy.sql.select, List, OrderedDict]:

ormar/queryset/queryset.py

@@ -873,12 +873,16 @@ class QuerySet(Generic[T]):
 
         expr = self.build_select_expression(
             limit=1,
-            order_bys=[
+            order_bys=(
+                [
                     OrderAction(
                         order_str=f"{self.model.Meta.pkname}",
                         model_cls=self.model_cls,  # type: ignore
                     )
                 ]
+                if not any([x.is_source_model_order for x in self.order_bys])
+                else []
+            )
             + self.order_bys,
         )
         rows = await self.database.fetch_all(expr)
@@ -909,7 +913,7 @@ class QuerySet(Generic[T]):
         except ormar.NoMatch:
             return None
 
-    async def get(self, *args: Any, **kwargs: Any) -> "T":
+    async def get(self, *args: Any, **kwargs: Any) -> "T":  # noqa: CCR001
         """
         Get's the first row from the db meeting the criteria set by kwargs.
 
@@ -931,12 +935,16 @@ class QuerySet(Generic[T]):
         if not self.filter_clauses:
             expr = self.build_select_expression(
                 limit=1,
-                order_bys=[
+                order_bys=(
+                    [
                         OrderAction(
                             order_str=f"-{self.model.Meta.pkname}",
                             model_cls=self.model_cls,  # type: ignore
                         )
                     ]
+                    if not any([x.is_source_model_order for x in self.order_bys])
+                    else []
+                )
                 + self.order_bys,
             )
         else:
@@ -1096,6 +1104,7 @@ class QuerySet(Generic[T]):
                 )
             new_kwargs = self.model.parse_non_db_fields(new_kwargs)
             new_kwargs = self.model.substitute_models_with_pks(new_kwargs)
+            new_kwargs = self.model.reconvert_str_to_bytes(new_kwargs)
             new_kwargs = self.model.translate_columns_to_aliases(new_kwargs)
             new_kwargs = {"new_" + k: v for k, v in new_kwargs.items() if k in columns}
             ready_objects.append(new_kwargs)

pyproject.toml

@@ -3,7 +3,7 @@ name = "ormar"
 
 [tool.poetry]
 name = "ormar"
-version = "0.10.22"
+version = "0.10.23"
 description = "A simple async ORM with fastapi in mind and pydantic validation."
 authors = ["Radosław Drążkiewicz <collerek@gmail.com>"]
 license = "MIT"
@@ -44,7 +44,7 @@ classifiers = [
 python = "^3.6.2"
 databases = ">=0.3.2,<0.5.4"
 pydantic = ">=1.6.1,!=1.7,!=1.7.1,!=1.7.2,!=1.7.3,!=1.8,!=1.8.1,<=1.8.2"
-SQLAlchemy = ">=1.3.18,<1.4.28"
+SQLAlchemy = ">=1.3.18,<1.4.29"
 asyncpg = { version = ">=0.24,<0.26", optional = true }
 psycopg2-binary = { version = "^2.9.1", optional = true }
 aiomysql = { version = ">=0.0.21,<0.0.23", optional = true }
@@ -82,7 +82,7 @@ pytest = "^6.2.5"
 pytest-cov = "^3.0.0"
 codecov = "^2.1.12"
 pytest-asyncio = "^0.16.0"
-fastapi = "^0.70.0"
+fastapi = "^0.70.1"
 flake8 = "^3.9.2"
 flake8-black = "^0.2.3"
 flake8-bugbear = "^21.11.29"
@@ -110,9 +110,9 @@ types-dataclasses = { version = "^0.6.1", markers = "python_version < '3.7'" }
 
 # Documantation
 mkdocs = "^1.2.3"
-mkdocs-material = "^8.0.5"
+mkdocs-material = "^8.1.2"
 mkdocs-material-extensions = "^1.0.3"
-pydoc-markdown = { version = "^4.3.2", markers = "python_version > '3.7'" }
+pydoc-markdown = { version = "^4.5.0", markers = "python_version > '3.7'" }
 dataclasses = { version = ">=0.6.0,<0.8 || >0.8,<1.0.0" }
 
 # Performance testing

tests/test_fastapi/test_binary_fields.py

@@ -1,11 +1,9 @@
 import base64
 import json
-import os
 import uuid
 from typing import List
 
 import databases
-import pydantic
 import pytest
 import sqlalchemy
 from fastapi import FastAPI

tests/test_model_definition/test_models.py

@@ -58,6 +58,21 @@ class LargeBinaryStr(ormar.Model):
     )
 
 
+class LargeBinaryNullableStr(ormar.Model):
+    class Meta:
+        tablename = "my_str_blobs2"
+        metadata = metadata
+        database = database
+
+    id: int = ormar.Integer(primary_key=True)
+    test_binary: str = ormar.LargeBinary(
+        max_length=100000,
+        choices=[blob3, blob4],
+        represent_as_base64_str=True,
+        nullable=True,
+    )
+
+
 class UUIDSample(ormar.Model):
     class Meta:
         tablename = "uuids"
@@ -231,6 +246,37 @@ async def test_binary_str_column():
             assert items[1].__dict__["test_binary"] == blob4
 
 
+@pytest.mark.asyncio
+async def test_binary_nullable_str_column():
+    async with database:
+        async with database.transaction(force_rollback=True):
+            await LargeBinaryNullableStr().save()
+            await LargeBinaryNullableStr.objects.create()
+            items = await LargeBinaryNullableStr.objects.all()
+            assert len(items) == 2
+
+            items[0].test_binary = blob3
+            items[1].test_binary = blob4
+
+            await LargeBinaryNullableStr.objects.bulk_update(items)
+            items = await LargeBinaryNullableStr.objects.all()
+            assert len(items) == 2
+            assert items[0].test_binary == base64.b64encode(blob3).decode()
+            items[0].test_binary = base64.b64encode(blob4).decode()
+            assert items[0].test_binary == base64.b64encode(blob4).decode()
+            assert items[1].test_binary == base64.b64encode(blob4).decode()
+            assert items[1].__dict__["test_binary"] == blob4
+
+            await LargeBinaryNullableStr.objects.bulk_create(
+                [LargeBinaryNullableStr(), LargeBinaryNullableStr(test_binary=blob3)]
+            )
+            items = await LargeBinaryNullableStr.objects.all()
+            assert len(items) == 4
+            await items[0].update(test_binary=blob4)
+            check_item = await LargeBinaryNullableStr.objects.get(id=items[0].id)
+            assert check_item.test_binary == base64.b64encode(blob4).decode()
+
+
 @pytest.mark.asyncio
 async def test_uuid_column():
     async with database:
@@ -306,14 +352,16 @@ async def test_model_get():
             lookup = await User.objects.get()
             assert lookup == user
 
-            user = await User.objects.create(name="Jane")
+            user2 = await User.objects.create(name="Jane")
             await User.objects.create(name="Jane")
             with pytest.raises(ormar.MultipleMatches):
                 await User.objects.get(name="Jane")
 
-            same_user = await User.objects.get(pk=user.id)
+            same_user = await User.objects.get(pk=user2.id)
-            assert same_user.id == user.id
+            assert same_user.id == user2.id
-            assert same_user.pk == user.pk
+            assert same_user.pk == user2.pk
+
+            assert await User.objects.order_by("-name").get() == user
 
 
 @pytest.mark.asyncio
@@ -449,6 +497,8 @@ async def test_model_first():
             with pytest.raises(NoMatch):
                 await User.objects.filter(name="Lucy").first()
 
+            assert await User.objects.order_by("name").first() == jane
+
 
 def not_contains(a, b):
     return a not in b

tests/test_model_definition/test_models_are_pickable.py

@@ -0,0 +1,61 @@
+import pickle
+from typing import Optional
+
+import databases
+import pytest
+import sqlalchemy
+
+import ormar
+from tests.settings import DATABASE_URL
+
+database = databases.Database(DATABASE_URL, force_rollback=True)
+metadata = sqlalchemy.MetaData()
+
+
+class User(ormar.Model):
+    class Meta:
+        tablename = "users"
+        metadata = metadata
+        database = database
+
+    id: int = ormar.Integer(primary_key=True)
+    name: str = ormar.String(max_length=100)
+    properties = ormar.JSON(nullable=True)
+
+
+class Post(ormar.Model):
+    class Meta:
+        tablename = "posts"
+        metadata = metadata
+        database = database
+
+    id: int = ormar.Integer(primary_key=True)
+    name: str = ormar.String(max_length=100)
+    created_by: Optional[User] = ormar.ForeignKey(User)
+
+
+@pytest.fixture(autouse=True, scope="module")
+def create_test_database():
+    engine = sqlalchemy.create_engine(DATABASE_URL)
+    metadata.drop_all(engine)
+    metadata.create_all(engine)
+    yield
+    metadata.drop_all(engine)
+
+
+@pytest.mark.asyncio
+async def test_dumping_and_loading_model_works():
+    async with database:
+        user = await User(name="Test", properties={"aa": "bb"}).save()
+        post = Post(name="Test post")
+        await user.posts.add(post)
+        pickled_value = pickle.dumps(user)
+        python_value = pickle.loads(pickled_value)
+        assert isinstance(python_value, User)
+        assert python_value.name == "Test"
+        assert python_value.properties == {"aa": "bb"}
+        assert python_value.posts[0].name == "Test post"
+        await python_value.load()
+        await python_value.update(name="Test2")
+        check = await User.objects.get()
+        assert check.name == "Test2"

tests/test_model_definition/test_setting_comments_in_db.py

@@ -0,0 +1,36 @@
+import databases
+import pytest
+import sqlalchemy
+
+import ormar
+from ormar.models import Model
+from tests.settings import DATABASE_URL
+
+metadata = sqlalchemy.MetaData()
+
+database = databases.Database(DATABASE_URL, force_rollback=True)
+
+
+class Comment(Model):
+    class Meta(ormar.ModelMeta):
+        tablename = "comments"
+        metadata = metadata
+        database = database
+
+    test: int = ormar.Integer(primary_key=True, comment="primary key of comments")
+    test_string: str = ormar.String(max_length=250, comment="test that it works")
+
+
+@pytest.fixture(autouse=True, scope="module")
+def create_test_database():
+    engine = sqlalchemy.create_engine(DATABASE_URL)
+    metadata.create_all(engine)
+    yield
+    metadata.drop_all(engine)
+
+
+@pytest.mark.asyncio
+async def test_comments_are_set_in_db():
+    columns = Comment.Meta.table.c
+    for c in columns:
+        assert c.comment == Comment.Meta.model_fields[c.name].comment

tests/test_queries/test_quoting_table_names_in_on_join_clause.py

@@ -0,0 +1,63 @@
+import datetime
+import uuid
+from typing import Dict, Optional, Union
+
+import databases
+import pytest
+import sqlalchemy
+from sqlalchemy import create_engine
+
+import ormar
+from tests.settings import DATABASE_URL
+
+database = databases.Database(DATABASE_URL)
+metadata = sqlalchemy.MetaData()
+engine = create_engine(DATABASE_URL)
+
+
+class Team(ormar.Model):
+    class Meta:
+        tablename: str = "team"
+        database = database
+        metadata = metadata
+
+    id: uuid.UUID = ormar.UUID(default=uuid.uuid4, primary_key=True, index=True)
+    name = ormar.Text(nullable=True)
+    client_id = ormar.Text(nullable=True)
+    client_secret = ormar.Text(nullable=True)
+    created_on = ormar.DateTime(timezone=True, default=datetime.datetime.utcnow())
+
+
+class User(ormar.Model):
+    class Meta:
+        tablename: str = "user"
+        database = database
+        metadata = metadata
+
+    id: uuid.UUID = ormar.UUID(default=uuid.uuid4, primary_key=True, index=True)
+    client_user_id = ormar.Text()
+    token = ormar.Text(nullable=True)
+    team: Optional[Team] = ormar.ForeignKey(to=Team, name="team_id")
+
+
+class Order(ormar.Model):
+    class Meta:
+        tablename: str = "order"
+        database = database
+        metadata = metadata
+
+    id: uuid.UUID = ormar.UUID(default=uuid.uuid4, primary_key=True, index=True)
+    user: Optional[Union[User, Dict]] = ormar.ForeignKey(User)
+
+
+@pytest.fixture(autouse=True, scope="module")
+def create_test_database():
+    metadata.create_all(engine)
+    yield
+    metadata.drop_all(engine)
+
+
+@pytest.mark.asyncio
+async def test_quoting_on_clause_without_prefix():
+    async with database:
+        await User.objects.select_related("orders").all()