Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pagination support to Files Source plugins #18059

Merged
merged 34 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e2d8164
Add pagination params to remote file sources
davelopez Apr 23, 2024
bd38198
Add supports_pagination flag to file sources
davelopez Apr 23, 2024
0151f9e
Implement pagination in Invenio fsplugin
davelopez Apr 23, 2024
ad67556
Add pagination support for all PyFilesystem2 sources
davelopez Apr 24, 2024
508b3e3
Add `temp` file source plugin for easy testing PyFilesystem2
davelopez Apr 25, 2024
080a724
Add unit tests for TempFilesSource
davelopez Apr 25, 2024
9fd3443
Fix recursive walking in PyFilesystem2FilesSource
davelopez Apr 25, 2024
451f8c7
Update client API schema
davelopez Apr 26, 2024
6aaae24
Use default behavior when no pagination params are set
davelopez Apr 26, 2024
8174633
Expose offset and limit pagination params in browseRemoteFiles
davelopez Apr 26, 2024
d0632a5
Refactor unit tests for TempFilesSource
davelopez Apr 26, 2024
a50d2d3
Refactor limit and offset query params documentation
davelopez May 3, 2024
af37cbe
Add `supports_search` flag and `query` parameter to file sources
davelopez May 3, 2024
8228d83
Implement search query in PyFilesystem plugins
davelopez May 3, 2024
ff83166
Add unit tests for search query
davelopez May 3, 2024
d42ada3
Implement search query in Invenio-compatible plugins
davelopez May 3, 2024
73b2ff7
Add query parameter to fetcher
davelopez May 13, 2024
867333f
Add `supports_sorting` flag
davelopez May 13, 2024
eadab2d
Add sortBy parameter to fetcher
davelopez May 13, 2024
e04fa52
Refactor query parameter annotations in remote files API
davelopez May 13, 2024
789f0ea
Add missing parameters to abstract method
davelopez May 13, 2024
8b71688
Refactor FilesSourceSupports in FilesSourceProperties
davelopez May 13, 2024
dd60362
Add total_matches header to file sources listing
davelopez May 16, 2024
069e4b2
Provide total items from FilesDialog to SelectionDialog
davelopez May 16, 2024
6354b49
Implement items provider for server-side pagination
davelopez May 16, 2024
22b0d14
Adapt unit tests with new response header
davelopez May 16, 2024
c53e1cf
Adapt unit tests to use pinia
davelopez May 17, 2024
13f8c47
Attempt to fix error when counting directories in Google Cloud Storag…
davelopez May 17, 2024
c7a2640
Attempt 2 to fix error when counting directories in Google Cloud Stor…
davelopez May 17, 2024
ae76501
Do not use bare `except`
davelopez May 17, 2024
ef30cca
Add debouncing time for data dialog search
davelopez May 17, 2024
03d9d12
Apply suggestions from code review
davelopez May 21, 2024
40fbc94
Add unit tests for invalid parameters
davelopez May 21, 2024
2b2d9b9
Raise error when invalid pagination parameters
davelopez May 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 29 additions & 3 deletions client/src/api/remoteFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,42 @@ export async function fetchFileSources(options: FilterFileSourcesOptions = {}):

export const remoteFilesFetcher = fetcher.path("/api/remote_files").method("get").create();

export interface BrowseRemoteFilesResult {
entries: RemoteEntry[];
totalMatches: number;
}

