Support for binary response and binary request body in java8 template

erlendv commented 5 years ago

The IResponse interface only have a setBody()/getBody() that works with strings. The entrypoint (com.openfaas.entrypoint.App) also tries to encode the body to UTF-8 when converting to bytes to write to the OutputStream.

Also, the IRequest interface also only have a getBody() that returns a String.

Expected Behaviour

The IResponse-interface should at least have a setBody(byte[] data) and a getBodyData() (that gets the byte). The entrypoint should use getBodyData() instead of getBody().

The best solution would be to write to an OutputStream on the response, ideally being the OutputStream from HttpExchange. getResponseBody().

The IRequest interface should have a getInputStream() method to return an InputStream, and the Request-implementation should return the one from HttpExchange.getRequestBody()

Current Behaviour

Unable to write binary data.

Possible Solution

See expected behaviour.

Steps to Reproduce (for bugs)

Response response = new Response();

ByteArrayOutputStream rawOutputStream = new ByteArrayOutputStream();
ObjectOutputStream objectOutputStream = new ObjectOutputStream(rawOutputStream);

Object resultObject = new Object();
objectOutputStream.writeObject(resultObject);
objectOutputStream.flush();

response.setBody(new String(rawOutputStream.toByteArray())); // have to set the raw data as a String on the response

Context

In my case I was serializing java objects between the function and a java client. Controlling both I base64-encoded the data to work around this. However if the response where to be an image for example it should be possible to return raw data.

Your Environment

alexellis commented 5 years ago

Hi @erlendv,

Thank you for your suggestion and for providing lots of detail. I could see this as being a useful change for Java users.

I can see that you've followed some of the contribution guide, but there is a part that states that work shouldn't be started until the feature is agreed on with the maintainers.

Given that you've already done the work, let's chat about it here?

Is this a breaking change?

Alex

erlendv commented 5 years ago

Hi Alex

Thank you for your reply. Sorry about that, I realise I did not quite follow the contribution guide here. I got a bit eager :)

Im new to openfass and not sure how this compares to the other language support. But I think being able to read and write binary data could be a good thing. For instance an image manipulation service.

Ideally I would have solved this a bit different. But the goal was not to break anything, so no, its not a breaking change. But I think this is a good compromise.

I added some tests also, they should verify that it doesnt break, as well as test the new functionality.

Best regards Erlend

fre. 19. jul. 2019 kl. 21:52 skrev Alex Ellis notifications@github.com:

Hi @erlendv https://github.com/erlendv,

Thank you for your suggestion and for providing lots of detail. I could see this as being a useful change for Java users.

I can see that you've followed some of the contribution guide, but there is a part that states that work shouldn't be started until the feature is agreed on with the maintainers.

Given that you've already done the work, let's chat about it here?

Is this a breaking change?

Alex

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openfaas/templates/issues/159?email_source=notifications&email_token=AAKSDYN5SU4GQPAMU2HDEKTQAILRLA5CNFSM4IFD6AN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2MTHCI#issuecomment-513356681, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKSDYNTNAPE5UMJRVH3R4LQAILRLANCNFSM4IFD6ANQ .

erlendv commented 5 years ago

Hi Alex

This is my first contribution to anything so I hope you can bear with me. I did forget to sign off the first commit and tried to amend the commit with signoff. But the result was two commits: https://github.com/openfaas/templates/pull/160/commits/ca71c0e9ff08dc10f819e1e874ecd764a79df35d https://github.com/openfaas/templates/pull/160/commits/ca71c0e9ff08dc10f819e1e874ecd764a79df35d https://github.com/openfaas/templates/pull/160/commits/d843d6f6be7999b5822708f0d348513a4c7bb1bf https://github.com/openfaas/templates/pull/160/commits/d843d6f6be7999b5822708f0d348513a4c7bb1bf

I have pushed the changes you proposed.

Best regards, Erlend

jul. 2019 kl. 21:52 skrev Alex Ellis notifications@github.com:

Hi @erlendv https://github.com/erlendv,

Thank you for your suggestion and for providing lots of detail. I could see this as being a useful change for Java users.

I can see that you've followed some of the contribution guide, but there is a part that states that work shouldn't be started until the feature is agreed on with the maintainers.

Given that you've already done the work, let's chat about it here?

Is this a breaking change?

Alex

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openfaas/templates/issues/159?email_source=notifications&email_token=AAKSDYN5SU4GQPAMU2HDEKTQAILRLA5CNFSM4IFD6AN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2MTHCI#issuecomment-513356681, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKSDYNTNAPE5UMJRVH3R4LQAILRLANCNFSM4IFD6ANQ.

magnusja commented 4 years ago

Any news on this? Cant be true that I can only exchange strings in functions right? Base64 encoding an image adds a substantial overhead I would like to avoid...

openfaas / templates