On Wed, May 25, 2011 at 11:49, Denis Gervalle <[email protected]> wrote: > On Wed, May 25, 2011 at 09:43, Thomas Mortagne > <[email protected]>wrote: > >> On Tue, May 24, 2011 at 21:17, Denis Gervalle <[email protected]> wrote: >> > On Tue, May 24, 2011 at 20:31, Thomas Mortagne < >> [email protected]>wrote: >> > >> >> On Tue, May 24, 2011 at 20:19, Denis Gervalle <[email protected]> wrote: >> >> > On Tue, May 24, 2011 at 18:59, Thomas Mortagne < >> >> [email protected]>wrote: >> >> > >> >> >> On Tue, May 24, 2011 at 17:55, Denis Gervalle <[email protected]> wrote: >> >> >> > On Tue, May 24, 2011 at 17:39, Thomas Mortagne < >> >> >> [email protected]>wrote: >> >> >> > >> >> >> >> On Tue, May 24, 2011 at 17:35, Thomas Mortagne >> >> >> >> <[email protected]> wrote: >> >> >> >> > On Tue, May 24, 2011 at 17:29, Denis Gervalle <[email protected]> >> >> wrote: >> >> >> >> >> On Tue, May 24, 2011 at 16:17, Thomas Mortagne < >> >> >> >> [email protected]>wrote: >> >> >> >> >> >> >> >> >> >>> On Tue, May 24, 2011 at 15:57, Denis Gervalle <[email protected]> >> >> >> wrote: >> >> >> >> >>> > Hi Devs, >> >> >> >> >>> > >> >> >> >> >>> > Anyone of you will surely agree that the hidden document >> >> feature >> >> >> >> >>> implemented >> >> >> >> >>> > in the store is very bad. >> >> >> >> >>> >> >> >> >> >>> The way this "feature" is implemented should never have been >> >> >> accepted, >> >> >> >> >>> it just broke an API for something that is not really related >> to >> >> >> >> >>> storage... >> >> >> >> >>> >> >> >> >> >>> See http://jira.xwiki.org/jira/browse/XWIKI-3925 and its >> >> >> dependencies. >> >> >> >> >>> >> >> >> >> >>> > IMO, it has never been fully implemented, probably in the >> hope >> >> of >> >> >> a >> >> >> >> >>> better >> >> >> >> >>> > way to go, and it is so for too long. I think it is the time >> to >> >> >> take >> >> >> >> some >> >> >> >> >>> > decision about it, or I do not see the direction and I do >> not >> >> >> >> understand >> >> >> >> >>> > where we want to go ? >> >> >> >> >>> > >> >> >> >> >>> > I see 3 possibilities: >> >> >> >> >>> > 1) we remove it and found other way to solve the problem it >> >> >> solves, >> >> >> >> >>> which >> >> >> >> >>> > are currently limited to the Blog, ColorThemes and Panels >> >> >> >> applications in >> >> >> >> >>> a >> >> >> >> >>> > standard XE. >> >> >> >> >>> >> >> >> >> >>> +1 >> >> >> >> >>> >> >> >> >> >> >> >> >> >> >> Do you means that you are +1 for reverting the code to what it >> was >> >> >> >> before >> >> >> >> >> that feature, and putting some code in each application using >> it >> >> to >> >> >> >> avoid >> >> >> >> >> the effet of the revert ? >> >> >> >> >> >> >> >> >> >> >> >> >> >> >>> >> >> >> >> >>> > 2) we keep it as it is, since it could be hard to implement >> >> >> higher >> >> >> >> in >> >> >> >> >>> the >> >> >> >> >>> > current implementation, but then we need to fix the places >> >> where >> >> >> it >> >> >> >> cause >> >> >> >> >>> > issues. >> >> >> >> >>> >> >> >> >> >>> -1, I can see it as a long term solution. It's something to >> say >> >> we >> >> >> >> >>> will fix it latter it's something else to validate it. Adding >> a >> >> >> >> >>> boolean to searchDocument as indicated in >> >> >> >> >>> http://jira.xwiki.org/jira/browse/XWIKI-3925 would already be >> a >> >> lot >> >> >> >> >>> better than the current situation. >> >> >> >> >>> >> >> >> >> >>> > 3) we implement the feature using another method ? >> >> >> >> >>> >> >> >> >> >>> I don't fully understand what is the difference between 1) and >> >> 3). >> >> >> >> >>> >> >> >> >> >> >> >> >> >> >> The difference is that in 3), you propose an alternative >> solution >> >> to >> >> >> the >> >> >> >> >> same issue, which is hiding document from public interface. >> >> >> >> > >> >> >> >> > "putting some code in each application using it to avoid the >> effet >> >> of >> >> >> >> > the revertv" is pretty much the same thing as "propose an >> >> alternative >> >> >> >> > solution to the same issue": in both case searchDocument go back >> to >> >> >> >> > what is used to be and you need to filter another way >> >> >> >> >> >> >> >> Anyway whatever the real difference between 1) and 3) I think we >> need >> >> >> >> a filtering system and the way it's done now is bad. >> >> >> >> >> >> >> > >> >> >> > So, you (Thomas and Jerome) would be in favor of reverting to the >> old >> >> >> > behavior, improving the feature by using a boolean for example, and >> >> >> setting >> >> >> > that boolean only when we want the filter applied (for example, in >> >> calls >> >> >> > from the public API). Since I completely agree that this should >> have >> >> been >> >> >> > done that way in the first place, I would like to propose that in >> >> vote. >> >> >> is >> >> >> > there any comments from others before I do ? >> >> >> >> >> >> An idea to avoid duplicating all the searchDocument methods is to add >> >> >> the hiddent document setup at org.xwiki.query.Query level. >> >> >> >> >> >> Basically we come back to clean searchDocument implementation in >> >> >> XWikiHibernateStore and we put the hidden document filter support in >> >> >> Query interface with a Query#setHiddenFiltered or something like >> that. >> >> >> In any case using query manager is supposed to be the proper current >> >> >> way of dealing with XWiki model so we don't have to support anything >> >> >> in "old" storage APIs. >> >> >> >> >> > >> >> > But to meet the objective of XWIKI-2843, which is to hide hidden >> document >> >> > from all public API (those not requiring PR), this would also means to >> >> use >> >> > org.xwiki.query.Query in all existing public API, or deprecate all >> those >> >> not >> >> > using it ? >> >> >> >> I'm ok with either refactoring them to use query manager behind the >> >> scene (if I don't have to do it ;)) or just deprecate them. Whatever >> >> the solution they should probably be deprecated anyway since they are >> >> supposed to be covered by query manager AFAIK. >> >> >> > >> > What about the default ? wouldn't it be to filter hidden document by the >> > default, since creating XWQL query is public and we want public API to be >> > filtered ? Then the function could be Query#withHiddenDocument(boolean) >> and >> > require PR to set it true ? >> >> No sure what you propose in detail but I'm -1 to have >> withHiddenDocument to true by default in DefaultQuery, it should be >> set to true in QueryManagerScriptService methods. >> > > I think you means that you want withHiddenDocument set to true in > DefaultQuery, which means no filtering.
Yes I misread you method name. I meant no filtering by default basically. > From my reading of the code, shouldn't it be defaulted to false when the > query manager is secure and to true otherwise ? Did not know secure thing. I always just used QueryManager#createQuery from java and QueryManagerScriptService#xwql or QueryManagerScriptService#hql from velocity. Just looked at it and indeed it make sense to filter by default in secure QueryManager. > > >> >> > Is there any existing usage of Query that should be fix to not became >> broken >> > by this new behavior ? >> >> Looks like there are not much (which is not very good...). >> >> I could only find the following: >> * Scheduler.WebHome >> * XWiki/WikiMacros >> * createinline.vm >> >> There is much more in java but it would not be affected. >> >> > WDYT ? >> > >> > >> >> >> >> > >> >> > >> >> >> >> >> >> > >> >> >> > >> >> >> >> >> >> >> >> > >> >> >> >> >> Maybe, 3) is more like your proposal for 2), while 2) means >> using >> >> >> search >> >> >> >> in >> >> >> >> >> place of searchDocument to bypass the filter. >> >> >> >> >> >> >> >> >> >> >> >> >> >> >>> >> >> >> >> >>> > >> >> >> >> >>> > If we choose 1), early in 3.x release is the probably best >> >> moment >> >> >> for >> >> >> >> it, >> >> >> >> >>> > since it is a breakage in compatibility, I am -0 on this >> >> however. >> >> >> >> >>> > >> >> >> >> >>> > If we choose 2), we need to make it work properly by fixing >> >> places >> >> >> >> where >> >> >> >> >>> we >> >> >> >> >>> > need to include all document, including hidden one. I have >> some >> >> >> old >> >> >> >> patch >> >> >> >> >>> to >> >> >> >> >>> > the application-manager to export hidden document (ie: >> >> currently >> >> >> the >> >> >> >> blog >> >> >> >> >>> > application does not export properly), to the skinx plugin >> >> that >> >> >> does >> >> >> >> not >> >> >> >> >>> > apply 'always' skin extensions contained in hidden document, >> >> and >> >> >> >> there is >> >> >> >> >>> > probably other places. >> >> >> >> >>> > >> >> >> >> >>> > If we choose 3) now, what do you proposed to better >> implement >> >> it. >> >> >> I >> >> >> >> have >> >> >> >> >>> > read some comments that it was a UI level stuff implemented >> at >> >> the >> >> >> >> store >> >> >> >> >>> > level, but I do not see how it could be done better in the >> >> current >> >> >> >> >>> > implementation. >> >> >> >> >>> > >> >> >> >> >>> > Moreover, if we keep the feature, I think that it should be >> >> >> exposed >> >> >> >> >>> somehow >> >> >> >> >>> > to the admins, allowing the creation of hidden document, but >> >> also >> >> >> >> listing >> >> >> >> >>> > them, deleting them properly, etc... Concerning the document >> >> >> provided >> >> >> >> >>> with >> >> >> >> >>> > XE, I also wonder what could be the rules for hiding them or >> >> not ? >> >> >> >> Why >> >> >> >> >>> not >> >> >> >> >>> > also hidding stock document in the XWiki space, just keeping >> >> users >> >> >> >> and >> >> >> >> >>> some >> >> >> >> >>> > top level documents ? >> >> >> >> >>> > >> >> >> >> >>> > WDYT ? >> >> >> >> >>> > >> >> >> >> >>> > -- >> >> >> >> >>> > Denis Gervalle >> >> >> >> >>> > SOFTEC sa - CEO >> >> >> >> >>> > eGuilde sarl - CTO >> >> >> >> >>> > _______________________________________________ >> >> >> >> >>> > 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 >> >> >> >> >>> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> -- >> >> >> >> >> Denis Gervalle >> >> >> >> >> SOFTEC sa - CEO >> >> >> >> >> eGuilde sarl - CTO >> >> >> >> >> _______________________________________________ >> >> >> >> >> 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 >> >> >> >> >> >> >> > >> >> >> > >> >> >> > >> >> >> > -- >> >> >> > Denis Gervalle >> >> >> > SOFTEC sa - CEO >> >> >> > eGuilde sarl - CTO >> >> >> > _______________________________________________ >> >> >> > 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 >> >> >> >> >> > >> >> > >> >> > >> >> > -- >> >> > Denis Gervalle >> >> > SOFTEC sa - CEO >> >> > eGuilde sarl - CTO >> >> > _______________________________________________ >> >> > 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 >> >> >> > >> > >> > >> > -- >> > Denis Gervalle >> > SOFTEC sa - CEO >> > eGuilde sarl - CTO >> > _______________________________________________ >> > 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 >> > > > > -- > Denis Gervalle > SOFTEC sa - CEO > eGuilde sarl - CTO > _______________________________________________ > 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
