Привет Хабр! Я понимаю, что история, о которой я хочу рассказать совсем обычная. У каждого программиста, работающего с Open Source, таких случаев до десяти на дню. Но я все равно решил о ней написать. Кому-то она реально поможет, а кому-то может просто улучшит настроение, что тоже неплохо.

Будет немного реверс-инжиниринга, немного философских размышлений, и конечно счастливый конец. Кому важно только исправление глюка – можете не читать весь этот бред и сразу скопировать хак из конца статьи. В любом случае, добро пожаловать под кат.

Немного про PrestaShop

Началось все с того, что начальство поставило задачу сделать Интернет-магазин. Выбор был сделан в пользу PrestaShop 1.6 по следующим причинам:

• Написано на PHP
• Адаптивный дизайн прямо из коробки
• Неплохо выглядит со стандартной темой (в том числе и на мобильных устройствах).
• Хорошо справилась с более 50 000 загруженных товаров
• Из коробки присутствует удобный и хорошо выглядящий блок фильтров (на языке PrestaShop он называется блок многоуровневой навигации)

С последним пунктом через некоторое время и возник вопрос, ставший предметом этой статьи.

В чем проявляется глюк

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

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

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

Здесь нужно отметить, что в PrestaShop есть понятия атрибутов и понятия свойств товара. Атрибуты (attributes) – это характеристики товара, которые участвуют в формировании разных версий одного и того же товара (например размер обуви для одной конкретной модели обуви). Свойства (features) — это характеристики, общие для всех вариантов товара. В формировании вариантов товара они не участвуют, а просто информируют пользователя о потребительских свойствах.

Ошибка проявляется, если вы отмечаете позиции в блоке фильтра по свойствам (features). В других фильтрах (например, по производителю) этот эффект не проявляется.

Первичные предположения

Стало ясно, что:

• Это глюк (так как с одной позицией все работает, то есть логика пересчета в код заложена)
• Этот глюк расположен в модуле многоуровневой навигации (blocklayered)
• Этот глюк скорее всего связан с неправильным построением условия SQL- запроса (объяснить я не могу, это больше на интуитивном уровне).

Поиски в Интернете ничего не дали, поэтому стоял выбор:

• Оставить все как есть, согласившись, что это особенность функционирования магазина.
• Залезть с головой в код и, проявив энтузиазм, найти причину глюка (и исправить).

Я принял второе решение. Энтузиазм убавился, когда я открыл файл blocklayered.php. Он содержал более 3,5 тысяч строк кода из которого 70% — многоэтажные SQL-запросы. Задача стала походить на поиски иголки в стоге сена. Поначалу я испугался, и даже нехорошо подумал о создателях PrestaShop. Но потом прикинул, как бы я стал программировать непростую логику работы подобного модуля и немного поуспокоился. Задача действительно сложная и, скорее всего, сложность кода вызвана объективными причинами. Но все равно, при работе с модулем не оставляла мысль, что можно сделать все это как-то покрасивее.

Инструменты и приемы

При решении проблемы будем использовать следующие инструменты:

WinSCP — надежный FTP-клиент с множеством функций. Ни разу не подводил даже на больших количествах файлов и объемах. Все функции доступны в том числе и из командной строки, что делает его полезным при написании скриптов.

UwAmp — простая в установке и настройке WAMP-сборка. Будем ее использовать для локального запуска исследуемого кода.

Notepad++ — отличный редактор для реверс-инжиниринга. Работа в разных кодировках и с разными концами строк. Хорошая подсветка синтаксиса. Открытие больших файлов. Поиск строк в том числе и по файлам в каталогах. Работает очень надежно.

HeidiSQL — GUI для MySQL. Бесплатный графический инструмент для баз данных. Иногда глючит, но в целом работать очень удобно. Используем его для изучения содержимого базы данных при анализе кода.

Основными приемами будет дамп переменных и поиск по исходному коду имен функций и кусков кода. Так как интересующие нас события происходят в том числе и в ajax-запросах дамп переменных делаем в файл. Для этого там где нужно вставляем следующий код:

$f=fopen('headfire.txt','a+');
fwrite($f,$very_important_variable);
fwrite($f, PHP_EOL);
fclose($f);

headfire в названии файла – мой псевдоним, я его использую, там где надо пометить именно мой код или файл. Вы должны использовать свою кодовую строку. Важно, чтобы ее легко было найти и трудно спутать с другими файлами или строками кода.

Начало анализа

Примерно половина кода отвечает за BackOffice. Этот код отсеиваем сразу и стараемся туда не заглядывать.

Раскрутку проблемы начинаем с поиска tpl-файла, ответственного за вывод наших фильтров на страницу. Искать долго не пришлось. Tpl-файл лежит в корне модуля и называется blocklayered.tpl. Заглянув в него убеждаемся, что в нем есть строчка вывода количества, которое у нас глючит.

<a href="{$value.link}" data-rel="{$value.rel}">{$value.name|escape:html:'UTF-8'}{if $layered_show_qties}<span> ({$value.nbr})</span>{/if}</a>

Краем глаза замечаем, что количество выводится по условию $layered_show_qties, а само количество имеет аббревиатуру nbr. Может это пригодится, а может нет.

Следующим шагом находим место, где вызывается шаблон blocklayered.tpl. Это оказывается функция

public function generateFiltersBlock($selected_filters);

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

public function getFilterBlock($selected_filters = array())

