Pull to refresh

Comments 12

За идею вам +
Но позвольте дать пару замечаний.

1. Как по мне так MixIn на то и MixIn чтобы не наследовать Model, а использоваться в множественном наследовании. Тогда при использовании надо будет создавать модель таким образом:
class TestCreatedUpdated(who_models.CreatedUpdatedByMixin, models.Model):
    value = models.CharField('Value', max_length=255)

В этом случае можно переопределить save и он будет корректно вызываться через super.

2. Singleton не учитывает потоки. Скорей всего приложение будет запущенно в несколько потоков, при частом изменении объектов будет шанс что при одновременных запросах второй юзер будет указан как автор для обоих объектов. Шанс небольшой, но ловить такой баг очень неприятно

3. Пожалуй стоит добавить в статью пример подключения сигнала. Лично мне не очень нравится что надо создавать пустую функцию. Как по мне так декоратор в таком случе ни к чему
Спасибо за замечания. Постараюсь на них ответить:
1.Хотелось в MixIn не наследовать Model, но не получилось так как не точно знаю как изнутри работает Model. Пытался реализовать такой вариант следующем образом:
from django.db import models


class TestMixin:
    mixin_field = models.CharField(max_length=23)
    

class RealClassWithMixin(TestMixin, models.Model):
    real_field = models.CharField(max_length=23)

Посмотрел как django генерирует sql:
(test_whovedonethis)zapix@zapix-Lenovo-B560:~/projects/test_whovedonethis$ python manage.py sql testmixin

BEGIN;
CREATE TABLE "testmixin_realclasswithmixin" (
    "id" integer NOT NULL PRIMARY KEY,
    "real_field" varchar(23) NOT NULL
)
;
COMMIT;

Решил, что там как-то по хитрому всё реализовано через мета-классы, и туда лезть не стоит.
К тому же переписывая метод save() мы не сможем использовать несколько Mixin'ов.
2.Насколько понял из PEP-0333 Каждый запрос выполняется в отдельном процессе, так что такой проблемы не может возникнуть.
3. Сейчас добавлю пример подключения сигнала, за это отдельное спасибо.
По-моему проще к пустому обработчику сигнала добавить декоратор, нежели писать один и тот-же код несколько раз. Ниже salvator Предложил вариант как можно избавиться от сигналов. Я его еще не рассматривал.
1. Да, тут я возможно ступил. Я использовал Mixin'ы в моделях Джанго, но поля таким образом не добовлял. У меня было что-то типа Вашего «created_by_field».

В любом случае использование super позволяет в большинстве случаев безопастно переопределять метод save у моделей.

2. У популярных способов деплоя, скажем uwsgi или mod_wsgi указывается, сколько процессов создавать и сколько потоков в каждом процессе.
Можно избавиться от декораторов сигналов, и следовательно, создания пустых функций-сигналов, используя для подключения сигналов post_save собственный менеджер. Примерно, так:

from django.db import models
from django.dispatch import receiver
from django.db.models.signals import class_prepared, post_save

def _save_created_by(sender, **kwargs):
"""
сохраняем пользователя, создавшего модель в нужное поле
"""
...


def _set_created_by_signals(sender):
post_save.connect(_save_created_by, sender=sender)


class CreatedByManager(models.Manager):

def contribute_to_class(self, model, name):
class_prepared.connect(_set_created_by_signals, sender=model.__class__)
return super(CreatedByManager, self).contribute_to_class(model, name)
Спасибо большое. Такой вариант не рассматривал. Постараюсь в нем разобраться.
В django мы не можем подключить сигнал непосредственно к абстрактной модели, т.к. в качестве сендера будет использоваться модель-потомок. Но мы можем подключить свой менеджер, который унаследует потомок.

При подключении менеджера вызывается contribute_to_class, в котором мы цепляем к модели обработчик сигнала class_prepared, срабатывающий после инициализации класса. в самом обработчике — подключаем сигнал post_save, выполняющий основную работу.

Такой workaround для сигналов к абстрактным моделям. Идею нашел на stackoverflow, сам не пробовал еще. Там же есть еще вариант создать обработчик сигналов для всех моделей, и проверять в нем класс сендера. Но гонять такую проверку для всех зарегистрированных моделей — плохая идея.
Думаю, неплохим решением будет сделать middleware, которое добавляет в threadlocals объект request (как делается, например, в Pylons). Этот middleware можно будет использовать для разных хаков и потом. А с помощью него получить ответ на вопрос «кто юзер» можно будет прямо в методе save().

Не смотрел, как django использует super(), но скорее всего можно будет описать save() в CreatedByMixin и UpdatedByMixin, а потом достаточно будет просто отнаследоваться от этих миксинов. Что-то вроде:
class UpdatedByMixin(models.Model):
    def save(self, *args, **kwargs):
        from threadlocals import request
        self.updated_by = request.user
        return super(UpdatedByMixin, self).save(*args, **kwargs)

это может быть решением. замечу только, что это чревато. ведь религиозная война на тему, почему так делается, например, в Pylons, и не делается в Django ещё не закончена). разработчики фреймворка вполне сознательно отключили профильную статью в своей вики code.djangoproject.com/wiki/CookBookThreadlocalsAndUser, где что-то подобное было описано.
Вы правы, войны на тему threadlocals идут, мне малопонятно только зачем. На мой взгляд, очень четкий коментарий по этому поводу дал Glenn Maynard на stackoverflow. По-моему ничего ужасного, главное использовать с умом threadlocals только там где это действительно имеет место.

Кстати, если загуглить django threadlocals, то найдется несколько готовых решений для django, реализующих упомянутый middleware.
Возможно, что threadlocals как лакмусовая бумажка, показывает потенциально опасные места в архитектуре. Меня настораживает, что предложенное решение привязывает модель к HTTP запросу. Т.е. пока модель используется сугубо как «штука, которая должна сохранять информацию из http запроса», всё здорово. Но если возникнет необходимость использовать модель из другого контекста — например, из консольной команды, где ни какого request нет и в помине — не знаю как в Pylons, но в Django будут заморочки, т.к. стек middleware в таком случае не отрабатывает.
по правде говоря, я не до конца понял, область применимости решения, предложенного в топике. Если речь идёт об учёте действий пользователей в системе, то модели об этом аспекте, в идеале, не должны знать ни чего (иначе, как ответить на вопрос «кто удалил объект?»), — тема интересна, но данное решение не подходит. Если же взаимодействие пользователя с моделью существенно, с точки зрения бизнес-логики, значит оно должно быть отражено в публичном интерфейсе модели — в таком случае пользователь будет передаваться явно, и threadlocals нафик не нужен.
Sign up to leave a comment.

Articles