Skip to content

Commit

Permalink
fix the performance of the exploration pages
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Changaco committed Sep 10, 2021
1 parent 6a410d8 commit 3fa0a53
Show file tree
Hide file tree
Showing 17 changed files with 121 additions and 226 deletions.
10 changes: 0 additions & 10 deletions liberapay/elsewhere/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 1 addition & 3 deletions liberapay/elsewhere/github.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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')
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions liberapay/elsewhere/twitter.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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')
7 changes: 0 additions & 7 deletions liberapay/models/account_elsewhere.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
2 changes: 0 additions & 2 deletions liberapay/models/repository.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
from time import sleep

from oauthlib.oauth2 import InvalidGrantError, TokenExpiredError
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion liberapay/utils/fake_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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='',
)

Expand Down
4 changes: 4 additions & 0 deletions sql/branch.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
SELECT 'after deployment';

ALTER TABLE elsewhere DROP COLUMN extra_info;
ALTER TABLE repositories DROP COLUMN extra_info;
6 changes: 3 additions & 3 deletions templates/macros/pagination.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@
</ul>
% 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
<ul class="pager">
% if current_page > 1
<li class="previous"><a href="{{ request.path.raw if current_page == 2 else '?page=%i' % (current_page - 1) }}">
← {{ _("Previous") }}
</a></li>
% endif
% if current_page < last_page
% if has_more
<li class="next"><a href="?page={{ current_page + 1 }}">
{{ _("View More") if current_page == 1 else _("Next") }} →
</a></li>
Expand Down
19 changes: 0 additions & 19 deletions tests/py/test_elsewhere.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,25 +341,6 @@ def test_take_over(self):
assert response.headers[b'Location'].endswith(b'/bob/edit/elsewhere')


class TestFriendFinder(Harness):

def test_twitter_get_follows_for(self):
platform = self.platforms.twitter
user_info = platform.extract_user_info(user_info_examples.twitter(), '')
account = AccountElsewhere.upsert(user_info)
follows, nfollows, pages_urls = platform.get_follows_for(account)
assert follows
assert nfollows > 0

def test_github_get_follows_for(self):
platform = self.platforms.github
user_info = platform.extract_user_info(user_info_examples.github(), '')
account = AccountElsewhere.upsert(user_info)
follows, nfollows, pages_urls = platform.get_follows_for(account)
assert follows
assert nfollows == len(follows) or nfollows == -1


class TestElsewhereDelete(Harness):

def test_delete_nonexistent(self):
Expand Down
2 changes: 1 addition & 1 deletion www/explore/communities.spt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
query_cache = website.db_qc1
query_cache = website.db_qc5

[---]

Expand Down
2 changes: 0 additions & 2 deletions www/explore/index.html.spt
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
query_cache = website.db_qc5

[---]

title = _("Explore")
Expand Down
61 changes: 24 additions & 37 deletions www/explore/individuals.spt
Original file line number Diff line number Diff line change
@@ -1,40 +1,28 @@
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:
nindividuals = cursor.one("""
CREATE TEMP TABLE individuals ON COMMIT DROP AS
SELECT p
, ( 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 participants p
WHERE p.kind = 'individual'
AND p.status = 'active'
AND (p.goal > 0 OR p.goal IS NULL)
AND p.hide_receiving IS NOT TRUE
AND p.hide_from_lists = 0
AND p.receiving > 0;

SELECT count(1) FROM individuals;
""", (locale.language,))
last_page = max(int(ceil(nindividuals / per_page)), 1)
if current_page > last_page:
response.redirect("?page=%i" % last_page)
individuals = cursor.all("""
SELECT *
FROM individuals i
ORDER BY convert((i.p).receiving, 'EUR') DESC, (i.p).join_time DESC
LIMIT %s
OFFSET %s
""", (per_page, (current_page - 1) * per_page))
individuals = website.db_qc5.all("""
SELECT p
, ( 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 participants p
WHERE p.kind = 'individual'
AND p.status = 'active'
AND (p.goal > 0 OR p.goal IS NULL)
AND p.hide_receiving IS NOT TRUE
AND p.hide_from_lists = 0
AND p.receiving > 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(individuals) > per_page

title = _("Explore")
subhead = _("Individuals")
Expand All @@ -52,16 +40,15 @@ subhead = _("Individuals")
<p>{{ _("The top {0} individuals on Liberapay are:", len(individuals)) }}</p>
% else
<p>{{ _(
"List of individuals on Liberapay, page {number} of {total}:"
, number=current_page, total=last_page
"List of individuals on Liberapay, page {number}:", number=current_page
) }}</p>
% endif
<div class="inline-boxes">
% for p, summary in individuals
% for p, summary in individuals[:per_page]
{{ profile_box_embedded(p, summary) }}
% endfor
</div>
{{ simple_pager(current_page, last_page) }}
{{ simple_pager(current_page, has_more) }}
<br>
% else
<p>{{ _("Nothing to show.") }}</p>
Expand Down
62 changes: 25 additions & 37 deletions www/explore/organizations.spt
Original file line number Diff line number Diff line change
@@ -1,40 +1,28 @@
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:
norganizations = cursor.one("""
CREATE TEMP TABLE organizations ON COMMIT DROP AS
SELECT p
, ( 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 participants p
WHERE p.kind = 'organization'
AND p.status = 'active'
AND (p.goal > 0 OR p.goal IS NULL)
AND p.hide_receiving IS NOT TRUE
AND p.hide_from_lists = 0
AND p.receiving > 0;

SELECT count(1) FROM organizations;
""", (locale.language,))
last_page = max(int(ceil(norganizations / per_page)), 1)
if current_page > last_page:
response.redirect("?page=%i" % last_page)
organizations = cursor.all("""
SELECT *
FROM organizations o
ORDER BY convert((o.p).receiving, 'EUR') DESC, (o.p).join_time DESC
LIMIT %s
OFFSET %s
""", (per_page, (current_page - 1) * per_page))
organizations = website.db_qc5.all("""
SELECT p
, ( 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 participants p
WHERE p.kind = 'organization'
AND p.status = 'active'
AND (p.goal > 0 OR p.goal IS NULL)
AND p.hide_receiving IS NOT TRUE
AND p.hide_from_lists = 0
AND p.receiving > 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(organizations) > per_page

title = _("Explore")
subhead = _("Organizations")
Expand All @@ -52,16 +40,16 @@ subhead = _("Organizations")
<p>{{ _("The top {0} organizations on Liberapay are:", len(organizations)) }}</p>
% else
<p>{{ _(
"List of organizations on Liberapay, page {number} of {total}:"
, number=current_page, total=last_page
"List of organizations on Liberapay, page {number}:",
number=current_page
) }}</p>
% endif
<div class="inline-boxes">
% for p, summary in organizations
% for p, summary in organizations[:per_page]
{{ profile_box_embedded(p, summary) }}
% endfor
</div>
{{ simple_pager(current_page, last_page) }}
{{ simple_pager(current_page, has_more) }}
<br>
% else
<p>{{ _("Nothing to show.") }}</p>
Expand Down
Loading

0 comments on commit 3fa0a53

Please sign in to comment.