Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Конфигурация модели #4078

Closed
lynicidn opened this issue Jun 27, 2014 · 88 comments
Closed

Конфигурация модели #4078

lynicidn opened this issue Jun 27, 2014 · 88 comments

Comments

@lynicidn
Copy link
Contributor

В данной реализации фреймворка невозможно никак сконфигурировать модель, т.к. все упирается на public static function instantiate($row). Хотелось, чтобы разработчики пересмотрели свое мнение касательно данного нюанса, и все таки дали разработчикам возможность задавать конфиг по умолчанию. На самом деле сложного здесь не очень много, но не хотелось бы создавать свои грабли. Для начала опишу случаи когда это может понадобиться - к примеру мы используем 1 модуль в нескольких местах и хотим чтобы наша модель в зависимости от конфигурации модуля реализовывала то или иное поведение. Как второй вариант задача сценария для релатед моделей. Собственно хотелось бы предложить в ActiveQueryTrait заменить modelClass на modelConfig и разрешить делать следующие 3 пункта:

  • $this->hasMany(['class'=>'ModelClass', 'property' => $var], ....)
  • hasOne
  • ModelClass::find(['property' => $var])

далее изменим вышеупомянутую функцию instantiate($row, $config = [])

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

@creocoder
Copy link
Contributor

Нужен конкретный пример конкретной задачи, которую вы пытаетесь решить. Т.к. это

передаче того же модуля в модель

Пахнет нарушением MVC. Скорее всего у задачи есть более правильный путь решения, без внесения каких либо изменений в фреймворк.

@lynicidn
Copy link
Contributor Author

к примеру у меня есть модуль Like, и я хочу на основе его сделать blacklist и whitelist а так же favorites, и хочу конфигурировать, к примеру имя таблицы в которой это будет храниться. 3 вышеописанных модуля не предел, под эту логику с 10 точно можно найти применение, а с моим вариантом и без писанины кода, а просто разруливая конфигами у 1 модуля

@lynicidn lynicidn reopened this Jun 27, 2014
@creocoder
Copy link
Contributor

и хочу конфигурировать, к примеру имя таблицы в которой это будет храниться

Вы хотите динамически менять название таблицы в конкретной модели?

@lynicidn
Copy link
Contributor Author

ну это как пример, не нравится имя, давайте пусть будет задать конкретное регулярное выражение для валидатора или же какойто класс для связи, в одном случае у меня будет связь на пользователей, в другом на книжки (т.е 1 случай пользователь заносит в список, во втором случае "я читал"

@creocoder
Copy link
Contributor

У меня складывается ощущение, что вы хотите 3 различные модели уместить в одной. Это конечно может быть заманчиво и казаться "умным" ходом, но в целом это bad practice.

@lynicidn
Copy link
Contributor Author

специально ничего не искал, в поиске вбил yii2-user, первые 3 вариант:

@lynicidn
Copy link
Contributor Author

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

@creocoder
Copy link
Contributor

В модели можно определять виртуальные атрибуты и передавать любые данные посредством их использования.

@lynicidn
Copy link
Contributor Author

собственно все началось с обсуждения на русскоязычном канале когда то вот такого подхода - https://github.com/artkost/yii2-qa/blob/master/ActiveRecord.php#L25 и было предложено проанализировать, что если 2 модуля подключены одинаковых, но с разными конфигами

@lynicidn
Copy link
Contributor Author

В модели можно определять виртуальные атрибуты и передавать любые данные посредством их использования.

а как быть с зависимыми моделями? hasOne, к примеру

@creocoder
Copy link
Contributor

https://github.com/artkost/yii2-qa/blob/master/ActiveRecord.php#L25

👍 Если честно то сложно придумать, что то более отвратительное, чем это.

@lynicidn
Copy link
Contributor Author

я начал городить сначала, что у модуля свой контейнер di и он не статический (т.к. статический бы не решил проблему), но все уперлось в instantiate

и юзать чтото типа $module->createObject($ModelClass) и там бы в конструктор и падал модуль

@lynicidn
Copy link
Contributor Author

собственно, если уж и быть откровенным, то я хочу в модель передавать ничто иное как id module- ну думаю это уже понятно :)

@lynicidn
Copy link
Contributor Author

кстати идея с изолированными контейнерами у модуля, тоже имела бы право на жизнь, но как выше сказано instantiate портит всю малину в целом

@creocoder
Copy link
Contributor

https://github.com/robregonm/yii2-auth/blob/master/models/User.php#L168

Решается заведением в модели свойства static $tableName и передачей в него нужной информации (извне).

https://github.com/amnah/yii2-user/blob/master/models/User.php#L114

Решается либо заведением свойства $requiredAttribute, либо динамическим изменением правил валидации в модели с внешней стороны (такая возможность есть даже в Yii 1)

