Pull to refresh

Comments 54

По поводу 1 пункта: ModelAdmin.has_delete_permission не решает поставленной задачи?
По поводу 2 пункта: мне кажется, проще вызвать Model.full_clean там, где требуется, чем неизвестно как переопределять Model.save
Ну и по поводу 3 пункта: Вы хотите сказать, что optimistic locking должен быть в Django по умолчанию? По мне так это сомнительное решение
Ну, говорить, что должно и не должно быть включено в тот или иной framework — вообще гиблое дело, т.к. на вкус и цвет… Я считаю, что включать какую-то фичу нужно полностью.

CSRF в django работает, и я знаю, что будет работать и не попортит данные.
Transactions тоже работают как ожидается.
Админка работает, но маленькое замечание — рано или поздно она испортит вам данные. И да, этого нет в документации.
Проблема не во вкусе и не в цвете, а конкретно в optimistic locking. Добавить его не сложно, когда оно надо. Выпилить его, когда оно не надо, будет сложнее
1) А когда оно не надо?
2) Предлагаю организовать запиливание / выпиливание строчкой 'django.contrib.admin.OptimisticLock' в settings.py, как и все остальные плюшки django. Сложно ли это убрать или добавить? Мне кажется, нет.
2) Этого нет в документации. Было бы — okay, допилим сами. Но эта проблема «из коробки», и про неё ничего не написано.
Когда не нужно дополнительное поле в модели и не нужен дополнительный оверхэд, если race condition для модели не предусматривается. Насчет опциональности соглашусь, только я бы это реализовал абстрактным наследованием от Model
Пожалуй, вы правы. Кстати говоря, если мне не изменяет память, в django-optimistic-lock всё именно наследованием и реализовано.
Мне кажется, quantity не должно лежать как параметр товара, вот и всё.
1) has_delete_permission — решает, разумеется. Это если говорить только о разрешении/запрете на удаление.
Пусть я хочу выполнить какое-то действие (послать email, забанить пользователя и т.д.), когда он удаляет определённую запись. Как я могу это реализовать? There's more than one way to do it. Вот, например, то, что первое приходит на ум:
— переопределить delete()
— использовать сигнал post_delete
— выполнить при проверке прав has_delete_permission
И тут — bingo! — нужно помнить, что эти три способа работают, но один из них — не всегда.

2) Мне кажется, что валидаторы должны валидировать, раз они называются валидаторами. Если бы он был нужен мне только для форм (как сейчас и реализовано в django) — я бы написал свою ModelForm с добавлением валидации.

Когда я пишу
class SomeModel(models.Model):
    name = CharField(..., validators=[])

я ожидаю, что это глобально — что у меня будет модель SomeModel с полем name и валидатором для него. И почему-то модель всегда есть, поле тоже доступно и никуда не пропадает, а вот валидатор в формах по дефолту есть, а при сохранении по дефолту нет.

Я не говорю, правильно это или нет — я просто хочу показать, что это неочевидно.
ModelAdmin.delete_model(self, request, obj)

The delete_model method is given the HttpRequest and a model instance. Use this method to do pre- or post-delete operations.