/**
* Get the list of files and directories from the server for the given file source URI.
* @param uri The file source URI to browse.
* @param isRecursive Whether to recursively retrieve all files inside subdirectories.
* @param writeable Whether to return only entries that can be written to.
* @param limit The maximum number of entries to return.
* @param offset The number of entries to skip before returning the rest.
* @param query The query string to filter the entries.
* @param sortBy The field to sort the entries by.
* @returns The list of files and directories from the server for the given URI.
*/
export async function browseRemoteFiles(uri: string, isRecursive = false, writeable = false): Promise<RemoteEntry[]> {
const { data } = await remoteFilesFetcher({ target: uri, recursive: isRecursive, writeable });
return data as RemoteEntry[];
export async function browseRemoteFiles(
uri: string,
isRecursive = false,
writeable = false,
limit?: number,
offset?: number,
query?: string,
sortBy?: string
): Promise<BrowseRemoteFilesResult> {
const { data, headers } = await remoteFilesFetcher({
target: uri,
recursive: isRecursive,
writeable,
limit,
offset,
query,
sort_by: sortBy,
});
const totalMatches = parseInt(headers.get("total_matches") ?? "0");
return { entries: data as RemoteEntry[], totalMatches };
}

const createEntry = fetcher.path("/api/remote_files").method("post").create();
Expand Down
63 changes: 63 additions & 0 deletions client/src/api/schema/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,8 @@ export interface paths {
* Displays remote files available to the user. Please use /api/remote_files instead.
* @deprecated
* @description Lists all remote files available to the user from different sources.
*
* The total count of files and directories is returned in the 'total_matches' header.
*/
get: operations["index_api_ftp_files_get"];
};
Expand Down Expand Up @@ -1453,6 +1455,8 @@ export interface paths {
/**
* Displays remote files available to the user.
* @description Lists all remote files available to the user from different sources.
*
* The total count of files and directories is returned in the 'total_matches' header.
*/
get: operations["index_api_remote_files_get"];
/**
Expand Down Expand Up @@ -2701,6 +2705,15 @@ export interface components {
* @description Only users with the roles specified here can access this files source.
*/
requires_roles?: string | null;
/**
* @description Features supported by this file source.
* @default {
* "pagination": false,
* "search": false,
* "sorting": false
* }
*/
supports?: components["schemas"]["FilesSourceSupports"];
/**
* Type
* @description The type of the plugin.
Expand Down Expand Up @@ -5335,6 +5348,15 @@ export interface components {
* @description Only users with the roles specified here can access this files source.
*/
requires_roles?: string | null;
/**
* @description Features supported by this file source.
* @default {
* "pagination": false,
* "search": false,
* "sorting": false
* }
*/
supports?: components["schemas"]["FilesSourceSupports"];
/**
* Type
* @description The type of the plugin.
Expand All @@ -5359,6 +5381,27 @@ export interface components {
| components["schemas"]["BrowsableFilesSourcePlugin"]
| components["schemas"]["FilesSourcePlugin"]
)[];
/** FilesSourceSupports */
FilesSourceSupports: {
/**
* Pagination
* @description Whether this file source supports server-side pagination.
* @default false
*/
pagination?: boolean;
/**
* Search
* @description Whether this file source supports server-side search.
* @default false
*/
search?: boolean;
/**
* Sorting
* @description Whether this file source supports server-side sorting.
* @default false
*/
sorting?: boolean;
};
/** FillStepDefaultsAction */
FillStepDefaultsAction: {
/**
Expand Down Expand Up @@ -15557,19 +15600,29 @@ export interface operations {
* Displays remote files available to the user. Please use /api/remote_files instead.
* @deprecated
* @description Lists all remote files available to the user from different sources.
*
* The total count of files and directories is returned in the 'total_matches' header.
*/
parameters?: {
/** @description The source to load datasets from. Possible values: ftpdir, userdir, importdir */
/** @description The requested format of returned data. Either `flat` to simply list all the files, `jstree` to get a tree representation of the files, or the default `uri` to list files and directories by their URI. */
/** @description Whether to recursively lists all sub-directories. This will be `True` by default depending on the `target`. */
/** @description (This only applies when `format` is `jstree`) The value can be either `folders` or `files` and it will disable the corresponding nodes of the tree. */
/** @description Whether the query is made with the intention of writing to the source. If set to True, only entries that can be written to will be returned. */
/** @description Maximum number of entries to return. */
/** @description Number of entries to skip. */
/** @description Search query to filter entries by. The syntax could be different depending on the target source. */
/** @description Sort the entries by the specified field. */
query?: {
target?: string;
format?: components["schemas"]["RemoteFilesFormat"] | null;
recursive?: boolean | null;
disable?: components["schemas"]["RemoteFilesDisableMode"] | null;
writeable?: boolean | null;
limit?: number | null;
offset?: number | null;
query?: string | null;
sort_by?: string | null;
};
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
header?: {
Expand Down Expand Up @@ -21873,19 +21926,29 @@ export interface operations {
/**
* Displays remote files available to the user.
* @description Lists all remote files available to the user from different sources.
*
* The total count of files and directories is returned in the 'total_matches' header.
*/
parameters?: {
/** @description The source to load datasets from. Possible values: ftpdir, userdir, importdir */
/** @description The requested format of returned data. Either `flat` to simply list all the files, `jstree` to get a tree representation of the files, or the default `uri` to list files and directories by their URI. */
/** @description Whether to recursively lists all sub-directories. This will be `True` by default depending on the `target`. */
/** @description (This only applies when `format` is `jstree`) The value can be either `folders` or `files` and it will disable the corresponding nodes of the tree. */
/** @description Whether the query is made with the intention of writing to the source. If set to True, only entries that can be written to will be returned. */
/** @description Maximum number of entries to return. */
/** @description Number of entries to skip. */
/** @description Search query to filter entries by. The syntax could be different depending on the target source. */
/** @description Sort the entries by the specified field. */
query?: {
target?: string;
format?: components["schemas"]["RemoteFilesFormat"] | null;
recursive?: boolean | null;
disable?: components["schemas"]["RemoteFilesDisableMode"] | null;
writeable?: boolean | null;
limit?: number | null;
offset?: number | null;
query?: string | null;
sort_by?: string | null;
};
/** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */
header?: {
Expand Down
6 changes: 5 additions & 1 deletion client/src/components/FilesDialog/FilesDialog.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { createTestingPinia } from "@pinia/testing";
import { mount, Wrapper } from "@vue/test-utils";
import flushPromises from "flush-promises";
import { getLocalVue } from "tests/jest/helpers";
Expand Down Expand Up @@ -88,7 +89,8 @@ function getMockedBrowseResponse(param: RemoteFilesParams) {
throw Error(someErrorText);
}
const result = mockedOkApiRoutesMap.get(responseKey);
return { data: result };
const headers = new Map([["total_matches", result?.length.toString() ?? "0"]]);
return { data: result, headers };
}

const initComponent = async (props: { multiple: boolean; mode?: string }) => {
Expand All @@ -97,9 +99,11 @@ const initComponent = async (props: { multiple: boolean; mode?: string }) => {
mockFetcher.path("/api/remote_files/plugins").method("get").mock({ data: rootResponse });
mockFetcher.path("/api/remote_files").method("get").mock(getMockedBrowseResponse);

const testingPinia = createTestingPinia({ stubActions: false });
const wrapper = mount(FilesDialog, {
localVue,
propsData: { ...props, modalStatic: true },
pinia: testingPinia,
});

await flushPromises();
Expand Down
70 changes: 66 additions & 4 deletions client/src/components/FilesDialog/FilesDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,22 @@ import {
} from "@/api/remoteFiles";
import { UrlTracker } from "@/components/DataDialog/utilities";
import { fileSourcePluginToItem, isSubPath } from "@/components/FilesDialog/utilities";
import { SELECTION_STATES, type SelectionItem } from "@/components/SelectionDialog/selectionTypes";
import {
ItemsProvider,
ItemsProviderContext,
SELECTION_STATES,
type SelectionItem,
} from "@/components/SelectionDialog/selectionTypes";
import { useConfig } from "@/composables/config";
import { useFileSources } from "@/composables/fileSources";
import { errorMessageAsString } from "@/utils/simple-error";

import { Model } from "./model";

import SelectionDialog from "@/components/SelectionDialog/SelectionDialog.vue";

const filesSources = useFileSources();

interface FilesDialogProps {
/** Callback function to be called passing the results when selection is complete */
callback?: (files: any) => void;
Expand Down Expand Up @@ -55,6 +63,7 @@ const selectedDirectories = ref<SelectionItem[]>([]);
const errorMessage = ref<string>();
const filter = ref();
const items = ref<SelectionItem[]>([]);
const itemsProvider = ref<ItemsProvider>();
const modalShow = ref(true);
const optionsShow = ref(false);
const undoShow = ref(false);
Expand All @@ -66,6 +75,7 @@ const currentDirectory = ref<SelectionItem>();
const showFTPHelper = ref(false);
const selectAllIcon = ref(SELECTION_STATES.UNSELECTED);
const urlTracker = ref(new UrlTracker(""));
const totalItems = ref(0);

const fields = computed(() => {
const fields = [];
Expand Down Expand Up @@ -156,7 +166,7 @@ function selectDirectoryRecursive(record: SelectionItem) {
const recursive = true;
isBusy.value = true;
browseRemoteFiles(record.url, recursive).then((incoming) => {
incoming.forEach((item) => {
incoming.entries.forEach((item) => {
// construct record
const subRecord = entryToRecord(item);
if (subRecord.isLeaf) {
Expand All @@ -167,6 +177,7 @@ function selectDirectoryRecursive(record: SelectionItem) {
selectedDirectories.value.push(subRecord);
}
});
totalItems.value = incoming.totalMatches;
isBusy.value = false;
});
}
Expand Down Expand Up @@ -230,6 +241,7 @@ function load(record?: SelectionItem) {
optionsShow.value = false;
undoShow.value = !urlTracker.value.atRoot();
if (urlTracker.value.atRoot() || errorMessage.value) {
itemsProvider.value = undefined;
errorMessage.value = undefined;
fetchFileSources(props.filterOptions)
.then((results) => {
Expand All @@ -241,6 +253,7 @@ function load(record?: SelectionItem) {
optionsShow.value = true;
showTime.value = false;
showDetails.value = true;
totalItems.value = convertedItems.length;
})
.catch((error) => {
errorMessage.value = errorMessageAsString(error);
Expand All @@ -257,9 +270,15 @@ function load(record?: SelectionItem) {
showDetails.value = false;
return;
}

if (shouldUseItemsProvider()) {
itemsProvider.value = provideItems;
}

browseRemoteFiles(currentDirectory.value?.url, false, props.requireWritable)
.then((results) => {
items.value = filterByMode(results).map(entryToRecord);
.then((result) => {
items.value = filterByMode(result.entries).map(entryToRecord);
totalItems.value = result.totalMatches;
formatRows();
optionsShow.value = true;
showTime.value = true;
Expand All @@ -271,6 +290,47 @@ function load(record?: SelectionItem) {
}
}

/**
* Check if the current file source supports server-side pagination.
* If it does, we will use the items provider to fetch items.
*/
function shouldUseItemsProvider(): boolean {
const fileSource = filesSources.getFileSourceById(currentDirectory.value?.id!);
const supportsPagination = fileSource?.supports?.pagination;
return Boolean(supportsPagination);
}

/**
* Fetches items from the server using server-side pagination and filtering.
**/
async function provideItems(ctx: ItemsProviderContext): Promise<SelectionItem[]> {
isBusy.value = true;
try {
if (!currentDirectory.value) {
return [];
}
const limit = ctx.perPage;
const offset = (ctx.currentPage - 1) * ctx.perPage;
const query = ctx.filter;
const response = await browseRemoteFiles(
currentDirectory.value?.url,
false,
props.requireWritable,
limit,
offset,
query
);
const result = response.entries.map(entryToRecord);
totalItems.value = response.totalMatches;
return result;
} catch (error) {
errorMessage.value = errorMessageAsString(error);
return [];
} finally {
isBusy.value = false;
}
}

function filterByMode(results: RemoteEntry[]): RemoteEntry[] {
if (!fileMode.value) {
// In directory mode, only show directories
Expand Down Expand Up @@ -346,6 +406,8 @@ onMounted(() => {
:fields="fields"
:is-busy="isBusy"
:items="items"
:items-provider="itemsProvider"
:total-items="totalItems"
:modal-show="modalShow"
:modal-static="modalStatic"
:multiple="multiple"
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/SelectionDialog/DataDialogSearch.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const placeholder = computed(() => `search ${props.title.toLowerCase()}`);

<template>
<BInputGroup class="w-100">
<BFormInput v-model="filter" :placeholder="placeholder" />
<BFormInput v-model="filter" :placeholder="placeholder" debounce="500" />
<BInputGroupAppend>
<BButton :disabled="!filter" @click="filter = ''"><FontAwesomeIcon icon="times" /></BButton>
</BInputGroupAppend>
Expand Down