Pass Storage::disk instance to the saveTo() method

[vanodevium]

What about the idea of making it possible to pass the Laravel's Storage::disk object to the save function.

As for me, it would be very convenient.

Below are an examples:

Zip::create("archive.zip")
    // ... add files ...
    ->saveTo(Storage::disk('s3'));

---

Zip::create("archive.zip")
    // ... add files ...
    ->saveTo(Storage::disk('local'));

---

[jszobody]

You can already specify S3 using s3://bucket/... paths, just like for file sources. So both local and S3 destinations are already supported.

Give that a try and let me know if you have trouble?

[vanodevium]

Oh, yes! I know this ability.

But when I am coding, i don't know bucket name because it dynamically set by env variable.

So I operate only with abstract disk with name s3

[ThaDaVos]

I would have used this package if it re-used the Storage::disk part - as many developers like me and @VanoDevium - don't know the exact bucket names while developing the code - one easy solution could be to use config() helper to the bucket name, but this feels dirty

Re-using disk instances feels way easier to use then magical strings

[aneesdev]

@vanodevium @ThaDaVos you can do something like this:

$bucket = Storage::disk('s3')->getAdapter()->getBucket(); // will return bucket name e.g blahblah123

$disk = "s3://{$bucket}/...";

[vanodevium]

Of course, I can extract bucket name from my adapter. But it is useless, imho

[aneesdev]

Of course, I can extract bucket name from my adapter. But it is useless, imho

Why it is useless? what your use case exactly?

[vanodevium]

I wanna simply pass my disk and then library will work with this disk automatically.

It will be nice if I can use saveTo() like:

Zip::create("archive.zip")
    // ... add files ...
    ->saveTo('path/to/folder', Storage::disk('s3'));

And library extract bucket name automatically.

It isn't best practice if user have to write Storage::disk('s3')->getAdapter()->getBucket();

[aneesdev]

I wanna simply pass my disk and then library will work with this disk automatically.

It will be nice if I can use saveTo() like:

Zip::create("archive.zip")
    // ... add files ...
    ->saveTo('path/to/folder', Storage::disk('s3'));

And library extract bucket name automatically.

It isn't best practice if user have to write Storage::disk('s3')->getAdapter()->getBucket();

Well, then feel free to make a PR.

[jszobody]

It makes sense, I get it.

Two things:

1) When adding files, I need a PSR7 stream, one that can provide filesize up front (prior to reading in the file contents). This is necessary to work with the underlying Zipstream package that I'm wrapping. I'm not sure I can accept source files from the Flysystem, I haven't looked into whether I can get a PSR7 stream and filesizes from all sources. It would be an interesting challenge to dig into, I haven't had time.

2) I wanted the saveTo method to work just like addFile. It seemed awkward to have different approaches, one where addFile requires s3://bucket/filename and another where you add with Storage facade. I let consistency dictate the current approach.

I agree that being able to save files with the Storage facade would be nice. Though I would suggest a change to what you have in mind, something like: ->saveTo('path/to/folder', 's3'). This would be more consistent with Laravel's approach to file uploads: https://laravel.com/docs/9.x/filesystem#specifying-a-disk.

I still feel like point 1 needs to be tackled for this to make sense though. I (or someone with the desire/time/energy) need to dig into how we can take a source from Storage and put it together for the Zipstream package to use. If that's possible, we can add full Storage facade support for both reading and writing files.

[vanodevium]

Nice one ->saveTo('path/to/folder', 's3')

[vanodevium]

Maybe I'll find time to dig into the code between missile alerts. Tnx!

[jszobody]

@vanodevium I'm pretty convinced that we'll need to extract the filepath from the Storage facade, and still use s3://bucket/path internally. We can't make use of the Storage calls directly, with how we're doing streaming.

That's actually a simpler way to go though. The saveTo method checks for a second parameter. If it exists, it uses the Storage internally to go build the full path of the file, extracting the bucket name and such.

That means this will only work though for Storage disks that are local or s3. We're not adding support for other Storage backends at this point.

[jszobody]

This is now possible with v5 beta, which will be tagged stable shortly.

To add files from a disk: $zip->addFromDisk('s3','my-key.pdf');

To save zip output to disk: $zip->saveToDisk('s3', 'my-folder')