Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions client/src/stores/__tests__/buildings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,18 @@ describe('useBuildingsStore', () => {
})

describe('fetch', () => {
it('does not call authenticatedFetch when already loading', async () => {
it('only calls the API once when multiple fetches happen concurrently', async () => {
vi.mocked(makeRequest).mockResolvedValue(
makeResponse(true, [makeBuilding('b1')]) as unknown as Response,
)
const store = useBuildingsStore()
store.loading = true
const memberships = [makeMembership('acme')]

await store.fetch([makeMembership('acme')])
const fetch1 = store.fetch(memberships)
const fetch2 = store.fetch(memberships)
await Promise.all([fetch1, fetch2])

expect(makeRequest).not.toHaveBeenCalled()
expect(makeRequest).toHaveBeenCalledTimes(1)
})

it('does not call authenticatedFetch when all memberships are already cached', async () => {
Expand Down
50 changes: 31 additions & 19 deletions client/src/stores/__tests__/domain.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('useDomainsStore', () => {
})

describe('fetchMemberships', () => {
it('calls authenticatedFetch with the account-scoped URL', async () => {
it('calls makeRequest with the account-scoped URL', async () => {
vi.mocked(makeRequest).mockResolvedValue(
makeResponse(true, { domains: [] }) as unknown as Response,
)
Expand Down Expand Up @@ -95,7 +95,7 @@ describe('useDomainsStore', () => {
expect(store.memberships).toEqual([])
})

it('does not call authenticatedFetch when memberships are already cached', async () => {
it('does not call makeRequest when memberships are already cached', async () => {
const store = useDomainsStore()
store.memberships = [makeMembership('acme')]

Expand All @@ -104,13 +104,17 @@ describe('useDomainsStore', () => {
expect(makeRequest).not.toHaveBeenCalled()
})

it('does not call authenticatedFetch when a fetch is already in flight', async () => {
it('only calls makeRequest once when multiple fetches happen concurrently', async () => {
vi.mocked(makeRequest).mockResolvedValue(
makeResponse(true, { domains: [] }) as unknown as Response,
)
const store = useDomainsStore()
store.loading = true

await store.fetchMemberships()
const fetch1 = store.fetchMemberships()
const fetch2 = store.fetchMemberships()
await Promise.all([fetch1, fetch2])

expect(makeRequest).not.toHaveBeenCalled()
expect(makeRequest).toHaveBeenCalledTimes(1)
})

it('does not overwrite an existing empty-array cache', async () => {
Expand Down Expand Up @@ -159,7 +163,7 @@ describe('useDomainsStore', () => {
})

describe('fetchAll', () => {
it('calls authenticatedFetch with the shared domains endpoint', async () => {
it('calls makeRequest with the shared domains endpoint', async () => {
vi.mocked(makeRequest).mockResolvedValue(
makeResponse(true, { domains: [] }) as unknown as Response,
)
Expand Down Expand Up @@ -190,7 +194,7 @@ describe('useDomainsStore', () => {
expect(store.allDomains).toEqual([])
})

it('does not call authenticatedFetch when allDomains is already cached', async () => {
it('does not call makeRequest when allDomains is already cached', async () => {
const store = useDomainsStore()
store.allDomains = [makeDomain('acme')]

Expand All @@ -199,13 +203,17 @@ describe('useDomainsStore', () => {
expect(makeRequest).not.toHaveBeenCalled()
})

it('does not call authenticatedFetch when a fetch is already in flight', async () => {
it('only calls makeRequest once when multiple fetchAll happen concurrently', async () => {
vi.mocked(makeRequest).mockResolvedValue(
makeResponse(true, { domains: [] }) as unknown as Response,
)
const store = useDomainsStore()
store.loadingAll = true

await store.fetchAll()
const fetch1 = store.fetchAll()
const fetch2 = store.fetchAll()
await Promise.all([fetch1, fetch2])

expect(makeRequest).not.toHaveBeenCalled()
expect(makeRequest).toHaveBeenCalledTimes(1)
})

it('does not overwrite an existing empty-array cache', async () => {
Expand Down Expand Up @@ -342,16 +350,20 @@ describe('useSubdomainsStore', () => {
})

describe('fetch', () => {
it('does not call authenticatedFetch when already loading', async () => {
it('does not call makeRequest when already loading', async () => {
vi.mocked(makeRequest).mockResolvedValue(makeResponse(true, ['sub1']) as unknown as Response)

const store = useSubdomainsStore()
store.loading = true
const memberships = [makeMembership('acme')]

await store.fetch([makeMembership('acme')])
const fetch1 = store.fetch(memberships)
const fetch2 = store.fetch(memberships)
await Promise.all([fetch1, fetch2])

expect(makeRequest).not.toHaveBeenCalled()
expect(makeRequest).toHaveBeenCalledTimes(1)
})
Comment on lines +353 to 364
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

Test name doesn’t match what it asserts: it says "does not call makeRequest when already loading", but the test actually verifies concurrent calls dedupe down to a single request (toHaveBeenCalledTimes(1)). Rename the test description to reflect the concurrency behavior being tested.

Copilot uses AI. Check for mistakes.

it('does not call authenticatedFetch when all memberships are already cached', async () => {
it('does not call makeRequest when all memberships are already cached', async () => {
const store = useSubdomainsStore()
store.byDomain = { acme: ['sub1'] }

Expand All @@ -360,13 +372,13 @@ describe('useSubdomainsStore', () => {
expect(makeRequest).not.toHaveBeenCalled()
})

it('does not call authenticatedFetch when the memberships list is empty', async () => {
it('does not call makeRequest when the memberships list is empty', async () => {
await useSubdomainsStore().fetch([])

expect(makeRequest).not.toHaveBeenCalled()
})

it('calls authenticatedFetch with the correct URL for each missing domain', async () => {
it('calls makeRequest with the correct URL for each missing domain', async () => {
vi.mocked(makeRequest).mockResolvedValue(
makeResponse(true, ['sub1']) as unknown as Response,
)
Expand Down
33 changes: 21 additions & 12 deletions client/src/stores/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const useAuthStore = defineStore('authentication', {
accountName: null as string | null,
isAuthenticated: false,
isHydrated: false, // has the /me call completed?
_hydratePromise: null as Promise<void> | null,
}),

actions: {
Expand All @@ -18,7 +19,7 @@ export const useAuthStore = defineStore('authentication', {
const data = await res.json()
if (!res.ok) {
console.log(`Failed to login: ${data.type} - ${data.message}`)
return;
return
}
this.accountName = data.account.accountName
this.isAuthenticated = true
Expand Down Expand Up @@ -52,18 +53,26 @@ export const useAuthStore = defineStore('authentication', {

// Called once on app startup to re-hydrate from the cookie
async hydrate() {
try {
const res = await makeRequest('/auth/me')
if (res.ok) {
const data = await res.json()
this.accountName = data.accountName
this.isAuthenticated = true
if (this.isHydrated) return Promise.resolve()
if (this._hydratePromise) return this._hydratePromise

this._hydratePromise = (async () => {
try {
const res = await makeRequest('/auth/me')
if (res.ok) {
const data = await res.json()
this.accountName = data.accountName
this.isAuthenticated = true
}
} catch {
// Cookie missing or expired — user is logged out, do nothing
} finally {
this.isHydrated = true
this._hydratePromise = null
}
} catch {
// Cookie missing or expired — user is logged out, do nothing
} finally {
this.isHydrated = true
}
})()

return this._hydratePromise
},
},
})
44 changes: 26 additions & 18 deletions client/src/stores/buildings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const useBuildingsStore = defineStore('buildings', {
state: () => ({
byDomain: {} as Record<string, Building[]>,
loading: false,
_fetchPromise: null as Promise<void> | null,
}),

getters: {
Expand All @@ -24,32 +25,39 @@ export const useBuildingsStore = defineStore('buildings', {
},

actions: {
async fetch(memberships: DomainMembership[]) {
if (this.loading) return
async fetch(memberships: DomainMembership[]): Promise<void> {
if (this._fetchPromise) {
return this._fetchPromise.then(() => this.fetch(memberships))
}
// Only fetch domains not yet in cache
const missing = memberships.filter((m) => !(m.domainName in this.byDomain))
if (missing.length === 0) return
if (missing.length === 0) return Promise.resolve()

this.loading = true
try {
// All domains fetched in parallel — the key fix vs the old sequential loop
await Promise.all(
missing.map(async (m) => {
try {
const res = await makeRequest(`/twin/buildings/${m.domainName}`)
this.byDomain[m.domainName] = res.ok ? await res.json() : []
} catch {
this.byDomain[m.domainName] = []
}
}),
)
} finally {
this.loading = false
}
this._fetchPromise = (async () => {
try {
await Promise.all(
missing.map(async (m) => {
try {
const res = await makeRequest(`/twin/buildings/${m.domainName}`)
this.byDomain[m.domainName] = res.ok ? await res.json() : []
} catch {
this.byDomain[m.domainName] = []
}
}),
)
} finally {
this.loading = false
this._fetchPromise = null
}
})()

return this._fetchPromise
},

invalidate() {
this.byDomain = {}
this._fetchPromise = null
},
},
})
Loading
Loading