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

Add event manager to session for it is needed. #6818

Closed
wants to merge 1 commit into from
Closed

Add event manager to session for it is needed. #6818

wants to merge 1 commit into from

Conversation

harikt
Copy link
Contributor

@harikt harikt commented Oct 29, 2014

When downloading zend-session and trying to use it as standalone with the script

<?php
require __DIR__ . '/vendor/autoload.php';

use Zend\Session\Config\StandardConfig;
use Zend\Session\SessionManager;
use Zend\Session\Container;

$config = new StandardConfig();
$config->setOptions(array(
    'remember_me_seconds' => 3600,
    'name'                => 'zf2',
));
$manager = new SessionManager($config);

$manager->start();

$container = new Container('namespace');
if (isset($container->item)) {
    echo $container->item . ' Session is set ';
} else {
    $container->item = 'foo';
}

throws

php -S localhost:8000 session.php 
PHP 5.5.9-1ubuntu4.4 Development Server started at Wed Oct 29 14:35:17 2014
Listening on http://localhost:8000
Document root is /var/www/github.com/harikt/zf2
Press Ctrl-C to quit.
[Wed Oct 29 14:35:25 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19
[Wed Oct 29 14:35:25 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19
[Wed Oct 29 14:35:25 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19
[Wed Oct 29 14:35:26 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19
[Wed Oct 29 14:35:27 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19
[Wed Oct 29 14:35:28 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19
[Wed Oct 29 14:35:28 2014] PHP Fatal error:  Class 'Zend\EventManager\EventManager' not found in /var/www/github.com/harikt/zf2/vendor/zendframework/zend-session/Zend/Session/ValidatorChain.php on line 19

I struggled to understand the documentation, may be better usage of standalone should be promoted.

Thanks

@harikt
Copy link
Contributor Author

harikt commented Nov 5, 2014

Wonder why the PR's taking so long? Zendcon ?

@carnage
Copy link
Contributor

carnage commented Nov 6, 2014

I'm not quite sure why the session validator uses the event manager; forms don't use it for validation.

@harikt
Copy link
Contributor Author

harikt commented Nov 14, 2014

@Ocramius ping?

@gianarb
Copy link
Contributor

gianarb commented Nov 14, 2014

@harikt validation into Zend\Session is manage with events
$session->isValid() trigger an event and the single validator is an event
https://github.com/zendframework/zf2/blob/master/library/Zend/Session/SessionManager.php#L334

The question is, why eventmanager is a suggested dependency and not a required dep?
I don't know but this question is similar for example in this issue #6667

Ocramius added a commit that referenced this pull request Nov 14, 2014
@Ocramius Ocramius closed this in 8389696 Nov 14, 2014
@harikt
Copy link
Contributor Author

harikt commented Nov 15, 2014

I really don't know how things works here.

I assume this PR #6818 being with fixed with #6877 . Isn't this itself is a PR ?

And PR 6877 have not removed the suggest for it is already a required dependency.

Anyway Thanks!

@harikt harikt deleted the patch-1 branch November 15, 2014 01:36
@Ocramius
Copy link
Member

@harikt I don't know why #6877 was opened in first place: sorry for implicitly superseding this one, I just happened to have some free minutes and got a notification about that one. I assume @gianarb mistakenly took this for an issue, not for a pr.

@Ocramius Ocramius self-assigned this Nov 15, 2014
@Ocramius Ocramius added this to the 2.3.4 milestone Nov 15, 2014
@harikt
Copy link
Contributor Author

harikt commented Nov 15, 2014

@Ocramius it will be nice to understand the issue than merging a PR which mentions it fixes #issue-number . Frankly I expect a bit more review before anything being merged like this.

@Ocramius
Copy link
Member

Frankly I expect a bit more review before anything being merged like this.

Indeed that is because I'm trying to get as much as possible done while I have time for it, so there will be mistakes, and I rely on the community to report those (like in this particular case).

To be specific, what happened is that I put some trust on @gianarb because I know him personally, which is indeed my mistake, so I also didn't go and check the issue myself, but simply limited myself to reviewing and merging.

I'm sorry if this has caused any disappointment, please do tell if you feel like I should revert the merge and merge this PR instead, I think that correct attribution is very relevant.

@gianarb
Copy link
Contributor

gianarb commented Nov 17, 2014

@Ocramius @harikt Sorry for this error

@Ocramius
Copy link
Member

@gianarb happens, it was also my fault: I just want a clean resolution in the end.

@gianarb
Copy link
Contributor

gianarb commented Nov 17, 2014

@Ocramius maybe is your fault but I'm crazy

@harikt
Copy link
Contributor Author

harikt commented Nov 17, 2014

@Ocramius only thing I felt sad was I did pinged you on irc. Anyway the disappointment was only for a short time . So now I don't feel any problem. Anyway you could not merge this PR for I deleted the entire repo :) .

@Ocramius
Copy link
Member

Anyway you could not merge this PR for I deleted the entire repo :)

I still have references to everything :-)

@harikt
Copy link
Contributor Author

harikt commented Nov 17, 2014

no problem. Let us leave it.

Thanks!

gianarb pushed a commit to zendframework/zend-session that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-session that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-session that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants