Die sieben Todsünden der Magento-Entwicklung Programmier-Praktiken in Magento, welche garantiert zu Problemen führen!

Sebastian Bork | Sonntag, 21 Juni 2015. magento, paktiken, paxistipp

Als Entwickler bekommt man ja nun einiges an ("Fremd"-)Code unter die Augen. Egal, ob von anderen Freiberuflern, engagierten - und wirklich nicht immer untalentierten - Kunden oder auch großen Firmen, welche in diesem Berufszweig ihre ganze Existenz begründen. Besonders in einer so anforderungsreichen Umgebung wie Magento, sollte man heutzutage dann auch meinen, dass sich alle Akteuere an den aktuellen Informationsstand halten.

Denn, hatte man bezüglich Magento vor einigen Jahren (meine erste Version war irgendwas bei 0.9.x Beta) noch arge Probleme qualifizierte Hilfestellung zu erhaschen und war man damls meist schneller den Quellcode dieses üppigen eCommerce-Systems zu durchwühlen, als im offziellen Magento-Forum - ohne Gold-Mitgliedschaft - eine Antwort zu bekommen, so hat sich vorallem hier ja nun wirklich einiges getan. Man könnte es fast schon als Hype bezeichnen; Und es gibt wohl kaum noch ungeklärte Fragen zu diesem System; Wenn man nur sucht.

Traurig ist es also um so mehr, dass ich mich wirklich noch veranlasst fühle diesen Artikel zu schreiben. Denn genau diese hier beschriebenen, eigentlich so essentiellen, Paradigmen, sind genau das was mir auch heute noch viel zu oft als Fehler begegnet!

Also, was sind nun in meinen Augen diese sieben Todsünden der Magento-Entwicklung?

 

1. Du sollst deinen Quellcode nicht in Core-Dateien hacken

Die absolute Top-Platzierung der Programmiersünden in Magento ist zum Glück nicht mehr all zu oft anzutreffen; Sie ist aber ein besonders ärgerlicher Kanditat und - zumindest in meinen Augen - auch absolut unverständlich, da das eigentlich vollkommen selbsterklärend ist!

Niemals, wirklich wirklich nie², sollte man irgend eine Anpassung direkt an einer Core Datei durchführen! Und dazu zählt generell alles, was bei der blanken Install von Magento mitkommt; Und erweitern kann man diese Regel getrost noch um die Definition, "alles was man als Extension von Drittanbietern installiert".

Ansich muss man es nicht erläutern, aber ich mach das trotzdem mal ganz rudimentär: Jeder Entwickler der an dem Shop arbeiten wird und vorallem jede Extension die man hierfür installieren wird, wird davon ausgehen, dass die Kern-Dateien im Originalzustand sind. Ist das nicht der Fall, bekommen die Entwickler graue Haare, der Kunde eine höhere Rechnung und die Extensions schrotten unter Umständen das ganze System. Wen das nicht interessiert, den interessiert vielleicht das banalere Problem: Bei einem Update ist alles weg!

Und nein, auch der bekannte Cache-Verzeichnis-Fehler (behebbar durch manuelles setzen des Verzeichnisses in Zend_Cache_Backend_File) ist hier keine Ausnahme!

 

2. Du sollst den Local-Pool nicht zum Überladen von Core-Dateien verwenden

Zugegeben, hier geht beim Update nichts verloren, ein Entwickler, welcher Magento kennt, wird hier zumindest keine grauen Haare bekommen und der Kunde daher auch nicht unbedingt eine höhere Rechnung; Dennoch kann der Platz 2 sehr gut die selben Probleme (aus den selben Gründen) bei Extensions hervorrufen, wie die Top-Platzierung; Gemeint ist das Überladen von Magentos Core-Dateien mittels des Local-Pools.

Genrell ist es in Magento so, dass das System, bspw. für /app/code/core/ oder auch für /lib, zuerst schaut ob es in /app/code/local ein Pendant gibt. Dadurch ist es z.B. möglich /lib/Zend/Cache/Backend/File.php mittels /app/code/local/Zend/Cache/Backend/File.php zu überladen, so dass die "lokale" (ggf. modifizierte) Version zum Einsatz kommt.

Auch wenn dieses Beispiel jetzt eine der sehr wenigen notwendigen Anwendungen war (siehe letzter Satz aus Punkt 1), ist dieses Prinzip wirklich nur sehr selten zu rechtfertigen. Nämlich nur da, wo keine andere Methode der Modifikation (Observer oder - als Notnagel auch - ein Rewrite) greift. Und das sind letztlich fast nur Klassen aus /lib und ein paar ganz wenige Module (bspw. Mage_Core), welche geldaden sind bevor etwaige Rewrites oder gar Observer greifen.

