[HttpFoundation] Create migration for session table when pdo handler is used #48059

alli83 · 2022-10-31T12:03:00Z

Q	A
Branch?	6.3
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets
License	MIT
Doc PR

The purpose of this PR is to automatically generate the session table with the make migration command in addition to https://symfony.com/doc/current/session/database.html#preparing-the-database-to-store-sessions

Even though WellKnownSchemaFilterPass is deprecated, atm the session table is blacklisted by the WellKnownSchemaFilterPass, that's why I need to set the SchemaAssetFilter to null.

todo:

doctrineBundle => Register PdoSessionHandlerSchemaSubscriber doctrine/DoctrineBundle#1569
Add mention to changelog
update documentation

ostrolucky

LGTM, but should go to 2.8.x. And should be merged only after upstream PR is merged.

/edit: Ah sorry, I intended to write this comment in doctrine-bundle (and I can't delete this comment now)

src/Symfony/Bridge/Doctrine/SchemaListener/PdoSessionHandlerSchemaSubscriber.php

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php

nicolas-grekas

Thanks for the PR!
I force-pushed some changes here and on doctrine/DoctrineBundle#1569
Once this PR is ready, we'll have to do the same for the lock component :)

nicolas-grekas · 2022-11-07T16:20:46Z

src/Symfony/Bridge/Doctrine/SchemaListener/AbstractSchemaSubscriber.php

+        ];
+    }
+
+    protected function getIsSameDatabaseChecker(Connection $connection): \Closure


When integrating a component with doctrine migrations, we want to known whether the database used by doctrine is the same as the one used by the component. In e.g. Messenger, this can be done by comparing the connection objects for identity, but in cases where several connections are used (eg session / lock use a different connection), then this doesn't work.

This method implements this check by creating a temporary table: if the component's connection can drop this table, then we're on the same db.

That makes sense. Thank you 🙏

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php

chalasr

LGTM

src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/DoctrineTransport.php

…is used

nicolas-grekas · 2022-12-16T10:50:53Z

Thanks @alli83 for working on this feature, this is much appreciated.

nicolas-grekas

thanks :)

…rSchemaSubscriber (alli83) This PR was merged into the 6.3 branch. Discussion ---------- [HttpFoundation] inject SessionHandler in PdoSessionHandlerSchemaSubscriber | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT | Doc PR | I'm working on it following the PR #48059. In #48059, it works only if you put the handler explicitly to PdoSessionHandler in the configuration (handler_id) but it stops working if the session is defined by the factory and we can't use it as follow: ```` handler_id: '%env(resolve:DATABASE_URL)%' ````` Linked to DoctrineBundle PR doctrine/DoctrineBundle#1623 Also imho, I think the second argument of configureSchema inPdoSessionHandler should be optional and set to return true if not defined since the function is public eg. one wants to add the table to the Schema. Commits ------- 9aded06 [HttpFoundation] inject SessionHandler in PdoSessionHandlerSchemaSubscriber

…rSchemaSubscriber (alli83) This PR was merged into the 6.3 branch. Discussion ---------- [HttpFoundation] inject SessionHandler in PdoSessionHandlerSchemaSubscriber | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT | Doc PR | I'm working on it following the PR symfony/symfony#48059. In symfony/symfony#48059, it works only if you put the handler explicitly to PdoSessionHandler in the configuration (handler_id) but it stops working if the session is defined by the factory and we can't use it as follow: ```` handler_id: '%env(resolve:DATABASE_URL)%' ````` Linked to DoctrineBundle PR doctrine/DoctrineBundle#1623 Also imho, I think the second argument of configureSchema inPdoSessionHandler should be optional and set to return true if not defined since the function is public eg. one wants to add the table to the Schema. Commits ------- 9aded06c0c [HttpFoundation] inject SessionHandler in PdoSessionHandlerSchemaSubscriber

…ndler::configureSchema() (nicolas-grekas) This PR was merged into the 6.3 branch. Discussion ---------- [HttpFoundation] Fix defining expiry index in PdoSessionHandler::configureSchema() | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Forgotten in #48059 Commits ------- 1a67728 [HttpFoundation] Fix defining expiry index in PdoSessionHandler::configureSchema()

