OID strings

[Robert Matthews <rm...@nakedobjects.org>]

Dan

Can you clarify the changes to OID encoding and decoding to and from 
Strings.  I see that you have allowed the version and user to be 
included, which could be useful in certain circumstances. This breaks 
some things though, or that extra could at least be redundant. I guess 
the real question is why this is the default behaviour rather than a 
special behaviour.

As an example, my data show this

               "inUseBy" : "CUS:28^8:admin:1354818065713"

where before it would be this

               "inUseBy" : "CUS:28"


I think this new form is the special case and not the normal case. That 
is, I think any existing code work the way it has and any code that 
wants to explicitly include the other details should make that change.

Regards

Rob

Re: OID strings

[Dan Haywood <da...@haywood-associates.co.uk>]

On 11 December 2012 20:21, Robert Matthews <rm...@nakedobjects.org>wrote:

> I'm now running a patched version with this change and all is working
> again.
>
That's good to hear.


>
> Before you press ahead with changes to the PersistenceManager

... I didn't intend to make any changes, actually.  At least, not until we
bottom out this thread.



> it not just the object stores that need just the identity, another example
> is in a URL for a web page showing an object. If that page can be
> bookmarked then just the identity is needed, whereas a form that will
> operate on an object will be one (hidden) field lighter with the merged
> version. I assume that restfulObjects needs to consider the same thing.
>

Hmm.  Actually, I see the OID used within a URL as being opaque from a
viewer's standpoint.  But what a viewer should be able to rely on is that,
when converted from string form back to an Oid object, that that Oid can be
used with AdapterManager#adapterFor(oid, ConcurrencyChecking.CHECK) and the
persistor would be able to enforce concurrency checking if that was what
was required.  If concurrency checking is not required (eg for the
bookmarks), then the viewer would simply
call AdapterManager#adapterFor(oid, ConcurrencyChecking.NO_CHECK).

Somewhat related - and associated with this concept of an OID's string form
being opaque - I do see it as a responsibility of a viewer to encrypt the
OID.  Otherwise the system becomes hackable... a malicious end-user can
type in random URLs trying to uncover information that they otherwise
wouldn't have access to.  Having said that, I'm not sure that any of our
viewers do this or do this well enough.

But back to the main point: I don't think that we should be stripping out
version information from OIDs in the viewers.  The whole point of putting
encoding the information in the OID is so that the viewer doesn't need to
separately track when a particular object was rendered to a user.

Let's keep talking on this thread; we obviously need to get to a common
understanding on this important topic.

Dan


