From 77859719f2f2f98295b9dc5f30ef0b73d0819c80 Mon Sep 17 00:00:00 2001 From: Matthew Care Date: Mon, 14 Nov 2022 07:49:18 +0100 Subject: [PATCH] Batch media upload (#12947) * Allow simultaneous uploads Upload files in batches for faster uploading * Fix concurrency issue Many requests to add files coming at the same time can cause duplicate folders to be created. In some cases, SQL exceptions can throw, and cause the application to hang. Add Semaphore to process files on a single thread. * Replace Semaphore with a Scope Provider As suggested in the PR comments, replaced Semaphore with ICoreScopeProvider * Revert "Replace Semaphore with a Scope Provider" This reverts commit 228e1010c61428c81f5162cb5e0dd2538357d816. * Add comment to make configurable Out of scope of the PR, but this should be configurable in appsettings * Apply PR 13345 Apply PR 13345 to resolve "freezing" issue caused by unhandled errors * Account for currently processing items If you drag files into the dropzone whilst other files are uploading then the "total" count is incorrect. --- .../Controllers/MediaController.cs | 9 + .../upload/umbfiledropzone.directive.js | 472 ++++++++++-------- .../components/upload/umb-file-dropzone.html | 153 +++--- 3 files changed, 336 insertions(+), 298 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs index b81d120295..26b776ddde 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs @@ -49,6 +49,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers; [ParameterSwapControllerActionSelector(nameof(GetChildren), "id", typeof(int), typeof(Guid), typeof(Udi))] public class MediaController : ContentControllerBase { + private static readonly Semaphore _postAddFileSemaphore = new(1, 1); private readonly AppCaches _appCaches; private readonly IAuthorizationService _authorizationService; private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; @@ -574,6 +575,7 @@ public class MediaController : ContentControllerBase public async Task PostAddFile([FromForm] string path, [FromForm] string currentFolder, [FromForm] string contentTypeAlias, List file) { + await _postAddFileSemaphore.WaitOneAsync(); var root = _hostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.TempFileUploads); //ensure it exists Directory.CreateDirectory(root); @@ -581,6 +583,7 @@ public class MediaController : ContentControllerBase //must have a file if (file.Count == 0) { + _postAddFileSemaphore.Release(); return NotFound(); } @@ -588,12 +591,14 @@ public class MediaController : ContentControllerBase ActionResult? parentIdResult = await GetParentIdAsIntAsync(currentFolder, true); if (!(parentIdResult?.Result is null)) { + _postAddFileSemaphore.Release(); return parentIdResult.Result; } var parentId = parentIdResult?.Value; if (!parentId.HasValue) { + _postAddFileSemaphore.Release(); return NotFound("The passed id doesn't exist"); } @@ -605,6 +610,7 @@ public class MediaController : ContentControllerBase if (!IsFolderCreationAllowedHere(parentId.Value)) { AddCancelMessage(tempFiles, _localizedTextService.Localize("speechBubbles", "folderUploadNotAllowed")); + _postAddFileSemaphore.Release(); return Ok(tempFiles); } @@ -638,6 +644,7 @@ public class MediaController : ContentControllerBase //if the media root is null, something went wrong, we'll abort if (mediaRoot == null) { + _postAddFileSemaphore.Release(); return Problem( "The folder: " + folderName + " could not be used for storing images, its ID: " + parentId + " returned null"); @@ -808,10 +815,12 @@ public class MediaController : ContentControllerBase KeyValuePair origin = HttpContext.Request.Query.First(x => x.Key == "origin"); if (origin.Value == "blueimp") { + _postAddFileSemaphore.Release(); return new JsonResult(tempFiles); //Don't output the angular xsrf stuff, blue imp doesn't like that } } + _postAddFileSemaphore.Release(); return Ok(tempFiles); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js index 98f02b7e06..1841548426 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfiledropzone.directive.js @@ -19,239 +19,267 @@ TODO */ angular.module("umbraco.directives") - .directive('umbFileDropzone', - function ($timeout, Upload, localizationService, umbRequestHelper, overlayService, mediaHelper, mediaTypeHelper) { - return { - restrict: 'E', - replace: true, - templateUrl: 'views/components/upload/umb-file-dropzone.html', - scope: { - parentId: '@', - contentTypeAlias: '@', - propertyAlias: '@', - accept: '@', - maxFileSize: '@', - - compact: '@', - hideDropzone: '@', - acceptedMediatypes: '=', + .directive('umbFileDropzone', + function ($timeout, Upload, localizationService, umbRequestHelper, overlayService, mediaHelper, mediaTypeHelper) { + return { + restrict: 'E', + replace: true, + templateUrl: 'views/components/upload/umb-file-dropzone.html', + scope: { + parentId: '@', + contentTypeAlias: '@', + propertyAlias: '@', + accept: '@', + maxFileSize: '@', - filesQueued: '=', - handleFile: '=', - filesUploaded: '=' - }, - link: function(scope, element, attrs) { - scope.queue = []; - scope.totalQueued = 0; - scope.currentFile = undefined; - scope.processed = []; - scope.totalMessages = 0; + compact: '@', + hideDropzone: '@', + acceptedMediatypes: '=', - function _filterFile(file) { - var ignoreFileNames = ['Thumbs.db']; - var ignoreFileTypes = ['directory']; + filesQueued: '=', + handleFile: '=', + filesUploaded: '=' + }, + link: function (scope, element, attrs) { + scope.queue = []; + scope.totalQueued = 0; + scope.processing = []; + scope.processed = []; + scope.totalMessages = 0; + // TODO - Make configurable in appsettings + scope.batchSize = 10; + scope.processingCount = 0; - // ignore files with names from the list - // ignore files with types from the list - // ignore files which starts with "." - if (ignoreFileNames.indexOf(file.name) === -1 && - ignoreFileTypes.indexOf(file.type) === -1 && - file.name.indexOf(".") !== 0) { - return true; - } else { - return false; - } - } + function _filterFile(file) { + var ignoreFileNames = ['Thumbs.db']; + var ignoreFileTypes = ['directory']; - function _filesQueued(files, event) { - //Push into the queue - Utilities.forEach(files, file => { - if (_filterFile(file) === true) { - file.messages = []; - scope.queue.push(file); - } - }); + // ignore files with names from the list + // ignore files with types from the list + // ignore files which starts with "." + if (ignoreFileNames.indexOf(file.name) === -1 && + ignoreFileTypes.indexOf(file.type) === -1 && + file.name.indexOf(".") !== 0) { + return true; + } else { + return false; + } + } - // Upload not allowed - if (!scope.acceptedMediatypes || !scope.acceptedMediatypes.length) { - files.map(file => { - file.messages.push({message: "File type is not allowed here", type: "Error"}); - }); - } + function _filesQueued(files, event) { + //Push into the queue + Utilities.forEach(files, file => { + if (_filterFile(file) === true) { + file.messages = []; + scope.queue.push(file); + } + }); - // If we have Accepted Media Types, we will ask to choose Media Type, if Choose Media Type returns false, it only had one choice and therefor no reason to - if (scope.acceptedMediatypes && _requestChooseMediaTypeDialog() === false) { - scope.contentTypeAlias = "umbracoAutoSelect"; - } + // Upload not allowed + if (!scope.acceptedMediatypes || !scope.acceptedMediatypes.length) { + files.map(file => { + file.messages.push({ message: "File type is not allowed here", type: "Error" }); + }); + } - // Add the processed length, as we might be uploading in stages - scope.totalQueued = scope.queue.length + scope.processed.length; + // If we have Accepted Media Types, we will ask to choose Media Type, if + // Choose Media Type returns false, it only had one choice and therefor no reason to + if (scope.acceptedMediatypes && _requestChooseMediaTypeDialog() === false) { + scope.contentTypeAlias = "umbracoAutoSelect"; + } - _processQueueItems(); - } + // Add all of the processing and processed files to account for uploading + // files in stages (dragging files X at a time into the dropzone). + scope.totalQueued = scope.queue.length + scope.processingCount + scope.processed.length; - function _processQueueItems() { - // if we have processed all files, either by successful - // upload, or attending to all messages, we deem the - // action complete, else continue processing files - scope.totalMessages = scope.processed.filter(e => e.messages.length > 0).length; - if (scope.totalQueued === scope.processed.length) { - if (scope.totalMessages === 0) { - if (scope.filesUploaded) { - //queue is empty, trigger the done action - scope.filesUploaded(scope.done); - } - //auto-clear the done queue after 3 secs - var currentLength = scope.processed.length; - $timeout(function() { - scope.processed.splice(0, currentLength); - }, 3000); - } - } else { - scope.currentFile = scope.queue.shift(); - _upload(scope.currentFile); - } - } + _processQueueItems(); + } - function _upload(file) { + function _processQueueItems() { - scope.propertyAlias = scope.propertyAlias ? scope.propertyAlias : "umbracoFile"; - scope.contentTypeAlias = scope.contentTypeAlias ? scope.contentTypeAlias : "Image"; + if (scope.processingCount === scope.batchSize) { + return; + } - Upload.upload({ - url: umbRequestHelper.getApiUrl("mediaApiBaseUrl", "PostAddFile"), - fields: { - 'currentFolder': scope.parentId, - 'contentTypeAlias': scope.contentTypeAlias, - 'propertyAlias': scope.propertyAlias, - 'path': file.path - }, - file: file - }) - .progress(function(evt) { - if (file.uploadStat !== "done" && file.uploadStat !== "error") { - // calculate progress in percentage - var progressPercentage = parseInt(100.0 * evt.loaded / evt.total, 10); - // set percentage property on file - file.uploadProgress = progressPercentage; - } - }) - .success(function (data, status, headers, config) { - // Set server messages - file.messages = data.notifications; - scope.processed.push(file); - //after processing, test if everything is done - scope.currentFile = undefined; - _processQueueItems(); - }) - .error(function(evt, status, headers, config) { - //if the service returns a detailed error - if (evt.InnerException) { - file.messages.push({ message: evt.InnerException.ExceptionMessage, type: "Error" }); - //Check if its the common "too large file" exception - if (evt.InnerException.StackTrace && - evt.InnerException.StackTrace.indexOf("ValidateRequestEntityLength") > 0) { - file.messages.push({ message: "File too large to upload", type: "Error" }); - } - } else if (evt.Message) { - file.messages.push({message: evt.Message, type: "Error"}); - } else if (evt && typeof evt === "string") { - file.messages.push({message: evt, type: "Error"}); - } - // If file not found, server will return a 404 and display this message - if (status === 404) { - file.messages.push({message: "File not found", type: "Error"}); - } - scope.currentFile = undefined; - _processQueueItems(); - }); - } + // if we have processed all files, either by successful + // upload, or attending to all messages, we deem the + // action complete, else continue processing files + scope.totalMessages = scope.processed.filter(e => e.messages.length > 0).length; - function _requestChooseMediaTypeDialog() { - - if (scope.queue.length === 0) { - // if queue has no items so there is nothing to choose a type for - return false; - } - - if (scope.acceptedMediatypes.length === 1) { - // if only one accepted type, then we wont ask to choose. - return false; - } - - var uploadFileExtensions = scope.queue.map(file => mediaHelper.getFileExtension(file.name)); - - var filteredMediaTypes = mediaTypeHelper.getTypeAcceptingFileExtensions(scope.acceptedMediatypes, uploadFileExtensions); - - var mediaTypesNotFile = filteredMediaTypes.filter(mediaType => mediaType.alias !== "File"); - - if (mediaTypesNotFile.length <= 1) { - // if only one or less accepted types when we have filtered type 'file' out, then we wont ask to choose. - return false; - } - - - localizationService.localizeMany(["defaultdialogs_selectMediaType", "mediaType_autoPickMediaType"]).then(function (translations) { - - filteredMediaTypes.push({ - alias: "umbracoAutoSelect", - name: translations[1], - icon: "icon-wand" - }); - - const dialog = { - view: "itempicker", - filter: filteredMediaTypes.length > 8, - availableItems: filteredMediaTypes, - submit: function (model) { - scope.contentTypeAlias = model.selectedItem.alias; - _processQueueItems(); - - overlayService.close(); - }, - close: function () { - - scope.queue.map(function (file) { - file.messages.push({message:"No files uploaded, no mediatype selected", type: "Error"}); - }); - scope.queue = []; - - overlayService.close(); - } - }; - - dialog.title = translations[0]; - overlayService.open(dialog); - }); - - return true; // yes, we did open the choose-media dialog, therefore we return true. - } - - scope.dismissMessages = function (file) { - file.messages = []; - _processQueueItems(); - } - - scope.dismissAllMessages = function () { - Utilities.forEach(scope.processed, file => { - file.messages = []; - }); - _processQueueItems(); - } - - scope.handleFiles = function(files, event, invalidFiles) { - const allFiles = [...files, ...invalidFiles]; - - // add unique key for each files to use in ng-repeats - Utilities.forEach(allFiles, file => { - file.key = String.CreateGuid(); - }); - - if (scope.filesQueued) { - scope.filesQueued(allFiles, event); - } - _filesQueued(allFiles, event); - }; + if (scope.totalQueued === scope.processed.length) { + if (scope.totalMessages === 0) { + if (scope.filesUploaded) { + //queue is empty, trigger the done action + scope.filesUploaded(scope.done); } - }; - }); + + //auto-clear the done queue after 3 secs + var currentLength = scope.processed.length; + $timeout(function () { + scope.processed.splice(0, currentLength); + }, 3000); + } + } else if (scope.queue.length) { + + var file = scope.queue.shift(); + scope.processing.push(file); + _upload(file); + + // If we still have items to process + // do so right away for parallel uploads + if (scope.queue.length > 0) { + _processQueueItems(); + } + } + } + + function _upload(file) { + + scope.propertyAlias = scope.propertyAlias ? scope.propertyAlias : "umbracoFile"; + scope.contentTypeAlias = scope.contentTypeAlias ? scope.contentTypeAlias : "Image"; + + scope.processingCount++; + + Upload.upload({ + url: umbRequestHelper.getApiUrl("mediaApiBaseUrl", "PostAddFile"), + fields: { + 'currentFolder': scope.parentId, + 'contentTypeAlias': scope.contentTypeAlias, + 'propertyAlias': scope.propertyAlias, + 'path': file.path + }, + file: file + }) + .progress(function (evt) { + if (file.uploadStat !== "done" && file.uploadStat !== "error") { + // calculate progress in percentage + var progressPercentage = parseInt(100.0 * evt.loaded / evt.total, 10); + // set percentage property on file + file.uploadProgress = progressPercentage; + } + }) + .success(function (data, status, headers, config) { + // Set server messages + file.messages = data.notifications; + file.done = true; + scope.processed.push(file); + scope.processingCount--; + _processQueueItems(); + }) + .error(function (evt, status, headers, config) { + //if the service returns a detailed error + if (evt.InnerException) { + file.messages.push({ message: evt.InnerException.ExceptionMessage, type: "Error" }); + //Check if its the common "too large file" exception + if (evt.InnerException.StackTrace && + evt.InnerException.StackTrace.indexOf("ValidateRequestEntityLength") > 0) { + file.messages.push({ message: "File too large to upload", type: "Error", header: "Error" }); + } + } else if (status === 413) { + file.messages.push({ message: "File too large to upload", type: "Error", header: "Error" }); + } else if (evt.Message) { + file.messages.push({ message: evt.Message, type: "Error", header: "Error" }); + } else if (evt && typeof evt === "string") { + file.messages.push({ message: evt, type: "Error", header: "Error" }); + } else if (status === 404) { + // If file not found, server will return a 404 and display this message + file.messages.push({ message: "File not found", type: "Error" }); + } else if (status !== 200) { + file.messages.push({ message: "An unknown error occurred", type: "Error", header: "Error" }); + } + + file.done = true; + scope.processed.push(file); + scope.processingCount--; + _processQueueItems(); + }); + } + + function _requestChooseMediaTypeDialog() { + + if (scope.queue.length === 0) { + // if queue has no items so there is nothing to choose a type for + return false; + } + + if (scope.acceptedMediatypes.length === 1) { + // if only one accepted type, then we wont ask to choose. + return false; + } + + var uploadFileExtensions = scope.queue.map(file => mediaHelper.getFileExtension(file.name)); + + var filteredMediaTypes = mediaTypeHelper.getTypeAcceptingFileExtensions(scope.acceptedMediatypes, uploadFileExtensions); + + var mediaTypesNotFile = filteredMediaTypes.filter(mediaType => mediaType.alias !== "File"); + + if (mediaTypesNotFile.length <= 1) { + // if only one or less accepted types when we have filtered type 'file' out, then we wont ask to choose. + return false; + } + + + localizationService.localizeMany(["defaultdialogs_selectMediaType", "mediaType_autoPickMediaType"]).then(function (translations) { + + filteredMediaTypes.push({ + alias: "umbracoAutoSelect", + name: translations[1], + icon: "icon-wand" + }); + + const dialog = { + view: "itempicker", + filter: filteredMediaTypes.length > 8, + availableItems: filteredMediaTypes, + submit: function (model) { + scope.contentTypeAlias = model.selectedItem.alias; + _processQueueItems(); + + overlayService.close(); + }, + close: function () { + + scope.queue.map(function (file) { + file.messages.push({ message: "No files uploaded, no mediatype selected", type: "Error" }); + }); + scope.queue = []; + + overlayService.close(); + } + }; + + dialog.title = translations[0]; + overlayService.open(dialog); + }); + + return true; // yes, we did open the choose-media dialog, therefore we return true. + } + + scope.dismissMessages = function (file) { + file.messages = []; + _processQueueItems(); + } + + scope.dismissAllMessages = function () { + Utilities.forEach(scope.processed, file => { + file.messages = []; + }); + _processQueueItems(); + } + + scope.handleFiles = function (files, event, invalidFiles) { + const allFiles = [...files, ...invalidFiles]; + + // add unique key for each files to use in ng-repeats + Utilities.forEach(allFiles, file => { + file.key = String.CreateGuid(); + }); + + if (scope.filesQueued) { + scope.filesQueued(allFiles, event); + } + _filesQueued(allFiles, event); + }; + } + }; + }); diff --git a/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html b/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html index 6581fca14d..c90f3139a8 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-file-dropzone.html @@ -1,91 +1,92 @@
- - -
+ + +
-
-

- Drag and drop your file(s) into the area -

+
+

+ Drag and drop your file(s) into the area +

- - + + - - -
+ + +
+
+ + +
    + + +
  • +
    + +
    +
    • - -
      +
    • +
      +
      + {{ file.name }} + + {{message.header}}: {{message.message}} + + "{{maxFileSize}}" + +
      -
    • -
      - - -
      -
      • + + + + -
    • + + +
+ -
-
- {{ file.name }} - - {{message.header}}: {{message.message}} - - "{{maxFileSize}}" - -
+ +
  • +
    {{file.name}} {{file.uploadProgress + '%'}}
    +
    + +
    +
    • - - - - +
  • +
    {{ file.name }}
    +
    • - - -
    - - -
  • -
    {{currentFile.name}} {{currentFile.uploadProgress + '%'}}
    -
    - -
    -
    • - -
  • -
    {{ file.name }}
    -
    • - - -     + +