Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XMLInvoice: gültige namespaces aus der xml holen ... #419

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wernerhahn
Copy link
Contributor

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

Hallo Herr Hahn,
ich gehe Ihre Anliegen mal Punkt für Punkt durch:

„Bei meinem Kunden xxx funktioniert seit Herbst nicht mehr der ZUGfERD Import in das Warenwirtschaftssystem kivitendo.“
    Ich habe einen kurzen Blick auf den Quellcode von kivitendo und dessen Online-Demo geworfen. Die vollständige Fehlermeldung lautet: Fehler bei //ram:DuePayableAmount :XPath error : Undefined namespace prefix
    Das (Und der Code in CrossIndustryInvoice.pm) deuten darauf hin, dass diese Software feste Namespace-Präfixe erwartet. Feste Namespace-Präfixe zu erwarten, ist schlicht nicht ZUGFeRD-kompatibel, weil Namespace-Präfixe im Ermessen des Erstellers liegen. Siehe dazu meinen Abschnitt zu XML-Namespace-Präfixes am Ende. 
„Der Onlinevalidator zeigt https://erechnungs-validator.de/ ‚Ungültige Anfrage‘ “
    Ich bekomme da ein etwas anderes Ergebnis („Keine gültige ZUGFeRD-Datei.“). Leider findet man im Internet immer mal Validatoren, die nicht korrekt validieren, entweder, weil sie noch nicht auf neuere ZUGFeRD-Versionen ausgelegt sind, oder, weil sie Fehler wie den mit den Namespace-Präfixen machen.
    Als Gegenbeispiel: Der Validator des Mustang-Projekts (https://www.mustangproject.org/kommandozeile/?lang=de#validate) validiert unsere Rechnungen einwandfrei

Ein kleiner Exkurs zu XML Namespaces und Präfixes: XML-Namespace-Präfixe können vom Ersteller des XML beliebig gewählt werden. Sie tragen grundsätzlich keine spezielle Bedeutung. Sie werden über eine 'xmlns'-Deklaration einer URI zugeordnet und diese URI ist das, was übereinstimmen muss.
Z.B. sind diese beiden XML-Dokumente inhaltlich absolut gleich:

<h:dokument xmlns:h="http://www.example.org/pfad/">
  <h:element>Text</h:element>
</h:dokument>
<ns1:dokument xmlns:ns1="http://www.example.org/pfad/">
  <ns1:element>Text</ns1:element>
</ns1:dokument>

Dies ist in https://www.w3.org/TR/REC-xml-names/ festgelegt.
Für Perl gibt es dafür ein entsprechendes Tutorial: http://grantm.github.io/perl-libxml-by-example/namespaces.html. Offenbar ist wichtig, einen XPathContext zu verwenden und die Namespace-Präfixe, die auf Seiten des XPath-Ausdrucks verwendet werden, mit den offiziellen URIs per registerNs zu verknüpfen. In SL/ZUGFeRD.pm z.B. wird das bereits gemacht.

Die namespaces CrossIndustryInvoice, ReusableAggregateBusinessInformationEntity, UnqualifiedDataType
können beliebig sein und sind von ZUGfERD nicht festgelegt.
Deswegen werden die ns jetzt vorher ausgelesen.
@wernerhahn wernerhahn requested a review from sschoeling January 11, 2025 17:41
@gftyftffgvgh
Copy link

Всем привет,все очень нравится

@gftyftffgvgh
Copy link

Всем привет все очень нравится

my ($self, $dom) = @_;
my $rootnode = $dom->documentElement;
my @nodes = $rootnode->findnodes('namespace::*');
my @namespaces = map {[ $_->getData, $_->getLocalName]} @nodes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Das hier sieht falsch aus. Müsste auch ein hash zurückgegeben werden wie in der anderen Datei oder?

Copy link
Member

@sschoeling sschoeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Du machst das vor dem bless (also einfach 2 Zeilen hochschieben).
  2. Du änderst die sub namespaces so, 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 der SL::XMLInvoice der 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 vor bless $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'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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';
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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';
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants