On Thu, Mar 7, 2013 at 9:08 AM, Vincent Massol <vincent(a)massol.net> wrote:
On Mar 6, 2013, at 9:31 PM, Denis Gervalle <dgl(a)softec.lu> wrote:
Hi Vincent,
On Wed, Mar 6, 2013 at 4:48 PM, Vincent Massol <vincent(a)massol.net>
wrote:
>
> On Mar 6, 2013, at 1:02 PM, Denis Gervalle <dgl(a)softec.lu> wrote:
>
>> Hi Vincent,
>>
>> On Wed, Mar 6, 2013 at 11:36 AM, Vincent Massol <vincent(a)massol.net>
> wrote:
>>
>>> Resending since I've made mistakes (it's only about ObjectReference,
not
>>> Properties), sorry about that.
Here's the new version:
>>>
>>> ------------
>>>
>>> Hi devs,
>>>
>>> ATM in the model module there's no ability to reference an xobject
other
> than by using a free form name.
We made this on purpose.
I know, I worked on this… but it doesn't make it right… :)
So from this and what I read below, I understand you have suddenly decide
to revert the new model back to the old way of addressing objects and
stop
using named objects. This is a major change that
should be discussed
first
and does not have my support right now.
No I haven't changed that. I'm proposing to have both ways. Either by name
or by class ref+index. Same as currently.
>>> The problem is that this is not
really usable. This is why we
introduced
>>> the BaseObjectReference in oldcore.
>>>
>>
>> We have introduce the BaseObjectReference in oldcore simply to easily
>> support the current storage implementation. At that time, we had a
>> discussion about using the object number (not to be confuse with the
> index)
>> or the object UUID. And, since the object UUID was suspicious in some
> edge
>> case, you convince me to use the object number to create the free form
> name
>> of these old objects (I should have resist more since I still do not
have
evidence of these edge case).
What? That defeats one of the main goal of references which is that you
can directly address an entity without having retrieved it first!
True for the old object storage, but was a better a way to avoid what you
seems to advocate now !
The reason entities have names is to be able to
easily reference them.
Ah, you are right, object should have name to be addressable properly.
> Using a UUID makes it impossible to reference something without getting
it
first...
Was only about a temporary compatibility solution for old objects,
obviously not a good way to be able to create reference from scratch,
but a
good way to get stable reference from objects.
If to reference an Object I have to load the
Document first, then in
practice it means we don't need Object references and they shouldn't be
considered entities.
I disagree, you do not always have to load the document to know an object
UUID. You may retrieve the UUID of object by searching the database. This
is the only way I have found until now to strongly link object together
when multiple objects are stored in the same document. However, most of
the
time, I avoid this by creating only a single
object of a given class per
documents when I want to reference them elsewhere. So I may use the
document itself for reference.
For any entity, you should be able to construct a
direct reference to it
without needing to load anything else.
This is precisely why we need name for referencing our objects.
Either name or class ref + index.
In the majority of use cases people are not going to use names and we
still need to be able to create a reference to an object easily and class
ref + index is the best solution I know of for that.
Sure, but they will if they need it. They will need it if they want to
create reference to an object of a given class and there is more then one
of them in the document. Else, they will rely on an iterator and will not
create such reference. Once again, without looking the object themselves,
talking about the nth objects is a really uncertain bet that you will
really get the one you want.
> In the 7-8 years of XWiki we've used
Class reference and
numbers/position
to
reference xobjects and it has worked quite well IMO.
I use it since as long as you, and I have mostly worked without any
reference to objects since these were missing. I mean I have usually not
accessed object unless I have them at hand, since the only correct way to
get them was exactly what you explain just above. I do not remember
having
really retain object number to reference them at
any moment except for a
few line of code. I have surely never based anything on "this is the
third
object of that class in that document" since
this is not really stable in
the current model.
Yes the concept of ref did not exist but the old apis are doing the same,
i.e. finding objects by class ref + number.
> In most cases you have only 1 xobject of a
type and it's very nice to be
> able to say: here's a reference to the first object of type N in
document
P.
I said that I agree with this special case, which is really different IMO
than accessing the nth object of document. We had for this special case
helper function in XWiki since a long time and these revealed to be
helpful.
yup
> Exposing an internal id of a document as
reference is a no go for me
since
it cannot
be constructed without retrieving the entity first.
That is not my purpose, it was only my temporary solution waiting for
truly
named objects, it have been decided to use object
numbers (not index !),
but this has nothing to do with the current discussion anyway. Maybe this
decision influence you however.
>>> However this is major PITA since we can't have clean code that create
an
> object reference and that doesn't depend on
oldcore.
>
Creating a reference to the third object of a given class in a document
(or
object number 3, or even the third object of a
document) has absolutely
no
meaning unless you have already that object at
hand
Why doesn't it have a meaning? If in my app, my spec says:
- each doc has 3 objects of type P and the first one means this, the
second one that and the 3rd one that other thing,
then the 3rd one has a meaning.
And when any object get deleted, your meaning vanished. Great design of
course !
errr? Whether it's the first one or the Nth, if it's deleted then it's no
longer there obviously :)
Same, if you delete a doc and there's a ref to it, you'll get a non
existing document… nothing special here...
No, this is not the same. When you have only one object of that classname,
it will be there or not, but when you have several, deleting one of them
will change the index of others, and your reference will not refer the same
object anymore. This is why using index is poor for creating reference IMO.
>> , and so you already
>> have a source for its reference (solved not nicely by the
>> BaseObjectReference actually, but this was another story).
>>
>>
>>> I'd like to propose the following:
>>> * Modify ObjectReference to add 2 named parameters: Class reference
and
>>> position
>>>
>>
>> Since position is really fragile without any real meaning,
>> having a
>> reference using a position will be a source for spurious issues, I am
>> definitely -1 for any positional reference.
>>
>>
>>> * Make the name optional in EntityReference
>>>
>>
>> This sounds like a sign of bad design to me... what is a entity
without a
name in general ?
It means it's a reference to an entity by a mean other than a name :)
The mean is bad, the position may change each time object are added or
deleted. And I do not think we have real positioning in the current
model,
which means you cannot really ensure object order
at any time.
AFAIK we do have an order:
private Map<DocumentReference, List<BaseObject>> xObjects = new
TreeMap<DocumentReference, List<BaseObject>>();
This means that for each class ref, there's an ordered list.
But you have currently nothing for reordering that list. Explain me how,
let say, how you recreate the second object in a list of three once it have
been deleted. We have no API for that.
> We need to decide but we could very well
decide that some entities don't
> have a name and that they can be found/addressed by some other means
(as in
the case
of Object references when locating them through class reference
and position).
We may, but we need strong reason to introduce that IMO. And other means
should be solid, position is not.
It's almost as "solid" (or as "weak") as names for documents.
Those docs
can be renamed and your ref is gone!
Sure, gone but not mixed by the deletion of another document.
As soon as you use names and not ids in references you
have "weak"
referencing, be it for objects or any other entity. That's not a problem.
It has the advantages that refs can be constructed.
Sure, but constructing ref using index is hazardous since you could ensure
the list is the way you expect it to be. When using names, there is no risk
another object cause the one you want to reach to have change its name.
The only other solution I can think of (which wouldn't work with the
current model and thus we would need to modify the current model to make it
work before we can implement the new model api with the old model impl)
would be to **force** always having a name for Objects when they are
created and when the user doesn't specify the name it's computed
automatically using a default scheme. For example: <class ref wiki>:<class
ref space>.<class ref page>[number] where [number] isn't specified for the
first xobject but only for the next ones.
For example for a class ref of (wiki = my:wiki, space = my.space, page =
mypage) we would have "my:xwiki:my.space.mypage" for the first object and
"my:xwiki:my.space.mypage[1]" for the second one, etc.
Note that it wouldn't be a serialized ref since we need to make it as easy
as possible to create it using string concatenation. It should never be
used as a serialized class ref and not used to find the class ref. It's
just a string.
Moving the object around would keep this name (unless the user manually
changes it). And renaming the xclass document would also not change this
name.
So here's what we would need to write to get the avatar property for a
user:
DocumentReference userDocumentReference = ...
ObjectReference userObjectReference = new
ObjectReference("XWiki.XWikiUsers", userDocumentReference);
ObjectPropertyReference avatarReference = new
ObjectPropertyReference("avatar", userObjectReference);
String fileName = this.entityManager.getEntity(avatarReference).getValue();
Again, if we want to go this way we need to modify the oldcore to add
internal support for names to XObjects. This means modifying the DB schema,
not a very small change...
You do not need all that. If an object is not the first one (for which I
d'like your idea to have a special syntax to reach it by classname), and
you have not set a name on it, it means you will be unable to create a
reference to it without asking the object itself for its reference. The
object should be able to create a unique reference to itself, and this is
why I would have preferred the UID solution to the current
BaseObjectReference, but that could work too except for the annotation
maybe.
Which is why I think that using class ref + index,
*in addition to name*
is probably better for now.
You will not be referring a particular object that way, only a position in
a list of object, this is absolutely not the same. If you want that, call
it a ObjectIndex not an ObjectReference.
WDYT?
see more below
> More below
>
>>> This means that when we use an EntityReferenceResolver to resolve
>>> "wiki:space.page^wiki2:space2.page2" we get an ObjetReference
with:
>>> * name = null
>>> * param1: name = "classReference", value = EntityReference
>>> * param2: name = "objectPosition", value = 0
>>>
>>
>>> Rationale:
>>> * This is exactly what we already do for Locale (and what we'll do for
>>> Version too probably) so it's logical to do it for Object References
too
>>>
>>
>> I agree with your sample and your rationale, there is a need to create
a
>> reference to the first (and probably the
only for such use case) object
> of
>> a given class in a given document without having to compose weird names
> or
>> positional references. This has definitely a meaning, much more than
the
>> second or any numbered objects... And
this object is not always "[0]"
in
>> base reference syntax !
>
> ah good, was trying to despair :)
>
> If the first one has a meaning, then 2nd or 3rd can also have a meaning,
> see my example above. Obviously the majority of use cases will be the
first
one but
that doesn't mean the other positions are not needed.
0 = first not null xobject
1 = second not null xobject
...
When you use the first one, and have only one, you may ensure you get the
right one whatever the creation/deletion of object happened. This is not
true for the other one, and we do not have anything for that.
The reason I am -1 for using position, is mainly because position may
change overtime for the same object, so this is a really fragile
reference,
and obviously this is not good design at all for
maintaining a reference.
Position is perfect during a small piece of code, and bad to be stored
as a
reference.
>
>> However, resolving "wiki:space.page^wiki2:space2.page2" to that object
is
>> not valid, since you do not really know
what you are doing here, are
you
>> speaking about the first object of a
given class or an object named "
>> wiki2:space2.page2" ?
>
> Exactly. Which is why in my proposal we would have to agree that the
> current notation (thomas calls it syntax) only describes class reference
> and NOT a name (name = null). If we ever want to also be able to write a
> string and specify a name in it then we would need to invent a syntax,
in
> the same manner that we currently have no
syntax to express a locale in
a
document
reference when expressed as a string.
I see it the other way round.
>> So if we need to resolve a string into the first (or,
>> even if I am against, any) object of a given class, we need another
> syntax
>> at least (BaseObjectReference had already cause some poor stuff in
>> LocalUidReferenceSerializer where we had to remove wiki in a very bad
way
currently).
Using a name is very very far in the future so we don't need to invent a
syntax for that now IMO.
If it is, what is the purpose of renewing the model ? Why do you prefer
to
step backward instead of going forward to solve
our initial issue ?
It's not a step backward. We still have the name while at the same time
making it possible to also reference objects by a construction (class ref +
index). BTW I don't consider class ref + index a bad thing. IMO it was
pretty clever. Doing a new model doesn't mean throwing out all good ideas
that existed in the old model :)
This is where we are not agreeing. Object indexes are not bad
by themselves but they are unrelated with references. Using an object
index for a reference is bad.
Note that the current model does not use an index, but a number. That
number is not always equal to the index. It is somewhat more stable, but it
does not really help creating reference from scratch anyway. It is like the
UUID.
We
don't even know if it'll ever happen actually…
This really afraid me, no more ambition to get it right ?
No, you don't get it at all...
As a user I don't **care at all** about object names in 99% of the cases.
I don't want to name them as a user. That's a pain and a constraint uses
shouldn't care about.
Again we don't do that now and it works very well.
No need to have the name as a constraint. This is only a constraint if you
want to create a reference from scratch to a given object in a list of
object of the same class in a document.
Currently we do not create reference to object from scratch, so of course
it works well.
>>> Consequences:
>>> * We need to modify the Seralizers/Resolvers accordingly
>>>
>>
>> According we have a new syntax for your kind of object reference, we
may
> do
>> so. I resolver/serializer (as well as setter of class reference) I
> suggest
>> like Thomas that we support relative reference to classes based on the
>> containing document, both simplifying creation and avoiding
>> useless repetition.
>
> That seems good for now, even though in some far future we may want to
> have a class defined in a wiki and an xobject located in another
subwiki…
or not…
;)
Not repeating useless information does not mean we should not support
absolute reference as well. Thinking about it twice, I agrre with you
that
this is more subtle than it look, since copying a
document may be greatly
influence by this. We should discuss that further once we agree on the
rest.
>
>>> * We need to modify EntityReference to support a null name
>>>
>>
>> Do we really need that, or the name would simply reflect our special
> syntax
>> ?
>
> It would be slightly misleading to use the serialized class ref as the
> name IMO but why not... One danger is in code that will use it instead
of
using the
well-known parameters for class ref and position.
You cannot always prevent bad coding practice, and you may also think
about
the easy way some may want to use in velocity for
example even if this is
not optimal.
>
>>> * We deprecate BaseObjectReference
>>>
>>
>> Does it really true ? It does not have the same meaning (even for your
>> initial proposal) since number and position are not the same and the
>> BaseObjectReference is a special reference, not a general object
> reference,
>> since it does not have a real free form name. Deprecating means we had
to
>> replace them everywhere, and I am not
sure it is easy to use index in
> place
>> of number in some places, like storage where we use reference for IDs.
>
> Maybe, in any case what I meant is that we'll need to make our code use
> the new way and not use BaseObjectReference as much as possible.
>
>>> * Probably some other stuff to modify like modifying event listeners
>>> listening on objects since it's now going to be much easier, etc
>>>
>>> WDYT?
>>>
>>
>> So, to resume, I understand the need to have a way to create reference
to
the first object of given class in a document. Even if
using an object
reference with, let say a special syntax, will introduce some bad
consequences on the implementation of equals
equals/hashcode shoudl already work since they already take into account
parameters.
But comparing two references to the same object, one using the name of
the
object (once we have it of course) and the other
the classname will not
compare to be equal while these are. We have the exact same situation for
locale or version.
They won't be the same reference, which is ok. You can have several refs
leading to the same entity. There's no issue about that.
It means comparing Object references is not that same as comparing Object
entities.
>> and related methods on entity
>> reference (like those we have for locale or version), we should
probably
>> consider it (Note that it will also
affect property reference).
>>
>> I am definitely -1 for any positional reference which are meaningless.
>
> I think this is a pity and you're closing the door to lots of use cases
> and you're going to make them really hard and not performant to
implement
as a
consequence.
I am not closing anything, I am just not agreeing to create a solution
that
will encourage bad design.
As a user, if I **want** to access the 3rd xobject because it has a
meaning
for my
app, I'm not going to be able to do so.
As a user I want to access the right object, I do not want to care about
its place, I just need to be able to name it properly. Your use case is
simply wrong and induced by the current design.
I'm going to need to load the doc and do some
queries to iterate over
xobject and count to 3… (which obviously is both painful and not
performant).
And is need it anyway to access that object, it is you or the API, but it
will be done at some point.
> BTW your counter-suggestion to drop the position means you're going to
> break everything that already exists since it's currently possible to
> reference an object with class and number. It means inventing a new
syntax,
which is
always painful to do.
Have I said you cannot use the BaseObjectReference ?
You may do so, but you are stick with the oldcore and the old and bad way
of working with object.
Again, why not using our energy to find a nice way to reach the named
object solution instead of defending a bad design we suffer from since
really too long ?
But it's not bad at all. It's very good! Forcing users to create a name
for all objects they create would be a very bad solution… Autogenerating
the name would be better of course but think about it twice because there's
a huge cost to doing that in the old model as I've explained above.
What I wanted to do here was to make progress since I had 2-3 days to work
on the new model again.
I see it's not easy and as a consequence we're not going to be able to
progress. It's just sad. I had the energy to code this quickly, modify
UserAvatarMacro to use the new model API and commit all that in master for
5.0M2… I don't have it anymore, nor do I have the time now (I had only 2
days and they're now gone… :(…). It'll have to be someone else doing it..
or wait till I get 10 free days in 1 year time to implement object naming
in the current model with all the breakages this would cause…
I had a good solution that was working both with the old model (class ref
+ index - or numbers, whatever would work) and the new model (names when
they're implemented) and you're throwing it out… too bad...
Sorry about that, I found this sad as well. Anyway, this is too important
to be overlooked.
Thanks
-Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Denis Gervalle
SOFTEC sa - CEO
eGuilde sarl - CTO