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. >From my reading of the code, shouldn't it be defaulted to false when the query manager is secure and to true otherwise ? > > > 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
