Skip to content

Commit aa407ca

Browse files
diocasclaude
andcommitted
feat(spaces): load project and mount point spaces on demand
Spaces were fetched in one bootstrap batch: personal and project in parallel on login, mount points lazily on first access. Every session therefore paid for listing every project drive - potentially hundreds on CERNBox - even when it never opened one. The personal space is still loaded up front, since virtually every part of the app needs it. Project and mount point spaces are now fetched only when something actually needs them: `loadSpacesByType` loads a type once per session, shares in-flight requests between concurrent callers so a type is never fetched twice, and deduplicates by id and driveAlias. The drive resolver walks personal -> project -> mount point, retrying after each type lands, and only falls back to the catch-all once every type has been tried - so a location is never attributed to the fallback merely because its real space hadn't been fetched yet. Views that show or depend on those types refresh them on entry, so newly created projects and newly accepted shares appear without a page reload: the Spaces overview and Shared with me both refresh project and mount point spaces, the latter via a new `force` flag that re-fetches an already initialized type (additively - known spaces are not duplicated). `spacesLoading` keeps meaning "the initial bootstrap is running" and is deliberately not flipped by on-demand loads: the application layout swaps the whole router view for a spinner while it is true, so a view that refreshes spaces on mount would unmount and remount itself in an endless loop. `spacesInitialized` likewise keeps meaning "bootstrap finished", not "everything is loaded" - redefining it would leave `areSpacesLoading` permanently true and hang the drive resolver. `reloadProjectSpaces` now also evicts same-alias share/mountpoint entries: with lazy loading those can arrive before the project spaces, and a synthesized share root must never shadow the real project space it was derived from. `initializedTypes` is exposed as state rather than through `isTypeInitialized` because createTestingPinia stubs every function a store returns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent be36e27 commit aa407ca

12 files changed

Lines changed: 476 additions & 191 deletions

File tree

