I'm a bit late to the party but +1 to the idea of making wikis "virtualMode=true" all of the time, it will allow code paths to be removed and simplified even though it will have a (probably marginal) performance hit. I don't see any need for distinguishing between a non-multiwiki and a multiwiki without any subwikis.
Thanks, Caleb On 02/27/2013 11:34 AM, Marius Dumitru Florea wrote: > On Wed, Feb 27, 2013 at 11: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. > > As I said in my comment on GitHub I think the isVirtualMode() test > should be replaced in this particular case with something like > hasVirtualWikis() . It makes sense to display EM buttons that target > multiple wikis only if you have multiple wikis. > >> >> 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. > > Yes. > > Thanks, > Marius > >> >> 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 > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