…d update tests (alli83) This PR was merged into the 6.3 branch. Discussion ---------- [DoctrineBridge] add missing UPGRADE notes for #50689 and update tests | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | | License | MIT | Doc PR | `$isSameDatabase` parameter was introduced in #48059. It has been added in order to know whether the database used by doctrine is the same as the one used by the component when integrating the latter with doctrine migrations. Also related to #50689 Commits ------- 5d2817d [DoctrineBridge] add missing UPGRADE notes for #50689

… introduced in symfony#48059

…eDatabase arg… (alli83) This PR was merged into the 6.3 branch. Discussion ---------- [DoctrineBridge] add missing changelog mention for isSameDatabase arg… | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | $isSameDatabase parameter was introduced in #48059. It has been added in order to know whether the database used by doctrine is the same as the one used by the component when integrating the latter with doctrine migrations. Also related to #50699 (UPGRADE mention) and #50689 Commits ------- 208861a [DoctrineBridge] add missing changelog mention for isSameDatabase arg introduced in #48059

* 6.3: [DoctrineBridge] add missing changelog mention for isSameDatabase arg introduced in #48059 Bump Symfony version to 6.3.2 Update VERSION for 6.3.1 Update CHANGELOG for 6.3.1 Bump Symfony version to 6.2.13 Update VERSION for 6.2.12 Update CHANGELOG for 6.2.12 Bump Symfony version to 5.4.26 Update VERSION for 5.4.25 Update CONTRIBUTORS for 5.4.25 Update CHANGELOG for 5.4.25

* 6.4: [DoctrineBridge] add missing changelog mention for isSameDatabase arg introduced in #48059 [DependencyInjection] Skip scanning scalar values in compiler passes when not needed Bump Symfony version to 6.3.2 Update VERSION for 6.3.1 Update CHANGELOG for 6.3.1 Bump Symfony version to 6.2.13 Update VERSION for 6.2.12 Update CHANGELOG for 6.2.12 Bump Symfony version to 5.4.26 Update VERSION for 5.4.25 Update CONTRIBUTORS for 5.4.25 Update CHANGELOG for 5.4.25

… $isSameDatabase to configureSchema() methods (alli83) This PR was merged into the 7.0 branch. Discussion ---------- [Cache][DoctrineBridge][Lock][Messenger] Add parameter $isSameDatabase to configureSchema() methods | Q | A | ------------- | --- | Branch? | 7.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | $isSameDatabase parameter was introduced in #48059. This PR aims to remove BC layer on configureSchema method and $isSameDatabase parameter This parameter has been added in UPGRADE-6.3 in #50699 Commits ------- 7a8a8bf [DoctrineBridge]rm BC layer configureSchema check database

carsonbot added Status: Needs Review Feature labels Oct 31, 2022

carsonbot added this to the 6.2 milestone Oct 31, 2022

alli83 mentioned this pull request Oct 31, 2022

Register PdoSessionHandlerSchemaSubscriber doctrine/DoctrineBundle#1569

Merged

alli83 force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch 2 times, most recently from f8cb980 to 734cf60 Compare October 31, 2022 12:17

alli83 changed the title ~~[Http-Foundation][Session]feat: create automatically session table wi…~~ [Http-Foundation][Session]feat: create automatically session table with pro connection Oct 31, 2022

alli83 changed the title ~~[Http-Foundation][Session]feat: create automatically session table with pro connection~~ [Http-Foundation][Session]feat: create automatically session table with pdo connection Oct 31, 2022

ostrolucky approved these changes Oct 31, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 31, 2022

ostrolucky approved these changes Oct 31, 2022

View reviewed changes

nicolas-grekas reviewed Oct 31, 2022

View reviewed changes

src/Symfony/Bridge/Doctrine/SchemaListener/PdoSessionHandlerSchemaSubscriber.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Oct 31, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php Outdated Show resolved Hide resolved

nicolas-grekas added the HttpFoundation label Oct 31, 2022

carsonbot changed the title ~~[Http-Foundation][Session]feat: create automatically session table with pdo connection~~ [HttpFoundation] [Http-Foundation][Session]feat: create automatically session table with pdo connection Oct 31, 2022

nicolas-grekas changed the title ~~[HttpFoundation] [Http-Foundation][Session]feat: create automatically session table with pdo connection~~ [HttpFoundation] Create migration for session table when pdo handler is used Oct 31, 2022

