среда, 17 августа 2016 г.

XC4: Как использовать динамическую патчилку для скинов при написании модуля

При генерации установочного пакета согласно
http://help.x-cart.com/index.php?title=X-Cart:Generating_module_distribution_packs_for_X-Cart_4

приходится генерировать diff на каждый стандартный скин. При этом не учитываются кустомы и 3rd-party скины, что ведет к проблемам при установке модуля.

Для решения этой проблемы можно использовать
smarty prefilter/outputfilters

которые вставляют точки входа модуля в текущий скин в момент компиляции(преподчтительно) или в момент вывода скина.

Для это можно использовать готовую библиотеку
http://help.x-cart.com/index.php?title=File:Dynamic_tpl_patcher.4.7.x.php
или
http://help.x-cart.com/index.php?title=File:Dynamic_tpl_patcher.4.6.x.php

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

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

Пример использования префильтров


1)в функции init модуля func_MODULE_init добавляем код

    if (AREA_TYPE == 'C') {
        require $xcart_dir . XC_DS . 'modules' . XC_DS . 'Pilibaba' . XC_DS . 'lib' . XC_DS . 'dynamic_tpl_patcher.php';
        modules\Pilibaba\lib\x_tpl_add_callback_patch('customer/main/cart.tpl', 'func_pilibaba_tpl_insertButton', X_TPL_PREFILTER);
        modules\Pilibaba\lib\x_tpl_add_callback_patch('customer/minicart.tpl', 'func_pilibaba_tpl_insertButton', X_TPL_PREFILTER);
        modules\Pilibaba\lib\x_tpl_add_callback_patch('modules/Add_to_cart_popup/product_added.tpl', 'func_pilibaba_tpl_insertButton', X_TPL_PREFILTER);
    }
 
2) в func_pilibaba_tpl_insertButton добавить

function func_pilibaba_tpl_insertButton($tpl_name, $tpl_source) {//{{{
    if (strpos($tpl_source, 'pilibaba_enabled') !== false) {
        return $tpl_source;
    }

    // add button {include file="modules/Pilibaba/checkout_btn.tpl" btn_place=...
    $search = '%{if \$amazon_pa_enabled}[^{]*{include file="modules/Amazon_Payments_Advanced/checkout_btn.tpl"[^{]*{/if}%Ss';
    $tpl_source = preg_replace_callback($search,
        function ($matches) {
            return $matches[0] . "\n" . str_replace(
                array('amazon_pa_enabled', 'Amazon_Payments_Advanced'),
                array('pilibaba_enabled','Pilibaba'), $matches[0]);
        },
        $tpl_source
    );

    $tpl_source = str_replace('{if $paypal_express_active || $amazon_pa_enabled', '{if $paypal_express_active || $amazon_pa_enabled || $pilibaba_enabled', $tpl_source);

    if (defined('XC_PILIBABA_DEBUG')) {
       x_log_add('pilibaba_patched_fiels', 'patched_file:' . $tpl_name . "\n" . $tpl_source);
    }

    return $tpl_source;
}//}}} 
 
 

Пример использования output фильтра, условно запускающегося только на некоторых страницах


if (
    'C' != x_get_area_type()
    && x_check_controller_condition(NULL, array('register', 'user_modify'))
) {
    x_tpl_add_regexp_patch(
        'modules/XAuth/linked_accounts_admin.tpl',
        '/(<form [^>]*name="registerform"[^>]*>.+)(<tr>.+<\/tr>)/USs',
'\1%%\2' ); } x_tpl_add_listener('modules/XAuth/register.tpl', 'before', 'func_xauth_prepare_register'); x_tpl_add_listener('modules/One_Page_Checkout/profile/account_info.tpl', 'before', 'func_xauth_prepare_register');

вторник, 21 июля 2015 г.

XC4: Как найти патч на решенную проблему в BT/Git. Использование BT как базы знаний.

Входные данные 

Пустая страница при добавлении продукта, если включены Socialize + Add To Cart Popup, причем последний должен быть настроен отображать "Customers who bought this also bought + Random products", с другими настройками не воспроизводится.
На 4.7.3 все ок
Текущая версия 4.6.1


Алгоритм

1) используем фильтр для облегчения поиска уже решенных тикетов
https://sd.x-cart.com/view_all_set.php?project_id=39&source_query_id=5549&type=3


2) там по ключевому слову
Socialize%add%cart
вышло около 10 тикетов

3)по названию сразу видно что решение может быть в
https://sd.x-cart.com/view.php?id=137402
Add To Cart Popup vs Socialize

или в

https://sd.x-cart.com/view.php?id=138302
Add To Cart Popup vs Socialize найти причину бага


