Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ability to configure and extend c5 session #7258

Merged

Conversation

Projects
None yet
3 participants
@KorvinSzanto
Copy link
Member

KorvinSzanto commented Oct 29, 2018

New configuration / extension improvements

Prior to this PR we only supported two session handlers: file (default), and database. If you wanted to add a new session handler of any type, you'd need to extend SessionFactory and reimplement quite a few important meaty functions in order to properly add the ability to return a custom handler.

Now with this PR, we support three session handlers by default: file (default), database, and memcached. Also we now have the ability to simply add new handlers. For example, if I wanted to implement a custom Foo session handler with the handle custom_foo, all I would need to do is:

  1. Create an extension of this class in my own namespace like \PortlandLabs\Foo\Session\Factory
  2. Tell the application to use that class instead of the core by adding
    $app->bind(
      \Concrete\Core\Session\SessionFactory::class, 
      \PortlandLabs\Foo\Session\Factory::class);
  3. Add a getCustomFooHandler(array $config): SessionHandlerInterface method:
    class Factory extends SessionFactory
    {
        public function getCustomFooHandler(array $config)
        {
            return new CustomFooHandler();
        }
    }

Now if you configure concrete.session.handler to be custom_foo, your method will be used to create the session handler without needing to override any actual methods on the core session factory class 馃憤


New memcached session handler

This doesn't require a new dependency, we already have it available in the core with symfony's session handler package.

You can enable the memcached session handler and manage the servers yourself if you like, just create a new \Memcached instance and add servers

NOTE: The memcached extension keeps servers around even across requests! An easy way to avoid issues is to just not add servers if the memcached instance already has some

concrete5 will manage the memcached sessions for you though, just configure the concrete.session.servers config item:

return [
    'session' => [
        'handler' => 'memcached',
        'servers' => [
            ['host' => 'some.host.com'], // Defaults: Port = 11211, weight = 0
            ['host' => 'some.other.host.com', 'port' => 5050, 'weight' = 2]
        ]
    ]
]

KorvinSzanto added some commits Oct 29, 2018

@deek87

This comment has been minimized.

Copy link
Contributor

deek87 commented Oct 30, 2018

Oh, now I'm going to have to change around #7206

Though I will say, are you sure that just Binding the class works? because previously I used to just bind the interface and that worked but since 8.4.0 I have to overwrite the session singleton.

       $this->app->singleton('session', function ($app) {
            return $app->make('Concrete\Core\Session\SessionFactoryInterface')->createSession();
        });

Also you should put in a check to see if the class memcached exists, if it doesnt get the default storage

@KorvinSzanto

This comment has been minimized.

Copy link
Member Author

KorvinSzanto commented Oct 30, 2018

Though I will say, are you sure that just Binding the class works? because previously I used to just bind the interface and that worked but since 8.4.0 I have to overwrite the session singleton.

It'd depend on when you override the class. If you override it after the singleton is resolved, then yes, you have to clear that singleton. That said, I'm not sure that clearing the singleton is enough since the session would've already been started with the wrong handler.

Since the core binds Concrete\Core\Session\SessionFactoryInterface -> Concrete\Core\Session\SessionFactory, it's enough to just override the SessionFactory binding. Doing that in application/bootstrap.php has the best chance of succeeding in most places.

Also you should put in a check to see if the class memcached exists, if it doesnt get the default storage

I don't think it should fall back to the default in any case, right now you will get an error complaining that the class doesn't exist. I don't like the idea of falling back the default in that case, but I agree we could make that error better and say something like "The memcached extension is required for the memcached session handler".

@aembler aembler merged commit b117a7b into concrete5:develop Oct 31, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@deek87 deek87 referenced this pull request Nov 3, 2018

Merged

[WIP] [Feature] Adding Redis as a Session and Cache handler #7206

5 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.