Эта функция занимает более 800 строк. В ней куча SQL-запросов. Скорее всего, здесь сосредоточена вся логика формирования фильтров. Что примечательно, что в модуле она вызывается 5 раз. Кажется, что слишком накладно вычислять столько запросов 5 раз подряд. Но потом замечаешь переменную

static $cache = null;

и понимаешь, что это старый добрый трюк с кэшированием в статической переменной. И еще понимаешь, что писали код отъявленные PHP-шники, которые не остановятся ни перед чем.

AND, OR и святая вода

Нужно как-то изучить работу функции. Глюк происходит в момент, когда в фильтре зажигается вторая галочка. А это сопровождается Ajax-запросом. Поэтому используем дамп переменной в файл.

С помощью крепкого кофе и танцев с бубнами вокруг газовой плиты находим место, где посылается основной запрос для каждого блока фильтра и вставляем туда отладочный код, выводящий этот запрос в файл (переменная $sql_query):

// headfire debug begin
$f=fopen('headfire.txt','a+');
fwrite($f,print_r($sql_query,true));
fwrite($f, PHP_EOL);
fclose($f);
// headfire debug end

$products = false;
if (!empty($sql_query['from']))
{
$products = Db::getInstance()->executeS($sql_query['select']."\n".$sql_query['from']."\n".$sql_query['join']."\n".$sql_query['where']."\n".$sql_query['group']);
}

Обратите внимание -$sql_query – массив. Это видно из кода, поэтому выводим в дамп с помощью print_r с флагом true.

Сразу первый же вывод в файл кричит нам о проблеме:

Array
(
    [select] => SELECT p.`id_product`, sa.`quantity`, sa.`out_of_stock` 
    [from] => 
					FROM ps_cat_restriction p
    [join] => LEFT JOIN `ps_stock_available` sa
						ON (sa.id_product = p.id_product AND sa.id_product_attribute=0  AND sa.id_shop = 1  AND sa.id_shop_group = 0 ) LEFT JOIN `ps_manufacturer` m ON (m.id_manufacturer = p.id_manufacturer) 
			INNER JOIN `ps_layered_price_index` psi ON (psi.id_product = p.id_product AND psi.id_currency = 1
			AND psi.price_min <= 3631136 AND psi.price_max >= 4618 AND psi.id_shop=1) 
    [where] => WHERE 1 AND EXISTS (SELECT * FROM ps_feature_product fp WHERE fp.id_product = p.id_product AND  fp.`id_feature_value` = 26634 OR fp.`id_feature_value` = 22096)
    [group] => 
    [second_query] => 
)

Обратите внимание на условие [where]: там написано в одну строчку AND и OR, причем OR находится между однородных условий и не выделено скобками.

fp.id_product = p.id_product AND  fp.`id_feature_value` = 26634 OR fp.`id_feature_value` = 22096

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

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

Сюрприз на последок: динамическая диспетчеризация

Пытаемся найти место, где формируется ошибочное условие. Используем для этого отрывки из трассировочного вывода. Поиск по 'fp.`id_feature_value` наводит нас на функцию:

private static function getId_featureFilterSubQuery($filter_value, $ignore_join = false)

Это то, что нужно. Видим код, который формирует условие без скобок и заменяем его на правильный. Попутно хочу заметить на то, как вставляются OR – они вставляются всегда, а последний OR откусывается (причем варварским методом).

  foreach ($filter_value as $filter_val)
	$query_filters .= 'fp.`id_feature_value` = '.(int)$filter_val.' OR ';
   $query_filters = rtrim($query_filters, 'OR ').') ';

Считаю это некрасивым. Поэтому переписываю этот кусок кода в своем стиле. Внизу приводится исходный и исправленный код функции.

//modules/blocklayered/block blocklayered.php

private static function getId_featureFilterSubQuery($filter_value, $ignore_join = false)
{
  if (empty($filter_value))
      return array();

  $query_filters = ' AND EXISTS (SELECT * FROM '._DB_PREFIX_.'feature_product fp WHERE    fp.id_product = p.id_product AND ';
  foreach ($filter_value as $filter_val)
	$query_filters .= 'fp.`id_feature_value` = '.(int)$filter_val.' OR ';
   $query_filters = rtrim($query_filters, 'OR ').') ';

   return array('where' => $query_filters);
	}

//modules/blocklayered/block blocklayered.php

private static function getId_featureFilterSubQuery($filter_value, $ignore_join = false)
{
  if (empty($filter_value))
      return array();

  //headfire hack begin
  $query_filters = ' AND EXISTS (SELECT * FROM '._DB_PREFIX_.'feature_product fp WHERE fp.id_product = p.id_product AND ';
  $query_filters1 = '';
  foreach ($filter_value as $filter_val) {
     if ($query_filters1)  $query_filters1 .= ' OR ';
     $query_filters1 .= 'fp.`id_feature_value` = '.(int)$filter_val;
   }
   $query_filters .= '( '.$query_filters1.' )'.')';
 // headfire hack end
		
  return array('where' => $query_filters);
 }

А сюрприз заключается в том, что в модуле эта функция напрямую нигде не вызывается. Корявое имя наводит на грустные мысли о том, что оно где-то формируется программно и вызывается динамически. Так оно и есть, причем в остальных подобных местах сделаны честные case. А здесь решили поиграть с динамической диспетчеризацией.

Конец истории

Глюк исправлен. Фильтры стали работать красиво. Возможно эта ошибка уже решена в новых релизах PrestaShop. Ну а если нет и у Вас вскрылась похожая проблема – я рад если смог Вам помочь. И еще – не скупитесь на скобки, даже если порядок действий очевиден.