From 2f34d7222e58c0835cb1b4c50e15f54526f4bf17 Mon Sep 17 00:00:00 2001 From: Val Hendrix Date: Fri, 13 Dec 2024 13:19:14 -0800 Subject: [PATCH] bug: Fixes loading large number data files in Metadata Editor Enables Batch fetch of member models and updates save button control toggling - Adds `fetchMemberModels` method to `DataPackage` to fetch member models in batches. - Updates `fetch` method in `DataPackage` to use `fetchMemberModels`. - Adds listener for `numLoadingFileMetadata` change in `EML211EditorView`. - Updates `toggleEnableControls` in `EML211EditorView` to handle `numLoadingFileMetadata`. - Adds `fetchBatchSize` configuration to `AppModel` to control batch size for fetching member models. Closes nceas/metacatui#2547 --- src/js/collections/DataPackage.js | 163 +++++++++++++++------- src/js/models/AppModel.js | 16 +++ src/js/views/metadata/EML211EditorView.js | 18 ++- 3 files changed, 144 insertions(+), 53 deletions(-) diff --git a/src/js/collections/DataPackage.js b/src/js/collections/DataPackage.js index e0911e30d..8665e5208 100644 --- a/src/js/collections/DataPackage.js +++ b/src/js/collections/DataPackage.js @@ -415,6 +415,116 @@ define([ } }, + + /** + * Fetches member models in batches to avoid fetching all members simultaneously. + * + * @param {Array} models - The array of member models to fetch. + * @param {number} [index=0] - The current index of the model being fetched. + * @param {number} [batchSize=10] - The number of models to fetch in each batch. + * @param {number} [timeout=5000] - The timeout for each fetch request in milliseconds. + * @param {number} [maxRetries=3] - The maximum number of retries for each fetch request. + */ + fetchMemberModels(models, index = 0, batchSize = 10, timeout = 5000, maxRetries = 3) { + // Update the number of file metadata items being loaded + this.packageModel.set("numLoadingFileMetadata", models.length - index); + + // If the index is greater than or equal to the length of the models array, stop fetching + if (index >= models.length) { + this.triggerComplete(); + return; + } + + // If batchSize is 0, set it to the total number of models + if (batchSize == 0) batchSize = models.length; + + const collection = this; + // Slice the models array to get the current batch + const batch = models.slice(index, index + batchSize); + + // Create an array of promises for fetching each model in the batch + const fetchPromises = batch.map((memberModel) => { + return new Promise((resolve, reject) => { + const attemptFetch = (retriesLeft) => { + // Create a promise for the fetch request + const fetchPromise = new Promise((fetchResolve, fetchReject) => { + memberModel.fetch({ + success: () => { + // Once the model is synced, handle the response + memberModel.once("sync", (oldModel) => { + const newModel = collection.getMember(oldModel); + + // If the type of the old model is different from the new model + if (oldModel.type != newModel.type) { + if (newModel.type == "DataPackage") { + // If the new model is a DataPackage, replace the old model with the new one + oldModel.trigger("replace", newModel); + fetchResolve(); + } else { + // Otherwise, fetch the new model and replace the old model with the new one + newModel.set("synced", false); + newModel.fetch(); + newModel.once("sync", (fetchedModel) => { + fetchedModel.set("synced", true); + collection.remove(oldModel); + collection.add(fetchedModel); + oldModel.trigger("replace", newModel); + if (newModel.type == "EML") collection.trigger("add:EML"); + fetchResolve(); + }); + } + } else { + // If the type of the old model is the same as the new model, merge the new model into the collection + newModel.set("synced", true); + collection.add(newModel, { merge: true }); + if (newModel.type == "EML") collection.trigger("add:EML"); + fetchResolve(); + } + }); + }, + error: (model, response) => fetchReject(new Error(response.statusText)) + }); + }); + + // Create a promise for the timeout + const timeoutPromise = new Promise((_, timeoutReject) => { + setTimeout(() => timeoutReject(new Error("Fetch timed out")), timeout); + }); + + // Race the fetch promise against the timeout promise + Promise.race([fetchPromise, timeoutPromise]) + .then(resolve) + .catch((error) => { + if (retriesLeft > 0) { + // Retry the fetch if there are retries left + console.warn(`Retrying fetch for model: ${memberModel.id}, retries left: ${retriesLeft}, error: ${error}`); + attemptFetch(retriesLeft - 1); + } else { + // Reject the promise if all retries are exhausted + console.error(`Failed to fetch model: ${memberModel.id} after ${maxRetries} retries, error: ${error}`); + reject(error); + } + }); + }; + + // Start the fetch attempt with the maximum number of retries + attemptFetch(maxRetries); + }); + }); + + // Once all fetch promises are resolved, fetch the next batch + Promise.allSettled(fetchPromises).then((results) => { + const errors = results.filter(result => result.status === "rejected"); + if (errors.length > 0) { + console.error("Error fetching member models:", errors); + } + // Fetch the next batch of models + this.fetchMemberModels.call(collection, models, index + batchSize, batchSize, timeout, maxRetries); + }).catch((error) => { + console.error("Error fetching member models:", error); + }); + }, + /** * Overload fetch calls for a DataPackage * @param {object} [options] - Optional options for this fetch that get sent with the XHR request @@ -472,6 +582,7 @@ define([ return Backbone.Collection.prototype.fetch .call(this, fetchOptions) .fail(() => + console.log("Fetch failed. Retrying with user login details..."), // If the initial fetch fails, retry with user login details retryFetch(), ); @@ -688,56 +799,9 @@ define([ // Don't fetch each member model if the fetchModels property on this Collection is set to false if (this.fetchModels !== false) { - // Add the models to the collection now, silently - // this.add(models, {silent: true}); - - // Retrieve the model for each member - _.each( - models, - function (memberModel) { - const collection = this; - - memberModel.fetch(); - memberModel.once("sync", (oldModel) => { - // Get the right model type based on the model values - const newModel = collection.getMember(oldModel); - - // If the model type has changed, then mark the model as unsynced, since there may be custom fetch() options for the new model - if (oldModel.type != newModel.type) { - // DataPackages shouldn't be fetched until we support nested packages better in the UI - if (newModel.type == "DataPackage") { - // Trigger a replace event so other parts of the app know when a model has been replaced with a different type - oldModel.trigger("replace", newModel); - } else { - newModel.set("synced", false); - - newModel.fetch(); - newModel.once("sync", (fetchedModel) => { - fetchedModel.set("synced", true); - - // Remove the model from the collection and add it back - collection.remove(oldModel); - collection.add(fetchedModel); - - // Trigger a replace event so other parts of the app know when a model has been replaced with a different type - oldModel.trigger("replace", newModel); - - if (newModel.type == "EML") - collection.trigger("add:EML"); - }); - } - } else { - newModel.set("synced", true); - collection.add(newModel, { - merge: true, - }); + // Start fetching member models + this.fetchMemberModels.call(this, models, 0, MetacatUI.appModel.get("batchModeValue")); - if (newModel.type == "EML") collection.trigger("add:EML"); - } - }); - }, - this, - ); } } catch (error) { console.log(error); @@ -3661,6 +3725,7 @@ define([ this.packageModel.updateSysMeta(); }, + /** * Tracks the upload status of DataONEObject models in this collection. If they are * `loading` into the DOM or `in progress` of an upload to the server, they will be considered as "loading" files. diff --git a/src/js/models/AppModel.js b/src/js/models/AppModel.js index d5af3c6c3..4f81fd210 100644 --- a/src/js/models/AppModel.js +++ b/src/js/models/AppModel.js @@ -2413,6 +2413,22 @@ define(["jquery", "underscore", "backbone"], function ($, _, Backbone) { * @example application%2Fbagit-097 */ packageFormat: "application%2Fbagit-1.0", + /** + * Whether to batch requests to the DataONE API. This is an experimental feature + * and should be used with caution. If set to a number greater than 0, MetacatUI will + * batch requests to the DataONE API and send them in groups of this size. This can + * improve performance when making many requests to the DataONE API, but can also + * cause issues if the requests are too large or if the DataONE API is not able to + * handle the batched requests. + * + * Currently, this feature is only used in the DataPackageModel when fetching the + * list of DataONE member models. + * + * @type {number} + * @default 0 + * @example 20 + */ + batchModeValue: 0, }, MetacatUI.AppConfig, ), diff --git a/src/js/views/metadata/EML211EditorView.js b/src/js/views/metadata/EML211EditorView.js index 30e4e66e7..95cda8c4c 100644 --- a/src/js/views/metadata/EML211EditorView.js +++ b/src/js/views/metadata/EML211EditorView.js @@ -552,6 +552,12 @@ define([ "change:numLoadingFiles", this.toggleEnableControls, ); + + this.listenTo( + MetacatUI.rootDataPackage.packageModel, + "change:numLoadingFileMetadata", + this.toggleEnableControls, + ); }, renderChildren: function (model, options) {}, @@ -1215,15 +1221,19 @@ define([ toggleEnableControls: function () { if (MetacatUI.rootDataPackage.packageModel.get("isLoadingFiles")) { let noun = - MetacatUI.rootDataPackage.packageModel.get("numLoadingFiles") > 1 - ? " files" - : " file"; + MetacatUI.rootDataPackage.packageModel.get("numLoadingFiles") > 1 + ? " files" + : " file"; this.disableControls( - "Waiting for " + + "Waiting for " + MetacatUI.rootDataPackage.packageModel.get("numLoadingFiles") + noun + " to upload...", ); + } else if (MetacatUI.rootDataPackage.packageModel.get("numLoadingFileMetadata") >0) { + this.disableControls("Waiting for " + + MetacatUI.rootDataPackage.packageModel.get("numLoadingFileMetadata") + + " file metadata to load..."); } else { this.enableControls(); }