From db7abd2f9a900944fcd06c32d4491ca53f4e3828 Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Thu, 21 Dec 2023 11:07:00 +0100 Subject: [PATCH 1/2] feat: introduce configuration for concurrent requests --- .../bugfix-configurable-request-concurrency | 6 ++++++ docs/getting-started.md | 6 ++++++ .../InviteCollaboratorForm.vue | 11 +++++++++- packages/web-app-files/src/store/actions.ts | 6 ++++-- .../tests/unit/store/actions.spec.ts | 7 +++++++ .../src/components/SideBar/FileSideBar.vue | 5 ++++- .../files/useFileActionsAcceptShare.ts | 4 +++- .../useFileActionsCreateSpaceFromResource.ts | 6 +++++- .../files/useFileActionsDeclineShare.ts | 4 +++- .../files/useFileActionsToggleHideShare.ts | 4 +++- .../helpers/useFileActionsDeleteResources.ts | 6 +++++- .../spaces/useSpaceActionsDuplicate.ts | 6 +++++- packages/web-pkg/src/configuration/manager.ts | 21 +++++++++++++++++++ packages/web-pkg/src/configuration/types.ts | 8 +++++++ .../web-runtime/src/container/bootstrap.ts | 8 +++++-- packages/web-runtime/src/index.ts | 2 +- 16 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 changelog/unreleased/bugfix-configurable-request-concurrency diff --git a/changelog/unreleased/bugfix-configurable-request-concurrency b/changelog/unreleased/bugfix-configurable-request-concurrency new file mode 100644 index 00000000000..56655620f7e --- /dev/null +++ b/changelog/unreleased/bugfix-configurable-request-concurrency @@ -0,0 +1,6 @@ +Bugfix: Configurable concurrent requests + +In order to ease the load on the backend we've introduced config options to limit the number of concurrent requests in certain areas. + +https://github.com/owncloud/web/issues/10227 +https://github.com/owncloud/web/pull/10230 diff --git a/docs/getting-started.md b/docs/getting-started.md index dd3ec35b7a2..c2ed26b8e27 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -81,6 +81,12 @@ Depending on the backend you are using, there are sample config files provided i - `options.logoutUrl` Adds a link to the user's profile page to point him to an external page, where he can manage his session and devices. This is helpful when an external IdP is used. This option is disabled by default. - `options.ocm.openRemotely` Specifies whether opening files in remote shares in their original ocm instance should be enabled. Defaults to `false`. - `options.userListRequiresFilter` Defines whether one or more filters must be set in order to list users in the Web admin settings. Set this option to 'true' if running in an environment with a lot of users and listing all users could slow down performance. Defaults to `false`. +- `options.concurrentRequests` This accepts an object with the following optional fields to customize the maximum number of concurrent requests in code paths where we limit concurrent requests + - `resourceBatchActions` Concurrent number of file/folder/space batch actions like e.g. accepting shares. Defaults to 4. + - `sse` Concurrent number of SSE event handlers. Defaults to 4. + - `shares` Accepts an object regarding the following sharing related options: + - `create` Concurrent number of share invites. Defaults to 4. + - `list` Concurrent number of individually loaded shares. Defaults to 2. #### Scripts and Styles diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue index 24226dbc10c..d44907cb513 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue @@ -152,6 +152,7 @@ import { useCapabilityFilesSharingResharingDefault, useCapabilityShareJailEnabled, useClientService, + useConfigurationManager, useStore } from '@ownclouders/web-pkg' @@ -198,6 +199,7 @@ export default defineComponent({ const saving = ref(false) const savingDelayed = ref(false) const notifyEnabled = ref(false) + const configurationManager = useConfigurationManager() watch(saving, (newValue) => { if (!newValue) { @@ -245,6 +247,10 @@ export default defineComponent({ { prefix: 'sm:', description: 'federated' } ] + const createSharesConcurrentRequests = computed(() => { + return configurationManager.options.concurrentRequests.shares.create + }) + return { resource: inject('resource'), hasResharing: useCapabilityFilesSharingResharing(store), @@ -260,6 +266,7 @@ export default defineComponent({ contextMenuButtonRef, notifyEnabled, federatedUsers, + createSharesConcurrentRequests, // CERN accountType, @@ -476,7 +483,9 @@ export default defineComponent({ this.saving = true const errors = [] - const saveQueue = new PQueue({ concurrency: 4 }) + const saveQueue = new PQueue({ + concurrency: this.createSharesConcurrentRequests + }) const savePromises = [] this.selectedCollaborators.forEach((collaborator) => { savePromises.push( diff --git a/packages/web-app-files/src/store/actions.ts b/packages/web-app-files/src/store/actions.ts index cdb9973d0e1..9d1308f0d8d 100644 --- a/packages/web-app-files/src/store/actions.ts +++ b/packages/web-app-files/src/store/actions.ts @@ -349,7 +349,7 @@ export default { context.commit('LOAD_INDICATORS', path) } }, - async loadShares(context, { client, path, storageId, useCached = true }) { + async loadShares(context, { client, configurationManager, path, storageId, useCached = true }) { if (context.state.sharesLoading) { await context.state.sharesLoading } @@ -366,7 +366,9 @@ export default { const currentlyLoadedShares = [...context.state.outgoingShares, ...context.state.incomingShares] const shares = [] - const shareQueriesQueue = new PQueue({ concurrency: 2 }) + const shareQueriesQueue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.shares.list + }) const shareQueriesPromises = [] const { highlightedFile } = context.getters diff --git a/packages/web-app-files/tests/unit/store/actions.spec.ts b/packages/web-app-files/tests/unit/store/actions.spec.ts index dda1b73b3f5..cac428f76da 100644 --- a/packages/web-app-files/tests/unit/store/actions.spec.ts +++ b/packages/web-app-files/tests/unit/store/actions.spec.ts @@ -8,6 +8,7 @@ import { } from '@ownclouders/web-client/src/helpers' import { mock, mockDeep } from 'jest-mock-extended' import { OwnCloudSdk } from '@ownclouders/web-client/src/types' +import { ConfigurationManager } from '@ownclouders/web-pkg' jest.mock('@ownclouders/web-client/src/helpers/share/functions', () => { return { @@ -151,10 +152,13 @@ describe('vuex store actions', () => { clientMock.shares.getShares.mockResolvedValueOnce([{ shareInfo: { id: 3 } }]) clientMock.shares.getShares.mockResolvedValueOnce([{ shareInfo: { id: 2 } }]) clientMock.shares.getShares.mockResolvedValueOnce([{ shareInfo: { id: 4 } }]) + const configurationManagerMock = mock() + configurationManagerMock.options.concurrentRequests = { shares: { list: 4 } } await actions.loadShares( { state: stateMock, getters: gettersMock, commit: commitMock, rootState: rootStateMock }, { client: clientMock, + configurationManager: configurationManagerMock, path: '/someFolder/someFile.txt', storageId: '1', useCached: false @@ -175,10 +179,13 @@ describe('vuex store actions', () => { const loadedShare = { id: 1, outgoing: true, indirect: true, path: '/someFile.txt' } stateMock.outgoingShares = [loadedShare] const clientMock = mockDeep() + const configurationManagerMock = mock() + configurationManagerMock.options.concurrentRequests = { shares: { list: 4 } } await actions.loadShares( { state: stateMock, getters: gettersMock, commit: commitMock, rootState: rootStateMock }, { client: clientMock, + configurationManager: configurationManagerMock, path: '/someFile.txt', storageId: '1', useCached: true diff --git a/packages/web-pkg/src/components/SideBar/FileSideBar.vue b/packages/web-pkg/src/components/SideBar/FileSideBar.vue index 647db83310a..d855d58f02d 100644 --- a/packages/web-pkg/src/components/SideBar/FileSideBar.vue +++ b/packages/web-pkg/src/components/SideBar/FileSideBar.vue @@ -49,7 +49,8 @@ import { useRouter, useActiveLocation, useExtensionRegistry, - useSelectedResources + useSelectedResources, + useConfigurationManager } from '../../composables' import { isProjectSpaceResource, @@ -83,6 +84,7 @@ export default defineComponent({ const clientService = useClientService() const extensionRegistry = useExtensionRegistry() const eventBus = useEventBus() + const configurationManager = useConfigurationManager() const loadedResource = ref() const isLoading = ref(false) @@ -191,6 +193,7 @@ export default defineComponent({ if (!unref(isPublicFilesLocation) && !unref(isTrashLocation)) { store.dispatch('Files/loadShares', { client: clientService.owncloudSdk, + configurationManager, path: resource.path, storageId: resource.fileId, // cache must not be used on flat file lists that gather resources from various locations diff --git a/packages/web-pkg/src/composables/actions/files/useFileActionsAcceptShare.ts b/packages/web-pkg/src/composables/actions/files/useFileActionsAcceptShare.ts index 197f5e055c0..ead7dcf68de 100644 --- a/packages/web-pkg/src/composables/actions/files/useFileActionsAcceptShare.ts +++ b/packages/web-pkg/src/composables/actions/files/useFileActionsAcceptShare.ts @@ -28,7 +28,9 @@ export const useFileActionsAcceptShare = ({ store }: { store?: Store } = {} const handler = async ({ resources }: FileActionOptions) => { const errors = [] const triggerPromises = [] - const triggerQueue = new PQueue({ concurrency: 4 }) + const triggerQueue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.resourceBatchActions + }) resources.forEach((resource) => { triggerPromises.push( triggerQueue.add(async () => { diff --git a/packages/web-pkg/src/composables/actions/files/useFileActionsCreateSpaceFromResource.ts b/packages/web-pkg/src/composables/actions/files/useFileActionsCreateSpaceFromResource.ts index cc57c638474..e711613d470 100644 --- a/packages/web-pkg/src/composables/actions/files/useFileActionsCreateSpaceFromResource.ts +++ b/packages/web-pkg/src/composables/actions/files/useFileActionsCreateSpaceFromResource.ts @@ -12,6 +12,7 @@ import { isLocationSpacesActive } from '../../../router' import { useCreateSpace } from '../../spaces' import { useSpaceHelpers } from '../../spaces' import PQueue from 'p-queue' +import { useConfigurationManager } from '../../configuration' export const useFileActionsCreateSpaceFromResource = ({ store }: { store?: Store } = {}) => { const { can } = useAbility() @@ -22,11 +23,14 @@ export const useFileActionsCreateSpaceFromResource = ({ store }: { store?: Store const clientService = useClientService() const router = useRouter() const hasCreatePermission = computed(() => can('create-all', 'Drive')) + const configurationManager = useConfigurationManager() const confirmAction = async ({ spaceName, resources, space }) => { const { webdav } = clientService store.dispatch('hideModal') - const queue = new PQueue({ concurrency: 4 }) + const queue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.resourceBatchActions + }) const copyOps = [] try { diff --git a/packages/web-pkg/src/composables/actions/files/useFileActionsDeclineShare.ts b/packages/web-pkg/src/composables/actions/files/useFileActionsDeclineShare.ts index d7da492c7d7..f82e397a97a 100644 --- a/packages/web-pkg/src/composables/actions/files/useFileActionsDeclineShare.ts +++ b/packages/web-pkg/src/composables/actions/files/useFileActionsDeclineShare.ts @@ -31,7 +31,9 @@ export const useFileActionsDeclineShare = ({ store }: { store?: Store } = { const handler = async ({ resources }: FileActionOptions) => { const errors = [] const triggerPromises = [] - const triggerQueue = new PQueue({ concurrency: 4 }) + const triggerQueue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.resourceBatchActions + }) resources.forEach((resource) => { triggerPromises.push( triggerQueue.add(async () => { diff --git a/packages/web-pkg/src/composables/actions/files/useFileActionsToggleHideShare.ts b/packages/web-pkg/src/composables/actions/files/useFileActionsToggleHideShare.ts index 8abb0d0d463..cb82adee704 100644 --- a/packages/web-pkg/src/composables/actions/files/useFileActionsToggleHideShare.ts +++ b/packages/web-pkg/src/composables/actions/files/useFileActionsToggleHideShare.ts @@ -27,7 +27,9 @@ export const useFileActionsToggleHideShare = ({ store }: { store?: Store } const handler = async ({ resources }: FileActionOptions) => { const errors = [] const triggerPromises = [] - const triggerQueue = new PQueue({ concurrency: 4 }) + const triggerQueue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.resourceBatchActions + }) const hidden = !resources[0].hidden resources.forEach((resource) => { diff --git a/packages/web-pkg/src/composables/actions/helpers/useFileActionsDeleteResources.ts b/packages/web-pkg/src/composables/actions/helpers/useFileActionsDeleteResources.ts index e08bb8a4b63..88382380b17 100644 --- a/packages/web-pkg/src/composables/actions/helpers/useFileActionsDeleteResources.ts +++ b/packages/web-pkg/src/composables/actions/helpers/useFileActionsDeleteResources.ts @@ -17,6 +17,7 @@ import { useRouter } from '../../router' import { useStore } from '../../store' import { useGettext } from 'vue3-gettext' import { ref } from 'vue' +import { useConfigurationManager } from '../../configuration' export const useFileActionsDeleteResources = ({ store }: { store?: Store }) => { store = store || useStore() @@ -28,8 +29,11 @@ export const useFileActionsDeleteResources = ({ store }: { store?: Store }) const clientService = useClientService() const loadingService = useLoadingService() const { owncloudSdk } = clientService + const configurationManager = useConfigurationManager() - const queue = new PQueue({ concurrency: 4 }) + const queue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.resourceBatchActions + }) const deleteOps = [] const resourcesToDelete = ref([]) diff --git a/packages/web-pkg/src/composables/actions/spaces/useSpaceActionsDuplicate.ts b/packages/web-pkg/src/composables/actions/spaces/useSpaceActionsDuplicate.ts index 1798d20ec79..eda3fce41dd 100644 --- a/packages/web-pkg/src/composables/actions/spaces/useSpaceActionsDuplicate.ts +++ b/packages/web-pkg/src/composables/actions/spaces/useSpaceActionsDuplicate.ts @@ -13,6 +13,7 @@ import { resolveFileNameDuplicate } from '../../../helpers' import PQueue from 'p-queue' import { useRouter } from '../../router' import { isLocationSpacesActive } from '../../../router' +import { useConfigurationManager } from '../../configuration' export const useSpaceActionsDuplicate = ({ store @@ -25,6 +26,7 @@ export const useSpaceActionsDuplicate = ({ const ability = useAbility() const clientService = useClientService() const loadingService = useLoadingService() + const configurationManager = useConfigurationManager() const isProjectsLocation = isLocationSpacesActive(router, 'files-spaces-projects') @@ -48,7 +50,9 @@ export const useSpaceActionsDuplicate = ({ const existingSpaceFiles = await clientService.webdav.listFiles(existingSpace) if (existingSpaceFiles.children.length) { - const queue = new PQueue({ concurrency: 4 }) + const queue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.resourceBatchActions + }) const copyOps = [] for (const file of existingSpaceFiles.children) { diff --git a/packages/web-pkg/src/configuration/manager.ts b/packages/web-pkg/src/configuration/manager.ts index 4a571232686..0730ac25e8e 100644 --- a/packages/web-pkg/src/configuration/manager.ts +++ b/packages/web-pkg/src/configuration/manager.ts @@ -143,6 +143,27 @@ export class ConfigurationManager { 'embed.delegateAuthenticationOrigin', get(options, 'embed.delegateAuthenticationOrigin', null) ) + + set( + this.optionsConfiguration, + 'concurrentRequests.resourceBatchActions', + get(options, 'concurrentRequests.resourceBatchActions', 4) + ) + set( + this.optionsConfiguration, + 'concurrentRequests.sse', + get(options, 'concurrentRequests.sse', 4) + ) + set( + this.optionsConfiguration, + 'concurrentRequests.shares.create', + get(options, 'concurrentRequests.shares.create', 4) + ) + set( + this.optionsConfiguration, + 'concurrentRequests.shares.list', + get(options, 'concurrentRequests.shares.list', 2) + ) } get options(): OptionsConfiguration { diff --git a/packages/web-pkg/src/configuration/types.ts b/packages/web-pkg/src/configuration/types.ts index 5e038b2a0c3..18eb16d1c18 100644 --- a/packages/web-pkg/src/configuration/types.ts +++ b/packages/web-pkg/src/configuration/types.ts @@ -43,6 +43,14 @@ export interface OptionsConfiguration { delegateAuthentication?: boolean delegateAuthenticationOrigin?: string | null } + concurrentRequests?: { + resourceBatchActions?: number + sse?: number + shares?: { + create?: number + list?: number + } + } } export interface OAuth2Configuration { diff --git a/packages/web-runtime/src/container/bootstrap.ts b/packages/web-runtime/src/container/bootstrap.ts index 2d87fd13b3c..bb2b1a21811 100644 --- a/packages/web-runtime/src/container/bootstrap.ts +++ b/packages/web-runtime/src/container/bootstrap.ts @@ -663,12 +663,16 @@ const onSSEProcessingFinishedEvent = ({ export const registerSSEEventListeners = ({ store, - clientService + clientService, + configurationManager }: { store: Store clientService: ClientService + configurationManager: ConfigurationManager }): void => { - const resourceQueue = new PQueue({ concurrency: 4 }) + const resourceQueue = new PQueue({ + concurrency: configurationManager.options.concurrentRequests.sse + }) store.watch( (state, getters) => getters['Files/currentFolder'], () => { diff --git a/packages/web-runtime/src/index.ts b/packages/web-runtime/src/index.ts index 7bfa4099683..44752a1f5ff 100644 --- a/packages/web-runtime/src/index.ts +++ b/packages/web-runtime/src/index.ts @@ -172,7 +172,7 @@ export const bootstrapApp = async (configurationPath: string): Promise => // Register SSE event listeners if (store.getters.capabilities?.core?.['support-sse']) { - registerSSEEventListeners({ store, clientService }) + registerSSEEventListeners({ store, clientService, configurationManager }) } // Load spaces to make them available across the application From 4c8ebe407601ceaee3cfe8a3c279fc92b44282fc Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Thu, 21 Dec 2023 13:14:14 +0100 Subject: [PATCH 2/2] test: fix unit tests --- .../components/AppBar/CreateAndUpload.spec.ts | 17 +++++- .../CreateAndUpload.spec.ts.snap | 58 ++++++++++++++++++- .../tests/unit/components/Search/List.spec.ts | 3 +- .../InviteCollaboratorForm.spec.ts | 12 ++++ .../tests/unit/views/Favorites.spec.ts | 4 ++ .../unit/views/shares/SharedViaLink.spec.ts | 4 ++ .../views/shares/SharedWithOthers.spec.ts | 3 +- .../unit/views/spaces/GenericSpace.spec.ts | 4 +- .../tests/unit/views/spaces/Projects.spec.ts | 8 ++- .../unit/components/AppBar/AppBar.spec.ts | 6 +- .../components/Search/ResourcePreview.spec.ts | 7 +++ .../files/useFileActionsCreateNewFile.spec.ts | 5 ++ .../useFileActionsDeleteResources.spec.ts | 12 ++++ .../Topbar/ApplicationsMenu.spec.ts | 5 ++ 14 files changed, 137 insertions(+), 11 deletions(-) diff --git a/packages/web-app-files/tests/unit/components/AppBar/CreateAndUpload.spec.ts b/packages/web-app-files/tests/unit/components/AppBar/CreateAndUpload.spec.ts index 8f963019369..18f8a0286d2 100644 --- a/packages/web-app-files/tests/unit/components/AppBar/CreateAndUpload.spec.ts +++ b/packages/web-app-files/tests/unit/components/AppBar/CreateAndUpload.spec.ts @@ -2,7 +2,7 @@ import CreateAndUpload from 'web-app-files/src/components/AppBar/CreateAndUpload import { mock } from 'jest-mock-extended' import { Resource, SpaceResource } from '@ownclouders/web-client/src/helpers' import { Drive } from '@ownclouders/web-client/src/generated' -import { useRequest } from '@ownclouders/web-pkg' +import { FileAction, useFileActionsCreateNewFile, useRequest } from '@ownclouders/web-pkg' import { eventBus, UppyResource } from '@ownclouders/web-pkg' import { createStore, @@ -14,12 +14,15 @@ import { import { RouteLocation } from 'vue-router' import { useExtensionRegistry } from '@ownclouders/web-pkg' import { useExtensionRegistryMock } from 'web-test-helpers/src/mocks/useExtensionRegistryMock' +import { ref } from 'vue' jest.mock('@ownclouders/web-pkg', () => ({ ...jest.requireActual('@ownclouders/web-pkg'), useAccessToken: jest.fn(), useExtensionRegistry: jest.fn(), - useRequest: jest.fn() + useRequest: jest.fn(), + useFileActionsCreateNewFile: jest.fn(), + useFileActions: jest.fn() })) const elSelector = { @@ -190,6 +193,16 @@ function getWrapper({ })) jest.mocked(useExtensionRegistry).mockImplementation(() => useExtensionRegistryMock()) + jest.mocked(useFileActionsCreateNewFile).mockReturnValue( + mock>({ + actions: ref([ + mock({ label: () => 'Plain text file', ext: 'txt' }), + mock({ label: () => 'Mark-down file', ext: 'md' }), + mock({ label: () => 'Draw.io document', ext: 'drawio' }) + ]) + }) + ) + const storeOptions = { ...defaultStoreMockOptions, getters: { diff --git a/packages/web-app-files/tests/unit/components/AppBar/__snapshots__/CreateAndUpload.spec.ts.snap b/packages/web-app-files/tests/unit/components/AppBar/__snapshots__/CreateAndUpload.spec.ts.snap index 4f601b22987..167ad8070bf 100644 --- a/packages/web-app-files/tests/unit/components/AppBar/__snapshots__/CreateAndUpload.spec.ts.snap +++ b/packages/web-app-files/tests/unit/components/AppBar/__snapshots__/CreateAndUpload.spec.ts.snap @@ -53,7 +53,34 @@ exports[`CreateAndUpload component file handlers should show additional handlers Folder - +
  • + +
    • +
  • + +
    • +
  • + +
    • +
    • - +
  • + +
    • +
  • + +
    • +
  • + +
    • +