2 Feb
2016
2 Feb
'16
2:44 p.m.
Hi,
On Tue, Feb 2, 2016 at 3:06 PM, vincent(a)massol.net <vincent(a)massol.net>
wrote:
Yes, I should have given some examples here. What I call "space attachment"
is "Space(a)file.ext" (without "WebHome"), while a "document
attachment"
would be "Space.Doc(a)file.ext" or even "Space.WebHome(a)file.ext".ext". We
have no
explicit syntax to differentiate between them (like we have for doc: and
space:) so a client would have to resolve the reference by himself.
Currently, XWikiWikiModel resolves it an produces the correct attachment
URL. This is where the above resolvers come very useful.
Thanks,
Eduard
Hi Edy,
On 2 Feb 2016 at 13:34:49, Eduard Moraru (enygma2002(a)gmail.com(mailto:
enygma2002(a)gmail.com)) wrote:
Yes, and it also increases the potential size of the cache to all possible
combinations of (caller, cached XDOM) pairs.
Hi,
So after an initial implementation we realized there are some issues with
the original approach.
Basically, it is not such a good idea to handle the resolving of untyped
links at the parser level because this is not compatible with the XDOM
caching that we have for XWikiDocuments, i.e. it`s not OK to cache a
document's XDOM that was resolved with links in the context of the
*first*
document that resolved them and then to reuse
that cached XDOM later or
from a different calling document when/where those links no longer make
sense. We would need to remove the XDOM caching and parse the XDOM every
time we need it in order to make sure that the links are resolved in the
context of the current calling document and that they are the correct
links. Obviously, we don`t want that and we want to keep the XDOM cache
which is vital for performance.
In order to address this issue and to still be able to do the fallback
mechanism (the one that allows us to be backwards compatible at the
syntax
level when resolving an untyped link), Thomas
suggested that we always
produce a static (cacheable/reusable) parsing result (i.e. URL or
DOCUMENT
types, with the DOCUMENT ResourceReference having
the isTyped flag set to
false) for untyped links and that the we introduce a new set of
EntityReferenceResolver API to be used on the client
side (i.e. when consuming the XDOM) to get the actual XWiki entities.
This
would be used, for example, by the XWikiWikiModel
when producing the URLs
and any other places that use DOCUMENT type ResourceReferences. So we
pass
the resolving responsibility to this new API and
expect the client to use
it.
This second proposal resolves the caching issue but introduces new
requirements on the clients of the rendering result, so in terms of
backwards compatibility, if the clients don`t use this new resolvers API,
some links might be interpreted incorrectly (i.e. assuming it's a
document
instead of a space reference). This adds to the
backwards compatibility
issues caused by the addition of the new SPACE link type, but there is
not
much way around it.
I agree that the XDOM returned by XWikiDocument.getXDOM() should contain
exactly what’s been parsed from the syntax without any interpretation.
We could imagine caching the XDOM along with the Context but it doesn’t
make much sense IMO. I find it better that it’s evaluated at the time it’s
needed with the context at that time.
You propose to provide an Entity Reference Resolver to
allow users of
getXDOM() to parse references. So I guess the users would check if the
ResourceReference is of type Attachment, Document or Space and then use
that resolver to convert the String into an EntityResourceReference.
We were going directly for EntityResource and want to reuse the
EntityResourceResolver<ResourceReference> API. Do you see any issue with
that?
Another idea (don’t know if you thought about it) would be to implement a
Transformation that would take that XDOM and generate a XDOM’ that is
resolved against the current Context (with all the links resolved). I guess
there could be use cases for both needs but having the fine-grained
solution is better to start with.
Interesting idea. Haven`t thought about it, but, as you also said, I find
the fine grained solution to be better since it also helps with
attachments, not only space/document links, since for attachments you would
still have to apply the manual/fine-grained solution.
I`d also like
to mention that we are already doing something similar to
this when resolving attachment/image references (i.e. the caller needs to
resolve the reference since the rendering does not distinguish between
"space attachments" or "document attachments") so this new resolver
API
is
a good/needed addition anyway and expanding this
approach to links makes
sense, IMO.
I don’t understand "since the rendering does not distinguish between
"space attachments" or "document attachments””, could you provide some
example in wiki syntax?
Additional note: typed links ([[doc:Doc]],
[[space:Space]] etc.) are
still
supported and not affected by this change of
direction, since this is
only
about (untyped [[Doc]]) links.
We`re starting work on this direction.
Please let us know what you think about this new approach and if you see
any use cases where it creates problems.
Sounds good to me.
Of course we’ll need to document this in the release notes.
I’ve done a quick check of usages of LinkBlock.getReference() since this
is where the problem could happen. I’ve found the following places to check:
- XWikiDocument.getUniqueLinkedPages()
- DefaultLinkedResourceHelper
- PlainTextBlockFilter
Note that the LinkCheckerTransformation is not affected since it only
checks URL-type links.
There are also some matches in xwiki-contrib to check:
https://github.com/search?q=LinkBlock+user%3Axwiki-contrib&ref=searchre…
Cool, I`ll look there too. Thanks!
-Vincent
PS: Good catch Thomas about this!
Thanks,
Eduard
On Thu, Jan 7, 2016 at 2:03 PM, Eduard Moraru wrote:
> Hi Thomas,
>
> On Wed, Jan 6, 2016 at 7:42 PM, Thomas Mortagne > > > wrote:
>
>> On Wed, Jan 6, 2016 at 5:57 PM, Eduard Moraru
>> wrote:
>> > Hi devs,
>> >
>> > As XWIKI-12920 [1] mentions, we need to hide "WebHome" references
in the
>> > wiki syntax for links and images
(for now) in order to be consistent
>> with
>> > what we did in the UI.
>> >
>> > What this means is that [[label>>Page.WebHome]] should be
equivalent to
>> > [[label>>Page]].
>> >
>> > The plan is to apply the same logic as we did for URLs and detailed
by
>> > Vincent in "Solution 2"
in his comment on the issue [2].
>> >
>> > In order to achieve this, Vincent proposed to introduce a new
"space:"
>> > resource type and make this new
type the default instead of "doc:"
>> which is
>> > the current default.
>> >
>> > Before:
>> > [[label>>Page]] => [[label>>doc:Page]]
>> > After:
>> > [[label>>Page]] => [[label>>space:Page]] if
"Page.WebHome" exists,
>> > or => [[label>>doc:Page]] if
>> ".Page"
>> > exists,
>> > or => wanted link to create "Page.WebHome" when
>> > clicked.
>> >
>> > Using either the "doc:" or the "space:" versions
manually will
resolve
>> the
>> > requested resource, without doing any fallback resolution (which is
>> applied
>> > only for the no-prefix version).
>> >
>> >
>> > For the same consistency reason, we need to apply the same fallback
to
>> the
>> > "attach:" resource type, e.g:
>> > [[attach:Page@file.ext]] => [[label>>attach:space:Page@file.ext]]
||
>> >
[[label>>attach:doc:Page@file.ext]]
>> >
>> > However, a resource is defined as ((type:) resource) so for
attachments
>> we
>> > would need to extend the type's definition to allow it to contain
":" in
>> > the type name (i.e.
"attach:doc" and "attach:space") so that more
>> > variations are tested when resolving a link reference before
treating
>> it as
>> > a generic/untyped reference (this is where the fallback mechanism
kicks
>> in).
>> >
>> >
>> > There is also the image syntax that needs to be extended to support
the
>> > same fallback logic, so something
like:
>> > image:Page => image:space:Page || image:doc:Page
>> >
>> >
>> > In all these cases:
>> > * link space: prefix,
>> > * link attach:doc and attach:space prefixes and
>> > * image space: and doc: prefix
>> > ... we would be breaking backwards compatibility in the sense that
if a
>> > wiki with the name
"space" (and/or "doc", depending on which case
of the
>> > above you are) already existed in
the wiki farm, any links pointing
to
>> > documents in that wiki from other
wikis will be broken, because, for
>> > example [[label>>space:Page]] no longer points to the document
>> > "space:Page.WebHome" (from wiki "space"), but to the
document
>> > "Page.WebHome" from the current wiki (where the link is made). To
fix
>> this,
>> > one would have to write [[label>>space:space:Page]] instead.
>>
>> "[[label>>space:Page]]" does not mean
"space:Page.WebHome" right now
>> but ":.space:Page"
>>
>
> You are right. Bad example (though I find that behavior a bit odd). It
> should have been:
>
> [[label>>space:Space.Page]] no longer points to the document
> "space:Space.Page" (from wiki "space"), but to the document
> "Space.Page.WebHome" from the current wiki (where the link is made).
To fix
> this, one would have to write
[[label>>space:space:Space.Page]]
instead.
>
> Thanks,
> Eduard
>
>
>> >
>> > I guess we could/should write a migrator to try to fix most of these
>> cases
>> > automatically (like we fix links to a document that was renamed),
but a
>> > couple of links will be unfixable
if they are built programatically
>> (e.g.
>> > by velocity scripts) and the process could prove to be extremely
lengthy
>> > one (due to the parsing,
processing, serialization and resaving of
each
>> > document).
>> >
>> >
>> > Please feel free to comment on the above described approach.
>> >
>> > Thanks,
>> > Eduard
>> >
>> > P.S.: I did not mention macro parameter references which would also
>> need a
>> > solution for hiding the "WebHome" part, but I`d prefer it if we
handle
>> that
>> > another time / in another thread.
>> >
>> > ----------
>> > [1] http://jira.xwiki.org/browse/XWIKI-12920
>> > [2]
>> >
>>
http://jira.xwiki.org/browse/XWIKI-12920?focusedCommentId=89129&page=co…
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs