flarum / flarum/issue-archive

Eliminate per-request data in application state

Open
#115 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
No merged PRs in 30d

Description

From Franz in https://github.com/flarum/core/issues/2307#issuecomment-698836620:

> I've taken great care to ensure that Flarum can easily be used with modern approaches like application servers (ReactPHP, PHP-PM, ...). This requires a mental shift from the good old days when everything was always "shared-nothing": global state was global, but only for the current request. No leaks, no worries.
>
> I want to keep the container and application classes free of per-request objects. They should only deal with truly application-wide state, whether the PHP process only handles one request or multiple. There is one object tree that can be long-living, and then there are object trees for each request. In the traditional world, there will be no difference between them, but once you run with a PHP application server, there is.
>
> Laravel does this, true, and I don't particularly like that. :wink: The downsides can be circumvented, alright, but you have to be aware of them. Just look at some of the [dark magic](https://github.com/swooletw/laravel-swoole/tree/4167206bfbaea752264c814cac5c2172e72a400a/src/Server/Resetters) that the Swoole adapter for Laravel has to do. Having this clean separation can sometimes make specific things a bit more complicated (though not impossible, in my experience), but has the benefit of being compatible with the different ways to run PHP applications - by design.

As pointed out above, Flarum is doing a relatively good job so far in terms of keeping per-request state separated from application state. However, there are still several places where we could be doing better. For now, the goal of this issue will be to catalogue these places and discuss how they could be improved.

- `SetLocale` middleware sets the current user's locale as a property on the global `LocaleManager` singleton
- `Discussion` has a static `setStateUser` method. Should be eliminatable via https://github.com/flarum/core/issues/1321

Contributor guide

Open the contributing guide

Research direction

Start by examining the SetLocale middleware and the global LocaleManager singleton, then review Discussion::setStateUser and the related issue #1321. Catalogue remaining per-request state and discuss how each case should be separated from application-wide state; done means the issue's identified locations and possible improvements are documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.