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 post cache #1185

Merged
merged 2 commits into from
Jan 18, 2024
Merged

Fix post cache #1185

merged 2 commits into from
Jan 18, 2024

Conversation

leogermani
Copy link
Contributor

Description of the Change

Fixes a typo in DistributorPost::parse_media_blocks that was not correctly parsing the post ID into the cache key.

This was causing the cache for all posts to be stored into the same key and override the cache from a previous cached post.

In environments that use persistent cache, this would cause distributed images to send the wrong images out sites.

How to test the Change

This will most likely only affect environments using persistent cache. But by adding some debugging before and after this change you can confirm that the cache is not being saved with the correct key, in which the post ID is parsed.

Changelog Entry

Fixed - Caching Post images

Credits

Props @leogermani

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@leogermani leogermani requested a review from a team as a code owner January 18, 2024 21:46
@leogermani leogermani requested review from faisal-alvi and removed request for a team January 18, 2024 21:46
@dkotter dkotter requested review from peterwilsoncc and removed request for faisal-alvi January 18, 2024 21:49
@jeffpaul jeffpaul added this to the 2.1.0 milestone Jan 18, 2024
@jeffpaul jeffpaul added the type:bug Something isn't working. label Jan 18, 2024
Copy link
Collaborator

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

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

Thanks for catching this @leogermani.

This looks good to me and tests well.

@peterwilsoncc peterwilsoncc merged commit 30bc0cf into 10up:develop Jan 18, 2024
14 of 15 checks passed
@leogermani
Copy link
Contributor Author

Thanks for catching this @leogermani.

This looks good to me and tests well.

@peterwilsoncc Thank you.

This creates a huge mess when using persistent cache in the network of sites. We are being able to workaround it by clearing this cache entry on every request, but it would be great to have this released as soon as you think it's convenient.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants