Skip to content

Commit

Permalink
Merge pull request #4443 from Shopify/cache-user-info
Browse files Browse the repository at this point in the history
Cache user info
  • Loading branch information
isaacroldan authored Sep 16, 2024
2 parents 40c0e86 + 0cf9b49 commit 591f653
Show file tree
Hide file tree
Showing 9 changed files with 269 additions and 20 deletions.
54 changes: 43 additions & 11 deletions packages/app/src/cli/services/context/partner-account-info.test.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,69 @@
import {AccountInfo, fetchCurrentAccountInformation} from './partner-account-info.js'
import {testDeveloperPlatformClient} from '../../models/app/app.test-data.js'
import {getCurrentAccountInfo} from '../../api/graphql/current_account_info.js'
import {describe, expect, test, vi} from 'vitest'
import {clearCachedAccountInfo, getCachedAccountInfo, setCachedAccountInfo} from '../../utilities/app-conf-store.js'
import {beforeEach, describe, expect, test, vi} from 'vitest'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputDebug} from '@shopify/cli-kit/node/output'

vi.mock('@shopify/cli-kit/node/session')
vi.mock('../../api/graphql/current_account_info.js')
vi.mock('@shopify/cli-kit/node/environment')
vi.mock('@shopify/cli-kit/node/output')

const userId = '1234-5678'
const accountInfo: AccountInfo = {
type: 'UserAccount',
email: '[email protected]',
}

