bug: Fixes handing large number data files in Metadata Editor

[vchendrix]

Summary

This pull request addresses two issue tickets related to handling large numbers of files when editing a dataset. One issue involves users uploading tens to hundreds of files at once, which inundates Metacat and leads to unrecoverable upload failures, rendering the dataset unsavable. The other issue pertains to loading the metadata editor when a dataset contains a large number of files. The former fix batches the upload and provides a configuration for batchSizeUpload. The latter batches the loading of the system metadata and prevents the user from saving until all data file metadata is loaded, with the batch size configurable via batchSizeFetch. Additionally, this pull request implements batch file upload functionality using promises and fixes issues related to loading a large number of data files in the Metadata Editor.

This fix is a high priority for ESS-DIVE and Wildland Fire Science Intitiative as it significantly improves user experience and it will prevent data corruption while handling a large number of files in datasets

Features and Fixes

Batch File Upload with Promises

• Added batchSizeUpload Configuration in AppModel.js: Introduced a new configuration to control the batch size for file uploads.
• Implemented uploadFilesInBatch Method in DataItemView.js: Added a method to handle batch file uploads using promises, ensuring that the _.each loop completes before proceeding to batch processing.

Fixes for Loading Large Number of Data Files

• Added fetchMemberModels Method to DataPackage: Introduced a method to fetch member models in batches.
• Updated fetch Method in DataPackage: Modified the fetch method to use the new fetchMemberModels method.
• Added Listener for numLoadingFileMetadata Change in EML211EditorView: Added a listener to handle changes in the number of loading file metadata.
• Updated toggleEnableControls in EML211EditorView: Enhanced the method to handle numLoadingFileMetadata.
• Added fetchBatchSize Configuration to AppModel: Introduced a configuration to control the batch size for fetching member models.

Testing

This fix has been tested on MacOS using the latest versions of Chrome, Firefox, and Safari.

Issues Closed

• Closes Limit the number of files a user can add at once #2224
• Closes Unauthorized Error when Loading Large Datasets in Metadata Editor #2547

[vchendrix]

@robyngit @rushirajnenuji let me know if you have any questions or concerns. This is a critical fix for us and I am happy to work with you to get it in.

[robyngit]

Hey Val, thanks for the PR!!

Great idea to upload/fetch in batches. I tested with large data packages, nested packages, packages with atLocation file hierarchies and found no issues, everything worked.

I expected to see some performance differences, but found no detectable difference between this PR (with batch sizes set to 20 or more) and the develop branch. I tested uploading 1000+ files at once, each file being ~8MB. The time taken to upload was the same for both branches, and I didn't run into any issues with files stalling at 100% or uploads failing on either branch. Watching the upload & network tab on the develop branch, it appears as though the browser is doing some work behind the scenes to upload files in batches already... So I'm wondering if you were able to consistently reproduce these issues on your end, and whether the changes in this PR truly fixed them?

Is it worth digging deeper into the root cause of the issue the user with 1000 files was experiencing, or are you confident that it was because of concurrent uploads? (vs. perhaps network instability)

If we move forward with this PR, I have just a few notes:

1. I've merged some updates that include linting and formatting fixes to the DataPackage collection and the EML211EditorView, which have created conflicts with your branch. Please merge the latest develop branch into your PR branch to resolve these conflicts.
2. Even though the codebase isn’t fully compliant yet with the new style guide, the goal is slowly to bring it in line by ensuring new contributions adhere to it. We use ESLint and prettier, see the Contribution Guide for instructions on how to run them. This shouldn't take long and I'm happy to help if you need it.
3. We're working toward increasing our test coverage, and so I'm trying to enforce that all new methods have unit tests. The tests might be easier to write if the uploadFilesInBatch and fetchMemberModels methods were broken down into smaller, more modular functions. If you feel that this will take too long, and given the urgency of this fix, I can merge your PR and create an issue to track the test writing.
4. fetchMemberModels method is really model/collection logic and should be moved out of the view.

