On Thu, Oct 17, 2024 at 03:32:58AM +0000, John Rouillard wrote:
> 
> I did a quick instrumentation of hasPermission in my custom tracker.
> I displayed an issue index page logged in as admin with Admin role.

Cool, I'll try running this for my tracker.

Concerning memoization: 
In method 'batch' in roundup/cgi/templating.py we have:

    def batch(self, permission='View'):
        check = self._client.db.security.hasPermission
        ...
        # filter for visibility
        allowed = [id for id in klass.filter(matches, filterspec, sort, group)
                   if check(permission, userid, self.classname, itemid=id)]

So we're calling the check method for *each issue found* (or whatever
table we're currently searching).
I would argue that if we find any permission on the given 'klass' that
does not have a check method, we do not need to do this: The check will
always tell us that we can see at least one property of *each* item in
the klass. So we do not need to filter and consequently do not need to
loop over all issues found when we establish that fact before the loop.

When rendering the HTML, the check is performed again for each property.
If the property should not be shown it is rendered as '[Hidden]'.

So no need for memoization.

If we find only permissions *with* a check method we *need* to check
*each of them* in the loop. And we cannot memoize things because the
check is different for each item found.

Now my second proposal comes into play: If we have check methods with
SQL (or some other mechanism that ultimately compiles to SQL) we can
*OR* all the SQL for each check method and put that with an AND into the
[sql generated by the] filter call. Then the loop would again only
return valid items and again we would not have to do it in python in the
loop.

Of course this applies only to View permissions. And only when
retrieving a list of issues (or other classes), either in the batch
method cited above or in the REST (and probably XMLRPC) api.

Note that I think the performance hog lies only in searching, not in
other permission checks like creation or editing. And it probably
applies only to 'View' permissions (I don't think batch is ever called
with an explicit permission other than 'View')

Thanks for looking into this
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com