downloadToFile (Cloud-Storage) creates empty file when files does not exists in bucket

[rodehoed]

Environment details

• OS: RHEL 7
• PHP version: PHP 7.2
• Package name and version: google/cloud-storage 1.22

Steps to reproduce

1. Download a non existing file
2. Exception is thrown, but there is a 0-size file created anyway

Code example

 $object = $bucket->object('non-existing-files.txt');
 $object->downloadToFile('download/non-existing-files.txt');

IMHO the file should never be created when an exceptions rises. Right now I'm catching the exception, but I have to remove the empty file afterwards.

[jdpedrie]

@rodehoed in cases where you do not know ahead of time that the requested file exists, you should use StorageObject::exists() to check its status prior to calling the download method.

$object = $bucket->object('non-existing-files.txt');
if ($object->exists()) {
	$object->downloadToFile('download/non-existing-files.txt');
} else {
	throw new \Exception('Object does not exist!');
}

@dwsupplee I'm not a fan of wrapping the exists() check into the download method since it seems to me that the majority of cases would know the existence status before downloading. I think this is something we should explain in the documentation. WDYT?

[rodehoed]

Hi,

Thanks for your prompt answer. I could use the exist function.....

I'm fine with the current exception.

But my main concern, when the download fails for whatever reason (not only because it doesn't exist) it should not write an empty file.

Op vr 28 aug. 2020 15:19 schreef John Pedrie [email protected]:

@rodehoed https://github.com/rodehoed in cases where you do not know ahead of time that the requested file exists, you should use StorageObject::exists() to check its status prior to calling the download method.

$object = $bucket->object('non-existing-files.txt');if ($object->exists()) { $object->downloadToFile('download/non-existing-files.txt'); } else { throw new \Exception('Object does not exist!'); }

@dwsupplee https://github.com/dwsupplee I'm not a fan of wrapping the exists() check into the download method since it seems to me that the majority of cases would know the existence status before downloading. I think this is something we should explain in the documentation. WDYT?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/googleapis/google-cloud-php/issues/3347#issuecomment-682562354, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRWVQ23UVOED4AJURCM2YTSC6VF3ANCNFSM4QN5IC5Q .

[vishwarajanand]

@rodehoed sorry for coming late to the issue. I understand the reason for insisting on not creating an empty file when the file download could not succeed in the first place.

I have raised a potential fix for this.