JsonArray.add(JsonObject) adds partial JsonObject when the JsonDocument is full

[gonzabrusco]

Describe the bug
Not sure if a bug or a design choice. But this is the workflow to reproduce this issue:

• Use ArduinoJSON V6
• Create a JsonDocument with fixed size (static or dynamic).
• Create a JsonArray inside that JsonDocument.
• Create a loop that fills the JsonArray with JsonObjects like this {"id":3,"test":"this is a test", "another_key":"random text"}
• The loop exits when JsonArray.add() returns false (no more memory in the JsonDocument to add another item in the array).

When checking what was saved in the JsonArray, I find something like this:

[
 {"id":3,"test":"this is a test", "another_key":"random text"}, 
 {"id":3,"test":"this is a test", "another_key":"random text"},
 {"id":3,"test":"this is a test", "another_key":"random text"},
 {"id":3,"test":"this is a test", "another_key":"random text"},
 {"id":3}  <--- PARTIAL JSON!!
]

It seems that when you call the method add() it returns false if there's no more memory to fully add the JsonObject, but it leaves a partial JSON in the array. I think this should not be the correct behavior. It should notify if there's not enough memory to store the JsonObject but leave the array untouched. Adding a partial JSON probably is a bad idea in most situations (API breaker).

[gonzabrusco]

Anticipating the possible response that may come, in my project's API, I don't have direct access to the JsonDocument, so I can't use the capacity() memoryUsage() methods to try to workaround this issue.

[gonzabrusco]

Ok. I worked around it keeping a count of the JsonObject that were successfully added to the JsonArray and when it exits the loop (because the last addition failed due to memory), I compare the size() of the JsonArray with that count. If they differ, then It means that although the addition failed, something was added nevertheless, so I have to delete the last item of the array (the partially added JsonObject).

[bblanchon]

Hi @gonzabrusco,

I guess I could add a special treatment for add() so as to provide this "transactional guarantee", but I cannot do the same for set() since the original value is lost.

Also, this "transactional guarantee" would be very limited since you'd not necessarily return to the state before the operation. For example, if add failed in the following program:

doc["config"]["wifi"].add(newAccessPointObject);

If doc was initially:

{"config":{"url":"api.example.com"}}

After "reverting" the failing add(), we would end up with:

{"config":{"url":"api.example.com","wifi":[]}}

It is probably okay, but definitely not the same as the original.

Moreover, because ArduinoJson 6's allocator is monotonic, we cannot recover the memory consumed by the removed object.

Best regards,
Benoit

[gonzabrusco]

Yes I understand what you are saying but I still think that what I propose would be the correct choice in this regard. If the add method fails, it should not leave "a partial copy" of a JSON.

In the example you provided, it "fails" because you are doing two things at a time (creating an array and then adding an item). I think in that case the user should use JsonDocument::createNestedArray() and check the return value before adding items. Or just leave the array empty (which is probably fine because the other end is probably expecting the array, empty or not).

[bblanchon]

I worked several hours on this issue but could not find an elegant solution that doesn't increase the code size significantly.
If it were a functionality used by many users, I would accept this penalty, but you are the first to notice this problem in 10 years.
I plan to implement the fix in ArduinoJson 7, which has looser code size constraints.

[gonzabrusco]

Fair enough. I will stick to my workaround then. Thank you for taking the time to evaluate this issue.

Feel free to what you want with this issue report.