You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2013/12/15 01:56:07 UTC
[jclouds] Convert Payload from InputSupplier to ByteSource (#233)
This allows use of the modern Guava IO idioms, e.g., ByteSource.read()
instead of ByteStreams.toByteArray(InputSupplier). Note that we must
replace calls to InputSupplier.getInput with overridden calls to
ByteSource.openStream to deal with InputSupplier.getInput checked
exceptions.
References JCLOUDS-410.
You can merge this Pull Request by running:
git pull https://github.com/maginatics/jclouds payload-byte-source
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/233
-- Commit Summary --
* Convert Payload from InputSupplier to ByteSource
-- File Changes --
M apis/atmos/src/main/java/org/jclouds/atmos/functions/ParseDirectoryListFromContentAndHeaders.java (2)
M apis/vcloud/src/main/java/org/jclouds/vcloud/functions/ParseLoginResponseFromHeaders.java (2)
M common/azure/src/main/java/org/jclouds/azure/storage/handlers/ParseAzureStorageErrorFromXmlContent.java (2)
M core/src/main/java/org/jclouds/http/HttpUtils.java (2)
M core/src/main/java/org/jclouds/http/functions/ParseJson.java (2)
M core/src/main/java/org/jclouds/http/functions/ParseSax.java (2)
M core/src/main/java/org/jclouds/http/functions/ParseURIFromListOrLocationHeaderIf20x.java (6)
M core/src/main/java/org/jclouds/http/functions/ParseXMLWithJAXB.java (2)
M core/src/main/java/org/jclouds/http/functions/ReturnInputStream.java (2)
M core/src/main/java/org/jclouds/http/functions/ReturnStringIf2xx.java (2)
M core/src/main/java/org/jclouds/io/Payload.java (16)
M core/src/main/java/org/jclouds/io/internal/BasePayloadSlicer.java (2)
M core/src/main/java/org/jclouds/io/payloads/BaseCipherPayload.java (4)
M core/src/main/java/org/jclouds/io/payloads/BasePayload.java (4)
M core/src/main/java/org/jclouds/io/payloads/ByteArrayPayload.java (2)
M core/src/main/java/org/jclouds/io/payloads/DelegatingPayload.java (8)
M core/src/main/java/org/jclouds/io/payloads/FilePayload.java (2)
M core/src/main/java/org/jclouds/io/payloads/InputStreamPayload.java (2)
M core/src/main/java/org/jclouds/io/payloads/InputStreamSupplierPayload.java (2)
M core/src/main/java/org/jclouds/io/payloads/MultipartForm.java (2)
M core/src/main/java/org/jclouds/io/payloads/PhantomPayload.java (2)
M core/src/main/java/org/jclouds/io/payloads/StreamingPayload.java (4)
M core/src/main/java/org/jclouds/io/payloads/StringPayload.java (2)
M core/src/main/java/org/jclouds/io/payloads/UrlEncodedFormPayload.java (2)
M core/src/main/java/org/jclouds/logging/internal/Wire.java (4)
M core/src/test/java/org/jclouds/http/functions/ParseURIFromListOrLocationHeaderIf20xTest.java (6)
M core/src/test/java/org/jclouds/io/payloads/MultipartFormTest.java (4)
M drivers/apachehc/src/main/java/org/jclouds/http/apachehc/ApacheHCUtils.java (4)
M drivers/gae/src/main/java/org/jclouds/gae/ConvertToGaeRequest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/233.patch
https://github.com/jclouds/jclouds/pull/233.diff
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Gaul <no...@github.com>.
This commit retains source compatibility with all existing callers since ByteSource implements InputSupplier.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30595749
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Phillips <no...@github.com>.
> but cannot see a backwards-compatible way to do this.
I feared as much. *sigh*. Thanks for explaining.
For which release are we targeting this? Would be interested to hear thoughts from others on this one...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30637392
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Gaul <no...@github.com>.
Another proposal, abandoning this commit:
For 1.7, add an ```openStream() throws IOException``` method and deprecate ```getInput``` without a checked exception.
For 1.8, change ```Payload``` to ```extend ByteSource``` and inherit the deprecated ```getInput throws IOException```.
This gives users a full major release if they use the legacy ```getInput``` methods.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30681538
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Phillips <no...@github.com>.
> For 1.7, add an openStream() throws IOException method and deprecate getInput without a checked exception.
Good idea! +1 from me.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30683934
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Phillips <no...@github.com>.
Is there any way to make this change in two stages:
1. Add the ability to use ByteSource but remain backwards compatible, whilst deprecating the `getInput` etc. methods related to InputSupplier
2. Remove support for InputSupplier (if deemed necessary) in 1.8
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30595728
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Phillips <no...@github.com>.
> I guess if users call getInput directly instead of using a Guava wrapper they will have to cope with the checked
> IOException. Unfortunately the Payload implementors wrap IOException in an unchecked RuntimeException. This
> is unfortunate and I would really like to declare the IOException instead of hiding it.
Nothing against that, but that would make it a breaking change, so something for 1.8.0 then? And I guess it's not possible, as a transitional step, to provide a backwards-compatible `getInput` that swallows the exception but immediately deprecate it and suggest that callers use Guava or the inherited version instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30624157
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Gaul <no...@github.com>.
Unfortunately Guava defines its ByteSouce.getInput as final, so we cannot override it and mask the exception. I feel like we should stop masking this entirely but cannot see a backwards-compatible way to do this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30636482
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Phillips <no...@github.com>.
The best thing I can think of for now that would be backwards compatible is:
1. Keep Payload a subinterface of InputSupplier for now
2. Add an `asByteSource` method to Payload that would return a view of a Payload as a ByteSource
3. Deprecate `Payload.getInput` with a warning to switch to ByteSource use (since ByteSource.getInput() also looks like it's on its way out)
4. For 1.8.0, make Payload a subclass of ByteSource. If ByteSource.getInput() still exists, users will just have to change their sig, but they shouldn't be using it anyway. Anyone using Payload.asByteSource() will just need to delete the "asByteSource()" part, or we give them more time by returning "this" from that method and deprecating asByteSource(). But if we're going to make breaking changes, better perhaps to just get it over with.
Not hugely satisfying, but would allow us to use ByteSource idioms now without introducing breaking changes..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30640454
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #472](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/472/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30596234
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Gaul <no...@github.com>.
Alternate approach:
https://github.com/jclouds/jclouds/pull/235
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30692611
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by Andrew Gaul <no...@github.com>.
I guess if users call getInput directly instead of using a Guava wrapper they will have to cope with the checked IOException. Unfortunately the Payload implementors wrap IOException in an unchecked RuntimeException. This is unfortunate and I would really like to declare the IOException instead of hiding it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30596157
Re: [jclouds] Convert Payload from InputSupplier to ByteSource (#233)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #935](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/935/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/233#issuecomment-30596326