Core

Suggested implementation in sandbox sandbox/thorsten/session-api

Cool stuff, Thorsten, you already seem to have made some nice progress!

Are you planning to move the whole database storage as it is now into a service also? That would enable you to get rid of all those "if (sessionStorage)" you have currently in the code.

Looking at how Flow Session handling currently is implemented, it's a mix of session storage and session keeping (resume, handle the ID, the cookie etc). Separating the "Storage" in it's own concearn sounds like a good plan.

Maybe you could also take a look at Symfony for some inspiration on the interface, as they provide some neat additions like Bags and some other helper-methods.

At the end, to be able to test this for review it would be great if you could provide some implementation example as an extension.

Ernesto Baschny wrote:

At the end, to be able to test this for review it would be great if you could provide some implementation example as an extension.

I've developed a proof of concept implementation in parallel to the API, of course. It uses Redis via TYPO3s Redis caching backend as a session storage backend. The extension isn't finished yet but already works (see dkd_redis_sessions.zip)

Applied in changeset c50ee9f32186eb71439cad2e80b6198b03d9a461.

[Edit: It seems Gerrit is a bit too avid ;-)]

TYPO3s services don't work too well as a way to select and instantiate a storage backend because GeneralUtitility::makeInstanceService() implements a pseudo-singleton. That's not a problem in the backend, but in the frontend BE user sessions are checked besides the FE user sessions. But due to the singleton characteristic of the session storage service init() is only called once and you can't switch between FE and BE session storages without an enormous effort.

In short: it's possible to instantiate different instances of the storage service for FE and BE, but not multiple which is required in FE.

1. Implement different classes for BE and FE session storage; these can be stubs which inherit all functionality from a common parent class and only serve to satisfy the pseudo-singleton behaviour
   This seems to be the leanest approach, yet it still means some overhead in terms of empty classes:
   

    1abstract class ClassicStorage extends \TYPO3\CMS\Core\Service\AbstractService implements StorageInterface {
    2// implementation here
    3}
    4class ClassicStorageFrontend extends ClassicStorage {
    5// no code here or method init() at most
    6}
    7class ClassicStorageBackend extends ClassicStorage {
    8// no code here or method init() at most
    9}

2. Make the service a factory that only creates a storage object of the requested type (FE, BE)
   Maybe the factory class could be generic and such part of the core. Then this wouldn't mean much of a change to the current design of the implementations. Concerning the architecture this is just one pattern too much to solve the underlying issue properly.
   

   1class AbstractUserAuthentication {
   2public function start() {
   3  $storageFactory = GeneralUtility::makeInstanceService('sessionStorage');
   4  $this->sessionStorage = $storageFactory->getInstance($this->session_table == 'fe_sessions' ? 'frontend' : 'backend');
   5  // ...
   6}
   

3. Change GeneralUtitility::makeInstanceService() to use $serviceType and serviceSubtype besides className to identify service instances
   This will break some service implementations for sure so IMHO it's not an option. Nevertheless the current behaviour could be achieved by marking the service class as singleton.
   

   1class AuthenticationService extends \TYPO3\CMS\Sv\AbstractAuthenticationService implements \TYPO3\CMS\Core\SingletonInterface {
   2// ...
   3}

4. Tell the storage which kind of session is meant
   This is as ugly as it gets, regardless of the implementation:
   

    1class AbstractUserAuthentication {
    2// ...
    3public function fetchUserSession() {
    4  $session = $this->sessionStorage->get($this->id, $this->session_table);
    5  // ...
    6}
    7// ...
    8}
    9class SessionStorage {
   10  public function get($identifier, $type) {
   11    if ($type === 'fe_sessions'} {
   12      // ...
   13    } else {
   14      // ...
   15    }
   16  }
   17// ...
   18}
   

    1class AbstractUserAuthentication {
    2// ...
    3public function fetchUserSession() {
    4  $this->sessionStorage->injectAuthentication($this);
    5  $session = $this->sessionStorage->get($this->id);
    6  // ...
    7}
    8// ...
    9}
   10class SessionStorage {
   11  public function injectAuthentication(AbstractUserAuthentication $auth) {
   12    $this->authentication = $authentication;
   13  }
   14  public function get($identifier) {
   15    if ($this->authentication->session_table === 'fe_sessions'} {
   16      // ...
   17    } else {
   18      // ...
   19    }
   20  }
   21// ...
   22}
   

Any other opinions or ideas?

Applied in changeset 97d077daafaa83d38d01eb1b263bb6e3f3661d82.

Merge was to sandbox only, patch is not in master, yet, reopened.

To get this cleanly done needs much more work and some major changes in AbstractUserAuthentication and its subclasses. Thus this feature is delayed to the next release after 6.2.