Freecode Academy

[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 у объекта, он во-первых, может затереть вообще другой объект (а тот мог быть измененным, а значит при сохранении главного объекта этот объект уже не будет сохранен), а во-вторых, этот новый объект может встать или впереди, или позади своей же копии, но эти две копии могут отличаться (новый объект перед добавлением изменили), так вот в зависимости от их очередности, при сохранении главного объекта, эти объекты будут сохранены, и не факт, что в правильной исторической последовательности.

Вот как-то так…

Comments

AlOshka

Боюсь представить — сколько нервных клеток ушло на отлов такого бага. Но по логике, правильней будет так:

$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 С Джейсоном общался, он взял на заметку. Фикс баги не сложный, так что наверняка скоро пофиксят.

AlOshka

Нет, так нельзя делать. Это ссумируем массивы, а нам надо заменять уже имеющиеся.

Так а разве это не оно:

$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"
// }

?

Николай Ланец
Николай Ланец
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 не заменился.

AlOshka

небольшой фокус :-)

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] новым массивом массивов. Вы слышали про ссылки на объекты? Вот такой пример рассмотрим:

$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 дает четкое понимание что и как происходит, и не заставит программиста лезть в мануалы изучать специфики редко используемых функций.

AlOshka

Хм, может я чего не понимаю, конечно, но

$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+a+b; Здесь вот так:

$a; $b;
$c = $a;
$a = $c + $b;

Это не очевидно, но по сути примерно это и происходит, так как исходному массиву выполняется полное присвоение всех элементов. В случае же перебора имеющиеся уже объекты вообще никак не затрагиваются (только перетираются те, которые должны перетереться).

AlOshka
<?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 — это не то, что надо? Если нет, то я не понял задачу.

AlOshka

Просто здесь, при сложении, математика не решает — от перемены мест слагаемых результат МЕНЯЕТСЯ :-)

Николай Ланец
Николай Ланец

Вот в этом четко понимаем, что есть что?

$a === $b;

То есть на выходе мы как бы получаем то же самое, похожее на правду, Но aможетнеравнятьсяa может не равняться a.

Думаю, на этом можно этот диалог прекращать. Эффект не стоит затрачиваемых калорий.

AlOshka

Это снова я.. В чём разница между $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
)

Я правду не понимаю :-(

Ну а вообще, здорово, конечно, что так оперативно)

AlOshka

Оператор + возвращает левый массив, к которому был присоединен правый массив. Для ключей, которые существуют в обоих массивах, будут использованы значения из левого массива, а соответствующие им элементы из правого массива будут проигнорированы.

Т.е. при сложении collection+collection + _relatedObjects (запись именно в таком порядке), если имеются одинаковые ключи и там, и там, то при совпадении ключей выберутся только ключи из $collection. Все остальные ключи из обоих массивов добавятся в результирующий массив.

array_diff_key ( array array1, array array2 [, array ...] ) возвращает массив, содержащий все значения array1, имеющие ключи, не содержащиеся в последующих параметрах. Обратите внимание, что ассоциации сохраняются.

При array_diff_key(relatedObjects,_relatedObjects, collection) мы получили расхождение этих массивов а потом добавляем к нему только что выкинутые ключи (*+ collection∗),т.о.замениввсеключи−дубликатыизcollection*), т.о. заменив все ключи-дубликаты из _relatedObjects ключами из $collection.

Результат в обоих случаях одинаков.

Николай Ланец
Николай Ланец

php.net/manual/ru/function.array-diff-key.php

array_diff_key — Вычисляет расхождение массивов, сравнивая ключи

То есть он уже отделяет от массива все уникальные элементы, скорее всего для того, чтобы быть четко уверенным в том, что это в конечный массив попадет сумма именно уникальных элементов. А то мало ли какие подводные камни есть в этом суммировании. Я думаю, не стоит так сильно голову ломать. В php есть много способов реализации одного и того же. Джейсон — очень опытный разработчик с огромным стажем. Наверняка он применил оптимальный вариант.

Николай Ланец
Николай Ланец

Результат в обоих случаях одинаков.

Я думаю, не стоит так сильно голову ломать. В php есть много способов реализации одного и того же.