From 3fa0a53709fa3c00d9e8304083b7037300806d4b Mon Sep 17 00:00:00 2001
From: Changaco
Date: Thu, 9 Sep 2021 17:07:44 +0200
Subject: [PATCH] fix the performance of the exploration pages
In addition to modifying the database queries used by the exploration pages, this commit also closes #1061 by dropping the `extra_info` columns, because fetching all this extraneous data from the DB significantly degrades the performance of two of the exploration pages.
---
liberapay/elsewhere/_base.py | 10 ----
liberapay/elsewhere/github.py | 4 +-
liberapay/elsewhere/twitter.py | 3 +-
liberapay/models/account_elsewhere.py | 7 ---
liberapay/models/repository.py | 2 -
liberapay/utils/fake_data.py | 1 -
sql/branch.sql | 4 ++
templates/macros/pagination.html | 6 +--
tests/py/test_elsewhere.py | 19 --------
www/explore/communities.spt | 2 +-
www/explore/index.html.spt | 2 -
www/explore/individuals.spt | 61 ++++++++++--------------
www/explore/organizations.spt | 62 ++++++++++---------------
www/explore/pledges.spt | 40 ++++++----------
www/explore/repositories.spt | 46 +++++++-----------
www/explore/teams.spt | 67 +++++++++++----------------
www/migrate.spt | 11 ++---
17 files changed, 121 insertions(+), 226 deletions(-)
create mode 100644 sql/branch.sql
diff --git a/liberapay/elsewhere/_base.py b/liberapay/elsewhere/_base.py
index 24faedf44f..1d9d10eb6e 100644
--- a/liberapay/elsewhere/_base.py
+++ b/liberapay/elsewhere/_base.py
@@ -272,9 +272,6 @@ def extract_user_info(self, info, source):
r.avatar_url = 'https://seccdn.libravatar.org/avatar/'+gravatar_id
r.is_team = self.x_is_team(r, info, False)
r.description = self.x_description(r, info, None)
- r.extra_info = info
- if hasattr(self, 'x_extra_info_drop'):
- self.x_extra_info_drop(r.extra_info)
return r
def get_team_members(self, account, page_url=None):
@@ -344,8 +341,6 @@ def get_follows_for(self, account, page_url=None, sess=None):
r = self.api_get(account.domain, page_url, sess=sess)
follows, count, pages_urls = self.api_paginator(r, self.api_parser(r))
follows = [self.extract_user_info(f, account.domain) for f in follows]
- if count == -1 and hasattr(self, 'x_follows_count'):
- count = self.x_follows_count(None, account.extra_info, -1)
return follows, count, pages_urls
def extract_repo_info(self, info):
@@ -363,9 +358,6 @@ def extract_repo_info(self, info):
r.last_update = parse_date(r.last_update)
r.is_fork = self.x_repo_is_fork(r, info, None)
r.stars_count = self.x_repo_stars_count(r, info, None)
- r.extra_info = info
- if hasattr(self, 'x_repo_extra_info_drop'):
- self.x_repo_extra_info_drop(r.extra_info)
return r
def get_repos(self, account, page_url=None, sess=None, refresh=True):
@@ -394,8 +386,6 @@ def get_repos(self, account, page_url=None, sess=None, refresh=True):
raise TokenExpiredError()
# Note: we can't pass the page_url below, because it contains the old user_name
return self.get_repos(account, page_url=None, sess=sess, refresh=False)
- if count == -1 and hasattr(self, 'x_repos_count'):
- count = self.x_repos_count(None, account.extra_info, -1)
return repos, count, pages_urls
def get_starred_repos(self, account, sess, page_url=None):
diff --git a/liberapay/elsewhere/github.py b/liberapay/elsewhere/github.py
index 397a2c370a..51cd147952 100644
--- a/liberapay/elsewhere/github.py
+++ b/liberapay/elsewhere/github.py
@@ -1,6 +1,6 @@
from liberapay.elsewhere._base import PlatformOAuth2
from liberapay.elsewhere._exceptions import CantReadMembership
-from liberapay.elsewhere._extractors import key, drop_keys
+from liberapay.elsewhere._extractors import key
from liberapay.elsewhere._paginators import header_links_paginator
@@ -43,7 +43,6 @@ class GitHub(PlatformOAuth2):
x_avatar_url = key('avatar_url')
x_is_team = key('type', clean=lambda t: t.lower() == 'organization')
x_description = key('bio')
- x_extra_info_drop = drop_keys(lambda k: k.endswith('_url'))
# Repo info extractors
x_repo_id = key('id')
@@ -54,7 +53,6 @@ class GitHub(PlatformOAuth2):
x_repo_is_fork = key('fork')
x_repo_stars_count = key('stargazers_count')
x_repo_owner_id = key('owner', clean=lambda d: d['id'])
- x_repo_extra_info_drop = drop_keys(lambda k: k.endswith('_url'))
def get_CantReadMembership_url(self, account):
return 'https://github.com/orgs/%s/people' % account.user_name
diff --git a/liberapay/elsewhere/twitter.py b/liberapay/elsewhere/twitter.py
index 60c362c27c..045575294e 100644
--- a/liberapay/elsewhere/twitter.py
+++ b/liberapay/elsewhere/twitter.py
@@ -1,5 +1,5 @@
from liberapay.elsewhere._base import PlatformOAuth1
-from liberapay.elsewhere._extractors import drop_keys, key, not_available
+from liberapay.elsewhere._extractors import key, not_available
from liberapay.elsewhere._paginators import query_param_paginator
@@ -36,4 +36,3 @@ class Twitter(PlatformOAuth1):
clean=lambda v: v.replace('_normal.', '.'))
x_follows_count = key('friends_count')
x_description = key('description')
- x_extra_info_drop = drop_keys('id')
diff --git a/liberapay/models/account_elsewhere.py b/liberapay/models/account_elsewhere.py
index 4d6d381ad4..14a818361b 100644
--- a/liberapay/models/account_elsewhere.py
+++ b/liberapay/models/account_elsewhere.py
@@ -2,14 +2,12 @@
import json
from urllib.parse import urlsplit, urlunsplit
import uuid
-import xml.etree.ElementTree as ET
from markupsafe import Markup
from oauthlib.oauth2 import InvalidGrantError, TokenExpiredError
from pando.utils import utcnow
from postgres.orm import Model
from psycopg2 import IntegrityError
-import xmltodict
from ..constants import AVATAR_QUERY, DOMAIN_RE, RATE_LIMITS, SUMMARY_MAX_SIZE
from ..cron import logger
@@ -122,11 +120,6 @@ def upsert(cls, i):
query = AVATAR_QUERY
i.avatar_url = urlunsplit((scheme, netloc, path, query, fragment))
- # Serialize extra_info
- if isinstance(i.extra_info, ET.Element):
- i.extra_info = xmltodict.parse(ET.tostring(i.extra_info))
- i.extra_info = json.dumps(i.extra_info)
-
d = dict(i.__dict__)
d.pop('email', None)
cols, vals = zip(*d.items())
diff --git a/liberapay/models/repository.py b/liberapay/models/repository.py
index 0cbe264348..a43c0c5807 100644
--- a/liberapay/models/repository.py
+++ b/liberapay/models/repository.py
@@ -1,4 +1,3 @@
-import json
from time import sleep
from oauthlib.oauth2 import InvalidGrantError, TokenExpiredError
@@ -48,7 +47,6 @@ def upsert_repos(cursor, repos, participant, info_fetched_at):
if not repo.owner_id or not repo.last_update:
continue
repo.participant = participant.id
- repo.extra_info = json.dumps(repo.extra_info)
repo.info_fetched_at = info_fetched_at
cols, vals = zip(*repo.__dict__.items())
on_conflict_set = ','.join('{0}=excluded.{0}'.format(col) for col in cols)
diff --git a/liberapay/utils/fake_data.py b/liberapay/utils/fake_data.py
index c79ff95a3d..d3d93ea7ef 100644
--- a/liberapay/utils/fake_data.py
+++ b/liberapay/utils/fake_data.py
@@ -128,7 +128,6 @@ def fake_elsewhere(db, participant, platform):
user_id=fake_text_id(),
user_name=participant.id,
participant=participant.id,
- extra_info=None,
domain='',
)
diff --git a/sql/branch.sql b/sql/branch.sql
new file mode 100644
index 0000000000..74da0d338d
--- /dev/null
+++ b/sql/branch.sql
@@ -0,0 +1,4 @@
+SELECT 'after deployment';
+
+ALTER TABLE elsewhere DROP COLUMN extra_info;
+ALTER TABLE repositories DROP COLUMN extra_info;
diff --git a/templates/macros/pagination.html b/templates/macros/pagination.html
index cc42256b08..fa21522801 100644
--- a/templates/macros/pagination.html
+++ b/templates/macros/pagination.html
@@ -23,15 +23,15 @@
% endmacro
-% macro simple_pager(current_page, last_page)
- % if last_page != 1
+% macro simple_pager(current_page, has_more)
+ % if current_page > 1 or has_more
- % for p in pledgees
+ % for p in pledgees[:per_page]
{{ profile_box_embedded_elsewhere(p) }}
% endfor
- {{ simple_pager(current_page, last_page) }}
+ {{ simple_pager(current_page, has_more) }}
% else
{{ _("There are no unclaimed donations right now.") }}
% endif
diff --git a/www/explore/repositories.spt b/www/explore/repositories.spt
index 9f0d75dcab..9fead8da96 100644
--- a/www/explore/repositories.spt
+++ b/www/explore/repositories.spt
@@ -1,32 +1,20 @@
-from math import ceil
-
[---]
per_page = 20
current_page = request.qs.get_int('page', default=1, minimum=1)
-with website.db.get_cursor() as cursor:
- nrepos = cursor.one("""
- CREATE TEMP TABLE repos ON COMMIT DROP AS
- SELECT r, p
- FROM repositories r
- JOIN elsewhere e ON e.platform = r.platform AND e.domain = '' AND e.user_id = r.owner_id
- JOIN participants p ON p.id = e.participant
- WHERE r.stars_count > 1
- AND r.show_on_profile
- AND p.status = 'active';
-
- SELECT count(1) FROM repos;
- """, (locale.language,))
- last_page = max(int(ceil(nrepos / per_page)), 1)
- if current_page > last_page:
- response.redirect("?page=%i" % last_page)
- repos = cursor.all("""
- SELECT *
- FROM repos
- ORDER BY (r).stars_count DESC, (r).id DESC
- LIMIT %s
- OFFSET %s
- """, (per_page, (current_page - 1) * per_page))
+repos = website.db_qc5.all("""
+ SELECT r, p
+ FROM repositories r
+ JOIN elsewhere e ON e.platform = r.platform AND e.domain = '' AND e.user_id = r.owner_id
+ JOIN participants p ON p.id = e.participant
+ WHERE r.stars_count > 1
+ AND r.show_on_profile
+ AND p.status = 'active'
+ ORDER BY r.stars_count DESC, r.id DESC
+ LIMIT %s
+ OFFSET %s
+""", (per_page + 1, (current_page - 1) * per_page))
+has_more = len(repos) > per_page
title = _("Explore")
subhead = _("Repositories")
@@ -46,13 +34,13 @@ subhead = _("Repositories")
) }}
% else
{{ _(
- "List of repositories currently linked to a Liberapay account, page {number} of {total}:"
- , number=current_page, total=last_page
+ "List of repositories currently linked to a Liberapay account, page {number}:",
+ number=current_page
) }}
% endif
-% for repo, owner in repos
+% for repo, owner in repos[:per_page]
@@ -75,7 +63,7 @@ subhead = _("Repositories")
% endfor
-{{ simple_pager(current_page, last_page) }}
+{{ simple_pager(current_page, has_more) }}
diff --git a/www/explore/teams.spt b/www/explore/teams.spt
index 69da55030c..a7846c0da0 100644
--- a/www/explore/teams.spt
+++ b/www/explore/teams.spt
@@ -1,42 +1,30 @@
-from math import ceil
-
[---]
per_page = 18
current_page = request.qs.get_int('page', default=1, minimum=1)
-with website.db.get_cursor() as cursor:
- nteams = cursor.one("""
- CREATE TEMP TABLE teams ON COMMIT DROP AS
- SELECT p AS participant
- , t.*
- , ( SELECT s.content
- FROM statements s
- WHERE s.participant = p.id
- AND s.type = 'summary'
- ORDER BY s.lang = %s DESC, s.id
- LIMIT 1
- ) AS summary
- FROM ( SELECT team AS id, count(member) AS nmembers
- FROM current_takes
- GROUP BY team
- ) AS t
- JOIN participants p ON p.id = t.id
- WHERE p.status = 'active'
- AND p.hide_from_lists = 0;
-
- SELECT count(1) FROM teams;
- """, (locale.language,))
- last_page = max(int(ceil(nteams / per_page)), 1)
- if current_page > last_page:
- response.redirect("?page=%i" % last_page)
- teams = cursor.all("""
- SELECT *
- FROM teams t
- ORDER BY convert((t.participant).receiving, 'EUR') DESC
- , (t.participant).join_time DESC
- LIMIT %s
- OFFSET %s
- """, (per_page, (current_page - 1) * per_page))
+teams = website.db_qc5.all("""
+ SELECT p AS participant
+ , t.*
+ , ( SELECT s.content
+ FROM statements s
+ WHERE s.participant = p.id
+ AND s.type = 'summary'
+ ORDER BY s.lang = %s DESC, s.id
+ LIMIT 1
+ ) AS summary
+ FROM ( SELECT team AS id, count(member) AS nmembers
+ FROM current_takes
+ GROUP BY team
+ ) AS t
+ JOIN participants p ON p.id = t.id
+ WHERE p.status = 'active'
+ AND p.hide_from_lists = 0
+ ORDER BY convert(p.receiving, 'EUR') DESC
+ , p.join_time DESC
+ LIMIT %s
+ OFFSET %s
+""", (locale.language, per_page + 1, (current_page - 1) * per_page))
+has_more = len(teams) > per_page
title = _("Explore")
subhead = _("Teams")
@@ -60,19 +48,16 @@ subhead = _("Teams")
len(teams)
) }}
% else
-
{{ _(
- "List of teams on Liberapay, page {number} of {total}:"
- , number=current_page, total=last_page
-) }}
+
{{ _("List of teams on Liberapay, page {number}:", number=current_page) }}
% endif
- % for team in teams
+ % for team in teams[:per_page]
{{ profile_box_embedded(team.participant, team.summary, nmembers=team.nmembers) }}
% endfor
-{{ simple_pager(current_page, last_page) }}
+{{ simple_pager(current_page, has_more) }}
{{ _("Create a team") }}
diff --git a/www/migrate.spt b/www/migrate.spt
index 2b3058ce5e..d9816d33e5 100644
--- a/www/migrate.spt
+++ b/www/migrate.spt
@@ -147,12 +147,12 @@ elif step == '2':
elsewhere = cursor.one("""
INSERT INTO elsewhere
(platform, user_id, user_name, display_name, email,
- is_team, avatar_url, extra_info, domain, participant)
+ is_team, avatar_url, domain, participant)
VALUES (%s, %s, %s, %s, %s,
- %s, %s, %s, '', %s)
+ %s, %s, '', %s)
RETURNING elsewhere::elsewhere_with_participant
""", (e.platform, e.user_id, e.user_name, e.display_name, e.email,
- e.is_team, e.avatar_url, json.dumps(e.extra_info), tippee.id))
+ e.is_team, e.avatar_url, tippee.id))
del e
tippee = elsewhere.participant
if tippee == p:
@@ -188,13 +188,12 @@ elif step == '2':
else:
elsewhere.append(('self-conflict', e))
continue
- e['extra_info'] = json.dumps(e['extra_info'])
added = website.db.one("""
INSERT INTO elsewhere
(platform, user_id, user_name, display_name, email,
- is_team, extra_info, domain, participant)
+ is_team, domain, participant)
VALUES (%(platform)s, %(user_id)s, %(user_name)s, %(display_name)s, %(email)s,
- %(is_team)s, %(extra_info)s, '', %(p_id)s)
+ %(is_team)s, '', %(p_id)s)
ON CONFLICT (platform, user_id, domain) DO NOTHING
RETURNING true AS added
""", dict(e, p_id=p.id))