Skip to content

Commit

Permalink
ApplicationTrait::onAfterRequest()
Browse files Browse the repository at this point in the history
Fixes various potential instability issues, when things were expecting to be triggered at the end of the request, if the app state was already past that point.
  • Loading branch information
brandonkelly committed Nov 16, 2023
1 parent b8f30d0 commit d7b75d6
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Fixed a bug where elements’ `localized` GraphQL field wasn’t returning any results for drafts or revisions. ([#13924](https://github.com/craftcms/cms/issues/13924))
- Fixed a bug where dropdown option labels within Table fields weren’t getting translated. ([#13914](https://github.com/craftcms/cms/issues/13914))
- Fixed a bug where “Updating search indexes” jobs were getting queued for Matrix block revisions. ([#13917](https://github.com/craftcms/cms/issues/13917))
- Fixed a bug where control panel resources weren’t getting published on demand. ([#13935](https://github.com/craftcms/cms/issues/13935))

## 4.5.10 - 2023-11-07

Expand Down
36 changes: 26 additions & 10 deletions src/base/ApplicationTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ public function getIsInitialized(): bool
}

/**
* Invokes a callback method when Craft is fully initialized.
* Invokes a callback function when Craft is fully initialized.
*
* If Craft is already fully initialized, the callback will be invoked immediately.
*
Expand All @@ -477,6 +477,29 @@ public function onInit(callable $callback): void
}
}

/**
* Invokes a callback function at the end of the request.
*
* If the request is already ending, the callback will be invoked immediately.
*
* @param callable $callback
* @since 4.5.11
*/
public function onAfterRequest(callable $callback): void
{
if (in_array($this->state, [
Application::STATE_AFTER_REQUEST,
Application::STATE_SENDING_RESPONSE,
Application::STATE_END,
], true)) {
$callback();
} else {
$this->on(Application::EVENT_AFTER_REQUEST, function() use ($callback) {
$callback();
});
}
}

/**
* Returns whether this Craft install has multiple sites.
*
Expand Down Expand Up @@ -783,16 +806,9 @@ public function saveInfoAfterRequest(): void
if (!$this->_waitingToSaveInfo) {
$this->_waitingToSaveInfo = true;

// If the request is already over, trigger this immediately
if (in_array($this->state, [
Application::STATE_AFTER_REQUEST,
Application::STATE_SENDING_RESPONSE,
Application::STATE_END,
], true)) {
$this->onAfterRequest(function() {
$this->saveInfoAfterRequestHandler();
} else {
Craft::$app->on(WebApplication::EVENT_AFTER_REQUEST, [$this, 'saveInfoAfterRequestHandler']);
}
});
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/helpers/FileHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Symfony\Component\Filesystem\Filesystem;
use Throwable;
use UnexpectedValueException;
use yii\base\Application;
use yii\base\ErrorException;
use yii\base\Exception;
use yii\base\InvalidArgumentException;
Expand Down Expand Up @@ -903,7 +902,9 @@ public static function deleteFileAfterRequest(string $filename): void
self::$_filesToBeDeleted[] = $filename;

if (count(self::$_filesToBeDeleted) === 1) {
Craft::$app->on(Application::EVENT_AFTER_REQUEST, [static::class, 'deleteQueuedFiles']);
Craft::$app->onAfterRequest(function() {
static::deleteQueuedFiles();
});
}
}

Expand All @@ -919,6 +920,8 @@ public static function deleteQueuedFiles(): void
self::unlink($source);
}
}

self::$_filesToBeDeleted = [];
}

/**
Expand Down
13 changes: 9 additions & 4 deletions src/mutex/MutexTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
namespace craft\mutex;

use Craft;
use yii\base\Application;
use yii\db\Connection;

/**
Expand Down Expand Up @@ -42,9 +41,15 @@ public function init(): void
parent::init();

$this->_db = Craft::$app->getDb();
$this->_db->on(Connection::EVENT_COMMIT_TRANSACTION, [$this, 'releaseQueuedLocks']);
$this->_db->on(Connection::EVENT_ROLLBACK_TRANSACTION, [$this, 'releaseQueuedLocks']);
Craft::$app->on(Application::EVENT_AFTER_REQUEST, [$this, 'releaseQueuedLocks']);
$this->_db->on(Connection::EVENT_COMMIT_TRANSACTION, function() {
$this->releaseQueuedLocks();
});
$this->_db->on(Connection::EVENT_ROLLBACK_TRANSACTION, function() {
$this->releaseQueuedLocks();
});
Craft::$app->onAfterRequest(function() {
$this->releaseQueuedLocks();
});
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/services/Elements.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
use craft\records\StructureElement as StructureElementRecord;
use craft\validators\HandleValidator;
use craft\validators\SlugValidator;
use craft\web\Application;
use DateTime;
use Throwable;
use UnitEnum;
Expand Down Expand Up @@ -1261,7 +1260,7 @@ public function updateCanonicalElement(ElementInterface $element, array $newAttr

$updatedCanonical = $this->duplicateElement($element, $newAttributes);

Craft::$app->on(Application::EVENT_AFTER_REQUEST, function() use ($canonical, $updatedCanonical, $changedAttributes, $changedFields) {
Craft::$app->onAfterRequest(function() use ($canonical, $updatedCanonical, $changedAttributes, $changedFields) {
// Update change tracking for the canonical element
$timestamp = Db::prepareDateForDb($updatedCanonical->dateUpdated);

Expand Down
4 changes: 3 additions & 1 deletion src/services/ProjectConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -715,8 +715,10 @@ public function updateParsedConfigTimesAfterRequest(): void
return;
}

Craft::$app->on(Application::EVENT_AFTER_REQUEST, [$this, 'updateParsedConfigTimes']);
$this->_waitingToUpdateParsedConfigTimes = true;
Craft::$app->onAfterRequest(function() {
$this->updateParsedConfigTimes();
});
}

/**
Expand Down
34 changes: 16 additions & 18 deletions src/web/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -482,25 +482,9 @@ private function _processResourceRequest(Request $request): void
$resourceUri = substr($requestPath, strlen($resourceBaseUri));
$slash = strpos($resourceUri, '/');
$hash = substr($resourceUri, 0, $slash);
$sourcePath = $this->resourceSourcePathByHash($hash);

$sourcePath = Craft::$app->getCache()->getOrSet(
Craft::$app->getAssetManager()->getCacheKeyForPathHash($hash),
function() use ($hash) {
try {
return (new Query())
->select(['path'])
->from(Table::RESOURCEPATHS)
->where(['hash' => $hash])
->scalar();
} catch (DbException) {
// Craft isn't installed yet
}

return false;
}
);

if (empty($sourcePath)) {
if (!$sourcePath) {
return;
}

Expand Down Expand Up @@ -531,6 +515,20 @@ function() use ($hash) {
$this->end();
}

private function resourceSourcePathByHash(string $hash): string|false
{
try {
return (new Query())
->select(['path'])
->from(Table::RESOURCEPATHS)
->where(['hash' => $hash])
->scalar();
} catch (DbException) {
// Craft isn't installed yet. See if it's cached as a fallback.
return Craft::$app->getCache()->get(Craft::$app->getAssetManager()->getCacheKeyForPathHash($hash));
}
}

/**
* Processes install requests.
*
Expand Down
18 changes: 8 additions & 10 deletions src/web/AssetManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,24 +93,22 @@ protected function hash($path): string

if ($this->cacheSourcePaths) {
// Store the hash for later
Craft::$app->on(Application::EVENT_AFTER_REQUEST, function() use ($hash, $alias) {
Craft::$app->onAfterRequest(function() use ($hash, $alias) {
try {
Db::upsert(Table::RESOURCEPATHS, [
'hash' => $hash,
'path' => $alias,
]);
} catch (DbException|DbConnectException) {
// Craft is either not installed or not updated to 3.0.3+ yet
// Craft is either not installed or not updated to 3.0.3+ yet,
// so cache the source path instead
Craft::$app->getCache()->set(
$this->getCacheKeyForPathHash($hash),
$alias,
dependency: new TagDependency(['tags' => [self::CACHE_TAG]]),
);
}
});

if (App::isEphemeral()) {
Craft::$app->getCache()->set(
$this->getCacheKeyForPathHash($hash),
$alias,
dependency: new TagDependency(['tags' => [self::CACHE_TAG]]),
);
}
}

return $hash;
Expand Down

0 comments on commit d7b75d6

Please sign in to comment.