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: clear cacheVersions on flushCache #807

Merged
merged 2 commits into from
Apr 15, 2024
Merged

Conversation

christianzoppi
Copy link
Contributor

@christianzoppi christianzoppi commented Apr 12, 2024

This PR fixes the issue number 2 from #806. The issue is intermittent because of a combination of factors, but in some cases it will result in the JS Client serving cached content even after flushing the cache, because the flushCache method clears the cache but it doesn't clear the cached cv value used for performing requests.

Pull request type

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Other (please describe):

How to test this PR

In order to simulate a case that would be affected by this bug, you can run the following script. To get the accessToken, spaceId, storyId and get added to the space for testing, please DM me. Of course you can setup your own space and script, but with this one it will be quick :-)

If you run it on the main branch, you will always get the same values for all the 3 requests. With this branch you will get the current value after flushing the cache.

import StoryblokClient from "storyblok-js-client";

const oauthToken = "";
const accessToken = ""
const spaceId = "";
const storyId = "";

const Storyblok = new StoryblokClient({
  accessToken,
  cache: {
    clear: "auto",
    type: "memory",
  },
});

const StoryblokMapi = new StoryblokClient({
  oauthToken
});

const updateStory = async () => {
  const hp = await StoryblokMapi.get(`/spaces/${spaceId}/stories/${storyId}`);
  hp.data.story.content.v = parseInt(hp.data.story.content.v) + 1;
  try {
    await StoryblokMapi.put(`/spaces/${spaceId}/stories/${storyId}`, {
      story: hp.data.story,
      publish: 1,
      force_update: 1,
    });
  } catch (e) {
    console.log(e);
  }
};

const getHome = async (cv) => {
  const res = await Storyblok.get("cdn/stories/home", {
    version: "published",
    ...(cv && { cv: Date.now() }),
  });
  console.log(`Test Field value: ${res.data.story.content.v} - Response CV value: ${res.data.cv}`);
};

(async () => {
  try {
    await getHome();
    await getHome();
    await updateStory();
    await Storyblok.flushCache();
    console.log("---");
    await getHome();
  } catch (err) {
    console.log(err);
  }
})();

What is the new behavior?

The cv value is set to 0 when the flushCache method is executed.

Other information

src/index.ts Outdated
@@ -722,6 +728,7 @@ class Storyblok {

public async flushCache(): Promise<this> {
await this.cacheProvider().flush()
this.clearCacheVersion();
Copy link
Member

Choose a reason for hiding this comment

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

I think you could remove the ;

Suggested change
this.clearCacheVersion();
this.clearCacheVersion()

Copy link
Member

@ademarCardoso ademarCardoso left a comment

Choose a reason for hiding this comment

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

I tested using the script and ran other tests and it seems to be working well.

@christianzoppi christianzoppi merged commit b6abea2 into main Apr 15, 2024
1 check passed
@christianzoppi christianzoppi deleted the fix/flushcache-clear-cv branch April 15, 2024 09:40
Copy link

🎉 This PR is included in version 6.7.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants