symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.79k stars 9.47k forks source link

[RFC] Add a `DoctrineOrmSessionHandler` #58257

Open Crovitche-1623 opened 2 months ago

Crovitche-1623 commented 2 months ago

Description

I think it could be an interesting feature to have a DoctrineOrmSessionHandler in addition of the PdoSessionHandler.

This would allow :

Then perhaps we could then use an interface with methods concerning the results expected on the doctrine entity ?

WDYT ?

Example

No response

stof commented 2 months ago

Managing the session with the ORM seems like a bad idea: as saving the session is managed by the PHP session system, it means that this custom handler would be the one triggering the flush. And this would be a total no-go if it uses the same EntityManager instance than your project code (having all your entities being flushed at the end of each request when the PHP session is saved will be a nightmare to manage, for instance if you use forms bound directly on entities by avoiding to flush entities in case of invalid form submission, as you would not be able to avoid the flush anymore). And the session data managed by PHP is an string, so there would be no benefit of using an ORM hydrating an object from which we would extract the string.

stof commented 2 months ago

I fixed my previous message regarding the format of the PHP data. At the level of the handler, the session data is a string (as it has already been serialized), not an array.

Crovitche-1623 commented 1 month ago

Thanks for the complete feedback @stof.

What do you recommend when advanced modifications need to be made to the database (apart from renaming columns, which is already possible)?

For instance:

I guess we create a class overriding the PdoSessionHandler with the required changes, configure the service definition and change the framework.session.handler_id ?

Many thanks in advance

stof commented 1 month ago

@Crovitche-1623 using a datetime column at the database level can be done with a custom handler even if it does not use the ORM (not sure you can extend the PdoSessionHandler or if you would have to start from scratch if you want that, as it will probably require changing the way the time column is managed in queries)

Regarding the session data, storing it as json would mean that the handler itself has to unserialize the data and then convert it on JSON when writing (and doing the opposite on reading). But even that does not require using an ORM at all. Not sure it is worth the cost.

And for the idCol, given that PHP forces the id to be a string, the only case I could imagine would be using a UUID type in the DB if you generate a UUID string. For that, the only need is to change the way the schema is created (by not using PdoSessionHandler::createTable, and if using doctrine/migrations by using your own schema listener instead of the core one to be able to modify the Schema after calling PdoSessionHandler::configureSchema). Passing the argument to the SQL query would still be a string. For the naming of the index, the same applies when using the Doctrine Schema system: you can customize the Schema after calling PdoSessionHandler::configureSchema.

derrabus commented 1 month ago

How about a session handler based on Doctrine DBAL?

Guikingone commented 1 month ago

Hi 👋🏻

Regarding the idea from @derrabus, mind if I ask what's the use case of using a DBAL handler?

For now, we can store the session using the PDO handler (doc), the PDO one seems to support a "Doctrine-like" DSN, what will be the pros/cons of using DBAL?

I was thinking about easing the migration path between storage engines but I might miss something, any extra thoughts? 🤔

Thanks 🙂

Crovitche-1623 commented 1 month ago

In my analysis and tests, here are the advantages and disadvantages I found when using DBAL :

Advantages:

Disadvantages:

And the problem I encountered: From what I've seen, GC queries don't appear in the profiler. Maybe because the queries :

I haven't managed to find the problem yet.

Guikingone commented 1 month ago

Thanks for the feedback, I'm currently working on a POC about this one using the idea brought by @derrabus, I'll post the progress here 🙂