describe('fetchCurrentAccountInformation', () => {
test('returns complete user account info', async () => {
beforeEach(() => {
clearCachedAccountInfo()
})

test('returns cached account info if available', async () => {
// Given
setCachedAccountInfo(userId, accountInfo)

// When
const got = await fetchCurrentAccountInformation(testDeveloperPlatformClient(), userId)

// Then
expect(got).toEqual(accountInfo)
expect(outputDebug).toHaveBeenCalledWith('Getting partner account info from cache')
expect(getCurrentAccountInfo).not.toHaveBeenCalled()
})

test('fetches and caches account info if not in cache', async () => {
// Given
const userAccountInfo: AccountInfo = {
type: 'UserAccount',
email: '[email protected]',
}
vi.mocked(getCurrentAccountInfo).mockResolvedValue(userAccountInfo)
vi.mocked(getCurrentAccountInfo).mockResolvedValue(accountInfo)

// When
const got = await fetchCurrentAccountInformation(testDeveloperPlatformClient())
const got = await fetchCurrentAccountInformation(testDeveloperPlatformClient(), userId)

// Then
expect(got).toEqual(userAccountInfo)
expect(got).toEqual(accountInfo)

// Verify that the info was cached
const cachedInfo = getCachedAccountInfo(userId)
expect(cachedInfo).toEqual(accountInfo)
})

test('when error fetching account info returns unkonwn partner info', async () => {
test('when error fetching account info returns unknown partner info', async () => {
// Given
clearCachedAccountInfo()
vi.mocked(getCurrentAccountInfo).mockRejectedValue(new AbortError('Error'))

// When
const got = await fetchCurrentAccountInformation(testDeveloperPlatformClient())
const got = await fetchCurrentAccountInformation(testDeveloperPlatformClient(), userId)

// Then
expect(got).toEqual({type: 'UnknownAccount'})
expect(outputDebug).toHaveBeenCalledWith('Error fetching user account info')

// Verify that no info was cached
const cachedInfo = getCachedAccountInfo(userId)
expect(cachedInfo).toBeUndefined()
})
})
13 changes: 12 additions & 1 deletion packages/app/src/cli/services/context/partner-account-info.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {getCurrentAccountInfo} from '../../api/graphql/current_account_info.js'
import {getCachedAccountInfo, setCachedAccountInfo} from '../../utilities/app-conf-store.js'
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {outputDebug} from '@shopify/cli-kit/node/output'

Expand Down Expand Up @@ -34,9 +35,19 @@ export function isServiceAccount(account: AccountInfo): account is ServiceAccoun

export async function fetchCurrentAccountInformation(
developerPlatformClient: DeveloperPlatformClient,
subject: string,
): Promise<AccountInfo> {
const cachedInfo = getCachedAccountInfo(subject)

if (cachedInfo) {
outputDebug('Getting partner account info from cache')
return cachedInfo
}

try {
return await getCurrentAccountInfo(developerPlatformClient)
const fromApi = await getCurrentAccountInfo(developerPlatformClient)
setCachedAccountInfo(subject, fromApi)
return fromApi
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
outputDebug('Error fetching user account info')
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/cli/services/dev/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ export async function fetchOrganizations(): Promise<Organization[]> {

if (organizations.length === 0) {
const developerPlatformClient = selectDeveloperPlatformClient()
const accountInfo = await fetchCurrentAccountInformation(developerPlatformClient)
const session = await developerPlatformClient.session()
const accountInfo = await fetchCurrentAccountInformation(developerPlatformClient, session.userId)
throw new NoOrgError(accountInfo)
}
return organizations
Expand Down
98 changes: 98 additions & 0 deletions packages/app/src/cli/utilities/app-conf-store.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import {
getConfigStoreForAccountInfoStatus,
getCachedAccountInfo,
setCachedAccountInfo,
clearCachedAccountInfo,
} from './app-conf-store.js'
import {AccountInfo} from '../services/context/partner-account-info.js'
import {vi, describe, test, expect, beforeEach, afterEach} from 'vitest'
import {LocalStorage} from '@shopify/cli-kit/node/local-storage'
import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs'

describe('app-conf-store', () => {
beforeEach(async () => {
vi.useFakeTimers()
vi.setSystemTime(new Date('2023-01-01'))
})

afterEach(() => {
vi.useRealTimers()
})

test('getConfigStoreForAccountInfoStatus returns a LocalStorage instance', async () => {
await inTemporaryDirectory(async (tempDir) => {
const result = getConfigStoreForAccountInfoStatus(tempDir)
expect(result).toBeInstanceOf(LocalStorage)
})
})

describe('getCachedAccountInfo', () => {
test('returns undefined when no cached data exists', async () => {
await inTemporaryDirectory(async (tempDir) => {
const result = getCachedAccountInfo('testSubject', tempDir)
expect(result).toBeUndefined()
})
})

test('returns undefined when cached data is expired', async () => {
// More than 72 hours old
await inTemporaryDirectory(async (tempDir) => {
const loadedAt = new Date('2022-11-28').toISOString()

const store = getConfigStoreForAccountInfoStatus(tempDir)
await store.set('testSubject', {
info: {type: 'UserAccount', email: '[email protected]'} as AccountInfo,
loadedAt,
})

const result = getCachedAccountInfo('testSubject', tempDir)
expect(result).toBeUndefined()
})
})

test('returns cached info when data is valid', async () => {
await inTemporaryDirectory(async (tempDir) => {
// Less than 72 hours old

const validDate = new Date('2022-12-31').toISOString()
const mockInfo: AccountInfo = {type: 'UserAccount', email: '[email protected]'}
const store = getConfigStoreForAccountInfoStatus(tempDir)
await store.set('testSubject', {
info: mockInfo,
loadedAt: validDate,
})

const result = getCachedAccountInfo('testSubject', tempDir)
expect(result).toEqual(mockInfo)
})
})
})

test('setCachedAccountInfo sets the data correctly', async () => {
await inTemporaryDirectory(async (tempDir) => {
const mockInfo: AccountInfo = {type: 'ServiceAccount', orgName: 'Test Org'}

await setCachedAccountInfo('testSubject', mockInfo, tempDir)

const store = getConfigStoreForAccountInfoStatus(tempDir)
const result = await store.get('testSubject')
expect(result).toEqual({
info: mockInfo,
loadedAt: '2023-01-01T00:00:00.000Z',
})
})
})

test('clearCachedAccountInfo clears the store', async () => {
await inTemporaryDirectory(async (tempDir) => {
const mockInfo: AccountInfo = {type: 'UnknownAccount'}
await setCachedAccountInfo('testSubject', mockInfo, tempDir)

await clearCachedAccountInfo(tempDir)

const store = getConfigStoreForAccountInfoStatus(tempDir)
const result = await store.get('testSubject')
expect(result).toBeUndefined()
})
})
})
36 changes: 36 additions & 0 deletions packages/app/src/cli/utilities/app-conf-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import {AccountInfo} from '../services/context/partner-account-info.js'
import {LocalStorage} from '@shopify/cli-kit/node/local-storage'

// max age is 72 hours (3 days)
const MAX_AGE_FOR_ACCOUNT_INFO_STATUS_MS = 3 * 24 * 60 * 60 * 1000

export function getConfigStoreForAccountInfoStatus(cwd?: string) {
return new LocalStorage<{[subject: string]: {info: AccountInfo; loadedAt: string}}>({
projectName: 'shopify-app-account-info',
cwd,
})
}

export function getCachedAccountInfo(subject: string, cwd?: string) {
const store = getConfigStoreForAccountInfoStatus(cwd)
const cached = store.get(subject)
if (cached) {
// get age of cached data
const age = new Date().valueOf() - new Date(cached.loadedAt).valueOf()
if (age > MAX_AGE_FOR_ACCOUNT_INFO_STATUS_MS) {
return undefined
}
return cached.info
}
return undefined
}

export function setCachedAccountInfo(subject: string, accountInfo: AccountInfo, cwd?: string) {
const store = getConfigStoreForAccountInfoStatus(cwd)
store.set(subject, {info: accountInfo, loadedAt: new Date().toISOString()})
}

export function clearCachedAccountInfo(cwd?: string) {
const store = getConfigStoreForAccountInfoStatus(cwd)
store.clear()
}
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ export class PartnersClient implements DeveloperPlatformClient {
accountInfo: {type: 'UnknownAccount'},
userId,
}
const accountInfo = await fetchCurrentAccountInformation(this)
const accountInfo = await fetchCurrentAccountInformation(this, userId)
this._session = {token, accountInfo, userId}
}
return this._session
Expand Down
39 changes: 38 additions & 1 deletion packages/cli-kit/src/private/node/conf-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import {
removeSession,
setSession,
runAtMinimumInterval,
getConfigStoreForPartnerStatus,
getCachedPartnerAccountStatus,
setCachedPartnerAccountStatus,
runWithRateLimit,
} from './conf-store.js'
import {LocalStorage} from '../../public/node/local-storage.js'
import {afterEach, describe, expect, test, vi} from 'vitest'
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs'

describe('getSession', () => {
Expand Down Expand Up @@ -400,3 +403,37 @@ describe('runWithRateLimit', () => {
})
})
})

describe('Partner Account Status Cache', () => {
beforeEach(() => {
// Clear the partner status store before each test
const store = getConfigStoreForPartnerStatus()
store.clear()
})

describe('getCachedPartnerAccountStatus', () => {
test('returns null for empty token', () => {
expect(getCachedPartnerAccountStatus('')).toBeNull()
})

test('returns null for non-existent token', () => {
expect(getCachedPartnerAccountStatus('non-existent-token')).toBeNull()
})

test('returns true for existing token', () => {
const token = 'existing-token'
setCachedPartnerAccountStatus(token)

expect(getCachedPartnerAccountStatus(token)).toBe(true)
})
})

describe('setCachedPartnerAccountStatus', () => {
test('sets a new token', () => {
const token = 'new-token'
setCachedPartnerAccountStatus(token)

expect(getCachedPartnerAccountStatus(token)).toBe(true)
})
})
})
24 changes: 24 additions & 0 deletions packages/cli-kit/src/private/node/conf-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,27 @@ export async function runWithRateLimit(options: RunWithRateLimitOptions, config

return true
}

export function getConfigStoreForPartnerStatus() {
return new LocalStorage<{[partnerToken: string]: {status: true; checkedAt: string}}>({
projectName: 'shopify-cli-kit-partner-status',
})
}

export function getCachedPartnerAccountStatus(partnersToken: string): true | null {
if (!partnersToken) return null
const store = getConfigStoreForPartnerStatus()

const hasPartnerAccount = store.get(partnersToken)
if (hasPartnerAccount) {
// this never needs to expire
return true
}
return null
}

export function setCachedPartnerAccountStatus(partnersToken: string) {
const store = getConfigStoreForPartnerStatus()

store.set(partnersToken, {status: true, checkedAt: new Date().toISOString()})
}
Loading

0 comments on commit 591f653

Please sign in to comment.