FactoryBoy / factory_boy

A test fixtures replacement for Python
https://factoryboy.readthedocs.io/
MIT License
3.49k stars 392 forks source link

DjangoOptions has no attribute `model` #857

Open isik-kaplan opened 3 years ago

isik-kaplan commented 3 years ago

Description

DjangoOptions class of a DjangoModelFactory is missing attributes post __init_subclass__

To Reproduce

Run the code below, blows up without actually creating the PollFactory type.

Model / Factory code
from app.models import Poll

class BaseDjangoModelFactory(factory.django.DjangoModelFactory):
    model_map = {}

    def __init_subclass__(cls, **kwargs):
        factory_init_subclass = super().__init_subclass__(**kwargs)
        BaseDjangoModelFactory.model_map[cls._meta.model] = cls
        return factory_init_subclass

    @classmethod
    def get_for_model(cls, model):
        return cls.model_map[model]

    class Meta:
        abstract = True

class PollFactory(BaseDjangoModelFactory):
    class Meta:
        model = Poll
The issue

When trying to keep track of the factories and factory models in a single place with what's above, there isn't model attribute on the factory options.

Python 3.8.5 (default, Jan 27 2021, 15:41:15) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.22.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from app.factories import PollFactory
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-37711cb22fce> in <module>
----> 1 from app.factories import PollFactory

~/Desktop/-/--/---/app/factories.py in <module>
     65 
     66 
---> 67 class PollFactory(BaseDjangoModelFactory):
     68     class Meta:
     69         model = Poll

~/Desktop/-/--/venv/lib/python3.8/site-packages/factory/base.py in __new__(mcs, class_name, bases, attrs)
     74         attrs['_meta'] = meta
     75 
---> 76         new_class = super().__new__(
     77             mcs, class_name, bases, attrs)
     78 

~/Desktop/-/--/---/app/factories.py in __init_subclass__(cls, **kwargs)
     53     def __init_subclass__(cls, **kwargs):
     54         factory_init_subclass = super().__init_subclass__(**kwargs)
---> 55         BaseDjangoModelFactory.model_map[cls._meta.model] = cls
     56         return factory_init_subclass
     57 

AttributeError: 'DjangoOptions' object has no attribute 'model'

Notes

Let me know if I'm doing anything wrong, but this generic init subclass and holding the child classes in the base class is something I've been using with almost all django libraries:

django-filter, drf, grapene, django.forms, django.contrib.admin

so I'd really love to be able to continue to my pattern without changing anything; currently debugging and will send my solution as a PR if y'all accept this above as a bug

isik-kaplan commented 3 years ago

Apparently I can't use model_map directly in the factory body anyway as it would be used as a constructor argument for the model class, however, I found the issue and will PR ASAP.

But I think it would be a good idea to allow the users to create attributes that are encapsulated and are not put into the model constructor, and in particularly this case I don't think it is appropriate to put it in an options class since it is not an option for a factory class but information that belongs to the base class. Yep. Meta.exclude, saw it.

rbarrois commented 3 years ago

Hi!

I'm sorry, but I'm not sure I understand what you're trying to do, so it's hard to suggest a good pattern to fix it :/

As a side note: your example code is using private APIs from the library; it is not the team's intention to "freeze" the behaviour of the metaclass — they are likely to change significantly whenever we get around to introducing the "auto-factory" feature.

isik-kaplan commented 3 years ago

Hey, thank you for the prompt response. I basically want to be able to just get the factory I created for a given model.

FooFactory = BaseModelFactory.get_for_model(Foo) 

Note that this is not auto generation, I just want to get the factory I've previously created. I can obviously create a dictionary and put everything after I created them but a simple metaclass that handles it feels nicer imo.

One of the places I use this, for example, is auto generic foreign key generation. Ex:

def generic_foreign_key_id_for_type_factory(generic_relation_type_field):
    def generic_foreign_key_id(obj):
        model = getattr(obj, generic_relation_type_field).model_class()
        related = BaseDjangoModelFactory.get_for_model(model).create()
        return related.id

    return factory.LazyAttribute(generic_foreign_key_id)

class BarF(BaseDjangoModelFactory):
    class Meta:
        model = Bar
        django_get_or_create = (
            "resource_type",
            "resource_id",
        )

    resource_type = factory.Iterator(
        ContentType.objects.filter(limit_generic_relations(*RELATION_ALLOWED_MODELS))
    )
    resource_id = generic_foreign_key_id_for_type_factory("resource_type")
    mapping = {}

As for your note, I assumed the _meta api is not as private as it is not in django. They even have documentation for that, https://docs.djangoproject.com/en/3.2/ref/models/meta/ hence I assumed meta api in factory boy would be similar. If that's not the case, my bad.

rbarrois commented 3 years ago

Thanks for the answer! It makes your objective clearer.

The main issue you'll face is that factory_boy explicitly supports there being several factories for the same model; hence, you'll have to write your own code there. I'll have to check, but I think it should be possible to reach your goal with the following snippet:

class MyMetaclass(FactoryMetaClass):
  def __new__(mcs, class_name, bases, attrs):
    factory_class = super().__new__(mcs, class_name, bases, attrs)
    REGISTRY.register(factory_class._meta.get_model_class(), factory_class)
    return factory_class

Basically, factory_boy hasn't migrated to the new, Python3-style metaclass helpers (__init_subclass__ in your snippet), which might be the cause of the issue you're seeing.

Regarding Factory._meta, only the parts described in https://factoryboy.readthedocs.io/en/stable/reference.html#meta-options are officially supported.

isik-kaplan commented 3 years ago

Ah, thanks for the clarification on meta.

Yeah the custom code is indeed necessary, I do realize that. But my problem wasn't that that feature should be in the library but just the metaclass part and if the old metaclasses work with the __new__method that's great. But my PR should make more of the options available for __init_subclass__too. Although it is just bodging, it shouldn't be that much of a problem I think. I'll just update the code to pass it through the CI/CD linters. So you people can take a better look at the PR and see if it is OK.

Thanks!

isik-kaplan commented 2 years ago

Any updates?