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
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>
--
Florent Guillaume, Director of R&D, Nuxeo
Open Source, Java EE based, Enterprise Content Management (ECM)
http://www.nuxeo.com http://www.nuxeo.org +33 1 40 33 79 87