изменения в LightMetaProperty могут поломать сохранение объектов

[dovg]

Добрый день. Речь про коммит 8485d51

Кейз

$property = ObjectProperty::create()->setName('someName');

$bigObject =
 BigObject::create()->setName('father')->
 setObjectProperty($property);

$property->dao()->add($property);
$bigObject->dao()->add($bigObject);

Вот с таким кодом в $bigObject потеряется ссылка на $property, ибо id в $property появился уже после того как он был присвоен $bigObject

У нас в коде кое-что сломалось. В нашем бранче я откатил изменения из этого коммита. Но про мастер я не могу принять соответствующего решения.

[ssserj]

Все правильно сделал. Странно, что у нас тесты не сломались. Будем думать!

[AlexeyDsov]

@ssserj А по какой причине это изменение было сделано?

[ssserj]

@AlexeyDsov необходима более быстрая вставка в Storage. Лех, если мы посмотрим sql, выполняющийся при save операциях, то мы увидим, что через get* достаются все свойства объекта, в данном случае, если объект большой, то делается много "ненужных" selecto'в

@dovg, а я же ничего не изменяю у объекта:

 $getter = $this->getter;
 if (
     $this->relationId
     && $this->strategyId == FetchStrategy::LAZY
 ) {
   $getter = $getter.'Id';
 }
 $value = $object->{$getter}();

[dovg]

@ssserj А я и не говорю, что что-то изменяется. ;)

Посмотри код - соль в том, что id у объекта появляется уже после того, как он был привязан к "контейнеру". Соответственно в object->getId() есть значение, а в objectId его нет.

[dovg]

добавил соответствующий тест в #56

[AlexeyDsov]

Вот блин дилема. С одной стороны фикс правильный - убивает лишние селекты, а с другой стороны ломается вот что-то внутреннее и с этим надо что-то делать.
Опять же с другой стороны сетить объект без айдишника в класс который использует его как lazy тоже не хорошо, т.к. для setLazy(Lazy $lazy) более чем желательно что б объект уже имел идентификатор иначе метож getLazyId отрабатывает некорректно. Даже если впоследствии у засеченного класса появится идентификатор метод getLazyId все равно будет возвращать null как будто бы его и небыло...

Возможно тут было бы решением поправить методы типа getLazyId что б если уже есть у них объект, то возвращаел идентификатор из него, а иначе возвращал бы просто иденификатор. Но это отступление от lazy в сторону и тоже может что-то в свою очередь сломать.

[lom]

Правильный метод на мой взгляд - бросать exception при попытке засетить lazy-объект без идшника.
Как аналогия - UnifiedContainer.fetch(), правда там немного другой случай.

Но опять же у ребят все сломается) Так что похоже придется откатить...

[dovg]

А кроме нас у кого-нибудь сломалось?

Для наших проектов мы используем свой бранч. Если проблема воспроизводится только у нас, то ИМХО можно пойти по пути @lom

[AlexeyDsov]

Не могу зарекаться что у меня не сломается. Где-то что-то подобное мог разок-другой сделать или просто видел. Щас через полчасика-часик вариант патча нарисую если получится :)

[AlexeyDsov]

Собственно вот фикс который решает проблему: 5ae2e37

[lom]

@AlexeyDsov +1

[dovg]

Это потребует пересбора меты, что несколько печалит.

[AlexeyDsov]

А в чем проблема мету то пересобрать?

[dovg]

Я правильно понимаю, что эти изменения предлагаются в замен того, что сделал @ssserj ?

В этом случае bc сохраняется, поэтому ++