storyblok / storyblok-php-client

Storyblok - PHP Client
https://www.storyblok.com
MIT License
33 stars 37 forks source link

Infinite recursion in V2, when _storyblok_published is set #54

Closed pscheit closed 1 year ago

pscheit commented 2 years ago

Hey there,

I have an issue, when using a cache and using "show published version" from the storyblok menu.

The link is then like:
https://mydomain.tld/mystories/?_storyblok_published=129972584

This leads to an non-stopping recursion and then of course hitting time or memory limits.

The recursion goes like this: \Storyblok\Client::getStoryBySlug is called from my controller \Storyblok\Client::getStory \Storyblok\Client::reCacheOnPublish \Storyblok\Client::setCacheVersion (since $_GET['...] is true) \Storyblok\Client::getStories \Storyblok\Client::reCacheOnPublish \Storyblok\Client::setCacheVersion (since $_GET[...] is true) \Storyblok\Client::getStories \Storyblok\Client::reCacheOnPublish \Storyblok\Client::setCacheVersion

etc etc.

Idea for a fix would be to not fetch an arbitrary story to get the cv value, but to invalidate the cv, fetch the first story (that we are fetching anyway, cause we've been called to summon a story/stories) and set the cv from there (maybe). I guess the problem here is, that the cv is missing for non-published?

or even do it afterwards, when the cv wasnt retrieved with the story.

We should add a test for that, i didnt blame it all the way, but i guess this was introduced in building v2? I updated the client and got this error immediately

roberto-butti commented 1 year ago

Hi @pscheit , thank you for your issue and feedback!!! Really appreciated.

I was taking care of the PHP Client. I replaced the Caching mechanism from Apix to Symfony. Doing that, I added some tests, and I see that your issue still exists. I know, it is not so correct injecting a $_GET parameter :) But at least I'm able to reproduce programmatically.

This is the typical issue where the fix looks easy, but I would see if there are some "side effects."

I will keep you updated.

The test used for testing this issue:

test('Integration: get one story with cache', function () {
    $client = new Client('your-access-token');
    $options = $client->getApiParameters();
    $client->setCache('filesystem', ['path' => 'cache']);
    $_GET['_storyblok_published'] = 129972584;
    $responses = $client->getStoryBySlug('home');
    $body = $responses->getBody();
    $this->assertArrayHasKey('story', $body);
    $this->assertArrayHasKey('name', $body['story']);
    $this->assertEquals('home', $body['story']['name']);
})->setGroups(['integration']);