Skip to content

Commit

Permalink
Batch asset invalidations (#11937)
Browse files Browse the repository at this point in the history
- Close enso-org/cloud-v2#1627
- Batch asset invalidations for:
- Bulk deletes
- Bulk undo deletes (restores)
- Bulk copy/move
- Bulk download
- This avoids flickering when the directory list is invalidated multiple times (once for the mutation corresponding to each asset)

Codebase changes:
- Remove all `AssetEvent`s and `AssetListEvent`s. Remaining events have been moved to TanStack Query mutations in this PR, as it is neccessary for batch invalidation functionality.
- Remove `key` and `directoryKey` from `AssetTreeNode`, and `key`s in general in favor of `id`s. Not *strictly* necessary, but it was causing logic issues and is (IMO) a nice simplification to be able to do.

# Important Notes
None
  • Loading branch information
somebody1234 authored Jan 8, 2025
1 parent 787372e commit 25933af
Show file tree
Hide file tree
Showing 65 changed files with 2,683 additions and 3,230 deletions.
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,9 @@ test-results/
test-traces/
playwright-report/
playwright/.cache/

#########
## Git ##
#########

/.mailmap
10 changes: 6 additions & 4 deletions app/common/src/backendQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export type BackendMethods = object.ExtractKeys<Backend, object.MethodOf<Backend

/** For each backend method, an optional function defining how to create a query key from its arguments. */
type BackendQueryNormalizers = {
[Method in BackendMethods]?: (...args: Parameters<Backend[Method]>) => queryCore.QueryKey
[Method in BackendMethods]?: (
...args: Readonly<Parameters<Backend[Method]>>
) => queryCore.QueryKey
}

const NORMALIZE_METHOD_QUERY: BackendQueryNormalizers = {
Expand All @@ -22,7 +24,7 @@ const NORMALIZE_METHOD_QUERY: BackendQueryNormalizers = {
/** Creates a partial query key representing the given method and arguments. */
function normalizeMethodQuery<Method extends BackendMethods>(
method: Method,
args: Parameters<Backend[Method]>,
args: Readonly<Parameters<Backend[Method]>>,
) {
return NORMALIZE_METHOD_QUERY[method]?.(...args) ?? args
}
Expand All @@ -31,7 +33,7 @@ function normalizeMethodQuery<Method extends BackendMethods>(
export function backendQueryOptions<Method extends BackendMethods>(
backend: Backend | null,
method: Method,
args: Parameters<Backend[Method]>,
args: Readonly<Parameters<Backend[Method]>>,
keyExtra?: queryCore.QueryKey | undefined,
): {
queryKey: queryCore.QueryKey
Expand All @@ -47,7 +49,7 @@ export function backendQueryOptions<Method extends BackendMethods>(
export function backendQueryKey<Method extends BackendMethods>(
backend: Backend | null,
method: Method,
args: Parameters<Backend[Method]>,
args: Readonly<Parameters<Backend[Method]>>,
keyExtra?: queryCore.QueryKey | undefined,
): queryCore.QueryKey {
return [backend?.type, method, ...normalizeMethodQuery(method, args), ...(keyExtra ?? [])]
Expand Down
4 changes: 4 additions & 0 deletions app/common/src/queryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ declare module '@tanstack/query-core' {
* @default false
*/
readonly awaitInvalidates?: queryCore.QueryKey[] | boolean
readonly refetchType?: queryCore.InvalidateQueryFilters['refetchType']
}

readonly queryMeta: {
Expand Down Expand Up @@ -98,6 +99,7 @@ export function createQueryClient<TStorageValue = string>(
mutationCache: new queryCore.MutationCache({
onSuccess: (_data, _variables, _context, mutation) => {
const shouldAwaitInvalidates = mutation.meta?.awaitInvalidates ?? false
const refetchType = mutation.meta?.refetchType ?? 'active'
const invalidates = mutation.meta?.invalidates ?? []
const invalidatesToAwait = (() => {
if (Array.isArray(shouldAwaitInvalidates)) {
Expand All @@ -113,6 +115,7 @@ export function createQueryClient<TStorageValue = string>(
for (const queryKey of invalidatesToIgnore) {
void queryClient.invalidateQueries({
predicate: query => queryCore.matchQuery({ queryKey }, query),
refetchType,
})
}

Expand All @@ -121,6 +124,7 @@ export function createQueryClient<TStorageValue = string>(
invalidatesToAwait.map(queryKey =>
queryClient.invalidateQueries({
predicate: query => queryCore.matchQuery({ queryKey }, query),
refetchType,
}),
),
)
Expand Down
22 changes: 15 additions & 7 deletions app/common/src/services/Backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,18 @@ export function assetIsType<Type extends AssetType>(type: Type) {
return (asset: AnyAsset): asset is Extract<AnyAsset, Asset<Type>> => asset.type === type
}

/** Extract the type of an id and return a discriminated union containing both id and type. */
export function extractTypeFromId(id: AssetId): AnyAsset extends infer T ?
T extends T ?
Pick<T, ('id' | 'type') & keyof T>
: never
: never {
return {
type: id.match(/^(.+?)-/)?.[1],
id,
} as never
}

/** Creates a new placeholder asset id for the given asset type. */
export function createPlaceholderAssetId<Type extends AssetType>(
type: Type,
Expand Down Expand Up @@ -1674,11 +1686,7 @@ export default abstract class Backend {
title: string,
): Promise<CreatedProject>
/** Return project details. */
abstract getProjectDetails(
projectId: ProjectId,
directoryId: DirectoryId | null,
getPresignedUrl?: boolean,
): Promise<Project>
abstract getProjectDetails(projectId: ProjectId, getPresignedUrl?: boolean): Promise<Project>
/** Return Language Server logs for a project session. */
abstract getProjectSessionLogs(
projectSessionId: ProjectSessionId,
Expand Down Expand Up @@ -1767,8 +1775,8 @@ export default abstract class Backend {
projectId?: string | null,
metadata?: object | null,
): Promise<void>
/** Download from an arbitrary URL that is assumed to originate from this backend. */
abstract download(url: string, name?: string): Promise<void>
/** Download an asset. */
abstract download(assetId: AssetId, title: string): Promise<void>

/**
* Get the URL for the customer portal.
Expand Down
4 changes: 0 additions & 4 deletions app/common/src/text/english.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
"editDescriptionError": "Could not edit description",
"canOnlyDownloadFilesError": "You currently can only download files.",
"noProjectSelectedError": "First select a project to download.",
"downloadInvalidTypeError": "You can only download files, projects, and Datalinks",
"downloadProjectError": "Could not download project '$0'",
"downloadFileError": "Could not download file '$0'",
"downloadDatalinkError": "Could not download Datalink '$0'",
Expand All @@ -64,9 +63,6 @@
"nameShouldNotContainInvalidCharacters": "Name should not contain invalid characters",
"invalidEmailValidationError": "Please enter a valid email address",

"projectHasNoSourceFilesPhrase": "project has no source files",
"fileNotFoundPhrase": "file not found",

"noNewProfilePictureError": "Could not upload a new profile picture because no image was found",

"registrationError": "Something went wrong! Please try again or contact the administrators.",
Expand Down
2 changes: 1 addition & 1 deletion app/common/src/utilities/data/array.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @file Utilities for manipulating arrays. */

export const EMPTY_ARRAY: readonly never[] = []
export const EMPTY_ARRAY: readonly [] = []

// ====================
// === shallowEqual ===
Expand Down
9 changes: 9 additions & 0 deletions app/common/src/utilities/data/object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,12 @@ export function useObjectId() {
* can be used to splice together objects without the risk of collisions.
*/
export type DisjointKeysUnion<A, B> = keyof A & keyof B extends never ? A & B : never

/**
* Merge types of values of an object union. Useful to return an object that UNSAFELY
* (at runtime) conforms to the shape of a discriminated union.
* Especially useful for things like Tanstack Query results.
*/
export type MergeValuesOfObjectUnion<T> = {
[K in `${keyof T & string}`]: T[K & keyof T]
}
7 changes: 0 additions & 7 deletions app/gui/integration-test/dashboard/delete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ test('delete and restore', ({ page }) =>
.contextMenu.restoreFromTrash()
.driveTable.expectTrashPlaceholderRow()
.goToCategory.cloud()
.expectStartModal()
.withStartModal(async (startModal) => {
await expect(startModal).toBeVisible()
})
.close()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))
Expand All @@ -50,8 +45,6 @@ test('delete and restore (keyboard)', ({ page }) =>
.press('Mod+R')
.driveTable.expectTrashPlaceholderRow()
.goToCategory.cloud()
.expectStartModal()
.close()
.driveTable.withRows(async (rows) => {
await expect(rows).toHaveCount(1)
}))
Expand Down
3 changes: 2 additions & 1 deletion app/gui/integration-test/dashboard/labelsPanel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,13 @@ test('labels', ({ page }) =>
// Labels panel with one entry
await locateCreateButton(locateNewLabelModal(page)).click()
await expect(locateLabelsPanel(page)).toBeVisible()
expect(await locateLabelsPanelLabels(page).count()).toBe(1)

// Empty labels panel again, after deleting the only entry
await locateLabelsPanelLabels(page).first().hover()

const labelsPanel = locateLabelsPanel(page)
await labelsPanel.getByRole('button').and(labelsPanel.getByLabel(TEXT.delete)).click()
await page.getByRole('button', { name: TEXT.delete }).getByText(TEXT.delete).click()
expect(await locateLabelsPanelLabels(page).count()).toBeGreaterThanOrEqual(1)
expect(await locateLabelsPanelLabels(page).count()).toBe(0)
}))
5 changes: 1 addition & 4 deletions app/gui/src/dashboard/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -560,10 +560,6 @@ function AppRouter(props: AppRouterProps) {
)
}

// ====================================
// === LocalBackendPathSynchronizer ===
// ====================================

/** Keep `localBackend.rootPath` in sync with the saved root path state. */
function LocalBackendPathSynchronizer() {
const [localRootDirectory] = localStorageProvider.useLocalStorageState('localRootDirectory')
Expand All @@ -575,5 +571,6 @@ function LocalBackendPathSynchronizer() {
localBackend.resetRootPath()
}
}

return null
}
Loading

0 comments on commit 25933af

Please sign in to comment.