jeremyharris / cakephp-lazyload

A lazy loader for CakePHP entities.
MIT License
61 stars 10 forks source link

Manually unsetting a property from an entity does not lazy load it again #13

Closed elboletaire closed 7 years ago

elboletaire commented 7 years ago

I've updated from 1.0.2 to 1.0.3 and after that all my tests started crashing because there's a moment in my code where I do the following:

// unset any possible previously related center
unset($member->center);

I do this because this is a signup process where the center is related to the member (and I need it that way), but if the member goes back and chooses another center i need to repopulate the $member->center object with the appropriate data.

The thing is that upgrading to 1.0.3 started throwing errors because after the unset($member->center) the center entity does not get populated again.

I've no idea if this is related to #12 or to the 1.0.3 change:

Fixes lazily loading a previously unset property that was eagerly loaded

But the thing is I cannot update from 1.0.2 to 1.0.3 as it's a critical change for my app.

Using CakePHP 3.3.15

PS. If you need any other info, please, tell me. I'm opening this issue as fast as I can because I'm in the final step of this week's sprint.

jeremyharris commented 7 years ago

It's definitely related; the previous lazy load behavior was incorrect as it would reload properties that were unset.

I'm not sure what you mean by center not being populated again - do you mean you were relying on the bug that lazy loaded it again, or do you mean that you can no longer set it to something after unsetting it?

Can you clarify what you are doing to get center to be populated again?

elboletaire commented 7 years ago

Well, with version 1.0.2 I rely on the lazy loader to load the belongsTo relation (I forgot to say that, Members => belongsTo => Centers).

Due to client specifications I need to pre-register the member before he can signup (pay the fee). In that process I check for previous pre-registered members and, if I found an existing once, I merge that data with the existing one.

In both cases I'm currently relying on the lazy loader to load the Center which the member belongs to.

The thing is that if the member goes back (after he pre-registered) and decides to change the center, I unset($member->center) in order to get it populated again when needed (thanks to the lazy loader).

With version 1.0.3 this stopped working. Once I unset($member->center) it doesn't get populated again.

Does this clarify my issue?

elboletaire commented 7 years ago

Checking my code further, I see that there's a case where I eager load the center relation:

// Check if member exists
$existent = $this->find('all', [
    'conditions' => [
        'Members.nif' => strtoupper(trim($data['nif']))
    ],
    'contain' => ['Centers']
])->first();

And if I see it's the same member...

// Member exists, we should merge their data.
else {
    $member = $existent;
    $member['contact_status_id'] = Configure::read('Members.contact_status.not_contacted');
}

So, as I'm seeing now.. it only lazy loads the relation for new members, not for those who go back and forward again (because are considered as existent).

If I'm not understanding it wrong... the 1.0.3 change brakes my code because I've unset a previously eagerly loaded relation.

So here's my question: is there any way to ensure it'll be always populated?

jeremyharris commented 7 years ago

Thanks for clarifying that!

I think you're understanding correctly, 1.0.3 will break your reliance on the lazy loading re-loading it automatically. This was considered a bug as it doesn't behave the way a normal entity would if the data was eagerly loaded (with contain()) and unset. While you could pin your dependency at 1.0.2, I wouldn't suggest it as you would miss out on any future bug fixes or features.

There's no way to tell the lazy loader to always reload a property - I think that would add some complexity to it that it doesn't really need. Instead, you can use the core-provided loadInto to load the association data back into the entity:

unset($member->center);
// manually load Centers associated data back onto $member
$this->Members->loadInto($member, ['Centers']);
elboletaire commented 7 years ago

Woh, thanks for that tip! Didn't know about it. I close the issue then as I'll update the code following your suggestions 😃

elboletaire commented 7 years ago

Again, thanks for the tip. I've been able to upgrade to 1.0.3 after using loadInto in the required moment. 😄

jeremyharris commented 7 years ago

@elboletaire great to hear!