Skip to content

Don't hit the reader if cache is fresh #412

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

Merged
merged 1 commit into from
May 16, 2021

Conversation

derrabus
Copy link
Member

Spotted while working on symfony/symfony#41230.

In debug mode, the PsrCachedReader hits the wrapped reader although the cache is still fresh. This happens because the cache items that stores the modification date is not written initially. This PR attempts to fix this problem.

alcaeus
alcaeus previously approved these changes May 16, 2021
@alcaeus alcaeus added the bug label May 16, 2021
@alcaeus alcaeus added this to the 1.13.1 milestone May 16, 2021
@derrabus
Copy link
Member Author

The PHPStan failure is unrelated.

@greg0ire
Copy link
Member

greg0ire commented May 16, 2021

ArrayCache was indeed removed 3 weeks ago by @alcaeus in doctrine/cache@b3fc4bc
The analyzed test that crash are meant to be run with doctrine/cache 1, but PHPStan uses v2

@greg0ire
Copy link
Member

#413 fixes the build issue (by not analyzing that file)

@derrabus derrabus force-pushed the bugfix/refresh-in-debug-mode branch from 320c01c to b5c257b Compare May 16, 2021 18:04
@derrabus
Copy link
Member Author

And we're green. 🎉

@alcaeus alcaeus merged commit e6e7b7d into doctrine:1.13.x May 16, 2021
@derrabus derrabus deleted the bugfix/refresh-in-debug-mode branch May 16, 2021 18:08
@alcaeus
Copy link
Member

alcaeus commented May 16, 2021

Thanks @derrabus!

@soyuka
Copy link

soyuka commented Jun 1, 2021

I have an issue on API Platform since that change. It looks like refresh can't be called if isHit is truthy. You can reproduce that error using:

docker run -p 27017:27017 mongo:latest
git clone git@github.com:soyuka/core
cd core
git switch fix/phpstan
composer update
composer req --dev symfony/uid symfony/intl
MONGODB_URL=mongodb://localhost:27017 APP_ENV=mongodb php -d memory_limit=-1 ./vendor/bin/behat --profile=mongodb --stop-on-failure features/doctrine/search_filter.feature:7

Check the stacktrace by adding in features/doctrine/search_filter:11:

    Then print last JSON response

Maybe that a fix should be done in mongodb-odm? Let me know if I can help further.

@alcaeus
Copy link
Member

alcaeus commented Jun 2, 2021

@soyuka can you please create a new issue? I fail to see the relationship between what this PR fixed, the issue you reported in the comment, and the PR that linked to this.

@dunglas
Copy link

dunglas commented Jun 4, 2021

I confirm that reverting this patch fixes the problem in API Platform.

@alcaeus
Copy link
Member

alcaeus commented Jun 4, 2021

Thanks for confirming @dunglas! We'll take another look.

@alcaeus
Copy link
Member

alcaeus commented Jun 30, 2021

@dunglas unfortunately I can't find the time to investigate this issue. Would you be able to create a minimal reproducer for us to start our investigation with? Thanks!

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

Successfully merging this pull request may close these issues.

None yet

6 participants