isomerpages / isomercms-backend

A static website builder and host for the Singapore Government
5 stars 1 forks source link

fix(cache): cached results on the same request #1393

Closed kishore03109 closed 4 months ago

kishore03109 commented 4 months ago

Problem

I don't think this PR solves the root cause, but it does prevent this behaviour during testing. This is a fix regarding incident.

To replicate this locally, go to SitesCacheService.ts and modify the function getAllRepoData()

export async function getAllRepoData(
  accessToken: string | undefined
): Promise<RepositoryData[]> {
  console.log({ accessToken }) // this one is local dev hor dont push to production

  // wait until time is a mutliple of 5 seconds
  const now = Date.now()
  const waitTime = 5000 - (now % 5000)
  await new Promise((resolve) => setTimeout(resolve, waitTime))
  const { repos: firstPageRepos, links } = await fetchPageOfRepositories({
    accessToken,
    page: 1,
    getLinks: true,
  })

  console.log(firstPageRepos)

  if (!links?.last || links.last.pagenum <= 1) {
    // There are no links, or no last link specifically, which is the behaviour when the page returned is the last page
    // (In the same manner has the links for the first page do not prev and first links)
    return firstPageRepos
  }

  // There are more pages to retrieve! Fetch all pages 2 to N in parallel, as was done before
  const pageNums = _.range(2, links.last.pagenum + 1)

  const pages2ToLast = await Promise.all(
    pageNums.map((page) => fetchPageOfRepositories({ accessToken, page }))
  )
  pages2ToLast.map((res) => console.log(res.repos))

  return firstPageRepos.concat(...pages2ToLast.map((res) => res.repos))
}

Create 2 different Github users with two different access permissions and spam the /sites page. Sometimes, the other person's response will be returned.

eg.

Expected (non-admin) : Screenshot 2024-05-29 at 3 33 54 PM

Unexpected (non-admin) : Screenshot 2024-05-29 at 3 28 24 PM

Expected (admin) :

Screenshot 2024-05-29 at 3 32 49 PM

Unexpected (admin) :

Screenshot 2024-05-29 at 3 31 57 PM

Solution

Noticed that Breadcrumbs[axios-cache-interceptor](https://github.com/arthurfiorette/axios-cache-interceptor/tree/v0.9.2) returned cache: true. Suspect that is the main culprit here.

By right, the documentation does not warn against setting different auth tokens for the get calls, and neither could I find anything useful in the source code that could suggest this weird behaviour. All I know is that I cannot replicate this behaviour after cache was set to false.

This will prob increase latency, but clearly this is a intentional trade off.

Breaking Changes

Tests

in local:

go to SitesCacheService.ts and modify the function getAllRepoData()

export async function getAllRepoData(
  accessToken: string | undefined
): Promise<RepositoryData[]> {
  console.log({ accessToken }) // this one is local dev hor dont push to production

  // wait until time is a mutliple of 5 seconds
  const now = Date.now()
  const waitTime = 5000 - (now % 5000)
  await new Promise((resolve) => setTimeout(resolve, waitTime))
  const { repos: firstPageRepos, links } = await fetchPageOfRepositories({
    accessToken,
    page: 1,
    getLinks: true,
  })

  console.log(firstPageRepos)

  if (!links?.last || links.last.pagenum <= 1) {
    // There are no links, or no last link specifically, which is the behaviour when the page returned is the last page
    // (In the same manner has the links for the first page do not prev and first links)
    return firstPageRepos
  }

  // There are more pages to retrieve! Fetch all pages 2 to N in parallel, as was done before
  const pageNums = _.range(2, links.last.pagenum + 1)

  const pages2ToLast = await Promise.all(
    pageNums.map((page) => fetchPageOfRepositories({ accessToken, page }))
  )
  pages2ToLast.map((res) => console.log(res.repos))

  return firstPageRepos.concat(...pages2ToLast.map((res) => res.repos))
}

Create 2 different Github users with two different access permissions (on two different browers) and spam the /sites page. assert that the pages returned are accurate to what the user expects.

In staging: 1.Login via github for a NON admin user, assert that you do not see any sites

  1. Login via github for a admin user, assert that you do see all sites