Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge release 8.10.1 into 8.11.x #311

Merged
merged 6 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,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 @@ -830,6 +833,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));
}
}