Example invoice coud not be parsed

[uwemock]

While implementing in Jes, I used the example invoices from OpenXRechnungToolbox for my tests. Most of them work well, but I got a ParseException for 04.01a-INVOICE_ubl.xml.

Could you check what's wrong?

[J-N-K]

This is because the TransactionCalculator does not take the prepaid amount into consideration when calculating the grand total (in fact we don't even parse that value).

[uwemock]

I don't understand why this has to be calculated. The "grand total" is included in the TaxInclusiveAmount field.

[J-N-K]

The parser does some kind of validation by calculating the total from the content and comparing that to the parsed value. If these values do not match, a ParseException is thrown because the invoice could not be reproduced.

[uwemock]

I understand what is happening - but why? I did not ask for validation. A ParseException would mean to me that something is wrong with the document structure so that the file contents could not be split up into its components. But the file seems to be well-formed and even valid XML. The actual problem is a semantic problem. And the library decides that this turns the file unusable. When I just want to extract a single piece of data from the data set, there is no need for validation.

[J-N-K]

Agreed. There should either be a full validation or none at all. A simple "amount looks good" check doesn't help.

The failed validation actually is a bug that I fixed in the attached PR.

[uwemock]

No, #477 does not fix this problem.

[J-N-K]

The test properly imports the invoice. What is happening in your setup?

[uwemock]

Sorry, I only saw the changed JavaDoc and wondered how new JavaDoc could fix a bug. In your changes I found that prepaidNodes has a length.When this is positive, your code just picks the first item from all items in prepaidNodes. I don't know much about the other details in the code, but what I see it looks odd to me. Could you check that?

[J-N-K]

Looks strange, indeed. However, only zero or one PrepaidAmount nodes are allowed:

   <xsd:complexType name="MonetaryTotalType">
      <xsd:sequence>
         <xsd:element ref="cbc:LineExtensionAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:TaxExclusiveAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:TaxInclusiveAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:AllowanceTotalAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:ChargeTotalAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:PrepaidAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:PayableRoundingAmount" minOccurs="0" maxOccurs="1"/>
         <xsd:element ref="cbc:PayableAmount" minOccurs="1" maxOccurs="1"/>
         <xsd:element ref="cbc:PayableAlternativeAmount" minOccurs="0" maxOccurs="1"/>
      </xsd:sequence>
   </xsd:complexType>

So either the XML is invalid or the prepaidNodes list has either zero or one element.

That there is a list at all is a result of the way in which the nodes are extracted from the XML (and I decided to keep the same style as it is done for PayableAmount which is required to exist exactly once).

[uwemock]

I understand your intention, but the clean coder inside me says that the code should in some way tell the specs. If the grammar changes some day to allow multiple items in prepaidNodes, the code will fail in the wrong place.

My suggestion would be to change the code to

if (prepaidNodes.getLength() == 1) {
    zpp.setTotalPrepaidAmount(new BigDecimal(prepaidNodes.item(0).getTextContent()));
} else if(prepaidNodes.getLength() > 1) {
    throw new ParseException("Only one prepaid item allowed");
}

This makes clear that the code uses constraints from the specs. The code will throw a suitable exception when multiple items are found. When multiple items will be allowed one day, the code will fail at the point where it needs to be changed.