Skip to content

Commit

Permalink
Merge pull request #1780 from UlrichB22/performance01
Browse files Browse the repository at this point in the history
move 'user.may.write' and 'destroy' call from template to views.py
  • Loading branch information
RogerHaase authored Oct 17, 2024
2 parents 807bfe1 + 00391d3 commit 467ae73
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 23 deletions.
47 changes: 44 additions & 3 deletions src/moin/apps/frontend/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,17 @@ def flash_if_item_deleted(item_name, rev_id, itemrev):
return False


def get_item_permissions(fqname, item):
"""
get users permissions for item
Return a dict with permission type and permission
"""
permission_list = ["write", "create", "destroy"]
user = flaskg.user.may.names
item_may = dict([(key, flaskg.storage.may(fqname, key, usernames=user, item=item)) for key in permission_list])
return item_may


# The first form accepts POST to allow modifying behavior like modify_item.
# The second form only accepts GET since modifying a historical revision is not allowed.
@frontend.route("/<itemname:item_name>", defaults=dict(rev=CURRENT), methods=["GET", "POST"])
Expand All @@ -579,7 +590,8 @@ def show_item(item_name, rev):
item = Item.create(item_name, rev_id=rev)
flaskg.user.add_trail(item_name)
item_is_deleted = flash_if_item_deleted(item_name, rev, item)
result = item.do_show(rev, item_is_deleted=item_is_deleted)
item_may = get_item_permissions(fqname, item)
result = item.do_show(rev, item_is_deleted=item_is_deleted, item_may=item_may)
except AccessDenied:
abort(403)
except FieldNotUniqueError:
Expand Down Expand Up @@ -633,6 +645,7 @@ def indexable(item_name, rev):
def highlight_item(item):
rev_navigation_ids_dates = rev_navigation.prior_next_revs(request.view_args["rev"], item.fqname)
item_is_deleted = flash_if_item_deleted(item.fqname.fullname, item.rev.meta[REVID], item)
item_may = get_item_permissions(item.fqname, item)
try:
ret = render_template(
"highlight.html",
Expand All @@ -644,6 +657,7 @@ def highlight_item(item):
rev_navigation_ids_dates=rev_navigation_ids_dates,
meta=item._meta_info(),
item_is_deleted=item_is_deleted,
may=item_may,
)
except UnicodeDecodeError:
return _crash(item, None, None)
Expand All @@ -655,6 +669,7 @@ def highlight_item(item):
def show_item_meta(item):
rev_navigation_ids_dates = rev_navigation.prior_next_revs(request.view_args["rev"], item.fqname)
item_is_deleted = flash_if_item_deleted(item.fqname.fullname, item.rev.meta[REVID], item)
item_may = get_item_permissions(item.fqname, item)
ret = render_template(
"meta.html",
item=item,
Expand All @@ -665,6 +680,7 @@ def show_item_meta(item):
rev_navigation_ids_dates=rev_navigation_ids_dates,
meta=item._meta_info(),
item_is_deleted=item_is_deleted,
may=item_may,
)
close_file(item.meta.revision.data)
return ret
Expand Down Expand Up @@ -742,10 +758,16 @@ def convert_item(item_name):
abort(403)
if isinstance(item, NonExistent):
abort(404, item_name)
item_may = get_item_permissions(item_name, item)
form = ConvertForm.from_flat(request.form)
if request.method in ["GET", "HEAD"]:
return render_template(
"convert.html", item=item, form=form, contenttype=item.contenttype, fqname=split_fqname(item_name)
"convert.html",
item=item,
form=form,
contenttype=item.contenttype,
fqname=split_fqname(item_name),
may=item_may,
)

item.rev.data.seek(0)
Expand Down Expand Up @@ -831,8 +853,9 @@ def modify_item(item_name):
abort(403)
if not flaskg.user.may.write(item_name):
abort(403)
item_may = get_item_permissions(item_name, item)
try:
ret = item.do_modify()
ret = item.do_modify(item_may=item_may)
except ValueError as err:
# user may have changed or deleted namespace, contenttype... causing meta data validation failure
# or data unicode validation failed
Expand Down Expand Up @@ -951,6 +974,7 @@ def rename_item(item_name):
abort(403)
if isinstance(item, NonExistent):
abort(404, item_name)
item_may = get_item_permissions(item_name, item)
subitem_names = []
if request.method in ["GET", "HEAD"]:
form = RenameItemForm.from_defaults()
Expand Down Expand Up @@ -988,6 +1012,7 @@ def rename_item(item_name):
form=form,
data_rendered=Markup(item.content._render_data()),
len=len,
may=item_may,
)
close_file(item.meta.revision.data)
return ret
Expand All @@ -1003,6 +1028,7 @@ def delete_item(item_name):
abort(403)
if isinstance(item, NonExistent):
abort(404, item_name)
item_may = get_item_permissions(item_name, item)
subitem_names = []
if request.method in ["GET", "HEAD"]:
form = DeleteItemForm.from_defaults()
Expand Down Expand Up @@ -1032,6 +1058,7 @@ def delete_item(item_name):
fqname=split_fqname(item_name),
form=form,
data_rendered=data_rendered,
may=item_may,
)
close_file(item.rev.data)
return ret
Expand Down Expand Up @@ -1232,6 +1259,7 @@ def destroy_item(item_name, rev):
abort(403)
if isinstance(item, NonExistent):
abort(404, fqname.fullname)
item_may = get_item_permissions(fqname, item)
item_is_deleted = flash_if_item_deleted(item_name, rev, item)
subitem_names = []
alias_names = []
Expand Down Expand Up @@ -1274,6 +1302,7 @@ def destroy_item(item_name, rev):
form=form,
data_rendered=Markup(item.content._render_data()),
item_is_deleted=item_is_deleted,
may=item_may,
)
close_file(item.meta.revision.data)
close_file(item.rev.data)
Expand Down Expand Up @@ -1411,6 +1440,7 @@ def name_initial(files, uppercase=False, lowercase=False):
item = Item.create(item_name) # when item_name='', it gives toplevel index
except AccessDenied:
abort(403)
item_may = get_item_permissions(item_name, item)

