This repository has been archived by the owner. It is now read-only.

Use Symfony2's session system. #136

Closed
wants to merge 1 commit into
base: staging
from

Conversation

Projects
None yet
7 participants
@realityking
Contributor

realityking commented Apr 16, 2013

Our Session component has gotten old and rusty, I propose we replace it with Symfony2's session system.

The API is very similar. We'll keep providing all storage engines we used to provide that Symfony2 does not provide, especially one for our database system, but also Xcache and APC.

What's missing is the token code. We'll have to provide this code later on. However we need a redesign here anyway, our old session code was tightly coupled to the application which is not acceptable anymore.

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Apr 17, 2013

Contributor

If you need any help, don't hesitate to ping us, the Symfony community is always willing to help projects that want to use some of our components (@Drak, @lsmith77, @stof, @fabpot, and the whole Symfony community for that matter!).

Contributor

fabpot commented Apr 17, 2013

If you need any help, don't hesitate to ping us, the Symfony community is always willing to help projects that want to use some of our components (@Drak, @lsmith77, @stof, @fabpot, and the whole Symfony community for that matter!).

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Apr 17, 2013

Feel free to contact me if you have any questions.

ghost commented Apr 17, 2013

Feel free to contact me if you have any questions.

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Apr 17, 2013

Contributor

@fabpot Is there updated written documentation on this? I checked the HttpFoundation page here: http://symfony.com/doc/2.0/components/http_foundation/introduction.html and it says the session documentation is yet to be written. I'd like to review the real world usage.

Edit: Thanks @lsmith77 and @stof for directing me to the right places.

Contributor

dongilbert commented Apr 17, 2013

@fabpot Is there updated written documentation on this? I checked the HttpFoundation page here: http://symfony.com/doc/2.0/components/http_foundation/introduction.html and it says the session documentation is yet to be written. I'd like to review the real world usage.

Edit: Thanks @lsmith77 and @stof for directing me to the right places.

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 Apr 17, 2013

as you probably want to work with master here you can find docs here http://symfony.com/doc/master/components/http_foundation/sessions.html

lsmith77 commented Apr 17, 2013

as you probably want to work with master here you can find docs here http://symfony.com/doc/master/components/http_foundation/sessions.html

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Apr 17, 2013

Contributor

Thanks @lsmith77!

Contributor

dongilbert commented Apr 17, 2013

Thanks @lsmith77!

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Apr 17, 2013

@dongilbert use either the doc at /master if using master or /current otherwise, not on /2.0 (which is for the 2.0 branch of Symfony now unmaintained). This is true for any doc

stof commented Apr 17, 2013

@dongilbert use either the doc at /master if using master or /current otherwise, not on /2.0 (which is for the 2.0 branch of Symfony now unmaintained). This is true for any doc

*
* @since 1.0
*/
class DatabaseSessionHandler implements \SessionHandlerInterface

This comment has been minimized.

@dongilbert

dongilbert Apr 17, 2013

Contributor

SessionHandlerInterface is 5.4+ only, did you mean to use the Symfony SessionInterface instead?

@dongilbert

dongilbert Apr 17, 2013

Contributor

SessionHandlerInterface is 5.4+ only, did you mean to use the Symfony SessionInterface instead?

This comment has been minimized.

@stof

stof Apr 17, 2013

No, the handler should not implement SessionInterface. They are different objects. the requirement is indeed implementing \SessionHandlerInterface for which HttpFoundation provide a userland interface for PHP 5.3 users: https://github.com/symfony/HttpFoundation/blob/master/Resources/stubs/SessionHandlerInterface.php

@stof

stof Apr 17, 2013

No, the handler should not implement SessionInterface. They are different objects. the requirement is indeed implementing \SessionHandlerInterface for which HttpFoundation provide a userland interface for PHP 5.3 users: https://github.com/symfony/HttpFoundation/blob/master/Resources/stubs/SessionHandlerInterface.php

This comment has been minimized.

@dongilbert

dongilbert Apr 17, 2013

Contributor

Ok. We handle our forward compatibility a little differently (using a compat package), so I was not aware. Thanks for clarifying that.

@dongilbert

dongilbert Apr 17, 2013

Contributor

Ok. We handle our forward compatibility a little differently (using a compat package), so I was not aware. Thanks for clarifying that.

@elkuku elkuku referenced this pull request May 2, 2013

Closed

Use of Joomla\Factory #96

@piotr-cz

This comment has been minimized.

Show comment
Hide comment
@piotr-cz

