On Thu, Mar 7, 2013 at 9:26 AM, Vincent Massol <[email protected]> wrote: > some amendments below. > > On Mar 7, 2013, at 9:08 AM, Vincent Massol <[email protected]> wrote: > >> >> On Mar 6, 2013, at 9:31 PM, Denis Gervalle <[email protected]> wrote: >> >>> Hi Vincent, >>> >>> >>> On Wed, Mar 6, 2013 at 4:48 PM, Vincent Massol <[email protected]> wrote: >>> >>>> >>>> On Mar 6, 2013, at 1:02 PM, Denis Gervalle <[email protected]> wrote: >>>> >>>>> Hi Vincent, >>>>> >>>>> On Wed, Mar 6, 2013 at 11:36 AM, Vincent Massol <[email protected]> >>>> 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. >> >>>> 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... >> >>>>> , 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. >> >>>> 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! >> >> 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. >> >> 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. > > Correction: remove the wiki part. > >> 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... >> >> Which is why I think that using class ref + index, *in addition to name* is >> probably better for now. >> >> 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 :) >> >>>> 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. >> >>>>>> 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. >> >>>>> 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… > > BTW I'm not saying that you're not justified in your answers. It's actually > quite good that you're taking the time to reply and brainstorm about it :) > > I just don't have much time to work on the new model and I'm frustrated that > I couldn't make progress as I would have liked in the little time I had > available. Starting next week it's going to be devoxx preparation and others > things, and I won't be able to work anymore on the new model for some time… > could be long... > > Looking at xwiki.hbm.xml I see that we have name, className and number so > maybe there's a solution to reuse the name property after all and not have to > change the schema.
This name field you are talking about is the document reference where this object is stored, not the object name. > > Thanks > -Vincent > > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