Это если про админку. Если нет — то «pre- or post-delete operations» делаются в той вьюхе, с помощью которой пользователь удаляет модель. Работает всегда
Делать «pre- or post-delete operations» в той вьюхе, где удаляется модель — это прямое нарушение DRY, если у меня несколько таких вьюх. А вдруг я в какой-нибудь из них забуду сделать это?
Обычно я пишу необходимое действие в сигнале / методе delete(), и навсегда про это забываю — ибо оно работает всегда, и ничего более для этого делать не нужно.
Приводите примеры поудачнее. Сигналы не для всего подходят, потому что отсутствует контекст, в котором вызван метод delete (тот же пользователь). Ну и вот это
1) Задачу решает. Проблему — нет. Проблема ведь в том, что мы не можем определить произвольный код в методе delete — у нас банально нет гарантий, что этот метод будет вызыван во всех случаях, когда модель удаляется. Ах да, ведь есть сигналы…
2) Не важно, что вызвать проще. Есть Data Layer, в нем мы задаем инварианты для данных… Но DL их не соблюдает, а только экспортирует для уровня выше (по сути Business Logic — Validation). На лицо существенный архитектурный просчет.
1) side effects в методе delete — это нарушение какого принципа, не подскажете? Да, есть сигналы
По второму пункту соглашусь
А не подскажите ли, какие полезные действия без side effects можно сделать в методе delete? Так мы приходим к выводу, что вообще переопределять delete никогда не стоит. Что, к сожалению, частенько делают. И это, опять же, косвенно свидетельствует о несоблюдении фреймворком принципа «наименьшего удивления».
В методе delete модель нужно удалять, что бы под этим не подразумевалось (вот для этого можно и переопределить). Никаких других полезных действий там быть не должно. Насчет принципа наименьшего удивления раскройте мысль пожалуйста (вы про пользовательский интерфейс — админку?)
Стоп, стоп. Стандартная реализация delete уже делает то, что нам нужно — удаляет. И получается, что переопределять его уже не нужно. Если считаете иначе, приведите примеры, когда это может быть необходимо/удобно.

По поводу принципа наименьшего удивления. Если толпа программистов используют инструмент не так, как следовало бы — либо они в среднем плохие программисты, либо инструмент слишком нелогичен. Отмечу, это не значит что инструмент плохой, не выполняет своих функций и прочее.
Пример — если вы не хотите фактически удалять запись в базе данных, а просто проставить отметку об удалении.

А Вы можете привести пример фреймворка, который убережет программиста от желания напихать полезных действий в медот delete модели данных?
В вашем примере совершаются побочные эффекты, и еще как. Возникнет проблема, о которой и писал автор поста — по одной модельки будут удалятся «мягко» (т.е. фактически отметкой), а вот пачкой — будут удаляться честно.

По поводу фреймворка — в документации самой Django приводятся примеры, как переопределять метод delete. Цитата:
There’s another set of model methods that encapsulate a bunch of database behavior that you’ll want to customize. In particular you’ll often want to change the way save() and delete() work.
You’re free to override these methods (and any other model method) to alter behavior.

Т.е. нас не то что бы берегут от плохого кода, нас к нему мягко и плавно подталкивают. Разве что картинки с trollface не хватает. Хотите примеры фреймворков, которые не дают вредных советов?
Это не побочный эффект, это переопределение операции удаление — теперь мы под этим понимаем установку отметки об удалении. Для этого надо переопределить метод delete модели. Но это не все, что нужно сделать, чтобы добиться такого поведения. Придется переопределять модель QuerySet и менять менеджер objects у модели. Тогда внешне работа с такой моделью не будет отличаться от обычной модели. Под alter behavior именно это подразумевается, а не то, что вы можете в этом методе отсылать e-mail или не удалять модель вопреки названию метода
У термина «побочный эффект» есть устоявшееся значение. Его я и понял из вашего комментария — вот и произошло недопонимание.

Но это не все, что нужно сделать, чтобы добиться такого поведения.
Так в этом и проблема! Нужно об этом банально знать, это неочевидно. В одном месте документации нам пишут «ок, можно переопределять delete». А спустя несколько глав мы понимам, что ведь не все так гладко…
Вы привели цитату из этого раздела. Процитировать текст в зеленой рамочке в конце этого раздела вы поленились?
Насчет «нескольких глав» погорячился. :)

