Eduard
On Tue, Feb 2, 2016 at 4:52 PM, vincent(a)massol.net <vincent(a)massol.net>
wrote:
On 2 Feb 2016 at 15:46:20, Thomas Mortagne (thomas.mortagne(a)xwiki.com
(mailto:thomas.mortagne@xwiki.com)) wrote:
On Tue, Feb 2, 2016 at 3:39 PM,
vincent(a)massol.net wrote:
>
>
> On 2 Feb 2016 at 15:37:10, Thomas Mortagne (thomas.mortagne(a)xwiki.com
(mailto:thomas.mortagne@xwiki.com)) wrote:
>
>> On Tue, Feb 2, 2016 at 3:32 PM, vincent(a)massol.net wrote:
>> >
>> >
>> > On 2 Feb 2016 at 14:56:42, Thomas Mortagne (
thomas.mortagne@xwiki.com(mailto:thomas.mortagne@xwiki.com)) wrote:
>> >
>> >> On Tue, Feb 2, 2016 at 2:06 PM, vincent(a)massol.net wrote:
>> >> > Hi Edy,
>> >> >
>> >> > On 2 Feb 2016 at 13:34:49, Eduard Moraru (enygma2002(a)gmail.com
(mailto:enygma2002@gmail.com)) wrote:
>> >> >
>> >> >> 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.
>> >>
>> >> No need to check the resource reference type. The user would use
the
>> >> EntityReferenceResolver which
return the right type
>> >> of entity reference based on what is found in the passed
>> >> ResourceReference. The the user can check the ResourceReference
type
>> >> (if it cares about it, in some
cases you will just call
>> >> XWiki#getDocument(EntityReference)).
>> >
>> > I don’t see how it’s possible since you can have Resource
Reference
that have no meaning as EntityResourcReference…
>> >
>> > For example “mailto:…”.
>>
>> Sure but in that case you will just get nothing. Just need to be
>> prepare that it's possible you get no result.
>
> What does it mean “nothing” for an EntityResourceReference? null
(yuck!)?
>
> Doesn’t feel the best way to me since a ResourceReference is not an
EntityResourceReference.
>
> I don’t see how you can escape checking the type… Checking the type
or
checking the null is the same except that checking for the type is much
nicer.
Checking for null is easy, for the type you have to know all the
possible types that could be related to an eniity.
Imagine we introduce a WIKI resource type to link to a wiki home page
for example, if your code is based on known resources types then you
won't support it, if you just check if you get a valid reference then
you support any future resource type that can be translated to entity
reference.
Ok, that’s a good point.
I guess another answer is that we need to refactor the Rendering (in
platform) to use the new Resource module instead and this Resource module
has the concept of EntityResourceReference (which doesn’t exist in the
Rendering). But that’s way too much work for now...
Thanks
-Vincent
> Thanks
> -Vincent
>
>
>> > Thanks
>> > -Vincent
>> >
>> >> > 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.
>> >> >
>> >> >> 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…
>> >> >
>> >> > 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