Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handleLazyLoadingViolation shouldn't throw for newly created model instances #41420

Closed
rogatty opened this issue Mar 10, 2022 · 1 comment · Fixed by #41549
Closed

handleLazyLoadingViolation shouldn't throw for newly created model instances #41420

rogatty opened this issue Mar 10, 2022 · 1 comment · Fixed by #41549
Labels

Comments

@rogatty
Copy link
Contributor

rogatty commented Mar 10, 2022

  • Laravel Version: 9.1.0
  • PHP Version: 8.1.3
  • Database Driver & Version: N/A

Description:

protected function handleLazyLoadingViolation($key)
{
if (isset(static::$lazyLoadingViolationCallback)) {
return call_user_func(static::$lazyLoadingViolationCallback, $this, $key);
}
throw new LazyLoadingViolationException($this, $key);
}

HasAttributes::handleLazyLoadingViolation shouldn't throw LazyLoadingViolationException if model instance was created in memory, and not fetched from the database. It's impossible to eager load relations for a model that doesn't exist yet. Throwing an exception in this scenario doesn't prevent unnecessary database queries and requires manual bypassing.

I solved it by adding the following to AppServiceProvider:

Model::handleLazyLoadingViolationUsing(
	static function (Model $model, string $relation) {
		// It's fine that you didn't eager load relations for a new model as it wouldn't make sense
		if (!$model->exists || $model->wasRecentlyCreated) {
			return;
		}

		throw new LazyLoadingViolationException($model, $relation);
	}
);

I believe this condition should be built into HasAttributes::handleLazyLoadingViolation.

Steps To Reproduce:

Fixed by checking !$model->exists:

$model = ParentModel::make();
$childModel = ChildModel::make();
$model->children->push($childModel);

Fixed by checking $model->wasRecentlyCreated:

$model = Model::make();
$model->save();
$model->children->each(
	function (ChildModel $child) {
		$child->parent_id = $model->id;
		$child->save();
	}
);

!$model->exists isn't enough in the second case because relation wasn't initialized before saving the model.

Why I bumped into this issue:

I expect the question "why do you use relations like that?" so let me answer upfront. I'm trying to use a DDD approach where the operations are split into stages (simplified):

  • Load models from database
  • Modify models – make new children instances and push them to relation collection (or not, depends on request data)
  • Persist models to database - iterate over relation collection items, set the parent ID and save to database
@driesvints
Copy link
Member

Heya, yes this seems reasonable to us. Can you send in a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants