On Tue, Nov 15, 2011 at 21:26, Vincent Massol <[email protected]> wrote:
> > On Nov 15, 2011, at 9:04 PM, Caleb James DeLisle wrote: > > > > On 11/15/2011 12:25 PM, Vincent Massol wrote: > >> > >> On Nov 15, 2011, at 5:29 PM, Denis Gervalle wrote: > >> > >>> On Tue, Nov 15, 2011 at 17:11, Vincent Massol <[email protected]> > wrote: > >>> > >>>> > >>>> On Nov 15, 2011, at 5:03 PM, Denis Gervalle wrote: > >>>> > >>>>> Hi Devs, > >>>>> > >>>>> The implementation of the immutable version of reference is almost > ready > >>>>> now. It introduce the parameters on reference as suggested, but we > now > >>>> have > >>>>> a discussion on how the constructors of "typed" entity reference > should > >>>> be. > >>>>> > >>>>> My initial dev was to provide constructor like: > >>>> > >>>> you're missing something here :) > >>>> > >>>>> But Vincent have different vision of this, here its comment extracted > >>>> from > >>>> > >>>> I don't have a different vision. It's just that you limited your > proposal > >>>> to just Locale which clearly isn't good enough. > >>>> > >>>>> GitHub ( > >>>>> > >>>> > https://github.com/xwiki/xwiki-platform/commit/cea424914f40ce924afbc49b3159bc8934251aac#commitcomment-721603 > >>>> ) > >>>>> : > >>>>> > >>>>> My proposal was: > >>>>>> - generic params in EntityReference > >>>>>> - helpers methods for get/setLocale and get/setVersion in > >>>> DocumentReference > >>>>>> > >>>>>> Now the fact that we're making the refs immutable changed this since > >>>> it's > >>>>>> no longer possible. > >>>>>> > >>>>>> I don't think multiplying the constructor signatures is a good idea. > >>>>>> > >>>>>> We could either have: > >>>>>> > >>>>>> public DocumentReference(String wikiName, List spaceNames, String > >>>>>> pageName, Map<String, Object> parameters) > >>>>>> > >>>>>> or > >>>>>> > >>>>>> public DocumentReference(String wikiName, List spaceNames, String > >>>>>> pageName, Pair<String, Object>... parameters) > > > > Neither of these signatures will work, If you want storage to have any > hope of being able to get the same object for the same reference, it must > be at least > > a Map or Pair of <String, Serializable>, if you allow people to pass > transient objects such as TCP sockets, they will. > > > > More importantly, if you introduce another parameter, there will be no > way to search for all documents where param X = value Y. To do it with SQL > would require > > an evil query which did text searching on the parameter field something > like: WHERE doc.reference LIKE '%param=value%' > > To add parameters to a reference and retain the ability to get all > documents where "param=value" you would have to modify the database schema > unless the information > > already exists in the XWikiDocument mapping as with language. > > > >>>>>> > >>>>>> where Pair is > >>>>>> > >>>> > http://commons.apache.org/lang/api-3.0-beta/org/apache/commons/lang3/Pair.html > >>>>>> > >>>>>> Maybe this needs some discussion on the devs list rather than here > to > >>>> make > >>>>>> sure everyone sees it? > >>>>>> > >>>>> > >>>>> I am myself not really happy with that since I dislike the idea that > >>>>> parameter are generic on "typed" references. > >>>>> Do not like either the idea to provide keys for creating a Map or > Pairs, > >>>>> since the implementation details that use Map should not be so > exposed > >>>> IMO. > >>>>> > >>>>> There should not be so much parameter on a single "typed" reference, > >>>> > >>>> I don't understand this sentence. > >>>> > >>>> The Map is <String, Object>. > >>>> > >>> > >>> I would means that this should not cause so many additional > constructors, > >>> if we list them individually. > >>> So your have not the same vision then I have :) > >> > >> See below. > >> > >>>>> and > >>>>> these should be easy to provide. Creating Maps in java is not fun in > >>>> syntax > >>>>> for that IMO, and is far too open. > >>>> > >>>> Sure but the goal here is not to redo java… > >>>> > >>> > >>> Not my goals, just want to be explicit and easy to use. > >>> > >>> > >>>> > >>>>> I had propose using overloaded constructor like > >>>>> > >>>>> public DocumentReference(String wikiName, List<String> spaceNames, > String > >>>>> pageName, Locale locale) > > > > +1 > > > >>>> > >>>> Again, this doesn't work. It only works for a **single** parameter. It > >>>> doesn't work for multiple parameters. How do you specify the Locale > or some > >>>> unknown String param? > >>>> > >>> > >>> I simply provide the need constructor, no more. > >> > >> If you say this then you say that we don't need generic parameters for > References which is what started the discussion… > >> > >> Also what you say is not correct at all since we already know we need > Version which you didn't put. > >> > >> Last, it means passing null to the constuctors when you don't use the > parameters which is really really bad IMO or you'll need to define lots of > various constructors which increase exponentially with the number of > parameters we support. > > > > A constant need for new parameters indicated design problems, including > locale information is fixing an error which limited the document reference > from the beginning. > > Building this infrastructure for additional parameters will foster bad > practice of using parameters where spaces should be used instead and will > allow the creation > > of arbitrary dimensional data trees where users are only familiar with > the standard 1 dimension of filesystems and URLs. > > > >> > >> However this won't even work since you won't be able to support adding > new parameters. Users of the Reference system will need to modify its > source if they wish to add a new parameter which again goes against the > initial proposal. > >> > >>>>> or if something more flexible should be used > >>>>> > >>>>> public DocumentReference(String wikiName, List<String> spaceNames, > String > >>>>> pageName, Object... parameters) > >>>>> > >>>>> where parameters is later interpreted based on object type and > limited to > >>>>> those used for a given typed reference. > >>>> > >>>> This doesn't work. If I have 2 parameters of type String, how do you > map > >>>> them automatically? > >>>> > >>> > >>> I suggest not to have loosely typed parameters, but strong one, like > Locale > >>> and Version. > > > > +1 > > > > I am -1 on version, I don't like it since a document history should be a > separate entity which yields documents, as it is now. > > How do you reference a given version of, say, a document? And how do you > pass this along the chain of calls? > > We'll need to introduce a new Reference (which is the initial reason I > made this proposal - I had to introduce a UniqueReference class in the new > Model before of this). > > > If we include version in the reference, storage will be forced to remove > it on save because we want users asking for a document > > with no knowledge of the current version to get the current version. > This means we already have a magic attribute which is treated > > differently by storage. > > All params are treated specially by the code, I don't understand what's > the problem. The exact same thing will happen for the Locale. > > > In addition, the introduction of a version parameter will allow a user > to load a document of a given version > > with a simple getDocument() call, > > Yes that's the idea! :) > > > fooling them into thinking all versions of every document are available > in the same namespace, > > Not sure what namespace you're referring to. > > > if getDocument(referenceToThirdVersionOfMainWebHome) works then it > stands to reason that search("WHERE doc.version=3 AND doc.name > ='Main.WebHome'") > > would as well but it won't because that's not how storage works. > > Maybe you meant that it's not how storage currently work? > > Remember that this proposal has nothing to do with either the current > model or the current storage. It's about the new model and the new storage > that we want to have. > > >> I'm a big big big -1 on this. > >> > >> There's no reason to create a fake String when all you need is a > string. Same for int, long, Number, etc. > >> Same for also if you want to have several Locales or several Versions. > >> > >>>> Also the goal is to have unknown parameters so how can you do a > mapping > >>>> for something unknown? :) > >>>> > >>> > >>> That is clearly not my goal, why do you want unknown parameters on > Document > >>> Reference ? > >> > >> Maybe this is where we don't agree. This is the original idea: to have > a set of "unknown" parameters for extensibility (from the POV of the > Reference of course, from the POV of XWiki they are not unknown obviously > ;)). This means that it's up to the users of the References to decide what > parameters to put and it's up to the consumers to decide what parameters to > support. > > > > The introduction of k-d trees for storing documents is not something > that anyone has ever used to my knowledge, it may be something that people > think they need > > but like the goto command, something that starts out nice ends up > creating more problems than it's worth. > > > >> > >> It seems to me your vision is to: > >> * Not support arbitrary parameters > >> * Only support Version and Locale > >> * Provide 3 constructors, one where there's no Locale no Version, one > with Locale no version and one with Version on Locale > >> > >> Whereas my vision is: > >> * Support arbitrary parameters for extensibility (like a URL if you > prefer which support arbitrary parameters) > > > > This is not correct, the parameters in a URL are never to my knowledge > used as a discriminator. > > Any webserver I can think of uses the parameters as arguments which are > passed to the page, not discriminators to select the page. > > Fun, you forgot XWiki itself! :) > We use version and language as URL parameters…. ?rev=X&language=Y > > Params reference are optional and are meant to be qualifier for the > reference. > > > for example: > > path/to/some/page.php?var=val&varb=42 > > will load the page "path/to/some/page.php" then execute it, passing > var=val and varb=42 as arguments. > > Seen all this I propose that stop this discussion and don't commit any of > this on master for the moment since we have too many diverging opinions on > the new model and we need to agree about the new model first. > > So I change my vote about the proposal I initially sent to -1 for now till > this is sorted out because I don't want to rush into something for which we > don't agree and have diverging ideas. > > I've been planning to put the work I started in the contrib sandbox into a > branch on master in order to start using it ASAP (Thomas and I had a > discussion and we think we could already start to slow use portions of it > to validate it). > > So here's what I propose: > > * I do this ASAP > * I write an email explaining the work done so far on the new model > * We discuss it and come to an agreement > * Whether we extend our current Entity References with params or not will > be a direct consequence of the model agreement > > I know Denis wanted to use the Locale param for his work. FTM Denis will > have to pass the Locale as a parameter alongside the Entity Reference as > it's done every where ATM. > > Is that ok? > I am absolutely not happy with that. I had a hard time putting it to work and I do not want to wait further to merge immutable references. Since parameters cames with them, I will not loose my time removing them just because there is now a doubt on the initial vote here. I stay convince that we need to have language in references, and I have seen nobody saying it should not. The initial discussion came from the implementation of the constructor for Document Reference, which simply provide optional Locale to a document reference. So I do not understand your -1 now. What we do not agree on are arbitrary parameters, and having version one of them. So what is the problem to have my current implementation committed ? (With Serializable in place of Object, which is a good remarks since we need serializable references) Denis > Thanks > -Vincent > > > Caleb > > > > > >> * Have some "well-known" parameters so that xwiki code that users > References know what they can put as reference parameters and what they can > extract from XWiki parameters. > >> > >> Thanks > >> -Vincent > >> > >>>> Thanks > >>>> -Vincent > >>>> > >>>>> Here Vincent comments on this: > >>>>> > >>>>>> The automatic mapping idea seems a bad idea to me (too magic and > doesn't > >>>>>> work in a lot of cases). > >>>>> > >>>>> > >>>>> Maybe some of you have an even better idea ? > >>>>> > >>>>> Denis > _______________________________________________ > 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
