[BUG]: Large amount of warnings when using Model query cache and Memory storage adapater under Phalcon 5.9.2

[Sn0wCrack]

Describe the bug

We are receiving a large number of warnings relating to an undefined index relating to the Memory storage adapter after upgrading to Phalcon 5.9.2 as per below:

Notice: Undefined index: ph-memo-class-50c6e46f32d577a0cc114f74b2b460fa333ef359aaa6d401fd9a57e4880f6da3_query-5410b946a803299b0fabfcc4853f61bfdd86458511120428849816bdeff7a18b_query-b0f5de0f73f69cee64fc77631b99f9e46f5130f3191634b9c19c852acde785cd_query-037e3e2f83263c7c50fef59ec6103be3488f2f31cf0ce614bc22050126e3f455 in phalcon/Storage/Adapter/Memory.zep on line 232

To Reproduce

Essentially attacking the cache parameter to a Model's find function using in-memory cache as the service should yield this result when the query is run for the first time.

Details

• Phalcon version: 5.9.2
• PHP Version: 8.2
• Operating System: Ubuntu 24.04
• Installation type: ondrej/php
• Zephir version (if any): N/A
• Server: NApache
• Other related info (Database, table schema): Using MySQL but shouldn't be relavent

Additional context

I found this PR here which removes the existence check from the base Abstract Adapter class that I believe is the culprit here: #16709

Additionally I've also noticed that technically if the cache returns the PHP value of false it will always return the default value, even if it does actually exist and stored cache value is just a literal PHP false value.

[niden]

Yes this is the bummer. You cannot reliably return a default value, whether this is false or null or otherwise for every adapter.

I am thinking that I can make this configurable, where if someone needs keys to be checked they will be, otherwise not. Alternatively we could change the memory adapter to check the keys all the time

Thoughts?

[Sn0wCrack]

Personally I think it would be preferable if the previous behaviour was restored for all adapters by reverting this change.

While returning false is probably a niche case, this still was previously possible from the majority of adapters, so imo this is a breaking change.

[noone-silent]

Yes this is the bummer. You cannot reliably return a default value, whether this is false or null or otherwise for every adapter.

I am thinking that I can make this configurable, where if someone needs keys to be checked they will be, otherwise not. Alternatively we could change the memory adapter to check the keys all the time

Thoughts?

I want to remember that that was exactly the case for the change. A cache needs to be fast. If you check all the time, sometimes multiple times if the cache has a key, that will reduce the performance. Therefor all the has() checks have been removed, with the hint that the has() check needs to be performed by the developer.

In the case of the model cache, here we should implement the has() check right before requesting the cache to restore the old behavior.

EDIT: basically here: https://github.com/phalcon/cphalcon/blob/master/phalcon/Mvc/Model/Query.zep#L310

[Sn0wCrack]

That would be a fine interim solution to just implement it as part of the model cache checks, but it also highlights the fact this change was indeed breaking to the point its caused issues within Phalcon itself.

A change like this should require a major point version change and not be something implemented in a patch release.