area17 / twill

Twill is an open source CMS toolkit for Laravel that helps developers rapidly create a custom admin console that is intuitive, powerful and flexible. Chat with us on Discord at https://discord.gg/cnWk7EFv8R.
https://twillcms.com
Apache License 2.0
3.8k stars 577 forks source link

When running in Octane, TwilAppSettings does not see groups #2281

Open mikerockett opened 1 year ago

mikerockett commented 1 year ago

Description

When running my application under Octane (RoadRunner), I get the following error:

The settings group 'site' does not exist

This group corresponds to what I have configured.

The error is thrown from multiple places, including middleware (where I check to see if the site is in a published state) and in views (where I get the default site title, description, and other SEO stuff).

What's interesting, is that if I sign into the admin panel, my modules don't show up at the top, but the activity log populates perfectly fine. (Notably, I am able to sign into my CMS account.)

However, if I revert to a valet/nginx-backed setup, everything works as you'd expect.

This tells me something is wrong in relation to Octane. I cannot tell if it's simply a database manager issue, or perhaps something is loading up as a singleton before it should (not sure if that is even a thing).

Any ideas?

Versions

Twill version: 3.0.2 Laravel version: 10.13.1 PHP version: 8.1.x Database engine: Postgres 14

mikerockett commented 1 year ago

Just checked in middleware (before the call to TwillAppSettings) what the database manager connection details are, using DB::connection()->getConfig(), and they are correct. I've cleared cache, views, and routes as well.

ifox commented 1 year ago

Hi @mikerockett,

Thanks for reporting this. We previously addressed issues to make sure Twill works with Octane, but this new feature from Twill 3 may have introduced a regression.

In traditional PHP, each request is a fresh PHP process and any static properties or static variables are reset between requests. With Octane, if you modify a static property or variable in one request, that modification will still be present for the next request.

This should be straightforward to fix. You may actually be able to address the problem in your own code. I'll give this a try asap if you don't come back with a solution.

mikerockett commented 1 year ago

Hi @ifox – thanks for the quick reply. :)

Could this potential regression on one class cause others to break? As noted, my modules don’t appear in the admin panel either.

Also, not sure where static properties come into the mix as the app settings class is bound to the container, and I would assume that the resolvable instance would refresh between requests as it is not a singleton.

Edit: sorry, it appears facades are singletons – I did not know this. Admittedly, I'm still confused, though, as surely if the code that hydrates the properties is executed at least once, they should be hydrated? ie, I should have settings (although these should actually be refreshed between requests) and navigation.

mikerockett commented 1 year ago

I played around a bit, and I can get things to work when I use a scoped binding on the container, via the register method of the app service provider:

$this->app->scoped(TwillAppSettings::class, fn () => new TwillAppSettings);
$this->app->scoped(TwillNavigation::class, fn () => new TwillNavigation);

Then, I can inject these anywhere, or resolve via the facades, and they resolve as I'd expect them to.

Edit: This is actually wrong, as it only works on the first request.

Update: The only way I can get it to work properly is to do this in the register method of my app service provider:

use A17\Twill\TwillAppSettings;
use A17\Twill\TwillNavigation;

$this->app->bind(
  abstract: TwillAppSettings::class,
  concrete: fn () => tap(new TwillAppSettings)
    ->registerSettingsGroup(
      SettingsGroup::make()
        ->name('site')
        ->label('Site')
    ),
);

$this->app->bind(
  abstract: TwillNavigation::class,
  concrete: fn () => (new TwillNavigation)
    ->addLink(
      NavigationLink::make()
        ->forSingleton('homepage')
        ->title('Home Page')
    )
    ->addLink(
      NavigationLink::make()
        ->forModule('pages')
        ->title('Pages')
    );
);

This is, obviously, not ideal.

mikerockett commented 1 year ago

New problem:

With the above “patch” in play, renderBlocks() now has an issue.

Call to a member function newInstance() on null

It works the first time I load the page, but the next load causes it to fail here in vendor/area17/twill/src/Helpers/BlockRenderer.php:260.

$class = Block::findFirstWithType($block->type)->newInstance();

For now, I'm just going to revert my patch-work and run without Octane 😅

mikerockett commented 1 year ago

I managed to get it working by creating a singleton for each of the concrete classes exposed by a Twill facade. And there I thought facades were singletons by default. Octane doesn't treat them like that, by the looks of it.

adss332 commented 11 months ago

Hi, could i ask u what did u do with the trouble BlockRenderer.php:260 ?

i tried to create singleton for Block, but it doesnt work.

mikerockett commented 11 months ago

@adss332 I did it like this:

Method (called in boot) to override bindings; `AppServiceProvider.php` ```php private function overrideTwillBindings(): void { $this->app->singleton( Twill\TwillAppSettings::class, static fn () => new Twill\TwillAppSettings() ); $this->app->singleton( Twill\TwillNavigation::class, static fn () => new Twill\TwillNavigation() ); $this->app->singleton( Twill\TwillBlocks::class, static fn () => new Twill\TwillBlocks() ); $this->app->singleton( Twill\TwillCapsules::class, static fn () => new Twill\TwillCapsules() ); $this->app->singleton( Twill\TwillConfig::class, static fn () => new Twill\TwillConfig() ); $this->app->singleton( Twill\TwillPermissions::class, static fn () => new Twill\TwillPermissions() ); $this->app->singleton( Twill\TwillRoutes::class, static fn () => new Twill\TwillRoutes() ); $this->app->singleton( Twill\TwillUtil::class, static fn () => new Twill\TwillUtil() ); } ```
adss332 commented 11 months ago

Thank u very much!

epicuristic commented 4 months ago

Have to say using Laravel 11 with Octane, impact of this issue is getting much higher, making panel absolutely unusable.

We got it working by moving previously mentioned overrides from AppServiceProvider to withSingletons method of bootstrap/app.php file, otherwise in modules with lots of blocks 60 secs wasn't enough to render the page.

It worked well, but we mentioned irrelevant including of CMS users navigation link into other, not related to settings, sections, and, by each new request, it was getting duplicated that mush, so we couldn't see any other section links.

Got it fixed by recreating UserController inside App\Http\Controllers\Twill namespace without calling TwillPermissions::showUserSecondaryNavigation() in the __construct method and manually registering TwillRoutes::module('users', ['except' => ['sort', 'feature']]); inside routes/twill.php.

Refs: UserController.php#L97, RoleController.php#L48, GroupController.php#L46