Fix issue in loading overdue.xml that uses totalUnpaidInvoiceBalanceEqualsOrExceeds condition

[reshmabidikar]

There is an issue in loading an overdue.xml file that has a totalUnpaidInvoiceBalanceEqualsOrExceeds condition. The problematic overdue.xml is attached. bad_overdue.zip

Steps to reproduce:

1. Create a new tenant
2. Upload attached overdue.xml
3. Click the + sign in the Overdue Show tab to upload overdue configuration again
4. This results in an Error while communicating with the Kill Bill server: error in Kaui and a NullPointerException in Kill Bill logs.

[xsalefter]

This is the stack trace log (not pasting here for readability).

The log says that: .... at org.killbill.billing.jaxrs.json.OverdueConditionJson.<init>(OverdueConditionJson.java:60) .... , it is the line where we creating new instance of DurationJson in OverdueConditionJson :

// See OverdueConditionJson line 60
this.timeSinceEarliestUnpaidInvoiceEqualsOrExceeds = new DurationJson(overdueCondition.getTimeSinceEarliestUnpaidInvoiceEqualsOrExceeds());

There's no <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> in BadOverdue.json, thus overdueCondition.getTimeSinceEarliestUnpaidInvoiceEqualsOrExceeds() will return null.

Furthermore, in stacktrace .... at org.killbill.billing.jaxrs.json.CatalogJson$DurationJson.<init>(CatalogJson.java:1075) .... explained that we calling Duration's method where the Duration parameter in DurationJson constructor is null:

// See CatalogJson line 1075
public DurationJson(final Duration duration) {
  this(duration.getUnit(), duration.getNumber());
}

Actually, when you doing point 2 (Uploading BadOverdue.xml), the file was uploaded, but throw NullPointerException immediately when marshalling (Bad) OverdueConfig into JSON.

So, + sign clicked or not, NullPointerException still thrown when loading the page, because of a problem I explained above.

I wrote several test cases for this:

• Test that simulate BadOverdue.xml .

• There's another test that if I adding <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> element, no NullPointerException and the test passed.

• I also add another test that proof <totalUnpaidInvoiceBalanceEqualsOrExceeds /> is not causing NullPointerException problem, as long as <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> exist.

However, I'm not proposed any fix/suggestion yet. Adding default value only when marshaling to JSON could lead to confusion. I think the best way is either:

• Set default value after unmarshalling XML to Java
• Or validate after unmarshalling XML to Java

[reshmabidikar]

Hi @xsalefter, let me try to add some context here. The overdue.xml without the <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> tag is not really a "bad" file, I apologize for the incorrect name in the original issue. <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> is one of the overdue conditions which has a duration element (unit and days). Similarly, there are other overdue conditions like <totalUnpaidInvoiceBalanceEqualsOrExceeds> etc. You can take a look at the overdue documentation to know more about the various overdue conditions. None of the conditions are mandatory, so it is not mandatory to include the <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> condition. The code however assumes that the <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> condition is always specified and tries to construct the OverdueConditionJson using overdueCondition.getTimeSinceEarliestUnpaidInvoiceEqualsOrExceeds() which results in a NullPointerException. Just adding this context since from your tests it appears that you think the overdue.xml without a <timeSinceEarliestUnpaidInvoiceEqualsOrExceeds /> condition is invalid.

[xsalefter]

Hi @reshmabidikar Thanks!