Given that this fix is potentially critical for ESS-DIVE, and notes # 3 and # 4 may take some time to address, we could merge this PR as is and then create an issue to track the test writing and refactoring. What do you think?

Happy to discuss further! Thanks again for the PR!

PS: I'm currently refactoring the DataPackage collection & related models to address the submission errors, so hopefully identifying and fixing issues like this one will be more straightforward in the future.

[robyngit]

Suggested change

	*/
	* @since 0.0.0
	*/

[robyngit]

Hard to catch on video, but this shows files uploading on develop branch. It appeared that files began uploading from the bottom of the list, and once a batch of them were complete, the next batch began uploading. Is this different from what you've been seeing @vchendrix ?

upload.on.develop.branch.mp4

[vchendrix]

Hard to catch on video, but this shows files uploading on develop branch. It appeared that files began uploading from the bottom of the list, and once a batch of them were complete, the next batch began uploading. Is this different from what you've been seeing @vchendrix ?

upload.on.develop.branch.mp4

Yes, that is what I am seeing.

[vchendrix]

Thanks @robyngit. I will make the changes and resubmit.

[vchendrix]

@robyngit I am still digesting the rest of your comments and may have questions about that in a little bit. First, I wanted to respond to this:

I expected to see some performance differences, but found no detectable difference between this PR (with batch sizes set to 20 or more) and the develop branch. I tested uploading 1000+ files at once, each file being ~8MB. The time taken to upload was the same for both branches, and I didn't run into any issues with files stalling at 100% or uploads failing on either branch.
That is great to hear!

Watching the upload & network tab on the develop branch, it appears as though the browser is doing some work behind the scenes to upload files in batches already... So I'm wondering if you were able to consistently reproduce these issues on your end, and whether the changes in this PR truly fixed them?

For me that is not the case, all fetches are submitted concurrently and this can cause errors 500 errors on metacat due to there being too many concurrent requests. I have serveral different cloud instances of metacat running and on google gloud I can see the container RAM going to the max during these large fetches.

Is it worth digging deeper into the root cause of the issue the user with 1000 files was experiencing, or are you confident that it was because of concurrent uploads? (vs. perhaps network instability)

I think that it is a confluence of issues but it is hard to pinpont. We have many users who do not have the greatest internest connection and they are consistently having problems.

[vchendrix]

@robyngit I tried rebasing with the develop branch but there seem to be bugs in this branch and I can't seem to satisfactorily test the code.

On save in the Metadata Editor, I get the following error. This is blocking me.

Uncaught TypeError: errors?.forEach is not a function
at n.showValidation (EML211EditorView.js?v=2.31.0:1218:17)
at _ (backbone.js:371:57)
at m (backbone.js:356:19)
at f (backbone.js:155:16)
at u.trigger (backbone.js:346:5)
at n._validate (backbone.js:728:12)
at n.isValid (backbone.js:718:19)
at DataPackage.js?v=2.31.0:1342:19
at m.every.m.all (underscore.js:245:12)
at n.save (DataPackage.js?v=2.31.0:1341:30)

[vchendrix]

@robyngit I did my best to add tests and fix the lint errors for the js files you requested but it seems there are still issues with the checks passing. Something about a Github token not having write permission. I am going to sign off until the new year. I hope this suffices.

My added tests all pass locally. NOTE that the develop branch has a regression issue with saving on the Metadata Editor page which has no relation to this PR.

[vchendrix]

@robyngit I did my best to add tests and fix the lint errors for the js files you requested but it seems there are still issues with the checks passing. Something about a Github token not having write permission. I am going to sign off until the new year. I hope this suffices.

My added tests all pass locally. NOTE that the develop branch has a regression issue with saving on the Metadata Editor page which has no relation to this PR.

@robyngit Happy New Year! I am circling back on this to see if there is anything you need from me to get this merged.

[vchendrix]

@robyngit I have updated this PR so that all the tests pass. Please note that I had to make a change to the test server configuration for puppeteer to fix an issue in which the tests were hanging on gh actions.

Please let me know if there are any further issues you would like me to address in order to get this merged. Thanks!