# request.args is a MultiDict instance, which degenerates into a normal
# single-valued dict on most occasions (making the first value the *only*
Expand Down Expand Up @@ -1488,6 +1518,7 @@ def name_initial(files, uppercase=False, lowercase=False):
selected_groups=selected_groups,
str=str,
app=app,
may=item_may,
)


Expand Down Expand Up @@ -1572,12 +1603,14 @@ def forwardrefs(item_name):
:returns: a page with all the items linked from this item
"""
refs = _forwardrefs(item_name)
item_may = get_item_permissions(item_name, None)
return render_template(
"link_list_item_panel.html",
item_name=item_name,
fqname=split_fqname(item_name),
headline=_("Items that are referred by '{item_name}'").format(item_name=shorten_item_id(item_name)),
fq_names=split_fqname_list(refs),
may=item_may,
)


Expand Down Expand Up @@ -1614,13 +1647,15 @@ def backrefs(item_name):
except AccessDenied:
abort(403)
refs_here = _backrefs(item_name)
item_may = get_item_permissions(item_name, None)
return render_template(
"link_list_item_panel.html",
item=item,
item_name=item_name,
fqname=split_fqname(item_name),
headline=_("Items which refer to '{item_name}'").format(item_name=shorten_item_id(item_name)),
fq_names=refs_here,
may=item_may,
)


Expand Down Expand Up @@ -1650,6 +1685,7 @@ def history(item_name):
abort(404, item_name)

item_is_deleted = flash_if_item_deleted(item_name, CURRENT, item)
item_may = get_item_permissions(item_name, item)
page_num = request.values.get("page_num", 1)
page_num = max(int(page_num), 1)
bookmark_time = int(request.values.get("bookmark", 0))
Expand Down Expand Up @@ -1712,6 +1748,7 @@ def history(item_name):
len=len,
trash=trash,
item_is_deleted=item_is_deleted,
may=item_may,
)
flaskg.clock.stop("renderrevs")
close_file(item.rev.data)
Expand Down Expand Up @@ -2808,6 +2845,7 @@ def similar_names(item_name):
except AccessDenied:
abort(403)
fq_name = split_fqname(item_name)
item_may = get_item_permissions(fq_name, item)
start, end, matches = find_matches(fq_name)
keys = sorted(matches.keys())
# TODO later we could add titles for the misc ranks:
Expand All @@ -2829,6 +2867,7 @@ def similar_names(item_name):
item_name=item_name, # XXX no item
fqname=split_fqname(item_name),
fq_names=fq_names,
may=item_may,
)


Expand All @@ -2849,6 +2888,7 @@ def sitemap(item_name):
abort(403)
if isinstance(item, NonExistent):
abort(404, item_name)
item_may = get_item_permissions(item_name, item)

backrefs, junk, junk2 = NestedItemListBuilder().recurse_build([fq_name], backrefs=True)
del backrefs[0] # don't show current item name as sole toplevel list item
Expand All @@ -2863,6 +2903,7 @@ def sitemap(item_name):
fqname=fq_name,
no_read_auth=no_read_auth,
missing=missing,
may=item_may,
)


Expand Down
8 changes: 6 additions & 2 deletions src/moin/items/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,7 @@ def _do_modify_show_templates(self):
itemtype=self.itemtype,
contenttype=self.contenttype,
template="",
may=None,
)
)
return render_template(
Expand All @@ -1434,9 +1435,10 @@ def _do_modify_show_templates(self):
last_rev_id=rev_ids and rev_ids[-1],
meta_rendered="",
data_rendered="",
may=None,
)

def do_show(self, revid, item_is_deleted=False):
def do_show(self, revid, item_is_deleted=False, item_may=None):
"""
Display an item. If this is not the current revision, page content will be
prefaced with links to the next-rev and prior-rev.
Expand All @@ -1459,6 +1461,7 @@ def do_show(self, revid, item_is_deleted=False):
data_rendered=Markup(self.content._render_data()),
html_head_meta=html_head_meta,
item_is_deleted=item_is_deleted,
may=item_may,
)

