colinmollenhour / Cm_RedisSession

Redis-based session handler for Magento with optimistic locking
208 stars 121 forks source link

3.0.1 can not load the session model using composer #189

Closed speller closed 2 years ago

speller commented 2 years ago

When I try to use the class:

<?php

include_once 'vendor/autoload.php';

//$c = new \Cm\RedisSession\Model\Session();
$c = new Cm_RedisSession_Model_Session();

In both cases I'm getting either

PHP Fatal error:  Uncaught Error: Class 'Cm\RedisSession\Model\Session' not found in /magento/1.php:5
Stack trace:
#0 {main}
  thrown in /magento/1.php on line 5

or

PHP Fatal error:  Uncaught Error: Class 'Cm_RedisSession_Model_Session' not found in /magento/1.php:6
Stack trace:
#0 {main}
  thrown in /magento/1.php on line 6

Composer version 2.2.17, PHP 7.1

speller commented 2 years ago

I guess the issue is in the absence of the autoload section in the composer.json file.

colinmollenhour commented 2 years ago

Thanks @speller - @justinbeaty do you have any additional insight on this, it does appear that an autoload declaration is needed but not sure what your plans were for it exactly.

justinbeaty commented 2 years ago

I guess it could be added, but I don't typically see Magento modules doing this.

If the module is installed with Cotya/magento-composer-installer (or modman, etc) then it is placed into app/code/community and Magento will find the class. For example:

<?php
define('MAGENTO_ROOT', '/home/www-data/magento-root/htdocs');

require MAGENTO_ROOT . '/app/bootstrap.php';
require_once MAGENTO_ROOT . '/app/Mage.php';

Mage::app();

$c = new Cm_RedisSession_Model_Session();

I think adding an autoloader would be equivalent to adding one to colinmollenhour/Cm_OrderProducts so someone could call new Cm_OrderProducts_Model_System_Config_Source_Filter(). But what is the point? Unless I am missing something...

Edit: So I think if there's a use-case for a separate entry point like in the OP's 1.php, why not just bootstrap Magento? Even if we did add an autoload entry I don't think any of the other Mage_ classes would work.

speller commented 2 years ago

In my case, I'm not using any module managers and use a customized Magento 1 instance that can load backend-only modules directly from the composer directory without copying or symlinking. So I need this package classes to be available for the composer autoloader.

The Cm_Cache_Backend_Redis module has the autoloader section and it works fine.

justinbeaty commented 2 years ago

Interesting; I’m surprised that works since this module has calls to Mage::app(). But in any case, I don’t think adding an autoload entry would negatively affect an install into OpenMage. If you want to make a PR it sounds like Colin would merge it. I would just double check that having the autoload entry still allows one to override the class via Magento‘s config.xml (which I can also test later today).

justinbeaty commented 2 years ago

I added an autoload entry in my fork and installed via composer https://github.com/justinbeaty/Cm_RedisSession/commit/2527c53764b67f7b7681bab3737ec1f184698586

The results are that the class can now be loaded with:

<?php

include_once 'vendor/autoload.php';

$c = new \Cm\RedisSession\Model\Session();

But this throws a new error:

$ php test.php 
PHP Fatal error:  Uncaught Error: Interface 'Zend_Session_SaveHandler_Interface' not found in /home/www-data/magento-root/htdocs/vendor/colinmollenhour/magento-redis-session/app/code/community/Cm/RedisSession/Model/Session.php:32
Stack trace:
#0 /home/www-data/magento-root/htdocs/vendor/composer/ClassLoader.php(478): include()
#1 /home/www-data/magento-root/htdocs/vendor/composer/ClassLoader.php(346): Composer\Autoload\includeFile()
#2 [internal function]: Composer\Autoload\ClassLoader->loadClass()
#3 /home/www-data/magento-root/htdocs/test.php(5): spl_autoload_call()
#4 {main}
  thrown in /home/www-data/magento-root/htdocs/vendor/colinmollenhour/magento-redis-session/app/code/community/Cm/RedisSession/Model/Session.php on line 32

Which is not surprising because the Zend classes do not have autoload entries... Maybe in your customized Magento 1 fork you have autoload entries for those classes too.

So overall I'm not sure this is needed for this repo. Alternatively, could you just define the autoload entry in your own composer.json?

"psr-0": {
    "Cm\\RedisSession\\": "vendor/colinmollenhour/magento-redis-session/app/code/community/"
}
speller commented 2 years ago

But this throws a new error:

The Cm_Cache_Backend_Redis module has the same issue but it works fine in my case.

So overall I'm not sure this is needed for this repo. Alternatively, could you just define the autoload entry in your own composer.json?

This is a working option, thank you. But as this doesn't break existing package functionality, could you add it to the package repo, please?

speller commented 2 years ago

I just double-checked the psr-0 solution and found that the $c = new \Cm\RedisSession\Model\Session() triggers a correct autoloading but the class name is still Cm_RedisSession_Model_Session and can't be found by PHP. The proper variant is:

  "autoload": {
    "psr-0": {
      "Cm_RedisSession_": "vendor/colinmollenhour/magento-redis-session/app/code/community/"
    }
  }

This makes the $c = new Cm_RedisSession_Model_Session() code working fine.

Moreover, the Cm\RedisSession\ autoload namespace is already taken by the colinmollenhour/php-redis-session-abstract package.

colinmollenhour commented 2 years ago

An important distinction between Cm_Cache_Backend_Redis and Cm_RedisSession is that the former is just a class that extends Zend_Cache_Backend_Abstract and can be used anywhere and the latter is a Magento module which is designed to have classes loaded via Varien_Autoload. Does your custom Magento setup not use Varien_Autoload?

colinmollenhour commented 2 years ago

Your last example looks fine for your project composer.json file but for a module I don't think it is a proper use of composer so I'd prefer not to commit it to this repo.