diff --git a/core/src/Core.php b/core/src/Core.php index d937b554ac..3167473c98 100644 --- a/core/src/Core.php +++ b/core/src/Core.php @@ -101,6 +101,14 @@ class Core extends AbstractLaravel implements Interfaces\CoreInterface * @since 3.5.9 */ public $documentTemplateView = ''; + + /** + * Whether the DocumentParser runs over what the template produced - False for a document rendered from a file + * Plugin view engine switches it to true on OnLoadWebDocument to not re-implement a parser. + * + * @since 3.5.8 + */ + public $runDocumentParser = true; public $documentOutput; public $tstart = 0; public $mstart = 0; @@ -3287,6 +3295,7 @@ public function prepareResponse() } $template = false; + $this->runDocumentParser = true; if ($this->documentContent == '') { // get document object from DB @@ -3319,6 +3328,9 @@ public function prepareResponse() $template = TemplateProcessor::getBladeDocumentContent(); $this->documentTemplateView = $template ? (string) $template : ''; + // A file is finished output; the database holds source. A plugin may flip this on OnLoadWebDocument, below. + $this->runDocumentParser = !$template; + if ($template) { $this->documentObject['cacheable'] = 0; /** @var \Illuminate\View\View $tpl */ @@ -3373,7 +3385,7 @@ public function prepareResponse() // invoke OnLoadWebDocument event $this->invokeEvent('OnLoadWebDocument'); - if (!$template) { + if ($this->runDocumentParser) { // Parse document source $this->documentContent = $this->parseDocumentSource($this->documentContent); } @@ -3389,15 +3401,15 @@ public function prepareResponse() } } - if ($template) { - $this->outputContent(false, false); - } else { + if (!$template) { register_shutdown_function([ &$this, 'postProcess' ]); // tell PHP to call postProcess when it shuts down - $this->outputContent(); } + + // [!snippets!], [^stats^], tag cleanup, URL rewriting, escaped tag recovery. + $this->outputContent(false, $this->runDocumentParser); } public function _sendErrorForUnpubPage() diff --git a/core/src/TemplateProcessor.php b/core/src/TemplateProcessor.php index b60c0650e3..10d79ed6fb 100644 --- a/core/src/TemplateProcessor.php +++ b/core/src/TemplateProcessor.php @@ -145,12 +145,27 @@ function ($item) { */ private function templateSource(array $doc): string { - $templateId = (int) get_by_key($doc, 'template', 0); + $row = $this->templateRow((int) get_by_key($doc, 'template', 0)); + + return (string) ($row->templatesource ?? ''); + } + + /** @var array */ + private array $templateRows = []; + + /** The template row, read once: two columns are wanted at different points. */ + private function templateRow(int $templateId): ?SiteTemplate + { if ($templateId === 0) { - return ''; + return null; } - return (string) SiteTemplate::whereKey($templateId)->value('templatesource'); + if (!array_key_exists($templateId, $this->templateRows)) { + $this->templateRows[$templateId] = SiteTemplate::whereKey($templateId) + ->first(['id', 'templatesource', 'templatefileextension']); + } + + return $this->templateRows[$templateId]; } /** @@ -175,12 +190,9 @@ private function pinnedTemplateFile(array $doc, string $templateAlias): string } } - $templateId = (int) get_by_key($doc, 'template', 0); - if ($templateId === 0) { - return ''; - } + $row = $this->templateRow((int) get_by_key($doc, 'template', 0)); - $extension = (string) SiteTemplate::whereKey($templateId)->value('templatefileextension'); + $extension = (string) ($row->templatefileextension ?? ''); if ($extension === '') { return ''; } diff --git a/core/tests/Unit/Manager/DocumentParserHookTest.php b/core/tests/Unit/Manager/DocumentParserHookTest.php new file mode 100644 index 0000000000..3888d1c264 --- /dev/null +++ b/core/tests/Unit/Manager/DocumentParserHookTest.php @@ -0,0 +1,84 @@ + str_replace(chr(13), '', (string) file_get_contents($path)); +$core = static fn (string $file): string => $read(dirname(__DIR__, 3) . '/src/' . $file); +$root = static fn (string $file): string => $read(dirname(__DIR__, 4) . '/' . $file); + +test('the decision is a property a plugin can reach', function () use ($core) { + $source = $core('Core.php'); + + expect($source)->toContain('public $runDocumentParser = true;') + ->and($source)->toContain('$this->runDocumentParser = !$template;'); +}); + +test('the decision is still open when OnLoadWebDocument fires', function () use ($core) { + // That is the whole hook: the event has to run after the property is set + // and before anything reads it, or a plugin has nothing to flip. + $source = $core('Core.php'); + + $set = strpos($source, '$this->runDocumentParser = !$template;'); + $event = strpos($source, "\$this->invokeEvent('OnLoadWebDocument');"); + $read = strpos($source, 'if ($this->runDocumentParser) {'); + $output = strpos($source, '$this->outputContent(false, $this->runDocumentParser);'); + + expect($set)->toBeLessThan($event) + ->and($event)->toBeLessThan($read) + ->and($read)->toBeLessThan($output); +}); + +test('the whole pipeline follows one decision, not the storage', function () use ($core) { + // outputContent()'s tail is DocumentParser work too - uncached [!snippets!], + // [^stats^], cleanUpMODXTags(), rewriteUrls(), escaped tag recovery - so a + // plugin that turns the pass on gets those in the core's own order rather + // than re-implementing them. + $source = $core('Core.php'); + + expect($source)->toContain('$this->outputContent(false, $this->runDocumentParser);') + ->and($source)->not->toContain('$this->outputContent(false, false);'); +}); + +test('a file rendered document is not parsed unless something asks', function () { + // The core's own answer, and the one the maintainers want: Blade has had + // its say, and a second dialect does not run over the result. + $decide = static fn (bool $renderedFromFile): bool => !$renderedFromFile; + + expect($decide(true))->toBeFalse() + ->and($decide(false))->toBeTrue(); +}); + +test('nothing stores the decision any more', function () use ($core, $root) { + // No column, no migration, no form control, no lexicon - the pass is a + // property of the request, not of the template. + expect($core('Core.php'))->not->toContain('templatedocumentparser') + ->and($core('TemplateProcessor.php'))->not->toContain('templatedocumentparser') + ->and($core('Models/SiteTemplate.php'))->not->toContain('templatedocumentparser') + ->and($core('Controllers/Template.php'))->not->toContain('templatedocumentparser') + ->and($root('manager/views/page/template.blade.php'))->not->toContain('templatedocumentparser') + ->and($root('manager/processors/save_template.processor.php'))->not->toContain('templatedocumentparser') + ->and($root('core/lang/en/global.php'))->not->toContain('template_document_parser') + ->and(glob(dirname(__DIR__, 3) . '/database/migrations/*templatedocumentparser*'))->toBe([]) + ->and(glob(dirname(__DIR__, 4) . '/install/stubs/migrations/*templatedocumentparser*'))->toBe([]); +}); + +test('the template row is still read once per request', function () use ($core) { + // Kept from the same piece of work, and unrelated to the column: where the + // code lives and which engine was pinned used to be a query each. + $source = $core('TemplateProcessor.php'); + + expect($source)->toContain("first(['id', 'templatesource', 'templatefileextension'])") + ->and($source)->not->toContain("->value('templatesource')") + ->and($source)->not->toContain("->value('templatefileextension')"); +});