Pull to refresh

Comments 48

Удаление связанных данных необходимо реализовать в beforeDelete() обработчике.


Все таки использование afterDelete более разумное в этом случае. Ведь если вы всё очистите, а удаление основной записи пройдёт с ошибкой, есть вероятность получить куча Fatal Error'ов, а вот ошибка в удалении связанных данных скорее всего просто оставит мусор в базе.
Transaction решит эти проблемы.
Транзакция не решит проблемы удаления связанных данных не из БД. Например: статических файлов. Их нужно удалять именно в afterDelete. А beforeDelete использовать для всевозможных проверок.
В вашем случае тоже будут траблы. Транзакция может откатится, а ваш afterDelete с удалением статик файлов уже отработает.
Чтобы правильно настроить удалением/изменение внешних данных в случае транзакций, можно хранить факт таких изменений в самой бд (в отдельной таблице, например вставлять туда строки с путями файлов на удаление). И уже отдельной командой по крону мониторить эту таблицу и удалять файлики.
В таком варианте у вас всегда будут консистентны бд и внешние данные
Речь об ошибках на стороне БД, которые могут возникнуть при удалении записи. В этом случае будет исключение и до afterDelete выполнение не дойдет, соответственно алгоритм удаления статики зашитый в afterDelete не отработает.
Это понятно. Я про то что даже если ваш afterDelete отработал это еще ни чего не гарантирует. Только фиксация транзакции гарантирует фиксацию изменений. Это одна из задач которую я даю сеньорам на собеседованиях. Спроектировать механизм который бы гарантировал непротиворечивость данных в бд и «вне бд». Яркий пример удаление профиля и удаление связанных с ним файлов. Варианты когда данные в бд и вне бд противоречивы
— профиль удален, файлы висят
— профиль не удален, файлы удалены

Кроме файлов, внешними данными могут быть сервисы (например нотифи на почту, если удалить не удалось и после отката транзакции у нас целехонький профиль, а мыло с «сожалением об утрате» уже отправлено)
Да, согласен с вами, если в рамках транзакции выполняется пакетная операция затрагивающая данные за пределами БД, задачу решать нужно с учетом архитектуры, и обрабатывать «внешние» данные только после успешного коммита.
Я имею в виду что, в обобщенном случае, afterDelete для атомарной операции более удачный обработчик удаления связанных данных чем beforeDelete.
Я полагаю речь идет об использовании MyISAM, в этом случае использование FK и транзакций невозможно.
Вы не сможете удалить FK если конечно не стоит CASCADE и потом, если запись будет удалена а связанные записи нет (например ошибка и исключение в середине afterDelete) в результате это приведет к ошибкам вида «объект не найден» по причине рваных связей.
А зачем ставить RESTRICT и усложнять себе жизнь, если workflow в этой ситуации требует именно CASCADE?
CASCADE ресурсоемок и его не всегда можно использовать, например когда проект находится на поддержке и FK нельзя применить технически, однако CASCADE — логически самый правильный подход, так как целостность данных обеспечивается не только исходниками, но и самой БД, так сказать — два бастиона безопасности.
UFO just landed and posted this here
Обычно как раз не касается. Редко в бизнес-сценариях есть что-то вроде «Счёт формируют менеджеры, а если его сформировала бухгалтер, то вызываем службу безопасности», обычно просто «мне приносят счёт, а я его подписываю или выкидываю». Контроль прав — это обычно логика приложения, а не бизнес-логика.
Часто необходимо провести операции с данными из консоли и тут будут проблемы, т.к. придется либо эмулировать пользователя и назначать на него RBAC правила, либо получить ошибку. И еще в этом случае полностью убивается переносимость модели.
UFO just landed and posted this here
Какая-нибудь модель Account становится привязанной к классу модели User с методом типа isAllowed($action), а то и вообще к классу приложения типа Context, который вообще в моделировании предметной области не участвует. В результате чтобы перенести Account в другое приложение нам нужно перетаскивать всю подсистему управления правами, даже если она вообще не нужна. В лучшем случае реализовать самим заглушку для isAllowed($action), всегда возвращающую true. Модели добавляется вторая, а то и третья (в случае ActiveRecord) ответственность и зависимость от одной из самой сложной в общем случае подсистемы приложения. Причём, если злоумышленник имеет доступ к файловой системе и может дергать модель напрямую из своего скрипта, то ему не составит труда передать модели информацию, что он нужными правами обладает. Потому лучше проверку прав оставить в контроллере (что и семантически более верно — права нужно контролировать, а не моделировать) или перенести в сервис, который контроллер будет дергать.
UFO just landed and posted this here
Не исчезает, поскольку файл с правами нужно будет править, ну или добавлять в какой-то конфиг приложения — как не крути ещё одна зависимость. А если взять один бандл из одного приложения, а другой из другого, то велика вероятность, что они будут использовать разные подходы к управлению и контролю прав, а также разные подходы к работе с конфигами, что тоже как-то нужно сводить к общему знаменателю. В общем создаем себе геморрой на ровном месте, лишь потому что не хотим почему-то считать, что управление правами в приложении это ответственность логики приложения, а не бизнес-логики.
UFO just landed and posted this here
Мой как раз поможет. Хоть части модели (подмножество связанных классов), хоть вся целиком переносится в другой фреймворк или голый пхп простым копипастом классов. Ни логики прав доступа, ни логики хранения, ни ещё чего-нибудь они за собой не тянут — только незамутненная бизнес-логика.
User::model()->withFlags(User::FLAG_CONFIRM_EMAIL)->findAll();

