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

WIP resource management in store #5892

Closed
wants to merge 1 commit into from
Closed

Conversation

kulmann
Copy link
Contributor

@kulmann kulmann commented Oct 10, 2021

Description

problem statement:

our files array in the vuex store of the files app is only capable of representing one state.
multiple PROPFINDs for different folders, fired at similar times, will override each other in that state.
the longest running propfind wins. Cancelling the requests is not solving all edge cases, as the
PROPFINDs might finish at similar times so that we're in postprocessing of the data and request cancellation is not possible anymore (because request already done).

idea for a solution:

loading resources in the different views should be converted into a two step process, each:

  1. set the activeView and activePath immediately after each navigation (SET_ACTIVE_VIEW, SET_ACTIVE_PATH)
  2. do the PROPFIND, convert the response into resources, and set them in a dedicated folder store module.
    That store module needs to get registered dynamically. The PROPFIND and conversion might be long running.
    It's really important that the activePath is set as early as possible!

the views then render the files of the store module that matches the current activePath.
as a bonus, requests could be cancelled and data loading method invocations stopped, so that we avoid unnecessary store module creation / data transformation / loading of additional data (e.g. if we know that the response of a PROPFIND is not needed anymore, because the user navigated away from the current location, we could skip loading indicators and previews entirely).

reasons for dynamically registered store modules: we need the reactivity of the individual files.
Which comes built in with the store modules. Another approach would be to build a cache of recent
propfind results (= response converted into a file listing). but that's not reactive by default.

Related Issue

Motivation and Context

Reduce glitchiness of the different resource listing views and their respective navigation.

How Has This Been Tested?

  • not yet (WIP)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • think about when to unregister the folder store modules. they might be big so removal should be happening as soon as not needed anymore.
  • further think about different views: propfind, fetch incoming shares, fetch outgoing shares, trashbin
  • the different loadResources methods from the views should live in a service, which then also takes care of creating the respective store module.
  • changelog
  • tests

@update-docs
Copy link

update-docs bot commented Oct 10, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kulmann kulmann marked this pull request as draft October 10, 2021 11:57
@kulmann kulmann changed the title resource management in store WIP resource management in store Oct 10, 2021
@kulmann kulmann force-pushed the resource-management-in-store branch 6 times, most recently from 892d47f to c70e2ee Compare October 13, 2021 08:36
@kulmann kulmann force-pushed the resource-management-in-store branch from c70e2ee to f965b5b Compare October 20, 2021 15:31
<list-loader v-if="loadResourcesTask.isRunning" />
=======
<list-loader v-if="isLoading" />
>>>>>>> c70e2ee84 (WIP resource management in store)
Copy link
Member

Choose a reason for hiding this comment

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

:trollface:

@kulmann
Copy link
Contributor Author

kulmann commented Jan 21, 2022

Uff, yes, I guess this can be closed by now. The resource loading has improved a lot in the meantime. Adapting the remaining parts of this PR doesn't bring too much benefit at the moment.

@kulmann kulmann closed this Jan 21, 2022
@kulmann kulmann deleted the resource-management-in-store branch January 21, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Responses are rendered in different places
2 participants