Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for unique_together #154

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add support for unique_together #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c66509e
feat: set attribue for unique together arguments
SepehrBazyar Apr 1, 2022
92377bc
feat: added unique parameters in build sql table
SepehrBazyar Apr 1, 2022
662119b
refactor: private class method get unique constraint
SepehrBazyar Apr 1, 2022
982066a
test: write two test check unique together columns
SepehrBazyar Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion orm/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ def __new__(cls, name, bases, attrs):
if field.primary_key:
model_class.pkname = name

unique_together = attrs.get("unique_together", ())
setattr(model_class, "unique_together", unique_together)

return model_class

@property
Expand Down Expand Up @@ -486,6 +489,7 @@ def _prepare_order_by(self, order_by: str):

class Model(metaclass=ModelMeta):
objects = QuerySet()
unique_together: typing.Sequence[typing.Union[typing.Sequence[str], str]]

def __init__(self, **kwargs):
if "pk" in kwargs:
Expand Down Expand Up @@ -515,10 +519,21 @@ def __str__(self):
def build_table(cls):
tablename = cls.tablename
metadata = cls.registry._metadata
unique_together = cls.unique_together

columns = []
for name, field in cls.fields.items():
columns.append(field.get_column(name))
return sqlalchemy.Table(tablename, metadata, *columns, extend_existing=True)

uniques = []
for fields_set in unique_together:
unique_constraint = cls.__get_unique_constraint(fields_set)
if unique_constraint is not None:
uniques.append(unique_constraint)

return sqlalchemy.Table(
tablename, metadata, *columns, *uniques, extend_existing=True
)

@property
def table(self) -> sqlalchemy.Table:
Expand Down Expand Up @@ -580,6 +595,24 @@ def _from_row(cls, row, select_related=[]):

return cls(**item)

@classmethod
def __get_unique_constraint(
cls,
columns: typing.Union[typing.Sequence[str], str],
) -> typing.Optional[sqlalchemy.UniqueConstraint]:
"""
Returned the Unique Constraint of SQLAlchemy.

:columns: Must be `str` or `Sequence[List or Tupe]` of Strings

If Type of 'columns' Didn't Match Above Nothing to Return Output
"""

if isinstance(columns, str):
return sqlalchemy.UniqueConstraint(columns)
elif isinstance(columns, (tuple, list)):
return sqlalchemy.UniqueConstraint(*columns)

def __setattr__(self, key, value):
if key in self.fields:
# Setting a relationship to a raw pk value should set a
Expand Down
29 changes: 29 additions & 0 deletions tests/test_columns.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ class User(orm.Model):
}


class Customer(orm.Model):
registry = models
fields = {
"id": orm.Integer(primary_key=True),
"fname": orm.String(max_length=100),
"lname": orm.String(max_length=100),
}
unique_together = (("fname", "lname"),)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @SepehrBazyar ,

Thank you for the PR. I haven't really given this much thought but what do you think about this style?

class Customer(orm.Model):
    registry = ...
    fields = ...
    constraints = (
        orm.UniqueConstraint("name"),
         ...
     )

And the constraints are just shortcuts to the SQLAlchemy constraints.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @aminalaee
Thank you for your attention.

This style is completely OK but I think we should pay attention to what we want and what matters to us; then choose accordingly.
So, if we want it to be more user-friendly and easier to employ, then my style would probably come in handy because it's more similar to Django.

But if we want it to be more raw and similar to SQLAlchemy constraints, then your style would be a perfect choice.
But there is one problem I can think of!
In here, we don't have a class Meta, thus all and all of our attributes will be defined under class Model! So if we want unique constraints, check constraints and etcetera in one attribute, this field could get crowded.

At last, it's better to support both styles and have it all!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I think so. Let's wait for more feedback and see what other people think.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, I think we can follow a way as long as it is documented.

With that, perhaps the SQLAlchemy approach is better since it is known that ORM uses the SQLAlchemy core.



@pytest.fixture(autouse=True, scope="module")
async def create_test_database():
await models.create_all()
Expand Down Expand Up @@ -159,3 +169,22 @@ async def test_bulk_create():
assert products[1].data == {"foo": 456}
assert products[1].value == 456.789
assert products[1].status == StatusEnum.DRAFT


async def test_unique_together_fname_lname():
sepehr = await Customer.objects.create(fname="Sepehr", lname="Bazyar")
sepehr: Customer = await Customer.objects.get(pk=sepehr.pk)

farzane = await Customer.objects.create(fname="Farzane", lname="Bazyar")
farzane: Customer = await Customer.objects.get(pk=farzane.pk)

assert sepehr.lname == farzane.lname
assert sepehr.fname == "Sepehr"
assert farzane.fname == "Farzane"


async def test_unique_together_fname_lname_raise_error():
with pytest.raises(Exception):

await Customer.objects.create(fname="Sepehr", lname="Bazyar")
await Customer.objects.create(fname="Sepehr", lname="Bazyar")