Skip to content

Commit

Permalink
Merge pull request #308 from scoutapp/possible-fix-for-last-compiled-…
Browse files Browse the repository at this point in the history
…reflection-exception

Added reference to decorated lastCompiled property if it exists
  • Loading branch information
asgrim committed Dec 4, 2023
2 parents a67f695 + 1382f81 commit 5f0b665
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 13 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,9 @@ jobs:
wget --content-on-error -O error.html http://localhost:8000/e || true
cat error.html
ps -ax
- name: "Output logs on failure"
if: failure()
run: cat test-app/storage/logs/laravel.log
- name: "Check logs for successful payload send"
run: |
cd test-app
Expand Down Expand Up @@ -791,6 +794,9 @@ jobs:
wget http://localhost:8000
cat index.html
ps -ax
- name: "Output logs on failure"
if: failure()
run: cat test-app/storage/logs/lumen.log
- name: "Check logs for successful payload send"
run: |
cd test-app
Expand Down
29 changes: 29 additions & 0 deletions src/Laravel/View/Engine/ScoutViewEngineDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

namespace Scoutapm\Laravel\View\Engine;

use Closure;
use Illuminate\Contracts\View\Engine;
use Illuminate\View\Compilers\CompilerInterface;
use Illuminate\View\Factory;
use Scoutapm\ScoutApmAgent;

use function assert;
use function method_exists;
use function property_exists;

/** @noinspection ContractViolationInspection */
final class ScoutViewEngineDecorator implements Engine
Expand All @@ -33,11 +35,38 @@ final class ScoutViewEngineDecorator implements Engine
/** @var Factory */
private $viewFactory;

/** @var array<array-key, mixed>|null */
protected $lastCompiled;

public function __construct(Engine $engine, ScoutApmAgent $agent, Factory $viewFactory)
{
$this->engine = $engine;
$this->agent = $agent;
$this->viewFactory = $viewFactory;

/**
* Unsure of which library or bit of code is trying to directly reflect on this protected property, but a
* customer reported a {@see \ReflectionException} when something was trying to reflect on `lastCompiled`
* (which was not a property of {@see ScoutViewEngineDecorator}, but it is a `protected` property in
* {@see CompilerEngine}. In order to satisfy the reflection, we can create a reference to the real
* {@see CompilerEngine::lastCompiled} property, so if someone mistakenly references our decorator directly,
* they should see the real value.
*/
if (! property_exists($engine, 'lastCompiled')) {
return;
}

/**
* @psalm-suppress MixedAssignment
* @psalm-suppress PossiblyInvalidFunctionCall
*/
$this->lastCompiled = & Closure::bind(
function & () {
return $this->lastCompiled;
},
$engine,
$engine
)->__invoke();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,21 @@

class EngineImplementationWithGetCompilerMethod implements Engine
{
/** @var list<non-empty-string> */
protected $lastCompiled = [];

/** @inheritDoc */
public function get($path, array $data = [])
{
return '';
}

/** @param list<non-empty-string> $newValue */
public function setLastCompiled(array $newValue): void
{
$this->lastCompiled = $newValue;
}

public function getCompiler(): CompilerInterface
{
return new class implements CompilerInterface {
Expand Down
40 changes: 27 additions & 13 deletions tests/Unit/Laravel/View/Engine/ScoutViewEngineDecoratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use ReflectionClass;
use ReflectionException;
use ReflectionMethod;
use ReflectionProperty;
use Scoutapm\Laravel\View\Engine\ScoutViewEngineDecorator;
use Scoutapm\ScoutApmAgent;
use Spatie\LaravelIgnition\Views\BladeSourceMapCompiler;
Expand Down Expand Up @@ -171,25 +173,20 @@ public function testScoutViewEngineDecoratorImplementsAllPublicApiOfCompilerEngi
}
}

/**
* The `spatie/laravel-ignition` package depends on the engine having a property called `lastCompiled`, which
* only exists in the `\Illuminate\View\Engines\CompilerEngine` Blade Compiler. The implementation does sort of
* account for decoration, but it expects the property to be called `engine`. Therefore, in this test, we
* invoke the problematic consumer to ensure our decorated view engine conforms to this assumption.
*
* @link https://github.com/spatie/laravel-ignition/blob/d53075177ee0c710fbf588b8569f50435e1da054/src/Views/ViewExceptionMapper.php#L124-L130
*/
public function testSpatieLaravelIgnitionCompatibility(): void
{
if (! class_exists(ViewExceptionMapper::class)) {
self::markTestSkipped('Test depends on `spatie/laravel-ignition`, but it is not installed');
}

/**
* The `spatie/laravel-ignition` package depends on the engine having a property called `lastCompiled`, which
* only exists in the `\Illuminate\View\Engines\CompilerEngine` Blade Compiler. The implementation does sort of
* account for decoration, but it expects the property to be called `engine`. Therefore, in this test, we
* invoke the problematic consumer to ensure our decorated view engine conforms to this assumption.
*
* @link https://github.com/spatie/laravel-ignition/blob/d53075177ee0c710fbf588b8569f50435e1da054/src/Views/ViewExceptionMapper.php#L124-L130
*
* @noinspection PhpPossiblePolymorphicInvocationInspection PhpUndefinedFieldInspection
* @psalm-suppress NoInterfaceProperties
*/
$this->realEngine->lastCompiled = [];

$viewEngineResolver = new EngineResolver();
$viewEngineResolver->register('blade', function () {
return $this->viewEngineDecorator;
Expand All @@ -205,4 +202,21 @@ public function testSpatieLaravelIgnitionCompatibility(): void
$vem = new ViewExceptionMapper($this->createMock(BladeSourceMapCompiler::class));
$vem->map(new ViewException('things (View: paththing)'));
}

/** @throws ReflectionException */
public function testDecoratorLastCompiledPropertyReferencesCompilerEngineLastCompiledPropertyWhenUsingReflection(): void
{
$realEngine = new EngineImplementationWithGetCompilerMethod();
$realEngine->setLastCompiled(['a', 'b']);

$this->viewEngineDecorator = new ScoutViewEngineDecorator($realEngine, $this->agent, $this->viewFactory);

$prop = new ReflectionProperty($this->viewEngineDecorator, 'lastCompiled');
$prop->setAccessible(true);
self::assertSame(['a', 'b'], $prop->getValue($this->viewEngineDecorator));

// Make sure the value can be changed at runtime, and the decorator's value is also changed
$realEngine->setLastCompiled(['a', 'b', 'c']);
self::assertSame(['a', 'b', 'c'], $prop->getValue($this->viewEngineDecorator));
}
}

0 comments on commit 5f0b665

Please sign in to comment.