26 Mar
2012
26 Mar
'12
7:10 p.m.
On Mon, Mar 26, 2012 at 18:21, Vincent Massol <vincent(a)massol.net> wrote:
This is precisely not what we want at the end. This is currently a
limitation of the store, not a limitation of the object API. Since the
object API has been written to support this, it would be a pity to abandon
it because the store currently does not allow 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:
There is nothing wrong on the long run, when we get a store that support
it. But for now, this is always the sign of a mistake. And this mistake has
been hidden by the current implementation, and is now coming to surface.
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.
This is nothing new or recent, it has always been that way. XClass
reference may be relative to the document containing the XObjects, which
allows cloning document between wikis without necessarily creating XObjects
that reference classes from the source wiki (Self containing document is
also workable, and space relative as well).
Using absolute reference is not wrong, but create absolute reference to the
XClass even when the XObject is cloned. This has been voted when the API
has been defined. This works in the same way parent reference works.
Are we talking about the same thing?
Probably not? We are not changing the API in fact. But since our current
store only support local XClass, and that referencing external XClass get
finally converted to local XClass by the implementation at the store level,
users (developers) of the API has not noticed they are incorrectly using
absolute references. So, what we want to do temporarily is keep this wrong
behavior doing it right with a warning in the logs, to allow gently
adapting existing unnoticed wrong usage. Not only ours, but also those of
users accessing this API. All it breaks is a feature that do not work
anyway with the current store.
Moreover the current store create absolute references while it store
relative one. This is obviously a mistake that stay unnoticed. The same is
true for the add XObject action.
Said another way, currently you have potentially different absolute
references that finally look at the same XClass, but potentially this
XClass is loaded several time with different references, since the wiki
part is not taken into account by the store. This unexpected feature does
not work anymore in 4.x since we use Object reference to compute IDs.
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 unique
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
--
Denis Gervalle
SOFTEC sa - CEO
eGuilde sarl - CTO
** 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