Skip to content

Support for prodiving a HttpEntity to save a couchdb attachement #180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

YannRobert
Copy link
Collaborator

This change provides a new method
in order to be able to publish an attachment document to couchdb providing an HttpEntity

this is usefull when the application does not have an InputStream to provide to the existing methods.
but have an Object that is able to be written to an OutputStream

without relying on a temporary ByteArrayOutputStream buffer (or similar)

About implementation details :
The first commit is a refactoring : I extracted all the methods related to attachment management into a new class StdAttachmentCouchDbConnector, and a matching AttachmentCouchDbConnector interface.

I made CouchDbConnector extend AttachmentCouchDbConnector in order to save code duplication.
And StdCouchDbConnector is encapsulating the StdAttachmentCouchDbConnector instance.

Still, I am unsure whether or not CouchDbConnector should extend AttachmentCouchDbConnector, or if we would better have code duplication.
This is because there may be some kind of leaking abstraction exposing HttpEntity in the CouchDbConnector interface. Or maybe we should not care ?

I'll leave it up to you to comment on this.

YannRobert added 2 commits May 9, 2014 15:58
…AttachmentCouchDbConnector. Existing methods now just delegates to it.
…tity, providing a way for user applications to save an Object directly in a streamed way without relying on temporary ByteArrayOutputStream+ByteArrayInputStream, just by implementing an appropriate HttpEntity
@YannRobert YannRobert changed the title Support for prodiving a HttpEntity to put an couchdb attachement Support for prodiving a HttpEntity to save a couchdb attachement May 9, 2014
@helun
Copy link
Owner

helun commented May 9, 2014

I cannot see the justification for all this, why not just use ByteArrayOutputStream as you say yourself?

@YannRobert
Copy link
Collaborator Author

At first, we used ByteArrayOutputStream + ByteArrayInputStream in order to get an InputStream to pass to Ektorp.
But this is a real waste of time and memory, because it can be done directly to the http request, so that we do not use an in-memory buffer that will have to be garbaged collected.
ByteArrayOutputStream is very inefficient when the size is unknown in advance. Also it is not safe as you can easily get OutOfMemory.
Any other alternative, like buffering to disk, have other drawbacks not worth mentioning.

TLDR : performances

@helun
Copy link
Owner

helun commented May 12, 2014

I have two concerns:

  1. I cannot see how this will improve performance since you always can use streams. What is the use case where you have an attachment that risk causing out of memory and you cannot handle it as a stream?
  2. It is not ok to expose org.apache.http.HttpEntity in CouchDbConnector

@YannRobert
Copy link
Collaborator Author

hi @helun, thank you for your comments.

I'll provide an integration test describing something close to my use case, so that it will be easier to understand.

Regarding exposure of org.apache.http.HttpEntity in CouchDbConnector, I agree abstraction is leaking. So I'll make the new method only available in AttachmentCouchDbConnector, and CouchDbConnector will not extend AttachmentCouchDbConnector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants