fix(storage): add null check for $stream->getMetadata#9071
fix(storage): add null check for $stream->getMetadata#9071mateuszanella wants to merge 5 commits intogoogleapis:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Perhaps |
|
Hello @mateuszanella! Thanks so much for your contribution. I think this is a simple fix, although we are missing the CLA. If we wish to merge this into the code, we require a signed CLA, more info here: Other than that, it does look like a good and simple fix, but may I suggest if we can add a unit test for this to keep it consistent? |
|
Done :) |
Storage/src/ReadStream.php
Outdated
| foreach ($this->stream->getMetadata('wrapper_data') as $value) { | ||
| $metadata = $this->stream->getMetadata('wrapper_data'); | ||
|
|
||
| if (!is_array($metadata)) { |
There was a problem hiding this comment.
It would be better to call this is_null. We should be more specific with our handling. If something that isn't null and isn't an array is returned, it's okay if the function errors (because that case is unknown, and so we should handle it).
There was a problem hiding this comment.
Yeah, that makes sense. Fixed it!
…la/google-cloud-php into fix/foreach-null-bug
luizautentique
left a comment
There was a problem hiding this comment.
very cool implementation @mateuszanella.
I think this will help so much.
In the instance that a Stream object does not contain a
wrapper_datametatada entry, the function returnsnulland throws the following exception when trying to read the size of the stream:This PR introduces a simple check to see if the response from the metadata call is in fact an array, and returns 0 otherwise (following the default function behavior).
To be honest, I am not really sure why there is no size metadata for the stream, but I believe that this case should be handled, either by just returning 0 or by throwing a proper exception.