Falls man jemals zu dieser Methode gezwungen ist, sollte man dann zumindest sich selbst und ggf. zukünftigen Entwicklern einen wirklich essentiellen Gefallen tun: Eine kurze Dokumenttion (txt) im Local-Pool, was wo und warum geändert wurde.

 

3. Du sollst Rewrites mit Bedacht verwenden

Rewrites sind ein Konstrukt, welches eigentlich nur durch seinen viel zu inflationären und oft gedankenlosen Einsatz problematisch ist. Als Entwickler, welcher mit dem finalen Shop betraut ist, kann man hier schon durchaus Rechtfertigungen für die Notwendigkeit finden; Aber - und das möchte ich hier explizit unterstreichen - es ist ein Prinzip, für welches ein Extension-Entwickler (welcher Produkte für den freien Vertrieb entwickelt) eigenlich auf den Scheiterhaufen gehört!

Ansich ist es in Magento also so, dass ich ein Modul entwickeln kann, welches auf einem anderen Modul basiert bzw. dessen Funktionen ergänzt oder ersetzt. Hierfür kann man eben Rewrites einsetzen, so dass mein Modul Abc_Def bspw. mit seinem Block Abc_Def_Block_Product_List_Upsells den Block Catalog_Product_List_Upsells, des Moduls Mage_Catalog überschreibt. Das funktioniert gleichermaßen mit Blöcken, Helpern und Models und ist ansich auch eine wirklich tolle Sache! (Für Controller ist ein anderer Weg erforderlich, aber das Problem bleibt das selbe.)

Der Fallstrick ist: Jede Klasse kann blos ein einziges mal mittels eines Rewrite überschrieben werden. Würde ich also nun mit einem weiteren Modul Ghi_Jkl ebenfalls den Block Catalog_Product_List_Upsells überschreiben, wäre der Rewrite meines ersten Moduls nicht mehr existent.

Für das was nun passiert, wenn drei verschiedene Extension-Entwickler drei verschiedene Module entwickeln, die dann alle drei versuchen diesen einen Block zu überschreiben, braucht man nicht mal hohe Mathematik.

 

4. Du sollst keine manuellen Änderungen an den EAV-Tables durchführen

Ok Ok, das fällt wohl wirklich nur den Masochisten unter meinen Kollegen ein, oder denen, die schon fast mit körperlicher Gewalt dazu gezwungen werden. Aber es findet statt! Und es geht schief!

Strapazieren wir einmal die beliebte Metapher, dass die Datenbank das Gehirn eines Shop-Systems ist, so sind die EAV-Tables unumstritten die Synapsen des Magento-Hirns. Sie enthalten alle im System verwendeten Attribute (und Magento lebt durch seine Attribute) und natürlich alle damit einhergehenden Parameter, Werte und Zuordnungen zwischen Entität (Produkt, Kategorie, etc.) und Wert; Und sie sind - oh Wunder - miteinander verknüpft.

Fängt man nun an, manuell hier und da einen Wert zu ändern oder versucht manuell ein Attribut zu löschen oder ggf. von einem Typ in einen anderen zu wandeln, sollte man bessern ganz genau wissen welche Aktion zu welchen Folgen führt bzw. in welchen Tabellen man diese Folgen zu beheben hat. Und nicht zu letzt hat es in Magento wirklich einen Grund, warum bei manchen Attributen keine nachträgliche Änderungen erlaubt oder bestimmte Attribute als "benötigt" gelten. Macht man hier etwas falsch, ist es sehr Wahrscheinlich, dass man das komplette System verkrüppelt!

Ein beliebter Anwendungsfall ist die Korrektur oder Löschung falsch installierter Attribute, während der Modul-Entwicklung. Bei Schreibfehlern in Label, Notiz oder sogar Attribut-Code mag das manuell noch klargehen. Bei allem anderen sollte man den Weg nutzen, über den das Attribut auch angelegt wurde; Den Installer! Es gibt in Magento 1.x zwar keinen Uninstall oder Rollback per se, aber zu jeder Funktion zum Anlegen/Zuordnen eines Attributs zumindest ein Gegenstück!

 

5. Du sollst dich für Kernfunktionen nicht vom Kern abwenden

