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 >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>>> >>> >>> >> >> >> >
