FreeCode.Academy

Стратегии обучения

Уроки

Технологии

Участники

Проекты

Топики

Блоги

О проекте

Николай Ланец
Николай Ланец
3 апр. 2013 г., 5:25

[FIXED] bug xPDO::_getRelatedObjectsByFK() (getMany())

Нашел сегодня еще одну очень неприятную багу. На самом деле она очень серьезная, и в отдельных случаях может не только доставить неудобства, но и к серьезным ошибкам привести.
Когда мы выполняем $modx->getCollection($class), мы получаем массив объектов. Но это не простой нумерованный массив (типа array(0 => object, 1 => object) и т.д.). В этом массиве у нас каждый ключ — это id объекта, то есть ключи часто идут не по порядку (3,7,9,57 и т.д.). Те, кто использует ->getMany(), знают — этот метод возвращает связанные объекты. По сути этот метод внутри себя выполняет тот же самый getCollection(). Но проблема кроется всего в одной строчке:
$this->_relatedObjects[$alias]= array_merge($this->_relatedObjects[$alias], $collection);
То есть все связанные объекты попадают в общий массив $this->_relatedObjects. Уточняю: getMany() не просто делает выборку getCollection(), но и набивает эти объекты в общий пулл связанных объектов. И с этим массивом связанных объектов $this->_relatedObjects можно (а иногда и нужно) работать. Так вот, проблема в том, что array_merge не сохраняет ключи-id объектов, а набивает все в новый нумерованный массив. Фокус хорошо демонстрирует вот этот пример: 
<?php
$a = array(
    2=>'object'    
);

$b = array_merge($a, array(
    5=>'object',
    7 => 'object',
));

print_r($b);
На выходе мы получаем:
Array
(
    [0] => object
    [1] => object
    [2] => object
)
Последствия (от неудобств до проблем)
Неудобство заключается вот в чем: Допустим, мы знаем, что уже получали все зависимые объекты, и теперь просто хотим проверить наличие объекта по его id. У наших объектов есть id-шники 7,9 и 12. По идее мы имеем возможность по id обратиться к ключу массива, то есть $object->_relatedObjects[$alias][7] должен нам вернуть нужный нам объект. Ан нет. Из-за этой баги у нас там теперь массив с ключами 0,1,2… То есть объект мы уже потеряли.
А что произойдет, если мы опять выполним $object->getMany()? По сути 3 объекта он уже в себе имеет. Но из-за этой фишки с array_merge, мы получим уже 6 объектов, так как в новом массиве объектов ключи 7,9 и 12, а в текущем массиве 0,1,2, и теперь станет еще и 3,4,5. А если у нас id-шники 3453,6757,90980 и мы выполним getMany() много раз? :-)
Еще пример. 
if(!$room = $modx->getObject('modResource', 107)){return '';}

// Получаем все связанные объекты
$vars = $room->getMany('Variables');

print count($room->_relatedObjects['Variables']); // Считаем - 2 объекта

// Получаем один из имеющихся объектов
$var = current($room->_relatedObjects['Variables']);

// Добавляем его же
$room->addMany($var);

print count($room->_relatedObjects['Variables']); // Уже 3 объекта
По сути это тоже самое, просто другим путем. Но суть в чем: вот одна строчка из этого метода
$this->_relatedObjects[$alias][$objpk]= $obj;
То есть затирается элемент массива объектов с ключем id этого объекта. Но по нашей схеме в зависимости от того, какой id у объекта, он во-первых, может затереть вообще другой объект (а тот мог быть измененным, а значит при сохранении главного объекта этот объект уже не будет сохранен), а во-вторых, этот новый объект может встать или впереди, или позади своей же копии, но эти две копии могут отличаться (новый объект перед добавлением изменили), так вот в зависимости от их очередности, при сохранении главного объекта, эти объекты будут сохранены, и не факт, что в правильной исторической последовательности.
Вот как-то так…
Боюсь представить — сколько нервных клеток ушло на отлов такого бага. Но по логике, правильней будет так: 
$this->_relatedObjects[$alias]= $collection + $this->_relatedObjects[$alias];
отсюда, хотя могу и ошибаться. а если не ошибаюсь, то нужен коммит, чтобы в 2.2.7 зарелизить успели
Да нет, калорий много не ушло. Просто столкнулся с новой задачей, и при реализации ее столкнулся с этой багой.
$this->_relatedObjects[$alias]= $collection + $this->_relatedObjects[$alias];
Нет, так нельзя делать. Это ссумируем массивы, а нам надо заменять уже имеющиеся. Для этого правильней использовать foreach и сравнивать по id-ключу.
а если не ошибаюсь, то нужен коммит, чтобы в 2.2.7 зарелизить успели
Да, забыл сказать, тикет я создал: tracker.modx.com/issues/9768 С Джейсоном общался, он взял на заметку. Фикс баги не сложный, так что наверняка скоро пофиксят.
Нет, так нельзя делать. Это ссумируем массивы, а нам надо заменять уже имеющиеся.
Так а разве это не оно: 
$a = array("a" => "apple", "b" => "banana");
$b = array("a" => "pear", "b" => "strawberry", "c" => "cherry");
$c = $b + $a;
var_dump($c);

// array(3) {
//   ["a"]=>
//   string(4) "pear"
//   ["b"]=>
//   string(10) "strawberry"
//   ["c"]=>
//   string(6) "cherry"
// }
?
<?php
print '<pre>';
$a = array(
    2=>'object'    
);

$b = $a + array(
    2 => 'new object',
    5 =>'object',
    7 => 'object',
);

print_r($b);
Результат:
Array
(
    [2] => object
    [5] => object
    [7] => object
)
То есть элемент с ключом 2 не заменился.
небольшой фокус :-) 
<?php
print '<pre>';
$a = array(
    2 => 'object'    
);

$b = array(
    2 => 'new object',
    5 => 'object',
    7 => 'object',
) + $a;

print_r($b);
результат: 
Array
(
    [2] => new object
    [5] => object
    [7] => object
)
Ага, и перетерли массив объектов $this->_relatedObjects[$alias] новым массивом массивов. Вы слышали про ссылки на объекты? Вот такой пример рассмотрим:
<?php
$a = $modx->getCollection('modResource', 3);
$b = $modx->getCollection('modResource', 3);

$obj = $a[3];
print ($a[3] === $obj ? 1: 0); // result 1

$obj = $b[3];
print ($a[3] === $obj ? 1: 0); // result 0
Хотя это вроде одинаковые объекты с одинаковыми данными, он не равны друг другу, так как это разные инстансы (если жестко сравнивать). А вы предлагаете этот массив затирать.
Конечно, может ваш метод и рабочий, может даже сбоев не будет, но он может содержать много подводных камней. Метод перебора foreach и присвоение по id дает четкое понимание что и как происходит, и не заставит программиста лезть в мануалы изучать специфики редко используемых функций.
Хм, может я чего не понимаю, конечно, но 
$this->_relatedObjects[$alias]= array_merge($this->_relatedObjects[$alias], $collection);
в $collection и в $this->_relatedObjects[$alias] и так будут разные интансы объектов с одинаковым id, разве нет? И в результате операции 
array_merge($this->_relatedObjects[$alias], $collection);
в $this->_relatedObjects[$alias] будут так же новые инстансы, пришедшие из $collection. Только array_merge затрёт ключи. А при сложении массивов (порядок элементов только другой) ключи не затрутся. Разве не это было нужно?
Начнем с того, что изначально все было не так, как надо. А если рассматривать предложенный вариант, то здесь не просто $a+$b; Здесь вот так: 
$a; $b;
$c = $a;
$a = $c + $b;
Это не очевидно, но по сути примерно это и происходит, так как исходному массиву выполняется полное присвоение всех элементов. В случае же перебора имеющиеся уже объекты вообще никак не затрагиваются (только перетираются те, которые должны перетереться).
<?php
echo '<pre>';
$sourceA = array(
	2	=> 'object 2',
	3	=> 'object 3',
	5	=> 'object 5'
);
$b = array(
	3	=> 'new instanceof object 3',
	5	=> 'new instanceof object 5',
	8	=> 'object 8',
	10	=> 'object 10',
	15	=> 'object 15',
);

/**
 * Print source arrays
 */
$a = $sourceA; // set default array
echo "Sources:\n";
echo "array a:\n";
print_r($a);
echo "\narray b:\n";
print_r($b);
echo "\n\n";

/**
 * array_merge result
 */
echo "Results:\n";
$a = array_merge($a, $b);
ksort($a);
echo "1. array_merge:\n";
print_r($a);

$a = $sourceA; // set default array
/**
 * (a + b) result
 */
$a = $a + $b;
ksort($a);
echo "\n\n2. summ (a + b):\n";
print_r($a);

$a = $sourceA; // set default array
/**
 * (b + a) result
 */
$a = $b + $a;
ksort($a);
echo "\n\n3. summ (b + a):\n";
print_r($a);
Sources:
array a:
Array
(
    [2] => object 2
    [3] => object 3
    [5] => object 5
)

