Skip to content

Fix: [Session] the problem of secondary retrieving values ​​in RedisHandler - #7887

Merged
kenjis merged 2 commits into
codeigniter4:developfrom
ping-yee:230901_redis_close
Sep 8, 2023
Merged

kenjis merged 2 commits into
codeigniter4:developfrom
ping-yee:230901_redis_close

Conversation

@ping-yee

Copy link
Copy Markdown
Contributor

Description
See #7695
The close function not only needs to delete the lock from redis but also needs to clear the variable value by using releaseLock function.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Sep 1, 2023
@kenjis

kenjis commented Sep 1, 2023

Copy link
Copy Markdown
Member

Thank you.

Is it impossible to write a test because of the behavior of the session?
How did you confirm the bug has been fixed?

@kenjis kenjis changed the title Fix: Fix the problem of secondary retrieving values ​​in Redis and Session Fix: the problem of secondary retrieving values ​​in Redis and Session Sep 1, 2023
@ping-yee

ping-yee commented Sep 1, 2023

Copy link
Copy Markdown
Contributor Author

I have added the test case.

This bug is that It cannot get the previous session data after calling the close function. (like the issue case)

And it works now.

@kenjis kenjis changed the title Fix: the problem of secondary retrieving values ​​in Redis and Session Fix: [Session] the problem of secondary retrieving values ​​in RedisHandler Sep 1, 2023

@kenjis kenjis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@kenjis
kenjis merged commit eb1d5e0 into codeigniter4:develop Sep 8, 2023
@ping-yee
ping-yee deleted the 230901_redis_close branch September 8, 2023 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants