Как не надо расширять MODX-процессоры

В MODX-2.4.0 появился новый процессор updatefromelement.class.php by Argnist, пришедший на замену обычному процессору updatefromelement.php. Заменять non-classed процессоры конечно дело хорошее, но делать надо это крайне осторожно и обдуманно.

Сразу уточню, что этот процессор используется для обновления параметров элементов (типа шаблонов, сниппетов и т.п.)


Так вот, в чем же неприятность данного процессора? А в том, что расширяет он процессор update.class.php, который обновлять должен объекты с классом modPropertySet, а на практике у нас получается, что новый процессор обновляет чаще элементы типа modTemplate, modSnippet и т.п.

Рассмотрим эту проблему детальней. Суть в том, что когда мы редактируем какой-то элемент (шаблон, к примеру), создаем или редактируем в нем параметры, то эти параметры для сохранения отправляются именно на этот процессор. И что там происходит? Там на самом деле выполняется очень сложная (с точки зрения логики) задача — Есть сам элемент (modTemplate), а есть наборы параметров (modPropertySet). (Есть еще для них связка modElementPropertySet, но она нам сейчас не интересна). Так вот, реально обновлять modPropertySet нужно только в том случае, если мы обновляем именно отдельный объект набора параметров.

А вот когда у нас не выбран набор параметров, тогда обновляется именно объект самого элемента modTemplate. А как мы посмотрели выше, процессор этот обновляет объекты с классом modPropertySet. А где он здесь, если мы его не выбрали? Как бы и нигде. Но на самом деле есть — первый же попавшийся набор параметров… Почему так происходит? На самом деле тут наследие от давнего старого минуса xPDO — это отсутствие контроля типов переменных для первичных ключей. К примеру, попробуйте выполнить так: $o = $modx->getObject('modResource', 'любая произвольная строка'). Как вы думаете, получите вы таким образом объект? К сожалению, да. Опять-таки — первый попавшийся. А вот если вот так сделать, то уже не будет: $o = $modx->getObject('modResource', (int)'любая произвольная строка'). То есть строка будет преобразована в 0, а с этим id вряд ли у вас документ есть. Так вот, когда мы запрос шлем на сервер, то там такие данные передаются:

Самое интересное там для нас — id: По умолчанию. Это еще одна бага озвученного процессора, так как там проверка идет для Default, которого просто нет, когда язык панели ru.

Короче, из-за всего этого у меня слетало главное меню (так как затирался набор параметров для Wayfinder) и пропала возможность редактировать существующие параметры шаблонов. Очень неприятно.

Из всего написанного выше можно извлечь главное правило: обновляться должен только главный объект, и только с ним уже можно обновлять сопутствующие объекты. В данном же процессоре логика была типа «можно обновить дополнительный объект шаблона без сохранения главного объекта наборов параметров, если они не были указаны», что как бы не соответствует реалиям. В виду этого здесь видится только два основных решения:
1. Или плодить дубли кода в двух отдельных процессорах (для обновления параметров один, а для обновления объекта — другой).
2. Или в процессоре вызывать еще один процессор (что тоже достаточно логично).

Но я попробовал все-таки модернизировать текущий процессор, чтобы он и параметры объекта обновлял, и наборы параметров (в зависимости от того, указаны они или нет). Суть его сводится к тому, что если не указан набор параметров, то $this->classKey меняется на класс обновляемого элемента, что переводит процессор из редактора набора параметров в редактор элемента. Вот код gist.github.com/Fi1osof/ff3ea018841b1bb1f99b

Я не стал его слать пуллреквестом, так как код не очень чистый и без локализаций. Виталий, раз ты это все делал, значит боле менее понимаешь как и что там работает. Потестируй и доработай свой процессор, плиз.

UPD: Поступил переводот Andreas Wettainen aka mrhaw. Большое ему спасибо за это! Отправил PR. Райну написал. Надеюсь скоро примут и опубликуют.
Fi1osof
22 августа 2015, 14:18
modx.pro
8
3 823
+10
Поблагодарить автора Отправить деньги

Комментарии: 22

Василий Наумкин
22 августа 2015, 17:25
+3
Спасибо за информацию!

