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

fix new cache instance cannot clean expired cache item #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

garayx
Copy link

@garayx garayx commented Jun 20, 2024

When starting a new cache instance, with already cached item in ravendb, we would get a error.
That was happening because after loading the document from ravendb, in the php client we would create an entity from document that was returned from ravendb server, and it would use the RavenProxy constructor with $serializeData = true and this would create the entity with wrong data in data field, since the serialize($data) was used. So instead of data: "First product" we would get data: "s:13:"FirstProduct". Then when we try to delete this document, we would get an exception since the tracked entity is changed.

@ppekrol
Copy link

ppekrol commented Aug 4, 2024

@Geolim4 is there a chance that this will be merged?

@Geolim4
Copy link
Member

Geolim4 commented Aug 4, 2024

Hello,

This PR needs to be reworked as data serialisation is mandatory to make the driver the more interoperable as possible.

Some object could not be natively stored to RavenDb without being serialized.

If you don't need serialized data, there's an option to disable it already:

protected bool $serializeData = false;

Copy link
Member

@Geolim4 Geolim4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fix the deserialization part of code

@garayx
Copy link
Author

garayx commented Aug 5, 2024

@Geolim4

Hi,

how would you like to rework it? and what do you mean by fix deserialization part of code? do you always want to serialize the data ?

the issue I was getting while using the extension, is that if I disable the serialize of data using the config you mentioned, it will save the document not serialized which is fine, but on load, inside the RavenDB Client it will create the entity by using the RavenProxy ctor, and it has serializeData = true by default, and doesn't care what you set up in the config.

I think need to get rid of the serialize(), or always serialize(), or maybe see if we can drop $serializeData = true from the ctor, but then I guess it will be always false by default.

@Geolim4
Copy link
Member

Geolim4 commented Sep 1, 2024

I can confirm you that new cache instance is working well with data created by Phpfastcache, you should no share the same "bucket" of Phpfastcache with other applications as, effectively, stored data wont be the same.

Phpfastcache does not only store "data" values, but many metadata: Tags, creation date, update date, expiration date, etc.

So unless you provide me a more explicit case, I will probably close this issue for now.

@garayx
Copy link
Author

garayx commented Sep 17, 2024

@Geolim4 I am not sure what is "more explicit case", since I provided you a failing test in this PR, but I will try.

  1. Setup RavenDB+empty database named "phpfastcache"
  2. Run the following code, it will save the document "pfc/product_page" with cached data.

image

<?php

// load fastphpcache package
require __DIR__ . '/vendor/autoload.php';

use Phpfastcache\CacheManager;
use Phpfastcache\Drivers\Ravendb\Config as RavenDBConfig;
use RavenDB\Documents\DocumentStore;
use RavenDB\Documents\Operations\CollectionStatistics;
use RavenDB\Documents\Operations\DeleteByQueryOperation;
use RavenDB\Documents\Operations\DetailedDatabaseStatistics;
use RavenDB\Documents\Operations\GetCollectionStatisticsOperation;
use RavenDB\Documents\Operations\GetDetailedStatisticsOperation;
use RavenDB\Documents\Queries\IndexQuery;
use RavenDB\Documents\Session\DocumentSession;
use RavenDB\Exceptions\RavenException;
use RavenDB\Http\ServerNode;
use RavenDB\ServerWide\Operations\BuildNumber;
use RavenDB\ServerWide\Operations\Configuration\GetDatabaseSettingsOperation;
use RavenDB\ServerWide\Operations\GetBuildNumberOperation;
use RavenDB\Type\Duration;
// create config
$config = new RavenDBConfig();
$config->setHost(['http://127.0.0.1:8080']);
$config->setCollectionName('phpfastcache');
$config->setDatabaseName('phpfastcache');
$config->setSerializeData(false);
$config->setItemDetailedDate(true);

// create cache instance
try {
    $cacheInstance = CacheManager::getInstance('RavenDB', $config);
} catch(Exception $e ){
    echo $e;
    throw $e;
}


$key = "product_page";
$product_data = 'First product';
try {
// init
    $cachedString = $cacheInstance->getItem($key);
    if ($cachedString->isHit() == false) {
        // if the item is not cached, add it
        // save the product_data string to cache under key
        $cachedString->set($product_data)->expiresAfter(5); // set expire to 5 seconds
        $cacheInstance->save($cachedString);

        echo "Added to cache the value of product_page: " . $cachedString->get();
    } else {
        // if the item is cached, return from cache
        echo "Returned from cache the value of product_page: " . $cachedString->get();
    }
} catch(Exception $e ){
    echo $e;
    throw $e;
}
  1. Run the code again, it will throw the following error:
RavenDB\Exceptions\IllegalStateException: Can't delete changed entity using identifier. Use delete(?object entity) instead. in C:\Work\Projects\sample-ravendb-phpfastcache-extension\vendor\ravendb\ravendb-php-client\src\Documents\Session\InMemoryDocumentSessionOperations.php:943
Stack trace:
#0 C:\Work\Projects\sample-ravendb-phpfastcache-extension\vendor\ravendb\ravendb-php-client\src\Documents\Session\InMemoryDocumentSessionOperations.php(888): RavenDB\Documents\Session\InMemoryDocumentSessionOperations->deleteById()
#1 C:\Work\Projects\sample-ravendb-phpfastcache-extension\vendor\phpfastcache\ravendb-extension\lib\Phpfastcache\Extensions\Drivers\Ravendb\Driver.php(223): RavenDB\Documents\Session\InMemoryDocumentSessionOperations->delete()
#2 C:\Work\Projects\sample-ravendb-phpfastcache-extension\vendor\phpfastcache\phpfastcache\lib\Phpfastcache\Core\Pool\CacheItemPoolTrait.php(593): Phpfastcache\Extensions\Drivers\Ravendb\Driver->driverDelete()
#3 C:\Work\Projects\sample-ravendb-phpfastcache-extension\vendor\phpfastcache\phpfastcache\lib\Phpfastcache\Core\Pool\CacheItemPoolTrait.php(263): Phpfastcache\Extensions\Drivers\Ravendb\Driver->handleExpiredCacheItem()
#4 C:\Work\Projects\sample-ravendb-phpfastcache-extension\vendor\phpfastcache\phpfastcache\lib\Phpfastcache\Core\Pool\CacheItemPoolTrait.php(268): Phpfastcache\Extensions\Drivers\Ravendb\Driver->Phpfastcache\Core\Pool\{closure}()
#5 C:\Work\Projects\sample-ravendb-phpfastcache-extension\example-ravendb-cache.php(43): Phpfastcache\Extensions\Drivers\Ravendb\Driver->getItem()
#6 {main}

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

Successfully merging this pull request may close these issues.

3 participants