piotr-cz May 15, 2013

Contributor

Recently I had to implement 'shopping cart' - like feature so items are remembered across browser sessions (see jgen thread Set front-end user session expiration). I created new package, but it's far from being perfect.

Does new session system allow handling multiple sessions (with different durations) per one visitor?

Contributor

piotr-cz commented May 15, 2013

Recently I had to implement 'shopping cart' - like feature so items are remembered across browser sessions (see jgen thread Set front-end user session expiration). I created new package, but it's far from being perfect.

Does new session system allow handling multiple sessions (with different durations) per one visitor?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 15, 2013

@piotr-cz you can make use of the session metadata. You can then use a listener to expire users based on your preferences. If you wanted it per session could then have a session attribute that sets a threshold and read that in conjunction with the session metadata in order to determine expiry.

ghost commented May 15, 2013

@piotr-cz you can make use of the session metadata. You can then use a listener to expire users based on your preferences. If you wanted it per session could then have a session attribute that sets a threshold and read that in conjunction with the session metadata in order to determine expiry.

@piotr-cz

This comment has been minimized.

Show comment
Hide comment
@piotr-cz

piotr-cz May 15, 2013

Contributor

@Drak I mean
there's application session, with lifetime usually around 30min (configured by app admin).
I want to store specific visitor data for as long as possible, let's say 6 months (configured by component).

Could both use same session system?

Contributor

piotr-cz commented May 15, 2013

@Drak I mean
there's application session, with lifetime usually around 30min (configured by app admin).
I want to store specific visitor data for as long as possible, let's say 6 months (configured by component).

Could both use same session system?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 16, 2013

@piotr-cz - yes you set a long cookie lifetime and write the listener as I showed you.

ghost commented May 16, 2013

@piotr-cz - yes you set a long cookie lifetime and write the listener as I showed you.

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking Jun 2, 2013

Contributor

Updated the branch so this is mergeable again.

Contributor

realityking commented Jun 2, 2013

Updated the branch so this is mergeable again.

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Jun 3, 2013

Contributor

I've been thinking a bit about this, and I don't feel like pulling in the Symfony2 session system is the best move for us. One reason being that it is the only part of HttpFoundation that we would be using. I would have a more favorable view of it if Session were a stand alone component. That's a bit of extra code to keep around that we won't be using.

Alternatively, I think it would be good for Joomla and Symfony to work together to come up with a SessionInterface that could then be presented to the PHP-FIG as the Session PSR. This would allow those who want to use HttpFoundation for it's Session code to do so, without dropping our own session package.

Thoughts?

Contributor

dongilbert commented Jun 3, 2013

I've been thinking a bit about this, and I don't feel like pulling in the Symfony2 session system is the best move for us. One reason being that it is the only part of HttpFoundation that we would be using. I would have a more favorable view of it if Session were a stand alone component. That's a bit of extra code to keep around that we won't be using.

Alternatively, I think it would be good for Joomla and Symfony to work together to come up with a SessionInterface that could then be presented to the PHP-FIG as the Session PSR. This would allow those who want to use HttpFoundation for it's Session code to do so, without dropping our own session package.

Thoughts?

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking Jun 3, 2013

Contributor

I understand the concern about pulling in so much code we don't use. However I'm not really comfortable asking them to split it unless we've decided we're going to use it.

As for a common interface, what we would we do different than the Symfony2 component? To me that's only valuable if agree on the API but want a different implementation. I hope we're not going down the route where we'll run to the FIG a PSR every time we consider using someone else's code.

Contributor

realityking commented Jun 3, 2013

I understand the concern about pulling in so much code we don't use. However I'm not really comfortable asking them to split it unless we've decided we're going to use it.

As for a common interface, what we would we do different than the Symfony2 component? To me that's only valuable if agree on the API but want a different implementation. I hope we're not going down the route where we'll run to the FIG a PSR every time we consider using someone else's code.

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 Jun 3, 2013

I am hoping for lots of PSRs that cover interfaces like this, but the process is long and so its impossible to plan a release cycle on its conclusion. So I encourage people to step up and champion PSRs but I do not think that you can rely on it passing let alone passing within a few months.

I do understand your concern over the unused code. However in terms of size its quite small compared to any images you might be shipping with joomla. There are limits to how granular we can sensibly make our packages, unfortunately that would indeed mean that you would pull in code that you might not use (initially?).

lsmith77 commented Jun 3, 2013