@lynicidn
Copy link
Contributor Author

передача нужной информации не будет работать со связанными моделями или мне каждый раз когда я вытаскиваю связанную модель передавать в нее модуль через статику?

@lynicidn
Copy link
Contributor Author

опять же статика не решит проблему 1 модель - несколько модулей

@lynicidn
Copy link
Contributor Author

можно конечно статику засунуть в трейт, ведь статика в трейте локальна для объекта, но опять же костыли

@creocoder
Copy link
Contributor

передавать в нее модуль через статику?

Передавать модуль в модель не нужно, это аналогично тому что вы передадите в модель контроллер или виды. ПОЛНОЕ нарушение MVC парадигмы. Нужно передавать конкретные данные необходимые для работы модели.

опять же статика не решит проблему 1 модель - несколько модулей

Shared модель для нескольких модулей это УЖЕ плохая идея сама по себе. Как вариант вместо статики можно пойти радикальным путем и генерировать модель в нужным tableName() один раз в соответствии с каким то шаблоном и настройками. Такая модель будет и быстрее и проще.

@lynicidn
Copy link
Contributor Author

хорошо, передавать модуль в модель плохая идея, но как конфигурировать модель? почему модель изолированна от внешних параметров? как быть если я хочу передать какойто параметр в релатед модель и этот параметр должен повлиять на инициализацию? Я и хочу передать модуль в модель как объект для конфигурации модели. Пусть это будет не модуль (раз нарушает парадигмы), а аля ConfigureObject

@creocoder
Copy link
Contributor

я хочу передать какойто параметр в релатед модель и этот параметр должен повлиять на инициализацию?

Это отдельный вопрос и над ним нужно подумать... В целом instantiate($row, $config = []) интересная идея, но во первых нужно убедиться, что этого нельзя достичь другими способами, во вторых нужен конкретный яркий (не нарушающий MVC) пример зачем это теоретически может понадобиться.

@lynicidn
Copy link
Contributor Author

интереснее я ничего не придумал :) ну, если не против пусть повисит пока открытым, может поддтянется народ к обсуждению

@creocoder
Copy link
Contributor

@lynicidn А вообще собственно в чем проблема?

    public static function instantiate($row)
    {
        return new static( //передавайте что хотите, можно использовать $this->... );
    }

@creocoder
Copy link
Contributor

$this? это статика!

$this это не статика а текущий инстанс объекта, но не суть.

как вариант передача регулярного выражения для логина в юзер модель, она нужна и в логин форме и регистрационной форме и в юзер моделе (или же создавать форму на добавление, тогда получается что модель у нас только для выборки) - в форму то передать модуль запросто

Мы же вели речь о проблемах передачи данных в related модели. Как это относится к этой теме? На текущий момент instantiate($row, $config = []) не совсем оправдан, т.к. этот $config можно запросто забрать из текущего инстанса.

    public static function instantiate($row)
    {
        return new static($this->config);
    }

например. Непонятна необходимость наличия $config в сигнатуре этого метода.

@creocoder
Copy link
Contributor

@lynicidn Сорри, не обратил внимания что метод статический )

@creocoder
Copy link
Contributor

@lynicidn Вероятно тут более рационально просто сделать метод нестатическим (не уверен). Т.к. даже если сделать $config в сигнатуре instantiate() то как это поможет? Чтобы им пользоваться придется перекрывать core классы и писать там свою логику по передаче этого $config. Может быть более рационально сделать instantiate($row, $parent). Где $parent это родительская модель из которой можно достать нужные для инстанса связанной данные.

@lynicidn
Copy link
Contributor Author

не статическим не получится, т.к. в query передается called class, я предлагаю к нему еще добавлять конфиг ModelClass::find(['property' => $var]), возможно и парент хорошая идея, но с конфигом вроде гибче

@lynicidn
Copy link
Contributor Author

сейчас набросаю коммит, для наглядности, что поменяется и как это скажется на BC

@klimov-paul
Copy link
Member

мы видимо о разном, вариант с записанием module_id в поле таблицы, я не считаю правильным

Забавно: создание динамических таблиц и нарушение принципов Active Record вы считаете нормальным, а дифференциацию данных по значению поля - не правильным.
Давайте попробуем такой вопрос: если есть товары разделенные по категориям, то как будет выглядеть структура базы данных? Неужели вы станете создавать динамические таблицы с названием каждой категории для сохранения товаров? Лично я бы просто ввел в единую таблицу "items" поле "categoryId" и все.

@lynicidn
Copy link
Contributor Author

динамическую таблицу не получится, т.к. static::tableName() ну вот в примере выше, мне надо по разному отреагировать на инициализацию модели, в зависимости от модуля которому она "принадлежит"

@lynicidn
Copy link
Contributor Author

