On 7 December 2012 09:43, Robert Matthews <[email protected]>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 <[email protected]>** >> 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 >>> >>> >