array b:
Array
(
    [3] => new instanceof object 3
    [5] => new instanceof object 5
    [8] => object 8
    [10] => object 10
    [15] => object 15
)


Results:
1. array_merge:
Array
(
    [0] => object 2
    [1] => object 3
    [2] => object 5
    [3] => new instanceof object 3
    [4] => new instanceof object 5
    [5] => object 8
    [6] => object 10
    [7] => object 15
)


2. summ (a + b):
Array
(
    [2] => object 2
    [3] => object 3
    [5] => object 5
    [8] => object 8
    [10] => object 10
    [15] => object 15
)


3. summ (b + a):
Array
(
    [2] => object 2
    [3] => new instanceof object 3
    [5] => new instanceof object 5
    [8] => object 8
    [10] => object 10
    [15] => object 15
)
Разве результат под номером 3 — это не то, что надо? Если нет, то я не понял задачу.
Просто здесь, при сложении, математика не решает — от перемены мест слагаемых результат МЕНЯЕТСЯ :-)
Вот в этом четко понимаем, что есть что? 
$a === $b;
То есть на выходе мы как бы получаем то же самое, похожее на правду, Но $a может не равняться $a.
Думаю, на этом можно этот диалог прекращать. Эффект не стоит затрачиваемых калорий.
Отличная новость! Багу пофиксили: github.com/modxcms/xpdo/commit/e5b28ec2cbea150d591eb07d1768ad97dc53895a
Это снова я.. В чём разница между $collection + $_relatedObjects и array_diff_key($_relatedObjects, $collection) + $collection? 
$relatedObjects = array(
        2	=> 'object 2',
	3	=> 'object 3',
	5	=> 'object 5'
);
$collection = array(
	3	=> 'new instanceof object 3',
	5	=> 'new instanceof object 5',
	8	=> 'object 8',
	10	=> 'object 10',
	15	=> 'object 15',
);

$_relatedObjects = $relatedObjects; // set default array
$_relatedObjects = $collection + $_relatedObjects;
ksort($_relatedObjects);
echo '$_relatedObjects = $collection + $_relatedObjects;' . "\n";
print_r($_relatedObjects);

echo "\n";

$_relatedObjects = $relatedObjects; // set default array
$_relatedObjects = array_diff_key($_relatedObjects, $collection) + $collection;
ksort($_relatedObjects);
echo '$_relatedObjects = array_diff_key($_relatedObjects, $collection) + $collection;' . "\n";
print_r($_relatedObjects);
$_relatedObjects = $collection + $_relatedObjects;
Array
(
    [2] => object 2
    [3] => new instanceof object 3
    [5] => new instanceof object 5
    [8] => object 8
    [10] => object 10
    [15] => object 15
)

$_relatedObjects = array_diff_key($_relatedObjects, $collection) + $collection;
Array
(
    [2] => object 2
    [3] => new instanceof object 3
    [5] => new instanceof object 5
    [8] => object 8
    [10] => object 10
    [15] => object 15
)
Я прадва не понимаю :-(
Ну а вообще, здорово, конечно, что так оперативно)
Оператор + возвращает левый массив, к которому был присоединен правый массив. Для ключей, которые существуют в обоих массивах, будут использованы значения из левого массива, а соответствующие им элементы из правого массива будут проигнорированы.
Т.е. при сложении $collection + $_relatedObjects (запись именно в таком порядке), если имеются одинаковые ключи и там, и там, то при совпадении ключей выберутся только ключи из $collection. Все остальные ключи из обоих массивов добавятся в результирующий массив.
array_diff_key ( array array1, array array2 [, array ...] ) возвращает массив, содержащий все значения array1, имеющие ключи, не содержащиеся в последующих параметрах. Обратите внимание, что ассоциации сохраняются.
При array_diff_key($_relatedObjects, $collection) мы получили расхождение этих массивов а потом добавляем к нему только что выкинутые ключи (+ $collection), т.о. заменив все ключи-дубликаты из $_relatedObjects ключами из $collection.
Результат в обоих случаях одинаков.
array_diff_key — Вычисляет расхождение массивов, сравнивая ключи
То есть он уже отделяет от массива все уникальные элементы, скорее всего для того, чтобы быть четко уверенным в том, что это в конечный массив попадет сумма именно уникальных элементов. А то мало ли какие подводные камни есть в этом суммировании. Я думаю, не стоит так сильно голову ломать. В php есть много способов реализации одного и того же. Джейсон — очень опытный разработчик с огромным стажем. Наверняка он применил оптимальный вариант.
Результат в обоих случаях одинаков.
Я думаю, не стоит так сильно голову ломать. В php есть много способов реализации одного и того же.

Добавить комментарий