I would like to see that sorted out soon. There are more and more users of OpenCMIS and API changes are usually painful for them. We should also consider a new release (0.2.0) when this is done.
So, where are we? The main question is where we want to handle transient data. We have two opinions: 1. Transient and non-transient data access should be covered by the CmisObject interface. (Florent) 2. Transient data access should be covered by a new interface and non-transient data access should be covered by the CmisObject interface. (Florian) Personally, I still want to go for option 2 since it provides cleaner semantics -- especially in multi-threaded environments. Other opinions? - Florian On 08/11/2010 11:51, David Caruana wrote: > Have we reached a conclusion on this issue? It's been a while since we last > discussed. > > Regards, > Dave > > On 22 Oct 2010, at 15:06, Florian Müller wrote: > >> Oh, right, there is yet another option. :) >> >> I support all your arguments against b), c) and d). But there are also two >> issues with e). >> 1. Which value wins if a property is changed in the transient object _and_ >> present in the map? >> 2. save() stores the whole object. If we make ACLs and Policies transient >> too (do we want that?) than updateProperties() has the side effect of >> storing those as well. >> >> Which leaves us with a) which is pretty strong. And in this case I prefer >> two interfaces because it much is easier to understand and provides more >> compile checks and less runtime surprises. >> >> >> - Florian >> >> >> On 22/10/2010 14:44, Florent Guillaume wrote: >>> On Fri, Oct 22, 2010 at 3:33 PM, Florian Müller >>> <[email protected]> wrote: >>>> So your proposal is to add a method getTransientObject() (or something >>>> like that) to CmisObject. And that method would return a transient, not >>>> thread-safe version of the object but with setProperty(), save(), etc. >>>> enabled? >>>> Is that correct? >>> >>> Yes. >>> >>>> In this case we should also add a method isTransient() to CmisObject. >>>> There should be a way to discover the state of the CmisObject. >>> >>> Yes that would be useful. >>> >>>> I suggest that if the object is not transient, the transient methods >>>> should throw an IllegalStateException. >>> >>> Ok. >>> >>>> I agree that keeping the number of interfaces and classes low helps to >>>> learn and understand an API. But blending two semantics in one class could >>>> be even more confusing. >>>> For example, what should happen if updateProperties(Map) is called on a >>>> transient object? There are multiple options: >>>> a) updateProperties(Map) throws an exception because it only works for >>>> non-transient objects. >>>> b) updateProperties(Map) writes instantly to the repository and the >>>> transient object is refreshed. >>> (which btw means that transient changes are lost) >>>> c) updateProperties(Map) writes instantly to the repository and but >>>> object keep its (now outdated) state. >>>> d) The property map is merged with the transient properties and written >>>> when save() is called. >>> >>> And also: >>> e) The property map is merged with the transient properties and then >>> save() is automatically called. >>> >>>> It is not obvious. All semantics would be right and wrong at the same time. >>> >>> Agreed, we have to make a choice. But we would make a similar choice >>> when deciding what methods would be available on the TransientDocument >>> interface and what they would do. >>> >>> But anyway my choices in order of preference would be e) then a). >>> I don't like b) because it loses changes. >>> I don't like c) because there's an issue with not keeping the changes in >>> order. >>> I don't like d) because if you didn't do any transient operation it >>> doesn't behave like its non-transient counterpart. >>> >>> Florent >>> >>>> >>>> >>>> On 22/10/2010 13:43, Florent Guillaume wrote: >>>>> Hi, >>>>> >>>>> Looking back at your example: >>>>> >>>>> Document doc = (Document) session.getObject(id); >>>>> TransientDocument transDoc = doc.createTransientDocument(): >>>>> transDoc.setProperty("prop1", "value1"); >>>>> transDoc.setProperty("prop2", "value2"); >>>>> transDoc.save(); >>>>> >>>>> I agree with the need to explicitly getting a transient object if you >>>>> need different semantics. >>>>> One thing that will be painful though is having different classes: >>>>> TransientCmisObject, TransientDocument, TransientFolder, etc. >>>>> Couldn't we simply have all the methods available on the CmisObject, >>>>> Document, Folder, etc. and make them throw >>>>> UnsupportedOperationException when called on a non-transient object? >>>>> This would tremendously help learning about the API and avoid juggling >>>>> with many different interfaces for the users. >>>>> >>>>> Florent >>>>> >>>>> >>>>> >>>>> >>>>> On Mon, Oct 18, 2010 at 10:43 PM, Florian Müller >>>>> <[email protected]> wrote: >>>>>> Hi Florent, >>>>>> >>>>>> Well, no, there would be no CmisObject.save() method. The save() method >>>>>> would only be on the transient object. >>>>>> >>>>>> CmisObject would have updateProperties(Map), applyAcl(List<Ace>, >>>>>> List<Ace>, >>>>>> AclPropagation), applyPolicy(ObjectId), etc. And all of them would write >>>>>> instantly to the repository and refresh the object. >>>>>> >>>>>> >>>>>> Florian >>>>>> >>>>>> >>>>>> On 18/10/2010 17:49, Florent Guillaume wrote: >>>>>>> >>>>>>> Ok understood and agreed. There would be a new CmisObject.save() to >>>>>>> replace updateProperties() then? That works for me as it's a less >>>>>>> restrictive name that allows for different implementations of the >>>>>>> transient space that don't deal with just properties. >>>>>>> >>>>>>> Regarding your examples I don't need to do all that, because in the >>>>>>> Nuxeo use cases I described the client Session I'm providing to the >>>>>>> user of the API is not a PersistentSessionImpl but a completely new >>>>>>> Nuxeo class that does all the wrapping it needs around native Nuxeo >>>>>>> objects. So I'm not constrained by the semantics of the current >>>>>>> PersistentSessionImpl. It's just that I need a save()-like API to >>>>>>> exist on the CmisObject. And of course I want to have semantics that >>>>>>> are not too far to what's available when you use an actual remote >>>>>>> connection using pure OpenCMIS PersistentSessionImpl. >>>>>>> >>>>>>> Florent >>>>>>> >>>>>>> >>>>>>> On Mon, Oct 18, 2010 at 6:06 PM, Florian Müller >>>>>>> <[email protected]> wrote: >>>>>>>> >>>>>>>> Hi Florent, >>>>>>>> >>>>>>>> We don't want to remove the transient space entirely. The proposal is >>>>>>>> to >>>>>>>> detach the transient from the non-transient part. We would have two >>>>>>>> separate >>>>>>>> objects. >>>>>>>> >>>>>>>> The non-transient object is always consistent, can be shared across >>>>>>>> threads >>>>>>>> and is cached. Changes are directly written to the repository. This >>>>>>>> object >>>>>>>> would have an updateProperties(Map) method that immediately refreshes >>>>>>>> the >>>>>>>> object after the update. >>>>>>>> >>>>>>>> The transient object is owned by one thread and not thread safe. That >>>>>>>> should >>>>>>>> prevent inconsistent views on the object. This object would have >>>>>>>> setProperty() and save() methods. Internally it would use the >>>>>>>> non-transient >>>>>>>> object to access all unchanged data. It's a wrapper around the >>>>>>>> non-transient >>>>>>>> object that holds the transient data. >>>>>>>> >>>>>>>> So you still could use the pattern that you are using today. The only >>>>>>>> difference would be that you have to create a transient wrapper object. >>>>>>>> >>>>>>>> >>>>>>>> That could look like this: >>>>>>>> >>>>>>>> Document doc = (Document) session.getObject(id); >>>>>>>> TransientDocument transDoc = new TransientDocument(doc); >>>>>>>> >>>>>>>> transDoc.setProperty("prop1", "value1"); >>>>>>>> transDoc.setProperty("prop2", "value2"); >>>>>>>> transDoc.save(); // that also refreshes the underlying non-transient >>>>>>>> object >>>>>>>> >>>>>>>> >>>>>>>> Or maybe we do something like this: >>>>>>>> >>>>>>>> Document doc = (Document) session.getObject(id); >>>>>>>> TransientDocument transDoc = doc.createTransientDocument(): >>>>>>>> >>>>>>>> transDoc.setProperty("prop1", "value1"); >>>>>>>> transDoc.setProperty("prop2", "value2"); >>>>>>>> transDoc.save(); >>>>>>>> >>>>>>>> >>>>>>>> Florian >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 18/10/2010 16:07, Florent Guillaume wrote: >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> In Nuxeo the OpenCMIS client API is made available locally as another >>>>>>>>> API to manipulate documents, in addition to the Nuxeo native APIs. >>>>>>>>> >>>>>>>>> I have a problem with removing CmisObject.updateProperties() and the >>>>>>>>> mini transient space because for me it's quite useful. The Nuxeo >>>>>>>>> internal non-CMIS session has a notion of a property-only transient >>>>>>>>> space, so in Nuxeo you update several properties and do some kind of >>>>>>>>> object save() to flush them. Without a similar flushing concept on the >>>>>>>>> OpenCMIS client API, I'm obliged to make a flush on every property >>>>>>>>> write, which leads to severely degraded performance. >>>>>>>>> >>>>>>>>> If all method calls on a CmisObject immediately write everything >>>>>>>>> through the network and potentially refetch a full object there will >>>>>>>>> be many people that aren't happy with CMIS performance once they get >>>>>>>>> to use OpenCMIS. The client API is supposed to provide some >>>>>>>>> convenience to the user, and having a mini transient space for >>>>>>>>> properties is IMHO the very first step of convenience. There may be >>>>>>>>> problems with the semantics of interactions between this transient >>>>>>>>> space and caching / refetches, but let's solve them rather than remove >>>>>>>>> them. >>>>>>>>> >>>>>>>>> Dave wrote: >>>>>>>>>> >>>>>>>>>> For example, with a transient space, what is the behaviour when >>>>>>>>>> setProperty has been called and an "update" method is then called >>>>>>>>>> prior >>>>>>>>>> to >>>>>>>>>> updateProperties. Are the transient changes discarded, flushed, or >>>>>>>>>> just >>>>>>>>>> left >>>>>>>>>> as is? >>>>>>>>> >>>>>>>>> So if I understand correctly you're talking about the case: >>>>>>>>> 1. doc.setPropertyValue("foo", ...); >>>>>>>>> 2. doc.updateProperties(map); >>>>>>>>> 3. doc.setPropertyValue("bar", ...); >>>>>>>>> 4. doc.updateProperties(); >>>>>>>>> >>>>>>>>> And you're asking if the "foo" change is sent with 2. or with 4. or >>>>>>>>> discarded? I'd say let's keep this undefined, as I feel that it's a >>>>>>>>> use pattern that's not natural. If we really want to specify this then >>>>>>>>> I have no problem mandating that the "foo" change should be sent with >>>>>>>>> 2. by saying that updateProperties(map) is the same as >>>>>>>>> setPropertyValue() on all the properties of the map then calling >>>>>>>>> updateProperties() with the transient map. >>>>>>>>> >>>>>>>>> Florent >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, Oct 18, 2010 at 4:05 PM, Klevenz, Stephan >>>>>>>>> <[email protected]> wrote: >>>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> Coming back to the cache discussion. I would like to support this >>>>>>>>>> proposal by Dave/Florian and think we can also delete Methods on >>>>>>>>>> Session >>>>>>>>>> class like cancel() and save() which are currently not implemented. >>>>>>>>>> >>>>>>>>>> +1 for this: >>>>>>>>>> >>>>>>>>>>> - All write operations provided by CmisObject should automatically >>>>>>>>>>> do >>>>>>>>>>> an >>>>>>>>>>> object >>>>>>>>>>> refresh after the update. That guarantees that the object is always >>>>>>>>>>> consistent. >>>>>>>>>>> The cost for this consistency is an additional call to the >>>>>>>>>>> repository. >>>>>>>>>> >>>>>>>>>>> - Is some cases you don't need or want this addition cost. Lets say >>>>>>>>>>> you >>>>>>>>>>> just >>>>>>>>>>> want to update a bunch of objects but you don't work with them >>>>>>>>>>> afterwards. >>>>>>>>>>> That's what the operations provided by Session are good for. They >>>>>>>>>>> just >>>>>>>>>>> do >>>>>>>>>>> that and nothing else. >>>>>>>>>> >>>>>>>>>> About the transient support we can design this as an optional add on >>>>>>>>>> with >>>>>>>>>> additional interfaces and additional implementations. With a clear >>>>>>>>>> separation the API become easier to use. >>>>>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> Stephan