> Rob
>
>
> On 12/07/12 11:36, Dan Haywood wrote:
>
>> On 7 December 2012 09:43, Robert Matthews <rm...@nakedobjects.org>**
>> wrote:
>>
>>  I think I'm presuming that the enstring mechanism was there first and was
>>> doing the work that use to be done with the identifiers in the viewers
>>> and
>>> elsewhere, but this change and your centralizing of the optimistic lock
>>> checking came along together.  I guess when you replaced the uses of the
>>> old OIDs you or I just used the enstring method globally.  Seaching
>>> through
>>> there are number of places where the non-versioned one are actually
>>> needed,
>>> including the SQL Store. So we just need to finish that off. I'll try
>>> that
>>> later today as I need to get objects persisting in my application.
>>>
>>>  OK, that's a good point.  The RootOidDefault class has an
>> "enStringNoVersion()" method that could be used.
>>
>> Or - and perhaps more correct - the PersistenceManager could "strip out"
>> the version information whenever it passes an oid to an objectstore.  This
>> could perhaps be through a new RootOidDefault#stripVersion() method.  It
>> ought to return a new instance of RootOidDefault, though ... oids are
>> immutable.
>>
>> Dan
>>
>> PS: as a by-the-by, oids also have value semantics, hence implement equals
>> and hashCode.  Note that the version information is NOT part of the
>> equality check... this is by design.
>>
>>
>>
>>
>>  Rob
>>>
>>>
>>>
>>>
>>> On 12/07/12 08:33, Dan Haywood wrote:
>>>
>>>  On 6 December 2012 18:57, Robert Matthews <rmatthews@nakedobjects.org
>>>> >**
>>>> wrote:
>>>>
>>>>   Dan
>>>>
>>>>> Can you clarify the changes to OID encoding and decoding to and from
>>>>> Strings.  I see that you have allowed the version and user to be
>>>>> included,
>>>>> which could be useful in certain circumstances.
>>>>>
>>>>>   Yup, it enables the PersistenceSession/****AdapterManager to be
>>>>> able to
>>>>>
>>>> do
>>>> optimistic locking on behalf of all viewers.
>>>>
>>>> Previously this was a responsibility of every viewer; each viewer impl
>>>> had
>>>> to somehow hold a cache of OIDs against versions and detect if the
>>>> object
>>>> returned by the PersistenceSession had changed compared to "last time".
>>>>    This was complicated by the fact that different objectstores had
>>>> different
>>>> Oid implementations, and not every viewer could be guaranteed to work
>>>> against every object store.  As you'll recall, our Oid implementations
>>>> are
>>>> now fixed: RootOidDefault, AggregatedOid, CollectionOid; these all
>>>> support
>>>> serialization into strings (useful for all viewers, in particular for
>>>> the
>>>> restfulobjects viewer).
>>>>
>>>> When I was working on the wicket viewer - which up until that point
>>>> didn't
>>>> have any optimistic locking support - I decided to move this
>>>> responsibility
>>>> for optimistic locking checking into the PersistenceSession.  This tied
>>>> in
>>>> with the simplification of the Oid into a single implementation, so that
>>>> the responsibility of object stores is merely to generate IDs.  (Again
>>>> as
>>>> you'll recall, the old OidGenerator interface has been replaced by
>>>> IdentifierGenerator)
>>>>
>>>> The last thing to state is that the PersistenceSession/****
>>>> AdapterManager
>>>>
>>>> API
>>>> to retrieve objects has been overloaded so that a viewer can optionally
>>>> specify whether an optimistic locking check should be performed.  This
>>>> is
>>>> in AdapterManager API:
>>>>
>>>>       ObjectAdapter adapterFor(TypedOid oid);
>>>>       ObjectAdapter adapterFor(TypedOid oid, ConcurrencyChecking
>>>> concurrencyChecking);
>>>>
>>>> The first of these delegates to the second, with
>>>> ConcurrencyChecking.NO_CHECK.  So for viewers (like HTML viewer and,
>>>> perhaps? Scimpi) that still do their own optimistic locking, it should
>>>> be
>>>> easy enough to replace the calls to adapterFor(oid) with adapterFor(oid,
>>>> ConcurrencyChecking.CHECK) wherever it is required.
>>>>
>>>>
>>>> So, to summarize:
>>>> - object stores are responsible for persisting objects and identifying
>>>> those objects with an Oid
>>>> - the PersistenceSession supplements those Oids with version info for
>>>> optimistic locking
>>>> - the viewers, when calling into the PersistenceSession, can use the
>>>> correct version of adapterFor(...) to indicate whether or not they wish
>>>> for
>>>> an optimistic locking check to be performed.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>   This breaks some things though, or that extra could at least be
>>>>
>>>>> redundant.
>>>>> I guess the real question is why this is the default behaviour rather
>>>>> than
>>>>> a special behaviour.
>>>>>
>>>>>   I don't think it does break things; or at least, if there are any
>>>>>
>>>> viewers
>>>> that are parsing an oid string, then they shouldn't.
>>>>
>>>> The default behaviour of the system is still NOT to perform optimistic
>>>> locking, unless asked.
>>>>
>>>>
>>>>
>>>>
>>>>   As an example, my data show this
>>>>
>>>>>                 "inUseBy" : "CUS:28^8:admin:1354818065713"
>>>>>
>>>>> where before it would be this
>>>>>
>>>>>                 "inUseBy" : "CUS:28"
>>>>>
>>>>>
>>>>> I think this new form is the special case and not the normal case. That
>>>>> is, I think any existing code work the way it has and any code that
>>>>> wants
>>>>> to explicitly include the other details should make that change.
>>>>>
>>>>>   OK, so I disagree...   To use our new vocabulary, I've moved the
>>>>>
>>>> responsibility for optimistic locking into core (PersistenceSession)
>>>> rather
>>>> than in the components (viewers).  This seems absolutely correct to
>>>> me...
>>>> it is a really useful bit of core functionality.  It also gives
>>>> PersistenceSession a raison d'etre: it does more than just wrap an
>>>> ObjectStore impl, it also takes care of lifecycle management of the
>>>> adapters, in particular with respect to optimistic locking.
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>>   Regards
>>>>
>>>>> Rob
>>>>>
>>>>>
>>>>>
>

Re: OID strings

[Robert Matthews <rm...@nakedobjects.org>]

I'm now running a patched version with this change and all is working again.

Before you press ahead with changes to the PersistenceManager it not 
just the object stores that need just the identity, another example is 
in a URL for a web page showing an object. If that page can be 
bookmarked then just the identity is needed, whereas a form that will 
operate on an object will be one (hidden) field lighter with the merged 
version. I assume that restfulObjects needs to consider the same thing.

Rob

On 12/07/12 11:36, Dan Haywood wrote:
> On 7 December 2012 09:43, Robert Matthews <rm...@nakedobjects.org>wrote:
>
>> I think I'm presuming that the enstring mechanism was there first and was
>> doing the work that use to be done with the identifiers in the viewers and
>> elsewhere, but this change and your centralizing of the optimistic lock
>> checking came along together.  I guess when you replaced the uses of the
>> old OIDs you or I just used the enstring method globally.  Seaching through
>> there are number of places where the non-versioned one are actually needed,
>> including the SQL Store. So we just need to finish that off. I'll try that
>> later today as I need to get objects persisting in my application.
>>
> OK, that's a good point.  The RootOidDefault class has an
> "enStringNoVersion()" method that could be used.
>
> Or - and perhaps more correct - the PersistenceManager could "strip out"
> the version information whenever it passes an oid to an objectstore.  This
> could perhaps be through a new RootOidDefault#stripVersion() method.  It
> ought to return a new instance of RootOidDefault, though ... oids are
> immutable.
>
> Dan
>
> PS: as a by-the-by, oids also have value semantics, hence implement equals
> and hashCode.  Note that the version information is NOT part of the
> equality check... this is by design.
>
>
>
>
>> Rob
>>
>>
>>
>>
>> On 12/07/12 08:33, Dan Haywood wrote:
>>
>>> On 6 December 2012 18:57, Robert Matthews <rm...@nakedobjects.org>**
>>> wrote:
>>>
>>>   Dan
>>>> Can you clarify the changes to OID encoding and decoding to and from
>>>> Strings.  I see that you have allowed the version and user to be
>>>> included,
>>>> which could be useful in certain circumstances.
>>>>
>>>>   Yup, it enables the PersistenceSession/**AdapterManager to be able to
>>> do
>>> optimistic locking on behalf of all viewers.
>>>
>>> Previously this was a responsibility of every viewer; each viewer impl had
>>> to somehow hold a cache of OIDs against versions and detect if the object
>>> returned by the PersistenceSession had changed compared to "last time".
>>>    This was complicated by the fact that different objectstores had
>>> different
>>> Oid implementations, and not every viewer could be guaranteed to work
>>> against every object store.  As you'll recall, our Oid implementations are
>>> now fixed: RootOidDefault, AggregatedOid, CollectionOid; these all support
>>> serialization into strings (useful for all viewers, in particular for the
>>> restfulobjects viewer).
>>>
>>> When I was working on the wicket viewer - which up until that point didn't
>>> have any optimistic locking support - I decided to move this
>>> responsibility
>>> for optimistic locking checking into the PersistenceSession.  This tied in
>>> with the simplification of the Oid into a single implementation, so that
>>> the responsibility of object stores is merely to generate IDs.  (Again as
>>> you'll recall, the old OidGenerator interface has been replaced by
>>> IdentifierGenerator)
>>>
>>> The last thing to state is that the PersistenceSession/**AdapterManager
>>> API
>>> to retrieve objects has been overloaded so that a viewer can optionally
>>> specify whether an optimistic locking check should be performed.  This is
>>> in AdapterManager API:
>>>
>>>       ObjectAdapter adapterFor(TypedOid oid);
>>>       ObjectAdapter adapterFor(TypedOid oid, ConcurrencyChecking
>>> concurrencyChecking);
>>>
>>> The first of these delegates to the second, with
>>> ConcurrencyChecking.NO_CHECK.  So for viewers (like HTML viewer and,
>>> perhaps? Scimpi) that still do their own optimistic locking, it should be
>>> easy enough to replace the calls to adapterFor(oid) with adapterFor(oid,
>>> ConcurrencyChecking.CHECK) wherever it is required.
>>>
>>>
>>> So, to summarize:
>>> - object stores are responsible for persisting objects and identifying
>>> those objects with an Oid
>>> - the PersistenceSession supplements those Oids with version info for
>>> optimistic locking
>>> - the viewers, when calling into the PersistenceSession, can use the
>>> correct version of adapterFor(...) to indicate whether or not they wish
>>> for
>>> an optimistic locking check to be performed.
>>>
>>>
>>>
>>>
>>>
>>>
>>>   This breaks some things though, or that extra could at least be
>>>> redundant.
>>>> I guess the real question is why this is the default behaviour rather
>>>> than
>>>> a special behaviour.
>>>>
>>>>   I don't think it does break things; or at least, if there are any
>>> viewers
>>> that are parsing an oid string, then they shouldn't.
>>>
>>> The default behaviour of the system is still NOT to perform optimistic
>>> locking, unless asked.
>>>
>>>
>>>
>>>
>>>   As an example, my data show this
>>>>                 "inUseBy" : "CUS:28^8:admin:1354818065713"
>>>>
>>>> where before it would be this
>>>>
>>>>                 "inUseBy" : "CUS:28"
>>>>
>>>>
>>>> I think this new form is the special case and not the normal case. That
>>>> is, I think any existing code work the way it has and any code that wants
>>>> to explicitly include the other details should make that change.
>>>>
>>>>   OK, so I disagree...   To use our new vocabulary, I've moved the
>>> responsibility for optimistic locking into core (PersistenceSession)
>>> rather
>>> than in the components (viewers).  This seems absolutely correct to me...
>>> it is a really useful bit of core functionality.  It also gives
>>> PersistenceSession a raison d'etre: it does more than just wrap an
>>> ObjectStore impl, it also takes care of lifecycle management of the
>>> adapters, in particular with respect to optimistic locking.
>>>
>>> Dan
>>>
>>>
>>>
>>>   Regards
>>>> Rob
>>>>
>>>>

Re: OID strings

[Dan Haywood <da...@haywood-associates.co.uk>]

On 7 December 2012 09:43, Robert Matthews <rm...@nakedobjects.org>wrote:

> I think I'm presuming that the enstring mechanism was there first and was
> doing the work that use to be done with the identifiers in the viewers and
> elsewhere, but this change and your centralizing of the optimistic lock
> checking came along together.  I guess when you replaced the uses of the
> old OIDs you or I just used the enstring method globally.  Seaching through
> there are number of places where the non-versioned one are actually needed,
> including the SQL Store. So we just need to finish that off. I'll try that
> later today as I need to get objects persisting in my application.
>

OK, that's a good point.  The RootOidDefault class has an
"enStringNoVersion()" method that could be used.

Or - and perhaps more correct - the PersistenceManager could "strip out"
the version information whenever it passes an oid to an objectstore.  This
could perhaps be through a new RootOidDefault#stripVersion() method.  It
ought to return a new instance of RootOidDefault, though ... oids are
immutable.

Dan

PS: as a by-the-by, oids also have value semantics, hence implement equals
and hashCode.  Note that the version information is NOT part of the
equality check... this is by design.




>
> Rob
>
>
>
>
> On 12/07/12 08:33, Dan Haywood wrote:
>
>> On 6 December 2012 18:57, Robert Matthews <rm...@nakedobjects.org>**
>> wrote:
>>
>>  Dan
>>>
>>> Can you clarify the changes to OID encoding and decoding to and from
>>> Strings.  I see that you have allowed the version and user to be
>>> included,
>>> which could be useful in certain circumstances.
>>>
>>>  Yup, it enables the PersistenceSession/**AdapterManager to be able to
>> do
>> optimistic locking on behalf of all viewers.
>>
>> Previously this was a responsibility of every viewer; each viewer impl had
>> to somehow hold a cache of OIDs against versions and detect if the object
>> returned by the PersistenceSession had changed compared to "last time".
>>   This was complicated by the fact that different objectstores had
>> different
>> Oid implementations, and not every viewer could be guaranteed to work
>> against every object store.  As you'll recall, our Oid implementations are
>> now fixed: RootOidDefault, AggregatedOid, CollectionOid; these all support
>> serialization into strings (useful for all viewers, in particular for the
>> restfulobjects viewer).
>>
>> When I was working on the wicket viewer - which up until that point didn't
>> have any optimistic locking support - I decided to move this
>> responsibility
>> for optimistic locking checking into the PersistenceSession.  This tied in
>> with the simplification of the Oid into a single implementation, so that
>> the responsibility of object stores is merely to generate IDs.  (Again as
>> you'll recall, the old OidGenerator interface has been replaced by
>> IdentifierGenerator)
>>
>> The last thing to state is that the PersistenceSession/**AdapterManager
>> API
>> to retrieve objects has been overloaded so that a viewer can optionally
>> specify whether an optimistic locking check should be performed.  This is
>> in AdapterManager API:
>>
>>      ObjectAdapter adapterFor(TypedOid oid);
>>      ObjectAdapter adapterFor(TypedOid oid, ConcurrencyChecking
>> concurrencyChecking);
>>
>> The first of these delegates to the second, with
>> ConcurrencyChecking.NO_CHECK.  So for viewers (like HTML viewer and,
>> perhaps? Scimpi) that still do their own optimistic locking, it should be
>> easy enough to replace the calls to adapterFor(oid) with adapterFor(oid,
>> ConcurrencyChecking.CHECK) wherever it is required.
>>
>>
>> So, to summarize:
>> - object stores are responsible for persisting objects and identifying
>> those objects with an Oid
>> - the PersistenceSession supplements those Oids with version info for
>> optimistic locking
>> - the viewers, when calling into the PersistenceSession, can use the
>> correct version of adapterFor(...) to indicate whether or not they wish
>> for
>> an optimistic locking check to be performed.
>>
>>
>>
>>
>>
>>
>>  This breaks some things though, or that extra could at least be
>>> redundant.
>>> I guess the real question is why this is the default behaviour rather
>>> than
>>> a special behaviour.
>>>
>>>  I don't think it does break things; or at least, if there are any
>> viewers
>> that are parsing an oid string, then they shouldn't.
>>
>> The default behaviour of the system is still NOT to perform optimistic
>> locking, unless asked.
>>
>>
>>
>>
>>  As an example, my data show this
>>>
>>>                "inUseBy" : "CUS:28^8:admin:1354818065713"
>>>
>>> where before it would be this
>>>
>>>                "inUseBy" : "CUS:28"
>>>
>>>
>>> I think this new form is the special case and not the normal case. That
>>> is, I think any existing code work the way it has and any code that wants
>>> to explicitly include the other details should make that change.
>>>
>>>  OK, so I disagree...   To use our new vocabulary, I've moved the
>> responsibility for optimistic locking into core (PersistenceSession)
>> rather
>> than in the components (viewers).  This seems absolutely correct to me...
>> it is a really useful bit of core functionality.  It also gives
>> PersistenceSession a raison d'etre: it does more than just wrap an
>> ObjectStore impl, it also takes care of lifecycle management of the
>> adapters, in particular with respect to optimistic locking.
>>
>> Dan
>>
>>
>>
>>  Regards
>>>
>>> Rob
>>>
>>>
>

Re: OID strings

[Robert Matthews <rm...@nakedobjects.org>]

I think I'm presuming that the enstring mechanism was there first and 
was doing the work that use to be done with the identifiers in the 
viewers and elsewhere, but this change and your centralizing of the 
optimistic lock checking came along together.  I guess when you replaced 
the uses of the old OIDs you or I just used the enstring method 
globally.  Seaching through there are number of places where the 
non-versioned one are actually needed, including the SQL Store. So we 
just need to finish that off. I'll try that later today as I need to get 
objects persisting in my application.

Rob



On 12/07/12 08:33, Dan Haywood wrote:
> On 6 December 2012 18:57, Robert Matthews <rm...@nakedobjects.org>wrote:
>
>> Dan
>>
>> Can you clarify the changes to OID encoding and decoding to and from
>> Strings.  I see that you have allowed the version and user to be included,
>> which could be useful in certain circumstances.
>>
> Yup, it enables the PersistenceSession/AdapterManager to be able to do
> optimistic locking on behalf of all viewers.
>
> Previously this was a responsibility of every viewer; each viewer impl had
> to somehow hold a cache of OIDs against versions and detect if the object
> returned by the PersistenceSession had changed compared to "last time".
>   This was complicated by the fact that different objectstores had different
> Oid implementations, and not every viewer could be guaranteed to work
> against every object store.  As you'll recall, our Oid implementations are
> now fixed: RootOidDefault, AggregatedOid, CollectionOid; these all support
> serialization into strings (useful for all viewers, in particular for the
> restfulobjects viewer).
>
> When I was working on the wicket viewer - which up until that point didn't
> have any optimistic locking support - I decided to move this responsibility
> for optimistic locking checking into the PersistenceSession.  This tied in
> with the simplification of the Oid into a single implementation, so that
> the responsibility of object stores is merely to generate IDs.  (Again as
> you'll recall, the old OidGenerator interface has been replaced by
> IdentifierGenerator)
>
> The last thing to state is that the PersistenceSession/AdapterManager API
> to retrieve objects has been overloaded so that a viewer can optionally
> specify whether an optimistic locking check should be performed.  This is
> in AdapterManager API:
>
>      ObjectAdapter adapterFor(TypedOid oid);
>      ObjectAdapter adapterFor(TypedOid oid, ConcurrencyChecking
> concurrencyChecking);
>
> The first of these delegates to the second, with
> ConcurrencyChecking.NO_CHECK.  So for viewers (like HTML viewer and,
> perhaps? Scimpi) that still do their own optimistic locking, it should be
> easy enough to replace the calls to adapterFor(oid) with adapterFor(oid,
> ConcurrencyChecking.CHECK) wherever it is required.
>
>
> So, to summarize:
> - object stores are responsible for persisting objects and identifying
> those objects with an Oid
> - the PersistenceSession supplements those Oids with version info for
> optimistic locking
> - the viewers, when calling into the PersistenceSession, can use the
> correct version of adapterFor(...) to indicate whether or not they wish for
> an optimistic locking check to be performed.
>
>
>
>
>
>
>> This breaks some things though, or that extra could at least be redundant.
>> I guess the real question is why this is the default behaviour rather than
>> a special behaviour.
>>
> I don't think it does break things; or at least, if there are any viewers
> that are parsing an oid string, then they shouldn't.
>
> The default behaviour of the system is still NOT to perform optimistic
> locking, unless asked.
>
>
>
>
>> As an example, my data show this
>>
>>                "inUseBy" : "CUS:28^8:admin:1354818065713"
>>
>> where before it would be this
>>
>>                "inUseBy" : "CUS:28"
>>
>>
>> I think this new form is the special case and not the normal case. That
>> is, I think any existing code work the way it has and any code that wants
>> to explicitly include the other details should make that change.
>>
> OK, so I disagree...   To use our new vocabulary, I've moved the
> responsibility for optimistic locking into core (PersistenceSession) rather
> than in the components (viewers).  This seems absolutely correct to me...
> it is a really useful bit of core functionality.  It also gives
> PersistenceSession a raison d'etre: it does more than just wrap an
> ObjectStore impl, it also takes care of lifecycle management of the
> adapters, in particular with respect to optimistic locking.
>
> Dan
>
>
>
>> Regards
>>
>> Rob
>>

Re: OID strings

[Dan Haywood <da...@haywood-associates.co.uk>]

On 6 December 2012 18:57, Robert Matthews <rm...@nakedobjects.org>wrote:

> Dan
>
> Can you clarify the changes to OID encoding and decoding to and from
> Strings.  I see that you have allowed the version and user to be included,
> which could be useful in certain circumstances.
>

Yup, it enables the PersistenceSession/AdapterManager to be able to do
optimistic locking on behalf of all viewers.

Previously this was a responsibility of every viewer; each viewer impl had
to somehow hold a cache of OIDs against versions and detect if the object
returned by the PersistenceSession had changed compared to "last time".
 This was complicated by the fact that different objectstores had different
Oid implementations, and not every viewer could be guaranteed to work
against every object store.  As you'll recall, our Oid implementations are
now fixed: RootOidDefault, AggregatedOid, CollectionOid; these all support
serialization into strings (useful for all viewers, in particular for the
restfulobjects viewer).

When I was working on the wicket viewer - which up until that point didn't
have any optimistic locking support - I decided to move this responsibility
for optimistic locking checking into the PersistenceSession.  This tied in
with the simplification of the Oid into a single implementation, so that
the responsibility of object stores is merely to generate IDs.  (Again as
you'll recall, the old OidGenerator interface has been replaced by
IdentifierGenerator)

The last thing to state is that the PersistenceSession/AdapterManager API
to retrieve objects has been overloaded so that a viewer can optionally
specify whether an optimistic locking check should be performed.  This is
in AdapterManager API:

    ObjectAdapter adapterFor(TypedOid oid);
    ObjectAdapter adapterFor(TypedOid oid, ConcurrencyChecking
concurrencyChecking);

The first of these delegates to the second, with
ConcurrencyChecking.NO_CHECK.  So for viewers (like HTML viewer and,
perhaps? Scimpi) that still do their own optimistic locking, it should be
easy enough to replace the calls to adapterFor(oid) with adapterFor(oid,
ConcurrencyChecking.CHECK) wherever it is required.


So, to summarize:
- object stores are responsible for persisting objects and identifying
those objects with an Oid
- the PersistenceSession supplements those Oids with version info for
optimistic locking
- the viewers, when calling into the PersistenceSession, can use the
correct version of adapterFor(...) to indicate whether or not they wish for
an optimistic locking check to be performed.






> This breaks some things though, or that extra could at least be redundant.
> I guess the real question is why this is the default behaviour rather than
> a special behaviour.
>

I don't think it does break things; or at least, if there are any viewers
that are parsing an oid string, then they shouldn't.

The default behaviour of the system is still NOT to perform optimistic
locking, unless asked.




>
> As an example, my data show this
>
>               "inUseBy" : "CUS:28^8:admin:1354818065713"
>
> where before it would be this
>
>               "inUseBy" : "CUS:28"
>
>
> I think this new form is the special case and not the normal case. That
> is, I think any existing code work the way it has and any code that wants
> to explicitly include the other details should make that change.
>

OK, so I disagree...   To use our new vocabulary, I've moved the
responsibility for optimistic locking into core (PersistenceSession) rather
than in the components (viewers).  This seems absolutely correct to me...
it is a really useful bit of core functionality.  It also gives
PersistenceSession a raison d'etre: it does more than just wrap an
ObjectStore impl, it also takes care of lifecycle management of the
adapters, in particular with respect to optimistic locking.

Dan



>
> Regards
>
> Rob
>