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 Jukka Zitting <ju...@gmail.com> on 2014/03/31 17:35:59 UTC

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Hi,

On Mon, Mar 31, 2014 at 6:14 AM,  <md...@apache.org> wrote:
> +    @Override
> +    public String getContentIdentity() {
> +        return getBlob().getReference();
> +    }

This is a bit troublesome, as the getContentIdentity() contract
requires that "Once an identifier is available, it will never change
because values are immutable." The Blob.getReference() method (that's
based on the ReferenceBinary design) does not necessarily return such
a stable identifier (references can expire, etc.).

If we want to expose a stable content identifier like in
getContentIdentity(), we should extend the Blob interface accordingly
and have the underlying implementation return such a value if
available.

BR,

Jukka Zitting

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Posted by Chetan Mehrotra <ch...@gmail.com>.
On Wed, Apr 2, 2014 at 12:18 PM, Jukka Zitting <ju...@gmail.com> wrote:
> The getContentIdentity() method has a specific contract and the return
> value should generally not be interpreted as a referenceable
> identifier.

Ack.

> If you need a method that exposes the blobId, it would be best to add
> a separate method for that. But note that not all Blob implementations
> have a blobId like in BlobStoreBlob.

For now there is no strong requirement for that. If need arises would
follow up this way
Chetan Mehrotra

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Tue, Apr 1, 2014 at 9:29 AM, Michael Dürig <md...@apache.org> wrote:
> 3rd try: http://svn.apache.org/r1583661

Looks good, thanks!

On Mon, Mar 31, 2014 at 11:56 PM, Chetan Mehrotra
<ch...@gmail.com> wrote:
> +1. I also missed getting a clean way to get blobId from Blob. So
> adding this method would be useful in other cases also

The getContentIdentity() method has a specific contract and the return
value should generally not be interpreted as a referenceable
identifier.

If you need a method that exposes the blobId, it would be best to add
a separate method for that. But note that not all Blob implementations
have a blobId like in BlobStoreBlob.

BR,

Jukka Zitting

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Posted by Michael Dürig <md...@apache.org>.
3rd try: http://svn.apache.org/r1583661

Michael

On 1 April 2014 05:56, Chetan Mehrotra <ch...@gmail.com> wrote:
> +1. I also missed getting a clean way to get blobId from Blob. So
> adding this method would be useful in other cases also
> Chetan Mehrotra
>
>
> On Tue, Apr 1, 2014 at 8:05 AM, Jukka Zitting <ju...@gmail.com> wrote:
>> Hi,
>>
>> On Mon, Mar 31, 2014 at 3:25 PM, Michael Dürig <md...@apache.org> wrote:
>>> 2nd try: http://svn.apache.org/r1583413
>>
>> That's more correct, but has horrible performance with any
>> implementation (including BlobStoreBlob and SegmentBlob) that doesn't
>> precompute the hash.
>>
>> As mentioned earlier, a better alternative would be to add an explicit
>> method for this and let the implementations decide what the best
>> identifier would be.
>>
>> For BlobStoreBlob that would likely be:
>>
>>     public String getContentIdentity() {
>>         return blobId;
>>     }
>>
>> And for SegmentBlob:
>>
>>     public String getContentIdentity() {
>>         return getRecordId().toString();
>>     }
>>
>> BR,
>>
>> Jukka Zitting

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Posted by Chetan Mehrotra <ch...@gmail.com>.
+1. I also missed getting a clean way to get blobId from Blob. So
adding this method would be useful in other cases also
Chetan Mehrotra


On Tue, Apr 1, 2014 at 8:05 AM, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> On Mon, Mar 31, 2014 at 3:25 PM, Michael Dürig <md...@apache.org> wrote:
>> 2nd try: http://svn.apache.org/r1583413
>
> That's more correct, but has horrible performance with any
> implementation (including BlobStoreBlob and SegmentBlob) that doesn't
> precompute the hash.
>
> As mentioned earlier, a better alternative would be to add an explicit
> method for this and let the implementations decide what the best
> identifier would be.
>
> For BlobStoreBlob that would likely be:
>
>     public String getContentIdentity() {
>         return blobId;
>     }
>
> And for SegmentBlob:
>
>     public String getContentIdentity() {
>         return getRecordId().toString();
>     }
>
> BR,
>
> Jukka Zitting

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Mon, Mar 31, 2014 at 3:25 PM, Michael Dürig <md...@apache.org> wrote:
> 2nd try: http://svn.apache.org/r1583413

That's more correct, but has horrible performance with any
implementation (including BlobStoreBlob and SegmentBlob) that doesn't
precompute the hash.

As mentioned earlier, a better alternative would be to add an explicit
method for this and let the implementations decide what the best
identifier would be.

For BlobStoreBlob that would likely be:

    public String getContentIdentity() {
        return blobId;
    }

And for SegmentBlob:

    public String getContentIdentity() {
        return getRecordId().toString();
    }

BR,

Jukka Zitting

Re: svn commit: r1583285 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/value/ValueImpl.java

Posted by Michael Dürig <md...@apache.org>.
2nd try: http://svn.apache.org/r1583413

Michael

On 31 March 2014 17:35, Jukka Zitting <ju...@gmail.com> wrote:
> Hi,
>
> On Mon, Mar 31, 2014 at 6:14 AM,  <md...@apache.org> wrote:
>> +    @Override
>> +    public String getContentIdentity() {
>> +        return getBlob().getReference();
>> +    }
>
> This is a bit troublesome, as the getContentIdentity() contract
> requires that "Once an identifier is available, it will never change
> because values are immutable." The Blob.getReference() method (that's
> based on the ReferenceBinary design) does not necessarily return such
> a stable identifier (references can expire, etc.).
>
> If we want to expose a stable content identifier like in
> getContentIdentity(), we should extend the Blob interface accordingly
> and have the underlying implementation return such a value if
> available.
>
> BR,
>
> Jukka Zitting