Skip to content

Commit

Permalink
Compatibility with latest versions of openmage (#187)
Browse files Browse the repository at this point in the history 
* Remove submodules

* Do not rewrite (ref: OpenMage/magento-lts#1513)

Co-authored-by: Justin Beaty <justin.beaty@gmail.com>
  • Loading branch information
@fballiano @justinbeaty
fballiano and justinbeaty authored Aug 3, 2022
1 parent 08025ef commit bc89343
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 23 deletions.
6 changes: 0 additions & 6 deletions .gitmodules
View file
Original file line number Diff line number Diff line change
@@ -1,6 +0,0 @@
[submodule "app/code/local/Cm/RedisSession/lib"]
path = app/code/local/Cm/RedisSession/lib
url = https://github.com/colinmollenhour/php-redis-session-abstract
[submodule "app/code/local/Credis"]
path = app/code/local/Credis
url = https://github.com/colinmollenhour/credis
10 changes: 0 additions & 10 deletions app/code/local/Cm/RedisSession/Model/Session.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,6 @@
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

// Modman install will have files in module directory (submodule) and composer install
// will have files in vendor directory with autoloader registered
if (is_dir(__DIR__.'/../lib/src/Cm/RedisSession')) {
require_once __DIR__.'/../lib/src/Cm/RedisSession/Handler/ConfigInterface.php';
require_once __DIR__.'/../lib/src/Cm/RedisSession/Handler/LoggerInterface.php';
require_once __DIR__.'/../lib/src/Cm/RedisSession/Handler.php';
require_once __DIR__.'/../lib/src/Cm/RedisSession/ConnectionFailedException.php';
require_once __DIR__.'/../lib/src/Cm/RedisSession/ConcurrentConnectionsExceededException.php';
}

class Cm_RedisSession_Model_Session implements \Zend_Session_SaveHandler_Interface
{

Expand Down
8 changes: 3 additions & 5 deletions app/code/local/Cm/RedisSession/etc/config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
</modules>
<global>
<models>
<core_resource>

This comment has been minimized.

Sign in to view

Copy link
@alexherbs

alexherbs Feb 19, 2023 •
edited
Loading

Contributor

Hi @fballiano I'm using OpenMage v19.4.23 at the moment and currently working to update it to PHP 8 and resulted in updating all my vendors files and also this Cm_RedisSession module to v3.0.1. But the problem now is, that this module doesn't work for 19.4.* versions because this rewrite is missing. Wouldn't it make sense to keep this rewrite because there are no bigger sideeffekts or consider the code (from https://github.com/OpenMage/magento-lts/pull/1513/files) below also for 19.4.* versions? I hope you understand what I mean. Best regards, Alexander

app/code/core/Mage/Core/Model/Session/Abstract/Varien.php::start($sessionName = null)
case 'redis': /* @var Cm_RedisSession_Model_Session $sessionResource */ $sessionResource = Mage::getSingleton('cm_redissession/session'); $sessionResource->setSaveHandler(); break;

This comment has been minimized.

Sign in to view

Copy link
@colinmollenhour

colinmollenhour Feb 20, 2023

Owner

So you suggest adding back these deleted lines and keeping the added lines so that the backwards compatibility remains? Are there any downsides to this? I'm not familiar with Amasty_Fpc or other extensions which may conflict.

This comment has been minimized.

Sign in to view

Copy link
@alexherbs

alexherbs Feb 20, 2023

Contributor

@colinmollenhour Yes, this is one solution. But there is also one other solution possible, to add a new "redis" to the app/code/core/Mage/Core/Model/Session/Abstract/Varien.php::start($sessionName = null) function. But this has @fballiano to decide, which one is better. Maybe there is even a better solution. Thanks in advance to everyone.

This comment has been minimized.

Sign in to view

Copy link
@fballiano

fballiano Feb 20, 2023

Author Contributor

I think I would prefer (but it's just my personal opinion) if it could be fixed in the module instead of in the core :-)

This comment has been minimized.

Sign in to view

Copy link
@colinmollenhour

colinmollenhour Feb 20, 2023

Owner

Are you saying to backport OpenMage/magento-lts#1513 into v19?

This comment has been minimized.

Sign in to view

Copy link
@alexherbs

alexherbs Feb 21, 2023

Contributor

@fballiano Just one question, what are your worries about adding "redis" as an entry in the Core session selection switch statement? Its also added in Openmage 20.0, then why not also in 19.4.*? Thanks.

This comment has been minimized.

Sign in to view

Copy link
@fballiano

fballiano Feb 21, 2023

Author Contributor

@colinmollenhour @alexherbs we could backport OpenMage/magento-lts#1513 to v19 actually, I thought that rewrite could be added to the module itself instead of the core, which would make it easier since the module development is faster than a core release :-)

This comment has been minimized.

Sign in to view

Copy link
@colinmollenhour

colinmollenhour Feb 21, 2023

Owner

I think core or module are fine, also local.xml as an immediate fix. I can't do it at the moment.

This comment has been minimized.

Sign in to view

Copy link
@alexherbs

alexherbs Feb 22, 2023

Contributor

@colinmollenhour @fballiano Okay it sounds like we don't really have a decision yet. I don't mind too what way we choose. @colinmollenhour Would you like to change (just adding the rewrite) it in the module then?

This comment has been minimized.

Sign in to view

Copy link
@fballiano

fballiano Feb 22, 2023

Author Contributor

@alexherbs you can open a PR on @colinmollenhour's repo if you want, it should be quite easy in this case :-)

This comment has been minimized.

Sign in to view

Copy link
@colinmollenhour

colinmollenhour Feb 22, 2023

Owner

PR #190 merged, thanks!
<rewrite>
<session>Cm_RedisSession_Model_Session</session>
</rewrite>
</core_resource>
<cm_redissession>
<class>Cm_RedisSession_Model</class>
</cm_redissession>
</models>
</global>
</config>
1 change: 0 additions & 1 deletion app/code/local/Cm/RedisSession/lib
Submodule lib deleted from 4cb15d
1 change: 0 additions & 1 deletion app/code/local/Credis
Submodule Credis deleted from 049ccf

0 comments on commit bc89343

Please sign in to comment.