kirkbushell / eloquence

A drop-in library for certain database functionality in Laravel, that allows for extra features that may never make it into the main project.
MIT License
537 stars 58 forks source link

Add a Force option to force-use the `rebuild` instead of the `update` method #63

Closed Xety closed 7 years ago

Xety commented 7 years ago

I think it should be nice to have a force option (default to false) to permit to use the rebuildCacheRecord method instead of the updateCacheRecord :

'force' => true

Because for me, a counter cache based only on an increment/decrement is not very efficient and can cause some issues in some cases. Of course this option can be applied to the Sum Behavior also.

This PR is only to show you the feature (with tests btw) i'm pretty sure you will implement it better then me. ^^

kirkbushell commented 7 years ago

I like this, cool idea. Could you elaborate on some other points you've made, however - so I fully understand the context? You said:

a counter cache based only on an increment/decrement is not very efficient and can cause some issues in some cases

What do you mean by both of those points? If it causes issues I'd like to know why.

Xety commented 7 years ago

I was talking about the issue when you add/deleted a row on the DB directly without using the ORM. (I know that should not happend in prod) But in dev env it can be very usefull, because you don't have to rebuild it yourself. It will be automatically fixed with the next create/update/delete on the DB.

kirkbushell commented 7 years ago

Ah I see. In those circumstances you should probably rebuild immediately - there's a command coming in next release that will allow you to do that. Is this the main reason you want this functionality? It's much more expensive to rebuild than update, so I'm not sure if we really want this as a key feature.

Xety commented 7 years ago

Oh nice for the command ! Yes this is the main reason. Or we can do it but only if the debug mode is enabled or the env set to local ?

kirkbushell commented 7 years ago

I think it's best if it's kept to the command. Going to close this one - it's a good idea and feature, but I don't think baking it into the model is the right approach.

I'll try and get that next release out shortly - if you want to check it out, I believe it's on dev-master.

Xety commented 7 years ago

It's ok i understand, i will wait the next release for the cmd.

PS: I will do another PR to update the Readme.