4)дальше либо смотреть тикеты в поисках патча
либо
можно сделать так


aim-server[~/www/xcart_4_6_x]$ git log -p --all --name-status --pretty=oneline -i --grep=138302
      ничего не нашли
aim-server[~/www/xcart_4_6_x]$ git log -p --all --name-status --pretty=oneline -i --grep=137402
068e287de35808bddd9e529ce0a5e46e4b197eee  M:0137402 [!] Bug: *DESIGN AND APPEARANCE* Add To Cart Popup vs Socialize problem.Blank page was fixed on the Add To Cart Popup page when "Show social buttons on the matrix products list" option was used.
M include/func/func.dev.php
M include/templater/plugins/function.include_cache.php
M skin/common_files/customer/main/buy_now.tpl
M skin/common_files/modules/Add_to_cart_popup/buy.tpl
M skin/common_files/modules/Add_to_cart_popup/product_added.tpl
M skin/ideal_comfort/customer/main/buy_now.tpl
M skin/ideal_responsive/customer/main/buy_now.tpl
M skin/ideal_responsive/modules/Add_to_cart_popup/product_added.tpl
M skin/vivid_dreams_aquamarine/customer/main/buy_now.tpl
M skin/vivid_dreams_chromo/customer/main/buy_now.tpl
M skin/vivid_dreams_lotus/customer/main/buy_now.tpl
M skin/vivid_dreams_violet/customer/main/buy_now.tpl
M tests/functest.func_tpl_get_all_variables
aim-server[~/www/xcart_4_6_x]$


5)потенциальный патч
068e287de35808bddd9e529ce0a5e46e4b197eee  M:0137402 [!] Bug: *DESIGN AND APPEARANCE* Add To Cart Popup vs Socialize problem.Blank page was fixed on the Add To Cart Popup page when "Show social buttons on the matrix products list" option was used.




6)проверяем версию куда вошел коммит
aim-server[~/www/xcart_4_6_x]$ git describe --contains --exact-match --match='xcart*' 068e287de35808bddd9e529ce0a5e46e4b197eee
xcart_4_6_2~99

да, в 461 этого коммита нет
также в тикетах есть поля типа
 Product Version
и
 Fixed in Version 

вероятность что фиксит проблему очень высока, так как текущая версия 4.6.1



7)иногда быстрее выполнить поиск по git, чем использовать bt
к примеру этот же самый коммит очень быстро находится одной командой 
aim-server[~/www/xcart_4_6_x]$ git log -p --all --name-status --pretty=oneline -i --grep='socialize.*add.*cart'    
068e287de35808bddd9e529ce0a5e46e4b197eee M:0137402 [!] Bug: *DESIGN AND APPEARANCE* Add To Cart Popup vs Socialize problem.Blank page was fixed on the Add To Cart Popup page when "Show social buttons on the matrix products list" option was used.
M include/func/func.dev.php
M include/templater/plugins/function.include_cache.php
M skin/common_files/customer/main/buy_now.tpl
M skin/common_files/modules/Add_to_cart_popup/buy.tpl
M skin/common_files/modules/Add_to_cart_popup/product_added.tpl
M skin/ideal_comfort/customer/main/buy_now.tpl
M skin/ideal_responsive/customer/main/buy_now.tpl
M skin/ideal_responsive/modules/Add_to_cart_popup/product_added.tpl
M skin/vivid_dreams_aquamarine/customer/main/buy_now.tpl
M skin/vivid_dreams_chromo/customer/main/buy_now.tpl
M skin/vivid_dreams_lotus/customer/main/buy_now.tpl
M skin/vivid_dreams_violet/customer/main/buy_now.tpl
M tests/functest.func_tpl_get_all_variables

четверг, 16 апреля 2015 г.

XC4: что должно быть в config.php/init.php модуля ?

что должно быть в config.php/init.php модуля ?

отвечу, что не должно быть

Не должно быть действий которые зависят от других файлов.
config.php должен быть полностью независимым в том числе и от себя самого и от своего собственного func.php

Вообще в init.php config.php незачем пихать вобще какие-то глобальные переменные/ смарти переменные/ вызовы функций

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

к примеру когда работает image.php или ajax get_block.php он может проинициализировать все активные модули, хотя это реально не нужно.

Отлично если от init.php модуль вообще отказался.

Что влечет несоблюдение этих правил ?

1 тормоза

2 fatal errors

3 логические баги. Когда время жизни переменной имеет огромное значение на несколько тысяч строк. В течении которых с переменной может случиться все что угодно. И ядро не может гарантировать модулю, что значение переменной не поменяется в каком-то темном закаулке. Модуль сам себя должен обеспечивать приемлемой устойчивостью к багам.

4 Проблемы из-за последовательности инициализации модулей


примеры

1) Так нельзя это Fatal error
/*
Load module functions
*/
if (!empty($include_func))
require_once $_module_dir . XC_DS . 'func.php';

if (func_xpc_check_iframe_methods()) {
$smarty->assign('xpc_iframe_methods', 'Y');
}

?>
"modules/XPayments_Connector/config.php" 61L, 3167C




2) Так нельзя это Fatal error и тормоза
.......
x_session_register('cart');

if (!defined('IS_ERROR_MESSAGE'))
include $xcart_dir.'/modules/Google_Checkout/gcheckout_button.php';

"modules/Google_Checkout/init.php" line 77 of 149 --51%-- col 1



3) Это fatal error

if (defined('AREA_TYPE') && in_array(constant('AREA_TYPE'), array('A', 'P'))) {
// Process changes on the module options page
if (isset($_GET['option']) && $_GET['option'] == 'Sitemap') {

if ($_SERVER['REQUEST_METHOD'] == 'POST' && isset($_POST['sitemap'])) {

if (!isset($_POST['sitemap'])) {
$sitemap_error = func_get_langvar_by_name('lbl_permission_denied');
} else {
switch ($_POST['sitemap']['config']) {
case 'add':
$sitemap_error = sitemap_extra_addurl($_POST['sitemap']['add']);
break;

case 'delete':
$sitemap_error = sitemap_extra_delurls($_POST['sitemap']['delete']);
break;

case 'update':
$sitemap_error = sitemap_extra_updateurls($_POST['sitemap']['update']);
break;

case 'generate_cache':
$sitemap_error = sitemap_start_generate_cache($config['sitemap']['cache_limit_general'], $config['sitemap']['cache_limit_categories']);

default:
break;
"modules/Sitemap/init.php"

XC4: Тестирование, выполняемое разработчиками.Чистые тесты.Эволюция ПП

Тестирование, выполняемое разработчиками

Разработчики обычно выполняют "чистые тесты" Разработчики склонны тестировать код на предмет того, работает ли он (чистые тесты), а не пытаться нарушить его работу всевозможными способами (грязные тесты). При незрелом процессе тестирования обычно выполняют около пяти чистых тестов на каждый грязный. В организациях со зрелым процессом тестирования на каждый чистый тест обычно приходятся пять грязных. Это отношение изменяется на противоположное не за счет снижения числа чистых тестов, а за счет создания в 25 раз большего числа грязных тестов ([Boris Beizer]).

Разработчики обычно идеализируют свой код. Они скорее проверяют, что код работает, чем он не работает. Но если код работает, то тестировщик(бывший разработчик) некачественно делает свою работу.Тк цель тестирования именно найти ошибки. Если разработчик рассчитывает, что в его коде нет ошибок, то он их там и не найдет.

Переключаться между двумя ипостасями хороший_тестировщик - хороший_разработчик сложно, но необходимо.

В X-Cart нет полноценного масштабного тестирования, поэтому основная надежда именно на тестирование, проводимое разработчиком.



Введена рекомендация
*Для каждой контрибуции указывается число чистых и грязных тестов.Рекомендуется кратко описывать все найденные и исправленные ошибки/проектные решения/исключения/предположения.


Соотношение грязь/чистота позволяет быть честным по отношению к себе.И быть средством самоконтроля.

Если ты проверил успешную регистрацию, проверил успешный checkout, создание ордера
ты сделал 3 чистых теста, но 0 грязных.

Для данного примера соотношение должно быть
3/15 или 3/12

Соотношение 3/0 показывает показывает, что тестирование проведено некачественно.
Мы только нашли узкую тропинку, по которой контрибуция работает,но не нашли огромное кол-во тропинок, когда контрибуция "заблудится"/не работает.
Мы не очертили границы контрибуции, мы не знаем, что она может делать, а что нет.
Значит контрибуция не работает.

Соотношения хуже 1:5-1:4 говорят, что контрибуция некачественная.

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


Одно из важнейших правил эволюционного развития ПП
Одно из важнейших правил эволюционного развития ПП
Главное ничего не сломать, а потом можно уже что-то менять/добавлять.


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

Как подходить к выбору грязных/чистых тестов ?
Для каждой контрибуции есть область изменений - это каждая точка подключения/ каждое обращение к модулю.
и собственно сам модуль


-Области изменений
Нужно поднятся на 1-2-3 уровня места интеграции модуля и проверить все фишки, которые относятся к этому уровню.
Это кстати повод делать модуль, как можно более независимым.


-Сам модуль
Тут просто тестирование самого модуля, согласно спецификации.

XC4: Тестирование кода/модульность/читабельность/запахи кода

Контрибьютерам/приемщикам в XC

Тестирование кода/модульность/читабельность/запахи кода


подходи к любому изменению - как к источнику багов

если ты не нашел багов - значит плохо поработал
баги есть всегда
весь вопрос в их серьезности



если изменений слишком много
то очень высока вероятность ошибок


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

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

требуй хорошей читабельности и модульности - это гарантия что ты найдешь все баги


если читабельность/модульность плохая,
то переделывай


плохая читабельность/модульность это всегда баг, если не сейчас, то при следующем изменении


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


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

Правильно найти и устранить корневую причину - кучу дерьма
нежели фиксить сам запах.

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

Без решения родительской-корневой причины фиксить дочерние причины методом заплаток - это неэффективная работа программиста.

XC4: Оформление точки входа модуля.Глобальные переменные

правильно думаю так
if (
active_module_XXX
&& func_XXX_is_allowed_something(common_vars)
) {
func_XXX_do_something1(common_vars);
}

где
func_XXX_do_something существующая функция, которая зависит от переменных ядра



Вопрос1
а как лучше передавать глобальные переменные? Если, например, мне надо записать результат в $smarty. Вызывать ее в функции как global или передавать через переменную по ссылке? и $xcart_dir - ее то наверно можно вызывать как global?

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

новых заводить нельзя

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


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

Комментарий1
> Если, например, мне надо записать результат в $smarty
Считаю, что как объект (который в РНР передается по ссылке)

Вдруг, рано или поздно, у нас будет несколько объектов Smarty, например,
class MailTemplater extens Smarty
и
class HTMLTemplater extens Smarty
- один для подготовки mail body, другой - для отображения в браузер.
В этом случае не придется модифицировать функцию, в зависимости от назначения твоей функции, приедется вызывать её либо дважды, либо единожды:

func_by_chemisk( $smarty, $params);
func_by_chemisk( $mail_smarty, $params);

Комментарий2
А вообще, мне не нравятся многоэтажные условия в if()
Мне не нравится, что код загроможден большим количеством однотипных конструкций вида
if (!empty($active_modules['Some_Module']))
some_function( $arg1, $arg2, $arg3);

Предлагаю вот такое решение чтобы убрать лишний код.
function func_if_module( $module, $function) { // {{{
global $active_modules;
assert( 'isset( $active_modules) && is_array( $active_modules)');
assert( func_num_args()." >= 2");
$arg_list = func_get_args();
$module = array_shift( $arg_list);
$function = array_shift( $arg_list);
if( !empty( $active_modules[ $module]) && $active_modules[ $module] == "Y") {
assert( 'function_exists( $function)');
return call_user_func_array( $function, $arg_list);
}
} // }}}

Комментарий3
когда ты пишешь в коде
func_if_module( "Some_Module", "some_function", $arg1, $arg2, $arg3);

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

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

в этом смысле код

if (
active_module_XXX
&& func_XXX_is_allowed_something(common_vars)
) {
func_XXX_do_something1(common_vars);
}

не меняет уровень абстракции


вызовы типа
if (!empty($active_modules['Some_Module']))
some_function( $arg1, $arg2, $arg3);
можно и переделать на
func_if_module( "Some_Module", "some_function", $arg1, $arg2, $arg3);


если вариант
if (
active_module_XXX
&& func_XXX_is_allowed_something(common_vars)
) {
func_XXX_do_something1(common_vars);
}

не будет читаться лучше

Комментарий4
func_if_module - интересная конструкция, вот только проблема: аргументы будут вычислены безусловно, то есть всегда.

да это точно
func_if_module может увеличивать кол-во нотисов в магазине

XC4: контрибьютерам X-Cart насчет безопасности

*подозрительно относиться к операциям обратным к операциям экранирования из prepare.php
stripslahes / func_unhtml / unhtmlentities / unhtml_entitity

*подозрительно относиться к переменным которые проходят двойную конвертацию
например urldecode может выдать SQL-инъекцию даже после прохождения prepare.php

*подозрительно относиться к trusted переменным

*лучше использовать ограниченные белые списки допустимых значений для переменной
пример
$orderby = (!in_array($orderby, array('login', 'name', 'usertype'))) ? 'login' : $orderby;

*не забывать делать escape в шаблонах

*максимально жестко ограничивать всю логику, которая расширяет права пользователя. Или вообще лучше не изменять переменные которые расширят права типа current_area / login/ logged_userid / login_type / identifiers

*делать проверки не хуже чем из login.php при альтернативных логинах. Или лучше полностью задействовать логику из login.php
аналогично для register.php

*не забывать про подключение
include/admin_security.php
include/security.php