Но я считаю, что переопределять delete в любом случае плохой совет. «Фишка» с QuerySet.delete — просто еще один довод против этого совета.
Не знаю, почему Вы считаете задокументированную возможность переопределить метод delete за совет так поступать. Я привел Вам пример такого переопределения. Хорошо, когда такая возможность есть, намного хуже, если бы ее не было. Согласен в том, что на переопределения этих методов нужны веские основания
Хорошие замечания, но слишком Вы наезжаете на concurency.
Django создавался как фреймворк для новостных сайтов, а не веб-магазинов.

От себя добавлю что при использовании ModelForm.save() ваш метод модели save не вызывается.
Там напрямую вызывается Model.save(object).
Нужно делать как-то так:
if form.is_valid():
    object = form.save(commit=false)
    object.save()
Извиняюсь, проверил и оно не так.
Видимо, у меня в голове засели сведения из документации Django 0.9
Мне кажется, что для новостных сайтов проблема concurrency точно так же актуальна. Там, где могут редактировать 2 и более человека — там эта проблема рано или поздно появится, вопрос лишь в том, когда это случится и что затрётся.
Да, вы правы, эта проблема актуальна и для новостных сайтов. Недаром недавняя версия Wordpress добавила Post Locking — функционал по управлению версиями статей при одновременной работе нескольких авторов.
Поведение валидаторов вполне ожидаемое, так работают многие вещи. Например, в JS form.submit() не вызывает обработчик onsubmit.

django предоставляет админку, но не предоставляет optimistic locking
«django предоставляет админку» != «django предоставляет идеальную админку, которая подходит для всех проектов без исключения». Она поэтому сделана расширяемой.

Мораль (настоящая): фантазии на тему, что как должно работать, очень плохо заменяют документацию. Не стоит полагаться на фантазии.
Спасибо за пример в JS. Это показывает, что содержимое статьи актуально не только для джанги.

Я бы не стал называть это фантазией. Фантазия — это ожидать, что django сам напишет сайт и будет привлекать клиентов.
Я бы назвал это ожиданием. Увидев название delete / validate, ожидаешь, что оно действительно удаляет и валидирует. Это моя ошибка, которой я как раз и хотел поделиться с общественностью.
3. Проблема version не решается так. Так как если сайт очень посещаемый и продукт постоянно заказывают Вы никогда не сможете его отредактировать.
Правильное решение как в системах контроля версий — хеши с солью в виде даты.
Согласен. Приведённые решения — лишь не позволяют допустить ошибку, но не решают её. Как раз для решения проблемы хотел форкнуть один из проектов и допилить туда diff и merge.

Не понял вас по поводу хэшей с солью — не могли бы поподробнее раскрыть мысль?
Я не знаю устройство ОРМ джанго, больше с bottlepy работаю, но принцип един для всех систем.
В БД добавляете два поля времени и хеша.
Хеш для определения уникальности.
Добавляете в переменную то, что может быть изменено + дату и делаете хеш (sha2, md5, что меньше коллизий имеет и быстрее), если хеш есть в БД — сохранение и изменение не вносится (т.к. это случайны дубликат).
Если хеша нет в бд, значит дата другая и/или данные.
В итоге клиенты заказавшие до изменения получат то что было до изменения, те кто заказали после изменения получат новую измененную версию, а админы уже кто последний сохранит. При этом все версии будут в БД и можно будет промоутить или заменять определенные версии в актуальную или для заказавших клиентов позже изменяя их заказанный продукт или хеш реляции.
Проблема решается с помощью select_for_update
Поправьте меня, если я не прав, но select_for_update блокирует выбранные строки до конца блока транзакции — до COMMIT или ROLLBACK. Я уже отвечал в почте, а теперь пишу и тут — проблема «optimistick lock» в контексте django связана не с транзакциями, а с разными для разных пользователей локальными версиями объектов.
Я бы сказал, в данном случае не стоит хранить остатки в таблице описания товара. Да и в общем случае это как-то странно выглядит, лучше отдельную таблицу сделать для остатков.
1. Удаление объектов в админке
Джанго админ — это приложение. Самое настоящее, такое. Оно там CRUD и прочее. Не магия! Вы скажете, она должна быть умнее, я скажу чем проще тем лучше. Мне кажется это очевидным: вы зарубили delete (полностью переопределили, даже не взглянув в оригинальный), метод не вызвал ошибки (вы ее не дали вызвать), вы видите, что метод ничего не возвращает — каким образом джанга должна понять, что что-то пошло не так? Через валидаторы — вы же принт не отправили в продакшен? Приложение совершенно точно сообщает вам, что метод отработал без экспешенов и показывает предписанное сообщение, заметим: сама джанга читать не умеет, она еще совсем маленькая.

