Bug #15840
closed
[Workbench2] Project page large file count hangs
Added by Eric Biagiotti over 4 years ago. Updated over 4 years ago.
Description
The project panel loads the entire collection file tree to calculate file count and total size. Uses the file_count and file_size_total collection attributes instead.
Updated by Eric Biagiotti over 4 years ago
Latest @ c6314ca47008f1ffd0fe70fe025a57475b64e773 in the wb2 repo.
Now uses the file_count and file_size_total collection attributes. Also updated the collection service to not do any tree manipulation when using propfind for files.
Updated by Eric Biagiotti over 4 years ago
- Target version changed from 2019-11-20 Sprint to 2019-12-04 Sprint
Updated by Peter Amstutz over 4 years ago
- Target version deleted (
2019-12-04 Sprint)
Functionally this seems to work fine but it took me some time to understand all the changes, this would be a good opportunity for some knowledge sharing.
collection-panel-files-actions.ts:28
I had a hard time understanding the purpose of each step in this method. Please add some comments.
export const loadCollectionFiles = (uuid: string) =>
async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => {
const files = await services.collectionService.files(uuid);
const tree = createCollectionFilesTree(files);
const sorted = sortFilesTree(tree);
const mapped = mapTreeValues(services.collectionService.extendFileURL)(sorted);
dispatch(collectionPanelFilesAction.SET_COLLECTION_FILES(mapped));
};
Looking at the diff, where the old code had dispatch(resourcesDataActions.SET_FILES({ uuid, files })); but now SET_FILES (actually all of ~/store/resources-data) is gone. I think maybe the purpose was to cache the results from WebDAV, does that mean it is reloading the file listing in cases it wouldn't have done it before? (That's probably fine if it only does it when you click on the collection page but still worth noting).
I think these are the two changes that actually matter, everything else is just cleanup?
- api.dispatch<any>(updateResourceData(resourceUuids));
- const resource = getResourceData(props.uuid)(state.resourcesData);
- return { fileSize: resource ? resource.fileSize : 0 };
+ const resource = getResource<CollectionResource>(props.uuid)(state.resources);
+ return { fileSize: resource ? resource.fileSizeTotal : 0 };
Otherwise LGTM!
Updated by Eric Biagiotti over 4 years ago
- Target version set to 2019-12-04 Sprint
Updated by Eric Biagiotti over 4 years ago
Peter Amstutz wrote:
Functionally this seems to work fine but it took me some time to understand all the changes, this would be a good opportunity for some knowledge sharing.
This should have been separated into a few commits. The refactoring included here was mainly to remove the tree parsing/construction from the files function in collection-service.ts, which should really only parse and return the webdav results. Not critical for this branch though, and I can remove if you think that would be better.
collection-panel-files-actions.ts:28
I had a hard time understanding the purpose of each step in this method. Please add some comments.
[...]
This was a result of the refactor mentioned above, but I added a comment to try and explain it a bit more.
Looking at the diff, where the old code had
dispatch(resourcesDataActions.SET_FILES({ uuid, files }));but now SET_FILES (actually all of~/store/resources-data) is gone. I think maybe the purpose was to cache the results from WebDAV, does that mean it is reloading the file listing in cases it wouldn't have done it before? (That's probably fine if it only does it when you click on the collection page but still worth noting).
store/resource-data code (dispatch(resourcesDataActions.SET_FILES({ uuid, files }))) was only used to reduce the collection file list to file count and total file size when the project-panel data explorer was being populated (ProjectPanelMiddlewareService.requestItems). It doesn't do any caching and since we now get those two attributes from the collection object, I was able to remove it all.
Latest @ ef8f1518f7d4537b63a5b0201ea10413a2712c0d
- Fixes webdav propfind parsing for directories and adds a comment
Updated by Peter Amstutz over 4 years ago
Eric Biagiotti wrote:
Peter Amstutz wrote:
Functionally this seems to work fine but it took me some time to understand all the changes, this would be a good opportunity for some knowledge sharing.
This should have been separated into a few commits. The refactoring included here was mainly to remove the tree parsing/construction from the
filesfunction in collection-service.ts, which should really only parse and return the webdav results. Not critical for this branch though, and I can remove if you think that would be better.collection-panel-files-actions.ts:28
I had a hard time understanding the purpose of each step in this method. Please add some comments.
[...]This was a result of the refactor mentioned above, but I added a comment to try and explain it a bit more.
Looking at the diff, where the old code had
dispatch(resourcesDataActions.SET_FILES({ uuid, files }));but now SET_FILES (actually all of~/store/resources-data) is gone. I think maybe the purpose was to cache the results from WebDAV, does that mean it is reloading the file listing in cases it wouldn't have done it before? (That's probably fine if it only does it when you click on the collection page but still worth noting).store/resource-data code (
dispatch(resourcesDataActions.SET_FILES({ uuid, files }))) was only used to reduce the collection file list to file count and total file size when the project-panel data explorer was being populated (ProjectPanelMiddlewareService.requestItems). It doesn't do any caching and since we now get those two attributes from the collection object, I was able to remove it all.Latest @ ef8f1518f7d4537b63a5b0201ea10413a2712c0d
- Fixes webdav propfind parsing for directories and adds a comment
This LGTM.
Updated by Eric Biagiotti over 4 years ago
- Status changed from In Progress to Resolved