15 Nov
2011
15 Nov
'11
10:44 p.m.
On Nov 15, 2011, at 10:14 PM, Denis Gervalle wrote:
On Tue, Nov 15, 2011 at 21:26, Vincent Massol
<vincent(a)massol.net> wrote:
1) I haven't checked your implementation details but from the discussion I assume it
has generic parameter support in EntityReferences and Locale being implemented as a
special case of those generic params. Caleb doesn't agree about those generic params.
2) For me Locale and Version (and potentially other params) were going together and were
used to achieve a goal (the removal of the UniqueReference class I had added and the
reduction in the scope of generic Resource module proposal that I have been wanting to
make for a long time now). Having only one of them (Locale) and not the other (Version) is
completely different and doesn't achieve the same goal at all so from my POV I need a
new VOTE different from the previous one since it's about something different with a
different goal.
BTW it's not just me voting -1, Caleb has voted -1 in his mail which anyway negates
the previous VOTE. Because of this we need a new proposal since again adding just Locale
doesn't serve the same goal at all.
I never said I'd be -1 to a new proposal. What I said is that a new proposal would
need to at least take into account the new model or rather explain what it would mean for
the new model.
Personally I would need a few days to think what it would mean to just add the Locale and
not the Version to the new Model I've been preparing. I've not been working on it
recently and last time I brainstormed on it with Thomas we were convinced we needed both
the Locale and the Version in the reference. I need to think again and find out why we
wanted that and what impact it has on the model.
Thanks
-Vincent
On Nov 15, 2011, at 9:04 PM, Caleb James DeLisle wrote:
hope of
being able to get the same object for the same reference, it must
be at least
or some
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).
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 ? 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 <vincent(a)massol.net>
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/cea424914f40ce924afbc49b3159…
>>
)
>>> :
>>>
>>> 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 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.h…
>>>>>
>>>>> 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
>>> 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 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?
(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