[8.x] move maintenance mode logic #40059
Conversation
Moving the maintenance mode logic to its own class. This enables users to extend/overwrite the class with their own logic. Overwriting the logic may be needed if for example the application is running on multiple servers. Fixes #36474
|
I think that up/down naming can be a bit confusing, because I think of it the opposite way of how it's currently implemented. Something like enable/disable or enter/leave feels like more natural verbs to work with. |
|
@vurpa The enter/leave names would be a good replacement. I used the up/down names for now to stay in sync with the commands used to start and stop the maintenance mode. If requested by the core developers I would be glad to change it. |
| /** | ||
| * Create a new middleware instance. | ||
| * | ||
| * @param \Illuminate\Contracts\Foundation\Application $app | ||
| * @return void | ||
| */ | ||
| public function __construct(Application $app) | ||
| public function __construct(Application $app, MaintenanceMode $maintenanceMode) |
There was a problem hiding this comment.
Adding a new dependency here is a breaking change unfortunately.
There was a problem hiding this comment.
@driesvints You're right. It would for example break implementation if people have extended the class. Should I point the update to the Laravel 9 branch instead?
| @@ -23,15 +24,21 @@ class PreventRequestsDuringMaintenance | |||
| */ | |||
| protected $except = []; | |||
|
|
|||
| /** | |||
| * @var MaintenanceMode | |||
There was a problem hiding this comment.
Please use FQN and a short description for DocBlocks
| { | ||
| try { | ||
| if (! is_file(storage_path('framework/down'))) { | ||
| if ($maintenanceMode->isDown() === false) { |
There was a problem hiding this comment.
Maybe add an isUp method as well.
| @@ -27,16 +28,16 @@ class UpCommand extends Command | |||
| * | |||
| * @return int | |||
| */ | |||
| public function handle() | |||
| public function handle(MaintenanceMode $maintenanceMode) | |||
| @@ -35,19 +36,16 @@ class DownCommand extends Command | |||
| * | |||
| * @return int | |||
| */ | |||
| public function handle() | |||
| public function handle(MaintenanceMode $maintenanceMode) | |||
There was a problem hiding this comment.
Another breaking change here.
| @@ -1109,7 +1110,7 @@ public function addAbsoluteCachePathPrefix($prefix) | |||
| */ | |||
| public function isDownForMaintenance() | |||
| { | |||
| return file_exists($this->storagePath().'/framework/down'); | |||
| return app(MaintenanceMode::class)->isDown(); | |||
There was a problem hiding this comment.
| return app(MaintenanceMode::class)->isDown(); | |
| return $this->make(MaintenanceMode::class)->isDown(); |
|
There's quite a few breaking changes here so I suggest this is best sent to master. Please also take care of the other comments. Thanks |
Moving the maintenance mode logic to its own class. This enables users to extend/overwrite the class with their own logic.
Overwriting the logic may be needed if for example the application is running on multiple servers.
Fixes #36474