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.

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

Support callables as default values in django model fields #116

Closed
pakal opened this issue Oct 8, 2020 · 3 comments · Fixed by #117
Closed

Support callables as default values in django model fields #116

pakal opened this issue Oct 8, 2020 · 3 comments · Fixed by #117

Comments

@pakal
Copy link

pakal commented Oct 8, 2020

When using django-model-utils TimeStampedModel subclass, which uses default=now callables in "created" and "modified" fields, generation of model instances (with _fill_optional=True) via baker fails:

  File "...site-packages\django\utils\dateparse.py", line 107, in parse_datetime
    match = datetime_re.match(value)   # here value is function "now"
TypeError: expected string or bytes-like object

This is probably because bakery uses the default value as is, instead of calling it (when relevant) to get the actual default value.

Versions

Python: 3.7.9
Django: 3.1.2
Model Bakery: 1.2.0

@timjklein36
Copy link
Collaborator

I was able to reproduce this issue with the following setup:

Versions

Python: 3.7.5

Django==3.1.2
django-model-utils==4.0.0
model-bakery==1.2.0

models.py

from django.db import models
from django.utils.timezone import now
from model_utils.models import TimeStampedModel


class Dummy(TimeStampedModel):
    pass


class SimpleDummy(models.Model):
    dt = models.DateTimeField(default=now)

tests.py

from datetime import datetime

from django.test import TestCase
from model_bakery import baker


class DummyTest(TestCase):

    def test_bakery_default_timestamped(self):
        dummy = baker.make('my_app.Dummy', _fill_optional=True)
        self.assertTrue(isinstance(dummy.created, datetime))

    def test_bakery_default_simple(self):
        dummy = baker.make('my_app.SimpleDummy', _fill_optional=True)
        self.assertTrue(isinstance(dummy.dt, datetime))

Here, baker.make works for SimpleDummy but fails for Dummy (the model extending TimeStampedModel).

The issue here is that model_bakery.generators has no default generator for the class of the field created on TimeStampedModel because it extends models.DateTimeField:

model_utils.fields.py

class AutoCreatedField(models.DateTimeField):
    ...

This is definitely a bug one way or another, but there are multiple things going on here.

The model_bakery code falls through the check in this block when generating a value for the field:

model_bakery.baker.py

if field.name in self.attr_mapping:
    generator = self.attr_mapping[field.name]
elif getattr(field, "choices"):
    generator = random_gen.gen_from_choices(field.choices)
elif is_content_type_fk:
    generator = self.type_mapping[contenttypes.models.ContentType]
elif generators.get(field.__class__):
    generator = generators.get(field.__class__)
elif field.__class__ in self.type_mapping:
    generator = self.type_mapping[field.__class__]
elif field.has_default():
    return field.default
else:
    raise TypeError("%s is not supported by baker." % field.__class__)

...which means that field.has_default() evaluates to True and the default value is returned, in this case django.utils.timezone.now which is a function. It is definitely evident that this should take into account if the default value is a callable and, if so, call it.

@berinhard
Should the conditional block be reorderd (or broken up) so that if there is a default it is always used?

The reason why SimpleDummy.dt, from above, works is because it never gets to the field.default check (because the elif field.__class__ in self.type_mapping check passes when it retrieves models.DateTimeField from the default type mapping). This means that it does not use the default value in the case where the field class has a generator available. Should this be true?

timjklein36 pushed a commit to timjklein36/model_bakery that referenced this issue Oct 9, 2020
- Also, always generate default value if `field.default` is supplied
  - Previously, this would not happen, since the `generator` determination
    was being done earlier in the same `if, elif` block

- Add unit tests for generating default value of callables of varying kinds
timjklein36 pushed a commit to timjklein36/model_bakery that referenced this issue Oct 9, 2020
- Also, always generate default value if `field.default` is supplied
  - Previously, this would not happen, since the `generator` determination
    was being done earlier in the same `if, elif` block

- Add unit tests for generating default value of callables of varying kinds
@berinhard
Copy link
Member

Fixed by #117

berinhard added a commit that referenced this issue Oct 13, 2020
…117)

* [#116] Call field default when it is callable

- Also, always generate default value if `field.default` is supplied
  - Previously, this would not happen, since the `generator` determination
    was being done earlier in the same `if, elif` block

- Add unit tests for generating default value of callables of varying kinds

* [116] Add a unit test to cover kwarg over attr_mapping

* [116] Reduce complexity of custom field class

* [116] Update CHANGELOG

Co-authored-by: Bernardo Fontes <[email protected]>
@pakal
Copy link
Author

pakal commented Oct 13, 2020

Awesome, thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants