On Tue, Feb 2, 2016 at 3:32 PM, vinc...@massol.net <vinc...@massol.net> wrote: > > > 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:…”.
Sure but in that case you will just get nothing. Just need to be prepare that it's possible you get no result. > > 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 -- Thomas Mortagne _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