nicolas-grekas reviewed Oct 31, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php Outdated Show resolved Hide resolved

alli83 force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch 2 times, most recently from aad3487 to ee96111 Compare October 31, 2022 20:49

fabpot modified the milestones: 6.2, 6.3 Nov 1, 2022

nicolas-grekas force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch from ee96111 to c312553 Compare November 7, 2022 16:05

nicolas-grekas requested review from wouterj and chalasr as code owners November 7, 2022 16:05

carsonbot added Status: Needs Review and removed Status: Reviewed labels Nov 7, 2022

nicolas-grekas force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch from c312553 to ac62915 Compare November 7, 2022 16:23

nicolas-grekas reviewed Nov 7, 2022

View reviewed changes

alli83 force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch from ac62915 to f95d882 Compare November 10, 2022 14:44

alli83 force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch 3 times, most recently from 3cdc426 to a62248e Compare November 10, 2022 16:23

alli83 commented Nov 21, 2022

View reviewed changes

src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php Show resolved Hide resolved

alli83 force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch from a62248e to 04e7960 Compare December 12, 2022 07:11

chalasr approved these changes Dec 13, 2022

View reviewed changes

src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/DoctrineTransport.php Show resolved Hide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 13, 2022

[HttpFoundation] Create migration for session table when pdo handler …

aebdc58

…is used

nicolas-grekas force-pushed the http-foundation-create-automatically-session-table-with-pdo-connection branch from 04e7960 to aebdc58 Compare December 16, 2022 10:50

nicolas-grekas merged commit 8e5e268 into symfony:6.3 Dec 16, 2022

nicolas-grekas mentioned this pull request Dec 16, 2022

[HttpFoundation] Create migration for session table when pdo handler is… symfony/symfony-docs#17569

Closed

nicolas-grekas reviewed Jan 17, 2023

View reviewed changes

alli83 mentioned this pull request Jan 24, 2023

[HttpFoundation] inject SessionHandler in PdoSessionHandlerSchemaSubscriber #49097

Merged

nicolas-grekas mentioned this pull request Jan 31, 2023

[HttpFoundation] Fix defining expiry index in PdoSessionHandler::configureSchema() #49172

Merged

fabpot mentioned this pull request May 1, 2023

Release v6.3.0-BETA1 #50199

Merged

This was referenced Jun 18, 2023

[DoctrineBridge] add missing UPGRADE notes for #50689 and update tests #50699

Merged

[Cache][DoctrineBridge][Lock][Messenger] Add parameter $isSameDatabase to configureSchema() methods #50689

Merged

alli83 added a commit to alli83/symfony that referenced this pull request Jun 26, 2023

[DoctrineBridge] add missing changelog mention for isSameDatabase arg…

06af15b

… introduced in symfony#48059

alli83 mentioned this pull request Jun 26, 2023

[DoctrineBridge] add missing changelog mention for isSameDatabase arg… #50784

Merged

alli83 added a commit to alli83/symfony that referenced this pull request Jun 26, 2023

[DoctrineBridge] add missing changelog mention for isSameDatabase arg…

208861a

… introduced in symfony#48059

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HttpFoundation] Create migration for session table when pdo handler is used #48059

[HttpFoundation] Create migration for session table when pdo handler is used #48059

alli83 commented Oct 31, 2022 •

edited by chalasr

ostrolucky left a comment •

edited

nicolas-grekas left a comment

nicolas-grekas Nov 7, 2022 •

edited

alli83 Nov 8, 2022

chalasr left a comment

nicolas-grekas commented Dec 16, 2022

nicolas-grekas left a comment

[HttpFoundation] Create migration for session table when pdo handler is used #48059

[HttpFoundation] Create migration for session table when pdo handler is used #48059

Conversation

alli83 commented Oct 31, 2022 • edited by chalasr

ostrolucky left a comment • edited

Choose a reason for hiding this comment

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas Nov 7, 2022 • edited

Choose a reason for hiding this comment

alli83 Nov 8, 2022

Choose a reason for hiding this comment

chalasr left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Dec 16, 2022

nicolas-grekas left a comment

Choose a reason for hiding this comment

alli83 commented Oct 31, 2022 •

edited by chalasr

ostrolucky left a comment •

edited

nicolas-grekas Nov 7, 2022 •

edited