Hi devs, On Wed, Feb 27, 2013 at 6:34 PM, Marius Dumitru Florea < [email protected]> 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). > > > Any input on this? > > > 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. > Adding $xwiki.hasVirtualWikis() (convenience for $xwiki.getWikiNames().size() > 1). > > > > > 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. > Any suggestions? Maybe use the EM script service to check if the extension xwiki-enterprise-ui or xwiki-manager-ui is installed? Thanks, Eduard > 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
