From ed756d2884f7e581d84d84ef5ae2fd5c130f2036 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Wed, 8 Nov 2023 21:33:59 +0100 Subject: [PATCH 1/2] [FIX] Revert the `filtered_programs_for_user` change In #4644, the call signature of `filtered_programs_for_user` was changed, and the call `public_programs_for_user` removed. Unfortunately, there is no index on the pair `(username, public)`. Instead, we have indexes on `(username)` and on `(public, date)`. The first is used to retrieve all programs by a single user, the second is used to retrieve all public programs (for the "Explore" page). To get the public programs for a single user, instead what we do is retrieve all programs for a single user, and filter them down in Python. Because of a bug, trying to retrieve programs by `{ "username": "henk", "public": 1 }` uses the `(public, date)` index. However, the database layerbut does not detect that the field `date` is missing and that the field `username` is unnecessarily passed. The query incorrectly succeeds on the local database emulation layer, and when passed to an actual DynamoDB database the server returns an error. This currently breaks the profile page, as well as some other pages probably. Revert the changes from #4644 that lead to this problem, restoring the `public_programs_for_user` method and undoing the change to `filtered_programs_for_user`. --- app.py | 10 +++++----- website/database.py | 22 +++++++++++++++++++--- website/for_teachers.py | 12 ++++++------ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/app.py b/app.py index 590e3442d54..39e5e5f930e 100644 --- a/app.py +++ b/app.py @@ -954,7 +954,7 @@ def programs_page(user): filter = request.args.get('filter', default=None, type=str) submitted = True if filter == 'submitted' else None - result = DATABASE.filtered_programs_for_user({"username": from_user or username}, + result = DATABASE.filtered_programs_for_user(from_user or username, level=level, adventure=adventure, submitted=submitted, @@ -1756,7 +1756,7 @@ def reset_page(): @requires_login_redirect def profile_page(user): profile = DATABASE.user_by_username(user['username']) - programs = DATABASE.filtered_programs_for_user({"username": user['username'], "public": 1}) + programs = DATABASE.public_programs_for_user(user['username']) public_profile_settings = DATABASE.get_public_profile_settings(current_user()['username']) classes = [] @@ -2371,9 +2371,9 @@ def public_user_page(username): user_public_info = DATABASE.get_public_profile_settings(username) page = request.args.get('page', default=None, type=str) if user_public_info: - user_programs = DATABASE.filtered_programs_for_user({"username": user['username'], "public": 1}, - limit=10, - pagination_token=page) + user_programs = DATABASE.public_programs_for_user(username, + limit=10, + pagination_token=page) next_page_token = user_programs.next_page_token user_programs = normalize_public_programs(user_programs) user_achievements = DATABASE.progress_by_username(username) or {} diff --git a/website/database.py b/website/database.py index 14db29c2173..586091583c4 100644 --- a/website/database.py +++ b/website/database.py @@ -198,14 +198,15 @@ def programs_for_user(self, username): """ return PROGRAMS.get_many({"username": username}, reverse=True) - def filtered_programs_for_user(self, program_query, level=None, adventure=None, submitted=None, - limit=None, pagination_token=None, public=None): + def filtered_programs_for_user(self, username, level=None, adventure=None, submitted=None, + limit=None, pagination_token=None): ret = [] # FIXME: Query by index, the current behavior is slow for many programs # (See https://github.com/hedyorg/hedy/issues/4121) - programs = dynamo.GetManyIterator(PROGRAMS, program_query, + programs = dynamo.GetManyIterator(PROGRAMS, {"username": username}, reverse=True, batch_size=limit, pagination_token=pagination_token) + for program in programs: if level and program.get('level') != int(level): continue @@ -225,6 +226,21 @@ def filtered_programs_for_user(self, program_query, level=None, adventure=None, return dynamo.ResultPage(ret, programs.next_page_token) + def public_programs_for_user(self, username, limit=None, pagination_token=None): + # Only return programs that are public but not submitted + programs = dynamo.GetManyIterator(PROGRAMS, {"username": username}, + reverse=True, batch_size=limit, pagination_token=pagination_token) + ret = [] + for program in programs: + if program.get("public") != 1 or program.get("submitted", False): + continue + ret.append(program) + + if limit is not None and len(ret) >= limit: + break + + return dynamo.ResultPage(ret, programs.next_page_token) + def program_by_id(self, id): """Get program by ID. diff --git a/website/for_teachers.py b/website/for_teachers.py index e21075ad2d3..2de0fe30827 100644 --- a/website/for_teachers.py +++ b/website/for_teachers.py @@ -222,12 +222,12 @@ def public_programs(self, user, class_id, username): filter = request.args.get('filter', default=None, type=str) submitted = True if filter == 'submitted' else None - result = self.db.filtered_programs_for_user({"username": from_user, "public": 1}, - level=level, - adventure=adventure, - submitted=submitted, - pagination_token=page, - limit=10) + result = self.db.public_programs_for_user(from_user, + level=level, + adventure=adventure, + submitted=submitted, + pagination_token=page, + limit=10) programs = [] for item in result: From 1a353b73e5a234e7b0586ea9f85271a7b701392a Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Wed, 8 Nov 2023 21:46:37 +0100 Subject: [PATCH 2/2] You know what, this is better --- app.py | 9 +++++---- website/database.py | 19 +++---------------- website/for_teachers.py | 13 +++++++------ 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/app.py b/app.py index 39e5e5f930e..187d4177494 100644 --- a/app.py +++ b/app.py @@ -1756,7 +1756,7 @@ def reset_page(): @requires_login_redirect def profile_page(user): profile = DATABASE.user_by_username(user['username']) - programs = DATABASE.public_programs_for_user(user['username']) + programs = DATABASE.filtered_programs_for_user(user['username'], public=True) public_profile_settings = DATABASE.get_public_profile_settings(current_user()['username']) classes = [] @@ -2371,9 +2371,10 @@ def public_user_page(username): user_public_info = DATABASE.get_public_profile_settings(username) page = request.args.get('page', default=None, type=str) if user_public_info: - user_programs = DATABASE.public_programs_for_user(username, - limit=10, - pagination_token=page) + user_programs = DATABASE.filtered_programs_for_user(username, + public=True, + limit=10, + pagination_token=page) next_page_token = user_programs.next_page_token user_programs = normalize_public_programs(user_programs) user_achievements = DATABASE.progress_by_username(username) or {} diff --git a/website/database.py b/website/database.py index 586091583c4..767425bfdaf 100644 --- a/website/database.py +++ b/website/database.py @@ -198,7 +198,7 @@ def programs_for_user(self, username): """ return PROGRAMS.get_many({"username": username}, reverse=True) - def filtered_programs_for_user(self, username, level=None, adventure=None, submitted=None, + def filtered_programs_for_user(self, username, level=None, adventure=None, submitted=None, public=None, limit=None, pagination_token=None): ret = [] @@ -218,6 +218,8 @@ def filtered_programs_for_user(self, username, level=None, adventure=None, submi if submitted is not None: if program.get('submitted') != submitted: continue + if public is not None and bool(program.get('public')) != public: + continue ret.append(program) @@ -226,21 +228,6 @@ def filtered_programs_for_user(self, username, level=None, adventure=None, submi return dynamo.ResultPage(ret, programs.next_page_token) - def public_programs_for_user(self, username, limit=None, pagination_token=None): - # Only return programs that are public but not submitted - programs = dynamo.GetManyIterator(PROGRAMS, {"username": username}, - reverse=True, batch_size=limit, pagination_token=pagination_token) - ret = [] - for program in programs: - if program.get("public") != 1 or program.get("submitted", False): - continue - ret.append(program) - - if limit is not None and len(ret) >= limit: - break - - return dynamo.ResultPage(ret, programs.next_page_token) - def program_by_id(self, id): """Get program by ID. diff --git a/website/for_teachers.py b/website/for_teachers.py index 2de0fe30827..90a515893c9 100644 --- a/website/for_teachers.py +++ b/website/for_teachers.py @@ -222,12 +222,13 @@ def public_programs(self, user, class_id, username): filter = request.args.get('filter', default=None, type=str) submitted = True if filter == 'submitted' else None - result = self.db.public_programs_for_user(from_user, - level=level, - adventure=adventure, - submitted=submitted, - pagination_token=page, - limit=10) + result = self.db.filtered_programs_for_user(from_user, + level=level, + adventure=adventure, + submitted=submitted, + pagination_token=page, + public=True, + limit=10) programs = [] for item in result: