From 3d5758773c8309f53f0150b99c1eca5bfa355040 Mon Sep 17 00:00:00 2001 From: Leo Hentschker Date: Sun, 15 Dec 2024 20:18:49 -0500 Subject: [PATCH 1/3] fix(api): adds v4 usage to api tracking --- cl/api/utils.py | 5 ++++- cl/tests/api/utils.py | 48 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 cl/tests/api/utils.py diff --git a/cl/api/utils.py b/cl/api/utils.py index 570c48c769..6a132fc297 100644 --- a/cl/api/utils.py +++ b/cl/api/utils.py @@ -736,7 +736,10 @@ def invert_user_logs( dates = make_date_str_list(start, end) for d in dates: - pipe.zrange(f"api:v3.user.d:{d}.counts", 0, -1, withscores=True) + for version in ["v3", "v4"]: + pipe.zrange( + f"api:{version}.user.d:{d}.counts", 0, -1, withscores=True + ) results = pipe.execute() # results is a list of results for each of the zrange queries above. Zip diff --git a/cl/tests/api/utils.py b/cl/tests/api/utils.py new file mode 100644 index 0000000000..6747d50c85 --- /dev/null +++ b/cl/tests/api/utils.py @@ -0,0 +1,48 @@ +from unittest.mock import MagicMock, patch + +from cl.api.utils import invert_user_logs +from cl.tests.cases import SimpleTestCase + +USER_1_ID = 1 +USER_2_ID = 2 + + +class TestApiUsage(SimpleTestCase): + def setUp(self): + self.start_date = "2023-01-01" + self.end_date = "2023-01-01" + self.expected_dates = ["2023-01-01"] + self.v3_results = [(USER_1_ID, 10), (USER_2_ID, 20)] + self.v4_results = [(USER_1_ID, 15), (USER_2_ID, 25)] + + @patch("cl.api.utils.get_redis_interface") + def test_invert_user_logs_shows_v4_logs(self, mock_get_redis_interface): + mock_redis = MagicMock() + mock_pipeline = MagicMock() + mock_get_redis_interface.return_value = mock_redis + mock_redis.pipeline.return_value = mock_pipeline + + mock_pipeline.execute.return_value = [ + self.v3_results, + self.v4_results, + ] + + results = invert_user_logs( + self.start_date, self.end_date, add_usernames=False + ) + + for date in self.expected_dates: + mock_pipeline.zrange.assert_any_call( + f"api:v3.user.d:{date}.counts", 0, -1, withscores=True + ) + mock_pipeline.zrange.assert_any_call( + f"api:v4.user.d:{date}.counts", 0, -1, withscores=True + ) + + self.assertIn(USER_1_ID, results) + self.assertIn(USER_2_ID, results) + + self.assertEqual(results[USER_1_ID]["2023-01-01"], 25) + self.assertEqual(results[USER_1_ID]["total"], 25) + self.assertEqual(results[USER_2_ID]["2023-01-01"], 45) + self.assertEqual(results[USER_2_ID]["total"], 45) From 1307dd48d8af2f3c431f1e99f24afb00b5dc533a Mon Sep 17 00:00:00 2001 From: Leo Hentschker Date: Sat, 21 Dec 2024 12:49:39 -0500 Subject: [PATCH 2/3] fix(api): accurately mocking + logic error fix --- cl/api/utils.py | 44 ++++++++++++++++++---------- cl/tests/api/utils.py | 68 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/cl/api/utils.py b/cl/api/utils.py index 6a132fc297..ea507c43d0 100644 --- a/cl/api/utils.py +++ b/cl/api/utils.py @@ -732,31 +732,43 @@ def invert_user_logs( :return The inverted dictionary """ r = get_redis_interface("STATS") - pipe = r.pipeline() dates = make_date_str_list(start, end) - for d in dates: - for version in ["v3", "v4"]: + + versions = ["v3", "v4"] + + for version in versions: + pipe = r.pipeline() + for d in dates: pipe.zrange( f"api:{version}.user.d:{d}.counts", 0, -1, withscores=True ) - results = pipe.execute() + + all_results = pipe.execute() + results_by_version_all_dates = [ + all_results[i : i + len(dates)] + for i in range(0, len(all_results), len(dates)) + ] # results is a list of results for each of the zrange queries above. Zip # those results with the date that created it, and invert the whole thing. out: defaultdict = defaultdict(dict) - for d, result in zip(dates, results): - for user_id, count in result: - if user_id == "None" or user_id == "AnonymousUser": - user_id = "AnonymousUser" - else: - user_id = int(user_id) - count = int(count) - if out.get(user_id): - out[user_id][d] = count - out[user_id]["total"] += count - else: - out[user_id] = {d: count, "total": count} + for d, *results_by_version_for_date in zip( + dates, *results_by_version_all_dates + ): + for result in results_by_version_for_date: + for user_id, count in result: + if user_id == "None" or user_id == "AnonymousUser": + user_id = "AnonymousUser" + else: + user_id = int(user_id) + count = int(count) + if out.get(user_id): + existing_count = out[user_id].get(d, 0) + out[user_id][d] = existing_count + count + out[user_id]["total"] += count + else: + out[user_id] = {d: count, "total": count} # Sort the values for k, v in out.items(): diff --git a/cl/tests/api/utils.py b/cl/tests/api/utils.py index 6747d50c85..a15ce276a8 100644 --- a/cl/tests/api/utils.py +++ b/cl/tests/api/utils.py @@ -5,33 +5,32 @@ USER_1_ID = 1 USER_2_ID = 2 +USER_3_ID = 3 class TestApiUsage(SimpleTestCase): - def setUp(self): - self.start_date = "2023-01-01" - self.end_date = "2023-01-01" - self.expected_dates = ["2023-01-01"] - self.v3_results = [(USER_1_ID, 10), (USER_2_ID, 20)] - self.v4_results = [(USER_1_ID, 15), (USER_2_ID, 25)] @patch("cl.api.utils.get_redis_interface") - def test_invert_user_logs_shows_v4_logs(self, mock_get_redis_interface): + def test_invert_user_logs_shows_v4_logs_one_date( + self, mock_get_redis_interface + ): mock_redis = MagicMock() mock_pipeline = MagicMock() mock_get_redis_interface.return_value = mock_redis mock_redis.pipeline.return_value = mock_pipeline mock_pipeline.execute.return_value = [ - self.v3_results, - self.v4_results, + # v3 + [(USER_1_ID, 10), (USER_2_ID, 20), (USER_3_ID, 30)], + # v4 + [(USER_1_ID, 15), (USER_2_ID, 25), (USER_3_ID, 35)], ] results = invert_user_logs( - self.start_date, self.end_date, add_usernames=False + "2023-01-01", "2023-01-01", add_usernames=False ) - for date in self.expected_dates: + for date in ["2023-01-01"]: mock_pipeline.zrange.assert_any_call( f"api:v3.user.d:{date}.counts", 0, -1, withscores=True ) @@ -46,3 +45,50 @@ def test_invert_user_logs_shows_v4_logs(self, mock_get_redis_interface): self.assertEqual(results[USER_1_ID]["total"], 25) self.assertEqual(results[USER_2_ID]["2023-01-01"], 45) self.assertEqual(results[USER_2_ID]["total"], 45) + self.assertEqual(results[USER_3_ID]["2023-01-01"], 65) + self.assertEqual(results[USER_3_ID]["total"], 65) + + @patch("cl.api.utils.get_redis_interface") + def test_invert_user_logs_shows_v4_logs_two_dates( + self, mock_get_redis_interface + ): + mock_redis = MagicMock() + mock_pipeline = MagicMock() + mock_get_redis_interface.return_value = mock_redis + mock_redis.pipeline.return_value = mock_pipeline + + mock_pipeline.execute.return_value = [ + # v3 + [(USER_2_ID, 20), (USER_3_ID, 30)], + [(USER_1_ID, 15), (USER_2_ID, 25), (USER_3_ID, 35)], + # v4 + [(USER_1_ID, 20), (USER_2_ID, 30)], + [(USER_1_ID, 25), (USER_2_ID, 35), (USER_3_ID, 45)], + ] + + results = invert_user_logs( + "2023-01-01", "2023-01-02", add_usernames=False + ) + + for date in ["2023-01-01", "2023-01-02"]: + mock_pipeline.zrange.assert_any_call( + f"api:v3.user.d:{date}.counts", 0, -1, withscores=True + ) + mock_pipeline.zrange.assert_any_call( + f"api:v4.user.d:{date}.counts", 0, -1, withscores=True + ) + + self.assertIn(USER_1_ID, results) + self.assertIn(USER_2_ID, results) + + self.assertEqual(results[USER_1_ID]["2023-01-01"], 20) + self.assertEqual(results[USER_1_ID]["2023-01-02"], 40) + self.assertEqual(results[USER_1_ID]["total"], 60) + + self.assertEqual(results[USER_2_ID]["2023-01-01"], 50) + self.assertEqual(results[USER_2_ID]["2023-01-02"], 60) + self.assertEqual(results[USER_2_ID]["total"], 110) + + self.assertEqual(results[USER_3_ID]["2023-01-01"], 30) + self.assertEqual(results[USER_3_ID]["2023-01-02"], 80) + self.assertEqual(results[USER_3_ID]["total"], 110) From 87d356d607f143aaa6d3258caf6bf24a08b7cecd Mon Sep 17 00:00:00 2001 From: Leo Hentschker Date: Sun, 22 Dec 2024 15:37:40 -0500 Subject: [PATCH 3/3] fix: resolve syntax error in cl/api/utils.py --- cl/api/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cl/api/utils.py b/cl/api/utils.py index ea507c43d0..e7ab931147 100644 --- a/cl/api/utils.py +++ b/cl/api/utils.py @@ -732,13 +732,13 @@ def invert_user_logs( :return The inverted dictionary """ r = get_redis_interface("STATS") + pipe = r.pipeline() dates = make_date_str_list(start, end) versions = ["v3", "v4"] for version in versions: - pipe = r.pipeline() for d in dates: pipe.zrange( f"api:{version}.user.d:{d}.counts", 0, -1, withscores=True