You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Angela Schreiber <an...@adobe.com> on 2012/10/19 09:54:20 UTC

ContentSession#createBlob ?

hi all

recent the ContentSession interface got a new method

  Blob createBlob(InputStream inputStream) throws IOException;

this there is no TODO comment associated with it i assume
that is intended to stay... however, it feels a bit odd to
me having that method on the content session interface and
i don't see how and why a regular API consumer of the
ContentSession would need/use this.

currently the only usage of the method is in a private
method on ValueFactoryImpl, which for that very purpose
needs to have the content session passed to the constructor
but doesn't otherwise need access to the content session
at all...

imo that should be reviewed again.

kind regards
angela




Re: ContentSession#createBlob ?

Posted by Angela Schreiber <an...@adobe.com>.
hi michael

i create an enhancement to track that issue at
https://issues.apache.org/jira/browse/OAK-392

and i added a suggestion on how to improve that in
rev. 1401331 (with TODOs for those parts that IMO
need some extra care and clarification). tests passed
but i didn't check if there are many tests verifying
the expected behavior of the former code and did not
yet add tests for my changes...
it would be perfect, if you had time to look at it and
check if my proposal fits your needs/expectations as well.

gruesse
angela


On 10/19/12 11:43 AM, Michael Dürig wrote:
>
>
> On 19.10.12 10:36, Angela Schreiber wrote:
>> hi michael
>>
>> thanks for the info
>>
>>> Note that createBlobd() is very much the same as the
>>> getCoreValueFactory() method, which it replaced.
>>
>> yes, i am aware of this... the reason why i don't like
>> the method on contentsession has nothing to do with the
>> 'what-it-does' but rather the fact i IMO it wasn't
>> the responsibility of a contentsession to deal with binary
>> values when at the other hand we have all value handling
>> separated out to a ValueFactory(Impl).
>
> Yes, but this is oak-jcr, not oak-core. If we forfeit the capability for
> creating "immediately streamed" blobs on the oak-core level, we can
> remove that method from the API.
>
> Michael.
>
>>
>> kind regards
>> angela
>>
>>> Michael
>>>
>>>
>>> On 19.10.12 8:54, Angela Schreiber wrote:
>>>> hi all
>>>>
>>>> recent the ContentSession interface got a new method
>>>>
>>>>     Blob createBlob(InputStream inputStream) throws IOException;
>>>>
>>>> this there is no TODO comment associated with it i assume
>>>> that is intended to stay... however, it feels a bit odd to
>>>> me having that method on the content session interface and
>>>> i don't see how and why a regular API consumer of the
>>>> ContentSession would need/use this.
>>>>
>>>> currently the only usage of the method is in a private
>>>> method on ValueFactoryImpl, which for that very purpose
>>>> needs to have the content session passed to the constructor
>>>> but doesn't otherwise need access to the content session
>>>> at all...
>>>>
>>>> imo that should be reviewed again.
>>>>
>>>> kind regards
>>>> angela
>>>>
>>>>
>>>>

Re: ContentSession#createBlob ?

Posted by Michael Dürig <md...@apache.org>.

On 19.10.12 10:36, Angela Schreiber wrote:
> hi michael
>
> thanks for the info
>
>> Note that createBlobd() is very much the same as the
>> getCoreValueFactory() method, which it replaced.
>
> yes, i am aware of this... the reason why i don't like
> the method on contentsession has nothing to do with the
> 'what-it-does' but rather the fact i IMO it wasn't
> the responsibility of a contentsession to deal with binary
> values when at the other hand we have all value handling
> separated out to a ValueFactory(Impl).

Yes, but this is oak-jcr, not oak-core. If we forfeit the capability for 
creating "immediately streamed" blobs on the oak-core level, we can 
remove that method from the API.

Michael.

>
> kind regards
> angela
>
>> Michael
>>
>>
>> On 19.10.12 8:54, Angela Schreiber wrote:
>>> hi all
>>>
>>> recent the ContentSession interface got a new method
>>>
>>>    Blob createBlob(InputStream inputStream) throws IOException;
>>>
>>> this there is no TODO comment associated with it i assume
>>> that is intended to stay... however, it feels a bit odd to
>>> me having that method on the content session interface and
>>> i don't see how and why a regular API consumer of the
>>> ContentSession would need/use this.
>>>
>>> currently the only usage of the method is in a private
>>> method on ValueFactoryImpl, which for that very purpose
>>> needs to have the content session passed to the constructor
>>> but doesn't otherwise need access to the content session
>>> at all...
>>>
>>> imo that should be reviewed again.
>>>
>>> kind regards
>>> angela
>>>
>>>
>>>

Re: ContentSession#createBlob ?

Posted by Angela Schreiber <an...@adobe.com>.
hi michael

thanks for the info

> Note that createBlobd() is very much the same as the
> getCoreValueFactory() method, which it replaced.

yes, i am aware of this... the reason why i don't like
the method on contentsession has nothing to do with the
'what-it-does' but rather the fact i IMO it wasn't
the responsibility of a contentsession to deal with binary
values when at the other hand we have all value handling
separated out to a ValueFactory(Impl).

kind regards
angela

> Michael
>
>
> On 19.10.12 8:54, Angela Schreiber wrote:
>> hi all
>>
>> recent the ContentSession interface got a new method
>>
>>    Blob createBlob(InputStream inputStream) throws IOException;
>>
>> this there is no TODO comment associated with it i assume
>> that is intended to stay... however, it feels a bit odd to
>> me having that method on the content session interface and
>> i don't see how and why a regular API consumer of the
>> ContentSession would need/use this.
>>
>> currently the only usage of the method is in a private
>> method on ValueFactoryImpl, which for that very purpose
>> needs to have the content session passed to the constructor
>> but doesn't otherwise need access to the content session
>> at all...
>>
>> imo that should be reviewed again.
>>
>> kind regards
>> angela
>>
>>
>>

Re: ContentSession#createBlob ?

Posted by Michael Dürig <md...@apache.org>.
The purpose of that method is to allow the API consumer to create Blob 
instances which are immediately streamed down to the BlobStore in the 
Microkernel. This way the API consumer can control at what point 
streaming takes place when setting values of Type.BINARY on a Tree: blob 
instances created through ContentSession.createBlob() are immediately 
streamed down while all other Blob instances are only streamed down on 
commit. There is also room for further optimisation here by making 
ContentSession.createBlob() stream in the background and a subsequent 
commit that relies on such a binary wait for the streaming to complete.

Note that createBlobd() is very much the same as the 
getCoreValueFactory() method, which it replaced. Through the latter you 
could also create binary values which had been immediately streamed down 
to the Microkernel.

Michael


On 19.10.12 8:54, Angela Schreiber wrote:
> hi all
>
> recent the ContentSession interface got a new method
>
>   Blob createBlob(InputStream inputStream) throws IOException;
>
> this there is no TODO comment associated with it i assume
> that is intended to stay... however, it feels a bit odd to
> me having that method on the content session interface and
> i don't see how and why a regular API consumer of the
> ContentSession would need/use this.
>
> currently the only usage of the method is in a private
> method on ValueFactoryImpl, which for that very purpose
> needs to have the content session passed to the constructor
> but doesn't otherwise need access to the content session
> at all...
>
> imo that should be reviewed again.
>
> kind regards
> angela
>
>
>