From ec53dfe3df80be38bcb3a01860d596ce2aed54e6 Mon Sep 17 00:00:00 2001 From: Freek Van der Herten Date: Thu, 3 Jun 2021 09:00:50 +0200 Subject: [PATCH] Add a solution for lazy loading violations (#392) * wip * wip * Fix styling Co-authored-by: freekmurze --- .github/workflows/php-cs-fixer.yml | 2 +- .github/workflows/psalm.yml | 33 -------------- .gitignore | 2 + .php_cs => .php_cs.php | 13 +++--- src/IgnitionServiceProvider.php | 2 + .../LazyLoadingViolationSolutionProvider.php | 41 ++++++++++++++++++ src/Support/LaravelVersion.php | 11 +++++ ...zyLoadingViolationSolutionProviderTest.php | 43 +++++++++++++++++++ 8 files changed, 105 insertions(+), 42 deletions(-) delete mode 100644 .github/workflows/psalm.yml rename .php_cs => .php_cs.php (76%) create mode 100644 src/SolutionProviders/LazyLoadingViolationSolutionProvider.php create mode 100644 src/Support/LaravelVersion.php create mode 100644 tests/Solutions/LazyLoadingViolationSolutionProviderTest.php diff --git a/.github/workflows/php-cs-fixer.yml b/.github/workflows/php-cs-fixer.yml index 84ab01ad..74172c66 100644 --- a/.github/workflows/php-cs-fixer.yml +++ b/.github/workflows/php-cs-fixer.yml @@ -13,7 +13,7 @@ jobs: - name: Fix style uses: docker://oskarstark/php-cs-fixer-ga with: - args: --config=.php_cs --allow-risky=yes + args: --config=.php_cs.php --allow-risky=yes - name: Extract branch name shell: bash diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml deleted file mode 100644 index c21694fe..00000000 --- a/.github/workflows/psalm.yml +++ /dev/null @@ -1,33 +0,0 @@ -name: Psalm - -on: - push: - paths: - - '**.php' - - 'psalm.xml' - -jobs: - psalm: - name: psalm - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - - name: Setup PHP - uses: shivammathur/setup-php@v2 - with: - php-version: '8.0' - extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, bcmath, soap, intl, gd, exif, iconv, imagick - coverage: none - - - name: Cache composer dependencies - uses: actions/cache@v1 - with: - path: vendor - key: composer-${{ hashFiles('composer.lock') }} - - - name: Run composer require - run: composer require -n --prefer-dist - - - name: Run psalm - run: ./vendor/bin/psalm -c psalm.xml diff --git a/.gitignore b/.gitignore index a1703e2a..a62a1b03 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,5 @@ composer.lock .phpunit.result.cache tests/stubs/Controllers/GitConflictController.php package-lock.json +.php-cs-fixer.cache + diff --git a/.php_cs b/.php_cs.php similarity index 76% rename from .php_cs rename to .php_cs.php index 4ca9a7f8..d612a548 100644 --- a/.php_cs +++ b/.php_cs.php @@ -1,27 +1,23 @@ notPath('bootstrap/*') - ->notPath('storage/*') - ->notPath('resources/view/mail/*') ->in([ __DIR__ . '/src', __DIR__ . '/tests', ]) ->name('*.php') ->notName('*.blade.php') - ->notName('GitConflictController.php') ->ignoreDotFiles(true) ->ignoreVCS(true); -return PhpCsFixer\Config::create() +return (new PhpCsFixer\Config()) ->setRules([ '@PSR2' => true, 'array_syntax' => ['syntax' => 'short'], - 'ordered_imports' => ['sortAlgorithm' => 'alpha'], + 'ordered_imports' => ['sort_algorithm' => 'alpha'], 'no_unused_imports' => true, 'not_operator_with_successor_space' => true, - 'trailing_comma_in_multiline_array' => true, + 'trailing_comma_in_multiline' => true, 'phpdoc_scalar' => true, 'unary_operator_spaces' => true, 'binary_operator_spaces' => true, @@ -33,6 +29,7 @@ 'method_argument_space' => [ 'on_multiline' => 'ensure_fully_multiline', 'keep_multiple_spaces_after_comma' => true, - ] + ], + 'single_trait_insert_per_statement' => true, ]) ->setFinder($finder); diff --git a/src/IgnitionServiceProvider.php b/src/IgnitionServiceProvider.php index d6991a82..a725aade 100644 --- a/src/IgnitionServiceProvider.php +++ b/src/IgnitionServiceProvider.php @@ -35,6 +35,7 @@ use Facade\Ignition\SolutionProviders\DefaultDbNameSolutionProvider; use Facade\Ignition\SolutionProviders\IncorrectValetDbCredentialsSolutionProvider; use Facade\Ignition\SolutionProviders\InvalidRouteActionSolutionProvider; +use Facade\Ignition\SolutionProviders\LazyLoadingViolationSolutionProvider; use Facade\Ignition\SolutionProviders\MergeConflictSolutionProvider; use Facade\Ignition\SolutionProviders\MissingAppKeySolutionProvider; use Facade\Ignition\SolutionProviders\MissingColumnSolutionProvider; @@ -404,6 +405,7 @@ protected function getDefaultSolutions(): array UndefinedPropertySolutionProvider::class, MissingMixManifestSolutionProvider::class, MissingLivewireComponentSolutionProvider::class, + LazyLoadingViolationSolutionProvider::class, ]; } diff --git a/src/SolutionProviders/LazyLoadingViolationSolutionProvider.php b/src/SolutionProviders/LazyLoadingViolationSolutionProvider.php new file mode 100644 index 00000000..1b772135 --- /dev/null +++ b/src/SolutionProviders/LazyLoadingViolationSolutionProvider.php @@ -0,0 +1,41 @@ +getPrevious()) { + return false; + } + + return $previous instanceof LazyLoadingViolationException; + } + + public function getSolutions(Throwable $throwable): array + { + $majorVersion = LaravelVersion::major(); + + return [BaseSolution::create( + 'Lazy loading was disabled to detect N+1 problems' + ) + ->setSolutionDescription( + 'Either avoid lazy loading the relation or allow lazy loading.' + ) + ->setDocumentationLinks([ + 'Read the docs on preventing lazy loading' => "https://laravel.com/docs/{$majorVersion}.x/eloquent-relationships#preventing-lazy-loading", + 'Watch a video on how to deal with the N+1 problem' => 'https://www.youtube.com/watch?v=ZE7KBeraVpc', + ]),]; + } +} diff --git a/src/Support/LaravelVersion.php b/src/Support/LaravelVersion.php new file mode 100644 index 00000000..42d9eb7b --- /dev/null +++ b/src/Support/LaravelVersion.php @@ -0,0 +1,11 @@ +version(), 0, 1); + } +} diff --git a/tests/Solutions/LazyLoadingViolationSolutionProviderTest.php b/tests/Solutions/LazyLoadingViolationSolutionProviderTest.php new file mode 100644 index 00000000..8e10d9de --- /dev/null +++ b/tests/Solutions/LazyLoadingViolationSolutionProviderTest.php @@ -0,0 +1,43 @@ +app->version(), '8.0.0', '<')) { + $this->markTestSkipped(); + } + } + + /** @test */ + public function it_can_solve_lazy_loading_violations() + { + $canSolve = app(LazyLoadingViolationSolutionProvider::class) + ->canSolve(new LazyLoadingViolationException(new User(), 'posts')); + + $this->assertTrue($canSolve); + + $canSolve = app(LazyLoadingViolationSolutionProvider::class) + ->canSolve(new Exception('generic exception')); + + $this->assertFalse($canSolve); + } + + public function it_can_provide_the_solution_for_lazy_loading_exceptions() + { + $solutions = app(LazyLoadingViolationSolutionProvider::class) + ->getSolutions(new LazyLoadingViolationException(new User(), 'posts')); + + $this->assertCount(1, $solutions); + } +}