Я немного отформатировал твой текст и залил картинки на наш файловый хостинг — так удобнее, и текст немного разбавляется. Надеюсь, ты не против.
    Fi1osof
    22 августа 2015, 17:27
    +3
    Всегда пожалуйста!

    Нет, не против.
    Александр
    22 августа 2015, 23:00
    +2
    Походу релиз 2.4.0 был в пьяном угаре. Фундаментальную вещь сломали по сути, сколько сайтов упало наверное… А есть обсуждение на эту тему у тех кто такой релиз выпустил?
      Fi1osof
      22 августа 2015, 23:03
      0
      Чессказать хз. Последнее время мало где общаюсь. Райну сообщил.
      К слову, у меня пока больше ничего не сломалось. Надеюсь это была последняя бага для меня в этом релизе.
      Сергей Шлоков
      23 августа 2015, 08:53
      0
      Зашел прочитать статью «Как не надо расширять MODX-процессоры», а тут баг репорт.
        Fi1osof
        23 августа 2015, 10:06
        0
        Это все же пример того, как не надо расширять процессоры. Если вы не учтете эти рекомендации и сделаете что-то подобное, то чтобы вы там не обновляли, это будет из той же серии.
          Сергей Шлоков
          23 августа 2015, 10:35
          0
          Тут же проблема не в самом принципе расширения, а в том, что не до конца продуман функционал. С таким же успехом можно было бы озаглавить топик «Как не надо программировать в MODX» или вообще обобщить «Как не надо программировать».
          Прошу не относится критически, просто размышление.
          П.С. Но к содержанию претензий нет, поэтому плюсанул.
            Fi1osof
            23 августа 2015, 10:40
            +2
            Нет, тут проблема в первую очередь именно в принципе расширения. Еще раз уточню: нельзя для работы с одними объектами расширять процессоры, предназначенные для работы с другими процессорами. Если рассматриваемый здесь update.class.php был создан для обновления объектов класса modPropertySet, то расширяющий процессор и должен был использоваться только для обновления объектов этого класса. А здесь же была попытка использовать его для обновления modTemplate, modSnippet и т.п. Это именно проблема самого подхода. А в топике я всего лишь подробней рассказал почему так не стоит делать.

            П.С. я не отношусь критически. Я просто объясняю.
              Fi1osof
              23 августа 2015, 10:42
              +2
              Вообще, в поисках более фэншуйного подхода для реализации данной задачи правильней было бы использовать метод modProcessor::getInstance, и в зависимости от того, что надо было обновлять, переключаться на тот или иной процессор.
                Сергей Шлоков
                23 августа 2015, 10:49
                0
                Понял.
          Николай
          23 августа 2015, 18:26
          0
          Николай, спасибо за то, что нашли баг!
          Баг подтверждаю горьким опытом. Заметил, что у меня при сохранении любого шаблона затирается один из параметров набора параметров. Остальное вроде работает.
          Обидно, что после обновления на 2.4.0 сделал уже много изменений по сайту и откат на старую версию означает сброс всех внесенных правок.

          Василий, можете в посте добавить жирным куда-то, что после обновления на 2.4 необходимо проверять на сохранность все наборы параметров после сохранения любого элемента (шаблон, снипет и т.п.)

          Это реально опасный баг тем, что портит не то с чем работаешь, поэтом сразу не заметный.
            Fi1osof
            23 августа 2015, 18:31
            1
            +1
            На самом деле предложенный багфикс вполне может спасти вашу работу. gist.github.com/Fi1osof/ff3ea018841b1bb1f99b
            Обновите этот класс и все. Других ошибок я не обнаружил. Так же при наличии файла core/model/modx/processors/element/propertyset/update.class.php (если вы обновлялись, а не устанавливали с нуля, должен присутствовать), можно просто удалить файл core/model/modx/processors/element/propertyset/updatefromelement.class.php, все должно работать.
              Николай
              23 августа 2015, 18:57
              0
              Николай, воспользовался вашим багфиксом, набор параметров перестал очищаться. Каких-то ошибок не заметил.
              Возможно еще люди согласятся потестировать и можно все же пул реквест отправлять, а то там долго будут переписывать.
                Fi1osof
                23 августа 2015, 19:01
                0
                Мне реально некогда красотой заниматься (лексиконы там прописывать и т.п.). Поэтому и выложил так код отдельно. Может кто причешет да отправит PR. Если кто отправит, маякните здесь, я Райну напишу чтобы по возможности быстрее накатили.
              Василий Наумкин
              23 августа 2015, 18:59
              +2
              Добавил жирным в нашем анонсе со ссылкой на эту статью.
              Василий Наумкин
              23 августа 2015, 19:02
              +1
              А вот и в репозиторий MODX добавили issue.
                Fi1osof
                28 августа 2015, 10:18
                +2
                Поступил переводот Andreas Wettainen aka mrhaw. Большое ему спасибо за это! Отправил PR. Райну написал. Надеюсь скоро примут и опубликуют.
                  r5uY40
                  09 сентября 2015, 16:34
                  0
                  Сейчас уже можно ставить 2.4.0-pl? Пофиксили баг?
                  Николай Савин
                  10 сентября 2015, 16:23
                  +3
                  Не нашел в этой ветке пошаговое решение проблемы для «Чайников» вроде меня. Спасибо Василию, наставил на путь.

                  Думаю будет полезно отписаться.
                  Насколько я понял для фикса бага надо:
                  1. Открываем файл /core/model/modx/processors/element/propertyset/updatefromelement.class.php
                  2. Заменяем его содержимое на это gist.github.com/Fi1osof/ff3ea018841b1bb1f99b
                    r5uY40
                    10 сентября 2015, 16:25
                    0
                    Спасибо, то что надо.
                      Fi1osof
                      10 сентября 2015, 18:04
                      0
                      Да, все верно.
                      Авторизуйтесь или зарегистрируйтесь, чтобы оставлять комментарии.
                      22