Fixed #3450

craftcms · Nov 27, 2018 · dfec67f · dfec67f
1 parent c7533a4
commit dfec67f
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 105 deletions.
diff --git a/CHANGELOG-v3.md b/CHANGELOG-v3.md
@@ -20,6 +20,7 @@
 - Fixed a bug where `craft\services\Sections::getAllSections()` could return stale results if a new section had been added recently. ([#3484](https://github.com/craftcms/cms/issues/3484))
 - Fixed a bug where “View entry” and “View category” element actions weren’t available when viewing a specific section or category group.
 - Fix a bug where Craft would attempt to index transforms.
+- Fix a bug where asset indexing was subject to race condition which could lead to inconsistent results. ([#3450](https://github.com/craftcms/cms/issues/3450))
 
 ### Security
 - Updated jQuery File Upload to 9.28.0.

diff --git a/src/controllers/UtilitiesController.php b/src/controllers/UtilitiesController.php
@@ -167,9 +167,14 @@ public function actionAssetIndexPerformAction(): Response
 
         // Initial request
         if (!empty($params['start'])) {
-            $batches = [];
+
             $sessionId = Craft::$app->getAssetIndexer()->getIndexingSessionId();
 
+            $response = [
+                'volumes' => [],
+                'sessionId' => $sessionId
+            ];
+
             // Selection of volumes or all volumes?
             if (is_array($params['volumes'])) {
                 $volumeIds = $params['volumes'];
@@ -197,7 +202,10 @@ public function actionAssetIndexPerformAction(): Response
                     $skippedFiles = $indexList['skippedFiles'];
                 }
 
-                $batch = [];
+                $response['volumes'][] = [
+                    'volumeId' =>$volumeId,
+                    'total' => $indexList['total'],
+                ];
 
                 for ($i = 0; $i < $indexList['total']; $i++) {
                     $batch[] = [
@@ -214,22 +222,12 @@ public function actionAssetIndexPerformAction(): Response
                 $batches[] = $batch;
             }
 
-            $batches[] = [
-                [
-                    'params' => [
-                        'overview' => true,
-                        'sessionId' => $sessionId,
-                    ]
-                ]
-            ];
-
             Craft::$app->getSession()->set('assetsVolumesBeingIndexed', $volumeIds);
             Craft::$app->getSession()->set('assetsMissingFolders', $missingFolders);
             Craft::$app->getSession()->set('assetsSkippedFiles', $skippedFiles);
 
             return $this->asJson([
-                'batches' => $batches,
-                'total' => $grandTotal
+                'indexingData' => $response,
             ]);
         }
 
@@ -250,13 +248,9 @@ public function actionAssetIndexPerformAction(): Response
             $responseArray = [];
 
             if (!empty($missingFiles) || !empty($missingFolders) || !empty($skippedFiles)) {
-                $responseArray['confirm'] = $this->getView()->renderTemplate('assets/_missing_items',
-                    [
-                        'missingFiles' => $missingFiles,
-                        'missingFolders' => $missingFolders,
-                        'skippedFiles' => $skippedFiles
-                    ]);
-                $responseArray['params'] = ['finish' => 1];
+                return $this->asJson([
+                    'confirm' => $this->getView()->renderTemplate('assets/_missing_items', compact('missingFiles', 'missingFolders', 'skippedFiles')),
+                ]);
             }
 
             // Clean up stale indexing data (all sessions that have all recordIds set)
@@ -278,16 +272,6 @@ public function actionAssetIndexPerformAction(): Response
                         ['not', ['sessionId' => $sessionsInProgress]])
                     ->execute();
             }
-
-            if (!empty($responseArray)) {
-                return $this->asJson([
-                    'batches' => [
-                        [
-                            $responseArray
-                        ]
-                    ]
-                ]);
-            }
         } else if (!empty($params['finish'])) {
             if (!empty($params['deleteAsset']) && is_array($params['deleteAsset'])) {
                 Craft::$app->getDb()->createCommand()
@@ -309,13 +293,12 @@ public function actionAssetIndexPerformAction(): Response
             if (!empty($params['deleteFolder']) && is_array($params['deleteFolder'])) {
                 Craft::$app->getAssets()->deleteFoldersByIds($params['deleteFolder'], false);
             }
-
-            return $this->asJson([
-                'finished' => 1
-            ]);
         }
 
-        return $this->asJson([]);
+        return $this->asJson([
+            'finished' => 1
+        ]);
+
     }
 
     /**

diff --git a/src/web/assets/assetindexes/dist/AssetIndexesUtility.js b/src/web/assets/assetindexes/dist/AssetIndexesUtility.js
@@ -10,6 +10,9 @@
             loadingActions: null,
             queue: null,
 
+            cacheImages: false,
+            sessionId: null,
+
             init: function(formId) {
                 this.$form = $('#' + formId);
                 this.$trigger = $('input.submit', this.$form);
@@ -25,18 +28,15 @@
                 if (!this.$trigger.hasClass('disabled')) {
                     if (!this.progressBar) {
                         this.progressBar = new Craft.ProgressBar(this.$status, true);
-                    }
-                    else {
+                    } else {
                         this.progressBar.resetProgressBar();
                     }
 
-                    this.totalActions = 1;
+                    this.totalActions = 0;
+                    this.loadingActions = 0;
                     this.completedActions = 0;
                     this.queue = [];
 
-                    this.loadingActions = 0;
-                    this.currentBatchQueue = [];
-
                     this.progressBar.$progressBar.removeClass('hidden');
                     this.progressBar.$progressBarStatus.removeClass('hidden');
 
@@ -48,12 +48,37 @@
                             complete: $.proxy(function() {
                                 var postData = Garnish.getPostData(this.$form),
                                     params = Craft.expandPostArray(postData);
-                                params.start = true;
-
-                                this.loadAction({
-                                    params: params
-                                });
-
+                                    params.start = true;
+
+                                this.cacheImages = params.cacheImages;
+
+                                Craft.postActionRequest('utilities/asset-index-perform-action', {params: params}, function (response) {
+                                    if (response.indexingData) {
+                                        this.sessionId = response.indexingData.sessionId;
+
+                                        // Load up all the data
+                                        for (var i = 0; i < response.indexingData.volumes.length; i++) {
+                                            var volumeData = response.indexingData.volumes[i];
+
+                                            for (var requestCounter = 0; requestCounter < volumeData.total; requestCounter++) {
+                                                this.queue.push({
+                                                    process: true,
+                                                    sessionId: this.sessionId,
+                                                    volumeId: volumeData.volumeId,
+                                                    cacheImages: this.cacheImages
+                                                });
+                                                this.totalActions++;
+                                            }
+                                        }
+
+                                        if (this.totalActions > 0) {
+                                            this.processIndexing();
+                                        } else {
+                                            this.onComplete();
+                                        }
+
+                                    }
+                                }.bind(this));
                             }, this)
                         });
 
@@ -72,14 +97,30 @@
                 this.progressBar.updateProgressBar();
             },
 
-            loadAction: function(data) {
-                this.loadingActions++;
+            processIndexing: function() {
+                if (this.completedActions + this.loadingActions < this.totalActions && this.loadingActions < Craft.AssetIndexesUtility.maxConcurrentActions) {
+                    this.loadingActions++;
 
-                if (typeof data.confirm !== 'undefined' && data.confirm) {
-                    this.showConfirmDialog(data);
-                }
-                else {
-                    this.postActionRequest(data.params);
+                    var params = this.queue.shift();
+                    Craft.postActionRequest('utilities/asset-index-perform-action', {params: params}, function (response) {
+                        this.loadingActions--;
+                        this.completedActions++;
+
+                        this.updateProgressBar();
+
+                        if (response && response.error) {
+                            alert(response.error);
+                        }
+
+                        if (this.completedActions == this.totalActions) {
+                            this.finishIndexing();
+                        } else {
+                            this.processIndexing();
+                        }
+                    }.bind(this));
+
+                    // Try again, in case we have more space.
+                    this.processIndexing();
                 }
             },
 
@@ -111,64 +152,26 @@
                     var params = Craft.expandPostArray(postData);
 
                     $.extend(params, data.params);
+                    params.finish = true;
 
-                    this.postActionRequest(params);
+                    Craft.postActionRequest('utilities/asset-index-perform-action', {params: params}, $.noop);
+                    this.onComplete();
                 });
             },
 
-            postActionRequest: function(params) {
-                var data = {
-                    params: params
+            finishIndexing: function() {
+                var params = {
+                    sessionId: this.sessionId,
+                    overview: true
                 };
 
-                Craft.postActionRequest('utilities/asset-index-perform-action', data, $.proxy(this, 'onActionResponse'),
-                    {
-                        complete: $.noop
-                    });
-            },
-
-            onActionResponse: function(response, textStatus) {
-                this.loadingActions--;
-                this.completedActions++;
-
-                // Add any new batches to the queue?
-                if (textStatus === 'success' && response && response.batches) {
-                    for (var i = 0; i < response.batches.length; i++) {
-                        if (response.batches[i].length) {
-                            this.totalActions += response.batches[i].length;
-                            this.queue.push(response.batches[i]);
-                        }
+                Craft.postActionRequest('utilities/asset-index-perform-action', {params: params}, function (response) {
+                    if (response.confirm) {
+                        this.showConfirmDialog(response);
+                    } else {
+                        this.onComplete();
                     }
-                }
-
-                if (response && response.error) {
-                    alert(response.error);
-                }
-
-                this.updateProgressBar();
-
-                // Load as many additional items in the current batch as possible
-                while (this.loadingActions < Craft.AssetIndexesUtility.maxConcurrentActions && this.currentBatchQueue.length) {
-                    this.loadNextAction();
-                }
-
-                // Was that the end of the batch?
-                if (!this.loadingActions) {
-                    // Is there another batch?
-                    if (this.queue.length) {
-                        this.currentBatchQueue = this.queue.shift();
-                        this.loadNextAction();
-                    }
-                    else {
-                        // Quick delay so things don't look too crazy.
-                        setTimeout($.proxy(this, 'onComplete'), 300);
-                    }
-                }
-            },
-
-            loadNextAction: function() {
-                var data = this.currentBatchQueue.shift();
-                this.loadAction(data);
+                }.bind(this));
             },
 
             onComplete: function() {