Hi Denis, Nice feedback! Please see below...
On Wed, Feb 27, 2013 at 10:23 PM, Denis Gervalle <[email protected]> wrote: > Hi again Eduard, > > Regarding point 5., this is where it may get complex to support properly > existing configuration file. So, the first question would be to know if we > accept to disrupt upgraded wiki that use an old configuration file ? > If the answer is yes, than good release notes should allow explaining how > to be careful and adapt existing configuration to avoid unreachable wiki. > > Personally, causing such disruption has not my preference, since upgrading > has not done so until now, AFAIK. Moreover, IMO being able to put by > configuration the main wiki unreachable could only cause a negative feeling > to newcomers. Option like autowww is probably useless complexity for the > benefit. And the worse is the unknown wiki page that is never reach since > it come from the main wiki to say it is itself unreachable (XWIKI-479). > > What you propose is not so easy, since creating the descriptor implies > knowing the hostname when usepath=0 which may happen in old config even if > unused. So here is what I would suggest: > > 1) Follow Thomas proposal: ensure getVirtualWikisDatabaseNames() always > return the main wiki, even when no descriptor is available, but not by > creating such descriptor. This ensure main wiki availability anytime. > Already done in the feature branch. However, how do we handle point 5, as I have described it wrt. the wiki-manager plugin? The getAllWikis() method basically returns the list of wiki document descriptors. While for XWiki.getVirtualWikisDatabaseNames() it was easy to add a new string to the list of results, how would we handle it for the wiki-manager plugin, where we have to add an unexistent document (with objects -- what values?) to the list of results? > 2) Stop using a redirect to reach the unknown wiki page. This is also > annoying for a simple typo in an URL since you loose that URL from the > address bar. I see no usual need for going to a page outside XWiki, so stop > using that configuration and eventually provide a configuration for > deciding the page name of the main wiki that will be rendered. Anyway, use > a .vm like other error to render that (optional) page or a default. This > way, you leave a workaround for anyone wishing to restore the old behavior > (you may even suggest commented code for that in the .vm) > I like this idea of using a vm instead of a redirect. > 3) Never proceed to unknown wiki page when there is only one wiki, > considering any unknown as the main. > While I theoretically agree with this, in practice it might be a bit confusing for some users that explicitly wrote a subwiki (in domain based) or maybe they have a bookmark but they end up on the main wiki, where they could continue working without realizing that they are not on the wiki they have requested. Considering this, I think I prefer to display an explicit error. > 4) Deprecate the autowww option, which is now useless > I have just came across this feature, so I`m not really familiar with the details and implications. As far as I understand it, by default, we allow the main wiki to be accessed trough the "www" subdomain (if tomcat is properly configured as well, I assume), "localhost" or ip address without having a descriptor. For all other cases, we proceed to get the descriptor and, if none is found, we redirect to whatever "xwiki.virtual.redirect" says. Which part of this (except point 3 which we cleared above) are you suggesting to deprecate? The default behaviour or the option to disable this default behaviour? Please detail this a bit. 5) Optional, but would be a nice to have for global admin, allow fixing the > main wiki descriptor from the unknown wiki page. > What do you mean by "fixing the main wiki descriptor from the unknown wiki page"? Please details this a bit as well. Thanks, Eduard > With those changes, you will fix most issue regarding the reachability of > wikis in virtual mode, and therefore, having virtual as the default should > not be an issue anymore, whatever your configuration is. > > WDYT ? > > > On Wed, Feb 27, 2013 at 10:21 AM, Eduard Moraru <[email protected] > >wrote: > > > Hi devs, > > > > To further fuel this discussion, I will mention some points that are more > > or less problematic when removing the notion of virtual mode (checks for > > xwiki.isVirtualMode()), since a direct change would leave a some pieces > of > > code dead (never called) or just need a bit of discussion on them: > > > > 1. c.x.x.s.XWikiHibernateBaseStore.initHibernate(XWikiContext context) > [1] > > > > That piece of code seems to only run when the wiki is not in virtual > mode. > > From what I managed to understand from Thomas [2], it's probably this > case: > > "in your hibernate configuration if you set a database which is not > called > > 'xwiki'. In virtual mode this configuration is not taken into account and > > you have to indicate the name of the database in xwiki.cfg". > > I think we need to review this behavior. IMO, we should do take this > > configuration into account in virtual mode as well, as long as there is > no > > other technical impediment. It is a configuration for the main wiki after > > all, and it should still be valid. > > > > 2. c.x.x.XWiki.onPluginPreferenceEvent(Event event, XWikiDocument doc, > > XWikiContext context) [3] > > > > This piece of code seems to reload the plugins if the "plugins" property > > changed in XWikiPreferences, but only if the wiki is not in virtual > mode. I > > think we need to change this to happen for any wiki and the plugins need > to > > be reloaded only for that specific wiki for which the change happened (if > > it is possible). > > > > 3. extension.vm - extensionActionButton macro [4] > > > > This UI code decides when to displays the (Un)Install button of an > > extension for the current wiki or for the entire farm. Removing the > virtual > > mode check will always show the (un)install on farm button. I think > Marius > > had some concerns about his one. > > > > 4. distribution.vm - displayMainUiStep_unknownPreviousUI [5] > > > > This code assumes that if virtualMode is true, then the product is XEM, > > otherwise it's XE. This is used to propose old version names for the 2 > > products. Obviously this is needs to be refactored and use a different > way > > of determining which is which. > > > > 5. c.x.x.p.w.WikiManager > > public List<Wiki> getAllWikis(XWikiContext context) throws > > XWikiException [6] > > > > We have decided that c.x.x.XWiki.getVirtualWikisDatabaseNames will also > > return the main wiki's name. However, the WikiManager plugin can not do > > this so easily because the main wiki's descriptor may not exist. How > could > > we handle this, in order to be consistent? Would creating > programmatically > > the main wiki's descriptor with default values on ApplicationReady event > do > > any good? Will it do any harm? > > > > Thanks, > > Eduard > > > > ---------- > > [1] > > > > > https://github.com/xwiki/xwiki-platform/blob/3019c6e0df112adf980641c75aaaffb2f0b17c6f/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/store/XWikiHibernateBaseStore.java#L214 > > [2] > > > > > https://github.com/xwiki/xwiki-platform/commit/ce3d9d28aefdb327d407ef179a4c869ccce74478#commitcomment-2679994 > > [3] > > > > > https://github.com/xwiki/xwiki-platform/blob/3019c6e0df112adf980641c75aaaffb2f0b17c6f/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java#L6521 > > [4] > > > > > https://github.com/xwiki/xwiki-platform/blob/3019c6e0df112adf980641c75aaaffb2f0b17c6f/xwiki-platform-core/xwiki-platform-web/src/main/webapp/templates/extension.vm#L971 > > [5] > > > > > https://github.com/xwiki/xwiki-platform/blob/3019c6e0df112adf980641c75aaaffb2f0b17c6f/xwiki-platform-core/xwiki-platform-web/src/main/webapp/templates/distribution.vm#L194 > > [6] > > > > > https://github.com/xwiki/xwiki-platform/blob/3019c6e0df112adf980641c75aaaffb2f0b17c6f/xwiki-platform-core/xwiki-platform-wiki-manager/xwiki-platform-wiki-manager-api/src/main/java/com/xpn/xwiki/plugin/wikimanager/WikiManager.java#L223 > > > > > > On Fri, Feb 22, 2013 at 3:48 PM, Eduard Moraru <[email protected]> > > wrote: > > > > > Hi devs, > > > > > > To be more precise, the essence of my proposed change [1] is to > actually > > > always return "true" in the isVirtualMode() method, ignoring any > > > configuration and marking the method as @deprecated, to be removed in > > > future versions. > > > > > > Is there any reason not to do this? (Why) Would we want to preserve the > > > possibility of going back to a non-virtual mode? (mostly addressed to > > > Thomas, but to others as well) > > > > > > Thanks, > > > Eduard > > > > > > ---------- > > > [1] > > > > > > https://github.com/xwiki/xwiki-platform/commit/ce3d9d28aefdb327d407ef179a4c869ccce74478 > > > > > > > > > > > > > > > On Fri, Feb 22, 2013 at 3:39 PM, Eduard Moraru <[email protected] > > >wrote: > > > > > >> Hi devs, > > >> > > >> As some of you have already noticed and already started providing > > >> feedback (thank you), I have pushed a feature branch [1] that proposes > > the > > >> removal of the virtual mode concept (making it enabled and fixed by > > >> default). > > >> > > >> I have also gone trough the code in xwiki-platform and removed, in the > > >> feature branch, dead code (code branches that will never be reached > > because > > >> of this change). Some tests were also adapted to the new change. I > have > > >> also tried a build of xwiki-platform with these changes and it built > > >> successfully. > > >> > > >> First of all, I want to make sure that we agree on the change, since > > >> Thomas already mentioned that he does not agree (but I need more > > details). > > >> > > >> Second, if we agree, please take the time to look over the changes > > >> (particularly in the modules that you each have experience in) and > > double > > >> check that the logic of the code was not altered in any way. > > >> > > >> I will not be merging this branch until I get a general green light > from > > >> the committers, and until everybody is happy :) > > >> > > >> Please share your opinion both on this mail and/or on the specific > > >> changes in the github commits. > > >> > > >> Thanks, > > >> Eduard > > >> > > >> ---------- > > >> [1] > > >> > > > https://github.com/xwiki/xwiki-platform/commits/feature-deprecate-virtual-mode > > >> > > >> > > >> > > >> On Mon, Feb 18, 2013 at 4:53 PM, Thomas Mortagne < > > >> [email protected]> wrote: > > >> > > >>> On Mon, Feb 18, 2013 at 3:48 PM, Thomas Mortagne > > >>> <[email protected]> wrote: > > >>> > On Mon, Feb 18, 2013 at 3:38 PM, Eduard Moraru < > [email protected] > > > > > >>> wrote: > > >>> >> Hi Thomas, > > >>> >> > > >>> >> On Mon, Feb 18, 2013 at 3:45 PM, Thomas Mortagne > > >>> >> <[email protected]>wrote: > > >>> >> > > >>> >>> On Mon, Feb 18, 2013 at 2:07 PM, Eduard Moraru < > > [email protected] > > >>> > > > >>> >>> wrote: > > >>> >>> > Hi devs, > > >>> >>> > > > >>> >>> > According to the Roadmap of 5.0 [1][2] we will be deprecating > the > > >>> virtual > > >>> >>> > mode API and moving to a virtual-by-default mode instead. > > >>> >>> > > > >>> >>> > The idea is that the multiwiki environment should be the > default > > >>> and, > > >>> >>> > anyone wanting to use the single-wiki mode (as XE was doing in > > the > > >>> past), > > >>> >>> > will simply not create any subwiki. It is just pointless to > have > > 2 > > >>> >>> products > > >>> >>> > for such a simple fact and it also confuses users that have > > >>> downloaded > > >>> >>> and > > >>> >>> > started to use one product and later on realise that they > needed > > >>> the > > >>> >>> other. > > >>> >>> > Also, when installing the wiki-manager and/or workspace > > >>> extension(s) > > >>> >>> > (because you might want to create subwikis/workspaces), there > > will > > >>> be no > > >>> >>> > need to stop the wiki, enable-virtual mode and restart it; all > > >>> will be > > >>> >>> > doable in the browser. > > >>> >>> > > > >>> >>> > Therefore, I`m sending this mail to ask your opinion about this > > >>> topic, in > > >>> >>> > case you might have something to say against it, and also to > > >>> brainstorm > > >>> >>> on > > >>> >>> > the required changes and implications on existing code so that > > the > > >>> >>> > transition is as smooth and invisible as possible. > > >>> >>> > > > >>> >>> > Proposed changes: > > >>> >>> > - Remove "xwiki.virtual" from xwiki.cfg(.vm) > > >>> >>> > -- remove the usage of the "$xwikiCfgVirtual" maven property > from > > >>> >>> > xwiki-platform (, xwiki-enteprise and xwiki-manager) > > >>> >>> > - Deprecate boolean com.xpn.xwiki.XWiki.isVirtualMode() (and > the > > >>> >>> api.XWiki > > >>> >>> > version) > > >>> >>> > -- change its code to > > >>> ((getVirtualWikisDatabaseNames(context).size() == > > >>> >>> 1) > > >>> >>> > ? true : false) until it is removed by the deprecation process. > > >>> >>> > > >>> >>> -1. It means that you are going to be in non virtual mode when > > >>> >>> starting XEM which is very wrong and the complete opposite of > what > > we > > >>> >>> want here. We should deprecate and don't touch it's > implementation > > in > > >>> >>> any way other that returning true by default instead of false > when > > >>> >>> there is nothing in the configuration. Again as you said the goal > > is > > >>> >>> to be in virtual mode by default, period. If the UI want to do > > >>> >>> something different when there is only one wiki the UI should > test > > if > > >>> >>> there is only one wiki but breaking all extensions counting on > the > > >>> >>> virtual mode is very bad API breakage. > > >>> >>> > > >>> >> > > >>> >> I agree. The isVirtualMode check was supposed to tell you if > "there > > >>> can be > > >>> >> more than one wiki", while I was suggesting to transform it to > "are > > >>> there > > >>> >> currently more than one wikis?", which is obviously not > equivalent. > > >>> >> > > >>> >> > > >>> >>> > > > >>> >>> > Possibly needed changes: > > >>> >>> > - Add main wiki default descriptor to xwiki-enterprise-ui (so > > that > > >>> >>> > getVirtualWikisDatabaseNames includes the main wiki and also to > > >>> avoid DNS > > >>> >>> > issues (?) caused by how we handle virtual mode) > > >>> >>> > > >>> >>> This is useless IMO: > > >>> >>> * the fact that getVirtualWikisDatabaseNames does not return the > > main > > >>> >>> wiki when there is no descriptor for it is a bug that should be > > fixed > > >>> >>> > > >>> >> > > >>> >> Ok. I have created the jira issue for it: > > >>> >> http://jira.xwiki.org/browse/XWIKI-8829 > > >>> >> > > >>> >> Also, on the same topic, I think that we should also expose the > > >>> >> getVirtualWikisDatabaseNames method in api.XWiki. Maybe we should > > >>> rename it > > >>> >> to something simpler like api.XWiki.getAllWikis() or > > >>> getAllWikiNames(). > > >>> > > > >>> > Yes, "getVirtualWikisDatabaseNames" is not very nice. > > >>> > > >>> This name is also pretty wrong btw since that's wiki names and not > > >>> database names which could be very different. > > >>> > > >>> > > > >>> >> > > >>> >> > > >>> >>> * I don't see how having a descriptor with localhost in it will > > help > > >>> >>> in any way not having DNS issue, it's not helping much in XEM > it's > > >>> not > > >>> >>> going to be better in XE. IMO the right solution is to fallback > on > > >>> >>> main wiki by default when the wiki is unknown instead of sending > a > > >>> >>> redirect to some URL which cannot work and only allow to enable > > this > > >>> >>> redirect URL as an option > > >>> >>> > > >>> >> > > >>> >> Ok, so we comment out xwiki.virtual.redirect in xwiki.cfg and, by > > >>> default, > > >>> >> redirect to the main wiki's Main.WebHome. > > >>> > > > >>> > I carefully chose "fallback" and not "redirect" because a real > > >>> > redirect is impossible here since no URL will really be right or > the > > >>> > main wiki. The idea would be to do exactly the same thing we do > with > > >>> > IP, www.*,. > > >>> > > > >>> >> > > >>> >> An existing jira issue that is very related to this topic is > > >>> >> http://jira.xwiki.org/browse/XWIKI-479 We could reuse that. > > >>> > > > >>> > Yes we can reuse it but not follow the proposal in the comments. > > >>> > > > >>> >> > > >>> >> Thanks, > > >>> >> Eduard > > >>> >> > > >>> >>> > > >>> >>> > -- or have it generated programmatically at startup if it does > > not > > >>> exist > > >>> >>> > (might be safer this way, since people might be using a > different > > >>> UI than > > >>> >>> > xwiki-enterprise-ui) > > >>> >>> > > > >>> >>> > I will start working on this locally and see if I can spot > other > > >>> issues, > > >>> >>> > but please share your thoughts. > > >>> >>> > > > >>> >>> > Thanks, > > >>> >>> > Eduard > > >>> >>> > > > >>> >>> > ---------- > > >>> >>> > [1] http://markmail.org/message/o6adfbscpidnn7zr > > >>> >>> > [2] http://jira.xwiki.org/browse/XWIKI-8822 > > >>> >>> > _______________________________________________ > > >>> >>> > devs mailing list > > >>> >>> > [email protected] > > >>> >>> > http://lists.xwiki.org/mailman/listinfo/devs > > >>> >>> > > >>> >>> > > >>> >>> > > >>> >>> -- > > >>> >>> Thomas Mortagne > > >>> >>> _______________________________________________ > > >>> >>> devs mailing list > > >>> >>> [email protected] > > >>> >>> http://lists.xwiki.org/mailman/listinfo/devs > > >>> >>> > > >>> >> _______________________________________________ > > >>> >> devs mailing list > > >>> >> [email protected] > > >>> >> http://lists.xwiki.org/mailman/listinfo/devs > > >>> > > > >>> > > > >>> > > > >>> > -- > > >>> > Thomas Mortagne > > >>> > > >>> > > >>> > > >>> -- > > >>> Thomas Mortagne > > >>> _______________________________________________ > > >>> devs mailing list > > >>> [email protected] > > >>> http://lists.xwiki.org/mailman/listinfo/devs > > >>> > > >> > > >> > > > > > _______________________________________________ > > devs mailing list > > [email protected] > > http://lists.xwiki.org/mailman/listinfo/devs > > > > > > -- > Denis Gervalle > SOFTEC sa - CEO > eGuilde sarl - CTO > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