def doc_link(self, content_name, link_text):
Expand Down Expand Up @@ -1513,7 +1516,7 @@ def meta_changed(self, meta):
return True
return False

def do_modify(self):
def do_modify(self, item_may=None):
if isinstance(self.content, NonExistentContent) and not flaskg.user.may.create(self.fqname):
abort(
403,
Expand Down Expand Up @@ -1737,6 +1740,7 @@ def do_modify(self):
draft_data=draft_data,
lock_duration=lock_duration,
tuple=tuple,
may=item_may,
)


Expand Down
5 changes: 5 additions & 0 deletions src/moin/items/blog.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from whoosh.query import Term, And, Prefix
from whoosh.sorting import FunctionFacet

from moin.apps.frontend.views import get_item_permissions
from moin.i18n import L_
from moin.themes import render_template
from moin.forms import Text, Tags, DateTime
Expand Down Expand Up @@ -87,6 +88,7 @@ def ptime_sort_key(searcher, docnum):

revs = flaskg.storage.search(query, sortedby=ptime_sort_facet, reverse=True, limit=None)
blog_entry_items = [Item.create(rev.name, rev_id=rev.revid) for rev in revs]
item_may = get_item_permissions(self.name, self)
return render_template(
"blog/main.html",
item_name=self.name,
Expand All @@ -95,6 +97,7 @@ def ptime_sort_key(searcher, docnum):
blog_entry_items=blog_entry_items,
tag=tag,
item=self,
may=item_may,
)


Expand Down Expand Up @@ -126,11 +129,13 @@ def do_show(self, revid, **kwargs):
if not isinstance(blog_item, Blog):
# The parent item of this blog entry item is not a Blog item.
abort(403)
item_may = get_item_permissions(blog_item_name, blog_item)
return render_template(
"blog/entry.html",
item_name=self.name,
fqname=blog_item.fqname,
blog_item=blog_item,
blog_entry_item=self,
item=self,
may=item_may,
)
9 changes: 5 additions & 4 deletions src/moin/security/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Copyright: 2003 Gustavo Niemeyer
# Copyright: 2005 Oliver Graf
# Copyright: 2007 Alexander Schremmer
# Copyright: 2024 MoinMoin:UlrichB
# License: GNU GPL v2 (or any later version), see LICENSE.txt for details.

"""
Expand Down Expand Up @@ -71,17 +72,17 @@ def read(self, itemname):
def __init__(self, user):
self.names = user.name

def read(self, itemname):
def read(self, itemname, item=None):
"""read permission is special as we have 2 kinds of read capabilities:
* READ - gives permission to read, unconditionally
* PUBREAD - gives permission to read, when published
"""
return flaskg.storage.may(itemname, rights.READ, usernames=self.names) or flaskg.storage.may(
itemname, rights.PUBREAD, usernames=self.names
return flaskg.storage.may(itemname, rights.READ, usernames=self.names, item=item) or flaskg.storage.may(
itemname, rights.PUBREAD, usernames=self.names, item=item
)

def __getattr__(self, attr):
def __getattr__(self, attr, item=None):
"""Shortcut to handle all known ACL rights.
if attr is a valid acl right, return a checking function for it.
Expand Down
14 changes: 10 additions & 4 deletions src/moin/storage/middleware/protecting.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Copyright: 2011 MoinMoin:ThomasWaldmann
# Copyright: 2024 MoinMoin:UlrichB
# License: GNU GPL v2 (or any later version), see LICENSE.txt for details.

"""
Expand All @@ -22,7 +23,6 @@
from moin.security import AccessControlList
from moin.utils import close_file
from moin.utils.interwiki import split_fqname, CompositeName

from moin import log

logging = log.getLogger(__name__)
Expand Down Expand Up @@ -292,7 +292,7 @@ def existing_item(self, **query):
item = self.indexer.existing_item(**query)
return ProtectedItem(self, item)

def may(self, fqname, capability, usernames=None):
def may(self, fqname, capability, usernames=None, item=None):
if usernames is not None and isinstance(usernames, (bytes, str)):
# we got a single username (maybe bytes), make a list of str:
if isinstance(usernames, bytes):
Expand All @@ -302,7 +302,10 @@ def may(self, fqname, capability, usernames=None):
fqname = fqname[0] if isinstance(fqname, list) else fqname
if isinstance(fqname, str):
fqname = split_fqname(fqname)
item = self.get_item(**fqname.query)
if item:
item = ProtectedItem(self, item)
else:
item = self.get_item(**fqname.query)
allowed = item.allows(capability, user_names=usernames)
return allowed

Expand Down Expand Up @@ -353,7 +356,10 @@ def full_acls(self):
including before/default/after acl.
"""
fqname = self.item.fqname
itemid = self.item.itemid
if hasattr(self.item, "itemid"):
itemid = self.item.itemid
else:
itemid = None
acl_cfg = self.protector._get_configured_acls(fqname)
before_acl = acl_cfg["before"]
after_acl = acl_cfg["after"]
Expand Down
Loading

0 comments on commit 467ae73

Please sign in to comment.