On 2 Feb 2016 at 14:56:42, Thomas Mortagne (thomas.morta...@xwiki.com(mailto:thomas.morta...@xwiki.com)) wrote:
> On Tue, Feb 2, 2016 at 2:06 PM, vinc...@massol.net wrote: > > Hi Edy, > > > > On 2 Feb 2016 at 13:34:49, Eduard Moraru > > (enygma2...@gmail.com(mailto:enygma2...@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:…”. 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=searchresults&type=Code&utf8=%E2%9C%93 > > > > 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:p...@file.ext]] => [[label>>attach:space:p...@file.ext]] || > >> >> > [[label>>attach:doc:p...@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=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-89129 > > _________ _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
