Theming, currentUser, and the assets service (deprecate per user theme)

[alexweissman]

Our current implementation of theming allows the currentUser service to dynamically load a sprinkle based on the users.theme column in the database. Since theme sprinkles can potentially contain assets, the assets service is overridden to force the currentUser service to boot up on every request.

This created a potential performance issue in that the database had to be queried for every request - including requests for raw assets. To solve this, the current user object is now automatically cached on disk (or in redis) for a period of time (522f99e).

This caching is done in the Authenticator:: validateUserAccount method. However this does not account for any custom implementations outside of the Authenticator class. For example, I override the currentUser service to dynamically load a user subtype model depending on the value of a user type column. This causes the database to be hit via the currentUser service, which is forced to boot by the assets service.

I actually don't even use the theming feature, so one possibility would be to simply disable theming somehow so that assets doesn't need to boot currentUser. Or, I could have the ability to define my own custom Authenticator class that caches my subtype model along with the original user model. This is not possible at the moment because there are a few hard dependencies on the Authenticator class instead of depending on an interface.

My final option is to check the request path in my currentUser model and skip loading the subtype model if the request is for a raw asset. This seems rather kludgy though.

Does it really make sense to implement per-user themes on the sprinkle level at all? Also, we should remove the hard dependencies on Authenticator and replace with an interface.

[lcharette]

I see many potential solutions :

1. Get rid of per user theme (not actually used anywhere afaik in the default install). It could still be added back in a user sprinkle.
2. Add more caching (for the user theme)
3. Add a config to disable user theme check globally
4. Find a better solution than currentUser service

[alexweissman]

Maybe we can deprecate sprinkle-based theming, with a config setting to disable it in the meantime? I'm not really sure that there is any reason to conditionally load assets/templates/whatever based on the current user. I've found it more tractable to add logic within specific endpoints when I need the interface to vary substantially from user to user.

I guess there is an argument for using themes to implement access control to images, for example, but this can just as easily be done with a custom endpoint.

On top of this, we should use an AuthenticatorInterface so that one could swap in their own custom authenticator class (and do more caching if they want).

[lcharette]

And considering the inevitable switch to a separate frontend system like vue or React in the future, per user theme on the server side will probably be obsolete anyway.

[lcharette]

I'll see if I can mark this feature as deprecated for 4.6, so we can completely remove it next version.