[5.x]: Saving Elements with eager loaded fields throws an exception

[Anubarak]

What happened?

Description

When saving an element with eager loaded field values the current implementation breaks, due to changes in BaseRelationField::normalizeValueForInput | Elements::duplicateElement.

Steps to reproduce

1. Query an Entry with an eager loaded field value
2. change another field
3. save the element
4. Exception

$entry = Entry::find()->with([['myField']])->one();
$entry->setFieldValue('headline', 'bar');
Craft::$app->getElements()->saveElement($entry);
// exception

Here is the full trace

> TypeError: craft\fields\BaseRelationField::_all(): Argument #1 ($query) must be of type craft\elements\db\ElementQueryInterface, craft\elements\ElementCollection given, called in /app/vendor/craftcms/cms/src/fields/BaseRelationField.php on line 1011 and defined in /app/vendor/craftcms/cms/src/fields/BaseRelationField.php:1858
Stack trace:
#0 /app/vendor/craftcms/cms/src/fields/BaseRelationField.php(1011): craft\fields\BaseRelationField->_all(Object(craft\elements\ElementCollection), Object(craft\elements\Entry))
#1 /app/vendor/craftcms/cms/src/fields/BaseRelationField.php(904): craft\fields\BaseRelationField->normalizeValueForInput(Object(craft\elements\ElementCollection), Object(craft\elements\Entry))
#2 /app/vendor/craftcms/cms/src/base/Field.php(975): craft\fields\BaseRelationField->serializeValue(Object(craft\elements\ElementCollection), Object(craft\elements\Entry))
#3 /app/vendor/craftcms/cms/src/services/Elements.php(4112): craft\base\Field->serializeValueForDb(Object(craft\elements\ElementCollection), Object(craft\elements\Entry))
#4 /app/vendor/craftcms/cms/src/services/Elements.php(1269): craft\services\Elements->{closure:craft\services\Elements::_saveElementInternal():3939}()
#5 /app/vendor/craftcms/cms/src/services/Elements.php(3939): craft\services\Elements->ensureBulkOp(Object(Closure))
#6 /app/vendor/craftcms/cms/src/services/Elements.php(1954): craft\services\Elements->_saveElementInternal(Object(craft\elements\Entry), false, false, NULL, Array, false, false, true)
#7 /app/vendor/craftcms/cms/src/services/Elements.php(1269): craft\services\Elements->{closure:craft\services\Elements::duplicateElement():1940}()
#8 /app/vendor/craftcms/cms/src/services/Elements.php(1940): craft\services\Elements->ensureBulkOp(Object(Closure))
#9 /app/vendor/craftcms/cms/src/services/Revisions.php(183): craft\services\Elements->duplicateElement(Object(craft\elements\Entry), Array, true, false, false, true)
#10 /app/vendor/craftcms/cms/src/elements/Entry.php(3209): craft\services\Revisions->createRevision(Object(craft\elements\Entry), 585, NULL)
#11 /app/vendor/craftcms/cms/src/services/Elements.php(4259): craft\elements\Entry->afterPropagate(false)
#12 /app/vendor/craftcms/cms/src/services/Elements.php(1269): craft\services\Elements->{closure:craft\services\Elements::_saveElementInternal():3939}()
#13 /app/vendor/craftcms/cms/src/services/Elements.php(3939): craft\services\Elements->ensureBulkOp(Object(Closure))
#14 /app/vendor/craftcms/cms/src/services/Elements.php(1355): craft\services\Elements->_saveElementInternal(Object(craft\elements\Entry), false, true, NULL, Array, false, false, false)
#15 /app/modules/api/base/base/BaseApiService.php(20): craft\services\Elements->saveElement(Object(craft\elements\Entry), false)
#16 /app/modules/api/base/controllers/actions/ElementCreateAction.php(150): modules\api\base\base\BaseApiService->saveElement(Object(craft\elements\Entry))
#17 /app/modules/api/base/controllers/actions/ElementCreateAction.php(118): modules\api\base\controllers\actions\ElementCreateAction->saveInternal(Object(craft\elements\Entry))
#18 /app/modules/api/base/controllers/actions/ElementCreateAction.php(69): modules\api\base\controllers\actions\ElementCreateAction->save(Object(craft\elements\Entry))
#19 [internal function]: modules\api\base\controllers\actions\ElementCreateAction->run()
#20 /app/vendor/yiisoft/yii2/base/Action.php(95): call_user_func_array(Array, Array)
#21 /app/vendor/yiisoft/yii2/base/Controller.php(182): yii\base\Action->runWithParams(Array)
#22 /app/vendor/yiisoft/yii2/base/Module.php(547): yii\base\Controller->runAction('update', Array)
#23 /app/vendor/craftcms/cms/src/web/Application.php(369): yii\base\Module->runAction('v1/applicants/u...', Array)
#24 /app/vendor/yiisoft/yii2/web/Application.php(105): craft\web\Application->runAction('v1/applicants/u...', Array)
#25 /app/vendor/craftcms/cms/src/web/Application.php(337): yii\web\Application->handleRequest(Object(craft\web\Request))
#26 /app/vendor/yiisoft/yii2/base/Application.php(382): craft\web\Application->handleRequest(Object(craft\web\Request))
#27 /app/web/index.php(21): yii\base\Application->run()
#28 {main}

