Swappable Models

[stephenmcd]

One of the biggest issues people hit with Mezzanine, and likely many other open source Django apps that provide models, is the ability to customize the fields that the models use. Django itself hit this early on with its User model, which can now be "swapped" by pointing a setting at the app/model name to use in its place. (Presumably, I've never used it) the swappable model must implement certain members, methods and/or attributes, and then consumers of the model (view functions and the like) can expect these to exist.

Mezzanine has the EXTRA_MODEL_FIELDS setting that allows fields to be injected, but it has a handful of problems. Primarily database migrations are difficult to deal with, since migrations for an app now spread across the original app and some external app/package. Apparently m2m fields can't be injected. Some related issues are #1031 and #1150.

Ideally we can make the core models throughout Mezzanine (and Cartridge) swappable, and remove the EXTRA_MODEL_FIELDS setting. This would give developers the power to customize things like:

• The core Page model that underpins all page types
• The BlogPost model
• Cartridge's Product model

The ability to swap out these three models would take Mezzanine and Cartridge's potential flexibility to a whole other level.

[jMyles]

I think this is a great early candidate for expanding swappable models beyond auth.User. I'm happy to chip in or liase with the django core team on this.

[stephenmcd]

Awesome. Like I said I haven't used it myself, so there's a big (naive) question in my head. Is the notion of swappable models something that's a general feature of Django, or is it specific only to its User model?

Another naive thought is that even if this isn't a general feature, is there really that much to it, considering that all it might entail is replacing hard-coded references to particular models throughout the code base and having them dynamically loaded via the setting name?

[jMyles]

It's absolutely intended to be used for other models. The author of the patch, Russell Keith-Magee, describes this notion here:

https://www.youtube.com/watch?feature=player_detailpage&v=KHg6AoExYjs#t=2165

[stephenmcd]

That clears that up then. Thanks!

[AlexHill]

Have been thinking about this a lot lately. From a quick inspection I think the Django feature is essentially what you've described above Steve, but with some added bits to support migrations properly. For example I think a swapped-out model won't have its tables created.

I haven't watched Russ's talk yet, but there's some interesting discussion about Django's swappable models here, from when they were being developed: https://groups.google.com/forum/#!searchin/django-developers/swappable/django-developers/YwEZUIMuIkE/g9-tAZ15RxMJ

Anyway, I think I'll post to django-developers about this tomorrow. There's an opportunity to add something really useful not only to Mezzanine but to Django on the whole.

On my wishlist for an official API:

• Being able to use a class decorator to swap out classes (in addition to Django's existing settings-based method).
• Having swapped-out models be considered abstract, so that you don't have to create a pair of abstract and concrete classes for every model you want to be swappable. I know django-shop and oscar both do this.

[frankier]

Also interesting: https://github.com/wq/django-swappable-models

It looks like this is small enough to be vendorised into Mezzanine if need be.

[dsanders11]

Is it possible that the new Applications API in Django 1.7+ could be useful here?

Specifically I'm thinking Mezzanine could have an app config that lends itself to being extended, like:

# mezzanine/apps.py

from django.apps import AppConfig

class MezzanineAppConfig(AppConfig):
    label = 'mezzanine'

    def get_models(self):
        models = super(MezzanineAppConfig, self).get_models()
        final_models = []

        for model in models:
            final_models.append(self.get_model(model.__name__))

        return final_models

Then to use it in Mezzanine you'd do:

# mezzanine/blog/views.py

from django.apps import apps

_mezzanine_app_config = apps,get_app_config('mezzanine')
BlogPost = _mezzanine_app_config.get_model('BlogPost')
## OR ##
BlogPost = apps.get_model('mezzanine', 'BlogPost')

And finally, to swap out a model in your project:

# myblogapp/models.py

from mezzanine.blog.models import BlogPost

class MyBlogPost(BlogPost):
    pass

# myproject/apps.py

from mezzanine.apps import MezzanineAppConfig

from myblogapp.models import MyBlogPost

class MyMezzanineAppConfig(MezzanineAppConfig):
    def get_model(self, model_name):
        if model_name.lower() == 'blogpost':
            return MyBlogPost
        else:
            return super(MyMezzanineAppConfig, self).get_model(model_name)

# myproject/settings.py

INSTALLED_APPS = [
    'myproject.apps.MyMezzanineAppConfig',
    # ...
]

This is untested, just me writing out some code as a thought experiment. I haven't had a chance to play with Django 1.7+, but doing a cursory check of their GitHub, it seems like they've moved the code for getting models from an app to using AppConfig.get_model, so it seems like this may work well with migrations and what not.

My understanding of the code so far would suggest that mezzanine.blog.models.BlogPost would be skipped over for having a table created in this situation because the migration code would use AppConfig.get_models which would return the model from myblogapp. I'm not sure what the app label would be for the model.

The timing of when to call get_model in mezzanine/blog/views.py would need to be tested, it may not be possible to do at the top-level since all of the app configs may not have been loaded yet at that point, I'm not sure.

Per @AlexHill's suggestion of having a decorator, a modified version of the above code could instead build up a list of swapped models in MezzanineAppConfig via the decorator and get rid of the need for the project subclass of MezzanineAppConfig, but I'm again not sure if that would occur in time to be useful (would myblogapp.models be imported and have the class decorators executed before the makemigrations command started in on MezzanineAppConfig would be the question).

Seems like an interesting direction to go, if someone has the time to play around with it. A nice '@swap_model(BlogPost)' decorator could be a clean way to trigger the functionality.

EDIT: On second thought, the decorator syntax may not be ideal. If there are two apps that both want to add something new to BlogPost, and I want to use both apps, that seems problematic if they both used the decorator. In that case I'd probably need to roll my own AppConfig anyway to resolve the conflict.

[AlexHill]

@dsanders11 interesting idea and thankyou for spending the time to think about this.

The obstacle is the one you already identified: AppConfig.get_model() and friends aren't guaranteed to be available until after Apps.populate() has been called, which means your model definitions can't reliably use them.

You can override these things of course, but messing with the internals of app loading and model importing is something we want to do less of – that's the motivation for getting rid of EXTRA_MODEL_FIELDS.

My understanding of the code so far would suggest that mezzanine.blog.models.BlogPost would be skipped over for having a table created in this situation because the migration code would use AppConfig.get_models which would return the model from myblogapp.

I think you're right, migrations wouldn't be created for the overridden model. What might happen is that a migration would be created in the blog app for your own custom BlogPost model, which would be a dealbreaker – that's one of the biggest annoyances with EXTRA_MODEL_FIELDS today. Maybe not, since the app_label would differ – would have to test.

[AlexHill]

Overall, I think any solution which makes models "impersonate" models in other apps is a dangerous path to go down.

Django's swappable user model avoids this. When you swap out auth.User, you're never pretending that your custom user model is actually a part of contrib.auth. Instead, models that need a user model just ask for whatever user model is configured, and work with that.

That approach is nice because models stay in their own apps, and because it's explicit: if you really do need auth.User, you can still depend on it directly without it being swapped out from underneath you.

[stephenmcd]

My 2 cents - sounds like the right thing to do would be to mimic whatever auth.User does.

[AlexHill]

Totally agree with that. IMO it's only a question of how much sugar we add on top.

[avery-laird]

I'm not sure if this conversation is still ongoing, but I have been looking into how auth.User works its magic recently. Some of the explanations from here (as well as a look at the source) have helped me try to understand the more minute details. From what I understand, since django doesn't support overriding model fields, the user would - at some point - have to copy over all of the required code from the default model to their own app. It makes sense (at least to me) to setup the internals of mezzanine to support swapped models (eg, if a user was overriding Page, all references would have to be replaced with settings.PAGES_PAGE_MODEL) and create a separate app, called swap for example, with all of the necessary code copied to its models.py.

To further explain the pages example, there would be a commented-out line in settings.py that would look something like PAGES_PAGE_MODEL = 'swap.Page' (kind of like how EXTRA_MODEL_FIELDS is right now). This is exactly how auth.User does it, with an AUTH_USER_MODEL setting. The swap app would not be used unless that line was uncommented, and the database migrated. However, since all of the necessary references and code have been copied to that app, it is easily overridden by the user in a couple steps.

Obviously, swap would also have to be added to INSTALLED_APPS, but only uncommenting the PAGES_PAGE_MODEL setting would trigger a swap. That being said, it should be left commented out in INSTALLED_APPS until use, otherwise it would just get in the way.

I'm not sure how much this method makes sense from your guys' point of view, since I'm by no means an experienced developer. Any thoughts?

[stephenmcd]

@avery-laird sounds right

[avery-laird]

I've been doing a bit of preliminary work to get this up and running, but when updating the Page reference in other app models to settings.PAGES_PAGE_MODEL, I'm getting a type error that I've never seen before:

TypeError: Error when calling the metaclass bases
    unicode() argument 2 must be string, not tuple

I haven't been able to track down the issue just from my own debugging, has anyone run into this exception before?

It seems like it's probably something really simple I'm just overlooking.

[bors-ltd]

Hi, I have swapped BlogPost on my 4.0.1 branch: bors-ltd@77001a4

It works for my case (inheriting from django.contrib.gs models and admin) but it lacks tests.

And I should take a look at that AppConfig thing too.

[dsanders11]

Noticed the second warning in the documentation regarding substituting a custom user model which points out that the model being swapped in has to be created in the initial migration for its app. Something to keep in mind if this progresses since it will be a bit of a headache for switching to swapped models since most people would probably use a model from an app which has an initial migration at that point.

[bors-ltd]

Hello, I'm waking up this rather old issue.

I was pretty sure I already opened a pull request for my "swappable_blogpost" branch and people commented it but it seems my memory failed me.

Anyway, here is my branch updated against the latest 4.2.2 release (not the master branch): 4.2.2...bors-ltd:4.2.2-swappable_blogpost

Do you need I go on with the Page model and make it a "swappable_models" branch? Against what base branch? How do you see tests about it?

The tests pass on my branch but I don't have any new test with a custom model. I could probably get somewhere with override_settings but not sure how to do it without adding a model that would be shipped along with Mezzanine.

[stephenmcd]

Hello, I'm waking up this rather old issue.

Thanks for waking it up! This is probably the number one feature Mezzanine needs right now, so it's very welcome.

Do you need I go on with the Page model and make it a "swappable_models" branch? Against what base branch?

Yes this definitely needs to be complete across all the concrete models before we can accept it (maybe that's why your old PR fell by the wayside, I honestly can't remember either). I'd suggest treating "swappable models" as one entire feature rather than breaking it down into blog, pages, etc, and looking at your current work, it doesn't look like there's too much involved, which lends itself toward doing it all together - so one branch/PR should do.

I could probably get somewhere with override_settings but not sure how to do it without adding a model that would be shipped along with Mezzanine.

Give it a try and see how far you get - it would be great to have specific coverage for this, but if we can't, at least having the existing tests pass when swapped models are explicitly used in your current project would definitely suffice for now.

Thanks again!

[bors-ltd]

Just to notify I've started the work. No big issue yet but I'm playing hide and seek everywhere content models are imported.

So far I have swapped:

• Page
• RichTextPage
• Link
• BlogPost
• BlogCategory
• Form
• FormEntry
• FieldEntry
• Gallery
• GalleryImage

And the first design decision I must report is when, for example, Gallery inherits from Page, it must be the vanilla Page. The swapped model is out of the Python scope. So if you want to add a field to every Page and Page-like models, you'll have to swap all of them.

[stephenmcd]

Awesome!

So if you want to add a field to every Page and Page-like models, you'll have to swap all of them.

Ideally we can resolve this so that swapping Page is actually useful. We can look at that more closely once the initial work is done.

[bors-ltd]

Hello again, this was close to be ready for initial review, but there are three tests not passing: test_delete_unused, test_keywords and test_keywords. (test_syntax too but expected on the 4.2.2 tag).

Here is the work in progress: https://github.com/bors-ltd/mezzanine/tree/4.2.2-swappable_models

I could make these tests pass by modifying RichTextPage to inherit directly from (Page, RichText), bypassing AbstractRichTextPage. It's quite strange, if you could take a look.

There is also the question of swapping Page itself, the root of all content models, and still having Page.objects.all() lists links, forms, galleries, etc. although they were not swapped.

I've also left a few XXX, mainly about translations.

By the way, how do you run Mezzanine's test suite in the context of a bootstraped project?

[ryneeverett]

The get_<model>_name and get_<model>_model functions seem tedious. Could they be done away with? E.g.:

    class AbstractRichTextPage(Page, RichText):
        @staticmethod
        def get_model():
            model_name = getattr(settings, "RICH_TEXT_PAGE_MODEL", "pages.RichTextPage")
            return utils.models.get_model(model_name)

    RichTextPage = AbstractRichTextPage.get_model()

[ryneeverett]

Or even more concise:

def get_swappable_model(setting_name, default):
    model_name = getattr(settings, setting_name, default)
    return get_model(model_name)

RichTextPage = get_swappable_model("RICH_TEXT_PAGE_MODEL", "pages.RichTextPage")

[bors-ltd]

I agree on get_<model>_name, I followed the get_user_model_name pattern but it doesn't make sense to begin with. Django already promotes settings.AUTH_USER_MODEL in the docs.

But I couldn't get rid of these model loading functions. These models must be loaded later in Django's bootstrap process. If I just load all these models early in the process, at best I get "Models are not ready yet", at worst I get obscure errors in tests.

I've just pushed forced another version. The get_<model>_model are spread out in the __init__ of each app, just like django.contrib.auth does.

At first I got quite many obscure errors, like blog URLs not reversing, but I delayed as much the loading of models and the errors disappeared. Which makes me feel this is quite fragile.

But I still get these three tests not passing and I lack a proper debug environment like running a single test and open an (i)pdb when a test fails.

[bors-ltd]

Hi,

I reworked my swappable models branch against the latest release: https://github.com/stephenmcd/mezzanine/compare/master...bors-ltd:4.3.1-swappable-models?expand=1

But tests are far from passing for now...

Github shows conflicts against the master branch, I don't have any "4.3.x" or "stable" branch to rebase upon.