3. Два админа
Блокировку надо где-то хранить, такие штуки нельзя предусмотреть для всех случаев, вы это делаете сами.

Второй пункт вполне справедливо, даже тупо, прям: валидация описывается в модели, но работает в админке.
Вы зря ждете от джанги магии, это приложение, смотрите сорцы — они ничуть не сложнее доков, зато в 10 раз нагляднее.
По первому пункту Вы нет так поняли — Django вообще не вызывает delete в случае, когда вы удаляете несколько моделей разом. Это не связано с django-admin — это поведение django-orm.
Тьфу. Вы правы. «Deleted successfully» и наличествующий root ввели меня в заблуждение.
Тогда мне вдвойне не понятно, чего еще ждать от метода модели, если речь идет о кверисете. Ну, как я и говорил, нет там магии: джанга не делает различий для одного или нескольких инстансов, экшены, ясное дело, работают через менеджер модели.
Чего ждать от кнопки «delete» — кверисета или метода модели? Узнайте первыми! Только в новейших исходниках django!
Ну вы еще скажите, что поведение bulk_create для вас тоже неожиданное.
Вот, кстати, не знал, что для bulk_create
The model’s save() method will not be called, and the pre_save and post_save signals will not be sent.


Поймите меня правильно, я жалуюсь не на то, что есть способы быстрого создания / удаления объектов в один запрос — это как раз удобно. Я жалуюсь, что две одинаковые по названию и смыслу кнопки делают разные вещи.
Поведение django касаемо пункта 3 вполне ожидаемо
Ну не знаю…
1) То что QuerySet.delete() выполняет одну операцию DELETE ... WHERE <условие>, а не 50 операций DELETE ... WHERE id=<id> в цикле, по-моему, вполне очевидно.
2) Может быть это не вполне очевидно с самого начала, но если бы валидация выполнялась во время Model.save() — то она бы выполнялась 2 раза (т. к. только что была выполнена во время ModelForm.is_valid()). Ну и вообще валидация — это защита от некорректного ввода пользователя, а пользователь должен вводить данные через формы.
3) Ну а что вы хотели от web-фреймворка? Это же не 1С: Бухгалтерия.
Скорее всего так и было. Под валидацией понимали валидацию ввода, но для удобства (тот же DRY), сделали определение валидаторов один раз в модели, а не в каждой форме. Резонно, что появляется ожидание использовать эту валидацию для собственно модели. Но теперь менять поведения метода save() — это ломать обратную совместимость
Еще пример:

Вы хотите проверить, одинаковые ли usename у двух пользователей. Вы ошибке вы написали:

if user1 == user2.username:

Можно ли сравнивать так поля моделей? Нет, мы всегда будем получать False. Даже не TypeError, хотя логичней было получить именно её. Сравнение экземпляров Django-модели переопределено интересным образом:
 def __eq__(self, other):
      return isinstance(other, self.__class__) and self._get_pk_val() == other._get_pk_val()