Мне кажется что лучше это делать при помощи scope(). Более гибкий и мощный инструмент.
ваш код:
const FLAG_CONFIRM_EMAIL    = 1; // 00000001
const FLAG_CONFIRM_PHONE    = 2; // 00000010
const FLAG_BEST_AUTHOR      = 4; // 00000100
...
    /**
     * Устанавливает флаг
     * @param integer $flag
     * @return \User
     */
    public function setFlag($flag)
    {
         $this->flag += intval($flag);
         return $this;
    }
...
$user->setFlag(User::FLAG_CONFIRM_EMAIL);
$user->setFlag(User::FLAG_CONFIRM_EMAIL);
$user->hasFlag(User::FLAG_CONFIRM_PHONE); // == true (внезапно)

это битовые поля, работая с ними нужно использовать битовые операции. (аналогичная ошибка и в unsetFlag(...))
поставил вам минус, но потом понял, что вы правы.

в setFlag должно быть битовое или:
$this->flag |= intval($flag);

в unsetFlag должно быть, например, так:
$this->flag &= ~intval($flag);

здесь разжевано
Кстати, и определения флаговых констант лучше делать через сдвиг единицы 1 << 0, 1 << 1, 1 << 2 и т. п., а не 1, 2, 4.
Модели предназначены только для оперирования данными (поиск, сохранение, удаление)

Весьма-весьма сомнительно. Модели в MVC предназначены для моделирования предметной области, а в них, как правило, отсутствуют понятия «поиск», «сохранение» и «удаление». Как раз такие операции кандидаты на вынос в сервисы из контроллера. Если взять систему документооборота, то там обычно такие понятия как «создать документ», «подписать», «передать в архив», «уничтожить», а операции поиска вообще особо не определена, в лучшем случае определена на уровне «возьми папку „договора“ и найди там договор с ООО „Рога и копыта“». Идеальная модель вообще никак не связана с системой хранения, предполагается что она вся находится в памяти всегда, а конкретный класс зависит только от других классов модели.
Выражение вида
if ($model->status !== 1 && $model->status !== 2 && $model->status !== 4) { }


Можно сократить до
if (!in_array($model->status, [1,2,4])) { }
Вы забыли передать true третьим параметром.
Тут предложили более производительный вариант.
if (array_key_exist($model->status, [1=>0, 2=>0, 4=>0])) {}
В большинстве случаев, это будет преждевременной оптимизацией, ухудшающей читаемость кода. Вводится три сущности (нули), назначение которых не очень ясно. Почему, кстати, 0, а не null? null семантически лучше подходит.
Не рекомендую использование флаговых полей, очень сильно проседает производительность. Заметно уже при кол-во записей от 10к
а можно чем-то обосновать? может замеры есть?
на собственном проекте использовал вот эту реализацию: habrahabr.ru/post/130427/
при фильтрации таблицы с кол-вом записей около 10тыс, сервер задумывался на 3-4 секунды
if($model->hasAvatar())

наверно правильнее

if($this->model->hasAvatar())
спасибо исправляю
У меня есть вопросы касательно сортировки моделей.

1. Чаще всего при изначальном отображении списка (например, в CGridView) он должен быть отсортирован по некоему дефолтовому критерию, например, по атрибуту name. Для этого я обычно включаю sort=>defaultOrder в методе model->search(), но не уверен, что это правильно.