выход есть всегда - расширить модель и перекрыть события новыми - согласен. Лишняя модель, т.е. возьми расширение и еще расширяй

@lynicidn
Copy link
Contributor Author

2 пример - 1 модель, 1 логика, работает с SenderInterface, но я хочу чтобы 1 модель отправляла смс уведомления, а другая email

@klimov-paul
Copy link
Member

2 пример - 1 модель, 1 логика, работает с SenderInterface, но я хочу чтобы 1 модель отправляла смс уведомления, а другая email

class MessageModel extends ActiveRecord
{
    public $messageStrategy;

    public function send()
    {
        switch ($this->messageStrategy) {
            case 'email' : ....
            case 'sms' : ....
        }
    }
}

class MessageController extends Controller
{
    public function send($id)
    {
        $model = $this->findModel($id);
        $model->messageStrategy = $this->getModule()->messageStrategy;
        $model->send();
    }
}

@lynicidn
Copy link
Contributor Author

т.е. со связями работать так? :

foreach ($group->getUsers() as $user) {
    $user->messageStrategy = 'email';
    $user->send();
}

@lynicidn
Copy link
Contributor Author

против:

$this->hasMany(['class' => User, 'messageStrategy' => 'email'], ....)

@klimov-paul
Copy link
Member

Думаю причина не приятия вами моих аргументов проста: вы создали решение, нарушив базовые принципы MVC, позволив модели напрямую взаимодействовать с модулем и забирать оттуда данные и параметры. Когда вы пришли к задаче подключения одного и того же модуля дважды, вместо того чтобы переосмыслить подход, вы ищете "костыль" который позволил бы вам все не переделывать.

@lynicidn
Copy link
Contributor Author

а разве модуль не подразумевает многократное использование? или же не более 1 раза в приложении?

@klimov-paul
Copy link
Member

т.е. со связями работать так?

Раз уж дошло до этого, то вероятно сам метод "send()" следует разместить в модуле (или его компоненте) а не в модели, тогда код будет выглядеть следующим образом:

foreach ($group->getUsers() as $user) {
    $this->getModule()->sendUserMessage($user);
}

@klimov-paul
Copy link
Member

а разве модуль не подразумевает многократное использование? или же не более 1 раза в приложении?

Это зависит от его реализации. Если вы хотите чтобы модуль можно было использовать дважды, то вы должны об этом позаботиться при его разработке.

@lynicidn
Copy link
Contributor Author

первый пример еще опишите, как бы вы решили и я отстаю

@klimov-paul
Copy link
Member

первый пример еще опишите

это какой?

@lynicidn
Copy link
Contributor Author

пример:
модуль имеет свойство $modelEvents
и через конфиг я устанавливаю эти события, одно из них EVENT_INIT
у меня 2 модуля и в каждом разная обработка этого события - модель одна, логика одна, а вот действие на событие разное, в зависимости от конфига

мне надо по разному отреагировать на инициализацию модели, в зависимости от модуля которому она "принадлежит"

@lynicidn
Copy link
Contributor Author

уменьшу - вообще как я могу задать логику модели для инициализации?

@klimov-paul
Copy link
Member

надо по разному отреагировать на инициализацию модели

Например?

задать логику модели для инициализации?

Конкретнее какие действия вы хотите произвести при инициализации модели? В чем их необходимость?

@samdark
Copy link
Member

samdark commented Jun 27, 2014

Мы с @lynicidn обсудим его задачи в Skype. Пока тикет ценности не несёт, формулировка задачи слишком расплывчатая. Закрываю. Если действительно будет задачка подходящая, я помогу оформить более чёткое описание.

@samdark samdark closed this as completed Jun 27, 2014
@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

@klimov-paul какой вариант ты считаешь правильным для конфигурирования имени класса и пк аттрибута у связанной модели, т.е. модуль каменты - модель Comment

function getUser()
{
    $this->hasOne(/*вот тут как сделать универсально, чтобы модуль можно было подключить без изменения кода модели*/)
}

@klimov-paul
Copy link
Member

#4078 (comment)

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

а класс какой указывать?

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

User class

@klimov-paul
Copy link
Member

Я вижу ты все еще не слушаешь.
Я уже достаточно времени потратил объясняя тебе базовые вещи. Перечитывай комментарии там все написано.

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

извини за тупость, но ткни плиз

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

как вариант делать $model->getUser($userModule)
а уже там $this->hasOne($userModule->userClass, [$userModule->userPkAttribute => 'id'])
но это не костыль?

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

если я связываю 2 модуля они не должны знать друг о друге явно и это должно конфигурироваться, разве нет?

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

в симфони продуманнее, через интерфейс и маппинг :( там связать бандлы можно

@lynicidn
Copy link
Contributor Author

lynicidn commented Jul 1, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants