PUT /api.xro/2.0/CreditNotes hangs indefinitely when LineItems is not an array.

[daveharding]

Firstly, thanks for all your work on this SDK.

SDK you're using (please complete the following information):

• Version 1.4.1, PHP 7.4

Describe the bug When trying to create new Credit Notes via the SDK, we encountered a bug whereby the PUT request would hang until it hit the timeout of 60 seconds. We've discovered why this issue occurs, which is when creating a new instance of \XeroAPI\XeroPHP\Models\Accounting\CreditNote, the setLineItems method expects null, or \XeroAPI\XeroPHP\Models\Accounting\LineItem[] (an array of LineItems). Our mistake was to pass in a single LineItem object here, which caused the outputted JSON to be:

...
"LineItems": {
    "Description": "test",
    "Quantity": 3,
    "UnitAmount": "2.00",
    "TaxType": "OUTPUT2"
},
...

It seems to be that because LineItems is an object in the above example when encoded to JSON, rather than an array, the Xero API hangs and eventually times out, with no exception nor validation error.

To Reproduce Steps to reproduce the behavior:

1. Create a new CreditNote
2. Create a new LineItem
3. Call setLineItems(), passing in the LineItem object.
4. Submit request using the $xeroApi->createCreditNotes() method.
5. Wait for PHP/Apache to time out.

Expected behavior I would expect the SDK to validate that either an array or NULL has been passed to setLineItems(), giving feedback to the developer to then be able to fix their mistake.

Happy to submit a PR for this change if it's something you're open to.

[SidneyAllen]

@daveharding we generate our SDKs from OpenAPI specs, so we can not accept PRs that modify the the AccountingApi and models, but if you want to share an example of how you'd implement this type of validation on CreditNote line items - happy to review it and see if we can apply the change to the templates. This could potentially solve this in all places where lineItems are used.

FYI - I believe line items are required for CreditNotes, Invoices, etc. So, not sure if NULL would be valid.

[daveharding]

@SidneyAllen Agreed, from my understanding there's no sense in having a CreditNote without any line items (as they define what has been credited), so it shouldn't be a nullable property. I think this particular issue could also be caught at the API side with a relevant error message, but as far as updating this SDK goes, I think there's 2 approaches that I would consider taking.

1. Update the setLineItems method to validate that it receives an array.

So the original method:

/**
 * Sets line_items
 *
 * @param \XeroAPI\XeroPHP\Models\Accounting\LineItem[]|null $line_items See Invoice Line Items
 *
 * @return $this
 */
public function setLineItems($line_items)
{
    $this->container['line_items'] = $line_items;

    return $this;
}

could become:

/**
 * Sets line_items
 *
 * @param \XeroAPI\XeroPHP\Models\Accounting\LineItem[] $line_items See Invoice Line Items
 *
 * @return $this
 */
 public function setLineItems($line_items)
 {
     if (is_null($line_items) || !is_array($line_items)) {
         throw new \InvalidArgumentException(
             "Invalid value for 'line_items', must be an array"
         );
     }
     
     $this->container['line_items'] = $line_items;  

     return $this; 
 }

2. More broadly though, I imagine there are more properties of CreditNote (and other properties in other models) that would need similar validation (even if it's just checking for non-nullable properties), so you could implement a method similar to getTypeAllowableValues() that returns a list of nullable properties, e.g. getNullAllowableProperties():

/**
 * Gets properties that are allowed to be null
 *
 * @return string[]
 */
public function getNullAllowableProperties()
{
    // These should probably be constants
    return [
        'property1',
        'property3',
        'property7',
        ...
    ];
}

and then use it in the setLineItems() method:

/**
 * Sets line_items
 *
 * @param \XeroAPI\XeroPHP\Models\Accounting\LineItem[] $line_items See Invoice Line Items
 *
 * @return $this
 */
public function setLineItems($line_items)
{
    // More general check for non-nullable values
    $allowedNullProperties = $this->getNullAllowableProperties();
    if (is_null($line_items) && !in_array('line_items', $allowedNullProperties, true)) {
        throw new \InvalidArgumentException(
            "Invalid value for 'line_items', cannot be null"
        );
    }

    // Specific check for this $line_items property, which must be an array
    if (is_null($line_items) || !is_array($line_items)) {
        throw new \InvalidArgumentException(
            "Invalid value for 'line_items', must be an array"
        );
    }
    
    $this->container['line_items'] = $line_items;  

    return $this; 
}

Hopefully that's helpful (and what you were after), but if not let me know, happy to take this further if it'd be helpful in improving the SDK.

[SidneyAllen]

@daveharding - great feedback. I'll keep this issue open and refer to it when we look at enhancing the SDK functionality.

We are currently prioritizing adding more API endpoint support over these type of enhancements -but I like what you've outlined here. I will explore if this is something already supported in our templates and driven from the OpenAPI spec or something we'll need to add in the templates themselves.

Cheers! 😄

[daveharding]

@SidneyAllen No worries 😄