jlevers / selling-partner-api

A PHP client library for Amazon's Selling Partner API
BSD 3-Clause "New" or "Revised" License
409 stars 200 forks source link

Check if data that is going to be deserialized is also empty #815

Open nelutihon opened 1 week ago

jlevers commented 1 week ago

Thanks for the contribution @nelutihon!

What problem are you trying to solve here? Using empty makes me a little nervous, because according to the PHP manual, here's how it works:

A variable is considered empty if it does not exist or if its value equals false.

That means that a boolean false, an empty string, etc would all be deserialized to null, which we don't want.

nelutihon commented 1 week ago

@jlevers I have this error because Amazon returns an empty array as a prep detail $data is looking like this: array(0) { }

This is the error: In PrepDetails.php line 23:

Too few arguments to function SellingPartnerApi\Seller\FBAInboundV0\Dto\PrepDetails::__construct(), 0 passed in /vendor/jlevers/selling-partner-api/src/Traits/Deserializes.php on line 71 and exactly 2 expected

iajrz commented 1 week ago

@nelutihon I understand the issue you're trying to correct: the semantics for what it means to deserialize is not complete, and you're covering a straightforward case of an empty array. You're going with the interpretation that an empty array means a null object.

As jlevers pointed out, empty does more than what you want to do: it also interprets false as a null object. I believe an exception is an appropriate response to trying to deserialize null into an instance of any class, or at least more appropriate than equating it to a null instance.

We've a fork in the road here. We could path is to rewrite the fix on our end instead of having your contribution (lame!).

Now for the cooler paths:

In either of those cooler paths you might even add a few tests for different scenarios to cement how deserialization is interpreted in this project, making you not only a bugfinder and bugfixer, but also a contributor to the maturation of a small but important corner of the project.

Are you interested in moving forward with one of the cooler paths?