2. Очень часто модель ссылается на другую сущность посредством relations. Например, есть модели Blog и BlogPost. BlogPost имеет атрибут blog_id, ссылающийся на Blog->id. При отображении в CGridView я вывожу столбец blog.name (где blog — это соотв. relation), но отсортировать по нему нет возможности — он не кликается. Это как-то можно исправить?

3. Как задать defaultSort (пункт 1) по полю из relation (пункт 2)?
Еще очень часто в моделях идет обращение к Yii::app()->user (например, для получения ID автора записи) и Yii::app()->request (например, для фиксации IP автора записи), на мой взгляд, этого тоже следует избегать.
Угу, подобные вещи должны передаваться из контролера.
1. К константам в моделях я часто сразу добавляю их названия и геттер. Удобно тем, что можно и в виджетах (ListView\GridView) можно сразу прописать вывод статуса и в формах сразу передать список возможных значений в radio или select.
И не смотря на абсолютно правильную заметку об отделении html кода от логики, иногда отступаю от правила и добавляю метод, который выводит «разукрашенную» версию названия, поскольку добавление нового класса ради одного метода, считаю, внесет больше неудобства и путаницы.

/**
* @property string statusTitle
*/
class MyModel extend CActive Record{
  const STATUS_CREATED = 1;
  const STATUS_SENT = 2;

  public static $statusTitles = array(
    self::STATUS_CREATED => 'Новый',
    self::STATUS_SENT => 'Отправленный',
  );
  
  public function getTypeTitle() {
    return self::$statusTitles[$this->status];
  }


  public function getTypeTitleDesigned() {
    return self::$statusTitles[$this->status];
  }

  /** 
  * для определенных случаев выводим "разукрашенную" версию статуса. 
  */ 
 public function getStatusTitleDesigned() {
    // по своей логике определяем украшения: скажем,  иконку и цвет в стиле Twitter Bootstrap'a
    $color = 'success'; 
    $icon = 'ok'; 


    return TbHtml::labelTb(TbHtml::icon($icon) . " " . $this->statusTitle, array(
      'color' => $color,
    ));
  }
}

}


2. Связанные данные. (relations)
У CActiveRecord есть замечательная особенность — можно целиком переопределять свойство модели зарезервированное уже под связные данные в relations.
Если есть две модели Team и Member (думаю, связи и поля этих двух моделей очевидны) я использую этот прием для сохранения связанных табличных данных. В отличие от «официального» подхода здесь сохранение данных вынесено из контроллера в модель и можно работать с данными по имени связи сразу после их получения. Также становиться удобно одновременно сохранять данные родительской модели и связанных данных. (например, сразу создаем команду и заполняем информацию о ее участниках)

Код моделей и контроллера
/** 
* @property Member[] $members 
*/ 
class MyModel extend CActive Record{
 public function rules() {
    return array(
      //....
      array('newMembers', 'safe'), // для того чтобы можно было получить данные из формы
      array('newMembers', 'validateData'), // для их валидации
      //....
    );
  }

 public function relations() {
    return array(
      //....
      'members' => array(self::HAS_MANY, 'Member', 'teamId', 'index' => 'id'),
      //....
    );
  }

   /**
   * Получение из формы табличных данных и объединение их с уже существующими.
   * @param mixed $newMembers
   */
  public function setNewMembers($newMembers) {
    $members= $this->members; 
    foreach ($newMembers as $memberId=> $data) {
      if (is_numeric($memberId) && $memberId!= 0) { // отсекаем ненужные данные
        if ($members[$memberId]) { // если уже существует
          $member= $members[$memberId];
        } else { // если не находим, создаем новую модель
          $member= new Member('create');
          $member->teamId= $this->id;
          $members[$memberId] = $member; 
        }
        $member->team= $this; // для того чтобы избегать лишних запросов к базе
        $member->attributes= $data;
      };
    }
    $this->rates = $rates; // вся магия здесь =)
  }
}

  /**
   * Проверка данных
   */
  public function validateData() {
    if (!$this->rates) return true;
    $ok = true;
    foreach ($this->rates as $rate) {
      $ok = $rate->validate() && $ok;
    }
    return $ok;
  }

  /**
   * Сохранение данных
   */
  protected function afterSave() {
    $this->saveData();
    parent::afterSave();
  }

  public function saveData() {
    if (!$this->members) return;
    foreach ($this->members as $member) {
      $member->save(false); // мы данные уже проверили в validateData()
    }
  }


 

