Hi Florent,
Could you elaborate a bit more on that proposal?
Would TransientDocument.class be an Interface or a Class?
I like the idea of adapters. But it is not intuitive that there is an
adapter the provides a transient paradigm. Would it make sense to add
another method that provides a shortcut to the "transient adapter"?
Thanks,
Florian
On 10/11/2010 13:27, Florent Guillaume wrote:
I've come around to agreeing that 2. is probably better.
I can see either:
TransientDocument transDoc = doc.createTransientDocument():
or
TransientDocument transDoc = doc.getAdapter(TransientDocument.class);
The latter is more extensible for other uses besides transience
(protocol extensions, application-specific adapters).
WDYT?
Florent
On Wed, Nov 10, 2010 at 10:41 AM, Florian Müller
<[email protected]> wrote:
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