Wenn man ein Modul entwickelt, welches Kern-Funktionen in Magento ergänzen soll (Zahlungsmöglichkeiten, Versandarten, etc.), dann kann man in Magento auf hauseigene Möglichkeiten zurückgreifen, welche diese ordnungsgemäß im Kern integrieren. Meist eben einfach über die config.xml des Moduls dem System bekanntgeben.

Andererseits könnte man das natürlich ignorieren und die zahllosen Events der Event-Observer-Architektur von Magento benutzen, um (wir bleiben jetzt einfach mal bei Zahlungschnittstelle ... auch, weil ich genau dort diesen Fall als letztes erlebt habe) die Bezahlmethode unseres Moduls direkt erst an den entsprechenden Stellen einzubeziehen.

Letztlich wäre es für unser Modul das selbe! Es würde im Bezahlprozess auftauchen, es wäre für Rechnungen und Bestellungen sichtbar/verfügbar; Ansich würde es genau so funktionieren, als wären wir den ersten Weg gegangen.

Aber! Was ist, wenn nun ein anderes Modul nach konfigurierten/aktiven Bezahlmethoden fragt? Und das ganz Magento-konform über die übliche Methode Mage::getSingleton('payment/config')->getActiveMethods()? Was dann?

Nix is dann! Sofern wir nicht auch diesen Fall bei unserer "eigenwilligen" Implementierung bedacht haben, wird kein anderes Modul auf diesem Weg von unserer Bezahlmethode erfahren. Ergo, andere Entwickler haben einen erhöhten Aufwand unser Modul mit einzubeziehen (worauf hin sie es ggf. vonvornherein ignorieren) oder sie übersehen es gar komplett.

Man sollte also immer den von Magento vorgesehenen Weg beschreiten und falls dieser als unpraktikabel erscheint, zuerst hinterfragen, ob man nicht selber einen Denkfehler in seiner Architektur hat. Und erst der wirklich allerletzte Ausweg sollte es sein, die vorgegeben Wege zu verlassen; Dann sollte man aber auch alle Eventualitäten mit einbeziehen!

 

6. Du sollst die Models nicht strapazieren

Hier kommen wir zu einer der Programmiersünden, welche potentiell nicht tödlich enden; Also ausser vielleicht für die Performance des Shops. Der ganze Punkt ist bitte als etwas exemplarischer anzusehen. Auf jeden einzelnen möglichen Fall einzugehen, würde den - ohnehin schon üppigen - Rahmen enorm sprengen.

Als aller erstes sei festzuhalten, man läd ein Model grundsätzlich nur einmal, wenn man es öfter braucht!

//falsch
foreach($attributes as $code=>$value) {
  $value = Mage::getModel('catalog/product')->load($id)->getData($code);
}

//nicht besser
$sku = Mage::getModel('catalog/product')->load($id)->getSku();
$name = Mage::getModel('catalog/product')->load($id)->getName();

//richtig
$product = Mage::getModel('catalog/product')->load($id);
$sku = $product->getSku();
$name = $product->getName();

Hinzufügen muss ich hier noch, dass man das auch gern über verschiedene Klassen hinweg beachten darf! Brauchen bspw. mein Controller, mein Helper und einer meiner Blöcke ein und das selbe Model - ja gar die selbe Instanz, sollte man eventuell gleich den Helper dafür verantwortlich machen, so dass auch Block und Controller die selbe Instanz aus dem Helper beziehen; Oder man geht einfach über die gern vernachlässigte (aber, da global einmalig, auch nur bei bestimmten Models zu empfehlende) Methode Mage::getSingleton().

Der letze Hinweis geht in eine sehr ähnliche Richtung: Kenne deine Models. Das verhindert unnötigen Ballast!

//unnötig
$id = Mage::getModel('catalog/product')->loadByAttribute('sku', $sku)->getId();

//richtig
$id = Mage::getModel('catalog/product')->getIdBySku($sku);

Im Grunde soll dieser Punkt hier "nur" sensibilisieren. Die wenigsten kennen alle Funktionen aller Models auswendig - ich auch nicht. Aber wenn man eine bestimmte Lösung vor Augen hat, schadet es nie sich die einzelnen beteiligten Models/Klassen dahingehend kurz anzusehen, ob es nicht vielleicht einen effizienteren Weg gibt; Und vor allem sollte die Frage nach Effizienz und Notwendigkeit in einem so üppigen eCommerce-System wie Magento ein stetiger Begleiter sein!

 

