Cache keys can conflict if both Data\Store and Data\Cache utilize Redis

[TSP]

After upgrading the server of one of our communities from PHP 7.4 to 8.1 I found the following error message popping up a lot: 

TypeError: count(): Argument #1 ($value) must be of type Countable|array, string given in /www/system/Data/Cache.php:151

This was caused by some custom code being referenced in a custom theme which requested a value with \IPS\Data\Cache::getWithExpire. This is how that function looks currently: 

<?php
public function getWithExpire( $key, $fallback=FALSE )
	{
		if ( !isset( $this->$key ) )
		{
			throw new \OutOfRangeException;
		}
		
		$data = $this->$key;
		if( \count( $data ) and isset( $data['value'] ) and isset( $data['expires'] ) )
		{
			// ...
		}
		else
		{
			unset( $this->$key );
			throw new \OutOfRangeException;
		}
	}

After some debugging I found that $data was indeed a string, and thus can't be counted. My first thought was that you should change from \count( $data ) to \is_array( $data ) in this function, which I still think you should, but the next mystery was to figure out why it would be a string, since you always expect it to be an array, and I used the regular storeWithExpire to save to Cache.

I found the culprit to be former abundance of caution, or what might have been a good reason at the time I implemented it (many many years ago): My code would first request the cache key from \IPS\Data\Cache::getWithExpire. If it didn't find it saved in Cache or it was expired, it would move on to check \IPS\Data\Store, with a 1/15 chance of unsetting it in \IPS\Data\Store to prevent the value from "never" being updated. If it didn't find it in any of them, then it would request the value from an external source and save it to both Data\Cache and Data\Store.

But if both methods utilizes Redis, then it means the second method will just overwrite the cache entry that the first one saved. Data\Cache stores an array to the cache key in Redis, but then Data\Store would immediately overwrite the same cache entry with a string. 

You can reproduce with this code: 

<?php
require_once 'init.php';
$cacheKey = 'storeItPlease3';
$saveToCache = 'Hello world :-) | ' . date(DATE_RFC2822);
$expire = \IPS\DateTime::ts( time() + 60 );

try
{
    $cached = \IPS\Data\Cache::i()->getWithExpire( $cacheKey, TRUE );
    echo $cached . "\n";
}
catch ( \OutOfRangeException $e )
{
        try {
                $cached = \IPS\Data\Store::i()->{$cacheKey};
                echo "Found in store: {$cached}\n";
          		if ( rand(1, 15) == 15 )
				{
					unset(\IPS\Data\Store::i()->{$cacheKey});
				}
        }
        catch(\OutOfRangeException $e2) {
                echo "Couldn't find in Data\Store!\n";
        }
    echo "Didn't find {$cacheKey}. Write it!\n";
    \IPS\Data\Cache::i()->storeWithExpire( $cacheKey, $saveToCache, $expire, TRUE );
    \IPS\Data\Store::i()->{$cacheKey} = $saveToCache;
}

While I understand this kind of situation arising being rare and you probably don't expect people using both methods simultaneously like this, I thought I would still make you aware of it in case you encounter similar reported issues in the future and maybe make some changes to your code. Having a key name be the same in both Store and Cache could also happen by accident, although I guess the chance is rare.
 

1. Consider use is_array() rather than, or in addition to count() in \IPS\Data\Cache::getWithExpire (Do you even need that first check, maybe it's enough with the isset()-calls?)
    
2. Consider prepending or appending to the cache key for one of the storage methods to differentiate entries in Redis saved by Store and Cache
   (Alternatively document that one shouldn't use a key for Store that's already used by Cache or vice versa)

Edited November 3, 2022 by TSP