Николай Ланец
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.
Думаю, на этом можно этот диалог прекращать. Эффект не стоит затрачиваемых калорий.
Это снова я.. В чём разница между $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 есть много способов реализации одного и того же.

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