28 Mar
2012
28 Mar
'12
9:42 a.m.
Thanks to Thomas and Denis for the explanations. I understand now, it's very clear :)
However I'd like to suggest a different solution.
Premises:
* 99% of the use cases are for relative references
* We need to make sure devs don't use the API wrongly and thus the API has to be easy
to use right
* We have never supported absolute references to XClasses (since we've never supported
this in the DB)
Based on these premises I'd like to suggest that:
* We automatically remove the wiki part when adding an XObject (specifically in
resolveClassReference())
* Later on when we implement a new storage that supports multiwiki for xclasses then we
add another method so that users who want to use an absolute reference do it voluntarily
and not by mistake. It could be: newAbsoluteXObject(…) for example. Alternatively we could
add: newXObject(EntityReference ref, boolean isAbsolute).
This will ensure that users of the api can't use it wrongly.
I really don't see any point in continuing to try to support a feature that
doesn't exist and that we can't support… It's better to add support for it
when we implement a storage that supports it.
Thanks
-Vincent
On Mar 26, 2012, at 8:07 PM, Thomas Mortagne wrote:
This has nothing to do with what we propose, the goal here is to find
code which is setting absolute reference for the class to that it's
fixed to not have issue when copying. If we do what you say then the
document copy will fail like it does now but simply for another reason
and it's not going to help fixing anything because the issue is not at
this level but in the code that put an absolute reference in the
original document.
On Mon, Mar 26, 2012 at 6:21 PM, Vincent Massol
<vincent(a)massol.net> wrote:
Got it!
Hi,
This looks like a very complex topic. Trying to answer below (or rather ask questions)…
On Mar 26, 2012, at 3:43 PM, Thomas Mortagne wrote:
It's not that simple, xclass reference in xobject are working like
parents reference in document: they are relative to keep what the user
explicitly asked even when the document is copied in another context.
So which reference is set to the object is very important because the
reference will be copied as it is when copying a document.
Here is the javadoc you wrote in BaseCollection:
/**
* The meaning of this reference fields depends on the element
represented. Examples:
* <ul>
* <li>If this BaseCollection instance represents an XObject then
refers to the document where the XObject's XClass
* is defined.</li>
* <li>If this BaseCollection instance represents an XClass then
it's not used.</li>
* </ul>
* Note that we're saving the XClass reference as a relative
reference instead of an absolute one. This is because
* We want the ability (for example) to create an XObject relative
to the current space or wiki so that a copy of
* that XObject would retain that relativity. This is for example
useful when copying a Wiki into another Wiki so
* that the copied XObject have a XClassReference pointing in the new wiki.
*/
private EntityReference xClassReference;
It's not only a storage issue, if you have an document object with
reference wiki1:Space:MyClass and copy that document to wiki2 then you
will have an object in wiki2 with class wiki1:Space:MyClass. The
storage does not have a word to said in this process it just get the
resulting new XWikiDocument to save and in this case we are asking the
store to store an object defined by a class which is in another wiki,
something that it does not support and more importantly something that
was not intended.
So we have several choices here:
1) continue hiding under the carpet that we are asking to the store
something that it can't do and have inconsistency between object and
storage and crappy hack here and there in the storage (in the case of
XWIKI-7628 it means not trusting what's returned by
BaseObject#getReference)
2) totally forget about relative xclass reference chosen outside of
BaseObject and always remove the wiki from the reference stored in
BaseCollection#xClassReference which mean that this API will
officially never support having an object with a class from another
wiki
3) keep the current API as it has been defined and voted and try to
make a bit more sure people are using it the way it's supposed to be
used
Here we are talking proposing 3) Hi devs,
we have an important inconsistency that we ignored since a long time
and that is producing http://jira.xwiki.org/browse/XWIKI-7628.
The main issue is that the storage save document containing object
linked explicitly to another wiki without complaining which mean that
user continue using API wrongly without really knowing it's wrong. And
now we have lots of code badly using object class related API and
contently creating object with absolute class references.
I don't see anything wrong with adding an XObject to a Document using an absolute
XClass reference. This is actually the API we've added and we've only added
relative XObject API more recently which IMO is the wrong one since we always said we
wanted to use absolute references everywhere in all APIs and it's up to the storage to
remove/add the wiki part.
Are we talking about the same thing? Until now it was OK even if a mess and it's still
not visible. But
that's mostly luck... until we start copying documents from one wiki
to another starting with 4.0 branch. What changed is that the uniquetemporary
identifier of the object in the database is "properly" generated by
using it's reference and since a class reference in a BaseObject
contain a wiki it's now taken into account. The old id generator used
to use the deprecated BaseCollection#getClassName which is forced
absolute for retro-compatibility reason even if it was actually
possible before to put absolutely anything as class reference.
So right now we have two issues:
* store save invalid document without complaining
* many code is wrongly using BaseObject and XWikiDocument API
regarding class references mostly because you don't really see it
right away when you don't use it properly. Even ObjectAddAction is
wrong, a lot of code that was using setClassName has been badly
converted to fix deprecated method code by resolving what was passed
to setClassName into an absolute reference which is very wrong and not
the behavior of setClassName.
Here is what we propose with Denis:
* 4.x (or until class coming from another wiki is supported but I
doubt this is going to happen anytime soon with this storage):
** throw an exception in the storage when saving a document containing
objects coming from another wiki
I don't like this. If the problem is that we don't want to allow an XObject to be
added when the XClass is coming from another wiki then this should be checked at the level
of the AddXObject APIs and throw an exception there.
Thanks
-Vincent
--
Thomas Mortagne
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs ** to temporary limit breakage while we fix wrong
code and help us
find this wrong code, modify BaseCollection#setXClassReference to
force it to remove the wiki from the passed reference and log a
warning about a bad usage
* 5.0: remove the bandage from BaseCollection#setXClassReference
WDYT ?
Here is my +1
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs