Skip to content

[RFC] Add a DoctrineOrmSessionHandler #58257

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Crovitche-1623 opened this issue Sep 13, 2024 · 11 comments
Closed

[RFC] Add a DoctrineOrmSessionHandler #58257

Crovitche-1623 opened this issue Sep 13, 2024 · 11 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Stalled

Comments

@Crovitche-1623
Copy link

Description

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

This would allow :

  • have SQL queries sent to the database in the Doctrine profiler
  • Manage the table as an entity, making it customizable and more generic to projects using Doctrine (use a migration to generate the table).
  • Delegate lock management to Doctrine.

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

WDYT ?

Example

No response

@carsonbot carsonbot added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Sep 13, 2024
@stof
Copy link
Member

stof commented Sep 13, 2024

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.

  • Seeing SQL queries in the Doctrine profiler does not requires using doctrine/orm but doctrine/dbal. We used to have a DbalSessionHandler in the past but it has been deprecated and removed (at some point, PdoSessionHandler supported using a DBAL connection as a replacement but this is also gone now).
  • Managing the table through doctrine/migrations does not require using doctrine/orm but only integrating with the Schema system of doctrine/dbal. And PdoSessionHandler supports such integration.
  • the lock management of the ORM is delegated to the database using advisory locks. PdoSessionHandler already supports using advisory locks (through its lock_mode option) so there is no benefit there

@stof
Copy link
Member

stof commented Sep 13, 2024

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
Copy link
Author

Crovitche-1623 commented Sep 17, 2024

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:

  • a different column type e.g. (session data if the session is serialized in JSON format instead of PHP's serialization format / a Datetime type for the timeCol / a different idCol datatype when the create_sid function from \SessionIdInterface is used) ;
  • More control over the naming of the lifetimeCol index

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
Copy link
Member

stof commented Sep 17, 2024

@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
Copy link
Member

How about a session handler based on Doctrine DBAL?

@Guikingone
Copy link
Contributor

Guikingone commented Oct 4, 2024

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
Copy link
Author

Crovitche-1623 commented Oct 4, 2024

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

Advantages:

Disadvantages:

  • Require an additional connection to avoid transaction issues
  • It's another layer of abstraction for perhaps few advantages...

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
Copy link
Contributor

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 🙂

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?
Every feature is developed by the community.
Perhaps someone would like to try?
You can read how to contribute to get started.

@carsonbot
Copy link

Friendly ping? Should this still be open? I will close if I don't hear anything.

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

@carsonbot carsonbot closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed) Stalled
Projects
None yet
Development

No branches or pull requests

5 participants