Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: sitescripts/notifications/web/notification.py

Issue 4883267715072000: Issue 2276 - Handle groups in notification.json requests (Closed)
Left Patch Set: Refactor Created April 17, 2015, 3 a.m.
Right Patch Set: Extract group parsing Created June 12, 2015, 12:11 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « sitescripts/notifications/web/__init__.py ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 # coding: utf-8 1 # coding: utf-8
2 2
3 # This file is part of the Adblock Plus web scripts, 3 # This file is part of the Adblock Plus web scripts,
4 # Copyright (C) 2006-2015 Eyeo GmbH 4 # Copyright (C) 2006-2015 Eyeo GmbH
5 # 5 #
6 # Adblock Plus is free software: you can redistribute it and/or modify 6 # Adblock Plus is free software: you can redistribute it and/or modify
7 # it under the terms of the GNU General Public License version 3 as 7 # it under the terms of the GNU General Public License version 3 as
8 # published by the Free Software Foundation. 8 # published by the Free Software Foundation.
9 # 9 #
10 # Adblock Plus is distributed in the hope that it will be useful, 10 # Adblock Plus is distributed in the hope that it will be useful,
11 # but WITHOUT ANY WARRANTY; without even the implied warranty of 11 # but WITHOUT ANY WARRANTY; without even the implied warranty of
12 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 12 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13 # GNU General Public License for more details. 13 # GNU General Public License for more details.
14 # 14 #
15 # You should have received a copy of the GNU General Public License 15 # You should have received a copy of the GNU General Public License
16 # along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. 16 # along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
17 17
18 import copy
18 import json 19 import json
19 import random 20 import random
20 import time 21 import time
21 from urlparse import parse_qs 22 import urlparse
22 23
23 from sitescripts.notifications.parser import load_notifications 24 from sitescripts.notifications.parser import load_notifications
24 from sitescripts.web import url_handler 25 from sitescripts.web import url_handler
25 26
27 def _determine_groups(version, notifications):
28 version_groups = dict(x.split("/") for x in version.split("-")[1:]
29 if x.count("/") == 1)
30 groups = []
31 for notification in notifications:
32 if "variants" not in notification:
33 continue
34 group_id = notification["id"]
35 if group_id in version_groups:
36 groups.append({"id": group_id, "variant": int(version_groups[group_id])})
37 return groups
38
26 def _assign_groups(notifications): 39 def _assign_groups(notifications):
27 groups = [] 40 groups = []
28 selection = random.random() 41 selection = random.random()
29 base = 0 42 start = 0
30 for notification in (x for x in notifications if "variants" in x): 43 for notification in notifications:
Sebastian Noack 2015/04/20 14:44:39 Nit: Just continue the loop instead creating a gen
Felix Dahlke 2015/04/23 22:15:18 Done.
44 if "variants" not in notification:
45 continue
31 group = {"id": notification["id"], "variant": 0} 46 group = {"id": notification["id"], "variant": 0}
32 groups.append(group) 47 groups.append(group)
33 for i, variant in enumerate(notification["variants"]): 48 for i, variant in enumerate(notification["variants"]):
34 if group["variant"] != 0: 49 sample_size = variant["sample"]
50 end = start + sample_size
51 selected = sample_size > 0 and start <= selection <= end
52 start = end
53 if selected:
54 group["variant"] = i + 1
35 break 55 break
Sebastian Noack 2015/04/20 14:44:39 Please move the break below where you set variant
Felix Dahlke 2015/04/23 22:15:18 Done.
36 sample_size = variant["sample"]
37 if sample_size > 0 and base <= selection <= base + sample_size:
Sebastian Noack 2015/04/20 14:44:39 Nit: The logic of adding base + sample_size is dup
Felix Dahlke 2015/04/23 22:15:18 Done.
38 group["variant"] = i + 1
39 base += sample_size
40 return groups 56 return groups
41 57
42 def _create_response(notifications, groups): 58 def _get_active_variant(notifications, groups):
43 response = {
44 "version": time.strftime("%Y%m%d%H%M", time.gmtime())
45 }
46 for group in groups: 59 for group in groups:
47 group_id = group["id"] 60 group_id = group["id"]
48 variant = group["variant"] 61 variant = group["variant"]
49 response["version"] += "-%s/%s" % (group_id, variant) 62 if variant == 0:
50 if variant > 0: 63 continue
51 notification = next(x for x in notifications if x["id"] == group_id) 64 notification = next(x for x in notifications if x["id"] == group_id)
52 notification = notification.copy() 65 notification = copy.deepcopy(notification)
Sebastian Noack 2015/04/20 14:44:39 Note that this will be a shadow copy. To create de
Felix Dahlke 2015/04/23 22:15:18 Done.
53 notification.update(notification["variants"][variant - 1]) 66 notification.update(notification["variants"][variant - 1])
54 for key_to_remove in ("sample", "variants"): 67 for key_to_remove in ("sample", "variants"):
55 notification.pop(key_to_remove, None) 68 notification.pop(key_to_remove, None)
Sebastian Noack 2015/04/20 14:44:39 I don't mind, but if you prefer to merge the dicts
Felix Dahlke 2015/04/23 22:15:18 I know I specifically asked for a nicer way - but
56 response["notifications"] = [notification] 69 return notification
57 if "notifications" not in response: 70
58 response["notifications"] = [x for x in notifications 71 def _generate_version(groups):
59 if "variants" not in x] 72 version = time.strftime("%Y%m%d%H%M", time.gmtime())
73 for group in groups:
74 version += "-%s/%s" % (group["id"], group["variant"])
75 return version
76
77 def _create_response(notifications, groups):
78 active_variant = _get_active_variant(notifications, groups)
79 if active_variant:
80 notifications = [active_variant]
81 else:
82 notifications = [x for x in notifications if "variants" not in x]
83 response = {
84 "version": _generate_version(groups),
85 "notifications": notifications
86 }
60 return json.dumps(response, ensure_ascii=False, indent=2, 87 return json.dumps(response, ensure_ascii=False, indent=2,
61 separators=(",", ": "), sort_keys=True) 88 separators=(",", ": "), sort_keys=True)
62 89
63 @url_handler("/notification.json") 90 @url_handler("/notification.json")
64 def notification(environ, start_response): 91 def notification(environ, start_response):
65 params = parse_qs(environ.get("QUERY_STRING", "")) 92 params = urlparse.parse_qs(environ.get("QUERY_STRING", ""))
66 version = params.get("lastVersion", [""])[0] 93 version = params.get("lastVersion", [""])[0]
67 current_groups = dict(x.split("/") for x in version.split("-")[1:]
Felix Dahlke 2015/04/23 22:15:18 Done.
68 if "/" in x)
69 notifications = load_notifications() 94 notifications = load_notifications()
70 groups = [] 95 groups = _determine_groups(version, notifications)
71 for notification in (x for x in notifications if "variants" in x):
Sebastian Noack 2015/04/20 14:44:39 See above.
Felix Dahlke 2015/04/23 22:15:18 Done.
72 group_id = notification["id"]
73 if group_id in current_groups:
74 groups.append({"id": group_id, "variant": int(current_groups[group_id])})
75 if not groups: 96 if not groups:
76 groups = _assign_groups(notifications) 97 groups = _assign_groups(notifications)
77 response = _create_response(notifications, groups) 98 response = _create_response(notifications, groups)
78 start_response("200 OK", [("Content-Type", "application/json")]) 99 start_response("200 OK",
Sebastian Noack 2015/04/20 14:44:39 Please send response charset.
Felix Dahlke 2015/04/23 22:15:18 UTF-8 is default for JSON. However, as discussed w
100 [("Content-Type", "application/json; charset=utf-8")])
79 return response.encode("utf-8") 101 return response.encode("utf-8")
LEFTRIGHT

Powered by Google App Engine
This is Rietveld