I am hoping for lots of PSRs that cover interfaces like this, but the process is long and so its impossible to plan a release cycle on its conclusion. So I encourage people to step up and champion PSRs but I do not think that you can rely on it passing let alone passing within a few months.

I do understand your concern over the unused code. However in terms of size its quite small compared to any images you might be shipping with joomla. There are limits to how granular we can sensibly make our packages, unfortunately that would indeed mean that you would pull in code that you might not use (initially?).

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Jun 3, 2013

Contributor

Rouven, we already have different implementations. Mainly different storage engines that we support and they don't, which has already been noted. That's possibly something that can be worked around rather simply, but it's a difference nonetheless.

And I'm not against using someone else's code, as can be seen by my usage of Symfony Yaml inside of Registry.

Contributor

dongilbert commented Jun 3, 2013

Rouven, we already have different implementations. Mainly different storage engines that we support and they don't, which has already been noted. That's possibly something that can be worked around rather simply, but it's a difference nonetheless.

And I'm not against using someone else's code, as can be seen by my usage of Symfony Yaml inside of Registry.

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Jun 3, 2013

Contributor

As for using the other parts of HttpFoundation, I'm open to that possibility. We don't have a Response class

Contributor

dongilbert commented Jun 3, 2013

As for using the other parts of HttpFoundation, I'm open to that possibility. We don't have a Response class

@dongilbert dongilbert closed this Jun 3, 2013

@dongilbert dongilbert reopened this Jun 3, 2013

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Jun 3, 2013

Contributor

Unintentional closing - sorry about that.

Contributor

dongilbert commented Jun 3, 2013

Unintentional closing - sorry about that.

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Jun 3, 2013

Contributor

I didn't get a chance to finish my last comment either. Probably hit some key combination that closed it - idk

We don't have a Response class, per sé. We just use our Application class to handle the response, which I'm not a fan of. So, it's a possibility we refactor and build on top of HttpFoundation? I don't know.

Contributor

dongilbert commented Jun 3, 2013

I didn't get a chance to finish my last comment either. Probably hit some key combination that closed it - idk

We don't have a Response class, per sé. We just use our Application class to handle the response, which I'm not a fan of. So, it's a possibility we refactor and build on top of HttpFoundation? I don't know.

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking Jun 3, 2013

Contributor

All the storage engine we currently support art still supported with this pull request. With the added advantage since they implement a standard PHP interface they can easily be used without any sort of session class. We'd also gain handlers for PDO and MongoDB for free.

As for our implementation - it has several big flaws, including the flowing:

  • Session handlers have to be implemented according to our own interface, have to follow a certain naming convention and can't take any dependencies so they rely on globals. (Like the current database handler relies on Factory::getDbo())
  • It's hard to create a mock session (for example for use in a CLI environment) because we're tightly coupled with $_SESSION. This also hurts our testability.
  • We haven't done most of the changes to use the newer features available in PHP 5.4 to make it more reliable.
Contributor

realityking commented Jun 3, 2013

All the storage engine we currently support art still supported with this pull request. With the added advantage since they implement a standard PHP interface they can easily be used without any sort of session class. We'd also gain handlers for PDO and MongoDB for free.

As for our implementation - it has several big flaws, including the flowing:

  • Session handlers have to be implemented according to our own interface, have to follow a certain naming convention and can't take any dependencies so they rely on globals. (Like the current database handler relies on Factory::getDbo())
  • It's hard to create a mock session (for example for use in a CLI environment) because we're tightly coupled with $_SESSION. This also hurts our testability.
  • We haven't done most of the changes to use the newer features available in PHP 5.4 to make it more reliable.
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jun 3, 2013

It would be a big win for you to use as much of HttpFoundation as possible. We did in Zikula and we're not looking back. you can make interesting BC layers in the beginning. Do it, you wont regret it.

ghost commented Jun 3, 2013

It would be a big win for you to use as much of HttpFoundation as possible. We did in Zikula and we're not looking back. you can make interesting BC layers in the beginning. Do it, you wont regret it.

@realityking

This comment has been minimized.

Show comment
Hide comment
@realityking

realityking Jun 3, 2013

Contributor

@Drak
That's really a completely different discussion. Our approach is fundamentally different from Request (unified input for CLI and HTTP, stronger focus on input serialization). I do like Symfony2's Response class though.

I've opened #174 so we have a place to discuss this independently of the Session.

Contributor

realityking commented Jun 3, 2013

