On Mar 4, 2013, at 9:23 PM, Marius Dumitru Florea <[email protected]> wrote:
> On Mon, Mar 4, 2013 at 5:46 PM, Eduard Moraru <[email protected]> wrote: >> 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? > > As discussed in private, for now it's better to use hasVirtualWikis() Just make sure you don't call it like this :) See the comments on http://jira.xwiki.org/jira/browse/XWIKI-8875 Thanks -Vincent > as a quick fix because I'm going to refactor this code soon to take > the list of available versions for the configured UI so there won't be > any need to distinguish between xwiki-enterprise-ui and > xwiki-manager-ui. Also, with Thomas' work to support DW on subwikis > we're going to split the distribution.vm template so don't spend time > on distribution.vm changes because soon they will be hard to > integrate. > > Thanks, > Marius > >> >> 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
