XMLInvoice: gültige namespaces aus der xml holen ... #419
Conversation
Die namespaces CrossIndustryInvoice, ReusableAggregateBusinessInformationEntity, UnqualifiedDataType können beliebig sein und sind von ZUGfERD nicht festgelegt. Deswegen werden die ns jetzt vorher ausgelesen.
|
Всем привет,все очень нравится |
|
Всем привет все очень нравится |
| my ($self, $dom) = @_; | ||
| my $rootnode = $dom->documentElement; | ||
| my @nodes = $rootnode->findnodes('namespace::*'); | ||
| my @namespaces = map {[ $_->getData, $_->getLocalName]} @nodes; |
There was a problem hiding this comment.
Das hier sieht falsch aus. Müsste auch ein hash zurückgegeben werden wie in der anderen Datei oder?
There was a problem hiding this comment.
Moin Werner.
Wie angefragt, einmal über den Code geschaut. Den offensichtlichen Bug in der namespaces von CrossIndustryDocument hatte ich schon ausserhalb des Reviews angemarkert.
Ansonsten:
Von den kommentierten Sachen ist mir vor allem wichtig zu verstehen wo der Code unten in parse_xml für den fehlenden udt namespace herkommt, der Rest sollte nur Kleinkram sein.
Ansonsten die übliche Wadenbeißerfrage: Tests? Gerade für sowas hier wären 1-2 Regressiontests toll, damit man sehen kann was das eigentlich fixen soll.
| my $namespaces = $self->namespaces($self->{dom}); | ||
|
|
||
| $self->{namespaces} = $namespaces; | ||
|
|
There was a problem hiding this comment.
Hmm. Das hier finde ich unschön.
Das ist ein bisschen akademisch, aber die Struktur hier ist ein Factory Pattern, wo die SL::XMLInvoice für eine Datei den passenden Worker instanziert. Das Instanzieren passiert hier aber nicht über einen Constructor, sondern in dem das hashref $self einfach in das entsprechende package ge-bless-ed wird.
Nach dem bless gilt das Worker-Objekt als fertig, und ab da sollte auch nicht mehr seinen Innereien gewühlt werden, sondern stattdessen mit den Objektmethoden drauf zugegriffen werden.
Möglichkeiten das anders zu machen wären also:
- Du machst das vor dem bless (also einfach 2 Zeilen hochschieben).
- Du änderst die
sub namespacesso, dass sie nicht mehr statisch ist (was sie eh nicht ist, weil sie ja zum Aufrufzeitpunkt schon ein Objekt hat), und lässt sie die namespaces selber in$self->{namespaces}speichern. Dann ist in derSL::XMLInvoiceder Aufruf nur noch$self->namespaces($self->{dom})oder sogar nur$self->namespaces- weil es nicht mehr statisch ist.
In beiden Fällen aber als Idee:
- Hashzugriffe (
$self->{...}) nur vorbless $self, $type - Danach nur Methodenzugriffe (
$self->method())
| taxnumber => ['//' . $ram . 'SellerTradeParty/' . $ram . 'SpecifiedTaxRegistration/' . $ram . 'ID[@schemeID="FC"]'], | ||
| type => ['//' . $rsm . 'HeaderExchangedDocument/' . $ram . 'TypeCode'], | ||
| ustid => ['//' . $ram . 'SellerTradeParty/' . $ram . 'SpecifiedTaxRegistration/' . $ram . 'ID[@schemeID="VA"]'], | ||
| vendor_name => ['//' . $ram . 'SellerTradeParty/' . $ram . 'Name'], |
There was a problem hiding this comment.
Mein Beileid, dass Du den Kram so tippen musstest. Ich habe geschaut ob das irgendwie eleganter geht, habe aber auch nichts gefunden. LibXML unterstützt leider kein XPath 3.0, da hätte es eine Syntax zumindest für namespaces per url gegeben. Optionale namespaces sind aber anscheinend überhaupt nicht unterstützt.
Ich habe aber auch nochmal im Zugferd Standard nachgeschaut, und wenn ich nicht blind bin, sind die namespaces nicht optional. Für mich wäre das also hier auch absolut okay, wenn das konsequent einen Fehler wirft wenn die 3 namespaces nicht gefunden werden.
Ist aber kein Grund das nochmal anzufassen.
| unless ($udt) { | ||
| $value = $self->{dom}->findnodes('//' . $ram . 'DueDateDateTime','DateTimeString') if $key eq 'duedate'; | ||
| $value = $self->{dom}->findnodes('//' . $ram . 'IssueDateTime','DateTimeString') if $key eq 'transdate'; | ||
| } |
There was a problem hiding this comment.
Diesen Block hier verstehe ich nicht.
Das ist in der Schleife für alle scalaren xpaths, und macht dann eine Ausnahme für duedate und transdate wenn der udt namespace nicht gefunden wurde und sucht dann durch das ganze DOM nach den nodes? Gibt es da einen Präzedenzfall? Ist das ein bekannter Bug in einer Datei gewesen?
Bitte zumindest dokumentieren was das sein soll.
| $rsm .= ":" if $rsm; | ||
| $udt .= ":" if $udt; | ||
| return '//' . $ram . 'IncludedSupplyChainTradeLineItem'; | ||
| } |
There was a problem hiding this comment.
Das braucht nur den ram namespace - braucht die anderen beiden hier also nicht reinkopieren.
Wenn Du das so oft kopierst, macht evtl eine Methode Sinn, die den namespace nachschlägt und wenn vorhanden mit : Suffix zurückgibt, oder?
Die namespaces CrossIndustryInvoice, ReusableAggregateBusinessInformationEntity, UnqualifiedDataType können beliebig sein und sind von ZUGfERD nicht festgelegt. Deswegen werden die ns jetzt vorher ausgelesen.
Ich hatte seit Herbst Probleme mit den ZUGfERD Rechnungen von MEMO. Und hatte die mal angeschrieben, da es zuvor ging. Hier die Antwort