@Drak
That's really a completely different discussion. Our approach is fundamentally different from Request (unified input for CLI and HTTP, stronger focus on input serialization). I do like Symfony2's Response class though.

I've opened #174 so we have a place to discuss this independently of the Session.

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Jun 4, 2013

Contributor

Considering the deficiencies in our own implementation that you pointed out, @realityking, I'm starting to see the benefits of just using this instead. Especially if we utilize the Response layer as well, it would be a "big win" as @Drak so eloquently stated. 👍

Contributor

dongilbert commented Jun 4, 2013

Considering the deficiencies in our own implementation that you pointed out, @realityking, I'm starting to see the benefits of just using this instead. Especially if we utilize the Response layer as well, it would be a "big win" as @Drak so eloquently stated. 👍

@AmyStephen

This comment has been minimized.

Show comment
Hide comment
@AmyStephen

AmyStephen Jun 4, 2013

Contributor

I've been watching this with great interest because I believe that these interproject collaborations are key to strengthening all involved. Very encouraged to see these discussions. To me, it's not a question of whether or not to forge these kinds of partnerships, but when, and how, and in conjunction with what framework strategy.

Unfortunately, the problems that @realityking lists for the session package (tight coupling, static connections, no interfaces, class construction logic within concrete classes, no DI approach, etc.), exist across the board with all Joomla framework packages. If it were merely a matter of fixing the session package, I'd say go for it, regardless of the extra HTTP classes, that's more of a minor annoyance than a problem. But, that's not the current situation.

If I understand the Joomla team's plan correctly, the first step was separating out those packages just so they are not one gigantic centipede of software. I think the team is nearing the conclusion of that phase and doing that opens the world up.

But, once that is done, I think there is an opportunity to talk architecture and envision where you want the framework. There are a number of key decisions that must be made. Once that framework vision starts to emerge, then, I think it makes sense to look around and see what is available that would help the team more quickly bring that vision to life. In fact, being able to share that strategy with the likes of a Symfony project will help them advise you better on how to benefit from that software.

At this time, though, my advise would be to not add anything of significance to the current environment, don't make any major changes, until you've had time to do that high level planning as a team, and you know what DI strategy you are going to use and where interfaces fit in and what types of extensibility you want to support. It will be more clear what you are looking for, you'll have better agreement as a team, and the work can be accomplished more quickly.

It is encouraging to see the Joomla framework developers understand their team extends outside of the project, and of course we've seen that from the Symfony folks for awhile, they are leaders in this area. All very good for open source.

Contributor

AmyStephen commented Jun 4, 2013

I've been watching this with great interest because I believe that these interproject collaborations are key to strengthening all involved. Very encouraged to see these discussions. To me, it's not a question of whether or not to forge these kinds of partnerships, but when, and how, and in conjunction with what framework strategy.

Unfortunately, the problems that @realityking lists for the session package (tight coupling, static connections, no interfaces, class construction logic within concrete classes, no DI approach, etc.), exist across the board with all Joomla framework packages. If it were merely a matter of fixing the session package, I'd say go for it, regardless of the extra HTTP classes, that's more of a minor annoyance than a problem. But, that's not the current situation.

If I understand the Joomla team's plan correctly, the first step was separating out those packages just so they are not one gigantic centipede of software. I think the team is nearing the conclusion of that phase and doing that opens the world up.

But, once that is done, I think there is an opportunity to talk architecture and envision where you want the framework. There are a number of key decisions that must be made. Once that framework vision starts to emerge, then, I think it makes sense to look around and see what is available that would help the team more quickly bring that vision to life. In fact, being able to share that strategy with the likes of a Symfony project will help them advise you better on how to benefit from that software.

At this time, though, my advise would be to not add anything of significance to the current environment, don't make any major changes, until you've had time to do that high level planning as a team, and you know what DI strategy you are going to use and where interfaces fit in and what types of extensibility you want to support. It will be more clear what you are looking for, you'll have better agreement as a team, and the work can be accomplished more quickly.

It is encouraging to see the Joomla framework developers understand their team extends outside of the project, and of course we've seen that from the Symfony folks for awhile, they are leaders in this area. All very good for open source.

@dongilbert

This comment has been minimized.

Show comment
Hide comment
@dongilbert

dongilbert Nov 6, 2013

Contributor

This will be considered for 2.0 (with a high probability of acceptance)

Contributor

dongilbert commented Nov 6, 2013

This will be considered for 2.0 (with a high probability of acceptance)

@dongilbert dongilbert closed this Nov 6, 2013

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.