Код контроллера получается логичным

class TeamController extends Controller{


  /**
   * Добавление нового команды и ее членов
   */
  public function actionCreate() {
    $model = new Team('create');


    if (isset($_POST['Team'])) {
      $model->attributes = $_POST['Team'];
      $model->setNewMembers($_POST['Member']);  
     /* 
     Подразумевается, что в представлении в одной форме мы 
     вместе заполняем данные и для команды и для ее членов.

     Хотя и здесь, конечно,  можно выпендриться и добавлять все данные о членах команды 
     в том же массиве, что и команда.    
     Таким образом сократиться второе присваивание, только можно будет запутаться в именах. =)
     */

      if ($model->save()){ 
         Yii::app()->user->setFlash('success', 'Все замечательно');
          $this->redirect(array('view', 'teamId' => $model->id));
      }else{
         Yii::app()->user->setFlash('alert', 'Возникли ошибки');
      }

    }

    $this->render('create', array(
      'model' => $model,
    ));
  }


}


Я не против критики, особенно конструктивной. Если уж и написал я пургу, то дайте знать в чем именно?
Не минусовал, но очень не понравилась идея внедрения в модель html (а ведь туда ещё и css/js может понадобиться включить. Чуть меньше не понравилась идея задавать символьные обозначения константам на естественном языке. Ладно бы ключи для перевода и то, не очень понятно почему просто не задавать константы статуса строками, а не числами const STATUS_CREATED = 'created'; В коде без разницы, в БД относительно небольшая разница.
1. Про html:
Я не отрицаю необходимость отделения кода от представления. О коде невозможно говорить в сферическом вакууме, и я скромно настаиваю лишь на адекватном отношении к каждой ситуации. Есть общие компоненты системы, которые почти не меняются от проекта к проекту: пользователи/rbac, а есть проектозависимые. И что в одной ситуации хорошо, то в другой будет плохо.

Задача: Есть в проекте модели, с парой полей, значения которых определяется константами нужно их отобразить значение и возможно оформление (иконка/текст).

Небольшие комментарии к модели
Я исхожу из того, что всегда надо сделать проще и понятнее:
  • Если константы, то значит выбор был небольшой и заранее известный, что не потребовало отдельного справочника
  • Поскольку полей мало, то это также не требует отдельной таблицы (скажем, id,modelName,key,value)
  • Мало/много — понятие относительное, надеюсь на вашу адекватную оценку, я отталкивался от границы около десятка-двух моделей.
  • Сложность логики отображения оцениваю в строках кода — больше нескольких строк, значит сложная. (об этом писал Макконелл)




Разберем разные ситуации:
  1. моделей мало, полей мало, логика отображения сложная — используем виджеты, код разделяется
  2. моделей мало, полей много, логика отображений простая — замечательно будет использовать класс хелпер, разные методы которого будут отображать разные вещи. При сложной логике используем, для отдельных элементов виджеты.
  3. моделей много, полей мало, логика простая — если создавать для отображения каждого поля отдельный виджет и хелпер в котором будет один метод с объемом в пару строк, то вы просто потонете в файлах. Надеюсь не ошибусь, если припомню статью на хабре, где обсуждалась ситуация в Java, когда повсеместным стало подобное поведение — там все было разложено по полочкам. И да, для сложных ситуаций все равно используем виджеты.


И моя ошибка была в том, что я не указал, что у меня третий случай и в проекте весь интерфейс построен на бутстрапе, а значит ваш комментарий про css\js просто снимается (все уже включено)

2.
Чуть меньше не понравилась идея задавать символьные обозначения константам на естественном языке.

Вы говорили об массиве $statusTitles? Это не обозначения констант, а расшифровка из значений.
если создавать для отображения каждого поля отдельный виджет и хелпер в котором будет один метод с объемом в пару строк, то вы просто потонете в файлах

Есть варианты и без этого. Но лучше, имхо, создавать, поскольку логика может из простой стать сложной, полей стать много и т. п. Конечно, если вы пишите приложения, развития которых не предполагается, есть чёткое ТЗ, выполнили его и забыли, то есть смысл забивать на разделение.
Вы говорили об массиве $statusTitles? Это не обозначения констант, а расшифровка из значений.

Имел в виду представления, но даже если расшифровка, то она есть и в названии, а в моем варианте и в строке.
Sign up to leave a comment.

Articles