packages/web-app-files/src/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ export const navItems = (context: ComponentCustomProperties): AppNavigationItem[
7777
: []
7878
},
7979
isVisible() {
80-
if (!spacesStores.spacesInitialized) {
80+
// personal spaces are fetched on demand, so "not loaded yet" must not read as "the user
81+
// has none" - that would hide the item for the rest of the session
82+
if (!spacesStores.initializedTypes.personal) {
8183
return true
8284
}
8385

packages/web-app-files/src/services/folder/loaderSharedWithMe.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,18 @@ export class FolderLoaderSharedWithMe implements FolderLoader {
2121
resourcesStore.clearResourceList()
2222
resourcesStore.setAncestorMetaData({})
2323

24+
// project and mount point spaces are loaded on demand - refresh both here so newly accepted
25+
// shares resolve to their real space instead of falling back
26+
yield spacesStore.reloadProjectSpaces({
27+
graphClient: clientService.graphAuthenticated,
28+
signal: signal1
29+
})
30+
2431
if (configStore.options.routing.fullShareOwnerPaths) {
2532
yield spacesStore.loadMountPoints({
2633
graphClient: clientService.graphAuthenticated,
27-
signal: signal1
34+
signal: signal1,
35+
force: true
2836
})
2937
}
3038

packages/web-app-files/src/views/spaces/Projects.vue

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,12 @@ export default defineComponent({
280280
const { setSelection, initResourceList, clearResourceList, setAncestorMetaData } =
281281
useResourcesStore()
282282
283-
const userHasPersonalSpace = !!spacesStore.spaces.find(
284-
(drive) => isPersonalSpaceResource(drive) && drive.isOwner(userStore.user)
283+
// must be reactive: personal spaces are loaded on demand, so evaluating this once during
284+
// setup would pin it to `false` on a cold start
285+
const userHasPersonalSpace = computed(() =>
286+
spacesStore.spaces.some(
287+
(drive) => isPersonalSpaceResource(drive) && drive.isOwner(userStore.user)
288+
)
285289
)
286290
const visibilityOption = useRouteQueryPersisted({
287291
name: 'q_projectVisibility',
@@ -311,10 +315,17 @@ export default defineComponent({
311315
const loadResourcesTask = useTask(function* (signal) {
312316
clearResourceList()
313317
setAncestorMetaData({})
318+
// project and mount point spaces are loaded on demand - refresh both here so newly created
319+
// projects and newly accepted shares show up without a page reload
314320
yield spacesStore.reloadProjectSpaces({
315321
graphClient: clientService.graphAuthenticated,
316322
signal
317323
})
324+
yield spacesStore.loadMountPoints({
325+
graphClient: clientService.graphAuthenticated,
326+
signal,
327+
force: true
328+
})
318329
initResourceList({ currentFolder: null, resources: unref(spaces) })
319330
})
320331
@@ -432,7 +443,7 @@ export default defineComponent({
432443
433444
const hasCreatePermission = computed(
434445
// if user has a personal space, it's not a lightweight account
435-
() => can('create-all', 'Drive') && userHasPersonalSpace
446+
() => can('create-all', 'Drive') && unref(userHasPersonalSpace)
436447
)
437448
438449
const extensionRegistry = useExtensionRegistry()

packages/web-app-files/tests/unit/index.spec.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,18 @@ describe('Web app files', () => {
2626
spacesStore.spaces = [
2727
mock<SpaceResource>({ id: '1', driveType: 'project', isOwner: () => false })
2828
]
29+
// only once personal spaces have actually been fetched does "none present" mean the user
30+
// has none
31+
spacesStore.setTypeInitialized('personal', true)
2932
const items = navItems(undefined)
3033
expect(items[0].isVisible()).toBeFalsy()
3134
})
35+
it('stays visible while personal spaces have not been loaded yet', () => {
36+
const spacesStore = useSpacesStore()
37+
spacesStore.spaces = []
38+
const items = navItems(undefined)
39+
expect(items[0].isVisible()).toBeTruthy()
40+
})
3241
})
3342
describe('Spaces', () => {
3443
it.each([

packages/web-pkg/src/components/AppTemplates/AppWrapper.vue

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
:resource="resource"
1212
@close="closeApp"
1313
/>
14-
<loading-screen v-if="loading" />
14+
<loading-screen v-if="isLoading" />
1515
<error-screen v-else-if="loadingError" :message="loadingError.message" />
1616
<div
1717
v-else
@@ -217,6 +217,12 @@ export default defineComponent({
217217
applicationId: props.applicationId
218218
})
219219
220+
// components that load their own resource keep `loading` false from the start, so without this
221+
// the slot would render before the drive resolver has produced a file context. Resolving a
222+
// space is asynchronous (drive types are fetched on demand), and everything below - `slotAttrs`
223+
// included - assumes a context is there.
224+
const isLoading = computed(() => unref(loading) || !unref(currentFileContext))
225+
220226
const { applicationMeta } = useAppMeta({ applicationId: props.applicationId, appsStore })
221227
222228
const fileSizeLimit = computed(() => {
@@ -669,7 +675,7 @@ export default defineComponent({
669675
670676
const slotAttrs = computed(() => ({
671677
url: unref(url),
672-
space: unref(unref(currentFileContext).space),
678+
space: unref(unref(currentFileContext)?.space),
673679
resource: unref(resource),
674680
activeFiles: unref(activeFiles),
675681
isDirty: unref(isDirty),
@@ -681,7 +687,7 @@ export default defineComponent({
681687
682688
'onUpdate:resource': (value: Resource) => {
683689
resource.value = value
684-
space.value = unref(unref(currentFileContext).space)
690+
space.value = unref(unref(currentFileContext)?.space)
685691
selectedResources.value = [value]
686692
},
687693
'onUpdate:currentContent': (value: unknown) => {
@@ -704,6 +710,7 @@ export default defineComponent({
704710
closeApp,
705711
fileActions,
706712
loading,
713+
isLoading,
707714
loadingError,
708715
pageTitle,
709716
resource,

packages/web-pkg/src/composables/driveResolver/useDriveResolver.ts

Lines changed: 100 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { useSpacesLoading } from './useSpacesLoading'
1212
import { queryItemAsString } from '../appDefaults'
1313
import { urlJoin } from '@ownclouders/web-client'
1414
import { useClientService } from '../clientService'
15-
import { useSpacesStore, useConfigStore } from '../piniaStores'
15+
import { useSpacesStore, useConfigStore, LOADABLE_DRIVE_TYPES } from '../piniaStores'
1616
import { onUnmounted } from 'vue'
1717

1818
interface DriveResolverOptions {
@@ -80,102 +80,118 @@ export const useDriveResolver = (options: DriveResolverOptions = {}): DriveResol
8080
}
8181
})
8282

83-
watch(
84-
[options.driveAliasAndItem, areSpacesLoading],
85-
async ([driveAliasAndItem, areSpacesLoading], [driveAliasAndItemOld, areSpacesLoadingOld]) => {
86-
if (driveAliasAndItem === driveAliasAndItemOld && areSpacesLoading === areSpacesLoadingOld) {
87-
return
88-
}
83+
const resolve = async (driveAliasAndItem: string) => {
84+
if (!driveAliasAndItem || driveAliasAndItem.startsWith('virtual/')) {
85+
space.value = null
86+
item.value = null
87+
return
88+
}
8989

90-
if (!driveAliasAndItem || driveAliasAndItem.startsWith('virtual/')) {
91-
space.value = null
92-
item.value = null
93-
return
90+
const resolvedSpace = unref(space)
91+
// never latch onto the fallback: a deeper path may be covered by a real space that we
92+
// either already hold or can still lazily load, so always re-resolve. For real spaces the
93+
// shortcut is kept, but segment-aware: `eos/project/c/cern` must not swallow
94+
// `eos/project/c/cernbox/x` (which would yield item `box/x` on the wrong space).
95+
const isOnlyItemPathChanged =
96+
!!resolvedSpace &&
97+
!isFallbackSpaceResource(resolvedSpace) &&
98+
isSegmentPrefix(driveAliasAndItem, resolvedSpace.driveAlias)
99+
if (isOnlyItemPathChanged) {
100+
item.value = urlJoin(driveAliasAndItem.slice(resolvedSpace.driveAlias.length), {
101+
leadingSlash: true
102+
})
103+
return
104+
}
105+
106+
let matchingSpace = null
107+
let path = null
108+
if (driveAliasAndItem.startsWith('public/') || driveAliasAndItem.startsWith('ocm/')) {
109+
const [publicLinkToken, ...item] = driveAliasAndItem.split('/').slice(1)
110+
matchingSpace = unref(spaces).find((s) => s.id === publicLinkToken)
111+
path = item.join('/')
112+
} else if (
113+
driveAliasAndItem.startsWith('share/') ||
114+
driveAliasAndItem.startsWith('ocm-share/')
115+
) {
116+
const [shareName, ...item] = driveAliasAndItem.split('/').slice(1)
117+
const driveAliasPrefix = driveAliasAndItem.startsWith('ocm-share/') ? 'ocm-share' : 'share'
118+
119+
let shareIdStr = queryItemAsString(unref(shareId))
120+
// keep compatibility with old share jail ids pre sharing NG
121+
if (shareIdStr?.includes(':')) {
122+
shareIdStr = [SHARE_JAIL_ID, shareIdStr].join('!')
94123
}
95124

96-
const resolvedSpace = unref(space)
97-
// never latch onto the fallback: a deeper path may be covered by a real space that we
98-
// either already hold or can still lazily load, so always re-resolve. For real spaces the
99-
// shortcut is kept, but segment-aware: `eos/project/c/cern` must not swallow
100-
// `eos/project/c/cernbox/x` (which would yield item `box/x` on the wrong space).
101-
const isOnlyItemPathChanged =
102-
!!resolvedSpace &&
103-
!isFallbackSpaceResource(resolvedSpace) &&
104-
isSegmentPrefix(driveAliasAndItem, resolvedSpace.driveAlias)
105-
if (isOnlyItemPathChanged) {
106-
item.value = urlJoin(driveAliasAndItem.slice(resolvedSpace.driveAlias.length), {
107-
leadingSlash: true
125+
matchingSpace =
126+
spacesStore.getSpace(shareIdStr) ||
127+
spacesStore.createShareSpace({
128+
driveAliasPrefix,
129+
id: shareIdStr,
130+
shareName: unref(shareName)
131+
})
132+
133+
path = item.join('/')
134+
} else {
135+
if (unref(fileId)) {
136+
matchingSpace = unref(spaces).find((s) => {
137+
return unref(fileId).startsWith(`${s.fileId}`)
108138
})
109-
return
110139
}
111140

112-
let matchingSpace = null
113-
let path = null
114-
if (driveAliasAndItem.startsWith('public/') || driveAliasAndItem.startsWith('ocm/')) {
115-
const [publicLinkToken, ...item] = driveAliasAndItem.split('/').slice(1)
116-
matchingSpace = unref(spaces).find((s) => s.id === publicLinkToken)
117-
path = item.join('/')
118-
} else if (
119-
driveAliasAndItem.startsWith('share/') ||
120-
driveAliasAndItem.startsWith('ocm-share/')
121-
) {
122-
const [shareName, ...item] = driveAliasAndItem.split('/').slice(1)
123-
const driveAliasPrefix = driveAliasAndItem.startsWith('ocm-share/') ? 'ocm-share' : 'share'
124-
125-
let shareIdStr = queryItemAsString(unref(shareId))
126-
// keep compatibility with old share jail ids pre sharing NG
127-
if (shareIdStr?.includes(':')) {
128-
shareIdStr = [SHARE_JAIL_ID, shareIdStr].join('!')
129-
}
141+
// real spaces are fetched per drive type on demand, cheapest first. Try what we already
142+
// hold, then load one type at a time and retry, so a location is never attributed to the
143+
// catch-all fallback just because its real space hadn't been fetched yet.
144+
if (!matchingSpace) {
145+
matchingSpace = getSpaceByDriveAliasAndItem(driveAliasAndItem, { includeFallback: false })
146+
}
130147

131-
matchingSpace =
132-
spacesStore.getSpace(shareIdStr) ||
133-
spacesStore.createShareSpace({
134-
driveAliasPrefix,
135-
id: shareIdStr,
136-
shareName: unref(shareName)
137-
})
138-
139-
path = item.join('/')
140-
} else {
141-
if (unref(fileId)) {
142-
matchingSpace = unref(spaces).find((s) => {
143-
return unref(fileId).startsWith(`${s.fileId}`)
144-
})
148+
for (const driveType of LOADABLE_DRIVE_TYPES) {
149+
if (matchingSpace || spacesStore.initializedTypes[driveType]) {
150+
continue
145151
}
146-
147-
// 1. real spaces we already know about
148-
if (!matchingSpace) {
149-
matchingSpace = getSpaceByDriveAliasAndItem(driveAliasAndItem, { includeFallback: false })
152+
// mount points only ever contribute an owner-path shaped driveAlias (and are expensive
153+
// to fetch), so they can't change the outcome unless full share owner paths are on
154+
if (driveType === 'mountpoint' && !configStore.options.routing.fullShareOwnerPaths) {
155+
continue
150156
}
151157

152-
// 2. the location may live in a received share whose owner-path root space hasn't been
153-
// fetched yet. Fetching mount points is expensive, so only once per session and only
154-
// when it can actually produce a matching driveAlias.
155-
if (
156-
!matchingSpace &&
157-
!spacesStore.mountPointsInitialized &&
158-
configStore.options.routing.fullShareOwnerPaths
159-
) {
160-
loading.value = true
161-
await spacesStore.loadMountPoints({ graphClient: clientService.graphAuthenticated })
162-
matchingSpace = getSpaceByDriveAliasAndItem(driveAliasAndItem, { includeFallback: false })
163-
}
158+
loading.value = true
159+
await spacesStore.loadSpacesByType(driveType, {
160+
graphClient: clientService.graphAuthenticated
161+
})
162+
matchingSpace = getSpaceByDriveAliasAndItem(driveAliasAndItem, { includeFallback: false })
163+
}
164164

165-
// 3. last resort: the synthetic catch-all space, if this deployment has one
166-
if (!matchingSpace) {
167-
matchingSpace = getSpaceByDriveAliasAndItem(driveAliasAndItem)
168-
}
165+
// last resort: the synthetic catch-all space, if this deployment has one
166+
if (!matchingSpace) {
167+
matchingSpace = getSpaceByDriveAliasAndItem(driveAliasAndItem)
168+
}
169169

170-
if (matchingSpace) {
171-
path = driveAliasAndItem.slice(matchingSpace.driveAlias.length)
172-
}
170+
if (matchingSpace) {
171+
path = driveAliasAndItem.slice(matchingSpace.driveAlias.length)
172+
}
173+
}
174+
space.value = matchingSpace
175+
item.value = urlJoin(path, {
176+
leadingSlash: true
177+
})
178+
}
179+
180+
watch(
181+
[options.driveAliasAndItem, areSpacesLoading],
182+
async ([driveAliasAndItem, areSpacesLoading], [driveAliasAndItemOld, areSpacesLoadingOld]) => {
183+
if (driveAliasAndItem === driveAliasAndItemOld && areSpacesLoading === areSpacesLoadingOld) {
184+
return
185+
}
186+
187+
// `loading` decides whether consumers have a usable file context at all, so it has to be
188+
// reset on every exit. An early return or a failing drive request would otherwise leave the
189+
// app stuck on a loading screen for the rest of the session.
190+
try {
191+
await resolve(driveAliasAndItem)
192+
} finally {
193+
loading.value = false
173194
}
174-
space.value = matchingSpace
175-
item.value = urlJoin(path, {
176-
leadingSlash: true
177-
})
178-
loading.value = false
179195
},
180196
{ immediate: true, deep: true }
181197
)

0 commit comments

Comments
 (0)