Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Destroy should only return false when removal of session truely fails. #66

Merged
merged 27 commits into from
Jun 19, 2017

Conversation

rarog
Copy link
Contributor

@rarog rarog commented Nov 18, 2016

Finally fixes #36

Until this commit, the Cache adapter still fails. This mimics the behaviour of the fix in DbTableGateway.php.

@markushausammann
Copy link

Please merge!

@Ocramius
Copy link
Member

Ocramius commented Feb 7, 2017 via email

@markushausammann
Copy link

@rarog can you provide the tests?

@rarog
Copy link
Contributor Author

rarog commented Feb 8, 2017

Sorry, I had those ready since long time, but never commited them, as I didn't manage to set up a local testing environment.

@rarog
Copy link
Contributor Author

rarog commented Feb 8, 2017

I give up so far, I'm just shooting into the dark, without being sure to hit the target. Can somebody help me resolving the issue with the test? I think I almost got it, but I'm not sure how to work around "expected to be a reference, value given" problem, so the parameter is passed by reference to the mocked function.

@rarog
Copy link
Contributor Author

rarog commented Feb 10, 2017

@Ocramius It seems, I can't mock for the 2nd test properly due to limitations of the framework (phpspec/prophecy#225)

If it is enough, I'd leave the first test in place and remove the 2nd.

Destroy should only return false when removal of session truely fails.
`Cache::getItem()` expects to return the second argument by reference.
Updated the expectation to do this.
@weierophinney
Copy link
Member

@rarog I've pushed a change (f7eac66) that accomplishes what is needed here. Essentially, the cache getItem() method needs the second argument returned by reference. I've done so, and it now passes.

@weierophinney weierophinney merged commit f7eac66 into zendframework:master Jun 19, 2017
weierophinney added a commit that referenced this pull request Jun 19, 2017
weierophinney added a commit that referenced this pull request Jun 19, 2017
weierophinney added a commit that referenced this pull request Jun 19, 2017
@weierophinney
Copy link
Member

Thanks, @rarog

@markushausammann
Copy link

Thanks! Finally we can use redis session caching.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP 7 and redis session caching: Session object destruction failed.
7 participants