Мне пост напомнил знаменитое видео «Let's talk about Javascript»
Django работает не так, как вы думаете. Django работает ровно так, как написано в его документации. Ни больше, ни меньше.
Гениальная фраза, к сожалению о многих творениях можно сказать: Творение работает не так, как вы думаете, а х.з. как оно работает.
p.s. любимый посыл RTFM :)
Несколько лет назад у меня была долгая и упорная переписка с разработчиками Джанго насчет проблем их админки. Не помню точно деталей, но вроде дело было в том, что можно было удалить любой объект без прямого доступа, если использовать специально сконструированный URL. Патч они не захотели принимать, т.к. дизайн админки не предполагал особой секьюрити и разграничение прав — если человек имеет доступ к админке, значит он бог и царь. Более того, админка — это всего лишь contrib-приложение, reference implementation, так что если надо, каждый затачивает ее под себя как хочет и добавляет любые разграничение доступа.

Насколько я знаю, эту конкретную проблему они все-таки решили потом. Оказалось что народ использует админку, не особо озадачиваясь вопросами безопасности, и вот как-то получается (сюрприз, сюрприз), что из взламывают. Однако прежнее отношение осталось: админка — это просто contrib-приложение, хотите большего — пишете сами. В принципе, после супермагии RoR я с ними согласен, любое магическое приложение имеет свои недостатки, и найти универсальное решение зачастую проблематично. Так что надо просто снизить ожидания, и написать свою админку (благо что возможностей хватает), если вас что-то не устраивает в существующей.
Зубрение документации не помогает (ну, разве что язык/фреймворк полностью состоит из противоречий и не поддается запоминанию обычными способами). Нелогичные вещи вылетают из памяти очень быстро, день-неделя — и нету. Вот когда с десяток раз набьешь на этом шишки, они въедаются в подкорку и неожиданно язык/фреймворк становится логичным и понятным, новички посылаются грызть мануал, а критики — лесом.
Я не говорю, конечно, что мануал вообще читать не нужно, просто такое наблюдение, вряд ли в этом плане что-то можно изменить ;)
Я не новичок, но я критикую django -> я сам себя должен послать лесом :)
К сожалению, изменить действительно вряд ли что-то можно — по пунктам 1 и 3 точно видел тикеты на code.djangoproject.com, датированные эпохой динозавров — но, как видно, разрабов они не заинтересовали.

Остаётся надеяться, что всякие статьи помогут новичкам не набивать шишки.
Странно что по поводу 3его пункта еще ни кто не скинул ссылку на softwaremaniacs.org/blog/2009/01/14/changed-data-in-forms/ этот замечательный пост Ивана Сагалаева.
А по остальным пунктам, поведение совершенно ожидаемое.
Хотите валидацию при создании?
Создавайте через
m= Model()
m.save()
И не забывайте про full_clean() который не вызывается в обычном clean.
Хотите массвое удаление? Удаляйте в цикле по одному или усложняйте логику, путем переопределения метода delete в Query, который будет удалять с exclude(name='root')
Благодарю за ссылку, действительно что-то новое.
После валидации у формы будет доступен form.changed_data — список с названиями измененных полей, который уже можно использовать по своему усмотрению.

Тем не менее, это лишь частный случай решения race condition, т.к. предусматривается одновременное редактирование разных полей. Для одного и того же поля, редактируемого разными людьми, надо всё равно использовать напильник.
Уголок буквоеда. Да, это все равно не устраняет race condition целиком. Но это и не цель. Мне нужно решить задачу правильного поведения в большинстве практических случаев. В остальных случаях люди просто вручную обновят тикет еще раз как надо.


Лично для меня на заре освоения Django было очень даже неочевиден 1ый пункт. Рассмотрим его с другой стороны. Окей, для списка моделей при удалении генерируется QuerySet — не вопрос. Тогда чем отличается вырожденный случай, когда модель одна? Пусть при удалении модели через её страничку тоже используется QuerySet, содержащий одну модель. В чём был смысл вводить промежуточный delete()?
Тестировать функционал фреймворка — нет уж, увольте. Да и какое тестирование ползволило бы мне узнать об описанных выше — даже не ошибках — особенностях?
Sign up to leave a comment.

Articles