On Mon, Mar 26, 2012 at 18:21, Vincent Massol <[email protected]> wrote:
> 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: > > > 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. 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. > 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. > 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. > > Thanks > -Vincent > > > ** 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 > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
