search

magento-hackathon / Responsive-Email-Templates-with-CSS-Inliner

Idea from Kalen Jordan. We can take http://zurb.com/ink/ as a base.
https://github.com/orgs/magento-hackathon
31 stars 10 forks source link

Solving rewrite conflicts with php 5.4 trait #6

Closed SchumacherFM closed 10 years ago

SchumacherFM commented 10 years ago

Assumption: magento-hackathon/Responsive-Email-Templates-with-CSS-Inliner is installed and also one of the Ebizmarts or Aschroder extension

This is a POC to make sure that your rewrite of Mage_Core_Model_Email_Template will always rewrite that class independently of the module loading order.

I do not expect that this will be merged. As usually Magento runs on PHP 5.3 ;-) and traits are introduced in PHP 5.4.0.

The rewrite is done via an observer. This solves the problem of a rewrite of a rewrite.

With this "pattern" neither Magicento nor n98-magerun will show you a rewrite conflict.

So use this mainly in your own local codepool where you know that the rewrite will happen via an observer.

The following example is too weird but it could be a real life one.

$  php n98-magerun.phar dev:module:rewrite:conflicts
┌────────┬──────────────────────────────┬────────────────────────────────────────────────────────────┬────────────────────────────────────────────────────────────┐
│models  │core/email_template           │Aschroder_SMTPPro_Model_Email_Template,                     │Zookal_Core_Model_Email_Template                            │
│        │                              │Ebizmarts_Mandrill_Model_Email_Template,                    │                                                            │
│        │                              │Hackathon_ResponsiveEmail_Model_Email_Template,             │                                                            │
│        │                              │Zookal_Core_Model_Email_Template                            │                                                            │
└────────┴──────────────────────────────┴────────────────────────────────────────────────────────────┴────────────────────────────────────────────────────────────┘
1 conflict was found!

But what if your local extension starts with the letter B instead of Z?

Magicento and n98 will of course still show rewrite conflicts but with the observer you can create a new local class (like Zookal_Core_Model_Email_Template) which solves all the rewrite issues and extend the community classes from each other (if no conflicting methods are available)

If Hackathon_ResponsiveEmail would use the observer rewrite pattern ... then module loading order must be considered.

There is also a tiny performance decrease with observer.

Any thoughts?

kalenjordan commented 10 years ago

Thanks Cyril!

So the way I usually handle it is if I have a rewrite in the local code pool then I already know which community modules I have so I can use <depends> to control load order. And in the event of community extensions, I think that the community extensions should add rewrite conflict support for extensions that they load after alphabetically.

But ya this will be a great solution once 5.4 is more wide spread!

Thanks!

fooman commented 10 years ago

One other approach I have learned from Vinai to create a dynamic rewrite. The long and short of it is to observe controller_front_init_before and then use Mage::getConfig()->setNode() to set the rewrite.

For details see the exercise for Meeting 2 from the Group Study Guide

SchumacherFM commented 10 years ago

@fooman have you had a look at my code? there is this event implemented.

Didn't know that the solution was/is in the Group Study Guide

fooman commented 10 years ago

Sorry actually only got through to the trait 5.4 requirements when posting. Traits are nice and looking forward to them being widely available. In theory I guess if one wanted to put in the effort one could solve it without them or just get Magento to include an event already!

SchumacherFM commented 10 years ago

Traits are a replacement for each time you think on copy&paste a piece of code ;-)

svenvarkel commented 10 years ago

Hi,

Regarding traits - have you seen this? http://measure9.varkel.net/2013/09/resolving-magento-extension-rewrite-conflicts-using-traits/

SchumacherFM commented 10 years ago

@svenvarkel Thanks! Haven't seen yet.

That would solve the problem with core/email_template and that many rewrites.

But the getInstance() method could become dangerous ... loading too much things, etc

Would be easier if a trait could extend from the first parent class ;-)

kalenjordan commented 10 years ago

:thumbsup:

Flyingmana commented 10 years ago

just for a note, php 5.3 is not supported anymore since half a year, since then security patches came in, which did not get backportet to 5.3 Everyone who uses 5.3 uses outdated insecure software

kalenjordan commented 10 years ago

@Flyingmana are you saying 5.3 is vulnerable to those 5.4 security patches?

Looping in @thebod - you have any thoughts on this?

melvyn-sopacua commented 10 years ago

On 05/07/2014 03:55 PM, Daniel Fahlke wrote:

just for a note, php 5.3 is not supported anymore since half a year, since then security patches came in, which did not get backportet to 5.3 Everyone who uses 5.3 uses outdated insecure software

You're confusing a few things:

The three are not related.

Met vriendelijk groeten / Best regards,

Melvyn Sopacua Senior Support Engineer SupportDesk B.V.

kalenjordan commented 10 years ago

@melvyn-sopacua when exactly was/is PHP 5.3 EOL?

melvyn-sopacua commented 10 years ago

On 05/07/2014 05:30 PM, kalenjordan wrote:

@melvyn-sopacua when exactly was/is PHP 5.3 EOL?

Ah, seems like we're not there just yet.

11th of July 2014 it will be [1]. Until then only critical security of fixes of which there has been one in December last year.

[1] Source: http://php.net/archive/2013.php#id2013-07-11-1

Met vriendelijk groeten / Best regards,

Melvyn Sopacua Senior Support Engineer SupportDesk B.V.