<rmcreative>

RSS

Yii: всегда учитывайте неудачный save модели

2 мая 2011

Те, кто раньше не работал с Yii, на первых порах часто совершают очень нехорошую ошибку:

$model = new Post();
if(!empty($_POST['Post']))
{
  $model->attributes = $_POST['Post'];
  $model->save();
  $this->redirect(array());
}
$this->render('form', array(
  'model' => $model,
));

Вот такой, казалось бы, простой код убить может немало нервов. Дело в том, что save запросто может не сработать, если не выполнится правило валидации. А т.к. мы этого не проверили, произойдёт редирект без каких-либо признаков ошибки.

Комментарии RSS

  1. №4527
    Ekstazi
    Ekstazi 03.05.2011, 12:05:00

    У меня проще, много раз об этом писал, но всем показалось неудобным :) :

    $model=new Post();
    if($model->saved())
        $this->redirect(array('index'));
    $this->render('create',array('model'=>$model))

    За метод saved отвечает поведение - EFormModelBehavior

  2. №4528
    Psih
    Psih 03.05.2011, 14:25:26

    Тут вообще вариаций много. Я предпочитаю вызывать validate сперва, да и в общем-то Gii так генерит. Иногда до валидации по событию что-то сделать надо или после валидации, а потом уже и сам save делаю. Местами вообще всё в транзакции занесено.

    Но основная ошибка типична и многие натыкаются.

  3. №4536
    Serge
    Serge 04.05.2011, 18:42:49

    Вообще, по хорошему, при не прохождении валидации должно кидаться исключение, жалко что изначально в Yii так не реализовано.

  4. №4537
    Sam
    Sam 04.05.2011, 18:54:16

    Serge, непрохождение валидации — это вполне штатная ситуация. В большинстве случаев необходимо выводить ошибки в форму, а не кидать исключение, поэтому try-catch вместо if сильно раздули бы код и сделали бы его не очень красивым.

  5. №4538
    Serge
    Serge 04.05.2011, 18:59:34

    2Sam: Я говорил о том моменте когда пользователь пытается сохранить модель не прошедшую валидацию в БД, именно тогда нужно кидать исключение, в других случаях я соглашусь что кидание исключения будет излишним. :)

  6. №4539
    Ekstazi
    Ekstazi 04.05.2011, 20:08:57

    2Serge, В корне не согласен. Никто не мешает подключить поведение типа:

    class EFormBehavior {
    public function submitted(button='submit',$method='POST')
    {
    $args=$method=='POST' ? $_POST : $_GET;
    if(isset($args[get_class($this->getOwner)])&&isset($args[$button])){
    $this->getOwner()->attributes=$args[get_class($this->getOwner())];
    return true;
    }
    return false;
    }
     
    public function validated(button='submit',$method='POST')
    {
    return $this->submitted($button,$method)&&$this->getOwner()->validate();
    }
    public function saved(button='submit',$method='POST')
    {
    return $this->submitted($button,$method)&&$this->getOwner()->save();
    }
     
    }

    Тем самым мы убираем еще один if и сворачиваем все в конструкцию:

    if($model->saved()){
    // Наши действия
    }

    Так что если не устраивает вариант по умолчанию, выше я описал альтернативу для избежания подобных ошибок.

  7. №4541
    Serge
    Serge 04.05.2011, 20:33:10

    2Ekstazi: Внесу немного ясности про моё высказывание про исключение. Оно должно кидаться только тогда, когда вызван метод save() и модель не валидна! Это позволит избежать ошибки описанной в данной статье. И действительно жаль что такое не реализованно по умолчанию в модели Yii

  8. №4542
    Ekstazi
    Ekstazi 04.05.2011, 21:18:54

    А вы разве не знаете что save еще и валидацию делает ?

  9. №4555
    mitallast
    mitallast 07.05.2011, 13:13:42

    Поддержу Serge. Мне должно быть абсолютно пофиг, делает save валидацию, или не делает - это же реализация метода, инкапсулированная логика. То, что save возвращает false - не говорит о характере ошибки, и в этом есть запах дурного кода. Код бизнес логики должен быть скрыт за интерфейсом. Обработка ошибок через исключения - часть этого интерфейса. Безусловно, паттерн Валидатор чрезвычайно удобен. Когда мы выполняет явную проверку типа isValid, то естественно ожидается boolean. Но когда я сохраняю объект, то я ничего не ожидаю. При любой ошибке сохранения желателен Exception, так как уровней вложенности сохранения может быть очень много при развитом домене с большим числом relations. В случае ifов будет настоящий говнокод, везде придется учитывать, что же вернул save. А мог бы быть один блок try catch на всю транзакцию. Exception объясняет, что может произойти при исполнении save, это хорошая документация.

    Если выполняется проверка - ожидается bool как результат. Если произошла ошибка выполнения - exception. Работа save - не проверка, следовательно более правильным считаю exception

    Я работаю над проектом legacy, все ошибки на if . Без слез не взглянешь на это.

  10. №4637
    Ncs
    Ncs 13.05.2011, 10:59:40

    Помогите в топике пожалуйста http://www.yiiframework.com/forum/index.php?/topic/19386-%d0%bf%d0%be%d0%bb%d1%83%d1%87%d0%b5%d0%bd%d0%b8%d0%b5-%d0%b4%d0%b0%d0%bd%d0%bd%d1%8b%d1%85-%d1%81%d0%b2%d1%8f%d0%b7%d0%b0%d0%bd%d0%bd%d0%be%d0%b9-%d0%bc%d0%be%d0%b4%d0%b5%d0%bb%d0%b8-many-many-%d0%b2-cactivedataprovider/

  11. №9374
    Виталий
    Виталий 13.11.2014, 17:46:58

    А как такое может быть, что была ошибка валидации, а $model->errors пустой? Это я про Yii 2 спрашиваю .

  12. №9376
    Sam
    Sam 14.11.2014, 2:29:19

    Виталий, лучше с вопросами на форум yiiframework.ru.

  13. №9829
    Fazliddin
    Fazliddin 30.05.2015, 9:10:00

    You have to use try-catch anyway, because PDO may throw exception.

    /**
         * Advanced save() which throws exception if validation failed
         * @param type $runValidation
         * @param type $attributes
         * @return boolean
         * @throws CDbException
         */
        public function ssave($runValidation=true,$attributes=null)
        {
            if(!static::save($runValidation,$attributes))
            {
                throw new CDbException($this->errorSummary());
            }
     
        return true;
        }
     
     
    try
    {
        $model->ssave();
        $this->redirect(array('index'));
    }
    catch(Exception $e)
    {
        Yii::app->user->setFlash('error','DB Error');
    }
    $this->render('create',array('model'=>$model))
  14. №10500
    ABariy
    ABariy 12.05.2016, 15:17:36

    Правильно ли я понимаю, проверять save() необходимо также и по причине "а вставились ли вообще данные в таблицу"?

  15. №10501
    Sam
    Sam 12.05.2016, 20:15:01

    Ну да.

  1. Почта опубликована не будет.

  2. Можно использовать синтаксис Markdown или HTML.

  3. Введите ответ в поле. Щёлкните, чтобы получить другую задачу.