From: Sven Schöling <s.schoeling@linet-services.de>
Date: Tue, 8 Jan 2013 15:37:27 +0000 (+0100)
Subject: Stillen Fehler bei cascade-save von one-to-many relations behoben.
X-Git-Tag: release-3.1.0beta1~755
X-Git-Url: http://wagnertech.de/git?a=commitdiff_plain;h=d16fc161b610f2ba59025fee1db94511a0c6c5cd;p=kivitendo-erp.git

Stillen Fehler bei cascade-save von one-to-many relations behoben.

Folgendes Phänomen:

       table X                       table X_items
       id                            X_id references X(id)

wird in Rose zu

SL::DB::X und SL::DB::XItems, wobei SL::DB::XItems::X eine automatische
relationship zu X bildet, und in den meisten Fällen SL::DB::X::items eine
manuelle Relationship in die Gegnrichtung.

Was nun passiert ist, war, dass ein save(cascade => 1) auf X die items nicht
mitegspeichert hat.

Das Problem war, dass unser Hooksystem nicht sichergestellt hat, dass die
überladene save Methode von SL::DB::Object immer das Ergebnis der eigentlichen
Speicherung zurückgeliefert.

Rose::DB::Object selber braucht diesen Rückgabewert nicht, und dokumentiert das
Verhalten auch nur informal. Die von den relations angelegten post-save Hooks
prüfen den aber, und schmeissen bei nichterfolg eine Exception.

Das nächste Problem ist jetzt, dass Rose::DB::Object intern die Fehler nicht
direkt wirft, sondern den letzten Fehler in $self->error speichert, und den
dann einfach wirft. Unser undef der überladenen save Methode wird als Fehler
erkannt, aber weil nie ein Fehler gesetzt wurde, wird effektiv "die undef"
aufgerufen.

Das landet dann als "Died at .../Rose/DB/Object/MakeMethods/Generic.pm line
3741." im eval error von Rose::DB::Object::save. Das gibt das Ganze weiter an
den Rose::DB::Object::Metadata::handle_error, der das wiederum an Carp::croak
weitergibt.

Carp packt das gnaze in eine weitere Lage "Died at" ein, und bubblet das ganze
weiter an unser Hooksystem, wo die Rose::DB::do_transaction den Fehler fängt,
und folgerictig ein rollback triggert.

Jetzt der Trick: Bei Rose ist Rose::DB::Object für die Eskalation zuständig.
Rose::DB::do_transaction beendet nur die Transaktion und sieht zu dass nichts
kaputtgeht, und gibt dann undef zurück. Die Exception ist damit im
Errorattribut der DB Connection versenkt.

Rose::DB::Object umgeht das gleiche Problem indem im Fehlerfall die Exception
gefangen wird, die Transaktion sauber beendet wird, und danach erst der
Metadata::handle_error den Fehler zur weiteren Eskalation bekommt.

Dieser Patch erweitert unser Hooksystem so, dass immer der Rückgabewert des
RDBO::save zurückgegeben wird, was dann den Fehler nicht mehr triggert.

Zusätzlich müssen später noch Exceptions im Hooksystem gefangen werden, und
auch da sauber die Transaktion beendet werden, bevor die gehandhabt werden.
---

diff --git a/SL/DB/Object.pm b/SL/DB/Object.pm
index d1e6cb0aa..d176a1f74 100755
--- a/SL/DB/Object.pm
+++ b/SL/DB/Object.pm
@@ -120,6 +120,8 @@ sub save {
       1;
     };
     SL::DB::Object::Hooks::run_hooks($self, 'after_save', $result);
+
+    return $result;
   };
 
   $self->db->in_transaction ? $worker->() : $self->db->do_transaction($worker);
@@ -140,6 +142,8 @@ sub delete {
       1;
     };
     SL::DB::Object::Hooks::run_hooks($self, 'after_delete', $result);
+
+    return $result;
   };
 
   $self->db->in_transaction ? $worker->() : $self->db->do_transaction($worker);