7. Du sollst die Collections nicht strapazieren

Auch bei der letzten der Programmiersünden in Magento geht es in erster Linie wieder um die Performance. Aber, hat man einen großen Datenbestand, können hier genannte Fehler auch schnell den Shop lahm legen, weil evtl. die Skriptlaufzeit überschritten oder ganz einfach der Speicher des Servers unnütz geflutet wird. Auch diese Erläuterungen hier sind bitte eher als exemplarisch anzusehen.

Das aller wichtigste ist: Man sollte wirklich nie² eine Collection ohne Limit bilden! Mag das bei wenigen Produkten/Kategorien ggf. noch klargehen, haben unlimitierte Collections bei großem Datenbestand schon so manchen Server in die Knie gezwungen!

//falsch
$collection = Mage::getModel('...')->getCollection();
return $collection;

//richtig
$collection = Mage::getModel('...')->getCollection();
$collection->setPageSize($limit)->setCurPage(1);
return $collection;

//richtig
$collection = Mage::getModel('...')->getCollection();
$collection->setPage(0, $limit);
return $collection;

//richtig
$collection = Mage::getModel('...')->getCollection();
$collection->getSelect()->limit($limit);
return $collection;

Hierbei ist auch wichtig, zu wissen, dass bestimmte Funktionen nicht automatisch eine Limitierung erzwingen!

//falsch
$collection = Mage::getModel('...')->getCollection();
$collection->getSelect()->order('rand()');
return $collection->getFirstItem();

//richtig
$collection = Mage::getModel('...')->getCollection();
$collection->getSelect()->order('rand()')->limit(1);
return $collection->getFirstItem();

Man sollte natürlich auch darauf achten, wie man mit seine Collections effizient verfährt. Ein entscheidender Fehler wird gern beim Abzählen der Collection gemacht.

//falsch
$collection = Mage::getModel('...')->getCollection();
$collection->getSelect()->limit(100);
$size = count($collection);

//auch falsch
$collection = Mage::getModel('...')->getCollection();
$collection->getSelect()->limit(100);
$size = $collection->count();

//richtig
$collection = Mage::getModel('...')->getCollection();
$collection->getSelect()->limit(100);
$size = $collection->getSize();

Alle drei Wege funktionieren, der gravierende Unterschied ist: Die ersten beiden laden erst die komplette Collection aus der Datenbank, wogegen getSize() wirklich nur die Anzahl der möglichen Datensätze aus der Datenbank holt (nicht die Datensätze selbst). Ein wenig Obacht ist bezüglich getSize() noch zu geben! Sie nutzt u.a. die Funktion getSelectCountSql() der Klasse Varien_Data_Collection_Db. Ansich setzt diese alle gesetzten Limits zurück, wodurch getSize() nicht in jedem Fall verlässlich ist! Aber: Einige Models (bspw. auch Mage_Catalog_Model_Resource_Product_Collection) überschreiben diese Methode intern, so dass getSize() hier verlässlich ist! Einfach mal testen oder Quellcode durchwühlen.

Ein letzter essentieller Punkt bei Collections ist wieder die berühmte Frage: Was brauch ich wirklich alles?! Unnötiger Ballast führt bei Collections zu argen Performance-Einbußen.

//falsch
$collection = Mage::getModel('catalog/product')->getCollection();
$collection->setAttributeToSelect('*');
$collection->getSelect()->limit(1000);

$brands = array();
foreach($collection as $item) {
	$brands[$item->getId()] = $item->getData('brand');
}
return $brands;

//richtig
$collection = Mage::getModel('catalog/product')->getCollection();
$collection->setAttributeToSelect('brand');
$collection->getSelect()->limit(1000);

$brands = array();
foreach($collection as $item) {
	$brands[$item->getId()] = $item->getData('brand');
}
return $brands;

Obiger Code stellt die Frage nach dem "wirklich benötigt" nur hinsichtlich der Attribute, was schon viel ausmacht. Ein noch signifikanteres Ergebnis bekommt man, wenn man diese Frage vorab auch bspw. für Produkt-Typ, Store/Store View, Kategorie und/oder Sichtbarkeit beantworten kann.

Kommentare aktivieren?

Diese Seite verwendet das externe Kommentarsystem DISQUS. Da es durch die Nutzung externer Dienste zwangsläufig dazu kommt, dass Daten des Nutzers an die Server dieses Dienstes gesendet werden, möchte ich jedem Nutzer diese Entscheidung selbst überlassen!.
DISQUS aktivieren