Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[HttpFoundation] Fix PdoSessionHandler to work properly with streams#12146

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
rybakit wants to merge5 commits intosymfony:masterfromrybakit:session
Closed

Conversation

@rybakit
Copy link
Contributor

QA
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT

Ref:#10931 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

every method can throw this. so this is not really helpful.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I added it because PhpStorm shows a warning otherwise: "PHPDoc comment doesn't contain all necessary@throws tag(s)". Should I remove it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes because this doesnt only apply to this method.

@Tobion
Copy link
Contributor

Did you test this class with a real sqlite connection? If so could you please also test the blocking withhttps://github.com/Tobion/symfony-standard/tree/session-locking-test ?

@rybakit
Copy link
ContributorAuthor

I didn't test it with a real sqlite connection as I don't use custom session handlers in my project. I've installed yoursession-locking-test branch and was able to read and write session data using/write/foo and/read requests. Is it enough or should I do something else to test blocking?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can you move everything on one line? Same below, thanks.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Done.

@Tobion
Copy link
Contributor

Did you updatehttps://github.com/Tobion/symfony-standard/blob/session-locking-test/app/config/config.yml#L46 to use postgres? You can test blocking by loading two pages/write/foo and/read at the same time. Just refresh write and then read. The read page should block until the write finished (which has a delay of 8 sec). Then do the same test with advisory locks by changinghttps://github.com/Tobion/symfony-standard/blob/session-locking-test/app/config/config.yml#L29 to1. The behavior should be the same as before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

errorMode and driverName can be private

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Done.

@rybakit
Copy link
ContributorAuthor

@Tobion I've testedpgsql driver as you described and it works as expected.

@Tobion
Copy link
Contributor

Cool thanks. And did pdo_pgsql indeed return a stream for the binary field?

@rybakit
Copy link
ContributorAuthor

Yes, and it's mentioned in themanual:

Note:
The bytea fields are returned as streams.

@Tobion
Copy link
Contributor

👍 Thanks for fixing this and doing functional tests.
In case you also have access to oracle db or mssql, it would be much appreciated to do the same tests for them.

@fabpot
Copy link
Member

Thank you@rybakit.

@rybakitrybakit deleted the session branchOctober 7, 2014 18:40
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rybakit@Tobion@fabpot@stof

[8]ページ先頭

©2009-2025 Movatter.jp