From b628170da522de862189a2e15a6e1e44c93b20ec Mon Sep 17 00:00:00 2001 From: Artur Kyryliuk Date: Mon, 21 Sep 2026 02:16:33 +0200 Subject: [PATCH 1/2] fix(cache): document saves no longer reset opcache or purge compiled views Document save/publish/unpublish/delete/undelete/move now call clearCache('document'): page caches are dropped and the site cache is rebuilt, but opcache is left alone and compiled Blade views are kept, so the manager no longer pays a cold recompile on the redirect after every save. Element saves and the explicit "Clear cache" keep the full clear. siteCache.idx.php and sitePublishing.idx.php are now written through a temp file + rename under a lock, so concurrent managers never include a partial file and the last writer always reflects the latest database state; the rewritten files are invalidated in opcache individually. Co-Authored-By: Claude Opus 5 --- core/src/Controllers/MoveDocument.php | 2 +- core/src/Core.php | 8 +- core/src/Legacy/Cache.php | 135 +++++++++++--- core/tests/Mocks/CacheRefreshHarness.php | 157 ++++++++++++++++ core/tests/Mocks/cache_refresh_worker.php | 24 +++ .../Unit/Legacy/CacheDocumentRefreshTest.php | 168 ++++++++++++++++++ .../processors/delete_content.processor.php | 2 +- .../processors/publish_content.processor.php | 2 +- .../processors/remove_content.processor.php | 2 +- manager/processors/save_content.processor.php | 4 +- .../processors/undelete_content.processor.php | 2 +- .../unpublish_content.processor.php | 2 +- 12 files changed, 469 insertions(+), 39 deletions(-) create mode 100644 core/tests/Mocks/CacheRefreshHarness.php create mode 100644 core/tests/Mocks/cache_refresh_worker.php create mode 100644 core/tests/Unit/Legacy/CacheDocumentRefreshTest.php diff --git a/core/src/Controllers/MoveDocument.php b/core/src/Controllers/MoveDocument.php index 7cd97535c3..14dfed2d9f 100644 --- a/core/src/Controllers/MoveDocument.php +++ b/core/src/Controllers/MoveDocument.php @@ -111,7 +111,7 @@ protected function handle() ]); // empty cache & sync site - $this->managerTheme->getCore()->clearCache('full'); + $this->managerTheme->getCore()->clearCache('document'); header('Location: index.php?a=3&id=' . $document->getKey() . '&r=9'); } diff --git a/core/src/Core.php b/core/src/Core.php index f7dd4269fa..57b1a922cd 100644 --- a/core/src/Core.php +++ b/core/src/Core.php @@ -4626,7 +4626,7 @@ public function getSnippetName() /** * Clear the cache of MODX. * - * @param string $type + * @param string|array $type '' page caches, 'full', 'document' (no opcache reset / compiled views) or a document id * @param bool $report * @return void */ @@ -4635,7 +4635,7 @@ public function clearCache($type = '', $report = false) $cache_dir = $this->bootstrapPath(); /*$this['command.view.clear']->handle();*/ - $path = $this['config']['view.compiled']; + $path = $type === 'document' ? '' : $this['config']['view.compiled']; if ($path) { foreach ($this['files']->glob("{$path}/*") as $view) { $this['files']->delete($view); @@ -4651,6 +4651,10 @@ public function clearCache($type = '', $report = false) $sync->setCachepath($cache_dir); $sync->setReport($report); $sync->emptyCache(); + } elseif ($type === 'document') { + $sync = new Legacy\Cache(); + $sync->setCachepath($cache_dir); + $sync->refreshDocumentCache($this); } elseif (preg_match('@^[1-9]\d*$@', $type)) { $key = ($this->getConfig('cache_type') == 2) ? $this->makePageCacheKey($type) : $type; $file_name = "docid_" . $key . "_*.pageCache.php"; diff --git a/core/src/Legacy/Cache.php b/core/src/Legacy/Cache.php index 2212eab663..0c73247c84 100644 --- a/core/src/Legacy/Cache.php +++ b/core/src/Legacy/Cache.php @@ -1,5 +1,6 @@ cachePath)) { - $evo->getService('ExceptionHandler')->messageQuit("Cache path not set."); - } + $evo = $this->resolveCore($evo); \Illuminate\Support\Facades\Cache::flush(); Models\UserSetting::query()->whereIn('setting_name', ['password', 'password_confirmation', 'clearPassword'])->delete(); - $files = glob(realpath($this->cachePath) . '/*.pageCache.php'); - $filesincache = count($files); - $deletedfiles = []; - while ($file = array_shift($files)) { - $name = basename($file); - clearstatcache(); - if (is_file($file)) { - if (unlink($file)) { - $deletedfiles[] = $name; - } - } - } - $opcache_restrict_api = trim(ini_get('opcache.restrict_api')); - $opcache_restrict_api = $opcache_restrict_api && mb_stripos(__FILE__, $opcache_restrict_api) !== 0; + $filesincache = count(glob(realpath($this->cachePath) . '/*.pageCache.php') ?: []); + $deletedfiles = $this->clearPageCache(); - if (!$opcache_restrict_api && function_exists('opcache_get_status')) { + if ($this->opcacheApiAllowed() && function_exists('opcache_get_status')) { $opcache = opcache_get_status(); if (!empty($opcache['opcache_enabled'])) { opcache_reset(); @@ -163,6 +147,83 @@ public function emptyCache($evo = null) } } + /** + * Document save/publish/move: drops page caches and rebuilds the site cache + * without resetting opcache or touching compiled views. + * @param null|Interfaces\CoreInterface $evo + * @since 3.5.8 + */ + public function refreshDocumentCache($evo = null) + { + $evo = $this->resolveCore($evo); + \Illuminate\Support\Facades\Cache::flush(); + $this->clearPageCache(); + $this->buildCache($evo); + $this->publishTimeConfig(); + } + + /** + * @return string[] deleted file names + */ + public function clearPageCache(): array + { + $deleted = []; + foreach (glob(realpath($this->cachePath) . '/*.pageCache.php') ?: [] as $file) { + clearstatcache(false, $file); + if (is_file($file) && @unlink($file)) { + $deleted[] = basename($file); + } + } + + return $deleted; + } + + /** + * @param mixed $evo + * @return Interfaces\CoreInterface + */ + protected function resolveCore($evo) + { + if (!($evo instanceof Interfaces\CoreInterface)) { + $evo = $GLOBALS['evo']; + } + if (!isset($this->cachePath)) { + $evo->getService('ExceptionHandler')->messageQuit("Cache path not set."); + } + + return $evo; + } + + protected function opcacheApiAllowed(): bool + { + $restrict = trim((string) ini_get('opcache.restrict_api')); + + return !($restrict && mb_stripos(__FILE__, $restrict) !== 0); + } + + /** + * Writes through a temp file + rename so concurrent readers never include a partial file, + * then drops the stale opcache entry (needed with opcache.validate_timestamps=0). + */ + protected function writeCacheFile(string $filename, string $content): bool + { + $tmp = $filename . '.' . uniqid('', true) . '.tmp'; + if (@file_put_contents($tmp, $content) === false) { + return false; + } + if (!@rename($tmp, $filename)) { + @unlink($tmp); + if (@file_put_contents($filename, $content) === false) { + return false; + } + } + if ($this->opcacheApiAllowed() && function_exists('opcache_invalidate')) { + @opcache_invalidate($filename, true); + } + + return true; + } + /** * @param string|int $cacheRefreshTime */ @@ -179,15 +240,10 @@ public function publishTimeConfig($cacheRefreshTime = '') $content .= '$recent_update=\'' . $this->request_time . '\';' . "\n"; $content .= '$cacheRefreshTime=\'' . $cacheRefreshTime . '\';' . "\n"; - $filename = evo()->getSitePublishingFilePath(); - if (!$handle = fopen($filename, 'w')) { - exit("Cannot open file ({$filename}"); - } - $content .= "\n"; - // Write $somecontent to our opened file. - if (fwrite($handle, $content) === false) { + $filename = evo()->getSitePublishingFilePath(); + if (!$this->writeCacheFile($filename, $content)) { exit("Cannot write publishing info file! Make sure the {$filename} and its directory is writable!"); } } @@ -233,6 +289,27 @@ public function getCacheRefreshTime() * @return boolean success */ public function buildCache($evo) + { + // serialise concurrent rebuilds so the last writer always reflects the latest DB state + $lock = @fopen($evo->getSiteCacheFilePath() . '.lock', 'c'); + if ($lock) { + flock($lock, LOCK_EX); + } + try { + return $this->writeSiteCache($evo); + } finally { + if ($lock) { + flock($lock, LOCK_UN); + fclose($lock); + } + } + } + + /** + * @param Interfaces\CoreInterface $evo + * @return boolean success + */ + protected function writeSiteCache($evo) { $content = "invokeEvent('OnBeforeCacheUpdate'); - if (@file_put_contents($filename, $content) === false) { + if (!$this->writeCacheFile($filename, $content)) { exit("Cannot write $filename! Make sure file or its directory is writable!"); } diff --git a/core/tests/Mocks/CacheRefreshHarness.php b/core/tests/Mocks/CacheRefreshHarness.php new file mode 100644 index 0000000000..a7a1360944 --- /dev/null +++ b/core/tests/Mocks/CacheRefreshHarness.php @@ -0,0 +1,157 @@ + 0]; + public array $events = []; + public string $dir; + + public function getConfig($name = '', $default = null) + { + return $this->config[$name] ?? $default; + } + + public function parseProperties($props, $name = null, $type = null): array + { + return []; + } + + public function invokeEvent($name, $params = []) + { + $this->events[] = $name; + return false; + } + + public function getSiteCacheFilePath(): string + { + return $this->dir . '/siteCache.idx.php'; + } + + public function getSitePublishingFilePath(): string + { + return $this->dir . '/sitePublishing.idx.php'; + } + + /** Boots Eloquent (with model events) and the Cache facade against the given sqlite database. */ + public static function boot(string $database = ':memory:'): Capsule + { + foreach (['IN_MANAGER_MODE', 'IN_INSTALL_MODE'] as $c) { + defined($c) || define($c, false); + } + defined('EVO_API_MODE') || define('EVO_API_MODE', true); + defined('EVO_CLASS') || define('EVO_CLASS', self::class); + require_once dirname(__DIR__, 2) . '/functions/preload.php'; + + $database === ':memory:' || is_file($database) || touch($database); + $capsule = new Capsule(); + $capsule->addConnection(['driver' => 'sqlite', 'database' => $database, 'prefix' => '']); + $capsule->setEventDispatcher(new Dispatcher(new Container())); + $capsule->setAsGlobal(); + $capsule->bootEloquent(); + Model::setConnectionResolver($capsule->getDatabaseManager()); + Model::clearBootedModels(); + + $container = new Container(); + $container->instance('cache', new Repository(new ArrayStore())); + Facade::setFacadeApplication($container); + + return $capsule; + } + + public static function schema(Capsule $capsule): void + { + $schema = $capsule->getConnection()->getSchemaBuilder(); + $schema->create('site_content', function (Blueprint $t) { + $t->increments('id'); + $t->string('type', 20)->default('document'); + $t->string('contentType', 50)->default('text/html'); + $t->string('pagetitle')->default(''); + $t->string('alias', 245)->nullable()->default(''); + $t->unsignedInteger('published')->default(0); + $t->unsignedInteger('pub_date')->default(0); + $t->unsignedInteger('unpub_date')->default(0); + $t->unsignedInteger('parent')->default(0); + $t->unsignedInteger('isfolder')->default(0); + $t->unsignedInteger('menuindex')->default(0); + $t->unsignedInteger('createdon')->default(0); + $t->unsignedInteger('editedon')->default(0); + $t->unsignedInteger('deleted')->default(0); + $t->unsignedInteger('deletedon')->default(0); + $t->unsignedInteger('alias_visible')->default(1); + }); + $schema->create('site_content_closure', function (Blueprint $t) { + $t->increments('closure_id'); + $t->unsignedInteger('ancestor'); + $t->unsignedInteger('descendant'); + $t->unsignedInteger('depth'); + }); + $schema->create('system_settings', function (Blueprint $t) { + $t->string('setting_name', 50)->primary(); + $t->text('setting_value')->nullable(); + }); + $schema->create('site_htmlsnippets', function (Blueprint $t) { + $t->increments('id'); + $t->string('name')->default(''); + $t->text('snippet')->nullable(); + $t->boolean('disabled')->default(0); + }); + foreach (['site_snippets' => 'snippet', 'site_plugins' => 'plugincode'] as $table => $code) { + $schema->create($table, function (Blueprint $t) use ($code) { + $t->increments('id'); + $t->string('name')->default(''); + $t->text($code)->nullable(); + $t->text('properties')->nullable(); + $t->string('moduleguid', 32)->default(''); + $t->boolean('disabled')->default(0); + }); + } + $schema->create('site_modules', function (Blueprint $t) { + $t->increments('id'); + $t->string('guid', 32)->default(''); + $t->text('properties')->nullable(); + }); + $schema->create('system_eventnames', function (Blueprint $t) { + $t->increments('id'); + $t->string('name', 50)->default(''); + }); + $schema->create('site_plugin_events', function (Blueprint $t) { + $t->unsignedInteger('pluginid'); + $t->unsignedInteger('evtid'); + $t->integer('priority')->default(0); + }); + $schema->create('user_settings', function (Blueprint $t) { + $t->unsignedInteger('user'); + $t->string('setting_name', 50); + $t->text('setting_value')->nullable(); + }); + $capsule->table('system_settings')->insert([ + ['setting_name' => 'friendly_urls', 'setting_value' => '1'], + ['setting_name' => 'use_alias_path', 'setting_value' => '1'], + ]); + } + + /** Includes a generated site cache the way Core::recoverySiteCache() does. */ + public static function listing(string $file): array + { + $reader = new class { + public $config = [], $aliasListing = [], $documentListing = [], $documentMap = [], $contentTypes = [], $chunkCache = [], $snippetCache = [], $pluginCache = [], $pluginEvent = []; + public function load(string $file): void { include $file; } + }; + $reader->load($file); + return ['aliasListing' => $reader->aliasListing, 'documentListing' => $reader->documentListing, 'config' => $reader->config]; + } +} diff --git a/core/tests/Mocks/cache_refresh_worker.php b/core/tests/Mocks/cache_refresh_worker.php new file mode 100644 index 0000000000..96eeb6e3e7 --- /dev/null +++ b/core/tests/Mocks/cache_refresh_worker.php @@ -0,0 +1,24 @@ + +[, $database, $dir, $alias] = $argv; +require dirname(__DIR__, 2) . '/vendor/autoload.php'; + +use EvolutionCMS\Legacy\Cache; +use EvolutionCMS\Models\SiteContent; +use Tests\Mocks\CacheRefreshHarness; + +$capsule = CacheRefreshHarness::boot($database); +$capsule->getConnection()->statement('PRAGMA busy_timeout = 10000'); +$GLOBALS['evo'] = $evo = new CacheRefreshHarness(); +$evo->dir = $dir; +$_SERVER['REQUEST_TIME'] = time(); + +$doc = new SiteContent(); +$doc->forceFill(['alias' => $alias, 'pagetitle' => $alias, 'parent' => 0]); +$doc->save(); + +$sync = new Cache(); +$sync->setCachepath($dir); +$sync->refreshDocumentCache(); +echo $doc->getKey(); diff --git a/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php b/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php new file mode 100644 index 0000000000..9cb964228a --- /dev/null +++ b/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php @@ -0,0 +1,168 @@ +forceFill($attrs + ['pagetitle' => $attrs['alias']]); + $doc->save(); + return $doc; +} + +function cacheRefreshListing(string $file): array +{ + return CacheRefreshHarness::listing($file)['documentListing']; +} + +beforeEach(function () { + $this->dir = str_replace('\\', '/', sys_get_temp_dir()) . '/evo-cache-test-' . uniqid(); + mkdir($this->dir); + $this->database = $this->dir . '/site.sqlite'; + CacheRefreshHarness::schema(CacheRefreshHarness::boot($this->database)); + + global $evo; + $evo = new CacheRefreshHarness(); + $evo->dir = $this->dir; + $this->evo = $evo; + $_SERVER['REQUEST_TIME'] = time(); + + $this->sync = new SyncCache(); + $this->sync->setCachepath($this->dir); +}); + +afterEach(function () { + global $evo; + $evo = null; + Capsule::connection()->disconnect(); + foreach (array_diff(scandir($this->dir), ['.', '..']) as $f) { + unlink($this->dir . '/' . $f); + } + rmdir($this->dir); +}); + +test('document refresh drops page caches, rebuilds listing and publishing file without opcache reset', function () { + cacheRefreshDoc(['id' => 1, 'alias' => 'root', 'parent' => 0, 'isfolder' => 1]); + cacheRefreshDoc(['id' => 2, 'alias' => 'child', 'parent' => 1, 'pub_date' => time() + 3600]); + file_put_contents($this->dir . '/docid_1.pageCache.php', 'x'); + file_put_contents($this->dir . '/docid_2_abc.pageCache.php', 'x'); + file_put_contents($this->dir . '/keep.txt', 'x'); + Cache::forever('snippet-output', 'stale'); + + $this->sync->refreshDocumentCache(); + + expect(glob($this->dir . '/*.pageCache.php'))->toBe([]) + ->and(file_exists($this->dir . '/keep.txt'))->toBeTrue() + ->and(Cache::get('snippet-output'))->toBeNull() + ->and($this->evo->events)->toBe(['OnBeforeCacheUpdate', 'OnCacheUpdate']); + + $listing = CacheRefreshHarness::listing($this->dir . '/siteCache.idx.php'); + expect($listing['documentListing'])->toBe(['root' => 1, 'root/child' => 2]) + ->and($listing['aliasListing'][2]['path'])->toBe('root'); + + $cacheRefreshTime = 0; + include $this->dir . '/sitePublishing.idx.php'; + expect((int) $cacheRefreshTime)->toBe(SiteContent::find(2)->pub_date) + ->and(glob($this->dir . '/*.tmp'))->toBe([]); +}); + +test('only the full clear resets opcache and cleans user password settings', function () { + $class = new ReflectionClass(SyncCache::class); + $lines = explode("\n", file_get_contents($class->getFileName())); + $body = function (string $name) use ($class, $lines) { + $m = $class->getMethod($name); + return implode("\n", array_slice($lines, $m->getStartLine() - 1, $m->getEndLine() - $m->getStartLine() + 1)); + }; + + expect($body('emptyCache'))->toContain('opcache_reset()')->toContain('UserSetting') + ->and($body('refreshDocumentCache'))->not->toContain('opcache_reset')->not->toContain('UserSetting'); +}); + +test('moving a document keeps the closure table consistent and the rebuilt listing follows the new parent', function () { + cacheRefreshDoc(['id' => 1, 'alias' => 'a', 'parent' => 0, 'isfolder' => 1]); + cacheRefreshDoc(['id' => 2, 'alias' => 'b', 'parent' => 0, 'isfolder' => 1]); + cacheRefreshDoc(['id' => 3, 'alias' => 'c', 'parent' => 1]); + cacheRefreshDoc(['id' => 4, 'alias' => 'd', 'parent' => 3]); + $this->sync->refreshDocumentCache(); + expect(cacheRefreshListing($this->dir . '/siteCache.idx.php'))->toHaveKey('a/c/d'); + + $doc = SiteContent::find(3); + $doc->parent = 2; + $doc->save(); + $this->sync = new SyncCache(); + $this->sync->setCachepath($this->dir); + $this->sync->refreshDocumentCache(); + + $ancestorsOf4 = ClosureTable::query()->where('descendant', 4)->orderBy('depth')->pluck('ancestor')->all(); + expect($ancestorsOf4)->toBe([4, 3, 2]) + ->and(ClosureTable::query()->where('ancestor', 1)->where('descendant', '!=', 1)->count())->toBe(0); + + $listing = cacheRefreshListing($this->dir . '/siteCache.idx.php'); + expect($listing)->toHaveKey('b/c/d')->not->toHaveKey('a/c/d') + ->and($listing['b/c'])->toBe(3); +}); + +test('site cache rebuild waits for a concurrent builder holding the lock', function () { + cacheRefreshDoc(['id' => 1, 'alias' => 'a', 'parent' => 0]); + $lockFile = $this->evo->getSiteCacheFilePath() . '.lock'; + $hold = 1.5; + $script = sprintf('$h=fopen(%s,"c");flock($h,LOCK_EX);echo "locked\n";flush();usleep(%d);flock($h,LOCK_UN);', var_export($lockFile, true), (int) ($hold * 1e6)); + $proc = proc_open([PHP_BINARY, '-r', $script], [1 => ['pipe', 'w']], $pipes); + expect(trim(fgets($pipes[1])))->toBe('locked'); + + $start = microtime(true); + $this->sync->buildCache($this->evo); + $elapsed = microtime(true) - $start; + proc_close($proc); + + expect($elapsed)->toBeGreaterThan($hold * 0.8) + ->and(cacheRefreshListing($this->dir . '/siteCache.idx.php'))->toBe(['a' => 1]); +}); + +test('concurrent managers saving documents all end up in the site cache', function () { + cacheRefreshDoc(['id' => 1, 'alias' => 'home', 'parent' => 0]); + $worker = dirname(__DIR__, 2) . '/Mocks/cache_refresh_worker.php'; + $procs = $pipes = []; + for ($i = 1; $i <= 6; $i++) { + $procs[$i] = proc_open([PHP_BINARY, $worker, $this->database, $this->dir, "manager$i"], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes[$i]); + } + foreach ($procs as $i => $proc) { + $out = stream_get_contents($pipes[$i][1]) . stream_get_contents($pipes[$i][2]); + expect(proc_close($proc))->toBe(0, "worker $i: $out"); + } + + $listing = cacheRefreshListing($this->dir . '/siteCache.idx.php'); + expect(array_keys($listing))->toEqualCanonicalizing(SiteContent::query()->pluck('alias')->all()) + ->and(count($listing))->toBe(7) + ->and(glob($this->dir . '/*.tmp'))->toBe([]) + ->and(ClosureTable::query()->where('depth', 0)->count())->toBe(7); +}); + +test('cache files are replaced atomically and stay includable', function () { + cacheRefreshDoc(['id' => 1, 'alias' => 'first', 'parent' => 0]); + $this->sync->buildCache($this->evo); + + SiteContent::find(1)->update(['alias' => 'second']); + $this->sync->buildCache($this->evo); + + expect(cacheRefreshListing($this->dir . '/siteCache.idx.php'))->toBe(['second' => 1]) + ->and(glob($this->dir . '/*.tmp'))->toBe([]); +}); + +test('document processors use the document cache path and element processors keep the full clear', function () { + $root = dirname(__DIR__, 4); + $document = ['manager/processors/save_content.processor.php', 'manager/processors/publish_content.processor.php', 'manager/processors/unpublish_content.processor.php', 'manager/processors/delete_content.processor.php', 'manager/processors/undelete_content.processor.php', 'manager/processors/remove_content.processor.php', 'core/src/Controllers/MoveDocument.php']; + $element = ['manager/processors/save_snippet.processor.php', 'manager/processors/save_plugin.processor.php', 'manager/processors/save_htmlsnippet.processor.php', 'manager/processors/save_template.processor.php', 'manager/processors/save_settings.processor.php', 'core/src/Controllers/RefreshSite.php']; + + foreach ($document as $file) { + expect(file_get_contents("$root/$file"))->toContain("clearCache('document')")->not->toContain("clearCache('full')"); + } + foreach ($element as $file) { + expect(file_get_contents("$root/$file"))->toContain("clearCache('full'"); + } +}); diff --git a/manager/processors/delete_content.processor.php b/manager/processors/delete_content.processor.php index 7498408b90..a905963c76 100755 --- a/manager/processors/delete_content.processor.php +++ b/manager/processors/delete_content.processor.php @@ -81,7 +81,7 @@ $_SESSION['itemname'] = $document->pagetitle; // empty cache -EvolutionCMS()->clearCache('full'); +EvolutionCMS()->clearCache('document'); // finished emptying cache - redirect $header = "Location: index.php?a=3&id=$pid&r=1" . $add_path; diff --git a/manager/processors/publish_content.processor.php b/manager/processors/publish_content.processor.php index 01ba0de7bf..e3109345df 100755 --- a/manager/processors/publish_content.processor.php +++ b/manager/processors/publish_content.processor.php @@ -53,7 +53,7 @@ $_SESSION['itemname'] = $content['pagetitle']; // empty cache -EvolutionCMS()->clearCache('full'); +EvolutionCMS()->clearCache('document'); $header="Location: index.php?a=3&id=$pid&r=1".$add_path; diff --git a/manager/processors/remove_content.processor.php b/manager/processors/remove_content.processor.php index 2473670251..d32463c7ec 100755 --- a/manager/processors/remove_content.processor.php +++ b/manager/processors/remove_content.processor.php @@ -30,7 +30,7 @@ ]); // empty cache - EvolutionCMS()->clearCache('full'); + EvolutionCMS()->clearCache('document'); // finished emptying cache - redirect $header="Location: index.php?a=2&r=1"; diff --git a/manager/processors/save_content.processor.php b/manager/processors/save_content.processor.php index e5b2459247..b13dde957e 100755 --- a/manager/processors/save_content.processor.php +++ b/manager/processors/save_content.processor.php @@ -466,7 +466,7 @@ if ($syncsite == 1) { // empty cache - $modx->clearCache('full'); + $modx->clearCache('document'); } // redirect/stay options @@ -671,7 +671,7 @@ if ($syncsite == 1) { // empty cache - $modx->clearCache('full'); + $modx->clearCache('document'); } if ($_POST['refresh_preview'] == '1') diff --git a/manager/processors/undelete_content.processor.php b/manager/processors/undelete_content.processor.php index 9c8fb38803..b1de4d0f9a 100755 --- a/manager/processors/undelete_content.processor.php +++ b/manager/processors/undelete_content.processor.php @@ -60,7 +60,7 @@ $_SESSION['itemname'] = $document->pagetitle; // empty cache -EvolutionCMS()->clearCache('full'); +EvolutionCMS()->clearCache('document'); // finished emptying cache - redirect $header = "Location: index.php?a=3&id=$pid&r=1" . $add_path; diff --git a/manager/processors/unpublish_content.processor.php b/manager/processors/unpublish_content.processor.php index 6b5811b839..d5cfa032ed 100755 --- a/manager/processors/unpublish_content.processor.php +++ b/manager/processors/unpublish_content.processor.php @@ -53,7 +53,7 @@ $_SESSION['itemname'] = $content['pagetitle']; // empty cache -EvolutionCMS()->clearCache('full'); +EvolutionCMS()->clearCache('document'); $header="Location: index.php?a=3&id=$pid&r=1".$add_path; From a595704ed226dba129dc4388983a6e986c9170fb Mon Sep 17 00:00:00 2001 From: Artur Kyryliuk Date: Mon, 21 Sep 2026 02:38:36 +0200 Subject: [PATCH 2/2] fix(tree): drag-and-drop move updates the closure table and refreshes caches The tree's movedocument AJAX handler changed the parent with a query builder update, so the SiteContent model events never ran and the closure table kept the old ancestry for the moved subtree; it also cleared no cache, leaving the alias listing and page caches stale. It now saves through the model and calls clearCache('document'), as the menu index sort action does after reordering. Co-Authored-By: Claude Opus 5 --- .../Unit/Legacy/CacheDocumentRefreshTest.php | 53 +++++++++++++++++++ .../actions/mutate_menuindex_sort.dynamic.php | 1 + manager/media/style/default/ajax.php | 9 ++-- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php b/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php index 9cb964228a..e90e5f2795 100644 --- a/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php +++ b/core/tests/Unit/Legacy/CacheDocumentRefreshTest.php @@ -107,6 +107,59 @@ function cacheRefreshListing(string $file): array ->and($listing['b/c'])->toBe(3); }); +test('tree drag-and-drop: moving a subtree to another parent, to root and reordering keeps closure and listing in step', function () { + cacheRefreshDoc(['id' => 1, 'alias' => 'a', 'parent' => 0, 'isfolder' => 1]); + cacheRefreshDoc(['id' => 2, 'alias' => 'b', 'parent' => 0, 'isfolder' => 1]); + cacheRefreshDoc(['id' => 3, 'alias' => 'c', 'parent' => 1, 'isfolder' => 1]); + cacheRefreshDoc(['id' => 4, 'alias' => 'd', 'parent' => 3]); + cacheRefreshDoc(['id' => 5, 'alias' => 'e', 'parent' => 2]); + $ancestors = fn (int $id) => ClosureTable::query()->where('descendant', $id)->orderBy('depth')->pluck('ancestor')->all(); + + // what manager/media/style/default/ajax.php movedocument does: model save, then explicit sibling order + $move = function (int $id, int $parent, array $order) { + $doc = SiteContent::withTrashed()->find($id); + $doc->parent = $parent; + $doc->save(); + foreach ($order as $key => $value) { + SiteContent::withTrashed()->where('id', $value)->update(['menuindex' => $key]); + } + $sync = new SyncCache(); + $sync->setCachepath($this->dir); + $sync->refreshDocumentCache(); + }; + + $move(3, 2, [3, 5]); + expect($ancestors(4))->toBe([4, 3, 2]) + ->and($ancestors(3))->toBe([3, 2]) + ->and(ClosureTable::query()->where('ancestor', 1)->count())->toBe(1); + $listing = cacheRefreshListing($this->dir . '/siteCache.idx.php'); + expect($listing)->toHaveKey('b/c/d')->not->toHaveKey('a/c') + ->and(array_keys($listing))->toBe(['a', 'b', 'b/c', 'b/e', 'b/c/d']); + + $move(3, 0, [1, 3, 2]); + expect($ancestors(4))->toBe([4, 3]) + ->and($ancestors(3))->toBe([3]); + $listing = cacheRefreshListing($this->dir . '/siteCache.idx.php'); + expect(array_keys($listing))->toBe(['a', 'c', 'b', 'b/e', 'c/d']); + + $move(3, 1, [3]); + expect($ancestors(4))->toBe([4, 3, 1]) + ->and(ClosureTable::query()->where('depth', '>', 0)->orderBy('descendant')->orderBy('depth')->get(['ancestor', 'descendant', 'depth'])->map(fn ($r) => "$r->ancestor>$r->descendant:$r->depth")->all()) + ->toBe(['1>3:1', '3>4:1', '1>4:2', '2>5:1']) + ->and(cacheRefreshListing($this->dir . '/siteCache.idx.php'))->toHaveKey('a/c/d'); +}); + +test('tree move and menuindex sort persist through the model and refresh the document cache', function () { + $root = dirname(__DIR__, 4); + $ajax = file_get_contents("$root/manager/media/style/default/ajax.php"); + $move = substr($ajax, strpos($ajax, "case 'movedocument'"), strpos($ajax, "case 'getLockedElements'") - strpos($ajax, "case 'movedocument'")); + + expect($move)->toContain('$document->parent = $parent;')->toContain('$document->save();')->toContain("clearCache('document')") + ->and($move)->not->toContain("update([ + 'parent'") + ->and(file_get_contents("$root/manager/actions/mutate_menuindex_sort.dynamic.php"))->toContain("clearCache('document')"); +}); + test('site cache rebuild waits for a concurrent builder holding the lock', function () { cacheRefreshDoc(['id' => 1, 'alias' => 'a', 'parent' => 0]); $lockFile = $this->evo->getSiteCacheFilePath() . '.lock'; diff --git a/manager/actions/mutate_menuindex_sort.dynamic.php b/manager/actions/mutate_menuindex_sort.dynamic.php index 4ceeb237e8..6ab9c493d1 100755 --- a/manager/actions/mutate_menuindex_sort.dynamic.php +++ b/manager/actions/mutate_menuindex_sort.dynamic.php @@ -34,6 +34,7 @@ \EvolutionCMS\Models\SiteContent::where('id', $docid)->withTrashed()->update(['menuindex' => $key]); } } + evo()->clearCache('document'); } } diff --git a/manager/media/style/default/ajax.php b/manager/media/style/default/ajax.php index 2fd250deae..482542a28b 100755 --- a/manager/media/style/default/ajax.php +++ b/manager/media/style/default/ajax.php @@ -653,10 +653,10 @@ if ($parent == 0 && $parent != $parentOld && !evo()->config['udperms_allowroot'] && $role != 1) { $json['errors'] = $_lang["error_no_privileges"]; } else { - // set new parent - SiteContent::withTrashed()->where('id', $id)->update([ - 'parent' => $parent, - ]); + // through the model so the closure table follows the new parent + $document = SiteContent::withTrashed()->find($id); + $document->parent = $parent; + $document->save(); if ($parent > 0) { // set parent isfolder = 1 @@ -692,6 +692,7 @@ 'old_parent' => $parentOld, 'new_parent' => $parent, ]); + evo()->clearCache('document'); } } }