Expected behavior

The element should save

Actual behavior

"craft\fields\BaseRelationField::_all(): Argument #1 ($query) must be of type craft\elements\db\ElementQueryInterface, craft\elements\ElementCollection given, called in /app/vendor/craftcms/cms/src/fields/BaseRelationField.php on line 998"

Reason:

The reason is the check for eager loaded field values in combination of the duplication logic, in BaseRelationField you check for eager loaded field value

cms/src/fields/BaseRelationField.php

Lines 995 to 999 in 6e9292f

	if ($element !== null && $element->hasEagerLoadedElements($this->handle)) {
	$value = $element->getEagerLoadedElements($this->handle)->all();
	} else {
	$value = $this->_all($value, $element)->all();
	}

When I dump these:

    private function normalizeValueForInput(mixed $value, ?ElementInterface $element, ?array &$initialValue = null): array
    {
        if ($element !== null && $element->hasEagerLoadedElements($this->handle)) {
            $value = $element->getEagerLoadedElements($this->handle)->all();
        } else {


            if($value instanceof Collection){
                echo "<pre>";
                var_dump($element->getFieldValue($this->handle)::class);
                var_dump(array_keys($element->_eagerLoadedElements));
                echo "</pre>";
                die();
            }

            $value = $this->_all($value, $element)->all();
        }

I can see the class is an instance of ElementCollection BUT my eager loaded elements are empty.
This is due to this code here

cms/src/services/Elements.php

Lines 1872 to 1877 in 6e9292f

	$dirtyFields = $mainClone->getDirtyFields();
	foreach ($mainClone->getFieldValues() as $handle => $value) {
	if (is_object($value) && !$value instanceof UnitEnum) {
	$mainClone->setFieldValue($handle, clone $value);
	}
	}

All dirty fields (essentially everything) is looped and then set via cloned value
However at this point

$mainClone->setFieldValue($handle, clone $value);

$value is not an instance of Element Query but an instance of Element Collection, the field was marked as eager loaded before this line.
However after setting the element collection as field value, Craft CMS will unset all eager loaded values in setFieldValue

cms/src/base/Element.php

Lines 5430 to 5445 in 6e9292f

	public function setFieldValue(string $fieldHandle, mixed $value): void
	{
	$behavior = $this->getBehavior('customFields');
	$behavior->$fieldHandle = $value;
	// Don't assume that $value has been normalized
	unset($this->_normalizedFieldValues[$fieldHandle]);
	// If the element is fully initialized, mark the value as dirty
	if ($this->_trackDirtyFields) {
	$this->_dirtyFields[$fieldHandle] = true;
	}
	// If the field value was previously eager-loaded, undo that
	unset($this->_eagerLoadedElements[$fieldHandle]);
	unset($this->_eagerLoadedElementCounts[$fieldHandle]);

Thus breaking the workflow.

Currently my only work around is to use events to mark the field as eager loaded in case the value is a Collection but it would be great if you could implement this better somehow.

Maybe mark it as eager loaded in case the value in normalizeValue is a Collection.

Craft CMS version

5.10.11

PHP version

8.5

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

[linear-code]

CMS-2292

[brandonkelly]

Thanks for reporting that! Craft 5.10.12 is out with the fix from #19287.