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

Ошибка при наследовании модуля плагина работающего на орм и с указанием кастомных имен таблиц через конфиг #150

Open
psnet opened this issue Jul 5, 2015 · 4 comments

Comments

@psnet
Copy link

psnet commented Jul 5, 2015

По-умолчанию таблицы орм для плагинов в лс 2.0 будут именоваться не так как в 1.0.3, а с именем префикса плагина, то этот баг для сложных проектов был бы очень веселой штукой.

Условия задачи

Наследовать орм модуль плагина, который дает свои имена таблицам плагина через конфиг. Чтобы было более понятно, то пример будет такой: есть плагин A с орм модулем и сущностью:

PluginA_ModuleA
PluginA_ModuleA_EntityA

и есть плагин B, который наследует только модуль плагина A:

PluginB_ModuleB -> PluginA_ModuleA

и для плагина A в конфиге указаны кастомные имена таблиц для сущностей.

Поехали

В орм в методах таких как GetByFilter, GetItemsByFilter и других есть конструкции:

if (is_null($sEntityFull)) {
  $sEntityFull=Engine::GetPluginPrefix($this).'Module'.Engine::GetModuleName($this).'_Entity'.Engine::GetModuleName(get_class($this));
} elseif (!substr_count($sEntityFull,'_')) {
  $sEntityFull=Engine::GetPluginPrefix($this).'Module'.Engine::GetModuleName($this).'_Entity'.$sEntityFull;
}

Нетрудно догадаться что при наследовании модуля плагина А переменная $sEntityFull получит префикс плагина В, который наследует исходный и в итоге будет получена сущность, которая реально не существует вообще - PluginB_ModuleB_EntityA. Далее эта переменная передается в маппер, где происходит получение этой не существующей сущности:

$oEntitySample=Engine::GetEntity($sEntityFull);

И тут вполне ожидаемо было бы ждать ексепшн, т.к. сущности такой нет, это ошибка, но тут работает самый давний, самый натуральный что ни есть хак, на который я всегда косо смотрел:

/**
 * If Plugin Entity doesn't exist, search among it's Module delegates
 */
if(isset($sPlugin) && !self::GetClassPath($sClass)) {
  $aModulesChain = Engine::GetInstance()->Plugin_GetDelegationChain('module','Plugin'.$sPlugin.'_Module'.$sModule);
  foreach($aModulesChain as $sModuleName) {
    $sClassTest=$sModuleName.'_Entity'.$sEntity;
    if(self::GetClassPath($sClassTest)) {
      $sClass=$sClassTest;
      break;
    }
  }
  if(!self::GetClassPath($sClass)) {
    $sClass='Module'.$sModule.'_Entity'.$sEntity;
  }
}

И ядро находит таки сущность среди сущностей делегируемых модулей плагина. Уже на данном этапе это проблема в архитектуре, которая должна быть решена, это неверное поведение.

Далее вызов обратно возвращается в орм маппер, где идет получение таблицы, но не с полученной корректной сущности от ядра, а снова от её (ошибочного) имени:

$sTableName = self::GetTableName($sEntityFull);

естественно метод GetTableName разбирает имя на составляющие и не найдет таблицу исходного плагина A для орм модуля, т.к. будет искать с именем плагина B, который наследует оригинальный и работа завершится ошибкой не нахождения таблицы.

Варианты решения

  • Заменить в орм маппере движка вызовы
$sTableName = self::GetTableName($sEntityFull);

на

$sTableName = self::GetTableName($oEntitySample);

это как минимум должно быть сделано в движке для новой версии лс. Примечательно что в некоторых методах именно так и сделано, например, в методе получения полей таблицы ShowColumnsFrom, получения примари ключа ShowPrimaryIndexFrom или просто добавления/обновления сущностей, но там изначально передается в методы сама сущность.

  • Сделать правку выше + исправить все места формирования сущности таким образом, чтобы избавится от вышеуказанного хака в движке и вынести формирование полного имени сущности:
if (is_null($sEntityFull)) {
  $sEntityFull=Engine::GetPluginPrefix($this).'Module'.Engine::GetModuleName($this).'_Entity'.Engine::GetModuleName(get_class($this));
} elseif (!substr_count($sEntityFull,'_')) {
  $sEntityFull=Engine::GetPluginPrefix($this).'Module'.Engine::GetModuleName($this).'_Entity'.$sEntityFull;
}

в методах орм в отдельный метод, который и получит корректную сущность, даже если он будет содержать тот же код хака из ядра, но это будет значительно лучше т.к. дальше по коду в методах орм (например, все тот же GetItemsByFilter) идет сессионный кеш:

$sEntityFullRoot=$this->Plugin_GetRootDelegater('entity',$sEntityFull);
$sCacheKey=$sEntityFullRoot.'_items_by_filter_'.serialize($aFilter);
$aCacheTags=array($sEntityFullRoot.'_save',$sEntityFullRoot.'_delete');

и он строится на неверном имени сущности и ключ и теги кеша получаются тоже не корректные (GetRootDelegater вернет тоже имя т.к. нету такой сущности и нет, соответственно, исходной).

  • А разработчикам придется писать в своих плагинах обходной временной хак с маппингом имени таблицы для каждой сущности оригинального плагина (А) на имя плагина-наследника (В):
$config['$root$']['db']['table']['ПлагинB_МодульB_СущностьА'] = '___db.table.ПлагинА_МодульА_СущностьА___';

Оригинал статьи

@psnet
Copy link
Author

psnet commented Jul 5, 2015

Хочу заметить, что данный баг критичен и потребует выпуска фикса для первой версии лс т.к. теги кеша орм сущностей (сущность_save, сущность_delete) формируются с ошибкой и поэтому при включенном кешировании кеш плагина частично не будет сброшен при обновлении/удалении сущности и это приведет к самым неожиданным ситуациям ("зависание" некоторых объектов и др.)

@mzhelskiy
Copy link
Contributor

По тексту так и не понял - это актуально для 2.0 или нет?

@psnet
Copy link
Author

psnet commented Jul 9, 2015

Нет, в 2.0 уже исправлено в т.ч. и вынесен код формирования полного имени сущности в отдельный метод _NormalizeEntityRootName который корректно получает сущность благодаря поэтапному получению корневого класса модуля и сущности:

        /**
         * Получаем корневой модуль
         */
        $sModuleRoot = $this->Plugin_GetRootDelegater('module', $sPluginPrefix . 'Module' . $sModuleName);
        /**
         * Возвращаем корневую сущность
         */
        return $this->Plugin_GetRootDelegater('entity', $sModuleRoot . '_Entity' . $sEntityName);

а не всей сущности сразу как в 1*:

$sEntityFullRoot=$this->Plugin_GetRootDelegater('entity',$sEntityFull);

@psnet
Copy link
Author

psnet commented Jul 9, 2015

Но в таком случае для 2.0 также нужно избавиться от хака в Engine::GetEntity:

/**
 * If Plugin Entity doesn't exist, search among it's Module delegates
 */
if(isset($sPlugin) && !self::GetClassPath($sClass)) {
  $aModulesChain = Engine::GetInstance()->Plugin_GetDelegationChain('module','Plugin'.$sPlugin.'_Module'.$sModule);
  foreach($aModulesChain as $sModuleName) {
    $sClassTest=$sModuleName.'_Entity'.$sEntity;
    if(self::GetClassPath($sClassTest)) {
      $sClass=$sClassTest;
      break;
    }
  }
  if(!self::GetClassPath($sClass)) {
    $sClass='Module'.$sModule.'_Entity'.$sEntity;
  }
}

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

2 participants