f-f / gogolica

Auto-generated Google APIs for Clojure
Eclipse Public License 1.0
2 stars 0 forks source link

Full media download support #30

Open zudov opened 6 years ago

zudov commented 6 years ago

Currently we simply allow the user to specify alt=media in order to get redirected to the downloadable content (I suppose the server then returns a SEE OTHER).

"Media Download" section however recommends constructing the download url by prefixing the path with /download/, which avoids the redirect.

Also there is a useMediaDownloadService, I'm not sure, but probably it means than when it's true the alt=media won't work. Or maybe it would, nevertheless they recommend to prefix url with /download/ at all times :man_shrugging:

I think it would be pretty nice if when the mediaDownload is true, we generate additional function (with -media) suffix that would perform that /download/ request.

So instead of (taken from README) doing:

(gcs/objects-get "my-new-bucket-name" "my-file-name" {:alt "media"})

One would do:

(gcs/objects-get-media "my-new-bucket-name" "my-file-name")

Alternative interface would be to allow some {:media-download true} option as an argument to normal objects-get, but it feels nice to have a separate function, since the functionality of "get the metadata of an object" and "get content of an object" feels like different functions (what you do with the response and the purpose of it is quite different).

The support for alt parameter would remain as part of #26, since it also allows to specify alternative response formats, we should be aware of that and decode the content appropriately.

zudov commented 6 years ago

On the other hand adding any suffix (like media) to the generated functions is dangerous, because it might lead to clashes. (there might be another resource at objects/get/ which happen to have a method named media). Maybe {:media-download true} or something else is a better option.

Or maybe just keep the interface as is (user has to say alt=media), but useMediaDownloadService when we notice that this is the case.

zudov commented 6 years ago

This issue is actually a sort of duplicate for #12

f-f commented 6 years ago

A bit of context on not adding the /download part on {:alt "media"}: I implemented it that way because if the method has both uploadMedia and downloadMedia keys, then the url generator gets confused on which url to use, /upload or /download. Easily fixable by generating an if which is aware of the runtime parameters and can switch when alt=media.

About providing a different function to do this: I agree that alt=media is not the best thing, but it's the way they support it, so any renaming that we do might get into name clashes. A nice solution for this whould be to provide an additional arity in which you can pass true (lol) to get your object instead of the metadata.